💬 hebasto commented on pull request "build: Fix `macdeployqtplus` after switching to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2060709771)
Thanks! Adjusted.

✅ glozow closed a pull request: "Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling"
(https://github.com/bitcoin/bitcoin/pull/28031)

💬 glozow commented on pull request "Package Relay 1/3: Introduce TxDownloadManager and improve orphan-handling":
(https://github.com/bitcoin/bitcoin/pull/28031#issuecomment-2831177590)
Closing because this is pretty completed (yay!). TxDownloadManager was added in #30110, and the multi-announcer orphan handling was added in #31397. We just need the last 2 commits to make txdownloadman internally thread-safe, but I believe we have somebody working on it.

💬 hebasto commented on pull request "Fix missing error check in `set_clo_on_exec` for FD_CLOEXEC handling":
(https://github.com/bitcoin/bitcoin/pull/32342#issuecomment-2831211867)
Changes look good.

> This should probably be going upstream (https://github.com/arun11299/cpp-subprocess). cc @hebasto.

@tomasandroil

Yes, please consider upstreaming the changes.

💬 JeremyRubin commented on issue "Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in `AddWalletDescriptor`":
(https://github.com/bitcoin/bitcoin/issues/31728#issuecomment-2831271463)
to be fair, i'm pretty sure i dumped my notes on the issue into GPT after encountering it :(

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2060783455)
Removed

💬 achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#issuecomment-2831273073)
> There's also a `libdb++-dev` instance in `workflows/ci.yml` & `deadlock:libdb` / `BerkeleyBatch` etc in tsan suppressions. `Berkeley*` in walletdb.h. `BerkeleyEnvironment::Salvage` in `utils_tests.cpp`.

Removed

💬 amtriorix commented on pull request "wallet: Disable creating and loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/31250#issuecomment-2831298379)
I do disagree with this merge
Are you crazy or what. Backwards compatibility is needed for wallet.dat

💬 jonatack commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2831313159)
ACK after reading the above, the IRC meeting discussion, and @achow101's gist.

🤔 darosior reviewed a pull request: "ipc: Handle unclean shutdowns better"
(https://github.com/bitcoin/bitcoin/pull/32345#pullrequestreview-2795154468)
Concept ACK. Will test using my Rust client for #29409 rebased on top of this.

💬 theuni commented on pull request "net: remove unnecessary check from AlreadyConnectedToAddress()":
(https://github.com/bitcoin/bitcoin/pull/32338#issuecomment-2831335989)
Looking over the old code, I believe it used to be true that `-connect`, `-addnode`, etc. used `m_addr_name` (and a dummy for `addr`) because they may have required a dns lookup. Which would have meant that checking both was required. There was also the case where we were connecting to an address (which may or may not need to be resolved) through a proxy.

These days I believe we're smarter and detect addresses which don't need to be resolved.

I'm pretty sure that `addr` should match `m_add
...

🤔 jonatack reviewed a pull request: "net: remove unnecessary check from AlreadyConnectedToAddress()"
(https://github.com/bitcoin/bitcoin/pull/32338#pullrequestreview-2795181683)
Concept ACK

💬 jonatack commented on pull request "net: remove unnecessary check from AlreadyConnectedToAddress()":
(https://github.com/bitcoin/bitcoin/pull/32338#discussion_r2060824325)
> we'd want to know if that didn't hold for some reason

Wonder if the code should assert they are the same, for debug builds.

💬 l0rinc commented on pull request "test: avoid stack overflow in `FindChallenges` via manual iteration":
(https://github.com/bitcoin/bitcoin/pull/32351#issuecomment-2831353392)
> miniscript_tests (SEGFAULT)

This was the original failure in debug mode

> [Here](https://github.com/hebasto/bitcoin-core-nightly/actions/runs/14667837569/job/41166981753) is a "Windows, Debug" CI job

While the build is till running, this test passed alerady:
```python
89/140 Test # 60: miniscript_tests ..................... Passed 126.23 sec
```

> Should this marked as Fixes https://github.com/bitcoin/bitcoin/issues/32341?

Added it to the description

💬 theuni commented on pull request "net: improve the interface around FindNode() and avoid a recursive mutex lock":
(https://github.com/bitcoin/bitcoin/pull/32326#issuecomment-2831361039)
Is making `m_nodes_mutex` non-recursive a goal? That sounds nice to me and I'm not opposed, but I'd rather not make changes just to get halfway there.

Concept ACK on turning the `FindNode`s into bools, though. That's a nice improvement.

💬 fanquake commented on pull request "build: Fix `macdeployqtplus` after switching to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/32287#discussion_r2060882568)
Same Q here as above.

💬 glozow commented on issue "estimateSmartFee error: "Insufficient data or no feerate found":
(https://github.com/bitcoin/bitcoin/issues/32178#issuecomment-2831440563)
I think there have been a few debates about whether it's safer to return nothing ("insufficient data") vs a potentially bad guess (mempool min could also be artificially low if we're just starting out). But also, lowballing isn't the end of the world since you should be able to replace transactions that didn't pay enough.

💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831451724)
Needed to reorg the commits a bit to get the test-every-commit CI to pass

💬 fjahr commented on pull request "index: Fix coinstats overflow and introduce index versioning":
(https://github.com/bitcoin/bitcoin/pull/30469#issuecomment-2831459829)
I guess this could use a bug label too if someone with permission can get around to that :)

💬 hodlinator commented on pull request "qa: Verify clean shutdown on startup failure":
(https://github.com/bitcoin/bitcoin/pull/30660#discussion_r2060900943)
(Done in latest push from ~8 hours ago).