💬 pinheadmz commented on issue "Bitcoin ignores datadir and blocksdir parameter in .conf":
(https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1570789627)
I think this can be closed as solved by https://github.com/bitcoin/bitcoin/pull/27302 ?

💬 pinheadmz commented on issue "Relative paths named in the -conf parameter reset when parsing datadir in named config":
(https://github.com/bitcoin/bitcoin/issues/19990#issuecomment-1570790910)
Closed by https://github.com/bitcoin/bitcoin/pull/27302

✅ pinheadmz closed an issue: "Relative paths named in the -conf parameter reset when parsing datadir in named config"
(https://github.com/bitcoin/bitcoin/issues/19990)

💬 jamesob commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#issuecomment-1570801767)
Rebased

💬 achow101 commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212219391)
Done

💬 achow101 commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212219741)
Adopted these suggestions. I've put the bug fixes in an separate commit.

💬 achow101 commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212220552)
Hmm, indeed. I've changed the test to check against the `i` stored in the key.

🤔 mzumsande reviewed a pull request: "addrman: select addresses by network follow-up"
(https://github.com/bitcoin/bitcoin/pull/27745#pullrequestreview-1454032733)
Code Review ACK cd8ef5b3e66b3f766c9c883259b5feb44540d7df

💬 mzumsande commented on pull request "addrman: select addresses by network follow-up":
(https://github.com/bitcoin/bitcoin/pull/27745#discussion_r1212228531)
I guess if the `Assume` assumption breaks the worst thing that could happen is that the while loop may run indefinitely - but it might also pick another address if there is one. So this seems better than crash with an assert.

💬 brunoerg commented on pull request "wallet: clarify replace fields in gettransaction":
(https://github.com/bitcoin/bitcoin/pull/27782#issuecomment-1570940892)
I suppose that changing them in `TransactionDescriptionString` will also affect `listsinceblock` and `listtransactions`. So, I think would be appropriate to update the PR title/commit message?

🤔 brunoerg reviewed a pull request: "wallet: clarify replace fields in gettransaction"
(https://github.com/bitcoin/bitcoin/pull/27782#pullrequestreview-1454150449)
Concept ACK

💬 Macoophonic commented on pull request "wallet: clarify replace fields in gettransaction":
(https://github.com/bitcoin/bitcoin/pull/27782#issuecomment-1570963364)
Wallet code It's so dark, thank you
Được gửi từ Yahoo Mail trên Android

Vào Th 5, 1 thg 6, 2023 lúc 4:00, Bruno ***@***.***> đã viết:


@brunoerg commented on this pull request.

Concept ACK

—
Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

👍 ryanofsky approved a pull request: "walletdb: Add PrefixCursor"
(https://github.com/bitcoin/bitcoin/pull/27790#pullrequestreview-1454166240)
Code review ACK 13476fe7bebdbf51e09821850b2c808c8ecf116a

💬 ryanofsky commented on pull request "walletdb: Add PrefixCursor":
(https://github.com/bitcoin/bitcoin/pull/27790#discussion_r1212315030)
In commit "wallet: Add GetPrefixCursor to DatabaseBatch" (13476fe7bebdbf51e09821850b2c808c8ecf116a)

Maybe add a comment above the batch.Write() call like "Convert the key and value to DataStream objects in order to bypass serialization. We want raw bytes to be written to the database, not serialized byte strings. The DatabaseBatch::Write template method normally serializes its arguments, but because DataStream has a Serialize method that does concatenation instead of serialization, it can be
...

💬 ryanofsky commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#issuecomment-1570974530)
> I wasn't sure if you were going to add the named-only parameters to the conversion table

Sorry, I just missed the part of your earlier comment where you suggested to this. I didn't realize the conversion table could handle parameters not passed by position. Should be fixed now though.

Updated 2808c33bae95a0d2516a6e9a550032af142a04de -> 5559ad2c69ef82c18843b9214e5ba3974834a1ae ([`pr/nonly.15`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.15) -> [`pr/nonly.16`](https://github.com/
...

💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1212354487)
Thanks, I got a slightly different result -- `malloc_usable_size() + sizeof(void*)` is perfect (note adding only `sizeof(void*)` instead of 2 times that). I'm not set up for 32-bit, can you try running this test program on 32-bit if you are able to easily?

Whichever is the most accurate, would a PR to improve [`memusage.h: MallocUsage()`](https://github.com/bitcoin/bitcoin/blob/6cf47a8f44f96099a84e5c6e628e3499045e024d/src/memusage.h#L50) be worth doing? (I would say a separate PR would be bet
...

📝 theuni opened a pull request: "depends: modernize clang flags for Darwin"
(https://github.com/bitcoin/bitcoin/pull/27798)
This is a cleaner and simpler alternative to #25098. Inspired by [this conversation](https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1562543301). The diff is large but the change itself is quite small.

Fixes builds with llvm >= 11 in guix by working around the problem. As a bonus, this is much cleaner and more maintainable than what we had before.

See the updated comment for more info. At a high level: rather than playing tricks and trying to work around clang's default includes
...

💬 theuni commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1571040493)
> > just need to decide out what to do about guix.
>
> What options are currently being considered. Is an upstream patch still possible?

See #27798 for an alternative that is imo better in every way :)

💬 mzumsande commented on pull request "p2p, rpc, test: `getpeerinfo` improv + add test coverage for invalid command":
(https://github.com/bitcoin/bitcoin/pull/26366#discussion_r1212373107)
The title and commit messages don't fit: This PR changes concerns `addnode`, not `getpeerinfo`.

💬 hebasto commented on pull request "depends: modernize clang flags for Darwin":
(https://github.com/bitcoin/bitcoin/pull/27798#issuecomment-1571046189)
Concept ACK.