Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 maflcko commented on issue "MSVC 17.12.0 internal compiler error ":
(https://github.com/bitcoin/bitcoin/issues/31303#issuecomment-2601818018)
> I've managed to provide a proper repro and submitted another issue: https://developercommunity.visualstudio.com/t/C1001:-Internal-compiler-error-with-st/10830350.

Thanks! Looks like it is already pending a release.
👍 hebasto approved a pull request: "ci: Supply `--platform` argument to `docker` commands."
(https://github.com/bitcoin/bitcoin/pull/31657#pullrequestreview-2561771898)
ACK 6e29de21010fc5213176a6ba29f754ca72612ea0
💬 hebasto commented on issue "cmake: Compiling for test coverage (low-priority workaround exists)":
(https://github.com/bitcoin/bitcoin/issues/31638#issuecomment-2601877525)
> ### Current behaviour
>
> When following Compiling for Test Coverage instructions: [doc/developer-notes-CompilingForTestCoverage](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage)
>
> I encounter error:
>
> ```
> CMake Error: File /home/alicebob/wkspc1/presets/bitcoin/cmake/cov_tool_wrapper.sh.in does not exist.
> CMake Error at /home/alicebob/wkspc1/build/bitcoin/CoverageInclude.cmake:14 (configure_file):
> configure_file Problem configuri
...
💬 maflcko commented on issue "Intermittent issue in p2p_i2p_ports.py AssertionError: [node 0] Expected messages "['Error connecting to [...].b32.i2p:0: Cannot connect to 127.0.0.1:60000']" does not partially match log:":
(https://github.com/bitcoin/bitcoin/issues/30030#issuecomment-2601878490)
Reproduced locally:


```
sh-5.2# history
1 curl -L -O "https://drahtbot.space/temp_scratch/p2p_i2p_ports_14.tar.zstd"
2 tar -xvf ./p2p_i2p_ports_14.tar.zstd
3 ./test/functional/combine_logs.py ./p2p_i2p_ports_14
```

```
test 2025-01-16T23:02:00.696000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/ci_container_base/test/functional/test_framework/test_framework.py", l
...
💬 laanwj commented on pull request "optimization: precalculate SipHash constant XOR with k0 and k1 in SaltedOutpointHasher":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-2601924070)
Sorry for taking so long.

FWIW, Code review ACK bc959f5ba9a62b41b36cc662aa176b03969be176. Moving the magic numbers to constants seems like a good idea tome. It's good to add the benchmarks. And although the speed change is neglilible (as would be expected from factoring out ~four xor instructions), it doesn't complicate the code much either.
💬 hebasto commented on pull request "depends: Always set `CMAKE_BUILD_TYPE=None` globally":
(https://github.com/bitcoin/bitcoin/pull/31661#issuecomment-2601944555)
> > For packages that do not set a default build type, this change does not affect their behaviour.
>
> Are you sure this is true? It definitely _could_ affect behavior. I'm wondering if we should do it per-package, even if it's every package, as a sign-off that we've actually checked that it does the right thing.

Reworked.
💬 Sjors commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2601998899)
On Windows it's a bit ugly to show the full path:

<img width="957" alt="help" src="https://github.com/user-attachments/assets/4a1cb5fe-b7e6-44f2-b860-05e10e1253cb" />

None of the commands work anymore though, except `version` and `help`. The commands `daemon` and `gui` do not start anything. When I start the GUI manually through the desktop item, the command `bitcoin rpc help` doesn't return anything.

This also doesn't return anything: `.\bitcoin.exe daemon --help`

I would expect try
...
📝 l0rinc reopened a pull request: "optimization: precalculate SipHash constant XOR with k0 and k1 in SaltedOutpointHasher"
(https://github.com/bitcoin/bitcoin/pull/30442)
Continuing the IBD-related micro-optimizations (started in https://github.com/bitcoin/bitcoin/pull/30326), here I'm precalculating the SipHash constants XOR with `k0` and `k1` for the map hash calculations .

before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 60.68 | 16,481,085.68 | 0.3% | 11.05 | `SaltedOutpointHasherBenchmark_create_set`
|
...
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r1922169426)
> Could make sense to run this before the `tee /tmp/iwyu_ci.out` command, given that it errors?

This approach allows a developer to check other includes, not only the first one that fails.

> Maybe a `-- --keep-going` could be used for a slightly better output, if stuff is possibly missing?

I'm not sure what criteria you are using to define "better" in this case. The current branch adds one more config/build round details to the CI log and the following:
1. A message like the one posted
...
📝 Eunovo opened a pull request: "Benchmark Chainstate::ConnectBlock duration"
(https://github.com/bitcoin/bitcoin/pull/31689)
Measure ConnectBlock performance of full taproot blocks without signature caching This will allow testing and measurement of performance improvement of batch verification of schnorr signatures

> cmake -B build -DBUILD_BENCH=ON
> cmake --build build
> build/src/bench/bench_bitcoin -filter=ConnectBlock

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests sh
...
📝 Eunovo converted_to_draft a pull request: "Benchmark Chainstate::ConnectBlock duration"
(https://github.com/bitcoin/bitcoin/pull/31689)
Measure ConnectBlock performance of full taproot blocks without signature caching This will allow testing and measurement of performance improvement of batch verification of schnorr signatures

> cmake -B build -DBUILD_BENCH=ON
> cmake --build build
> build/src/bench/bench_bitcoin -filter=ConnectBlock

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests sh
...
💬 laanwj commented on pull request "fuzz: add targets for PCP and NAT-PMP port mapping requests":
(https://github.com/bitcoin/bitcoin/pull/31676#issuecomment-2602079647)
Code review ACK.
The extra commits "[pcp: make the ToString method const](https://github.com/bitcoin/bitcoin/pull/31676/commits/4e9a4cc23a49cf1147570a94c89f680843c76f5a)" and "[pcp: make NAT-PMP error codes uint16_t](https://github.com/bitcoin/bitcoin/pull/31676/commits/19423e86fdb80b6666d9c5fa6439e7e909a56321)" also LGTM.

Have tried to investigate what is happening in @dergoegge 's case but was unable to find the issue, how `PCPSendRecv` can return a response while the buffer is uninitiali
...
💬 maflcko commented on pull request "refactor: Enforce readability-avoid-const-params-in-decls":
(https://github.com/bitcoin/bitcoin/pull/31650#issuecomment-2602087719)
rebased to drop hunk which was already merged via one of the conflicting pulls
💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r1922217359)
I think the message you posted in the PR description only shows a single error in a single dependency. I am wondering what happens if there is more than one error in different targets, possibly ones where cmake will never get to print the second error.

I am asking, because after the cmake migration the behavior of `make -k` changed. Previously, everything would be attempted to be built. Now, cmake will skip and exit early in some cases.
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2602130705)
ACK fd3103142de8bcb92b5db6a6501c4c66abc12abd
💬 itornaza commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2602154368)
> From my understanding, the issue arises because the `make` provided by Xcode does not set the `.SHELLFLAGS` variable properly.

With your insight, I have tested just setting `.SHELLFLAGS=-c` at the top of `depends/hosts/darwin.mk`

The error seems now to be suppressed for the non-GNU Make version that macos uses.

Any thoughts?
👍 TheCharlatan approved a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2562133874)
ACK 6f0513613a98

Guix build (aarch64):
```
a22efc082966018f6ddd17d1a7092ffda56900895dd4694080244257e1c5c974 guix-build-6f0513613a98/output/aarch64-linux-gnu/SHA256SUMS.part
ad65ac733f93931c5fcfa09afc2a1f8bed76cd5280be9b569ff9bdf15dec3abe guix-build-6f0513613a98/output/aarch64-linux-gnu/bitcoin-6f0513613a98-aarch64-linux-gnu-debug.tar.gz
abc9037335e719927525c0474a7959db57a6f3f64d9522474ede4d88e721f73c guix-build-6f0513613a98/output/aarch64-linux-gnu/bitcoin-6f0513613a98-aarch64-linux-gn
...
📝 maflcko opened a pull request: "[doc] Amend notes on benchmarking"
(https://github.com/bitcoin/bitcoin/pull/31690)
This updates the example output and gives some more context on the motivation and larger picture of benchmarks.
💬 maflcko commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2602211600)
Not sure why it was closed. It seemed like a useful doc update to me, but maybe I am missing something. Feel free to ACK/NACK https://github.com/bitcoin/bitcoin/pull/31690, which is just the doc update from here cherry-picked.
💬 hebasto commented on pull request "depends: Override default build type for `libevent`":
(https://github.com/bitcoin/bitcoin/pull/31661#issuecomment-2602221386)
My Guix build:
```
aarch64
9642c7c336c6dda11fd7f0e085910ab9f4edf73d41401a6df333f7ac979fd845 guix-build-d7d3bb1bbcc0/output/aarch64-linux-gnu/SHA256SUMS.part
f5f224be7a011f1465f995bb21a9f3af18567911272d795a937913370012d83f guix-build-d7d3bb1bbcc0/output/aarch64-linux-gnu/bitcoin-d7d3bb1bbcc0-aarch64-linux-gnu-debug.tar.gz
7cd81a1879b84a00a9a623f056f8f06127377cedd87220d577a32a81beda79de guix-build-d7d3bb1bbcc0/output/aarch64-linux-gnu/bitcoin-d7d3bb1bbcc0-aarch64-linux-gnu.tar.gz
1d75c0d8
...