8996fef8ae test: p2p: check that INV messages not matching wtxidrelay are ignored (Sebastian Falbesoner)
e0b3336822 test: p2p: fix sending of manual INVs in tx download test (Sebastian Falbesoner)
Pull request description:
The `test_inv_block` sub-test in p2p_tx_download.py has a subtle bug: the manual msg_inv announcements from peers currently have no effect, since they don't match the wtxidrelay setting (=true by default for `P2PInterface` instances) and are hence ignored by the nodes (since 2d282e0c / PR #18044):
e7c4794955/src/net_processing.cpp (L3904-L3911)
Though the test still passes on master, it does so without the intended scenario of asking an additional peer (triggering the GETDATA_TX_INTERVAL delay). Fix this by sending the INV message with MSG_WTX instead of MSG_TX. This increases the test run time by about one minute intentionally.
It might be good to avoid issues like this in the future, happy to add test framework improvements if someone has a concrete idea.
(Got into the topic of tx/wtx announcements via the discussion https://github.com/bitcoin/bitcoin/pull/31397#discussion_r1904121487)
ACKs for top commit:
maflcko:
ACK 8996fef8ae😸
danielabrozzoni:
ACK 8996fef8ae
mzumsande:
Code Review ACK 8996fef8ae
Tree-SHA512: 3da26f9539c89d64c3b0d0579d9af2a6a4577615eed192506e1fb4318421b235f99a6672a497dea3050fba85dad32678f37fd2cda9ecb70cbf52982db37982e8
fad83e759a doc: Fix incorrect send RPC docs (MarcoFalke)
Pull request description:
It would be good to have accurate RPC docs, so that humans and machines can read them and rely on them.
This fixes one issue.
ACKs for top commit:
fjahr:
utACK fad83e759a
rkrux:
tACK fad83e759a
luke-jr:
tACK fad83e759a
Tree-SHA512: 65d0cc18a62ef44833621464d74b743d24ffe2b853596dce2c4f423df0495142d50387c02ba1b54f5ca77d4ddb083d55116a8ac92698aa6558762d841664911e
5c3e4d8b29 doc: add a section about using MSan (Antoine Poinsot)
Pull request description:
Just a couple lines in a subsection of the sanitizers section mentioning that using the memory sanitizer is a bit more involve than other sanitizers, describing the steps and pointing to an example.
ACKs for top commit:
fanquake:
ACK 5c3e4d8b29
dergoegge:
ACK 5c3e4d8b29
Tree-SHA512: 4ff73c2dd0f25cb96148e54bd867b8d340bd0fbc9b9a736a705125039352eb1d40bd724f9f262a44d3dbd1bea8f03166cf30e571d882fec02ceb1dd399ef7422
4da7bfdcc9 test: add coverage for unknown address type for `createwalletdescriptor` (brunoerg)
Pull request description:
Calling `createwalletdescriptor` RPC with an unknown address type throws an error. This PR adds test coverage for it as done for other RPCs (`getnewaddress `, `getrawchangeaddress`, etc).
ACKs for top commit:
maflcko:
lgtm ACK 4da7bfdcc9
rkrux:
tACK 4da7bfdcc9
Tree-SHA512: 490bc3ffeb70b0f26db0a44d3950d7410fef35d4056487f2e82c081fb14ca277a18943c487235e0163a29f90fc741a262c29835beb9f41936affa4e73ddad25f
fa9aced800 test: Check that reindex with prune wipes blk files (MarcoFalke)
fa9593efc2 test: Use high-level python types (MarcoFalke)
Pull request description:
This adds missing test coverage for `CleanupBlockRevFiles`.
ACKs for top commit:
TheCharlatan:
Re-ACK fa9aced800
l0rinc:
ACK fa9aced800
tdb3:
re ACK fa9aced800
Tree-SHA512: b31ff8a896ce344437715e7fb7efdb8cd7e11470e8465d8972fddfdb58ffd78257786c4060e8596cc53b6278f8ac6a9b6eb05a06e9df58b8b240bdaa719a8e5b
d38ade7bc4 qa: Use `sys.executable` when invoking other Python scripts (Hennadii Stepanov)
Pull request description:
This PR fixes the `rpc_signer.py` and `wallet_signer.py` functional tests on systems where `python3` is not available in the `PATH`, causing the shebang `#!/usr/bin/env python3` to fail.
Here are logs on NetBSD 10.0:
- without this PR:
```
$ python3.12 ./build/test/functional/test_runner.py rpc_signer.py wallet_signer.py
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_160538
Remaining jobs: [rpc_signer.py, wallet_signer.py --descriptors]
1/2 - rpc_signer.py failed, Duration: 1 s
stdout:
2024-12-19T16:05:40.012000Z TestFramework (INFO): PRNG seed is: 1833166631173850775
2024-12-19T16:05:40.012000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1
2024-12-19T16:05:40.754000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 160, in try_rpc
fun(*args, **kwds)
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(/home/hebasto/dev/bitcoin/test/functional/mocks/signer.py enumerate) returned 127: env: python3: No such file or directory
(-1)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_framework.py", line 135, in main
self.run_test()
File "/home/hebasto/dev/bitcoin/build/test/functional/rpc_signer.py", line 72, in run_test
assert_raises_rpc_error(-1, 'fingerprint not found',
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 151, in assert_raises_rpc_error
assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/util.py", line 166, in try_rpc
raise AssertionError(
AssertionError: Expected substring not found in error message:
substring: 'fingerprint not found'
error message: 'RunCommandParseJSON error: process(/home/hebasto/dev/bitcoin/test/functional/mocks/signer.py enumerate) returned 127: env: python3: No such file or directory
'.
2024-12-19T16:05:40.756000Z TestFramework (INFO): Stopping nodes
2024-12-19T16:05:40.873000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1
2024-12-19T16:05:40.873000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1/test_framework.log
2024-12-19T16:05:40.873000Z TestFramework (ERROR):
2024-12-19T16:05:40.873000Z TestFramework (ERROR): Hint: Call /home/hebasto/dev/bitcoin/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20241219_160538/rpc_signer_1' to consolidate all logs
2024-12-19T16:05:40.873000Z TestFramework (ERROR):
2024-12-19T16:05:40.873000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-12-19T16:05:40.873000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-12-19T16:05:40.873000Z TestFramework (ERROR):
stderr:
Remaining jobs: [wallet_signer.py --descriptors]
2/2 - wallet_signer.py --descriptors failed, Duration: 1 s
stdout:
2024-12-19T16:05:40.014000Z TestFramework (INFO): PRNG seed is: 7530764367977090686
2024-12-19T16:05:40.014000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0
2024-12-19T16:05:40.526000Z TestFramework (ERROR): JSONRPC error
Traceback (most recent call last):
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_framework.py", line 135, in main
self.run_test()
File "/home/hebasto/dev/bitcoin/build/test/functional/wallet_signer.py", line 66, in run_test
self.test_valid_signer()
File "/home/hebasto/dev/bitcoin/build/test/functional/wallet_signer.py", line 83, in test_valid_signer
self.nodes[1].createwallet(wallet_name='hww', disable_private_keys=True, descriptors=True, external_signer=True)
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/test_node.py", line 935, in createwallet
return self.__getattr__('createwallet')(wallet_name, disable_private_keys, blank, passphrase, avoid_reuse, descriptors, load_on_startup, external_signer)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/hebasto/dev/bitcoin/test/functional/test_framework/authproxy.py", line 146, in __call__
raise JSONRPCException(response['error'], status)
test_framework.authproxy.JSONRPCException: RunCommandParseJSON error: process(/home/hebasto/dev/bitcoin/test/functional/mocks/signer.py enumerate) returned 127: env: python3: No such file or directory
(-1)
2024-12-19T16:05:40.528000Z TestFramework (INFO): Stopping nodes
2024-12-19T16:05:40.645000Z TestFramework (WARNING): Not cleaning up dir /tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0
2024-12-19T16:05:40.646000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0/test_framework.log
2024-12-19T16:05:40.646000Z TestFramework (ERROR):
2024-12-19T16:05:40.646000Z TestFramework (ERROR): Hint: Call /home/hebasto/dev/bitcoin/test/functional/combine_logs.py '/tmp/test_runner_₿_🏃_20241219_160538/wallet_signer_0' to consolidate all logs
2024-12-19T16:05:40.646000Z TestFramework (ERROR):
2024-12-19T16:05:40.646000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
2024-12-19T16:05:40.646000Z TestFramework (ERROR): https://github.com/bitcoin/bitcoin/issues
2024-12-19T16:05:40.646000Z TestFramework (ERROR):
stderr:
TEST | STATUS | DURATION
rpc_signer.py | ✖ Failed | 1 s
wallet_signer.py --descriptors | ✖ Failed | 1 s
ALL | ✖ Failed | 2 s (accumulated)
Runtime: 1 s
```
- with this PR:
```
$ python3.12 ./build/test/functional/test_runner.py rpc_signer.py wallet_signer.py
Temporary test directory at /tmp/test_runner_₿_🏃_20241219_160011
Remaining jobs: [rpc_signer.py, wallet_signer.py --descriptors]
1/2 - rpc_signer.py passed, Duration: 2 s
Remaining jobs: [wallet_signer.py --descriptors]
2/2 - wallet_signer.py --descriptors passed, Duration: 3 s
TEST | STATUS | DURATION
rpc_signer.py | ✓ Passed | 2 s
wallet_signer.py --descriptors | ✓ Passed | 3 s
ALL | ✓ Passed | 5 s (accumulated)
Runtime: 3 s
```
ACKs for top commit:
maflcko:
lgtm ACK d38ade7bc4
stickies-v:
ACK d38ade7bc4 . I have a minor concern about `sys.executable` not being guaranteed to return a valid Python path, but this patch seems good enough as is so no blocker.
Tree-SHA512: 91fe0abc0b7e2b599c5562f8b225ba60f94c5bd6baa77d8df532155ef4d3ef6c6a862cee7f4a7f565ed4bb3251adcda813b4a4f79be1aa6a4ffdfda8b4e53415
733fa0b0a1 miner: never create a template which exploits the timewarp bug (Antoine Poinsot)
Pull request description:
This check was introduced in #30681 but only enabled for testnet4. To avoid potentially creating an invalid block template if a soft fork to fix the timewarp attack were to activate in the future, we should have this check on all networks. It also seems wise for our miner to not support it whether or not a soft fork activates to fix it at the consensus level.
ACKs for top commit:
Sjors:
ACK 733fa0b0a1
fjahr:
utACK 733fa0b0a1
TheCharlatan:
ACK 733fa0b0a1
Tree-SHA512: 9b3bc8b26a57f93425b17dda80bcfac4ecb750a3d26bc3eb8df619135634e369ac15982fac0c9770b1df207bd2e418ffe02a98f37968f024e55262d97715a4f5
c31166ac77 cmake: Fail if `Libmultiprocess` is missing when `WITH_MULTIPROCESS=ON` (Hennadii Stepanov)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/31708:
```
$ cmake -B build -DWITH_MULTIPROCESS=ON
-- The CXX compiler identification is GNU 13.3.0
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found SQLite3: /usr/include (found suitable version "3.45.1", minimum required is "3.7.17")
CMake Error at CMakeLists.txt:146 (find_package):
By not providing "FindLibmultiprocess.cmake" in CMAKE_MODULE_PATH this
project has asked CMake to find a package configuration file provided by
"Libmultiprocess", but CMake did not find one.
Could not find a package configuration file provided by "Libmultiprocess"
with any of the following names:
LibmultiprocessConfig.cmake
libmultiprocess-config.cmake
Add the installation prefix of "Libmultiprocess" to CMAKE_PREFIX_PATH or
set "Libmultiprocess_DIR" to a directory containing one of the above files.
If "Libmultiprocess" provides a separate development package or SDK, be
sure it has been installed.
-- Configuring incomplete, errors occurred!
```
ACKs for top commit:
vasild:
ACK c31166ac77
ryanofsky:
Code review ACK c31166ac77
TheCharlatan:
ACK c31166ac77
Tree-SHA512: 503e6d7ff253c9ae95b13ff0649af7db97c74a97c04ca6fe88130defae251b94bfe9f4466300d3fab16397c7c8346b392a80a7b80a2d6517464a4eabe3aa40db
a4df12323c doc: add release notes (Sjors Provoost)
c75872ffdd test: use DIFF_1_N_BITS in tool_signet_miner (tdb3)
4131f322ac test: check difficulty adjustment using alternate mainnet (Sjors Provoost)
c4f68c12e2 Use OP_0 for BIP34 padding in signet and tests (Sjors Provoost)
cf0a62878b rpc: add next to getmininginfo (Sjors Provoost)
2d18a078a2 rpc: add target and bits to getchainstates (Sjors Provoost)
f153f57acc rpc: add target and bits to getblockchaininfo (Sjors Provoost)
baa504fdfa rpc: add target to getmininginfo result (Sjors Provoost)
2a7bfebd5e Add target to getblock(header) in RPC and REST (Sjors Provoost)
341f932516 rpc: add GetTarget helper (Sjors Provoost)
d20d96fa41 test: use REGTEST_N_BITS in feature_block (tdb3)
7ddbed4f9f rpc: add nBits to getmininginfo (Sjors Provoost)
ba7b9f3d7b build: move pow and chain to bitcoin_common (Sjors Provoost)
c4cc9e3e9d consensus: add DeriveTarget() to pow.h (Sjors Provoost)
Pull request description:
**tl&dr for consensus-code only reviewers**: the first commit splits `CheckProofOfWorkImpl()` in order to create a `DeriveTarget()` helper. The rest of this PR does not touch consensus code.
There are three ways to represent the proof-of-work in a block:
1. nBits
2. Difficulty
3. Target
The latter notation is useful when you want to compare share work against either the pool target (to get paid) or network difficulty (found an actual block). E.g. for difficulty 1 which corresponds to an nBits value of `0x00ffff`:
```
share hash: f6b973257df982284715b0c7a20640dad709d22b0b1a58f2f88d35886ea5ac45
target: 7fffff0000000000000000000000000000000000000000000000000000000000
```
It's immediately clear that the share is invalid because the hash is above the target.
This type of logging is mostly done by the pool software. It's a nice extra convenience, but not very important. It impacts the following RPC calls:
1. `getmininginfo` displays the `target` for the tip block
2. `getblock` and `getblockheader` display the `target` for a specific block (ditto for their REST equivalents)
The `getdifficulty` method is a bit useless in its current state, because what miners really want to know if the difficulty for the _next_ block. So I added a boolean argument `next` to `getdifficulty`. (These values are typically the same, except for the first block in a retarget period. On testnet3 / testnet4 they change when no block is found after 20 minutes).
Similarly I added a `next` object to `getmininginfo` which shows `bit`, `difficulty` and `target` for the next block.
In order to test the difficulty transition, an alternate mainnet chain with 2016 blocks was generated and used in `mining_mainnet.py`. The chain is deterministic except for its timestamp and nonce values, which are stored in `mainnet_alt.json`.
As described at the top, this PR introduces a helper method `DeriveTarget()` which is split out from `CheckProofOfWorkImpl`. The proposed `checkblock` RPC in #31564 needs this helper method internally to figure out the consensus target.
Finally, this PR moves `pow.cpp` and `chain.cpp` from `bitcoin_node` to `bitcoin_common`, in order to give `rpc/util.cpp` (which lives in `bitcoin_common`) access to `pow.h`.
ACKs for top commit:
ismaelsadeeq:
re-ACK a4df12323c
tdb3:
code review re ACK a4df12323c
ryanofsky:
Code review ACK a4df12323c. Only overall changes since last review were dropping new `gettarget` method and dropping changes to `getdifficulty`, but there were also various internal changes splitting and rearranging commits.
Tree-SHA512: edef5633590379c4be007ac96fd1deda8a5b9562ca6ff19fe377cb552b5166f3890d158554c249ab8345977a06da5df07866c9f42ac43ee83dfe3830c61cd169
fa3c787b62 fuzz: Abort when global PRNG is used before SeedRand::ZEROS (MarcoFalke)
Pull request description:
This adds one more check to abort when global PRNG is used before SeedRand::ZEROS in fuzz tests. This is achieved by carving out the two remaining uses. First, `g_rng_temp_path_init`, and second the random fallback for `RANDOM_CTX_SEED`, which isn't used in fuzz tests anyway.
Requested in https://github.com/bitcoin/bitcoin/pull/31521#issuecomment-2554669015
Can be tested by reverting fadd568931 and observing an abort when running the `utxo_total_supply` fuzz target.
ACKs for top commit:
marcofleon:
ACK fa3c787b62
hodlinator:
re-ACK fa3c787b62
ryanofsky:
Code review ACK fa3c787b62. This adds a new check to make that sure that RNG is never seeded during fuzzing after the RNG has been used. Together with existing checks which ensure RNG can only be seeded with zeroes during fuzzing, and that RNG must was seeded at some point if used after fuzzing, this implies it must have been seeded by zeros before being used.
Tree-SHA512: 2614928d31c310309bd9021b3e5637b35f64196020fbf9409e978628799691d0efd3f4cf606be9a2db0ef60b010f890c2e70c910eaa2934a7fbf64cd1598fe22
223081ece6 scripted-diff: rename block and undo functions for consistency (Lőrinc)
baaa3b2846 refactor,blocks: remove costly asserts and modernize affected logs (Lőrinc)
fa39f27a0f refactor,blocks: deduplicate block's serialized size calculations (Lőrinc)
dfb2f9d004 refactor,blocks: inline `WriteBlockToDisk` (Lőrinc)
42bc491465 refactor,blocks: inline `UndoWriteToDisk` (Lőrinc)
86b85bb11f bench: add SaveBlockBench (Lőrinc)
34f9a0157a refactor,bench: rename bench/readblock.cpp to bench/readwriteblock.cpp (Lőrinc)
Pull request description:
`UndoWriteToDisk` and `WriteBlockToDisk` were delegating a subset of their functionality to single-use methods that didn't optimally capture a meaningful chunk of the algorithm, resulting in calculating things twice (serialized size, header size).
This change inlines the awkward methods (asserting that all previous behavior was retained), and in separate commits makes the usages less confusing.
Besides making the methods slightly more intuitive, the refactorings reduce duplicate calculations as well.
The speed difference is insignificant for now (~0.5% for the new `SaveBlockToDiskBench`), but are a cleanup for follow-ups such as https://github.com/bitcoin/bitcoin/pull/31539
ACKs for top commit:
ryanofsky:
Code review ACK 223081ece6. Since last review, "Save" was renamed to "Write", uint32_t references were dropped, some log statements and comments were improved as suggested, and a lot of tweaks made to commits and commit messages which should make this easier to review.
hodlinator:
ACK 223081ece6
TheCharlatan:
ACK 223081ece6
andrewtoth:
ACK 223081ece6
Tree-SHA512: 951bc8ad3504c510988afd95c561e3e259c6212bd14f6536fe56e8eb5bf5c35c32a368bbdb1d5aea1acc473d7e5bd9cdcde02008a148b05af1f955e413062d5c
e94c9d1712 [doc] Amend notes on benchmarking (dergoegge)
Pull request description:
This gives some more context on the motivation and larger picture of benchmarks.
ACKs for top commit:
l0rinc:
ACK e94c9d1712
instagibbs:
reACK e94c9d1712
darosior:
reACK e94c9d1712
brunoerg:
reACK e94c9d1712
Tree-SHA512: 2cbf51f283f2efc0938e7021ae48db51fe89caf9ef9780821e99fa745dff839e2d202ca956ce6cc48b8319db304069728e77883feefe486264eb1783a0610c93
For blocks 1 through 15 the script_BIP34_coinbase_height appends OP_1
to comply with BIP34 and avoid bad-cb-length.
This is inconsistent with BlockAssembler::CreateNewBlock() which adds
OP_0 instead.
The utxo_total_supply fuzzer and MinerTestingSetup::Block also use OP_0.
Changing it is required to import the test vectors in the next commit.
It also ensures the test vectors can be regenerated using the CPU miner
at https://github.com/pooler/cpuminer without patches (it uses OP_0).
The same helper is used by the signet miner, so this will impact newly
bootstrapped signets.
66d21d0eb6 qa: check parsed multipath descriptors dont share references (Antoine Poinsot)
09a1875ad8 miniscript: Make NodeRef a unique_ptr (Ava Chow)
9ccb46f91a miniscript: Ensure there is no NodeRef copy constructor or assignment operator (Ava Chow)
6d11c9c60b descriptor: Add proper Clone function to miniscript::Node (Ava Chow)
Pull request description:
Multipath descriptors requires performing a deep copy, so a Clone function that does that is added to miniscript::Node instead of the current shallow copy.
Fixes#30864
ACKs for top commit:
darosior:
re-ACK 66d21d0eb6
hodlinator:
re-ACK 66d21d0eb6🚀
brunoerg:
reACK 66d21d0eb6
Tree-SHA512: bea017497ed3cc0b2da2df7e3ccae1fa4a324769b7da1065963da131235bd8bfdcdfe337a3fabbb3ab4d3822611211fca6a9772e18e2ee1cb3d853e831ff6f88
Split CheckProofOfWorkImpl() to introduce a helper function
DeriveTarget() which converts the nBits value to the target.
The function takes pow_limit as an argument so later commits can
avoid having to pass ChainstateManager through the call stack.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Multipath descriptors requires performing a deep copy, so a Clone
function that does that is added to miniscript::Node instead of the
current shallow copy.
Co-Authored-By: Antoine Poinsot <darosior@protonmail.com>
d44626a9c2 depends: Override default build type for `libevent` (Hennadii Stepanov)
Pull request description:
This PR fixes a regression for the `libevent` package introduced in https://github.com/bitcoin/bitcoin/pull/29835.
The `libevent` package defaults to the "Release" build type, which overrides our per-build-type optimization flags with `-O3`.
To prevent this behavior, set `CMAKE_BUILD_TYPE` to "None", consistent with how other packages are handled.
ACKs for top commit:
fanquake:
ACK d44626a9c2
Tree-SHA512: 77abd2e28ad8dda86eb0548d8e49ecf23bac08a2e07dc35c71db62539aa659d471c863d361534c3cf693f9945c1b4f12de7e04eef05d11f8cc5e86d6eff5242d
fa80a7dac4 test: Bump sync_mempools timeout in p2p_1p1c_network.py (MarcoFalke)
1111b0ac19 ci: Add missing --combinedlogslen to test-each-commit task (MarcoFalke)
Pull request description:
This should address the two issues that happened in https://github.com/bitcoin/bitcoin/actions/runs/12885576442/job/35924329657?pr=25832#step:6:7601:
* The combined log isn't printed on a test failure.
* The timeout is too strict for the GHA virtual machines.
For reference, the output was:
```
...
149/315 - rpc_blockchain.py --v2transport passed, Duration: 10 s
150/315 - p2p_addrfetch.py passed, Duration: 1 s
151/315 - p2p_1p1c_network.py failed, Duration: 31 s
stdout:
2025-01-21T12:05:49.465000Z TestFramework (INFO): PRNG seed is: 6581340712385622842
2025-01-21T12:05:49.466000Z TestFramework (INFO): Initializing test directory /tmp/test_runner_₿_🏃_20250121_120233/p2p_1p1c_network_207
2025-01-21T12:05:52.408000Z TestFramework (INFO): Fill mempools with large transactions to raise mempool minimum feerates
2025-01-21T12:05:52.408000Z TestFramework (INFO): Fill the mempool until eviction is triggered and the mempoolminfee rises
2025-01-21T12:05:59.692000Z TestFramework (INFO): Pre-send some transactions to nodes
2025-01-21T12:06:00.203000Z TestFramework (INFO): Submit full packages to node0
2025-01-21T12:06:00.220000Z TestFramework (INFO): Wait for mempools to sync
2025-01-21T12:06:20.384000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 135, in main
self.run_test()
File "/home/runner/work/bitcoin/bitcoin/build/test/functional/p2p_1p1c_network.py", line 153, in run_test
self.sync_mempools(timeout=20)
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 803, in sync_mempools
raise AssertionError("Mempool sync timed out after {}s:{}".format(
AssertionError: Mempool sync timed out after 20s:
...
ACKs for top commit:
l0rinc:
utACK fa80a7dac4
glozow:
ACK fa80a7dac4
Tree-SHA512: b326b7906b184fb47abc50d0d7ec91a6c90d324997f2abc40f156f588090e8d89bd8486bb8950cac604e77b1b336142a47b53ad463b2670d81222814eeb313d4
The `libevent` package defaults to the "Release" build type, which
overrides our per-build-type optimization flags with `-O3`.
To prevent this behavior, set `CMAKE_BUILD_TYPE` to "None", consistent
with how other packages are handled.
c0045e6cee Add test for multipath miniscript expression (David Gumberg)
b4ac48090f descriptor: Use InferXOnlyPubkey for miniscript XOnly pubkey from script (Ava Chow)
4c50c21f6b tests: Check ExpandPrivate matches for both parsed descriptors (Ava Chow)
092569e858 descriptor: Try the other parity in ConstPubkeyProvider::GetPrivKey() (Ava Chow)
Pull request description:
When a `ConstPubkeyProvider` is xonly, the stored pubkey does not necessarily have the correct parity bit. `ToPrivateString()` is correctly handling this by looking up the keys for both parity bits, but `GetPrivKey` does not. This results in not finding the private key when it is actually available if its pubkey has the other parity bit value.
To fix this, this key finding is refactored into `GetPrivKey()` so that its behavior is corrected, and `ToPrivateString()` is changed to use `GetPrivKey()` as well.
Additionally, the descriptor test checks are updated to include a check for `ExpandPrivate()` to verify that both the parsed public and private descriptors produce `SigningProvider`s with the same contents.
Fixes#31589
ACKs for top commit:
Pttn:
ACK c0045e6cee
davidgumberg:
utACK c0045e6cee
kevkevinpal:
Concept and Code review ACK [c0045e6](c0045e6cee)
furszy:
ACK c0045e6cee
theStack:
re-ACK c0045e6cee
rkrux:
Concept ACK c0045e6cee
Tree-SHA512: 3dcf2a802b996e0680a3f819075e5a689eb22e484c81ea79b40ec04197ee4ba3f6b9c87c45dfe8a847c9b805b2fd0fad77ffb92a93e65dc3aad74d69d9e3d97f
01df180bfb depends: add mold & ld.lld to gen_id (fanquake)
d032ac8063 depends: add *FLAGS to gen_id (fanquake)
Pull request description:
The depends cache should be busted when flags change, the same as any other tooling change. I'd also like to start passing `*FLAGS` into depends inside the Guix env, which, without this change, doesn't bust the cache.
ACKs for top commit:
hebasto:
ACK 01df180bfb.
Tree-SHA512: 3809359fe763af9dde484e0c6bd3e262c4c09fcbe2f96ccf64194f5f9f840f5476b9c9929cf7bda7b8c14efeffd369cdb8c233625b79a944e1380df20698246f
faaabfaea7 ci: Bump centos stream 10 (MarcoFalke)
Pull request description:
This is a follow-up to fa47baa03b, which bumped the gcc version to avoid a warning bloat in the CI log. However, it is also required to bump python3, see https://github.com/bitcoin/bitcoin/issues/31476#issue-2735206340
> This will uncover an issue in the centos task that the correct python version is missing. I guess this should be fixed by installing and activating an acceptable python version.
Instead of bumping the packages individually in centos stream 9, just bump to stream 10.
ACKs for top commit:
fanquake:
ACK faaabfaea7
Tree-SHA512: a564ff3a2a0dc4d39874e87540e67072f293bbed82c8eca22266fcadc16c5571e0e41d38576a63e466b64d13f7e3acbd95be10cf2420de33127aa420eca3b928
6e29de2101 ci: Supply `platform` argument to docker commands. (David Gumberg)
Pull request description:
I ran into this issue when following the instructions in `ci/README.md` for running CI locally.
Newer versions of docker require a `--platform` argument when building from a platform-specific image that differs from the host platform, I'm not sure when this change took place, but trying to build any of the cross-platform CI images on Docker 27.5.0 fails in the following manner:
```console
$ # From ci/README.md
$ env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_arm.sh" ./ci/test_run_all.sh'
WARNING: The requested image's platform (linux/arm64/v8) does not match the detected host platform (linux/amd64/v4) and no specific platform was requested
Creating docker.io/arm64v8/debian:bookworm container to run in
+ docker build --file $BITCOIN_SRC/ci/test_imagefile --build-arg CI_IMAGE_NAME_TAG=docker.io/arm64v8/debian:bookworm --build-arg FILE_ENV=./ci/test/00_setup_env_arm.sh --label=bitcoin-ci-test --tag=ci_arm_linux $BITCOIN_SRC
[+] Building 0.6s (2/2) FINISHED docker:default
=> [internal] load build definition from test_imagefile 0.0s
=> => transferring dockerfile: 600B 0.0s
=> WARN: InvalidDefaultArgInFrom: Default value for ARG ${CI_IMAGE_NAME_TAG} results in empty or invalid base image name (line 8) 0.0s
=> ERROR [internal] load metadata for docker.io/arm64v8/debian:bookworm 0.5s
------
> [internal] load metadata for docker.io/arm64v8/debian:bookworm:
------
1 warning found (use docker --debug to expand):
- InvalidDefaultArgInFrom: Default value for ARG ${CI_IMAGE_NAME_TAG} results in empty or invalid base image name (line 8)
test_imagefile:8
--------------------
6 |
7 | ARG CI_IMAGE_NAME_TAG
8 | >>> FROM ${CI_IMAGE_NAME_TAG}
9 |
10 | ARG FILE_ENV
--------------------
ERROR: failed to solve: docker.io/arm64v8/debian:bookworm: failed to resolve source metadata for docker.io/arm64v8/debian:bookworm: no match for platform in manifest: not found
```
This branch fixes this by setting the `--platform` argument of `docker build` and `docker run` with an environment variable `CI_IMAGE_PLATFORM` for each platform specific job, and `linux/{$cpuarch}` for any native jobs.
Thi
## Steps to reproduce
1. Install relevant dependencies, on Ubuntu:
```bash
sudo apt install bash docker.io python3 qemu-user-static
```
2. Run one of the platform-specific CI images, e.g.:
```bash
env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'FILE_ENV="./ci/test/00_setup_env_arm.sh" ./ci/test_run_all.sh'
```
ACKs for top commit:
maflcko:
lgtm ACK 6e29de2101
hebasto:
ACK 6e29de2101
Tree-SHA512: 81b9fa8ec1f3d21619d37d864047c8d7917ef2c8536851f80facf7f1973dfe14628d7755f12d2a9c6edebb6cb16877c582d4d41cdab52b73b23c44f08c6e6b30
31a0e5f090 depends: Qt 5.15.16 (fanquake)
Pull request description:
Contains a handful of miscellaneous bug fixes.
We can drop a few of our patches.
See https://github.com/qt/qtbase/compare/v5.15.14-lts-lgpl...v5.15.16-lts-lgpl.
ACKs for top commit:
hebasto:
ACK 31a0e5f090.
TheCharlatan:
ACK 31a0e5f090
Tree-SHA512: dd7b3332dd6ecb95189bc72364883425fb8869e03850791d2ee92555a37046c7abaaee16575a0396f1ce9674856b894563dbd36868c2cf46f9fee48028fd967b