💬 ryanofsky commented on pull request "ipc: Handle unclean shutdowns better":
(https://github.com/bitcoin/bitcoin/pull/32345#issuecomment-2831035109)
Updated e8372fa3a606d35146c72fc61663e17c67a4621a -> a56468ab0bf2154159919a2c3feba9974486d10e ([`pr/ipc-stop.1`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.1) -> [`pr/ipc-stop.2`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-stop.2), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-stop.1..pr/ipc-stop.2)) to fix draftbot typos and CI failures, though I could not reproduce the previous releases AppInitMain failure in local CI so it could still be broken

💬 hebasto commented on pull request "[29.x] qt: 29.1 translations update":
(https://github.com/bitcoin/bitcoin/pull/32352#issuecomment-2831070314)
> So I rerun the script and found some more issues and updated Transifex. v29 is not translated 100% yet from what I see, but at least the worst offenders: spam, typos, gross errors should be fixed now.

Thank you! Updated.

👍 hebasto approved a pull request: "Replace stray tfm::format to cerr with qWarning"
(https://github.com/bitcoin-core/gui/pull/868#pullrequestreview-2794899658)
ACK edd46566bd66cea7d7f4116429fe1c11d2187ba2, I have reviewed the code and it looks OK.

💬 Sjors commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2831098171)
Code review ACK a58cb3b1c12c8cb75a87375c50f94c4605bb805d

I'm a bit reluctant to have this merged while BIP54 is in limbo (https://github.com/bitcoin/bips/pull/1800).

Additionally I think it's worth waiting a few weeks (?) to see if someone responds to https://delvingbitcoin.org/t/great-consensus-cleanup-revival/710/85 about the use of `nLockTime` as an extranonce.

Let's add this to the v30 milestone though. Unless by that time someone advocates a different approach, it's better to just
...

🤔 darosior reviewed a pull request: "test: avoid stack overflow in `FindChallenges` via manual iteration"
(https://github.com/bitcoin/bitcoin/pull/32351#pullrequestreview-2794937685)
Concept ACK. Should this marked as Fixes https://github.com/bitcoin/bitcoin/issues/32341?

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