Bitcoin Core Github
44 subscribers
121K links
Download Telegram
👍 i-am-yuvi approved a pull request: "test: deduplicates p2p_tx_download constants"
(https://github.com/bitcoin/bitcoin/pull/31758#pullrequestreview-2582964859)
ACK efeaf2e0c1f0de0d7c3819eb9beaefee2ddf0710

```
TEST | STATUS | DURATION

p2p_tx_download.py | ✓ Passed | 33 s

ALL | ✓ Passed | 33 s (accumulated)
```
👍 i-am-yuvi approved a pull request: "test: make sure we are on sync with a peer before checking if they have sent a message"
(https://github.com/bitcoin/bitcoin/pull/31760#pullrequestreview-2583021496)
ACK 3f4b104b1b7e1b87c0be8e395e02b6ae3c5d7b08
💬 Sjors commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2623772947)
@fjahr the new rule only applies to blocks at `height % 2016 == 0`, so I'm confused about the heights you found.

The change here only affects what miners put in their blocks, it doesn't change the consensus check. So it shouldn't be dangerous in any case.
💬 Sjors commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2623801480)
The "test each commit" job jails on 36db25f0bcaab6af9358faaad9214d5b77a71cb2 which is part of #31741:

```
Start 5: mptest
Unable to find executable: /home/runner/work/bitcoin/bitcoin/build/src/ipc/libmultiprocess/test/mptest
```

The ASan + LSan job also fails over missing mptest.

The macOS 14 native no depends job fails with https://github.com/chaincodelabs/libmultiprocess/issues/138.

The tidy job found several issues:

```
[13:27:31.569] /ci_container_base/src/ipc/libmultipr
...
💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2623803509)
I found several issue while unleashing the CI on these changes: https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2623801480

Let me know which ones you want to address here, and which can be dealt with later.
💬 Sjors commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2623824284)
> First of all why is the default position that we should ship a new binary on short notice right before feature freeze

#30975 was opened in October and from the start had a clear intention of shipping these binaries in a release. This goes back to several discussions in the months before that, including [at core dev](https://btctranscripts.com/bitcoin-core-dev-tech/2024-10/multiprocess-binaries) (the minutes aren't precise, so don't take them as gospel).

So it wasn't short notice. However, ov
...
📝 maximevtush opened a pull request: "Update LICENSE"
(https://github.com/bitcoin/bitcoin/pull/31761)
Updated the copyright year to 2025 in the LICENSE file
fanquake closed a pull request: "Update LICENSE"
(https://github.com/bitcoin/bitcoin/pull/31761)
📝 fanquake locked a pull request: "Update LICENSE"
(https://github.com/bitcoin/bitcoin/pull/31761)
Updated the copyright year to 2025 in the LICENSE file
💬 hebasto commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2624104354)
>With this PR, these would now all need to figure out where the binaries live, and that seems to be significantly less trivial to do than detecting autotools or cmake.

Something like this should work:
```sh
[ -e build/bin/test_bitcoin ] && ln -s $(realpath build/bin/test_bitcoin) build/src/test/
```

> I would've preferred that this was done with cmake, or immediately after it, than now...

I agree with you in hindsight.

> ... where we're a few weeks before feature freeze for 29.0.
...
⚠️ stickies-v opened an issue: "bug: cmake installs empty manpages for non-configured targets"
(https://github.com/bitcoin/bitcoin/issues/31762)
### Is there an existing issue for this?

- [x] I have searched the existing issues

### Current behaviour

When configuring cmake with a subset of targets (e.g. kernel-only), a blanket `cmake --install` will still install the placeholder manpages files for all other targets.

For example:

```
cmake -B build -DBUILD_KERNEL_LIB=ON -DBUILD_BENCH=OFF -DBUILD_DAEMON=OFF -DBUILD_CLI=OFF -DBUILD_GUI=OFF -DBUILD_FUZZ_BINARY=OFF -DBUILD_TESTS=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DBUILD_UTIL_CHAINSTATE=
...
👍 TheCharlatan approved a pull request: "rpc: have getblocktemplate mintime account for timewarp"
(https://github.com/bitcoin/bitcoin/pull/31600#pullrequestreview-2583553574)
ACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9
📝 Sjors opened a pull request: "Pass custom DEP_OPTS and CONFIG_FLAGS to guix-build"
(https://github.com/bitcoin/bitcoin/pull/31763)
This allows building deterministic binaries with multiprocess like so:

```sh
DEP_OPTS="MULTIPROCESS=1" contrib/guix/guix-build
```

Additionally custom options can be passed to `cmake -B` using `CONFIG_FLAGS`. Example (turns off external signer support, the rest is default):

```sh
CONFIG_FLAGS="-DENABLE_EXTERNAL_SIGNER=OFF -DREDUCE_EXPORTS=ON -DBUILD_BENCH=OFF -DBUILD_GUI_TESTS=OFF -DBUILD_FUZZ_BINARY=OFF" contrib/guix/guix-build
```

The `CONFIG_FLAGS` is less useful in practice,
...
👋 Sjors's pull request is ready for review: "Pass custom DEP_OPTS and CONFIG_FLAGS to guix-build"
(https://github.com/bitcoin/bitcoin/pull/31763)
💬 Sjors commented on pull request "multiprocess: Add libmultiprocess git subtree":
(https://github.com/bitcoin/bitcoin/pull/31741#issuecomment-2624277683)
If I cherry-pick #31763 on top of this, I'm unable to make a Guix build with `DEP_OPTS="MULTIPROCESS=1" contrib/guix/guix-build`, at least not with the hosts I tried (`x86_64-linux-gnu`, `aarch64-linux-gnu` and `arm64-apple-darwin`).

```
Checking that we can connect to the guix-daemon...

Hint: If this hangs, you may want to try turning your guix-daemon off and on
again.

make: Entering directory '/home/sjors/bitcoin/depends'
make[1]: Entering directory '/home/sjors/bitcoin/depen
...
📝 hebasto opened a pull request: "cmake: Generate man pages at install time"
(https://github.com/bitcoin/bitcoin/pull/31764)
This PR enables the man pages to be generated on the fly during installation.

As the GNU help2man tool is required to generate man pages, the `INSTALL_MAN` build option is disabled by default.

Addresses this comment:https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456

Fixes https://github.com/bitcoin/bitcoin/issues/31762.
💬 hebasto commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#issuecomment-2624395483)
cc @theuni as an author of the TODO [comment](https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/CMakeLists.txt#L456).
💬 fanquake commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#discussion_r1935544996)
How does this change the guix dep graph?
💬 fanquake commented on pull request "cmake: Generate man pages at install time":
(https://github.com/bitcoin/bitcoin/pull/31764#discussion_r1935549376)
> --skip-missing-binaries

I don't think we should be hardcoding a flag to ignore/skip errors into a script that you're making a requirement for release builds.