💬 m3dwards commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889628006)
I'm struggling to exactly replicate this locally. I get a failure and on the same line but my error seems to be the assert failing that expects an exception:

```
2024-01-12T16:34:39.449000Z TestFramework (INFO): Backup can't be loaded during background sync
2024-01-12T16:34:39.496000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "C:\Users\Max\source\bitcoin\test\functional\test_framework\util.py", line 140, in try_rpc
fun(*args, **kwds)
File "C
...

💬 achow101 commented on issue "ci: failure in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1889634306)
> Could this be a bug in the wallet?

Indeed, this looks more like a windows related bug when there's a wallet restoring failure.

> Why would it call `remove_all` on the wallet file of a completely different node?

I don't think it is. I think the problem is more along the lines of the same bitcoind has the database open at the time it tries to cleanup the failed restore.

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1889635234)
I pulled #29032 into this branch to remove the need for branch-switching while testing on a custom signet.

💬 kristapsk commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1889643297)
Concept ACK

💬 achow101 commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#issuecomment-1889645425)
I don't think this fixes the problem.

💬 Eunovo commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1889653618)
I tested this locally and I've observed that the test doesn't fail if you modify the external wallet's timelocks, see below:
`external = multisig.getdescriptorinfo(f"wsh(thresh({self.N},pk({f'),s:pk('.join(external_xpubs)}),sln:after(128)))")`
The reason is that the test only sends from the external wallet once using all keys and the transactions that use the `locktimes` send from the internal wallet because that's where the funds are. Hence, changing the timelocks for the external descriptor
...

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

👍 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)
```