💬 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 :)

💬 Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2209097995)
> the new rpc could accept a list of template configs and the zmq publisher publishes a template for each config.

I suppose the external application could forward each to the right client.

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

Because each consumer of this interface will have its own `coinbase_tx_additional_output_size`, which it can't communicate without RPC access. This can be avoided by picking a single size f
...

💬 willcl-ark commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2209098078)
> There's a lot of stuff happening in one commit:

Right, I wanted to open this up specifically while WIP as I ended up less-sure about the idea of the change in general (nor the implementation approach). The final commit does indeed need breaking up, but I wanted to get feedback on the mechanics of the whole change before investing more time into it, so thank you very much for your detailed review!

> 1. behaviour change: allow removing a v2 connection when `!node_v2transport`

This was a
...

🤔 marcofleon reviewed a pull request: "fuzz: fix key size in `crypter`"
(https://github.com/bitcoin/bitcoin/pull/30373#pullrequestreview-2159032605)
It still crashes with the same bad_alloc error. But changing this line
https://github.com/bitcoin/bitcoin/blob/bd5d1688b4311e21c0e0ff89a3ae02ef7d0543b8/src/wallet/test/fuzz/crypter.cpp#L85
to `ConsumeRandomLengthByteVector(fuzzed_data_provider, 64)` fixes it for me. I thnk this is fine because `crypted_secret` ends up playing the same role as `cipher_text_ed` does in `Decrypt`.

🤔 furszy reviewed a pull request: "validation: sync chainstate to disk after syncing to tip"
(https://github.com/bitcoin/bitcoin/pull/15218#pullrequestreview-2159040159)
Code ACK 8887d28a014420668801d7e4c5d1bf45c5e93684

💬 furszy commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1665775393)
Ok. np.

Still.. I know it is an overkill if we only introduce what I'm going to suggest for this PR but.. thought on adding a signal for the ibd completion state https://github.com/furszy/bitcoin-core/commit/85a050a8690e5431848658604e913c58ae45a4aa. It might be useful if we ever added other scenarios apart from this one.

💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2209139299)
> to ConsumeRandomLengthByteVector(fuzzed_data_provider, 64) fixes it for me. I thnk this is fine because crypted_secret ends up playing the same role as cipher_text_ed does in Decrypt.

Nice, I missed this but surely it's fine to limit it as well. Gonna change it.

💬 brunoerg commented on pull request "fuzz: fix key size in `crypter`":
(https://github.com/bitcoin/bitcoin/pull/30373#issuecomment-2209143368)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/30373#pullrequestreview-2159032605

Now, we basically got rid of `ConsumeRandomLengthByteVector` with no specified `max_length`.