💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1665666251)
nit: Comment could be a little more detailed

```suggestion
# On node 0, create an alternative chain containing 2 new blocks, forking off the main chain at the block before the snapshot block. This simulates a longer chain than the main chain when submitting these two block headers to node 1 because it is only aware of the main chain headers up to the snapshot height.
```

💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1665674679)
nit: making the variables a bit more detailed could make the test a bit easier to parse

`parent` = > `parent_block_hash`
`fork1` => `fork_block1`
`main1` = `main_block1`

💬 hebasto commented on pull request "contrib: use c++ compiler rather than c compiler for binary checks":
(https://github.com/bitcoin/bitcoin/pull/30387#issuecomment-2208959206)
My Guix build:
```
x86_64
33e4e9bba968ed9ee96f67a8535b71bc1b1c5ede7038a2b636f406262474ddca guix-build-98ff3703b81f/output/aarch64-linux-gnu/SHA256SUMS.part
d670e9a8cc34c7b8e9567ee8654b5323ee102e18cbbd53d541deae779100f344 guix-build-98ff3703b81f/output/aarch64-linux-gnu/bitcoin-98ff3703b81f-aarch64-linux-gnu-debug.tar.gz
8a96d7fb1539524b8d29eb768cf3d4ed2428df203fba3bce0ff173b733cb8d51 guix-build-98ff3703b81f/output/aarch64-linux-gnu/bitcoin-98ff3703b81f-aarch64-linux-gnu.tar.gz
d0e0d9556
...

👍 dergoegge approved a pull request: "refactor: use existing RNG object in ProcessGetBlockData"
(https://github.com/bitcoin/bitcoin/pull/30393#pullrequestreview-2158916349)
utACK fa01c2af9eb91aebcdb87947145ba34b519bdfd8

There was some discussion on #29625 about reverting #28558 but this lgtm for now.

👍 dergoegge approved a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2158685410)
Code review ACK 17d41e7a547f16f0dd3802dac73a63772ca000b2

💬 dergoegge commented on pull request "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip":
(https://github.com/bitcoin/bitcoin/pull/30111#discussion_r1665560023)
```suggestion
virtual void ActiveTipChange(const CBlockIndex* new_tip, bool is_ibd) {};
```

👍 hebasto approved a pull request: "contrib: use c++ compiler rather than c compiler for binary checks"
(https://github.com/bitcoin/bitcoin/pull/30387#pullrequestreview-2158920731)
ACK 98ff3703b81fbcece22eed55433cfe0fe101704f, I have reviewed the code and it looks OK. The Guix build script works as expected..

💬 brunoerg commented on pull request "Tr partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#discussion_r1665706819)
In 6908349fa559c16164c76fff2f140a39f52279e2 "descriptor: Implement rawleaf(() partial descriptor": Please correct me if I'm wrong, but is this check needed? If the expected leaf version is not provided, won't it be verified before (in `if (!Const(",", expr))`?.

🚀 glozow merged a pull request: "validation: Check if mempool exists before size check in ActivateSnapshot"
(https://github.com/bitcoin/bitcoin/pull/30388)

🚀 fanquake merged a pull request: "depends: build libevent with CMake"
(https://github.com/bitcoin/bitcoin/pull/29835)

💬 fanquake commented on pull request "contrib: use c++ compiler rather than c compiler for binary checks":
(https://github.com/bitcoin/bitcoin/pull/30387#issuecomment-2208993921)
Concept ACK - probably also the right time to switch the C code to something more C++. i.e:
```c
#include <stdio.h>
int main() {
printf("the quick brown fox jumps over the lazy god\\n");
return 0;
}
```
to
```cpp
#include <cstdio>
int main() {
std::printf("the quick brown fox jumps over the lazy god\n");
return 0;
}
```

💬 marcofleon commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#discussion_r1665723276)
If we're using constants from `crypter.h`, then this can be changed to `WALLET_CRYPTO_IV_SIZE` for consistency.

💬 brunoerg commented on pull request "fuzz: fuzz connman with non-empty addrman + ASMap":
(https://github.com/bitcoin/bitcoin/pull/29536#issuecomment-2209011214)
Rebased

💬 marcofleon commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#discussion_r1665725484)
Is there a specifc reason for 100 here or is it not that important?

💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#discussion_r1665725982)
Sure, I'll address it.

💬 dergoegge commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2209018643)
> it limits the number of connected stratum clients to 1, since there's only one ZMQ template feed (that seems ok for the standard suggested sv2 configurations)

I'm sure we could design the interface in a way that allows for more than one template config (if needed), e.g. the new rpc could accept a list of template configs and the zmq publisher publishes a template for each config.

> it precludes the ability to make the template provider public facing, even with an external tool

Why? In
...

💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#discussion_r1665727291)
I used the size we reserve in the RPC for this as reference.

💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2209030226)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/30373#discussion_r1665723276. Thanks, @marcofleon

💬 maflcko commented on pull request "refactor: use existing RNG object in ProcessGetBlockData":
(https://github.com/bitcoin/bitcoin/pull/30393#issuecomment-2209034642)
> There was some discussion on #29625 about reverting #28558 but this lgtm for now.

Happy to drop it, if the reason still applies, but I couldn't find one.

👍 TheCharlatan approved a pull request: "fuzz: improve utxo_snapshot target"
(https://github.com/bitcoin/bitcoin/pull/30329#pullrequestreview-2158973782)
ACK b9ba1a73094f4ad593b527e23d2681f41c225397

... but it would be nice to address the nits, will re-ACK quickly :)