Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 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 :)
💬 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
...