💬 theuni commented on pull request "checkqueue: make the queue non-optional for CCheckQueueControl and drop legacy locking macro usage":
(https://github.com/bitcoin/bitcoin/pull/32467#discussion_r2103446971)
Yep, no reason other than I thought it was more readable that way.

💬 willcl-ark commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2902759379)
> > Provides stronger guarantees about where dependencies come from during a (depends) build; i.e. only from depends.
>
> Can you elaborate on this? Currently, CMake configured with a depends toolchain, should never look outside depends (doing so would be a bug). What are the stronger guarantees?

Right, perhaps I didn't use the best wording here.

When using the toolchain I _have not_ found us pulling in extraneous deps from outside of depends, but _have_ encountered us missing dependenc
...

📝 theStack opened a pull request: "wallet, rpc, doc: various legacy wallet removal cleanups in RPCs"
(https://github.com/bitcoin/bitcoin/pull/32596)
This PR contains a few smaller wallet RPC cleanups based on that we only ever operate on descriptor wallets now:
* remove the now obsolete "keypoololdest" field from the `getwalletinfo` RPC and the corresponding CWallet/ScriptPubKeyMan methods
* in RPCs where potential fast wallet rescan is documented, remove the "descriptor wallet" mentions (back then introduced in commit ca48a4694f73e5be8f971ae482ebc2cce4caef44, PR #25957)
* for the `createwallet` RPC examples, remove the "descriptors" para
...

💬 mzumsande commented on pull request "refactor: Split multithreaded case out of CheckInputScripts":
(https://github.com/bitcoin/bitcoin/pull/32575#discussion_r2103526405)
I like that idea.

I think it would be a small change in behaviour, because if I read [this line](https://github.com/bitcoin/bitcoin/blob/2df824f4e62b6bc569044819cd64f66f3839ba13/src/validation.cpp#L2248) right it will add entries to the cache if
1)`pvChecks` is not set (single-threaded)
2) `cacheFullScriptStore` is set (i.e. if `ConnectBlock()` was called with `fJustCheck`, so currently only from `TestBlockValidity`).
I wonder if the current behavior is intended - it seems very strange
...

💬 davidgumberg commented on pull request "thread-safety: fix annotations with REVERSE_LOCK":
(https://github.com/bitcoin/bitcoin/pull/32465#discussion_r2103549376)
In commit https://github.com/bitcoin/bitcoin/commit/832c57a53410870471a52647ce107454de3bc69c (_thread-safety: modernize thread safety macros_)

----

Feel free to disregard, I am just trying to understand the motivation of this commit:

> unlock_function actually maps to release_generic_capability, which does not
work properly when implementing a scoped unlocker.

Looking at the example code in clang's [documentation](https://clang.llvm.org/docs/ThreadSafetyAnalysis.html#mutex-h):

`
...

💬 theStack commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2103562671)
That makes sense, thanks for clarifying (missed that the one-parameter variant was not `noexcept`). Might be worth it to add a small comment about this in a follow-up PR.

💬 sipa commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#discussion_r2103574633)
Expanded the comment to say that.

🤔 1440000bytes reviewed a pull request: "wallet, rpc, doc: various legacy wallet removal cleanups in RPCs"
(https://github.com/bitcoin/bitcoin/pull/32596#pullrequestreview-2862873626)
ACK https://github.com/bitcoin/bitcoin/pull/32596/commits/e5cbea416b2f63e5d99819052f3e69a6383336d6

<details>
<summary>Signature</summary>

<pre>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK e5cbea416b2f63e5d99819052f3e69a6383336d6
-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQSSUwYqT5LWNFkIXKYtIwUgISpZAQUCaC++7QAKCRAtIwUgISpZ
AZIaAP9bxiLFrXtgDk7+yoGgHAUO2xP2AmG5qRcYtZDSxuqkmAEAotL4R3L9iDoX
NZeq+w5EEr+4SO7zLYo+Cnb7krWYpQg=
=pUtX
-----END PGP SIGNATURE-----
</pre>
</detai
...

📝 achow101 opened a pull request: "wallet: Always set descriptor cache upgraded flag for new wallets"
(https://github.com/bitcoin/bitcoin/pull/32597)
Newly created wallets will always have an upgraded descriptor cache, so set those.

Also, to verify this behavior, add a new `flags` field to `getwalletinfo` and check that in the functional tests.

Split from #32489

📝 achow101 opened a pull request: "walletdb: Log additional exception error messages for corrupted wallets"
(https://github.com/bitcoin/bitcoin/pull/32598)
Many exceptions thrown for corruption are `std::runtime_error`; we should catch those and log the message to help with debugging.

Split from #32489

💬 1440000bytes commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2903002368)
Concept ACK

💬 davidgumberg commented on pull request "wallet: Fix logging of wallet version":
(https://github.com/bitcoin/bitcoin/pull/32553#issuecomment-2903003935)
post-merge ACK https://github.com/bitcoin/bitcoin/pull/32553/commits/4b2cd0b41ff4800c8801f2c44883eaec60a035fa

💬 hebasto commented on issue "`system_tests/run_command` test fails on illumos OS":
(https://github.com/bitcoin/bitcoin/issues/32574#issuecomment-2903353573)
The underlying issue is not specific to illumos.

With the following patch:
```diff
--- a/src/test/system_tests.cpp
+++ b/src/test/system_tests.cpp
@@ -67,7 +67,7 @@ BOOST_AUTO_TEST_CASE(run_command)
const std::string expected{"err"};
BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const std::runtime_error& e) {
const std::string what(e.what());
- BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned
...

💬 maflcko commented on pull request "rev_32343_wip_nomerge_ci":
(https://github.com/bitcoin/bitcoin/pull/32535#issuecomment-2903371304)
The failure remains, when triggered, so it would be good to investigate this. Let's continue discussion in https://github.com/bitcoin/bitcoin/issues/32524

✅ maflcko closed a pull request: "rev_32343_wip_nomerge_ci"
(https://github.com/bitcoin/bitcoin/pull/32535)

💬 maflcko commented on issue "intermittent issue in rpc_signer.py (enumeratesigners timeout) under GLIBCXX debug mode":
(https://github.com/bitcoin/bitcoin/issues/32524#issuecomment-2903373050)
This is closed, but the failure remains, when triggered, so it would be good to investigate this long-term at some point. Discussion can continue here, or in a new issue.

💬 hebasto commented on pull request "subprocess: Let shell parse command on non-Windows systems":
(https://github.com/bitcoin/bitcoin/pull/32577#issuecomment-2903379032)
> * It's not clear to me what the problem in [#32574](https://github.com/bitcoin/bitcoin/issues/32574) on illumos or actually is or how this change fixes it.

I've updated https://github.com/bitcoin/bitcoin/issues/32574 with more details.

> On the c++11 issue, I think maybe just compiling the subprocess unit test in c++11 would be a good way to ensure compatibility. It seems cmake has a [CXX_STANDARD](https://cmake.org/cmake/help/latest/prop_tgt/CXX_STANDARD.html) per-target property. I
...

💬 hebasto commented on pull request "Use subprocess library for notifications":
(https://github.com/bitcoin/bitcoin/pull/32566#discussion_r2103886263)
A related discussion takes place at https://github.com/bitcoin/bitcoin/pull/32577.

⚠️ maflcko opened an issue: "intermittent issue in feature_init.py (bitcoind should have exited with expected error LevelDB error: Corruption)"
(https://github.com/bitcoin/bitcoin/issues/32600)
https://cirrus-ci.com/task/5185890844147712?logs=ci#L3553:

```
267/267 - [1mfeature_init.py [0m failed, Duration: 2420 s
[17:16:50.450]
[17:16:50.450] [1mstdout:
[17:16:50.450] [0m2025-05-21T20:36:30.563000Z TestFramework (INFO): PRNG seed is: 630547506121562679

...

[17:16:50.450] 2025-05-21T20:36:49.282000Z TestFramework (INFO): Perturbing file to ensure failure /ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20250521_201845/feature_init_53/node0/regtest/indexes/coinstats/db/0
...

💬 vasild commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2903394731)
Concept ACK to improve this, but:

> what should happen in the case of a stalling situation where the addnode peer is actually slow and causing congestion? ... one addnode peer the power to let our entire IBD come to a halt completely

Would it be too difficult to request a given block from more than one peer concurrently? That is, if a block is requested from a peer and not received within some time, then request it from other(s) as well, but don't cancel the original request. Then we take
...