🤔 furszy reviewed a pull request: "qt, wallet: Convert uint256 to Txid"
(https://github.com/bitcoin/bitcoin/pull/32238#pullrequestreview-2844335248)
Code review ACK 0671d66a8ee07584fab6f1ddaaa188c6a4ac25c1

Happy rebase for everyone.

💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091584150)
I started with this but then playing with the feature I made it optional because if you receive a payment you'd see a tx with a bunch of outputs that all say `ischange: false`. I felt like that was confusing because you didn't create this tx and that might imply that there is some way to detect a change output in someone else's tx.

📝 instagibbs converted_to_draft a pull request: "test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage"
(https://github.com/bitcoin/bitcoin/pull/32516)
`DisconnectedBlockTransactions::LimitMemoryUsage()` has unit test coverage, but the default value end to end doesn't have coverage.

This test adds exercised coverage of memory limiting of the disconnect pool, and some basic behavior sanity checks.

I didn't assert the exact order of pruning, but can if desired.

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#issuecomment-2884446941)
I don't think I'm actually hitting that particular limit, getting weird results when looking closer, draft for now while I look

💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091591988)
Not sure what you mean here -- `help gettransaction` doesn't list this at all, it just tells the user to check `decoderawtransaction`

📝 maflcko opened a pull request: "ci: Enable feature_init and wallet_reorgsrestore in valgrind task"
(https://github.com/bitcoin/bitcoin/pull/32519)
The `fork()` isn't needed and in fact causes issues, so just avoid it and run the valgrind process directly in the CI tasks.

💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091600215)
No, this [doc](https://docs.github.com/en/actions/sharing-automations/creating-actions/setting-exit-codes-for-actions) just says use any exit code other than 0 (which throw does do anyway).

I've tested throw works here: https://github.com/m3dwards/bitcoin/actions/runs/15050358923/job/42303402427

All that said, I'm really not wedded to it and happy to change to a simple `exit 1` if you prefer.

💬 Sjors commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2884478736)
Rebased after #31895.

📝 Sjors converted_to_draft a pull request: "Add bitcoin-{node,gui} to release binaries for IPC"
(https://github.com/bitcoin/bitcoin/pull/31802)
Have depends make libmultiprocess by default. This PR causes the following behavior changes:

1. **bitcoin-node and bitcoin-gui binaries are included in releases**, due to `ENABLE_IPC` option being switched on by default in depends builds
2. `ENABLE_IPC` is also switched on by default in non-depends builds
3. Various changes to CI: switching on `ENABLE_IPC` on in most configurations and using `bitcoin-node` binary for functional tests in two of them.
4. The `bitcoin-node` and `bitcoin-gui`
...

💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091630238)
Hmm, looks like you are right. Though, that doesn't seem very machine readable. It is a bit unrelated, but I can try to fix it, if you don't want to include it here.

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091630858)
I have no preferences either.

💬 theStack commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2091632787)
Not exactly sure if this is what you are asking, but: new blocks with coinbase transaction outputs for MiniWallet can be generated by passing an instance of the latter to `self.generate` (instead of the node object), i.e. in this case:
```
self.generate(wallet, 300)
```
(added +100 to let the coins mature)

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2091634715)
Oh, mine to wallet, not to node which has attached wallet :facepalm:

👍 stickies-v approved a pull request: "refactor: reenable `implicit-integer-sign-change` check for `serialize.h`"
(https://github.com/bitcoin/bitcoin/pull/32296#pullrequestreview-2844465655)
ACK 516f0689b511c09153e4b6b4a58dfedd61c6cda7, nice cleanup!

💬 maflcko commented on issue "test: failure in `mining_basic.py` AssertionError: not(4.656542373906924E-10 == 4.656542373906925E-10)":
(https://github.com/bitcoin/bitcoin/issues/32515#issuecomment-2884556411)
What are the exact steps to reproduce? It passes for me: https://cirrus-ci.com/task/6221957416353792?logs=functional_test#L92

💬 maflcko commented on issue "test: failure in `mining_basic.py` AssertionError: not(4.656542373906924E-10 == 4.656542373906925E-10)":
(https://github.com/bitcoin/bitcoin/issues/32515#issuecomment-2884558586)
Oh, you are saying this only happens under valgrind?

💬 pinheadmz commented on issue ""rpcallowip=" configuration directive doesn't accept RFC4193 addresses":
(https://github.com/bitcoin/bitcoin/issues/32433#issuecomment-2884595969)
The problem here is that the `fc` ipv6 prefix identifies the address as CJDNS and then the subnet becomes invalid:

https://github.com/bitcoin/bitcoin/blob/725c9f7780e0def3d79be151c08a36fe7a9dc59c/src/netaddress.cpp#L924-L930

I'm not sure if allowing `addr.IsCJDNS()` here would break anything else.

💬 pinheadmz commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091688341)
Ill give it a shot in this branch

🤔 janb84 reviewed a pull request: "doc: Add missing top-level description to pruneblockchain RPC"
(https://github.com/bitcoin/bitcoin/pull/32333#pullrequestreview-2844543008)
re ACK [135a0f0](https://github.com/bitcoin/bitcoin/commit/135a0f0aa711b95c50aa4cbe0c38d82d647f1c8b)

Changes since last ACK:
- Textual improvements of description to be more precise.
- Removal of leading new line (/n)

🤔 mzumsande reviewed a pull request: "wallet: Ensure best block matches wallet scan state"
(https://github.com/bitcoin/bitcoin/pull/30221#pullrequestreview-2844554561)
Code Review ACK 30a94b1ab9ae850d55cb9eb606a06890437bc75e