Before bip94 there was an assumption that the minimum permitted
timestamp is GetMedianTimePast() + 1.
This commit splits a helper function out of UpdateTime() to
obtain the minimum time in a way that takes the
timewarp attack rule into account.
fa8ade300f refactor: Avoid GCC false positive error (MarcoFalke)
fa40807fa8 ci: Enable DEBUG=1 for one GCC-12+ build to catch 117966 regressions (MarcoFalke)
Pull request description:
It is possible that someone accidentally removes the workaround in fa9e0489f5, or more likely that someone accidentally adds new code without the workaround.
Avoid this by adding a temporary CI check.
This can be tested by reverting the workaround and observing a failure.
ACKs for top commit:
hebasto:
ACK fa8ade300f, I've tested locally on Ubuntu 24.04.
Tree-SHA512: 7ee1538fd5304a5ab91ac8c7619a573548d7e0345592a1e9d38b3b73729e09e7c77a9ee703d64cf02a8218de3148376d7836e294abb939aa7533034ba36dfb6c
faf2f2c654 test: Avoid redundant stop and error spam on shutdown (MarcoFalke)
fae3bf6b87 test: Avoid redundant stop and error spam on startup failure (MarcoFalke)
fa0dc09b90 test: Remove --noshutdown flag (MarcoFalke)
fad441fba0 test: Treat leftover process as error (MarcoFalke)
Pull request description:
The `--noshutdown` flag is brittle, confusing, and redundant:
* Someone wanting to inspect the state after a test failure will likely also want to debug the state on the python side, so the option is redundant with `--pdbonfailure`. If there was a use case to replicate `--pdbonfailure` without starting pdb, a dedicated flag could be added for that use case.
* It is brittle to use the flag for a passing test, because it will disable checks in the test. For example, on shutdown LSan will perform a leak check, and the test framework will check that the node did not crash, and it will check that the node did not print errors to stderr.
Fix all issues by removing it.
Also, tidy up startup error messages to be less confusing as a result.
ACKs for top commit:
hodlinator:
re-ACK faf2f2c654
pablomartin4btc:
re tACK faf2f2c654
Tree-SHA512: 46d7ae59c7be88b93f1f9e0b6be21af0fc101e646512e2c5e725682cb18bfec8aa010e0ebe89ce9ffe239e5caac0da5f81cc97b79e738d26ca5fa31930e8e4e3
f5883286e3 Add a fuzz test for Num3072 multiplication and inversion (Pieter Wuille)
a26ce62894 Safegcd based modular inverse for Num3072 (Pieter Wuille)
91ce8cef2d Add benchmark for MuHash finalization (Pieter Wuille)
Pull request description:
This implements a safegcd-based modular inverse for MuHash3072. It is a fairly straightforward translation of [the libsecp256k1 implementation](https://github.com/bitcoin-core/secp256k1/pull/831), with the following changes:
* Generic for 32-bit and 64-bit
* Specialized for the specific MuHash3072 modulus (2^3072 - 1103717).
* A bit more C++ish
* Far fewer sanity checks
A benchmark is also included for MuHash3072::Finalize. The new implementation is around 100x faster on x86_64 for me (from 5.8 ms to 57 μs); for 32-bit code the factor is likely even larger.
For more information:
* [Original paper](https://gcd.cr.yp.to/papers.html) by Daniel J. Bernstein and Bo-Yin Yang
* [Implementation](https://github.com/bitcoin-core/secp256k1/pull/767) for libsecp256k1 by Peter Dettman; and the [final](https://github.com/bitcoin-core/secp256k1/pull/831) version
* [Explanation](https://github.com/bitcoin-core/secp256k1/blob/master/doc/safegcd_implementation.md) of the algorithm using Python snippets
* [Analysis](https://github.com/sipa/safegcd-bounds) of the maximum number of iterations the algorithm needs
* [Formal proof in Coq](https://medium.com/blockstream/a-formal-proof-of-safegcd-bounds-695e1735a348) by Russell O'Connor (for the 256-bit version of the algorithm; here we use a 3072-bit one).
ACKs for top commit:
achow101:
ACK f5883286e3
TheCharlatan:
Re-ACK f5883286e3
dergoegge:
tACK f5883286e3
Tree-SHA512: 275872c61d30817a82901dee93fc7153afca55c32b72a95b8768f3fd464da1b09b36f952f30e70225e766b580751cfb9b874b2feaeb73ffaa6943c8062aee19a
1b51616f2e test: improve rogue calls in mining functions (i-am-yuvi)
Pull request description:
#31403 follow-up, see [comment](https://github.com/bitcoin/bitcoin/pull/31403#pullrequestreview-2498806354)
- Rename `invalid_call` parameter to `called_by_framework` in `generateblock`, `generatetoaddress` and `generatetodescriptor` mining methods to better express its intended usage.
- Add explicit assertion message clarifying that these functions should only be called by TestFramework itself to maintain proper node synchronization.
ACKs for top commit:
maflcko:
lgtm ACK 1b51616f2e
achow101:
ACK 1b51616f2e
hodlinator:
re-ACK 1b51616f2e
Prabhat1308:
ACK [1b51616](1b51616f2e)
Tree-SHA512: 56832626fe54dcaa07dacb4f9c960c0a83fad3fb12272155114ac697856c59b7f44805e1152eddeec7a5e8f7daf487382dc01b5b9ae2e74b62b2df6bd1f81f77
92787dd52c test: raise an error when target_vsize is below tx virtual size (ismaelsadeeq)
a8780c937f test: raise an error if output value is <= 0 in `create_self_transfer` (ismaelsadeeq)
f6e88931f0 test: test that `create_self_transfer_multi` respects `target_vsize` (ismaelsadeeq)
Pull request description:
This is a simple test PR that does two things:
1. Raise an exception in `_bulk_tx_` when `target_vsize` is too low, i.e., below the tx vsize.
2. Addresses some review comments from https://github.com/bitcoin/bitcoin/pull/30162, which are:
- Raise an error if the output value is less than or equal to zero in `create_self_transfer`.
This prevents creating transactions with a value of 0 or less.
- Add a test to verify that `create_self_transfer_multi` also respects the passed `target_vsize`.
ACKs for top commit:
achow101:
ACK 92787dd52c
theStack:
ACK 92787dd52c
rkrux:
reACK 92787dd52c
glozow:
ACK 92787dd52c
Tree-SHA512: 1f2767f2cf715ed65074c5fff347eec160b142685777d833d5e872cfef364d3dc1916b52ee442e99c7b9a8d514ff62bc67a9899d8854f65a4b93ac3ae300d18e
18619b4732 wallet: remove BDB dependency from wallet migration benchmark (furszy)
Pull request description:
Part of the legacy wallet removal working path #20160.
Stops creating a bdb database in the wallet migration benchmark.
Instead, the benchmark now creates the db in memory and re-uses it for the migration process.
ACKs for top commit:
achow101:
ACK 18619b4732
brunoerg:
code review ACK 18619b4732
theStack:
Code-review ACK 18619b4732
Tree-SHA512: a107deee3d2c00b980e3606be07d038ca524b98251442956d702a7996e2ac5e2901f656482018cacbac8ef6a628ac1fb03f677d1658aeaded4036d834a95d7e0
2656a5658c tests: add a test for the new blocksdir lock (Cory Fields)
bdc0a68e67 init: lock blocksdir in addition to datadir (Cory Fields)
cabb2e5c24 refactor: introduce a more general LockDirectories for init (Cory Fields)
1db331ba76 init: allow a new xor key to be written if the blocksdir is newly created (Cory Fields)
Pull request description:
This probably should've been included in #12653 when `-blocksdir` was introduced. Credit TheCharlatan for noticing that it's missing.
This guards against 2 processes running with separate datadirs but the same blocksdir. I didn't add `walletdir` as I assume sqlite has us covered there.
It's not likely to happen currently, but may be more relevant in the future with applications using the kernel. Note that the kernel does not currently do any dir locking, but it should.
ACKs for top commit:
maflcko:
review ACK 2656a5658c 🏼
kevkevinpal:
ACK [2656a56](2656a5658c)
achow101:
ACK 2656a5658c
tdb3:
Code review and light test ACK 2656a5658c
Tree-SHA512: 3ba17dc670126adda104148e14d1322ea4f67d671c84aaa9c08c760ef778ca1936832c0dc843cd6367e09939f64c6f0a682b0fa23a5967e821b899dff1fff961
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
Trying to shut down a node after a test failure may fail and lead to an
RPC error.
Also, it is confusing to sidestep the existing fallback to kill any
leftover nodes on a test failure.
So just rely on the fallback.
Idea by Hodlinator.
Co-Authored-By: Hodlinator <172445034+hodlinator@users.noreply.github.com>
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
