💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1676379848)
e9a436b7796cb285730d179fb3317ad50234ab04 changed it to: `This is not a uint256 constructor because of historical fears of uint256(0) resolving to a NULL string and crashing.`

💬 pinheadmz commented on pull request "fix: increase consistency of rpcauth parsing":
(https://github.com/bitcoin/bitcoin/pull/30401#issuecomment-2226264572)
concept ACK, this simplifies a confusing behavior with `rpcuser` misconfiguration. Code changes look simple enough, will test locally and run through your (very exhaustive!) behavior comparison

💬 mzumsande commented on pull request "init: change shutdown order of load block thread and scheduler":
(https://github.com/bitcoin/bitcoin/pull/30435#issuecomment-2226324701)
> It would help us catch these type of errors (if we still have them).

Do you mean throwing an assert instead of blocking indefinitely? That might be more convenient than hanging indefinitely, but I'm not sure if this really makes much of a difference in practice, because both ways should be easily recognizable both by users and tests.

An alternative approach would be to _allow_ it - just return instead of waiting forever, if we are in Shutdown mode and rely on a later `FlushBackgroundCal
...

👍 hebasto approved a pull request: "init: change shutdown order of load block thread and scheduler"
(https://github.com/bitcoin/bitcoin/pull/30435#pullrequestreview-2175766383)
re-ACK 5fd48360198d2ac49e43b24cc1469557b03567b8.

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

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

Looks good. Ran a few manual tests:
```
$ src/bitcoin-cli createwallet test
$ src/bitcoin-cli -generate 101
$ src/bitcoin-cli -named sendtoaddress address=bcrt1qg2yks0c0slr8nklg30ts48lcq7yslcvaak7n9k amount=39 fee_rate=3
(0383cce20ad8c1f119f4f1e027bbb132c5e0500518c6adfd79098242ad733284)
$ src/bitcoin-cli -generate 1
$ curl localhost:18443/rest/getutxos/0383cce20ad8c1f119f4f1e027bbb132c5e0500518c6adfd79098242ad733284-0.json
{"chainHeight":1
...

📝 knst opened a pull request: "fix: correct replacement of amp character in the wallet name for QMenu"
(https://github.com/bitcoin/bitcoin/pull/30446)
The comment in the code is misleading, if you replace one & to double &&, the next & will be "single one". All of them in the string should be replaced, not only the first occurrence.

In the current implementation Qt uses '&' as a signal to underscore letter and use it as a hot-key, which is not excepted for case of wallet.


See screenshots before & after:
![Screenshot_20240713
...

💬 regressivetassive commented on issue "intermittent issue in feature_reindex.py: Reindex fails to continue after restart; `initload` thread hangs forever":
(https://github.com/bitcoin/bitcoin/issues/30424#issuecomment-2226797954)
I've been exploring different options for managing my BTC transactions and came across an interesting alternative: flash BTC software. I've found that using a reliable flash BTC software can provide a more efficient and secure way to handle transactions. If anyone is interested in learning more, look at that repository (https://github.com/CrypToprexofficial/Free-flash-btc-sender) that provides more information and resources on the topic. Please keep in mind that it's essential to do your own res
...

👋 paplorinc's pull request is ready for review: "optimization: Precalculate SipHash constant XOR with k0 and k1 in SaltedOutpointHasher"
(https://github.com/bitcoin/bitcoin/pull/30442)

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2226843029)
@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 volatile).

Does this seem like a realistic measurement?
```bash
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/BitcoinData -dbcache=16384 -pru
...

💬 Sjors commented on pull request "multiprocess: add bitcoin-mine test program":
(https://github.com/bitcoin/bitcoin/pull/30437#issuecomment-2226847354)
I wrote:

> I'll expand the demo app to simulate all the steps a Template Provider would do

Once that works the next step for me would be to open an alternative to #29432 which runs the Template Provider in `bitcoin-tp`. I might wait with that until we ship a release with this interface enabled on the node side, ideally in `bitcoind` and `bitcoin-qt` rather than in a seperate `bitcoin-node` and `bitcoin-gui` binaries.

I could then modify the guix build to _only_ produce `bitcoin-tp`. Thi
...

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676801669)
this seems redundant, if we're not sure about this, I'd rather add a test that can bite back (as you can tell, I'm not a big fan of dead comments :p)

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676800505)
nit: "move coin into us" sounds a bit weird.
And now that the code is cleaner, does the comment add anything that the code doesn't already say?

* `Since this entry will be erased` -> `if (cursor.WillErase(*it)) {`
* `we can move the coin into us instead of copying it` -> `std::move(it->second.coin)` vs `it->second.coin`

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676801797)
+1

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676802135)
this should have been there all along, i.e. you've fixed a small independent accounting bug here, right?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676801434)
I believe in making important parts of the code stick out and secondary importance going in the background.
This move is secondary importance, right? But it occupies a big part of the method, maybe we can simplify it with a ternary:
```suggestion
CCoinsCacheEntry& entry{itUs->second};
entry.coin = cursor.WillErase(*it) ? std::move(it->second.coin) : it->second.coin;
```

📝 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_
...