👍 theStack approved a pull request: "rpc: Make v2transport default for addnode RPC when enabled"
(https://github.com/bitcoin/bitcoin/pull/29239#pullrequestreview-1818732808)
Code-review ACK 3ba815b42db74804e341ce15f648c2b297af55ca

💬 jamesob commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889725982)
I've reverted to the dumb fix that will unbreak CI (but still add @Sjors' useful coverage for non-Windows platforms). If anyone can come up with something smarter that actually works, I'll be happy to review.

📝 instagibbs opened a pull request: "Mempool util: Add RBF diagram checks for single chunks against clusters of size 2"
(https://github.com/bitcoin/bitcoin/pull/29242)
This is a smaller piece of https://github.com/bitcoin/bitcoin/pull/28984 broken off for easier review.

Up to date explanation of diagram checks are here: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393

This infrastructure has two near term applications prior to cluster mempool:
1) Limited Package RBF(https://github.com/bitcoin/bitcoin/pull/28984): We want to allow package RBF only when we know it improves the mempool. This narrowly scoped functionality allows
...

💬 alfonsoromanz commented on pull request "doc: Add missing backtick in developer notes logging section":
(https://github.com/bitcoin/bitcoin/pull/29241#issuecomment-1889740992)
ACK c003562120c193c296ac754b4bb8cff02bbbe4dc

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#issuecomment-1889742216)
broke off all the incentive compatibility check stuff into its own PR: https://github.com/bitcoin/bitcoin/pull/29242

Putting this in draft for now to divert attention there

📝 instagibbs converted_to_draft a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984)
Allows any 2 transaction package with no in-mempool ancestors to do package RBF when directly conflicting with other mempool clusters of size two or less.


Proposed validation steps:
1) If the transaction package is of size 1, legacy rbf rules apply.
2) Otherwise the transaction package consists of a (parent, child) pair with no other in-mempool ancestors (or descendants, obviously), so it is also going to create a cluster of size 2). If larger, fail.
3) The package rbf may not evict more
...

✅ brunoerg closed a pull request: "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound"
(https://github.com/bitcoin/bitcoin/pull/26441)

💬 brunoerg commented on pull request "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound":
(https://github.com/bitcoin/bitcoin/pull/26441#issuecomment-1889742666)
Closing it to focus on other approach.

🤔 jonatack reviewed a pull request: "rpc: Make v2transport default for addnode RPC when enabled"
(https://github.com/bitcoin/bitcoin/pull/29239#pullrequestreview-1818764745)
Approach ACK

💬 jonatack commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450757056)
Test coverage for this conditional logic and error? Haven't looked deeply but didn't see it in https://github.com/bitcoin/bitcoin/pull/24748 @stratospher.

💬 jonatack commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450777060)
Perhaps good to disambiguate the config option from the rpc option.
```suggestion
{"v2transport", RPCArg::Type::BOOL, RPCArg::DefaultHint{"set by -v2transport configuration option"}, "Attempt to connect using BIP324 v2 transport protocol (ignored for 'remove' command)"},
```

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450778587)
@jamesob I think you can just close the wallet after this call?

💬 jonatack commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450782082)
fwiw for the v2transport config option help

current
```
-v2transport
Support v2 transport (default: 0)
```

suggestion
```
-v2transport
Support BIP324 v2 transport (default: 0)
```
or
```
-v2transport
Support BIP324 v2 transport protocol (default: 0)
```

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450784477)
From reading the test again (it's a been a while) I don't think we care about this loaded wallet `w` anymore.

💬 Sjors commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889752866)
Left an alternative suggestion here that (hopefully) doesn't require an exception for Windows: https://github.com/bitcoin/bitcoin/pull/28838/files#r1450778587

💬 achow101 commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450785203)
I don't think the problem is that the original wallet is open. The error is the path for the restored wallet on node1, not the original on node0.

💬 Sjors commented on pull request "rpc: add path to gethdkey":
(https://github.com/bitcoin/bitcoin/pull/22341#issuecomment-1889756175)
Given that #26728 was closed, I'll just modify this PR to build on top of #29130.

(bit busy with stratum v2 stuff, but it will happen)

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450789885)
I see (just saw the log in #29234).

Sounds like the node that made the backup holds on to it after writing? Maybe restart the node? Or copy the backup file?

💬 jamesob commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889762142)
> Left an alternative suggestion here that (hopefully) doesn't require an exception for Windows: https://github.com/bitcoin/bitcoin/pull/28838/files#r1450778587

Sadly (per the thread), not so simple. I think we should probably unbreak and then reassess but I'll leave that as a call for the maintainers.

💬 Sjors commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889763775)
Skipping Windows for now is fine by me. Best just leave #29234 open until it's more properly solved.