📝 TheCharlatan opened a pull request: "fuzz: Deglobalize signature cache in sigcache test"
(https://github.com/bitcoin/bitcoin/pull/30447)
The body of the fuzz test should ideally be a pure function. If data is persisted in the cache over many iterations, and there is a crash, reproducing it from the input might be difficult. Solve this by getting rid of the global state. This is a follow-up from #30425.

💬 0xB10C commented on pull request "fix: rendering an amp characters in the wallet name for QMenu":
(https://github.com/bitcoin/bitcoin/pull/30446#issuecomment-2226870406)
Thanks for the contribution knst. I think this PR might be better suited for https://github.com/bitcoin-core/gui. The gui repo is regularly synced with the bitcoin/bitcoin repo.

💬 hebasto commented on pull request "fix: rendering an amp characters in the wallet name for QMenu":
(https://github.com/bitcoin/bitcoin/pull/30446#issuecomment-2226873163)
> Thanks for the contribution knst. I think this PR might be better suited for https://github.com/bitcoin-core/gui. The gui repo is regularly synced with the bitcoin/bitcoin repo.

Yes. @knst please close this PR and open it in https://github.com/bitcoin-core/gui.

👍 hodlinator approved a pull request: "rest: Reject negative outpoint index early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30444#pullrequestreview-2176241736)
ACK fab54db9f1d0e634f4a697480dbb87b87940dc5c

Good to see more improvements to the robustness of that part of **rest.cpp**.
Ran the test-change without the C++ modifications and verified that signed (+/-) output indexes were previously supported. As that capability served no purpose and is unlikely to have been used, I support this narrowing of the API.

Also noticed what appears to be missing test coverage of `Parse[U]Int*`:

```diff
diff --git a/src/test/util_tests.cpp b/src/test/util_
...

📝 knst opened a pull request: "fix: rendering an amp characters in the wallet name for QMenu"
(https://github.com/bitcoin-core/gui/pull/828)
In the current implementation Qt uses '&' as a signal to underscore letter and use it as a hot-key, which is not expected for case of wallet name.

The [comment in the code](https://github.com/bitcoin/bitcoin/pull/30446/files#diff-2ecf8cbf369cf3d2f3d2b1cf5cfe4c1a647d63e11e2885d2fd0ac11fb5f7a804L402-L404) is misleading, if you replace one & to double &&, the next & will be a "single one". All of them in the string should be replaced, not only the first occurrence.


See screenshots before &
...

✅ knst closed a pull request: "fix: rendering an amp characters in the wallet name for QMenu"
(https://github.com/bitcoin/bitcoin/pull/30446)

💬 knst commented on pull request "fix: rendering an amp characters in the wallet name for QMenu":
(https://github.com/bitcoin/bitcoin/pull/30446#issuecomment-2226884796)
closed as moved to bitcoin-core/gui: https://github.com/bitcoin-core/gui/pull/828

👍 hodlinator approved a pull request: "rpc: Use CHECK_NONFATAL over Assert"
(https://github.com/bitcoin/bitcoin/pull/30429#pullrequestreview-2176246261)
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f

Ran linter without the C++ change:
```console
$ ./test/lint/lint-assertions.py
CHECK_NONFATAL(condition) or NONFATAL_UNREACHABLE should be used instead of assert for RPC code.
src/rpc/blockchain.cpp:804: const auto& first_unpruned{*Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
src/rpc/blockchain.cpp:811: return Assert(first_unpruned.pprev)->nHeight;
```
Including the C++ change silences the
...

👍 hodlinator approved a pull request: "refactor: Remove Span operator==, Use std::ranges::equal"
(https://github.com/bitcoin/bitcoin/pull/29071#pullrequestreview-2176250245)
ACK fabcd0c4fe44e3bc2d6a59f2839f459fd5c81171

💬 hodlinator commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#discussion_r1676823487)
nit: Double newline

💬 hodlinator commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#discussion_r1676825369)
(Attempted switching around the parameters to `lexicographical_compare()` and confirmed that it broke `test_bitcoin -t db_tests`).

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2226936005)
> @andrewtoth, I'm running the benchmark now on a cosy Hetzner dedicated server (without a local Bitcoin node, let's see if that makes it too unreliable).
>
> Does this seem like a realistic measurement?
>
> ```shell
> hyperfine \
> --warmup 1 --runs 2 \
> --show-output \
> --export-markdown bench.md \
> --parameter-list COMMIT 0383de,21090d \
> --prepare 'git checkout {COMMIT} && make -j10 && rm -rf /mnt/hdd1/BitcoinData/*' \
> 'echo {COMMIT} && ./src/bitcoind -datadir=/mnt/hdd1/Bi
...

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676837143)
Yes!

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676838221)
There's a long thread [here](https://github.com/bitcoin/bitcoin/pull/17487#discussion_r402037193) that is where this comment and `if/else` block arose from. I think you might have to click expand a few times to see it.

From that thread, I think using the ternary operator will result in an extra move instead of branching for the different cases.

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2226949831)
Also, you should probably wrap hyperfine with `nohup ... &` so you can exit the ssh session.

💬 fjahr commented on pull request "index: Check all necessary block data is available before starting to sync":
(https://github.com/bitcoin/bitcoin/pull/29770#issuecomment-2226993556)
Rebased with updates that resulted from the changes in https://github.com/bitcoin/bitcoin/pull/29668 before merge plus an additional commit that addresses left-over comments in https://github.com/bitcoin/bitcoin/pull/29668.

💬 cbergqvist commented on pull request "net: Favor peers from addrman over fetching seednodes":
(https://github.com/bitcoin/bitcoin/pull/29605#issuecomment-2227100893)
CI failure would be fixed by.
```diff
diff --git a/src/net.cpp b/src/net.cpp
index 01522a6286..36889a6da6 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -3269,9 +3269,7 @@ bool CConnman::Start(CScheduler& scheduler, const Options& connOptions)

// Randomize the order in which we may query seednode to potentially prevent connecting to the same one every restart (and signal that we have restarted)
std::vector<std::string> seed_nodes = connOptions.vSeedNodes;
- if (!seed_node
...

💬 sutt0n commented on issue "contrib/signet/getcoins.py no longer useable":
(https://github.com/bitcoin/bitcoin/issues/29679#issuecomment-2227104524)
@willcl-ark @kallewoof It seems that this issue is still occurring, or at least there's another issue related to this one with the Signet faucet -- is the faucet drained, or non-functioning at the moment? I've failed to get _any_ transactions to go through whenever it says "A pay-out has been queued"

💬 hodlinator commented on pull request "rest: Reject negative outpoint index early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30444#issuecomment-2227109971)
Have you considered migrating away from the other `ParseInt32()` call in the same file? Gets rid of the Valgrind comment.
This version keeps this one signed since it used in 2 places expecting signed:
```diff
diff --git a/src/rest.cpp b/src/rest.cpp
index 185508d2c9..e3e2dd96e6 100644
--- a/src/rest.cpp
+++ b/src/rest.cpp
@@ -954,8 +954,8 @@ static bool rest_blockhash_by_height(const std::any& context, HTTPRequest* req,
std::string height_str;
const RESTResponseFormat rf = Par
...

👍 tdb3 approved a pull request: "init: change shutdown order of load block thread and scheduler"
(https://github.com/bitcoin/bitcoin/pull/30435#pullrequestreview-2176555055)
ACK 5fd48360198d2ac49e43b24cc1469557b03567b8
Nice work.
Had to use a higher sleep than 50ms to reproduce the error on my local machine (used 150ms).