👍 rkrux approved a pull request: "walletdb: Log additional exception error messages for corrupted wallets"
(https://github.com/bitcoin/bitcoin/pull/32598#pullrequestreview-2866176097)
ACK ad9a13fc424e9deb262e2b1d54bcdc7370263ea0

📝 koyaness opened a pull request: "Small improvement to the documentation"
(https://github.com/bitcoin/bitcoin/pull/32609)
Small improvement to the documentation to improve readability and understanding.

💬 vasild commented on pull request "net: replace manual reference counting of CNode with shared_ptr":
(https://github.com/bitcoin/bitcoin/pull/32015#issuecomment-2906544041)
> `CNode` was supposed to become internal and not passed into `PeerManager` at all

Do you need any help with that?

💬 rkrux commented on pull request "wallet, rpc: clarify wallet version in getwalletinfo help":
(https://github.com/bitcoin/bitcoin/pull/32603#discussion_r2105749398)
Makes sense, done.

💬 spikeyrock commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2906624037)
maybe spin up two regtest nodes, mine a fork on one, and then connect them at just the right time to trigger the reorg. That way, the reorg happens "naturally" through P2P sync rather than forcing it.

🤔 rkrux reviewed a pull request: "rpc: Note in fundrawtransaction doc, fee rate is for package"
(https://github.com/bitcoin/bitcoin/pull/32607#pullrequestreview-2866228840)
ACK fe0432b1c4a10b74844c2dedefccfe340c0d3b10

This is a good note, helpful. I verified in the code that the outputs selected in the transaction have a corresponding `ancestor_bump_fees` param that is used in calculating the `TotalBumpFees` inside `CreateTransactionInternal` function.

https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/src/wallet/coinselection.h#L75-L76

https://github.com/bitcoin/bitcoin/blob/638a4c0bd8b53766faeb437244b2aae4eed28dcf/src/wallet
...

💬 rkrux commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2105759450)
I see it needs a full stop after `watch-only`.

💬 rkrux commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#discussion_r2105759505)
Nit:

```diff
- not the resulting transaction.",
+ not only the resulting transaction.",
```

💬 zaidmstrr commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r2105780723)
This new PR https://github.com/bitcoin/bitcoin/pull/32438/commits/fa62a013a558338dc6ee5fb4cfd6fc7c782c301b removes this `flush()` from the codebase. I think it should also be removed from `chain.capnp` otherwise it will give build errors.

🤔 naiyoma reviewed a pull request: "rpc: generatetomany"
(https://github.com/bitcoin/bitcoin/pull/32468#pullrequestreview-2866255285)
I'm a bit confused about the current approach. Are you planning to add `generatetomany` and also extend `generateblock` to accommodate an array of addresses?

I suggest that you put one approach in your fork and indicate this clearly in your description—separation of concerns makes it easier to follow and review.

For example:
Approach 1
Approach 2 → Link to the PR in your fork

💬 polespinasa commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2906764458)
> I'm a bit confused about the current approach. Are you planning to add `generatetomany` and also extend `generateblock` to accommodate an array of addresses?

`generatetomany` will not be added. I have to delete that code.

💬 maflcko commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#issuecomment-2906765296)
> `generatetomany` will not be added. I have to delete that code.

Generally, it is best to just push the final version of the code, so that it is ready for further review. Prior versions of the code can trivially be retrieved via the commit hash (or a named alias), if needed.

💬 naiyoma commented on pull request "rpc: generatetomany":
(https://github.com/bitcoin/bitcoin/pull/32468#discussion_r2105783773)
11 shouldn't be str
```diff
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -351,7 +351,7 @@ static RPCHelpMan generatetomany()
}},
RPCExamples{
"\nGenerate 11 blocks to two different addresses:\n"
- + HelpExampleCli("generatetomany", "\"11 '[\\\"bcrt1qal6p633hvwz2yp5mav0qy7u2az8gkn2xywnj6v\\\", \\\"bcrt1qvr3qgyhw6y0e0zj97v0j5yc40xtpea4wqj0g43\\\"]'\"")
+ + HelpExampleCli("generatetomany", "11 '[\"bcrt1qal6p633hvwz2yp5mav0
...

💬 maflcko commented on issue "test: `feature_fee_estimation` failure after duplicate coinbase tx weight reservation fix":
(https://github.com/bitcoin/bitcoin/issues/32461#issuecomment-2906769514)
https://cirrus-ci.com/task/5716739608018944?logs=ci#L3079

💬 maflcko commented on pull request "rpc: Note in fundrawtransaction doc, fee rate is for package":
(https://github.com/bitcoin/bitcoin/pull/32607#issuecomment-2906770023)
> Interesting that the `feature_fee_estimation.py ` test fails in the CI coincidentally but seems unrelated to me as it passed in my system. Maybe a rerun/push would fix it?

See https://github.com/bitcoin/bitcoin/issues/32461

💬 maflcko commented on pull request "rpc, doc: clarify wallet version in getwalletinfo help":
(https://github.com/bitcoin/bitcoin/pull/32603#discussion_r2105786933)
I don't think this is right. The current wallet version is `169900` (for a freshly created descriptor wallet). However, I don't think a Bitcoin Core 16.99 client can open descriptor wallets at all.

Generally, all modules in Bitcoin Core moved away from the client version to use a module-specific and module-internal version. (You can see this when you look at all other files written (fee estimates, mempool, addr ...). Also the p2p version, but that again has been replaced by feature-messages.)
...

💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#discussion_r2105795801)
thx, done because it is less code and the suggestion was fine

💬 maflcko commented on pull request "rpc: Round verificationprogress to 1 for a recent tip":
(https://github.com/bitcoin/bitcoin/pull/32528#issuecomment-2906792097)
> Might warrant a release note: "Verification progress will be 100% if all known blocks are verified and timestamped within the last two hours when previously a stale tip might result in progress < 100%".

I consider the changes here only stylistic (similar to changing the `bitcoin-cli -color=...` colours). No one should be relying on the informational-only value anyway.

💬 maflcko commented on pull request "validation: Do less work in NeedsRedownload":
(https://github.com/bitcoin/bitcoin/pull/31714#issuecomment-2906794331)
Yeah, if we are fine with the rare edge case, it should also be fine to fully remove it, and instead replace the "unsupported utxo db" error message from reindex-chainstate to reindex. Alternatively, it could automatically force a reindex to avoid the user picking the wrong setting.

💬 rkrux commented on pull request "rpc, doc: clarify wallet version in getwalletinfo help":
(https://github.com/bitcoin/bitcoin/pull/32603#discussion_r2105799056)
Hmm I see. Thanks for highlighting. Descriptor wallets were indeed introduced after v16.99.

I'm not sure then what does the "client version" here refers to. Maybe it is referring to a wallet client somehow as mentioned above that versioning has moved to being module specific.

I will dig more to see how this can be made clearer.