Bitcoin Core Github
42 subscribers
126K links
Download Telegram
💬 maflcko commented on pull request "ci: Supply `--platform` argument to `docker` commands.":
(https://github.com/bitcoin/bitcoin/pull/31657#discussion_r1921975444)
> but it seems docker infers the arch if an OS is specified, and doesn't just fall back on a cache hit for that OS:

Thanks for checking! In case this ever changes, it should be trivial to re-add the code you wrote by re-applying the diff: https://github.com/bitcoin/bitcoin/compare/1299bd89f8661028f9e1826c51a29d4e3fe39cdc..e1c7c29727faf2d0a5c6fa01505418aac26958fe
💬 maflcko commented on pull request "ci: Supply `--platform` argument to `docker` commands.":
(https://github.com/bitcoin/bitcoin/pull/31657#issuecomment-2601759698)
lgtm ACK 6e29de21010fc5213176a6ba29f754ca72612ea0
:lock: fanquake locked an issue: "Gift me btc buy"
(https://github.com/bitcoin/bitcoin/issues/31687)
📝 fanquake locked a pull request: "Update and rename README.md to GET /api/v1/lightning/channels?public_…"
(https://github.com/bitcoin/bitcoin/pull/31688)
```javascript
…key=:pubKey&status=:channelStatus

Bitcoin Core integration/staging tree
=====================================

https://bitcoincore.org

For an immediately usable, binary version of the Bitcoin Core software, see https://bitcoincore.org/en/download/.

What is Bitcoin Core?
---------------------

Bitcoin Core connects to the Bitcoin peer-to-peer network to download and fully validate blocks and transactions. It also includes a wallet and graphical user interface, which
...
🤔 hebasto reviewed a pull request: "fuzz: add cstdlib to FuzzedDataProvider"
(https://github.com/bitcoin/bitcoin/pull/31448#pullrequestreview-2561696993)
Post-merge ACK bb7e686341e437b2e7aae887827710918c00ae0f.
💬 maflcko commented on pull request "init: Lock blocksdir in addition to datadir":
(https://github.com/bitcoin/bitcoin/pull/31674#discussion_r1922011567)
Sorry, I completely mis-read the test in the last commit. All good, please resolve.
💬 maflcko commented on pull request "init: Lock blocksdir in addition to datadir":
(https://github.com/bitcoin/bitcoin/pull/31674#discussion_r1922011646)
I suggested it, because it avoids the two negations and it is shorter, so it reads more natural, at least to me. But it is just a nit.
💬 maflcko commented on pull request "init: Lock blocksdir in addition to datadir":
(https://github.com/bitcoin/bitcoin/pull/31674#issuecomment-2601784981)
No change since my last review. I just mis-read the test commit.

re-ACK 2656a5658c14b43c32959db7235e9db55a17d4c8 🐓

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A
...
🤔 hebasto reviewed a pull request: "ci: Supply `--platform` argument to `docker` commands."
(https://github.com/bitcoin/bitcoin/pull/31657#pullrequestreview-2561731819)
Tested 6e29de21010fc5213176a6ba29f754ca72612ea0 on Ubuntu 24.10:
```
$ docker --version
Docker version 27.1.1, build 6312585
$ env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_arm.sh" ./ci/test_run_all.sh'
++++ dirname ./ci/test/00_setup_env.sh
+++ cd ./ci/test/../../
+++ pwd
++ BASE_READ_ONLY_DIR=/home/hebasto/git/bitcoin
++ export BASE_READ_ONLY_DIR
++ export BASE_ROOT_DIR=/ci_container_base
++ BASE_ROOT_DIR=/ci_container_base
++ export DEPEND
...
💬 maflcko commented on pull request "ci: Supply `--platform` argument to `docker` commands.":
(https://github.com/bitcoin/bitcoin/pull/31657#issuecomment-2601808032)
> 0.099 exec /usr/bin/bash: exec format error

This is unrelated to the changes in this pull request, as it happens on master as well.

You'll have to set up qemu. Possibly with `podman run --rm --privileged docker.io/multiarch/qemu-user-static --reset -p yes`
💬 maflcko commented on issue "CI: Improve documentation around replicating CI locally":
(https://github.com/bitcoin/bitcoin/issues/31199#issuecomment-2601811725)
Also, setup of qemu is missing: https://github.com/bitcoin/bitcoin/pull/31657#issuecomment-2601808032
💬 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
...