👍 theStack approved a pull request: "test: Treat msg_version.relay as unsigned, Remove `struct` packing in messages.py"
(https://github.com/bitcoin/bitcoin/pull/29067#pullrequestreview-1807941957)
Code-review ACK fa3b4dbf45b2b7ff1bdac6f68cf23275102cc775

👍 kristapsk approved a pull request: "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo"
(https://github.com/bitcoin/bitcoin/pull/29058#pullrequestreview-1807944717)
ACK fb5bfed26a564014b83ccfc96ff00b630930fc61

💬 jonatack commented on pull request "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption":
(https://github.com/bitcoin/bitcoin/pull/29200#issuecomment-1880308961)

💬 ajtowns commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880309647)
Not an objection, but this feels backwards to me: presumably the user has manually listed those peers as addnode/connect because it's important that their node be connected to those peers, but if there are any bugs in the "v2 didn't work, retry as v1" logic, it's exactly those peers that will be inaccessible. But if we're confident there aren't bugs in that logic, why not just apply it to all peers? The "if you don't like it, just set -v2transport=0" approach to avoiding bugs/risk works just as
...

📝 jonatack opened a pull request: "Remove no-longer-needed NOLINTNEXTLINE since C++20 upgrade"
(https://github.com/bitcoin/bitcoin/pull/29202)
This was a temporary workaround for C++17 in https://github.com/bitcoin/bitcoin/pull/28583 and should no longer be needed for the CI tidy task now that the project has moved to C++20.

💬 mzumsande commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1880319951)
> why not just apply it to all peers

Do you mean why not apply it to automatic peers as well per default (and ignoring the information from the service flag)? I thought the idea was to save one disconnection / reconnection, and the time spent doing these when we probably know better. But as I wrote in https://github.com/bitcoin/bitcoin/pull/29058#issuecomment-1854197471, I think that we depend on the reconnection mechanism for automatic connection as well, because anyone can add a wrong "v2"
...

💬 kevkevinpal commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1880357066)
Concept ACK [2422b90](https://github.com/bitcoin/bitcoin/pull/29156/commits/2422b90978f4ea13ee49954598dc8aef841e36df)

I've done a little code review and so far it is looking good, I think it would be cool if instead of always using the pubkeys in the same order when decaying we could randomly use different signers, I just quickly tested it worked the other way around with this diff (not sure how you feel about adding this randomness)

```
for m in range(self.M):
-
...

✅ jonatack closed a pull request: "refactor: use push_back in TaprootBuilder::Add(), rm NOLINTNEXTLINE and comment"
(https://github.com/bitcoin/bitcoin/pull/29202)

💬 totient commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1880361556)
> > It's meant to limit extra data in transactions. OP_RETURN was supposed to be the only tolerated way to do that.
>
> Can you add any references for this? because the GitHub and git history very clearly contradicts your statements ([#29187 (comment)](https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1878972398)).

"There was been some confusion and misunderstanding in the community, regarding the OP_RETURN feature in 0.9 and data in the blockchain. This change is not an endorse
...

🤔 BrandonOdiwuor reviewed a pull request: "refactor(tidy): Use C++20 contains method"
(https://github.com/bitcoin/bitcoin/pull/29191#pullrequestreview-1808113803)
Concept ACK using ::contains(Key& key) (C++20) to check for elements

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#issuecomment-1880584066)
> Pre-review note: have you considered that wallet_import_rescan.py is a legacy wallet only test?

Ah good point. I added a commit to run this test with descriptors as I couldn't find a reason why we don't do this. Lmk if this seems sufficient / ok to do?

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359148)
taken

👍 brunoerg approved a pull request: "net: create I2P sessions using both ECIES-X25519 and ElGamal encryption"
(https://github.com/bitcoin/bitcoin/pull/29200#pullrequestreview-1808534184)
crACK 9d728916b27e18efc6f8839770ed5ec14789fc08

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359693)
I've added an `Assume` checking that there is a v3 tx in the package and exiting early if not

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359794)
taken

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444359931)
taken

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360019)
fixed

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360107)
taken, thanks

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360587)
because otherwise it is always 0

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1444360834)
taken