💬 tdb3 commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1722987831)
Ah, that's right (no hungarian). Thanks sipa

👍 marcofleon approved a pull request: "fuzz: Faster utxo_snapshot fuzz target"
(https://github.com/bitcoin/bitcoin/pull/30644#pullrequestreview-2247458898)
Re ACK fa899fb7aa8a14acecadd8936ad5824fa0f697ff

📝 Sjors opened a pull request: "Clang 19"
(https://github.com/bitcoin/bitcoin/pull/30682)
This tests https://github.com/bitcoin/bitcoin/pull/30634 and https://github.com/bitcoin/bitcoin/pull/30639 for the TSAN/MSAN failure with `vm.mmap_rnd_bits=32` described in https://github.com/bitcoin/bitcoin/issues/30674.

Since I have two different machines for CI and only one has `vm.mmap_rnd_bits=32`, I normally expect either TSAN or MSAN to fail, but not both. I'll manually restart where needed to get it to run on the right machine.

✅ Sjors closed a pull request: "Clang 19"
(https://github.com/bitcoin/bitcoin/pull/30682)

💬 Sjors commented on pull request "Clang 19":
(https://github.com/bitcoin/bitcoin/pull/30682#issuecomment-2298428620)
(sorry, meant to open this against my own fork)

🤔 BrandonOdiwuor reviewed a pull request: "Improve user dialog when signing multisig psbts"
(https://github.com/bitcoin-core/gui/pull/832#pullrequestreview-2247585717)
Concept ACK

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723086287)
Agree `sizeof` was more of a footgun.

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723087133)
Done in latest push without any hoisting.

💬 theStack commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#issuecomment-2298558011)
Thanks for the reviews! Took the suggestions and refined the UTXO check against the `scantxoutset` result to also include the vout index (=0), rather than only verifying the txid.

💬 theStack commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#discussion_r1723108255)
Introduced `snapshot_hash` and `snapshot_num_coins` in the beginning and used the already existing `SNAPSHOT_BASE_HEIGHT` for the height.

✅ fjahr closed a pull request: "wallet: Write best block to disk before backup"
(https://github.com/bitcoin/bitcoin/pull/30678)

💬 fjahr commented on pull request "wallet: Write best block to disk before backup":
(https://github.com/bitcoin/bitcoin/pull/30678#issuecomment-2298564117)
> See #30221. It fixes this in a more comprehensive way.

Ok, I didn't find it and weirdly there seems to be no conflict...

💬 fjahr commented on pull request "test: assumeutxo: check that UTXO-querying RPCs operate on snapshot chainstate":
(https://github.com/bitcoin/bitcoin/pull/30636#issuecomment-2298593717)
utACK 917e70a6206c62c4c492fa922425fc8e00d3f328

Only changes since last review were addressing my minor review comments.

💬 Sjors commented on issue "TSAN/MSAN fails with vm.mmap_rnd_bits=32 even with llvm 18.1.3":
(https://github.com/bitcoin/bitcoin/issues/30674#issuecomment-2298597110)
@maflcko clang 19 fixes neither, see https://github.com/Sjors/bitcoin/pull/59.

🤔 fjahr reviewed a pull request: "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test"
(https://github.com/bitcoin/bitcoin/pull/30681#pullrequestreview-2247673045)
Concept ACK

💬 fjahr commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1723134640)
Could you choose a different name for this variable than `nHeight`? Aside from the outdated style `next_height` or something like that is describing it better.

💬 fjahr commented on pull request "Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test":
(https://github.com/bitcoin/bitcoin/pull/30681#discussion_r1723117531)
This is also used to enforce the block storm mitigation.

👍 stickies-v approved a pull request: "refactor: Replace ParseHex with consteval HexLiteral"
(https://github.com/bitcoin/bitcoin/pull/30377#pullrequestreview-2247641047)
re-ACK e29741857e900d933b5cf0fb22e3a63bfa1ecd6a

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723140049)
nit to reduce variable scope and make it slightly more readable:

<details>
<summary>git diff on bad1ffb725</summary>

```diff
diff --git a/src/wallet/crypter.cpp b/src/wallet/crypter.cpp
index a574c8909e..a4b1fd9b3b 100644
--- a/src/wallet/crypter.cpp
+++ b/src/wallet/crypter.cpp
@@ -100,12 +100,11 @@ bool CCrypter::Decrypt(const std::vector<unsigned char>& ciphertext, CKeyingMate
plaintext.resize(ciphertext.size());

AES256CBCDecrypt dec(vchKey.data(), vchIV.data(), tru
...

💬 stickies-v commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1723098790)
nit: `std::remove_reference_t`
```suggestion
static_assert(WALLET_CRYPTO_IV_SIZE <= std::remove_reference_t<decltype(iv)>::size());
```