Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 TheCharlatan commented on pull request "build, ci: Fix linking `bitcoin-chainstate.exe` to `bitcoinkernel.dll` on Windows":
(https://github.com/bitcoin/bitcoin/pull/31158#issuecomment-2728786844)
I'm not sure about the current approach of this change. I think we could go the way of manually exporting symbols in our c++ code, but then I would call into question the need for this project to ship a C header in the first place. Having two symbol-exporting headers, one for our internal code, and one for a potential future C header seems very confusing.

Also, as maflcko already said, we could just not directly use these globals from code that links to the shared library. If I understand th
...
💬 maflcko commented on issue "Release Schedule for 29.0":
(https://github.com/bitcoin/bitcoin/issues/31029#issuecomment-2728893884)
@jsarenik If you just want to build a tag or commit for yourself, you can use the `doc/build-*.md` instructions, or the guix build (`contrib/guix/README.md`). With the guix build you can even contribute attestations, see https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#building. If you are waiting on the pre-compiled rc binary, I suspect it will be available some time this week.
💬 glozow commented on pull request "[29.x] backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/32062#issuecomment-2728946957)
> Can also pull in [bitcoin-core/gui#858](https://github.com/bitcoin-core/gui/pull/858) (if not this rc than next).

added
💬 suiyuan1314 commented on pull request "docs: fix typos":
(https://github.com/bitcoin/bitcoin/pull/32006#discussion_r1998453937)
Thanks for your reply, I have removed this.
💬 vasild commented on pull request "net: Only attempt v2->v1 transport downgrade if online":
(https://github.com/bitcoin/bitcoin/pull/32073#discussion_r1998469228)
`fNetworkActive` will be read 2 times in `CConnman::DisconnectNodes()`. Given that it can change any time, it could happen that is has one value in the first read and another value here in the second read (this is actually inside a loop so it is many second reads, one for each node). This makes it a bit involved to reason all possible outcomes and could produce unexpected results.

I think it would be better if the variable is read just once in the `CConnman::DisconnectNodes()` method. This wi
...
🤔 vasild reviewed a pull request: "net: Only attempt v2->v1 transport downgrade if online"
(https://github.com/bitcoin/bitcoin/pull/32073#pullrequestreview-2690008496)
Approach ACK 0f4e20728d231935f0140845370090a760ae93af
💬 maflcko commented on pull request "docs: fix typos":
(https://github.com/bitcoin/bitcoin/pull/32006#discussion_r1998471816)
You'll have to adjust the pull title, description and commit message as well to include the rationale. Removing a section is a bit different from just a typo fix.
💬 TheCharlatan commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#discussion_r1998479545)
@hebasto should we unset `CMAKE_RUNTIME_OUTPUT_DIRECTORY` here to avoid `mpgen` and `mpgen` landing in the `bin/` directory? Maybe it would be better to move the `libmultiprocess` subtree up one level?
💬 Sjors commented on pull request "[29.x] backports and rc2":
(https://github.com/bitcoin/bitcoin/pull/32062#issuecomment-2729135997)
ACK 74df31cb0bdef9cce31ae62ed71a1e386cba0274

Don't forget to add https://github.com/bitcoin-core/gui/pull/858 to the PR description.
👍 hebasto approved a pull request: "[29.x] backports and rc2"
(https://github.com/bitcoin/bitcoin/pull/32062#pullrequestreview-2690117498)
ACK 74df31cb0bdef9cce31ae62ed71a1e386cba0274, I have reviewed the code and it looks OK.

I've backported PRs and generated `contrib/devtools/gen-bitcoin-conf.sh` locally and got zero-diff with this PR branch.
💬 hebasto commented on pull request "build: use make < 3.82 syntax for define directive":
(https://github.com/bitcoin/bitcoin/pull/32070#issuecomment-2729155648)
> But note that I run the stock macOS GNU make all the time, as do many other devs. So new issues are likely to be caught during review, though they'll still waste time debugging.

FWIW, the stock macOS Make has [other issues](https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849).
💬 Sjors commented on pull request "build: use make < 3.82 syntax for define directive":
(https://github.com/bitcoin/bitcoin/pull/32070#issuecomment-2729165254)
Is there a way we can pick up a modern homebrew version of make without forcing macOS devs and users to change their default make? (and without the use of env variables)
👍 willcl-ark approved a pull request: "ci: [lint] Use Cirrus dockerfile cache"
(https://github.com/bitcoin/bitcoin/pull/31948#pullrequestreview-2690146465)
ACK fa3b4427158d48f7d899582580f8f6a7b1bc981d

Sensible change to speedup the lint task startup.

The change looks correct, and to me it looks like it also works, judging by the log lines:

```log
11:51:41 PM Successfully pulled image "gcr.io/cirrus-ci-community/bitcoin/bitcoin/ci/lint_imagefile:714fc33ccc0872eef4aa515b2c226494648ec0f52aa6d3afbaf779c6d1a976b3" in 17.583s (18.557s including waiting). Image size: 699506090 bytes.
```

...and from the "dependency run" step, which is buildi
...
💬 hebasto commented on pull request "build: use make < 3.82 syntax for define directive":
(https://github.com/bitcoin/bitcoin/pull/32070#issuecomment-2729173888)
> Is there a way we can pick up a modern homebrew version of make without forcing macOS devs and users to change their default make? (and without the use of env variables)

I imagine something like that:
```
gmake -C depends -j 4
```
🤔 l0rinc reviewed a pull request: "fuzz: Speed up *_package_eval fuzz targets a bit"
(https://github.com/bitcoin/bitcoin/pull/31457#pullrequestreview-2690077410)
I will run the `tx_package_eval` fuzz corpora a bit later, but my previous benchmarks indicated that unordered map/set may not always be faster.

Is this a correct way to measure the speed difference (on a Mac)?

```bash
git clone --depth=1 https://github.com/bitcoin-core/qa-assets
cmake --preset=libfuzzer-nosan \
-DCMAKE_C_COMPILER="$(brew --prefix llvm)/bin/clang" \
-DCMAKE_CXX_COMPILER="$(brew --prefix llvm)/bin/clang++" \
-DCMAKE_EXE_LINKER_FLAGS="-fuse-ld=lld"
cmake --bui
...
💬 l0rinc commented on pull request "fuzz: Speed up *_package_eval fuzz targets a bit":
(https://github.com/bitcoin/bitcoin/pull/31457#discussion_r1998511564)
While I was investigating https://github.com/bitcoin/bitcoin/pull/31682/files#diff-c613867a2f35f2221c4e37262bee556e5b472aa95cd95e1eb0cbf8fe49ad83ffL41 I have also measured something like this, i.e. `std::set<COutPoint>` vs `std::unordered_set<COutPoint, SaltedOutpointHasher>` but interestingly I found the opposite - maybe it's size dependent and small trees are faster than the `SipHash` calculation:

<details>
<summary>`CheckBlockBench` measurements repeated 3x</summary>

> std::set<COutPo
...
💬 l0rinc commented on pull request "fuzz: Speed up *_package_eval fuzz targets a bit":
(https://github.com/bitcoin/bitcoin/pull/31457#discussion_r1998549014)
If you think `unordered_map` is indeed faster (I'll measure it once my benchmarking servers will be available), we could change the `std::set` instances as well - and given that they're array based, we might as well pre-allocate them.

<details>
<summary>Details</summary>

```patch
diff --git a/src/test/fuzz/package_eval.cpp b/src/test/fuzz/package_eval.cpp
--- a/src/test/fuzz/package_eval.cpp (revision 147655098edde6feffbbe2c601d5874a7d3a3c5c)
+++ b/src/test/fuzz/package_eval.cpp (date
...
💬 suiyuan1314 commented on pull request "docs: remove CI passing requirement for PR merging":
(https://github.com/bitcoin/bitcoin/pull/32006#discussion_r1998570290)
I have changed the commit message, the PR title and PR description, are they okay now?

Thanks for your review ^_^
📝 l0rinc opened a pull request: "Add `deterministic-fuzz-coverage/Cargo.lock` file to git"
(https://github.com/bitcoin/bitcoin/pull/32082)
This untracked file generated from https://github.com/bitcoin/bitcoin/pull/31836/files#diff-46025ad2b0c46d10d0fd6585c2985aab8044c203e4758f2b4f052ad173557a15 prevents branch switches.

Wanted to `.gitignore` at first, but found a similar commit in
https://github.com/bitcoin/bitcoin/commit/bbbbdb0cd57d75a06357d2811363d30a498f4499#diff-1ad842b18e768175e065568f0c834b736881839873b8bb8d36c8ebb43abbefa1
👍 vasild approved a pull request: "build: Drop option to disable hardening."
(https://github.com/bitcoin/bitcoin/pull/32071#pullrequestreview-2690198569)
ACK ecf2046d4b5c43ddf64f62f09cd3ed70dd5caafb