💬 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

💬 stringintech commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#discussion_r2091754628)
Thank you for the clarification.

✅ pinheadmz closed an issue: "Open Alias support?"
(https://github.com/bitcoin/bitcoin/issues/32284)

💬 pinheadmz commented on issue "Open Alias support?":
(https://github.com/bitcoin/bitcoin/issues/32284#issuecomment-2884725663)
Closing this as not planned for now, thanks.

🤔 w0xlt reviewed a pull request: "wallet: Use `util::Error` throughout `AddWalletDescriptor` instead of returning `nullptr` for some errors"
(https://github.com/bitcoin/bitcoin/pull/32475#pullrequestreview-2844660917)
ACK https://github.com/bitcoin/bitcoin/pull/32475/commits/f17945b347f6a46dee3b56f86a557eaccec1bc72

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2091162026)
nit: could add a comment where this constant is coming from (https://github.com/bitcoin/bips/blob/master/bip-0328.mediawiki#cite_note-1)

💬 theStack commented on pull request "descriptors: MuSig2":
(https://github.com/bitcoin/bitcoin/pull/31244#discussion_r2091770520)
note for other reviewers that might wonder: these two functions and the involved `secp256k1_musig_keyagg_cache` instance are not publicly used within this PR yet, but they will be in #29675 (or any other future PRs that get split up from that one)

💬 pinheadmz commented on issue "Migrate from BTC/kvB to sat/vB on RPC and startup options":
(https://github.com/bitcoin/bitcoin/issues/32093#issuecomment-2884755244)
Don't forget that `fundrawtransaction` accepts two different units, depending on snake case / camel case (!)

```
"fee_rate": amount, Specify fee rate in sat/vB.
"feeRate": amount, Specify fee rate in BTC/kvB.
```

I wonder if we could even handle complete strings so users can just do whatever they need, i.e. correctly parse strings like `100s/B` vs `0.00001 BTC/kb`

👍 willcl-ark approved a pull request: "[28.x] 28.2rc1"
(https://github.com/bitcoin/bitcoin/pull/32480#pullrequestreview-2844686716)
ACK 186e3f1fb65bb693154c0b63b690bea1d279f089

`git grep v28` doesn't show any un-bumped versions.

✅ pinheadmz closed an issue: "TSan warning with legacy wallet on macos ("too long mutex cycle found")"
(https://github.com/bitcoin/bitcoin/issues/31986)

💬 pinheadmz commented on issue "TSan warning with legacy wallet on macos ("too long mutex cycle found")":
(https://github.com/bitcoin/bitcoin/issues/31986#issuecomment-2884764494)
Fixed by removing legacy wallet https://github.com/bitcoin/bitcoin/pull/28710