Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ’¬ danielabrozzoni commented on pull request "rpc: Add optional peer_ids param to filter getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/32741#discussion_r2611574944)
nit if retouched: "peer IDs" (so it stays consistent with the rest of the help text)
βœ… l0rinc closed a pull request: "bench: run `FindByte` across block-sized buffer"
(https://github.com/bitcoin/bitcoin/pull/34046)
πŸ“ l0rinc reopened a pull request: "bench: run `FindByte` across block-sized buffer"
(https://github.com/bitcoin/bitcoin/pull/34046)
The current benchmark doesn't represent a realistic scenario:
* it only checks 200 bytes, while in reality we have 8 MB: https://github.com/bitcoin/bitcoin/blob/2c44c41984e0a170cf38d595635a9e861854573c/src/validation.cpp#L5009
* it only checks the very last position (which is basically the missing case), while in reality it's a circular buffer, so the magic bytes can be anywhere
* plain array was needlessly serialized instead of written directly

The benchmark was updated to start from diff
...
πŸ’¬ l0rinc commented on pull request "bench: run `FindByte` across block-sized buffer":
(https://github.com/bitcoin/bitcoin/pull/34046#issuecomment-3643249468)
(close/open dance to fix boost download issue)
βœ… l0rinc closed a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497)
πŸ“ l0rinc reopened a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497)
#### Summary

`ComputeMerkleRoot` [duplicates the last hash](https://github.com/bitcoin/bitcoin/blob/39b6c139bd6be33699af781f1d71f6fed303d468/src/consensus/merkle.cpp#L54-L56) when the input size is odd. If the caller provides a `std::vector` whose capacity equals its size, that extra `push_back` forces a reallocation, doubling its capacity (causing peak memory usage of 3x the necessary size).

This affects roughly half of the created blocks (those with odd transaction counts), causing unnec
...
πŸ’¬ theuni commented on issue "ci: The bitcoincore.org depends fallback is missing sqlite-autoconf-3500400.tar.gz":
(https://github.com/bitcoin/bitcoin/issues/34021#issuecomment-3643265612)
Yeah, there was a firewall issue after a server move. Fixed now, and sqlite-autoconf-3500400.tar.gz is available.
βœ… theuni closed an issue: "ci: The bitcoincore.org depends fallback is missing sqlite-autoconf-3500400.tar.gz"
(https://github.com/bitcoin/bitcoin/issues/34021)
πŸ’¬ l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3643280306)
reACK 1a0423cf25bb60f98a29e06c39b7db2998860cd9
βœ… plebhash closed an issue: "consider adding a new `interface RawTxFeed` on Mining IPC"
(https://github.com/bitcoin/bitcoin/issues/34030)
πŸ’¬ sipa commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3643378098)
Concept ACK, but disagree with the approach of the first commit.

I don't think it tests anything useful:
* If it wanted to test the exact behavior of the skip-height pointer building, it could test that directly. No need for averages or medians here; it could test that block 171 points to block 161 and no other, for example.
* If it wanted to test something related to the performance of the constructed index, without hardcoding the exact behavior, it would need to do something like pick ran
...
πŸ“ maflcko opened a pull request: " log: Remove brittle and confusing LogPrintLevel "
(https://github.com/bitcoin/bitcoin/pull/34051)
`LogPrintLevel` has many issues:

* It encourages to log several levels in one source location. This is problematic, because all levels (even warnings and errors) will be rate limited equally for the same location.
* Its warning and error logs are specially formatted compared to all other warning and error logs in the codebase, making them harder to spot (both in the debug log and in the code).
* It is verbose to type and read.
* It is confusing, because the majority of code uses the `Log$L
...
πŸ’¬ maflcko commented on pull request "scripted-diff: Unify error and warning log formatting":
(https://github.com/bitcoin/bitcoin/pull/34033#issuecomment-3643390479)
> something like https://github.com/stickies-v/bitcoin/commits/2025-12/separate-log-print-level/ seems worth doing. As a bonus, it prevents issues such as in #34008 where debug logs can lead to unconditional logs being ratelimited.

Thx, create a pull request with your bugfix commits in https://github.com/bitcoin/bitcoin/pull/34051
πŸ’¬ fanquake commented on issue "29.x depends: fallback server missing capnp downloads":
(https://github.com/bitcoin/bitcoin/issues/33901#issuecomment-3643393113)
This is now fixed.
βœ… fanquake closed an issue: "29.x depends: fallback server missing capnp downloads"
(https://github.com/bitcoin/bitcoin/issues/33901)
πŸ’¬ optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3643415546)
> I don't think it tests anything useful:
>
> * If it wanted to test the exact behavior of the skip-height pointer building, it could test that directly. No need for averages or medians here; it could test that block 171 points to block 161 and no other, for example.
>
> * If it wanted to test something related to the performance of the constructed index, without hardcoding the exact behavior, it would need to do something like pick random pairs of blocks in the chain, and see how
...
πŸ’¬ sipa commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-3643423211)
My point is that these two things, regardless of how they are accomplished, are the interesting ones.
* Your test achieves the first, but only approximately, and is much more complicated than needed for that purposes.
* Your test does not achieve the second at all.
πŸ’¬ achow101 commented on pull request "validation: periodically flush dbcache during reindex-chainstate":
(https://github.com/bitcoin/bitcoin/pull/32414#issuecomment-3643539057)
ACK c1e554d3e5834a140f2a53854018499a3bfe6822
πŸ’¬ mzumsande commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2611870620)
I think so. But I think that `tor_port(MAX_NODES) + 1` isn't a good choice because it will lead to collisions among parallel running tests as soon as there is more than one test using the feature. I changed it to `p2p_port(n) + PORT_RANGE *3`.
πŸš€ achow101 merged a pull request: "validation: periodically flush dbcache during reindex-chainstate"
(https://github.com/bitcoin/bitcoin/pull/32414)
πŸ’¬ maflcko commented on pull request "test: add functional test for outbound connection management":
(https://github.com/bitcoin/bitcoin/pull/33954#discussion_r2611907262)
Haven't looked here in detail, but my preference would be to not call `+` or `+=` on an integral port returned by a helper function. This will lead to issues down the line, and it would be better to always call the helper that internally asserts and checks the port range. See https://github.com/bitcoin/bitcoin/pull/33260/files