b8710201fb guix: disable timezone tools & profiling in glibc (fanquake)
23b8a424fb guix: bump glibc 2.31 to 7b27c450c34563a28e634cccb399cd415e71ebfe (fanquake)
Pull request description:
An additional commit has been backported to the 2.31 branch:
https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/release/2.31/master.
Pass `--disable-timezone-tools`: removes `var/profiles/x86_64-linux-gnu/sbin/zdump`.
Pass `--disable-profile`: profiling is disabled by default, but make that explicit.
ACKs for top commit:
theuni:
utACK b8710201fb
hebasto:
ACK b8710201fb.
Tree-SHA512: 0d9a0e7451cc42384bbdd0b46c740c7aa964dc12e3f0376de586bf90e57799ebb04675892861cb38a53b5ca0e265061fa7111596cf1c94171303d0d048785ab4
be1a2e5dfb doc: Install `py3-zmq` port on OpenBSD for `interface_zmq.py` (Hennadii Stepanov)
Pull request description:
On OpenBSD, Python's `zmq` module is provided as a separate [port](https://www.ports.to/path/net/py-zmq,python3.html).
This PR updates the OpenBSD Build Guide to include this port, enabling the `interface_zmq.py` functional test.
Also updates the documented OpenBSD version.
ACKs for top commit:
theStack:
Tested ACK be1a2e5dfb
Tree-SHA512: 4d560385b94e8c7491aa19d2157d8a799617e08136601dc565a909d4c74e12582a1d273bc97ad7c2d0e57c5cf7377560ba02ef58c12f8991652322553740d2ba
e196190a28 cmake: Remove unused `BUILD_TESTING` variable from "dev-mode" preset (Hennadii Stepanov)
Pull request description:
On the master branch @ bb57017b29:
```
$ cmake -B build --preset dev-mode -DWITH_MULTIPROCESS=OFF
<snip>
-- Configuring done (12.0s)
-- Generating done (0.1s)
CMake Warning:
Manually-specified variables were not used by the project:
BUILD_TESTING
-- Build files have been written to: /home/hebasto/git/bitcoin/build
```
This PR resolves the issue.
The removed `BUILD_TESTING` variable is a part of the [`CTest`](https://cmake.org/cmake/help/latest/module/CTest.html) module, which we do not include in the project.
ACKs for top commit:
TheCharlatan:
ACK e196190a28
Tree-SHA512: 8110a0f5bdcdd0844ce7dd75160a61d8b3aff95e12da1ec4d55c56c82da41145736da0fad072adeb97551c99e46683a3493435c3bac7d8e4e62ea6086f60fb7a
fadd568931 fuzz: Fix misplaced SeedRand::ZEROS (MarcoFalke)
Pull request description:
After commit fae63bf130 this must be placed even before test_setup. This is nice, because it makes the usage consistently appear in the first line.
The change is moving a `SeedRandomForTest(SeedRand::ZEROS)` to happen earlier. This is fine, because it will either have no effect, or make the code more deterministic, because after commit fae63bf, no other re-seeding other than `ZEROS` can happen in fuzz tests.
ACKs for top commit:
marcofleon:
Re ACK fadd568931
brunoerg:
code review ACK fadd568931
hodlinator:
ACK fadd568931
Tree-SHA512: 54eadf19a1e850157a280fb252ece8797f37a9a50d3b0a01aa2c267bacbe8ef4ddea6cf3faadcbaa4ab9f53148edf08e3cee5dfb3eae928db582adf8373a5206
81cea5d4ee Ensure m_tip_block is never ZERO (Sjors Provoost)
e058544d0e Make m_tip_block an std::optional (Sjors Provoost)
Pull request description:
Suggested in https://github.com/bitcoin/bitcoin/pull/31297#discussion_r1844244309
ACKs for top commit:
fjahr:
re-ACK 81cea5d4ee
tdb3:
code review re ACK 81cea5d4ee
l0rinc:
ACK 81cea5d4ee
Tree-SHA512: 31a75ba29e3d567bab32e4e7925a419d9d7a4d2d85ed1c1012116d8d22adc14d31d5b4ce5f6c499c994188dcd26a01cced05be74f94c892fc90ae17a6783a472
c991cea1a0 Remove processNewBlock() from mining interface (Sjors Provoost)
9a47852d88 Remove getTransactionsUpdated() from mining interface (Sjors Provoost)
bfc4e029d4 Remove testBlockValidity() from mining interface (Sjors Provoost)
Pull request description:
There are three methods in the mining interface that can be dropped. The Template Provider doesn't need them and other application should probably not use them either.
1. `processNewBlock()` was added in 7b4d3249ce, but became unnecessary with the introduction of interfaces::BlockTemplate::submitSolution in 7b4d3249ce.
Dropping it was suggested in https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2404460342
2. `getTransactionsUpdated()`: this is used in the implementation of #31003 `waitFeesChanged`. It's not very useful generically because the mempool updates very frequently.
3. `testBlockValidity()`: it might be useful for mining application to have a way to check the validity of a block template they modified, but the Stratum v2 Template Provider doesn't do that, and this method is a bit brittle (e.g. the block needs to build on the tip).
ACKs for top commit:
TheCharlatan:
Re-ACK c991cea1a0
ryanofsky:
Code review ACK c991cea1a0. Since last review, just rebased to avoid conflicts in surrounding code, and edited a commit message
tdb3:
code review ACK c991cea1a0
Tree-SHA512: 2138e54f920b26e01c068b24498c6a210c5c4358138dce0702ab58185d9ae148a18f04c97ac9f043646d40f8031618d80a718a176b1ce4779c237de6fb9c4a67
facb4d010c refactor: Move GuessVerificationProgress into ChainstateManager (MarcoFalke)
Pull request description:
Currently the function is standalone, which means any passed-in data like `TxData` or the block pointer needs to be taken from the `ChainstateManager` and passed in. This is currently verbose and may become even more verbose if the function is reworked in the future. As the function can not be called without a `ChainstateManager` in production code anyway, make it a member function on the class.
ACKs for top commit:
ryanofsky:
Code review ACK facb4d010c. Nice cleanup, that should make this code less awkward to work with
TheCharlatan:
ACK facb4d010c
danielabrozzoni:
reACK facb4d010c
Tree-SHA512: b17977e12cd7c6e308c47e6a1aa920acecd4442696e46d1f30bd7c201e9898ca2d581ff0bf2cc9f7334e146c1b0c50925adb849c8c17f65dcdf6877be1c5f776
processNewBlock was added in 7b4d3249ce, but became unnecessary with the introduction of interfaces::BlockTemplate::submitSolution in 7b4d3249ce.
getTransactionsUpdated() is only needed by the implementation of waitFeesChanged() (not yet part of the interface).
fa9e0489f5 refactor: Use immediate lambda to work around GCC bug 117966 (MarcoFalke)
Pull request description:
Currently the libstdc++ debug mode can only be used with version 11, or 15 (and later), due to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117966
This seems restrictive.
Add a temporary workaround for now, which makes the global (temporary) `std::span` local to a lambda.
ACKs for top commit:
theuni:
utACK fa9e0489f5
hebasto:
ACK fa9e0489f5, tested on Ubuntu 24.10.
vasild:
ACK fa9e0489f5
Tree-SHA512: 0cc54f089f329592f7a92a6f938b7de46c92d5362615310748225a42789e858e871432721e3101271b00871d523af5fbaadba2f52433fe79e928b1d1253931f6
fae63bf130 fuzz: Clarify that only SeedRandomStateForTest(SeedRand::ZEROS) is allowed (MarcoFalke)
fa18acb457 fuzz: Abort when using global PRNG without re-seed (MarcoFalke)
fa7809aeab fuzz: Add missing SeedRandomStateForTest(SeedRand::ZEROS) (MarcoFalke)
Pull request description:
This is the first step toward improving fuzz stability and determinism (https://github.com/bitcoin/bitcoin/issues/29018).
A fuzz target using the global test-only PRNG will now abort if the seed is re-used across fuzz inputs.
Also, temporarily add `SeedRandomStateForTest(SeedRand::ZEROS)` to all affected fuzz targets. This may slow down the libfuzzer leak detector, but it will disable itself after some time, or it can be disabled explicitly with `-detect_leaks=0`.
In a follow-up, each affected fuzz target can be stripped of the global random use and a local `RandomMixin` (or similar) can be added instead.
(Can be tested by removing any one of the re-seed calls and observing a fuzz abort)
ACKs for top commit:
hodlinator:
ACK fae63bf130
dergoegge:
utACK fae63bf130
marcofleon:
Tested ACK fae63bf130
Tree-SHA512: 4a0db69af7f715408edf4f8b08b44f34ce12ee2c79d33b336ad19a6e6bd079c4ff7c971af0a3efa428213407c1171f4e2837ec6a2577086c2f94cd15618a0892
f86678156a Check leaves size maximum in MerkleComputation (Sjors Provoost)
4d57288246 refactor: use CTransactionRef in submitSolution (Sjors Provoost)
2e81791d90 Drop TransactionMerklePath default position arg (Sjors Provoost)
39d3b538e6 Rename merkle branch to path (Sjors Provoost)
Pull request description:
This PR implements the refactors suggested in https://github.com/bitcoin/bitcoin/pull/30955#pullrequestreview-2354931253.
ACKs for top commit:
tdb3:
code review re-ACK f86678156a
itornaza:
re ACK f86678156a
ryanofsky:
Code review ACK f86678156a only changes since last review are a whitespace change and adding an Assume statement to check for size_t -> uint32_t overflows
Tree-SHA512: 661b5d5d0e24b2269bf33ab1484e37c36e67b32a7796d77ca3b1856d3043378b081ad43c32a8638b46fa8c0de51c823fd9747dd9fc81f958f20d327bf330a47c
52fd1511a7 test: drop scriptPubKeyIn arg from CreateNewBlock (Sjors Provoost)
ff41b9e296 Drop script_pub_key arg from createNewBlock (Sjors Provoost)
7ab733ede4 rpc: rename coinbase_script to coinbase_output_script (Sjors Provoost)
Pull request description:
Providing a script for the coinbase transaction is only done in test code and for (unoptimized) CPU solo mining.
Production miners use the `getblocktemplate` RPC which omits the coinbase transaction entirely from its block template, leaving it to external (pool) software to construct it.
This commit removes the `script_pub_key argument` from `createNewBlock()` in the Mining interface.
A coinbase script can still be passed via `BlockCreateOptions` instead. Tests are modified to do so.
ACKs for top commit:
ryanofsky:
Code review ACK 52fd1511a7. No change since last review other than rebase
TheCharlatan:
Re-ACK 52fd1511a7
vasild:
ACK 52fd1511a7
Tree-SHA512: c4b3a53774d9a5dc90950e77f47a64dbb68f971baffbb9a0d8f59332ef8e52d0c039130c925bde73135b3d0e79e65d91d1df30dc4cff13f32d8a72e5c56669d8
fa0e30b93a fuzz: Fix test_runner error reporting (MarcoFalke)
Pull request description:
The error reporting is confusing, because right now it prints:
https://cirrus-ci.com/task/4846031060336640?logs=ci#L4931
```
...
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 411, in <module>
main()
File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 199, in main
run_once(
File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 376, in run_once
assert len(done_stat) == 1
^^^^^^^^^^^^^^^^^^^
AssertionError
```
This is harmless, but confusing.
Fix it by collecting statistics only when the program has not aborted. (Can be reviewed with `--color-moved=dimmed-zebra`)
Also, reword the error message to align it with error messages in other test_runners in this repo.
ACKs for top commit:
dergoegge:
utACK fa0e30b93a
brunoerg:
code review ACK fa0e30b93a
marcofleon:
Tested ACK fa0e30b93a. Prints out the error for the target that crashed. Much clearer than the current error message.
Tree-SHA512: 5e8d3fc0e4837b3264ff0c3cb322fe7fe2ec7af48d35e2a14f82080d03ace793963c3314611b0a170a38e200497d7ba703d9c35c9a7ed3272d93e43f0f0e4c2b
2b9ff4a66d build: use `-mbig-obj` for mingw-w64 Debug builds (fanquake)
Pull request description:
Windows cross builds using `-O0` (`-DCMAKE_BUILD_TYPE=Debug`) currently fail to compile, as some objects have too many sections. As a convenience, add `-mbig-obj` to our compile flags when using the `Debug` build type, so that if someone tries to build this way, it will work.
This would also be needed if we switched the depends flags to -O0. (maybe in #29796).
`-mbig-obj`
> On PE/COFF target this option forces the use of big object
> file format, which allows more than 32768 sections.
Closes#28109. Seems unlikely that we are going to break up the relevant object files, and the main issue is still the inclusion of Boost.
ACKs for top commit:
theuni:
utACK 2b9ff4a66d
hebasto:
ACK 2b9ff4a66d, tested in the following scenarios:
Tree-SHA512: 9ad36de172629a8b7e5371fe3cd75ac2f3c29856040569052cc59e42825eec9121e012dd2178e00b163173c98e78f79dd16b8cee2c93daa2ee0d7e99799325cd
a10bb400e8 depends: Fix CXXFLAGS on NetBSD (Hennadii Stepanov)
Pull request description:
This PR corrects an issue where `CXXFLAGS` were mistakenly overridden by `CFLAGS`. This behaviour was introduced in 7e7b3e42fa (from https://github.com/bitcoin/bitcoin/pull/22380).
On the master branch:
```
$ gmake --no-print-directory -C depends print-x86_64_netbsd_CXXFLAGS
x86_64_netbsd_CXXFLAGS=-pipe -std=c11
```
With this PR:
```
$ gmake --no-print-directory -C depends print-x86_64_netbsd_CXXFLAGS
x86_64_netbsd_CXXFLAGS=-pipe -std=c++20
```
ACKs for top commit:
theuni:
utACK a10bb400e8
Tree-SHA512: 0c842db2965ebb0a58693394715922810235d9e5f2a7416fe258eb252dbd68ec04f90a0f7948abe938caf94a9194cca7deb53a08335c4404cce3a40c5cb44944
46e207d329 cmake: Link `bitcoin_consensus` as a library (Hennadii Stepanov)
Pull request description:
The [`TARGET_OBJECTS`](https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#genex:TARGET_OBJECTS) generator expression was introduced in the staging branch when we aimed to build the libbitcoinconsensus shared library. However, `bitcoin_consensus` is a `STATIC` library, not an `OBJECT` library.
This change updates the build system to link `bitcoin_consensus` normally to `test_bitcoin`, resolving [linking issues](https://github.com/bitcoin/bitcoin/issues/31456#issuecomment-2538798107) when building with clang-cl.
ACKs for top commit:
TheCharlatan:
ACK 46e207d329
theuni:
utACK 46e207d329
Tree-SHA512: b5400be8e8350f80c9fc8b66c4a22032a51578e409eb1817309116fbf0bddeb5fcadd5fda685c98859730ee6cc904adb29d54207387732c8b574a1feb2be906f
3353d4a5e9 depends: Ignore prefix directory on OpenBSD (Hennadii Stepanov)
Pull request description:
On OpenBSD, the prefix directory is named as follows:
```
$ gmake --no-print-directory -C depends print-x86_64_openbsd_prefix
x86_64_openbsd_prefix=/home/hebasto/dev/bitcoin/depends/amd64-unknown-openbsd7.6
```
This name does not match any pattern in `depends/.gitignore`.
This PR resolves this issue.
ACKs for top commit:
tdb3:
ACK 3353d4a5e9
theuni:
utACK 3353d4a5e9
theStack:
Tested ACK 3353d4a5e9🐟
Tree-SHA512: 82dfff1af974aa43c21e5e5a4483256d5ab4efdf1a15073fb864e635eff52eb8414346cda125f097af59e3342ac031a52683529f4e64df9fc60c8783fcd85e74
To avoid future code changes from reintroducing the ambiguity fixed
by the previous commit, mark m_tip_block private and Assume that
it's not set to uint256::ZERO.
Belt and suspenders for future code changes.
Currently this function is only called from TransactionMerklePath() which sets leaves to the block transactions, so the Assume always holds.
The TARGET_OBJECTS generator expression was introduced in the staging
branch when we aimed to build the libbitcoinconsensus shared library.
However, `bitcoin_consensus` is a STATIC library, not an OBJECT library.
This change updates the build system to link `bitcoin_consensus`
normally to `test_bitcoin`, resolving linking issues when building with
clang-cl.
1dd3af8fbc Add release note for #31223 (Martin Zumsande)
997757dd2b test: add functional test for -port behavior (Martin Zumsande)
0e2b12b92a net, init: derive default onion port if a user specified a -port (Martin Zumsande)
Pull request description:
This resolves#31133 (setups with multiple local nodes each using a different `-port` no longer working with v28.0, see the issue description for more details) by deriving the default onion listening port to be the value specified by `-port` incremented by 1 (idea by vasild / laanwj).
Note that with this fix, the chosen `-port` values of two local nodes cannot be adjacent, otherwise there will be port collisions again.
From the discussion in the linked issue, this was the most popular option, followed by doing nothing and telling affected users to change their setups to use `-bind` instead of `-port`. But more opinions are certainly welcome!
I think that if we decide to do something about the problem described in the issue, we should do so soon (in 28.1.), so I opened this PR.
Fixes#31133
ACKs for top commit:
achow101:
ACK 1dd3af8fbc
laanwj:
Tested ACK 1dd3af8fbc
tdb3:
Code review ACK 1dd3af8fbc
Tree-SHA512: 37fda2b23bbedcab5df3a401cf5afce66ae5318fb78f9660f83e3fd075b528e8156d7a0903f9a12ffe97ab5d83860587116b74af28670a1f4c2f0d1be4999f40
Windows cross builds using `-O0` currently fail to compile, as some
objects have too many sections. As a convenience, add `-mbig-obj` to
our compile flags when using the `Debug` build type, so that if someone
tries to build this way, it will work.
This would also be needed if we switched the depends flags to -O0.
`-mbig-obj`
> On PE/COFF target this option forces the use of big object
> file format, which allows more than 32768 sections.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
fa47baa03b ci: Bump centos gcc (MarcoFalke)
Pull request description:
Currently the centos stream9 CI task is using gcc-11. This is fine, because this is also the minimum supported.
However:
* There is already a CI task that is checking the minimum supported version: 62bd61de11/ci/test/00_setup_env_native_previous_releases.sh (L11-L12)
* The CI log is a bit useless, because it is mostly just `#warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform [-Werror=cpp]`. This makes it harder to spot real warnings, such as https://github.com/bitcoin/bitcoin/issues/31476
Fix both issues by using gcc-12.
ACKs for top commit:
hebasto:
ACK fa47baa03b.
Tree-SHA512: 573618efc949437d33365a24f77a26a9b68457f7fb9bd603ee92bc5f17fec73ccba114cafb900eddee3531af47508ce5c246def93268787cdfa2b99e6f45a13d
5cd9e95eea depends: update capnproto to 1.0.2 (fanquake)
Pull request description:
This fixes compilation on FreeBSD:
```bash
-- Build files have been written to: /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4
Building native_capnp...
gmake[1]: Entering directory '/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4'
[ 1%] Building CXX object src/kj/CMakeFiles/kj.dir/array.c++.o
[ 2%] Building CXX object src/kj/CMakeFiles/kj.dir/cidr.c++.o
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:112:71: error: member access into incomplete type 'const struct sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:112:51: note: forward declaration of 'sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:123:63: error: member access into incomplete type 'const struct sockaddr_in'
&reinterpret_cast<const struct sockaddr_in*>(addr)->sin_addr.s_addr);
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:123:44: note: forward declaration of 'sockaddr_in'
&reinterpret_cast<const struct sockaddr_in*>(addr)->sin_addr.s_addr);
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:133:69: error: member access into incomplete type 'const struct sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:133:49: note: forward declaration of 'sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
3 errors generated.
```
See: 1c19c362b4.
ACKs for top commit:
kevkevinpal:
Concept ACK [5cd9e95](5cd9e95eea)
theuni:
utACK 5cd9e95eea
ryanofsky:
Code review ACK 5cd9e95eea. Downloaded the file and checked the hash. Also followed theuni's lead and looked at the source changes which were very minor. It did look like thousands of lines changed in the autotools build, but this should not affect us as we are using the cmake build.
Tree-SHA512: 5d78887a9e950c8532c427b17969128de0c6d466ec5ffba85241457e8e19673c22ddb3493cdfce5086f57ba760eac5e91f703992b2f70f2a7c82ba885255279c
62b2d23edb wallet: Migrate non-HD keys to combo() descriptor (Ava Chow)
Pull request description:
Non-HD keys do not have an HD seed ID associated with them, so if this value is the null value (all 0s), then we should not perform any seed ID comparison that would result in excluding the keys from combo() migration.
This changes the migration of non-HD wallets (or blank wallets with imported private keys) to make a single combo() descriptors for the non-HD/imported keys, rather than pk(), pkh(), sh(wpkh()), and wpkh() descriptors for the keys.
Implements https://github.com/bitcoin/bitcoin/pull/31374#discussion_r1876650074
ACKs for top commit:
laanwj:
Concept and code review ACK 62b2d23edb
brunoerg:
code review ACK 62b2d23edb
furszy:
Nice catch. ACK 62b2d23edb
theStack:
ACK 62b2d23edb
rkrux:
tACK 62b2d23edb
Tree-SHA512: 86a80b7dcc1598ab18068a2572ff4b4920b233178b760f7b76c5b21a9e6608005ac872f90e082a8f99b51daab0b049e73e4bee5b8e0b537d56ed0d34122a1f49
