💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676324251)
Could we do that and delegate the deletion to a separate "service"? Seems complicated enough to warrant better encapsulation

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676328303)
I had it like that before but others suggested to move everything into 1 loop instead of looping through the dirty entries again after BatchWrite to clear flags and delete spent entries.

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676330337)
Can we have it both, single loop, but the mutation and its side-effects (like the move) separated clearly in the code from everything else?

💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676334189)
Will give it some thought

💬 pinheadmz commented on pull request "include verbose "reject-details" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#issuecomment-2226183342)
Thanks @DrahtBot good bot, have a biscuit.

Rebased on master to fix silent conflict that failed function test. Also updated PR title and description since the name of the new field has changed

👍 pinheadmz approved a pull request: "Fix cases of calls to `FillPSBT` errantly returning `complete=true`"
(https://github.com/bitcoin/bitcoin/pull/30357#pullrequestreview-2175585156)
re-ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902

Changes since last ACK are minimal, slight simplification in tests to enable more legacy compatability

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 7e36dca657c66bc70b04d5b850e5a335aecfb902
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaRgqoACgkQ5+KYS2KJ
yTq5NA/+NL1fOaCfl+P9yDIsXebxam/8Adf6iH5WUBt2TvUju06+305KL1dwvO6O
n0j8wmAeaJQe9tevuSTFYopxRaH+7
...

👍 pinheadmz approved a pull request: "Make it possible to disable Tor binds and abort startup on bind failure"
(https://github.com/bitcoin/bitcoin/pull/22729#pullrequestreview-2175592085)
ACK bca346a97056748f1e3fb899f336d56d9fd45a64

Changes since last ACK extremely minimal, cleaning up one line in the test

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK bca346a97056748f1e3fb899f336d56d9fd45a64
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmaRhAIACgkQ5+KYS2KJ
yTr7nA//Sq7hxrI2I5/n/paZ+F6aMbZSxIkv7ynizzr3WfzBimoFmh+RcXkpyg5I
OK32DfJbKod4U1EgkTXY6RkjF8uYftCqLugF9qwDKC4NhkWQxEjepxX75Vrs
...

🤔 furszy reviewed a pull request: "init: change shutdown order of load block thread and scheduler"
(https://github.com/bitcoin/bitcoin/pull/30435#pullrequestreview-2175592670)
q: what about disallowing the blocking-wait after stopping the scheduler? maybe only on debug mode. e.g. implementing an `isActive` method in the task runner and calling it prior to creating the promise.

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1676377257)
Improved testing and error reporting in 01fa92928e6f9509935840a8c57d29774a0d14a1.

Made 3d0aec126f8103310b741a5c21e94cf537fcd191 use `util::Split`. It allocates a vector for each parsed UTXO but is more readable than my former approach.

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