💬 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.

💬 sr-gi commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1465595068)
Oh, I thought the `recvbuf` was a growing-only buffer that was not cleared until the handshake was over.

💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908992212)
> but I'm guessing that since [the utxoset has doubled in size over the last year](https://statoshi.info/d/000000009/unspent-transaction-output-set?orgId=1&refresh=10m&from=now-1y&to=now&viewPanel=8), these devices too will soon start to become unviable.

UTXO set has not doubled because of bare multisig: https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666471945

💬 Retropex commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1908994332)
> UTXO set has not doubled because of bare multisig

we know it, unfortunately no mempool option for inscriptions is available at the moment, so even if Baremultisig is less used it remains spam for my node.

💬 furszy commented on pull request "init: settings, do not load auto-generated warning msg":
(https://github.com/bitcoin/bitcoin/pull/29301#discussion_r1465610725)
> In commit "init: settings, do not load auto-generated warning msg" ([987a1b5](https://github.com/bitcoin/bitcoin/commit/987a1b51eeb72c7fcb085470817a4fe85fcc3c7c))
>
> Not sure, but maybe this can be changed to std::string_view to avoid the need to allocate memory before main() runs.

As the settings map take strings for keys, wouldn't this change mean that we would need to static cast string_view to string to not create a copy during the push/erase methods call?

💬 chrisguida commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909013326)
> > but I'm guessing that since [the utxoset has doubled in size over the last year](https://statoshi.info/d/000000009/unspent-transaction-output-set?orgId=1&refresh=10m&from=now-1y&to=now&viewPanel=8), these devices too will soon start to become unviable.
>
> UTXO set has not doubled because of bare multisig: [#28217 (comment)](https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666471945)

Yes, I know. Just because there isn't a perfect solution for all utxoset abuse doesn't mean
...

👍 TheCharlatan approved a pull request: "set `DEFAULT_PERMIT_BAREMULTISIG` to false"
(https://github.com/bitcoin/bitcoin/pull/28217#pullrequestreview-1842509290)
ACK 8672721deb06e66854a085c9994e99c99160b8a1

Close to the only current usage of P2MS is data embedding and meta protocols, of which the majority is through the Counterparty protocol. I am not familiar with the Counterparty protocol, so I might have gotten some points here wrong. The [Bitcoin Stamps](https://github.com/mikeinspace/stamps/blob/main/BitcoinStamps.md), which are mentioned throughout this pull request discussion, are built with Counterparty transactions. P2MS is one of three metho
...

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

Yes. For further node support, please come to IRC or use bitcoin.stackexchange. This is a project tracking issue.

💬 DoctorBuzz1 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909020625)
> UTXO set has not doubled because of bare multisig: [#28217 (comment)](https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1666471945)

UTXO has rapidly increased over the past year. It *started* with Inscriptions but is now primarily due to BRC-20 tokens. A static dust limit of 3 sat /vByte fees makes zero sense anymore, when those fees have only been seen like for a hot second since the spam started a year ago... https://statoshi.info/d/000000009/unspent-transaction-output-set?orgI
...

💬 ns-xvrn commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909047847)
ACK 8672721deb06e66854a085c9994e99c99160b8a1

Tested locally, all unit and functional tests pass.

Transaction tested on mainnet using txid: `883b35fbd5a8b703574752b879a027ef81ae091b0bc61527309b0cf53760854f` (which worked fine without this change and confirmed using `getmempoolentry` and `getrawtransaction` after doing `sendrawtransaction` while it was still in the mempool). Output of `sendrawtransaction` after compiling and running this PR:
```
error code: -26
error message:
bare-multi
...

💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1909048780)



Since DrahtBot has incorrectly assumed one of my previous comment as ` A-C-K`, this comment is to correct it:

NACK

Reason: https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1860915662

Apart from the reason shared in linked comment, other reasons why this pull request makes no sense:

1. Some nodes will keep using older versions of core and this is enough to relay bare mul
...

👍 ryanofsky approved a pull request: "validation: improve checkblockindex comments"
(https://github.com/bitcoin/bitcoin/pull/29299#pullrequestreview-1842689957)
Code review ACK 9819db4ccaa03519a78d4d9ecce9f89f5be669e5. Thanks for figuring this issue out and fixing it. Would suggest change pr name from "improve comments" to "fix misleading comments" since previous comments were wrong about the reasons the conditions are needed.

I also think it would be good to split up the assert and make it stricter, so feel free to use the code from https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1908513362 to use in the second commit, if that help. (Wo
...