d3b8a54a81 Refactor CFeeRate to use FeeFrac internally (Pol Espinasa)
Pull request description:
The `FeeFrac` type represents a fraction, intended to be used for `sats/vbyte` or `sats/WU`. It was added to improve accuracy when evaluating fee rates in cluster mempool. [1]
But it can also be used to fix the precision issues that the current `CFeeRate` class has now.
At the moment, `CFeeRate` handles the fee rate as satoshis per kilovirtualbyte: `CAmount / kvB` using an integer.
This PR fix `CFeeRate` precision issues by encapsulating `FeeFrac` internally keeping backwards compatibility.
This PR can also be used as a based to use multiple units on RPC calls as detailed in this issue [2].
Some previous discussions:
[1] https://github.com/bitcoin/bitcoin/pull/30535
[2] https://github.com/bitcoin/bitcoin/issues/32093
ACKs for top commit:
achow101:
ACK d3b8a54a81
murchandamus:
code review, lightly tested ACK d3b8a54a81
ismaelsadeeq:
re-ACK d3b8a54a81📦
theStack:
Code-review ACK d3b8a54a81
Tree-SHA512: 5a8149d81e82ad4e60a0e76ff6a82a5b1c4e212cf5156c1cdd16bf9acbb351e7be458eac3f0a2ae89107f331062b299c1d9ca649d3b820ad0b68e6d1a14292e5
The `-assumevalid` option skips script verification for a specified block and all its ancestors during Initial Block Download.
Many new users are surprised when this suddenly slows their node to a halt.
This commit adds a log message to clearly indicate when this optimization ends and full validation begins (and vice versa).
When using `-assumeutxo`, logging is suppressed for the active assumed-valid chainstate and for the background validation chainstate to avoid the confusing toggles.
-------
> cmake -B build && cmake --build build && mkdir -p demo && build/bin/bitcoind -datadir=demo -stopatheight=500 | grep 'signature validation'
```
2025-08-08T20:59:21Z Disabling signature validations at block #1 (00000000839a8e6886ab5951d76f411475428afc90947ee320161bbf18eb6048).
2025-08-08T20:59:21Z Enabling signature validations at block #100 (000000007bc154e0fa7ea32218a72fe2c1bb9f86cf8c9ebf9a715ed27fdb229a).
2025-08-08T20:59:21Z Disabling signature validations at block #200 (000000008f1a7008320c16b8402b7f11e82951f44ca2663caf6860ab2eeef320).
2025-08-08T20:59:21Z Enabling signature validations at block #300 (0000000062b69e4a2c3312a5782d7798b0711e9ebac065cd5d19f946439f8609).
```
In LogPrintStr_:
- remove an unnecessary BCLog since we are in the BCLog namespace.
- remove an unnecessary \n when rate limiting is triggered since
FormatLogStrInPlace will add it.
- move the ratelimit bool into an else if block.
- prefix all log lines with [*] when suppressions exist. Previously this
was only done if should_ratelimit was true.
In Reset:
- remove an unnecessary \n since FormatLogStrInPlace will add it.
- Change Level::Info to Level::Warning.
Previously, we would check failing input scripts twice when considering
a transaction for the mempool, in order to distinguish policy failures
from consensus failures. This allowed us both to provide a different
error message and to discourage peers for consensus failures. Because we
are no longer discouraging peers for consensus failures during tx relay,
and because checking a script can be expensive, only do this once.
Also renames non-mandatory-script-verify-flag error to
mempool-script-verify-flag-failed.
Do not discourage nodes even when they send us consensus invalid
transactions.
Because we do not discourage nodes for transactions we consider
non-standard, we don't get any DoS protection from this check in
adversarial scenarios, so remove the check entirely both to simplify the
code and reduce the risk of splitting the network due to changes in tx
relay policy.
exFAT is known to cause corruption on macOS. See #28552.
Therefore we should warn when using this fs format for either the blocks
or data directories on macOS.
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
27aefac425 validation: detect witness stripping without re-running Script checks (Antoine Poinsot)
2907b58834 policy: introduce a helper to detect whether a transaction spends Segwit outputs (Antoine Poinsot)
eb073209db qa: test witness stripping in p2p_segwit (Antoine Poinsot)
Pull request description:
Since it was introduced in 4eb515574e (#18044), the detection of a stripped witness relies on running the Script checks 3 times. In the worst case, this consists in running Script validation for every single input 3 times.
Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with txid-based orphan resolution as used in 1p1c package relay.
However it is not necessary to run Script validation to detect a stripped witness (much less so doing it 3 times in a row). There are 3 types of witness program: defined program types (Taproot, P2WPKH and P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus). For undefined program types, Script validation is always going to fail regardless of the witness (by standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation. However this might lead to more "false positives" (cases where we return witness stripping for an otherwise invalid transaction) than the existing implementation. For instance a transaction with one P2PKH input with an invalid signature and one P2WPKH input with its witness stripped. The existing implementation would treat it as consensus invalid while the implementation in this PR would always consider it witness stripped.
h/t AJ: this essentially implements a variant of https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3135258539.
ACKs for top commit:
sipa:
re-ACK 27aefac425
Crypt-iQ:
re-ACK 27aefac425
glozow:
reACK 27aefac425
Tree-SHA512: 70cf76b655b52bc8fa2759133315a3f11140844b6b80d9de3c95f592050978cc01a87bd2446e3a9c25cc872efea7659d6da3337b1a709511771fece206e9f149
656e16aa5e qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file (Hennadii Stepanov)
a0eaa44925 Fix typos (Hennadii Stepanov)
8d4aaaec49 Update Transifex slug for 30.x (Hennadii Stepanov)
Pull request description:
This PR follows our [Release Process](53a996f122/doc/release-process.md).
It is required to open Transifex translations for v30.0, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
For reference, see the previous similar PR: https://github.com/bitcoin/bitcoin/pull/31809.
**Note for reviewers:**
To reproduce the diff in the last commit, run:
```
cmake --preset dev-mode
cmake --build build_dev_mode --target translate
```
ACKs for top commit:
laanwj:
Code review ACK 656e16aa5e
stickies-v:
ACK 656e16aa5e , was able to reproduce
Tree-SHA512: 403b534329755079584fcdf98b696e3e75952dfc8d069f305843dbfa85de95f6816ee1d5dfc9b553c7c7f52cc296cb8d3cb03207051d26e0e76ff30d377f49e4
Since it was introduced in 4eb515574e (#18044), the detection of a
stripped witness relies on running the Script checks 3 times. In the worst case, this consists in
running Script validation 3 times for every single input.
Detection of a stripped witness is necessary because in this case wtxid==txid, and the transaction's
wtxid must not be added to the reject filter or it could allow a malicious peer to interfere with
txid-based orphan resolution as used in 1p1c package relay.
However it is not necessary to run Script validation to detect a stripped witness (much less so
doing it 3 times in a row). There are 3 types of witness program: defined program types (Taproot,
P2WPKH, P2WSH), undefined types, and the Pay-to-anchor carve-out.
For defined program types, Script validation with an empty witness will always fail (by consensus).
For undefined program types, Script validation is always going to fail regardless of the witness (by
standardness). For P2A, an empty witness is never going to lead to a failure.
Therefore it holds that we can always detect a stripped witness without re-running Script validation.
However this might lead to more "false positives" (cases where we return witness stripping for an
otherwise invalid transaction) than the existing implementation. For instance a transaction with one
P2PKH input with an invalid signature and one P2WPKH input with its witness stripped. The existing
implementation would treat it as consensus invalid while the implementation in this commit would
always consider it witness stripped.
f49840dd90 doc: Fix typo in files.md (Ryan Ofsky)
f5cf0b1ccc bitcoin wrapper: improve help output (Ryan Ofsky)
c810b168b8 doc: Add description of installed files to files.md (Ryan Ofsky)
94ffd01a02 doc: Add release notes describing libexec/ binaries (Ryan Ofsky)
cd97905ebc cmake: Move internal binaries from bin/ to libexec/ (Ryan Ofsky)
Pull request description:
This change moves binaries that are not typically invoked directly by users from the `bin/` directory to the `libexec/` directory in CMake installs and binary releases. The goal of the PR is to introduce a distinction between internal and external binaries so starting with #31802, we can use IPC to implement features in new binaries without adding those binaries to the CLI. The change also helps reduce clutter in `bin/`, making it easier for users to identify useful tools to run. Summary of changes:
- For **source builds** (i.e. developer builds) — There are no changes.
- For **source installs** (i.e. `cmake --install` result) — `test_bitcoin`, `test_bitcoin-qt`, and `bench_bitcoin` are installed in `${CMAKE_PREFIX_PATH}/libexec` instead of `${CMAKE_PREFIX_PATH}/bin`, so they are no longer on the system `PATH`. However, they can still be invoked from the `libexec/` directory, or from the CLI as `bitcoin test`, `bitcoin test-gui`, and `bitcoin bench`, respectively.
- For **binary releases** — Since `test_bitcoin` is the only test binary enabled in releases, the only change is moving `test_bitcoin` from `bin/` to `libexec/`.
<details><summary>Details</summary>
<p>
The table below shows the install location of each binary after this change, and the availability of each binary.
| Binary | Location | Availability | Change |
|----------------------|--------------|----------------------|-------------------------------|
| `bitcoin` | `bin/` | 📦 Binary release (since #31375) | Unchanged |
| `bitcoin-cli` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoind` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-qt` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-tx` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-util` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-wallet` | `bin/` | 📦 Binary release | Unchanged |
| `bench_bitcoin` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
| `bitcoin-chainstate` | `libexec/` | 🛠 Source build only | Newly installed (was built) |
| `bitcoin-gui` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `bitcoin-node` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `test_bitcoin` | `libexec/` | 📦 Binary release | Moved from `bin/` |
| `test_bitcoin-qt` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
</p>
</details>
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
l0rinc:
re-ACK f49840dd90
Sjors:
re-ACK f49840dd90
achow101:
ACK f49840dd90
janb84:
re ACK f49840dd90
BrandonOdiwuor:
Tested ACK f49840dd90
hodlinator:
re-ACK f49840dd90
willcl-ark:
utACK f49840dd90
Tree-SHA512: 858a2e1a53db11ee3c5c759bfdeea566f242b9ce5e8a898fa435222e41662b8184577c0dc2c4c058294b4de41d8cb3ba3e5d24c748c280efa4a3f84e3ec4344d
9a5d29711a Squashed 'src/crc32c/' changes from b60d2b7334..efb8ea04e4 (fanquake)
Pull request description:
Sync the subtree with latest upstream. The changes here are a no-op, but pull them to fix the drive-by-typo-fixing: #33057.
Includes https://github.com/bitcoin-core/crc32c-subtree/pull/8.
ACKs for top commit:
maflcko:
lgtm ACK 8ef8dd6871
janb84:
ACK 8ef8dd6871
Tree-SHA512: b20a47514218206b934c4aa27ec667fb9b3ec7f7388a78725c52fc6e916358d2b9a2075a37808dbc2430b4c7816511ecf20e36bfe2fbd2d8a26bc8882a46d5e7
86e3a0a8cb refactor: standardize obfuscation memory alignment (Lőrinc)
13f00345c0 refactor: write `Obfuscation` object when new key is generated in dbwrapper (Lőrinc)
e5b1b7c557 refactor: rename `OBFUSCATION_KEY_KEY` (Lőrinc)
298bf95105 refactor: simplify `Obfuscation::HexKey` (Lőrinc)
2dea045425 test: make `obfuscation_serialize` more thorough (Lőrinc)
a17d8202c3 test: merge xor_roundtrip_random_chunks and xor_bytes_reference (Lőrinc)
Pull request description:
Follow up for https://github.com/bitcoin/bitcoin/pull/31144
Applied the remaining comments in separate commits - except for the last one where I could group them.
Please see the commit messages for more context.
ACKs for top commit:
achow101:
ACK 86e3a0a8cb
ryanofsky:
Code review ACK 86e3a0a8cb, just tweaking key write assert as suggested
hodlinator:
ACK 86e3a0a8cb
Tree-SHA512: 967510a141fbb57bf9d088d92b554cf2fffc2f6aa0eab756cbae3230f53e9b04ceebcc6fea5f3383c01ad41985ecde5b5686c64a771ca9deae3497b9b88c1c8b
18d1071dd1 init: replace deprecated PermissionsStartOnly systemd directive (Florian Schmaus)
1caaf65043 init: remove Group= as it will default to the user's default group (Florian Schmaus)
Pull request description:
> This removes the redundant 'Group=' directive and replaces the deprecated 'PermissionsStartOnly' directive.
Picks up #16994 / #19513. The concern in both of these PRs was changing this too early, while systemd v240 was still prelevant on supported systems. That was ~5 years ago, and from what I can see, no modern/supported OS is still using an older systemd.
Separately , I am wondering if we should move these files out to https://github.com/bitcoin-core/packaging/.
ACKs for top commit:
willcl-ark:
reACK 18d1071dd1
Tree-SHA512: a994e38099e68e8377ac820d3cd2047cbfca065ba617eff0d621e3c3b99b05bbd2329631aa8c885a83cf5d0066d97ff5be75bf5834e9f759d8f0d2c6c9b64851
fdbade6f8d kernel: create monolithic kernel static library (Cory Fields)
Pull request description:
Currently, consuming `libbitcoinkernel.a` requires all its dependency static libraries to be available. A switch to a monolithic variant, which contains object files from its dependencies, was discussed in the Kernel WG. The necessary preparations in the libsecp256k1 build scripts were completed in https://github.com/bitcoin-core/secp256k1/pull/1678, which are now available in this repository since https://github.com/bitcoin/bitcoin/pull/33036.
The changes in this PR were picked from https://github.com/theuni/bitcoin/commits/static_kernel/, with an additional adjustment in `libbitcoinkernel.pc.in`.
This PR can be tested as described in https://github.com/bitcoin/bitcoin/pull/30814#issue-2505698234.
ACKs for top commit:
TheCharlatan:
ACK fdbade6f8d
stickies-v:
tACK fdbade6f8d
Tree-SHA512: bd9e9dbb0b765bdcb162fb3f4ad3c4e01fe5fa0b7061f97d0bad64442b21db036cbe0e4341fd45d43a8862df76d62c9532ca8945f76423aca753c6b528f70873
b093a19ae2 cmake: Proactively avoid use of `SECP256K1_DISABLE_SHARED` (Hennadii Stepanov)
eb59a192d9 cmake, refactor: Encapsulate adding secp256k1 subtree in function (Hennadii Stepanov)
Pull request description:
The `SECP256K1_DISABLE_SHARED` CMake variable has been [removed](https://github.com/bitcoin-core/secp256k1/pull/1688) upstream.
This PR removes its usage ahead of the next `secp256k1` subtree update to prevent breakage and simplify integration.
ACKs for top commit:
fanquake:
ACK b093a19ae2
Tree-SHA512: a87cee71cf356f458f68d3163253ca5c4f86e56d268006b6b8e1d4b2c009ba436148a07a6b67b89ddbb2d0e3c1113ab4b4906c5fc5624cb3082b20e916e0e82b
3543bfdfec test: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify 'spend_vin' is the correct field (Chris Stewart)
Pull request description:
Fixes bug in `getdescriptoractivity` RPC help manual.
Here is the line that pushes `spend_vin` field, there is no `spend_vout` json field.
https://github.com/bitcoin/bitcoin/blob/master/src/rpc/blockchain.cpp#L2757
ACKs for top commit:
nervana21:
tACK 3543bfd
luke-jr:
utACK 3543bfdfec
jonatack:
ACK 3543bfdfec
Tree-SHA512: 2cd543569a87261d8d804d9afe36f8e8ead55839c01da9c4831aea3ced7d1251e6885621e628898105700aae4d76cbb8a682f518f33c1c52163e66f75ec87a61
a26fbee38f qt: Translations update (Hennadii Stepanov)
ca04eebd72 cmake: Switch to generated `ts_files.cmake` file (Hennadii Stepanov)
95341de6ca cmake, refactor: Move handling of Qt TS files into `locale` directory (Hennadii Stepanov)
Pull request description:
This PR:
1. Moves handling of Qt TS files into the `locale` directory.
2. Switches from inferior globbing to the explicit file list generated by the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) script.
Closes#32653.
ACKs for top commit:
fanquake:
ACK a26fbee38f
Tree-SHA512: 10596768c120d9da21a2340b693c5f39e9e1e02976805e14284cf9785780756f953d73d3d2b4a7246ada37acafe3e5d0e8927a8f51bf9fce3bc93a3544231489
fa1d2f6380 ci: Pass CI_FAILFAST_TEST_LEAVE_DANGLING into container (MarcoFalke)
Pull request description:
After commit fd813bf863, the env var `CI_FAILFAST_TEST_LEAVE_DANGLING` is no longer passed into the container.
This is harmless, because it isn't needed for the Linux containers and macos doesn't use containers at all.
However, it would be nice to document it as an allowed setting and consistently pass it on, when set. So do that here.
ACKs for top commit:
fanquake:
ACK fa1d2f6380
Tree-SHA512: b61780a27f4c2e11359827b1360a34a132e15bff94f358cbf4d453805afcafb873e2c395908b17610fae026ead93226c19a8f6be40fbb87b623ee992f3e2b43e
3333d3f75f ci: Only pass documented env vars (MarcoFalke)
Pull request description:
The CI currently inherits almost all env vars from the host. This was problematic in the past and causing non-determinism, e.g. the fix in commit fa12558d21. It is still problematic today, see e.g. https://github.com/bitcoin/bitcoin/pull/31349#issuecomment-2586828644, or https://github.com/bitcoin/bitcoin/issues/32935
This fixes https://github.com/bitcoin/bitcoin/issues/32935 by only passing env vars documented in `./ci/test/00_setup_env.sh`.
Implementation-wise, instead of cramming the python code into the `python -c ""` statement, just start a fresh py file, which is easier to handle.
ACKs for top commit:
willcl-ark:
ACK 3333d3f75f
Tree-SHA512: f922e481a844128d7fbf773563278a3992c178ead60a3050eceb9ded2aad979afc815a5cbdb9f68494493c5d8d942cdd1111c21e32a5746d19505b87745cb84a
ad132761fc [allocators] Apply manual ASan poisoning to PoolResource (dergoegge)
Pull request description:
Currently ASan will not detect use-after-free issues for memory allocated by a `PoolResource`. This is because ASan is only aware of the memory chunks allocated by `PoolResource` but not the individual "sub-chunks" within.
E.g. this test will not produce an ASan error even though the referenced coin has been deallocated:
```c++
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index c46144b34b..aa6ca15ce1 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -508,6 +508,17 @@ BOOST_FIXTURE_TEST_CASE(updatecoins_simulation_test, UpdateTest)
BOOST_CHECK(spent_a_duplicate_coinbase);
}
+BOOST_AUTO_TEST_CASE(asan_uaf)
+{
+ CCoinsMapMemoryResource cache_coins_memory_resource{};
+ CCoinsMap map(0, SaltedOutpointHasher(/*deterministic=*/true), CCoinsMap::key_equal{}, &cache_coins_memory_resource);
+ COutPoint outpoint{};
+ map.emplace(outpoint, Coin{});
+ auto& coin = map.at(outpoint);
+ map.erase(outpoint);
+ coin.coin.nHeight = 1;
+}
+
BOOST_AUTO_TEST_CASE(ccoins_serialization)
{
// Good example
```
Fix this by applying [manual ASan poisoning](https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning) for memory allocated by `PoolResource`:
* Newly allocated chunks are poisoned as a whole
* "Sub-chunks" are unpoisoned/re-poisoned during allocation/deallocation
With the poisoning applied, ASan catches the issue in the test above:
```
$ ./build_unit/bin/test_bitcoin --run_test="coins_tests/asan_uaf"
Running 1 test case...
=================================================================
==366064==ERROR: AddressSanitizer: use-after-poison on address 0x7f99c3204870 at pc 0x55569dab6f8a bp 0x7ffe0210e4d0 sp 0x7ffe0210e4c8
READ of size 4 at 0x7f99c3204870 thread T0 (b-test)
```
ACKs for top commit:
achow101:
ACK ad132761fc
marcofleon:
code review ACK ad132761fc
Tree-SHA512: eb5e80bfa9509225e784151807bd8aa21fb0826ca1781dfe81b1d60bd3766019384ea3f9cb8e53398fde2f4e994a9c201b5a9962b4d279d7e52bb60e8961be11
1252eeb997 rpc: fix getpeerinfo ping duration unit docs (0xb10c)
Pull request description:
The docs have been incorrect since a3789c700b (released in v25; master since Sept. 2022). Noticed while setting up monitoring using getpeerinfo.
0cb1ed2b7c/src/rpc/net.cpp (L249-L257)
ACKs for top commit:
luke-jr:
utACK 1252eeb997
maflcko:
lgtm ACK 1252eeb997
jonatack:
ACK 1252eeb997
theStack:
ACK 1252eeb997
janb84:
ACK 1252eeb997
Tree-SHA512: 33f576336b2a4d9533f51f4641d564ee59ef692c5fa9a3cad239fc31465883d5da534bfd0e069be1e1d688e5f0dea3fe6850be19bf35335041b8f414d08f7f09
When the detailed peers list is requested, return the shortened services in the
-netinfo header in the same format as the "serv" column, instead of the full names
list in the report.
