💬 glozow commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#discussion_r1450477325)
@maflcko knows best, changed.

👍 furszy approved a pull request: "wallet: Fix migration of blank wallets"
(https://github.com/bitcoin/bitcoin/pull/28976#pullrequestreview-1818312971)
reACK c11c404. CI failure is unrelated.

💬 glozow commented on issue "ci: failure in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1889316636)
https://github.com/bitcoin/bitcoin/actions/runs/7503027363/job/20426899651

💬 sipa commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450511206)
Unsure how relevant that is, but I've changed it to drop the 3rd argument whenever it matches the node `-v2transport` setting (I think).

💬 sipa commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450511652)
Nice, done.

📝 GoodDaisy opened a pull request: "Fix typos"
(https://github.com/bitcoin-core/gui/pull/787)
fix: typo in doc/Doxyfile.in
fix: typo in test/functional/wallet_backup.py

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1450515730)
> Why is the approach to add "in" and "out" instead of just having specified permissions apply to both? Are there instances where you want to specify different permissions for the same address depending on the connection direction?

For the same address especifically, idk. But since we can use `-whitelist` to give permissions for "all" nodes (e.g. using `0.0.0.0`), it could be worth to specify the direction.

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450509572)
Seems pretty common to specify both. They are technically different; one says no legacy and the other says need descriptors. I know that's effectively the same thing and we don't have 3 types, but I think it's harmless to keep this.

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450510684)
nice, taken

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450514108)
Keeping as is because I prefer the peace of mind of knowing `tester_wallet` has a mature coinbase.

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450512137)
dropped the extra args

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450511111)
dropped the args

💬 glozow commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450510108)
done

💬 maflcko commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450520489)
```suggestion
bool node_v2transport = connman.GetLocalServices() & NODE_P2P_V2;
```

nit, if you re-touch

💬 glozow commented on pull request "log mempool loading progress":
(https://github.com/bitcoin/bitcoin/pull/29227#issuecomment-1889361221)
CI failure is unrelated, #29234

💬 maflcko commented on pull request "test: unbreak: exclude windows from wallet_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/29238#discussion_r1450528234)
```suggestion
n0.unloadwallet('w') # Required, because on Windows only one process can access a file, when it is written
```

nit: Seems better to run the same test on all platforms, than to run different branches on different platforms, and thus make it harder to debug test failures?

💬 maflcko commented on issue "ci: failure in `wallet_assumeutxo.py --descriptors`":
(https://github.com/bitcoin/bitcoin/issues/29234#issuecomment-1889369430)
Could this be a bug in the wallet? Why would it call `remove_all` on the wallet file of a completely different node?

💬 furszy commented on pull request "test: wallet rescan with reorged parent + IsFromMe child in mempool":
(https://github.com/bitcoin/bitcoin/pull/29179#discussion_r1450532808)
> Keeping as is because I prefer the peace of mind of knowing `tester_wallet` has a mature coinbase.

You shouldn't be concerned about that. Thats how the test framework behaves when `setup_clean_chain` is not set. See [code](https://github.com/bitcoin/bitcoin/blob/3ba8de1b704d590fa4e1975620bd21d830d11666/test/functional/test_framework/test_framework.py#L381). If the wallet wouldn't be having mature coins, the test would be always failing due a lack of funds.

👍 stickies-v approved a pull request: "test: wallet rescan with reorged parent + IsFromMe child in mempool"
(https://github.com/bitcoin/bitcoin/pull/29179#pullrequestreview-1818404985)
re-ACK 9db0d42509e0e223b45238fd8087fc98ef32c091

💬 sipa commented on pull request "rpc: Make v2transport default for addnode RPC when enabled":
(https://github.com/bitcoin/bitcoin/pull/29239#discussion_r1450536603)
Done. I needed to re-touch anyway because apparently `nool` is not a C++ type.