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

💬 achow101 commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450792644)
> Sounds like the node that made the backup holds on to it after writing? Maybe restart the node? Or copy the backup file?

We don't remove the backup. The failure is in cleaning up the restored wallet after it fails to restore. The backup file is copied to a new path, and that is what `remove_all` is trying to remove.

I think it's more likely that the database isn't fully closed yet even though the `CWallet` should be destructed.

💬 achow101 commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#discussion_r1450795240)
I think it would be better if we used `skip_if_no_module` so that the test is reported as skipped instead of silently "passing" on windows. Could also use `skip_if_platform_not_posix` for that.

💬 Sjors commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450796060)
Ok, so it has nothing to do with what n0 did. I'm surprised none of the other wallet backup & restore tests have this issue. But I guess it's a different failure condition.

💬 jamesob commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#discussion_r1450796816)
Oh, good call. Will do.

💬 achow101 commented on pull request "test: add assumeutxo wallet test":
(https://github.com/bitcoin/bitcoin/pull/28838#discussion_r1450797858)
It looks like we don't have any tests that have a failure in `CWallet::Create`, which I don't think is that surprising since generally the wallet needs to be corrupted for that to happen.