Bitcoin Core Github
44 subscribers
121K links
Download Telegram
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/30848)
👍 dergoegge approved a pull request: "fuzz: Test headers pre-sync through p2p"
(https://github.com/bitcoin/bitcoin/pull/30661#pullrequestreview-2292038673)
Code review ACK 1243d2ffa30ebc8a148623e3cdabd61f70714932

CI timeout is unrelated
👍 fanquake approved a pull request: "build: Use correct variable name"
(https://github.com/bitcoin/bitcoin/pull/30791#pullrequestreview-2292069963)
ACK 2d68c3b1c2e4f8fb881efc3569506d426ee5155d
🚀 fanquake merged a pull request: "build: Use correct variable name"
(https://github.com/bitcoin/bitcoin/pull/30791)
💬 marcofleon commented on pull request "fuzz: Test headers pre-sync through p2p":
(https://github.com/bitcoin/bitcoin/pull/30661#issuecomment-2340343047)
Addressed comments by @instagibbs. Thanks for the review!
👍 dergoegge approved a pull request: "fuzz: Test headers pre-sync through p2p"
(https://github.com/bitcoin/bitcoin/pull/30661#pullrequestreview-2292103593)
reACK a97f43d63a6e835bae20b0bc5d536df98f55d8a0
💬 l0rinc commented on pull request "test: cover base32/base58/base64 with symmetric roundtrip fuzz (and padding) tests":
(https://github.com/bitcoin/bitcoin/pull/30746#discussion_r1751765403)
Thanks for the review!

> is usually not a good idea

Can you please expand on that? I don't mind splitting, as you've recommended, but I have to understand what the current drawbacks are.
🤔 BrandonOdiwuor reviewed a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2292156787)
Concept ACK
👋 brunoerg's pull request is ready for review: "fuzz: wallet: add target for spkm migration"
(https://github.com/bitcoin/bitcoin/pull/29694)
💬 fanquake commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2340408539)
Yea. Seems like it is also not a difference between PRs and merge commits, given that now the current merge run is slow, and a simultaneously running PR is fast:
ASAN (most recent merge commit): [` Total Test time (real) = 1086.64 sec`](https://github.com/bitcoin/bitcoin/actions/runs/10790934929/job/29927228108#step:7:4710).
ASAN (current PR run): [` Total Test time (real) = 510.99 sec`](https://github.com/bitcoin/bitcoin/actions/runs/10790805641/job/29926810997?pr=30433#step:7:4710).
💬 brunoerg commented on pull request "fuzz: wallet: add target for spkm migration":
(https://github.com/bitcoin/bitcoin/pull/29694#discussion_r1751778401)
Just changed it to use `LegacyDataSPKM`. Note that this target is about the spkm migration only, not the whole process.
💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2340420173)
It also doesn't look cmake related, because the latest 28.x build also times out: https://github.com/bitcoin/bitcoin/runs/29755145359
📝 hebasto opened a pull request: "build: Improve `ccache` performance for different build directories"
(https://github.com/bitcoin/bitcoin/pull/30861)
This PR makes it possible to reuse the `ccache` cache populated during a build in another build directory:
```
$ cmake -B build_1 -DWITH_CCACHE=ON
$ cmake --build build_1 -t bitcoind
$ cmake -B build_2 -DWITH_CCACHE=ON
$ cmake --build build_2 -t bitcoind # 100% cache hit rate is expected.
```

The first commit has been picked from https://github.com/bitcoin/bitcoin/pull/30803.

Please refer to https://github.com/bitcoin/bitcoin/pull/20353 for historical context.

Addresses this [com
...
💬 hebasto commented on pull request "build: Unify `-logsourcelocations` format":
(https://github.com/bitcoin/bitcoin/pull/30811#issuecomment-2340474631)
> However, ccache does _not_ hit across two different build dirs, compiling the same commit.

Fixed in https://github.com/bitcoin/bitcoin/pull/30861.
💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#issuecomment-2340474919)
ACK faa32adbcf4c04f0a426eaba4a43b29a293de72b

<details>
<summary>Details</summary>

* brace initialization in base.cpp
* pos*t*itional typo fix
* added ConstevalFormatString overload

</details>
📝 hebasto converted_to_draft a pull request: "build: Unify `-logsourcelocations` format"
(https://github.com/bitcoin/bitcoin/pull/30811)
Closes https://github.com/bitcoin/bitcoin/issues/30799:

```
$ ./build/src/bitcoind -logsourcelocations -asmap=/tmp/no_file 2>&1 | head -1
2024-09-04T07:01:03Z [init/common.cpp:149] [LogPackageVersion] Bitcoin Core version v28.99.0-b94fe85fbe2f (release build)
```
💬 l0rinc commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751802942)
can we still have errors like [this](https://github.com/bitcoin/bitcoin/blob/master/src/logging.h#L253-L265) after this change?
👍 theStack approved a pull request: "test: Add explicit onion bind to p2p_permissions"
(https://github.com/bitcoin/bitcoin/pull/30805#pullrequestreview-2292244981)
Tested ACK e9ad9e01409eaa2009f75deb1cf834983c5a6137
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751829061)
Yes, in theory. The goal of this pull request is to make the less likely. According to the pull request description, "This is the first step toward https://github.com/bitcoin/bitcoin/issues/30530 and a follow-up will apply the approach to the other places."
💬 maflcko commented on pull request "util: Use consteval checked format string in FatalErrorf":
(https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1751830708)
Thanks, adjusted in the last push.