- This changes `CBlockPolicyEstimator` to a shared pointer
this gives us three advantages.
- Registering to validation interface using shared pointer
- Scheduling block policy estimator flushes using shared pointer
- Registering block policy estimator to forecaster_man
- Also move them to policy/fees/ and update includes
- Note: the block_policy_estimator_args.h include in block_policy_estimator_args.cpp was done manually.
- Its a module for managing and utilising multiple
fee rate forecasters to provide fee rate forecast.
- The ForecasterManager class allows for the registration of
multiple fee rate forecasters.
Co-authored-by: willcl-ark <will@256k1.dev>
- ForecastType will be used to identify forecasters.
- Each time a new forecaster is added, a corresponding
enum value should be added to ForecastType.
- This allows users to identify which forecasting strategy
was used to make a fee rate estimate.
- This commit implements `Forecaster` abstract class
as the base class of fee rate forecasters.
- Derived classes must provide concrete implementation
of the virtual methods.
Co-authored-by: willcl-ark <will@256k1.dev>
a4df12323c4e9230bca58562ba17ecee4233f8fe doc: add release notes (Sjors Provoost)
c75872ffdd98ce9f04fb8489515d1b63853f03b4 test: use DIFF_1_N_BITS in tool_signet_miner (tdb3)
4131f322ac0abe43639065362cd3c4ea36d2c5c3 test: check difficulty adjustment using alternate mainnet (Sjors Provoost)
c4f68c12e228818f655352d17d038dcc7ba1db3a Use OP_0 for BIP34 padding in signet and tests (Sjors Provoost)
cf0a62878be214cd4ec779aab214221b27b769b6 rpc: add next to getmininginfo (Sjors Provoost)
2d18a078a2d9eaa53b8b7acc7600141c69f0d742 rpc: add target and bits to getchainstates (Sjors Provoost)
f153f57acc9f9a6f84af161d5bed9aa9965abaa3 rpc: add target and bits to getblockchaininfo (Sjors Provoost)
baa504fdfaff4a9f61bc939035df5d5f2978cfd7 rpc: add target to getmininginfo result (Sjors Provoost)
2a7bfebd5e788e1d9e7e07a9f1b8e3625a0301cd Add target to getblock(header) in RPC and REST (Sjors Provoost)
341f93251677fee66c822f414b75499e8b3b31f6 rpc: add GetTarget helper (Sjors Provoost)
d20d96fa41ce706ccc480b4f3143438ce0720348 test: use REGTEST_N_BITS in feature_block (tdb3)
7ddbed4f9fc0c90bfed244a71194740a4a1fa1be rpc: add nBits to getmininginfo (Sjors Provoost)
ba7b9f3d7bf5a1ad395262b080e832f5c9958e4d build: move pow and chain to bitcoin_common (Sjors Provoost)
c4cc9e3e9df2733260942e0513dd8478d2a104da 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 a4df12323c4e9230bca58562ba17ecee4233f8fe
tdb3:
code review re ACK a4df12323c4e9230bca58562ba17ecee4233f8fe
ryanofsky:
Code review ACK a4df12323c4e9230bca58562ba17ecee4233f8fe. 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
fa3c787b62af6abaac35a8f0d785becdb8871cc0 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 fadd568931a2d21e0f80e1efaf2281f5164fa20e and observing an abort when running the `utxo_total_supply` fuzz target.
ACKs for top commit:
marcofleon:
ACK fa3c787b62af6abaac35a8f0d785becdb8871cc0
hodlinator:
re-ACK fa3c787b62af6abaac35a8f0d785becdb8871cc0
ryanofsky:
Code review ACK fa3c787b62af6abaac35a8f0d785becdb8871cc0. 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
223081ece651dc616ff63d9ac447eedc5c2a28fa scripted-diff: rename block and undo functions for consistency (Lőrinc)
baaa3b284671ba28dbbcbb43851ea46175fd2b13 refactor,blocks: remove costly asserts and modernize affected logs (Lőrinc)
fa39f27a0f8b8d14f6769d48f43999a3a1148e4f refactor,blocks: deduplicate block's serialized size calculations (Lőrinc)
dfb2f9d004860c95fc6f0d4a016a9c038d53a475 refactor,blocks: inline `WriteBlockToDisk` (Lőrinc)
42bc4914658d9834a653bd1763aa8f0d54355480 refactor,blocks: inline `UndoWriteToDisk` (Lőrinc)
86b85bb11f8999eb59e34bd026b0791dc866f2eb bench: add SaveBlockBench (Lőrinc)
34f9a0157aad7c10ac364b7e4602c5f74c1f9e20 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 223081ece651dc616ff63d9ac447eedc5c2a28fa. 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 223081ece651dc616ff63d9ac447eedc5c2a28fa
TheCharlatan:
ACK 223081ece651dc616ff63d9ac447eedc5c2a28fa
andrewtoth:
ACK 223081ece651dc616ff63d9ac447eedc5c2a28fa
Tree-SHA512: 951bc8ad3504c510988afd95c561e3e259c6212bd14f6536fe56e8eb5bf5c35c32a368bbdb1d5aea1acc473d7e5bd9cdcde02008a148b05af1f955e413062d5c
e94c9d171239c1fc44fa9c77a4595ecd626b767f [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 e94c9d171239c1fc44fa9c77a4595ecd626b767f
instagibbs:
reACK e94c9d171239c1fc44fa9c77a4595ecd626b767f
darosior:
reACK e94c9d171239c1fc44fa9c77a4595ecd626b767f
brunoerg:
reACK e94c9d171239c1fc44fa9c77a4595ecd626b767f
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.
66d21d0eb6517e04ebfb9fad4085e788de51b4dc qa: check parsed multipath descriptors dont share references (Antoine Poinsot)
09a1875ad8cddeb17c19af34b8282d37fed0937e miniscript: Make NodeRef a unique_ptr (Ava Chow)
9ccb46f91ac231d55a85bf2f61bd2f3276b54b01 miniscript: Ensure there is no NodeRef copy constructor or assignment operator (Ava Chow)
6d11c9c60b5b4a71f8f75cb4eee2f9d0021de310 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 66d21d0eb6517e04ebfb9fad4085e788de51b4dc
hodlinator:
re-ACK 66d21d0eb6517e04ebfb9fad4085e788de51b4dc 🚀
brunoerg:
reACK 66d21d0eb6517e04ebfb9fad4085e788de51b4dc
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>
d44626a9c2eb95b434edc7a22a70f2cce63c9d84 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 d44626a9c2eb95b434edc7a22a70f2cce63c9d84
Tree-SHA512: 77abd2e28ad8dda86eb0548d8e49ecf23bac08a2e07dc35c71db62539aa659d471c863d361534c3cf693f9945c1b4f12de7e04eef05d11f8cc5e86d6eff5242d
fa80a7dac4be8a1d8e32d88e46da23f14e06bade test: Bump sync_mempools timeout in p2p_1p1c_network.py (MarcoFalke)
1111b0ac196d9d26c89c9f932c9784a99cdb218d 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 fa80a7dac4be8a1d8e32d88e46da23f14e06bade
glozow:
ACK fa80a7dac4be8a1d8e32d88e46da23f14e06bade
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.
c0045e6cee06bc0029fb79b5a531aa1f2b817424 Add test for multipath miniscript expression (David Gumberg)
b4ac48090f259dbef567b49fa36a8bf192209710 descriptor: Use InferXOnlyPubkey for miniscript XOnly pubkey from script (Ava Chow)
4c50c21f6bfc1d88846be571055b481ab14b086f tests: Check ExpandPrivate matches for both parsed descriptors (Ava Chow)
092569e8580b7c2c13b6cc9d29bcb4c5e85bbb44 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 c0045e6cee06bc0029fb79b5a531aa1f2b817424
davidgumberg:
utACK c0045e6cee
kevkevinpal:
Concept and Code review ACK [c0045e6](c0045e6cee)
furszy:
ACK c0045e6cee06bc0029fb79b5a531aa1f2b817424
theStack:
re-ACK c0045e6cee06bc0029fb79b5a531aa1f2b817424
rkrux:
Concept ACK c0045e6cee06bc0029fb79b5a531aa1f2b817424
Tree-SHA512: 3dcf2a802b996e0680a3f819075e5a689eb22e484c81ea79b40ec04197ee4ba3f6b9c87c45dfe8a847c9b805b2fd0fad77ffb92a93e65dc3aad74d69d9e3d97f
01df180bfb82c7eafac4638ced249bee4409784b depends: add mold & ld.lld to gen_id (fanquake)
d032ac80633aa6dab7244ec66edd73f4c8ed4ff2 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 01df180bfb82c7eafac4638ced249bee4409784b.
Tree-SHA512: 3809359fe763af9dde484e0c6bd3e262c4c09fcbe2f96ccf64194f5f9f840f5476b9c9929cf7bda7b8c14efeffd369cdb8c233625b79a944e1380df20698246f
faaabfaea768deb7767c489d32fd2097fd180872 ci: Bump centos stream 10 (MarcoFalke)
Pull request description:
This is a follow-up to fa47baa03bcfcf44fb2ed05f009a32d32f860c45, 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 faaabfaea768deb7767c489d32fd2097fd180872
Tree-SHA512: a564ff3a2a0dc4d39874e87540e67072f293bbed82c8eca22266fcadc16c5571e0e41d38576a63e466b64d13f7e3acbd95be10cf2420de33127aa420eca3b928
6e29de21010fc5213176a6ba29f754ca72612ea0 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 6e29de21010fc5213176a6ba29f754ca72612ea0
hebasto:
ACK 6e29de21010fc5213176a6ba29f754ca72612ea0
Tree-SHA512: 81b9fa8ec1f3d21619d37d864047c8d7917ef2c8536851f80facf7f1973dfe14628d7755f12d2a9c6edebb6cb16877c582d4d41cdab52b73b23c44f08c6e6b30
31a0e5f0905bfc6b22ceaaeca53466dfd74967ab 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 31a0e5f0905bfc6b22ceaaeca53466dfd74967ab.
TheCharlatan:
ACK 31a0e5f0905bfc6b22ceaaeca53466dfd74967ab
Tree-SHA512: dd7b3332dd6ecb95189bc72364883425fb8869e03850791d2ee92555a37046c7abaaee16575a0396f1ce9674856b894563dbd36868c2cf46f9fee48028fd967b
