👍 ryanofsky approved a pull request: "init: settings, do not load auto-generated warning msg"
(https://github.com/bitcoin/bitcoin/pull/29301#pullrequestreview-1842105713)
Code review ACK 987a1b51eeb72c7fcb085470817a4fe85fcc3c7c. Seems like a clean, simple fix

💬 ryanofsky commented on pull request "init: settings, do not load auto-generated warning msg":
(https://github.com/bitcoin/bitcoin/pull/29301#discussion_r1465379877)
In commit "init: settings, do not load auto-generated warning msg" (987a1b51eeb72c7fcb085470817a4fe85fcc3c7c)

Not sure, but maybe this can be changed to std::string_view to avoid the need to allocate memory before main() runs.

👍 BrandonOdiwuor approved a pull request: "doc: clarify `BroadcastTransaction` comment"
(https://github.com/bitcoin/bitcoin/pull/29308#pullrequestreview-1842137635)
lgtm ACK cca20d94b787d0881a61509445f4827f913e051d

👍 kristapsk approved a pull request: "doc: clarify `BroadcastTransaction` comment"
(https://github.com/bitcoin/bitcoin/pull/29308#pullrequestreview-1842149187)
ACK cca20d94b787d0881a61509445f4827f913e051d

💬 mzumsande commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1465420112)
The python code only supports the "reconnection" use case in one direction: TestNode connects to the P2PConnection with v2, and reconnects with v1 without anything breaking (e.g. timeouts) on the python side. The reverse direction (P2PConnection actively does reconnections itself for outgoing connections) is not supported and I don't think it needs to be. So P2PConnection is receiving an inbound connection, and here it must wait to send `on_connection_send_msg` with sending until the version is
...

💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1908758657)
From some more local tests, it looks like it's the clz changes slowing things down for whatever reason. Still investigating.

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908758998)
tACK 8672721deb06e66854a085c9994e99c99160b8a1

Steps to test:

1) Build and install master, sync to tip (for this test I used 207220ce8b767d8efdb5abf042ecf23d846ded73)
2) Go to mempool.space, click on the block to be confirmed next (get the block before that if you want a bit more time for the testing before it gets confirmed); hover over the block animation and click the goggles icon in the top left of the animation.
3) Select "Bare multisig" and then click any of the highlighted transact
...

💬 ryanofsky commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1465280323)
In commit "[refactor] Make MainSignals RAII styled" (dddaa962cfbeb96a8148535e74e1b53c9c717376)

Maybe unintended diff moving this line

👍 ryanofsky approved a pull request: "kernel: Remove dependency on CScheduler"
(https://github.com/bitcoin/bitcoin/pull/28960#pullrequestreview-1841942054)
Code review ACK bdd985af62ff9e94490ecc701d6063eaab172add. Letting signals be optional and moving more initialization to AppInitMain seem like nice changes among other cleanups.

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908780342)
> > Concept ACK, what will it take to get this merged? Stamps are absolutely ravaging the UTXO set.
>
> what do we mean by ravaging?

@moonsettler low-resource devices tend to struggle with IBD once the utxoset no longer fits in memory. My own experience with this comes from helping users sync Raspberry Pi 4 devices for a couple of years while working at Start9. (The Raspberry Pi is especially bad because the bridge connecting external USB disks to the CPU doesn't work very well.) Once you
...

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1908785694)
@ryanofsky thanks again for thorough review. I addressed all your comments, and rebased on master to fix the "tidy" CI failure. The one commit I am not fully confident in is 50debf781dbc4ac73cd1c0138ed9d405ea60eacb "rpc: use move semantics in JSONRPCReplyObj()" which is the commit I added to address

https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1446816704
and
https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1446847073

👍 TheCharlatan approved a pull request: "refactor: Add util::Result failure values, multiple error and warning messages"
(https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1842217136)
Re-ACK f822ac9a24d684937f1258da89812e99c4b205ba

💬 chrisguida commented on issue "Prune Node Rescan Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/29183#issuecomment-1908843014)
Excellent, thanks!!

💬 chrisguida commented on issue "Prune Node Rescan Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/29183#issuecomment-1908844984)
What's the process for building the blockfilterindex on an already-synced pruned node? Enable blockfilterindex, reindex?

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1465515386)
I updated the function `JSONRPCReplyObj` to expect plain ol' `UniValue` for both response and error, and added `std::move()` where it seemed applicable. Tidy didn't like `std::move()` here because `objError` is already a `const`, so my understanding is that the error message will continue to be passed by reference through the end of `JSONRPCReplyObj` but my understanding is limited

👍 willcl-ark approved a pull request: "doc: clarify `BroadcastTransaction` comment"
(https://github.com/bitcoin/bitcoin/pull/29308#pullrequestreview-1842344725)
ACK cca20d94b787d0881a61509445f4827f913e051d

💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1908900398)
Updated bdd985af62ff9e94490ecc701d6063eaab172add -> 536429372dfd9759dc8f089962f3a15a94b54751 ([noGlobalSignals_10](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_10) -> [noGlobalSignals_11](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_11), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_10..noGlobalSignals_11))

* Addresesd @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1465280323), reverting unintended d
...

💬 TheCharlatan commented on pull request "depends: patch libool out of libnatpmp/miniupnpc":
(https://github.com/bitcoin/bitcoin/pull/29298#discussion_r1465542113)
Can the ` $(package)_build_opts_darwin=LIBTOOL="$($(package)_libtool)"` line above (and same for miniupnpc) be removed now? I removed them and things compiled fine.

💬 morehouse commented on pull request "RBF: Require unconfirmed inputs to come from a single conflicting transaction":
(https://github.com/bitcoin/bitcoin/pull/29297#issuecomment-1908932220)
As another benefit, I think this change helps prevent replacement cycling attacks against LN. The attacker would be unable to make their preimage spend depend on a second unconfirmed transaction, and therefore couldn't evict their preimage spend.

💬 achow101 commented on pull request "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs":
(https://github.com/bitcoin/bitcoin/pull/24128#issuecomment-1908951226)
ACK fa08291375ea00cfacd52956bcac7468824a0bf4

With #25273 now merged, I believe this now applies the anti-fee-sniping consistently. It won't apply it to any transactions that are created by calling `FundTransaction` (that should only affect `fundrawtransaction` and `walletcreatefundedpsbt`) since those transactions always have preset sequences. But that can be improved in the future and this will at least behave consistently.