👍 ryanofsky approved a pull request: "refactor: Replace ParseHex with consteval HexLiteral"
(https://github.com/bitcoin/bitcoin/pull/30377#pullrequestreview-2245582079)
Code review ACK 67fc994bedf14e360b3e51fa1a71dc6c1684b532, but it'd be fine to drop last two commits of this PR. I do think they would be improvement despite drawbacks Marco listed, but they aren't necessary and could be saved for a followup which fixes their shortcomings.

💬 ryanofsky commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1721808875)
In commit "refactor: Make code more tolerant of constexpr std::arrays" (72256f9d78d2187411f55aa3f779170bed9bde1e):

Would be nice to add:

```c++
static_assert(WALLET_CRYPTO_IV_SIZE <= iv.size());
```

Since it's not obvious `iv` size matches `WALLET_CRYPTO_IV_SIZE` (I was surprised to see it's actually twice the size).

💬 Sjors commented on pull request "CMake replaces Autotools":
(https://github.com/bitcoin/bitcoin/pull/30664#issuecomment-2296707458)
Concept ACK on dropping Autotools quickly after merging cmake. Maintaining both sounds like a nightmare. Mainly for build system folks, but it's also more work for people making pull requests that add/remove files. Best just get it over with.

I tested rebasing my Stratum v2 related pull requests (#29346, #30315, #29432 and several on my own fork). It was pretty straight-forward, see https://github.com/Sjors/bitcoin/pull/58.

I haven't tried rebasing the IPC (multiprocess) variant of my PR.
...

💬 paplorinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1721879299)
I'm fine with that, extracted to a branch for now, will push as a draft after your PR stabilizes

👍 ryanofsky approved a pull request: "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals"
(https://github.com/bitcoin/bitcoin/pull/30409#pullrequestreview-2245698982)
Code review ACK e8176694bb5508b2b488a8c79a6001d2fa625302, just reduced a variable scope since last review.

💬 ryanofsky commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1721880533)
In commit "Replace RPCNotifyBlockChange with waitTipChanged()" (b4d37fa70506129ba4a4d218bc49ce8ca9d8380c)

Not important, but could simplify these two lines to `while (IsRPCRunning() && block.hash != hash)`

Similarly, could simplify to `while (IsRPCRunning() && block.height < height)` in waitforblockheight below.

💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2296723316)
Ok, the flamegraph helps. With a quick skim, it looks like all flames are somehow IO (read or write) related. This means that you are not evaluating the fuzz target itself, but the speed of your storage and how it can handle the data that the fuzz engine produces for it to read and write. I guess, starting with an empty fuzz input set somehow encourages the fuzz engine to create large inputs, which then cause a high IO usage.

My recommendation would be to make sure you are fuzzing on a ramdis
...

💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2296762977)
Makes sense, was just working with Niklas to get this done. The flame graphs are a useful new tool for me too, I'll be utilizing those more. Thanks for the help @maflcko.

💬 brunoerg commented on pull request "test: check xor.dat creation when missing":
(https://github.com/bitcoin/bitcoin/pull/30669#discussion_r1721922038)
```suggestion
assert_equal(read_xor_key(node=node), NULL_BLK_XOR_KEY)
```

💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2296780238)
ACK fa0b66400c12d11579b38b1ac76f677b23e8c82a

💬 Sjors commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1721929753)
Thanks, will do if I have to retouch.

💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2296824949)
Force pushed to remove stray, unused lines in the diff of the last commit. Should be easy to re-review.

📝 romanz opened a pull request: "http: set TCP_NODELAY when creating HTTP server"
(https://github.com/bitcoin/bitcoin/pull/30675)
Otherwise, the default HTTP server config may result in high latency, due to Nagle's algorithm (on the server) and delayed ACK (on the client):

[1] https://www.extrahop.com/blog/tcp-nodelay-nagle-quickack-best-practices
[2] https://eklitzke.org/the-caveats-of-tcp-nodelay


Without the fix, fetching a small block takes ~40ms (when connection keep-alive is enabled):
```
$ ab -k -c 1 -n 100 http://localhost:8332/rest/block/00000000000002b5898f7cdc80d9c84e9747bc6b9388cc989971d443f05713ee.bi
...

💬 andrewtoth commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1721978296)
`LogPrintf` is deprecated. See https://github.com/bitcoin/bitcoin/pull/29641

💬 romanz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#issuecomment-2296860648)
`TCP_NODELAY` has been set on p2p connections by https://github.com/bitcoin/bitcoin/pull/6867.

💬 achow101 commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#issuecomment-2296871606)
> I noticed the `seeds.txt.gz` file I used (~2024-08-19T08:30Z) file did not contain any good I2P nodes.

I think it got disconnected from I2P and started marking every I2P node as down when it was actually that the crawler's I2P connection was down. That's something I'll need to fix.

⚠️ MarkLTZ opened an issue: "Unable to execute './contrib/guix/guix-build' on a fresh installed server."
(https://github.com/bitcoin/bitcoin/issues/30676)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Hello there,

executing `./contrib/guix/guix-build` I have the following error:

```
-builder for `/gnu/store/0h7r1766n7brqlvzpnb0y3vank5faizz-linux-libre-5.4.20-gnu.tar.xz.drv' failed to produce output path `/gnu/store/0zcl1i3rbjc356138hx86b7yaz29g6fj-linux-libre-5.4.20-gnu.tar.xz'
build of /gnu/store/0h7r1766n7brqlvzpnb0y3vank5faizz-linux-libre-5.4.20-gnu.tar.xz.drv failed
View bu
...

💬 pinheadmz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#issuecomment-2296950626)
concept ACK, I'm kinda surprised we don't have this already. I did a quick check into libevent and don't see any helpers there like `evutil_make_listen_socket_reuseable()` (which sets `SO_REUSEADDR` so this might be the best approach

💬 romanz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1722060528)
Many thanks! Fixed in f14484184f8cae134f65801647280c2c1f4930b2.

⚠️ hebasto opened an issue: "Control-flow application capabilities for `x86_64-linux-gnu` release binaries"
(https://github.com/bitcoin/bitcoin/issues/30677)
When building static binaries for `x86_64-linux-gnu`, one can verify that both Control-flow Enforcement Technology (CET) capabilities--indirect branch tracking (IBT) and shadow stack--are enabled by running the following command:
```
$ readelf -n src/bitcoind | grep feature
Properties: x86 feature: IBT, SHSTK
```
However, that is not the case for the Guix binaries:
```
$ readelf -n bin/bitcoind | grep feature
Properties: x86 feature used: x86, x87, XMM, YMM, XSAVE
```