💬 fanquake commented on pull request "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc":
(https://github.com/bitcoin/bitcoin/pull/32336#issuecomment-2827779344)
Backported to 29.x in #32292.

📝 vasild opened a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338)
`CConnman::AlreadyConnectedToAddress()` searches the existent nodes by address or by address-and-port:

```cpp
FindNode(static_cast<CNetAddr>(addr)) || FindNode(addr.ToStringAddrPort())
```

but:

* if there is a match by just the address, then the address-and-port search will not be evaluated and the whole condition will be `true`
* if the there is no node with the same address, then the second search by address-and-port will not find a match either.

The search by address-and-por
...

💬 vasild commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#discussion_r2058610798)
Opened https://github.com/bitcoin/bitcoin/pull/32338 with this single change, dropping the `|| FindNode(addr.ToStringAddrPort())` condition.

👍 laanwj approved a pull request: "Crash fix, disconnect numBlocksChanged() signal during shutdown"
(https://github.com/bitcoin-core/gui/pull/864#pullrequestreview-2791496832)
Code review ACK 0ee5376fb302ebb21fb6192ed708800ee8035eb5
Nice solution, also handles the (hypothethical) case where a client model is replaced with a new one.

💬 theStack commented on issue "intermittent issue in feature_bip68_sequence.py: sequence_value = utxos[j]["confirmations"] IndexError: list index out of range":
(https://github.com/bitcoin/bitcoin/issues/32334#issuecomment-2827953910)
I can reproduce this issue locally by passing the randomseed as shown in the CI logs, i.e.
```
$ ./build/test/functional/feature_bip68_sequence.py --randomseed 6169832640268785903
```
Seems that the number of UTXOs generated in the `test_sequence_lock_confirmed_inputs` sub-test (currently 200) is not enough in some cases. A simple minimal-diff fix might be to simply increase that number in this line:

https://github.com/bitcoin/bitcoin/blob/458720e5e98c6e9103aea1fdfcd39bafc26c27bb/test/functiona
...

🤔 furszy reviewed a pull request: "test: Test that migration automatically repairs corrupted metadata with doubled derivation path"
(https://github.com/bitcoin/bitcoin/pull/29124#pullrequestreview-2791552950)
utACK bfcf5c5ab1fdba46d492aa0872c3d5f9a4a87633

📝 maflcko opened a pull request: "ci: Merge fuzz task for macOS and Windows"
(https://github.com/bitcoin/bitcoin/pull/32339)
Currently macOS and Windows eat 4 CI tasks. This is fine, but not needed and duplicates compilation time and space requirements.

Slim down the tasks into just two to reduce the ccache footprint, as well as total compilation time.

💬 dergoegge commented on pull request "ci: Merge fuzz task for macOS and Windows":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828129019)
* Don't these jobs run in parallel? i.e. how does this save time?
* This requires changing the build type to `Debug`, do we really want that?

💬 pablomartin4btc commented on pull request "wallet: migration, avoid loading legacy wallet after failure when BDB isn't compiled":
(https://github.com/bitcoin/bitcoin/pull/31451#issuecomment-2828134729)
I think there's an issue with this fix which is that when we call the `restorewallet` RPC, before this PR the wallet was loaded and the "`load_on_startup`" argument was set, this is no longer the case unless we pass `load_after_restore=true`. Having said that, if that gets fixed, since we are disabling loading legacy wallets (#31250) if someone calls `restorewallet` RPC with `load_on_startup=true` on a legacy wallet, next time the node or `QT` starts it would fail and the app will be [closed](ht
...

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2058783987)
Upstreamed in https://github.com/arun11299/cpp-subprocess/pull/113.

💬 hebasto commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#discussion_r2058786523)
Upstreamed in https://github.com/arun11299/cpp-subprocess/pull/112.

💬 fanquake commented on pull request "test: Suppress upstream `-Wduplicate-decl-specifier` in bpfcc":
(https://github.com/bitcoin/bitcoin/pull/32336#issuecomment-2828151516)
Backported to 28.x in #32299.

💬 maflcko commented on pull request "ci: Merge fuzz task for macOS and Windows":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828183599)
> Don't these jobs run in parallel? i.e. how does this save time?

The tasks don't run in parallel, when the GHA limit is reached. However this should be rare. The time and space savings come from re-using build artefacts that can be shared between the fuzz binary and other binaries.

It is true that the end-to-end duration of the combined task is now longer, albeit it being less than the sum of the times of the split tasks. I don't think this is a concern, but no strong opinion.

> This r
...

💬 pablomartin4btc commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#issuecomment-2828199316)
If a user tries to restore a legacy wallet (using RPC or QT) setting "load_on_startup" (can't be done on QT but it's being set in the wallet interface code), next time the node or QT starts it will be closed with the error _"... Failed to open database path ... The wallet appears to be a Legacy wallet, please use the wallet migration tool... "_. That case shouldn't be handled here? We shouldn't allow `load_on_startup` on legacy...

(Currently this situation is not happening as a side effect of
...

💬 pablomartin4btc commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2058574120)
nit: something like this just for reference, if you agree ofc...
```suggestion

These wallets, Legacy Wallets, are not longer supported (since v3x), can't be created nor loaded but migrated into SQLite (Descriptor Wallets).

```

💬 pablomartin4btc commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2058386261)
nit: shouldn't this one be removed too?
```suggestion
```

💬 maflcko commented on pull request "doc: document workaround and fallback for macOS fuzzing":
(https://github.com/bitcoin/bitcoin/pull/32084#discussion_r2058845822)
Is this actually an issue that anyone ran into, other than yourself?

The CI is running on Apple Silicon, and it looks fine. Also, I haven't seen others report this issue. So it seems most likely that the problem exists on your machine. It seems fine if you want to work around it without fixing it, but I am not sure if `doc/fuzzing.md` is the right place to document your personal and temporary workarounds?

💬 maflcko commented on pull request "Crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828280254)
I tried 0ee5376fb302ebb21fb6192ed708800ee8035eb5 with my steps to reproduce and still got:

```
==301067== Process terminating with default action of signal 11 (SIGSEGV): dumping core

💬 laanwj commented on pull request "Crash fix, disconnect numBlocksChanged() signal during shutdown":
(https://github.com/bitcoin-core/gui/pull/864#issuecomment-2828299385)
That's really strange. Is there some thread race maybe?

💬 maflcko commented on pull request "ci: Merge fuzz task for macOS and Windows":
(https://github.com/bitcoin/bitcoin/pull/32339#issuecomment-2828303457)
To give some numbers for the macOS task: It passed here in 29min (with an empty ccache), whereas on master, the times were 19min+14min=32min (https://github.com/bitcoin/bitcoin/actions/runs/14638899803/job/41076397151)

⚠️ achow101 opened an issue: "Moving this repo to bitcoin-core"
(https://github.com/bitcoin/bitcoin/issues/32340)
Organizationally, it doesn't make sense for Bitcoin Core to be under the bitcoin GitHub organization. All of our other repos (gui, qa-assets, dev wiki, etc.) are under bitcoin-core, so having just this one repo in a different organization doesn't totally make sense. It's only in bitcoin/ for historical reasons. The bitcoin-core organization was created several years ago for the express purpose of moving this repo there, but we still haven't done it.

Conceptually, it makes sense to complete the
...