Add a verbosity option to the existing estimatesmartfee options object.
The default verbosity remains 1.
When verbosity is at least 2 include mempool_health_statistics in the response.
The array reports the mined blocks tracked by the mempool fee rate estimator in
most-recent-first order, with each entry containing:
- block_height
- block_weight: total non-coinbase transaction weight in the block
- mempool_txs_weight: weight of transactions removed from our mempool
for that block
Expose these stats through the fee rate estimator manager so RPC users
can inspect the block coverage data used by the mempool health check.
Gate the mempool fee rate estimator on a coverage check: recent
connected blocks must be well represented by transactions removed from
our mempool.
Track per-block weight for the last MEMPOOL_HEALTH_WINDOW_BLOCKS blocks.
AddMinedBlockStats drops stats at or above a connected block's height
before appending it, and resets the window on a forward height gap so
tracked heights stay consecutive.
Only apply the coverage ratio once the window holds at least one block
of transactions; below that activity is too low for the ratio to be
meaningful, so treat the mempool as healthy.
Replace a boolean health check with a MempoolHealth enum so
EstimateFeeRate() can report whether estimation is unavailable because
too few recent blocks have been tracked (INSUFFICIENT_DATA) or because
recent blocks poorly represent the mempool (LOW_COVERAGE).
Return the removed mempool transaction info from
CTxMemPool::removeForBlock instead of dispatching the
MempoolTransactionsRemovedForBlock notification from the mempool.
Emit it from ConnectTip after mempool removal and before BlockConnected,
passing the connected block, the removed mempool transactions, and the
block height to the callback.
Because the signal now originates from ConnectTip, where the IBD state is
known, gate it on !IsInitialBlockDownload(): the notification is no longer
fired for blocks connected during initial block download or reindex, while
the mempool removal in removeForBlock still runs unconditionally. This keeps
fee rate estimators from recording blocks connected before the node is
synced.
Integrate MemPoolFeeRateEstimator into FeeRateEstimatorManager.
When both estimators succeed, select the lower of the block policy
and mempool estimates.
When either estimator fails, return its error instead of falling back
to the block policy estimate: if the mempool estimator cannot produce
an estimate, the combined estimate fails.
Callers that want a block-policy-only estimate can request it explicitly
via fee_rate_estimator option.
estimatesmartfee now emits the estimator field only for successful
manager-selected estimates.
Add a test that ensures estimatesmartfee returns the mempool fee rate
estimate when it is lower than the block policy estimate, and can request
the mempool policy estimator explicitly
Two wallet functional tests also need adjusting. When the mempool is
too sparse to fill its percentile buckets, MemPoolFeeRateEstimator
returns a relayable floor of max(min relay fee, mempool min fee), so in
regtest getFeeRateEstimate now returns the min relay fee where the
wallet previously had no estimate and fell back to a higher rate:
- wallet_taproot.py: the cleanup sendall used automatic fee estimation.
GetMinimumFeeRate previously fell back to the wallet fallback fee
(fallbackfee, 20 sat/vB in the test framework); it now uses the min
relay fee floor. At that lower feerate the wallet's underestimate of
the taproot script-path witness size drops the effective feerate
below min relay, so the transaction is rejected. Pin fee_rate=20 to
match the framework fallbackfee.
- wallet_bumpfee.py: GetDiscardRate() previously fell back to the
wallet discard rate (-discardfee); it now takes the minimum of that
and the estimate, so the min relay fee floor collapses the discard
rate down to the dust relay feerate. The lower discard rate reduces
the cost of change, so the ~614 sat leftover change in
test_dust_to_fee is now retained instead of being dropped to fee.
Rework the test to leave a sub-dust (20/270 sat) change that is
dropped regardless of the discard rate.
Co-authored-by: willcl-ark <will@256k1.dev>
Cache previous mempool fee rate estimates.
Cached estimates are tagged with the chain tip they were computed on
(the template's hashPrevBlock). They are only served while they are
not stale and the chain tip has not changed. This avoids generating
block templates too often.
The estimator lock is not held while building a block template, so
concurrent callers may duplicate estimation work; the tip tag keeps
stale results out of the cache.
Co-authored-by: willcl-ark <will@256k1.dev>
Add MemPoolFeeRateEstimator, which calls Bitcoin Core's block
assembler with the mempool and chainstate to build a block template and
use its chunk fee rates for fee rate estimation.
Add CalculateMaxWeightPercentiles to return the 50th and 75th
percentile chunk feerates by cumulative block weight. If sparse,
EstimateFeeRate uses the higher of the minimum relay fee rate and the
current mempool minimum fee rate.
The 50th percentile is returned as the conservative estimate, and the
75th percentile as the economical estimate.
Wire MemPoolFeeRateEstimator into FeeRateEstimatorManager and add
FeeRateEstimatorType::MEMPOOL_POLICY for result attribution.
Add unit tests for the mempool fee rate estimator and fee estimator
string conversions, plus fuzz coverage for the string conversions.
Co-authored-by: willcl-ark <will@256k1.dev>
Add a string fee_rate_estimator option (default "none") to
estimatesmartfee options. "block_policy" consults only the block
policy fee rate estimator, "none" uses the fee rate estimator
manager selected behaviour, and unknown values are treated as
"none". Unknown option keys are rejected.
Still only the block policy fee rate estimator, so the result is
unchanged; a subsequent commit will change the default behaviour.
Also adds GetFeeRateEstimate(FeeRateEstimatorType, target, conservative)
to FeeRateEstimatorManager so callers can query a single estimator by
type; NONE returns the manager-selected combined estimate.
Introduce FeeRateEstimatorManager to wrap CBlockPolicyEstimator and
act as the single point of contact for fee rate estimation in the node.
It inherits CValidationInterface so it can register directly with the
validation signals and receive mempool/block events.
Wire it into NodeContext (fee_estimator_man), init, shutdown, the RPC
server utility helpers (EnsureAnyFeeEstimatorMan), and the wallet-facing
interfaces::Chain API.
The Chain method estimateSmartFee is renamed to getFeeRateEstimate and
now returns util::Expected<FeeRateEstimation, FeeRateEstimationError>
instead of CFeeRate, so callers get the full estimation context without
needing FeeCalculation. estimateMaxBlocks is renamed to
maximumFeeEstimationTargetBlocks (still returns the max target).
CBlockPolicyEstimator no longer inherits CValidationInterface; the
manager now receives the mempool/block validation events and forwards
them to the CBlockPolicyEstimator.
Co-authored-by: willcl-ark <will@256k1.dev>
Introduce that common interface:
- FeeRateEstimatorType identifies the source estimator in a result.
- FeeRateEstimation carries the feerate and returned target of a
successful estimate; FeeRateEstimationError carries the error
message alongside a zero-value estimation.
- EstimateFeeRate wraps estimateSmartFee and returns
util::Expected<FeeRateEstimation, FeeRateEstimationError>.
- MaximumTarget delegates to HighestTargetTracked(LONG_HALFLIFE) so
callers do not need to know about block policy horizons.
Update call sites in rpc/fees.cpp and node/interfaces.cpp.
A later commit introduces FeeRateEstimatorManager, which selects
between multiple fee rate estimators. To compare estimates and
report which estimator produced them, the manager needs each fee
rate estimator to expose a uniform output, whereas
estimateSmartFee's CFeeRate/FeeCalculation output is specific to
the block policy fee rate estimator.
Co-authored-by: willcl-ark <will@256k1.dev>
The purpose of the test was to exercise CBlockPolicyEstimator behavior, but it
previously used a real CTxMemPool plus validation signals to track the
txs. Since TryAddToMempool does not emit TransactionAddedToMempool
callbacks, the test also had to fire those callbacks manually and sync the
validation interface queue around estimate checks.
Call processTransaction() and processBlock() directly instead. This removes
the mempool and validation-signal plumbing from the test, makes event
ordering explicit and synchronous, and avoids coupling the test to the
validation interface notifications.
This is useful because subsequent commits moved CBlockPolicyEstimator
from being validation interface client to FeeRateEstimatorManager.
Rename policyestimator_tests.cpp to blockpolicyestimator_tests.cpp.
Also rename the policy_estimator fuzz target to block_policy_estimator so the
test names match CBlockPolicyEstimator.
This makes the block policy fee rate estimator test files accurate and concise,
which makes adding another fee rate estimator test files straightforward.
Now that the wallet reports its own FeeReason, StringForBlockPolicyEstimateReason
is only used internally by the block policy estimator. Move it from
common/messages into the block policy fee rate estimator.
Also add the detailed FeeCalculation debug log to estimateSmartFee, where
the FeeCalculation data originates, and always populate feeCalc locally so
the log is available even when the caller does not pass a valid
FeeCalculation pointer.
The block policy estimator's FeeReason enum mixed two unrelated
concerns: the threshold that produced an estimateSmartFee result
(NONE, HALF_ESTIMATE, ...) and the reason the wallet selected a fee
rate (FALLBACK, MEMPOOL_MIN, REQUIRED).
Split them so each layer owns the reasons it reports:
- Add a wallet-facing FeeReason enum with the reasons the wallet can
select a fee rate: FEE_RATE_ESTIMATOR, MEMPOOL_MIN, USER_SPECIFIED,
FALLBACK, and REQUIRED.
- Rename the estimator enum to BlockPolicyEstimateReason and narrow it
to estimator reasons: NONE, HALF_ESTIMATE, FULL_ESTIMATE,
DOUBLE_ESTIMATE, and CONSERVATIVE.
- Return wallet fee selection metadata through MinimumFeeRateResult
instead of exposing FeeCalculation to wallet callers. The returned
target is now optional and is only set for fee rate estimator results.
Flatten GetMinimumFeeRate() with early returns while preserving the fee
selection order: user feerate still only applies the required-fee check,
while smart-fee results keep fallback, mempool-min, and required fallbacks.
The returned target is cleared for fallback, mempool-min, and required
results.
Replace the CreateTransactionInternal log with a simpler message that
does not depend on estimateSmartFee internals. Detailed estimator
logging will be added in a follow-up commit.
01dde6b205 fuzz: Fix assertion in txorphan (marcofleon)
Pull request description:
`EraseTx()` calls `LimitOrphans()`, which may evict announcements from a peer that didn't announce the erased transaction, causing that peer's usage to decrease. Relax the assertion in the `EraseTx()` branch that claimed usage of a non-announcer peer should be unchanged. Also, add assertions for the other cases.
ACKs for top commit:
dergoegge:
utACK 01dde6b205
instagibbs:
ACK 01dde6b205
Tree-SHA512: 2e597b85fd41058c2fa79fa55f0d37e12505065b5e27aba7b9680e0c249a5450e6fa97b45394d6ffe1318f42538134ffa9c423b126c455f6f8e6d8ca59eed4b6
9954aa7728 http: don't parse any new requests from a client if m_req_busy = true (Matthew Zipkin)
c7db3ae1f9 test: cover HTTPRequest state machine (Matthew Zipkin)
90676e24ad Add state to HTTPRequest to avoid duplicate work over I/O cycles (Matthew Zipkin)
507e528e84 http: reuse HTTPHeaders to parse chunked trailer (Matthew Zipkin)
902d8908c9 http: only read one HTTPRequest at a time per client (Matthew Zipkin)
Pull request description:
This PR reduces the memory consumption of the HTTP Server when reading data from connected clients, and improves performance especially when requests are large (i.e. requiring multiple TCP packets).
In https://github.com/bitcoin/bitcoin/pull/35182 the server copies as much data as it can from the socket into application memory, and then tries to parse as many complete HTTP requests as possible from that data. If a request is discovered to be incomplete, the in-progress request is abandoned. The server tries again on the next I/O cycle to read the same data from the buffer, duplicating work as many times as it takes before the client finishes sending the request (or times out).
This PR implements two improvements to this:
1. Only parse one request at a time from the receive buffer. The server processes requests from each client in series anyway.
2. Add state to `HTTPRequest` so it can be filled with data from the receive buffer over multiple I/O loop iterations without losing progress.
If a client sends large or multiple requests, that data will sit in the kernel's socket buffer instead of the application memory. Eventually the socket buffer will fill up and TCP backpressure will kick in, dropping the TCP window to 0 and blocking the client from sending any more.
A state machine for `HTTPRemoteClient` was [discussed previously](https://github.com/bitcoin/bitcoin/pull/35182#pullrequestreview-4322490068) to control resource consumption. Another nice benefit of this model (for a follow-up PR) will be to insert the RPC authentication check after reading 8kB-limited headers but before the 32MB-limited request body.
ACKs for top commit:
winterrdog:
re-ACK 9954aa7728
janb84:
re ACK 9954aa7728
frankomosh:
ACK 9954aa7728.
fjahr:
ACK 9954aa7728
Tree-SHA512: b7c913114283fbf1f360b40f6c65a01390a26731bf3b166f460ec260f9206f25d738b3a06887bfa839911c1c6aaf634448181da47a752a9a881aebd907e44868
fabe100c2b test: Use throwing config parser getters without fallback (MarcoFalke)
fa8acd57cd test: Write true/false values in config.ini (MarcoFalke)
Pull request description:
Currently, the called `getboolean` member function is *not* the throwing https://docs.python.org/3/library/configparser.html#configparser.ConfigParser.getboolean, but a non-throwing member function on a dict-like proxy object.
This is confusing and brittle, because tests shouldn't silently skip when a config key is missing. Instead, tests should loudly fail, e.g. when the config key is renamed in one place, but not the other.
ACKs for top commit:
jeanpablojp:
tACK fabe100c2b
willcl-ark:
ACK fabe100c2b
Tree-SHA512: a970d74ad285372b8adcce8e2a52b01f5a3b563899dfc5262e6ffbf3d8aba43e72f7b03111e8d5188924c7d3d789992407cddb5d182d9be5e42f07896d8ad4a3
b3d77ea027 test: Speedup fee estimation functional test with batching (sedited)
Pull request description:
The fee estimation functional test is currently the slowest one by a good margin. It is a bit annoying, because it also increases the total runtime of the functional tests.
It seems like most of the slowness comes from the transactions propagating between the nodes. This patch helps them do that by submitting them directly to all the nodes. Also take this opportunity to batch the transaction submissions.
On my machine this speeds up the fee estimation functional test from around 71 seconds to 25 seconds.
ACKs for top commit:
151henry151:
tACK b3d77ea027
maflcko:
review ACK b3d77ea027🐇
ismaelsadeeq:
ACK b3d77ea027
Tree-SHA512: f76415dca7997577ca39ac6b95dfdf32b930dd64b4311e3da34e34adb16107ff4ea2d9fa679f3ca50540e80c38af7f9390b44f21ad1b8107fbbc3586edb2ef19
25bed560be test: add forward-compat functional test for txindex (sedited)
703304ed8c doc: add release notes for txindex disk usage and downgrading (Andrew Toth)
8e5320a2d2 tests: cover txindex hash prefix collisions and legacy fallback (Andrew Toth)
b75efa19ba txindex: skip bloom filters and legacy lookups for new databases (Andrew Toth)
004d7c098c txindex: hash key prefixes and pack block positions (Andrew Toth)
5a255970fd refactor: move txindex db constants and legacy key to txindex_key.h (Andrew Toth)
327660134c txindex: pass the full block to DB::WriteTxs (Andrew Toth)
42771e7998 txindex: use a new block locator for downgrade safety (Andrew Toth)
4b08baed72 txindex: return optional tx and block hash from FindTx (Andrew Toth)
Pull request description:
The current txindex uses the full 32-byte txid as keys, which takes up about 66 GB of disk space today on mainnet. Using a 5-byte key prefix instead drops the disk usage to 26 GB - cutting the size to less than half.
Using the full 32-bytes is unnecessary since a 5-byte salted siphash will produce collisions in about 1 in 1.1 trillion. Some collisions will occur, but the penalty is just an extra disk read, deserialization and hash.
The tx position can be appended to the key instead of used as a value, and a LevelDB iterator can seek to the prefix and then scan for the correct tx. This is an almost identical approach to `txospenderindex`.
Also instead of storing the file position of the block, we can store only the sequence of the connected block and offset of the transaction in the block. This can be packed into a 6-byte key suffix using 3-byte representations of the sequence and offset in the block. The block file can be recovered by the CBlockIndex that is already in memory. The sequence is mapped to the block hash in the db, so we can lookup the block hash to find the CBlockIndex during reads.
If a tx is not found with this method, we fallback to looking up the legacy entry. With this method a user with an existing db can opt to erase the `indexes/txindex` folder and reindex, or keep the current index and new entries will be appended with the smaller footprint.
The time to index was faster on my machine with this method, 1h19m vs current 1h50m.
Lookups are roughly the same, around 0.2ms per lookup with `getrawtransaction`.
When testing on mainnet, I got 894,549 2-way collisions, 395 3-way collision, and 1 4-way collision that worst case could cause an extra 3 false positives when reading.
ACKs for top commit:
l0rinc:
diff reACK 25bed560be
sedited:
ACK 25bed560be
ajtowns:
ACK 25bed560be
Tree-SHA512: a25c79ca7e722e2f372b65f5fc11c8b194ad49f2240b4881c7e606306aabbd3604aede3f1c33606b467486affac3a3f503638f513c896935cebbc02709cb60d8
75a4e6c678 gui: fix allow restore wallets without .dat file extension (Pol Espinasa)
6ed7e05e20 gui: fix add .dat file extension automatically when exporting watchonly (Pol Espinasa)
Pull request description:
fixes https://github.com/bitcoin-core/gui/issues/956
Unlike `backup wallet`, `export watch-only wallet` was not automatically adding the file extension to the exported file, making restoring difficult if the user doesn't manually add the file extension after exporting.
Allows also to restore a wallet from a non specified `.dat` file extension. This is achieved by removing the filter in the select file screen, matching the RPC behavior.
ACKs for top commit:
hebasto:
ACK 75a4e6c678.
Tree-SHA512: 7c45d51205f9abf2b67233e8abd3297e49a4230eb32aa4118b37ab9da0a8d692aae4b67a8880881e5fab42256d1cf52ccf1b289b8f23f85930354130c192b33d
c3945bfd2b doc: use derivehdkey in multisig tutorial (Sjors Provoost)
3662e33669 test: use derivehdkey in M-of-N multisig demo (Sjors Provoost)
d9570f0838 rpc: add derivehdkey (Sjors Provoost)
62da9f9614 wallet: add GetExtKey helper (Sjors Provoost)
aaf1548475 wallet: generalize GetActiveHDPubKeys helper (Sjors Provoost)
3821452c4a refactor: add hardened derivation helper (Sjors Provoost)
0ab61caafd rpc: ParsePathBIP32 helper (Sjors Provoost)
e36c4b76e1 util: reject out-of-range BIP32 keypath indices (Sjors Provoost)
ba78c31a00 fuzz: check ParseHDKeypath/WriteHDKeypath round-trip (Sjors Provoost)
8cce969085 Have ParseHDKeypath handle h derivation marker (Sjors Provoost)
fc53077762 test: move parse_hd_keypath test to bip32_tests (Sjors Provoost)
dab525eb77 key: add DeriveExtKey() helper (Sjors Provoost)
Pull request description:
Adds a `derivehdkey` RPC that returns an xpub, or optionally the xprv, at an arbitrary BIP32 path (with at least one hardened step), derived from a wallet HD key.
The main use case is coordinating a multisig setup, where each participant shares an xpub derived at a hardened path (e.g. `m/87h/0h/0h`) distinct from their default single-signature descriptors. See the (updated) `doc/multisig-tutorial.md` and (updated) functional test to see how that workflow improves.
The first commits are some helpful helpers:
- _key: add DeriveExtKey() helper_ - performs the actual derivation
- _test: move parse_hd_keypath test to bip32_tests_ - from `psbt_wallet_tests`
- _Have ParseHDKeypath handle h derivation marker_
- _util: reject out-of-range BIP32 keypath indices_ - `ParseHDKeypath` would previously map overflowing values without `h` to hardened.
- _fuzz: check ParseHDKeypath/WriteHDKeypath round-trip_
- _rpc: ParsePathBIP32 helper_
- _refactor: add hardened derivation helper_ - `HasHardenedDerivation()`, to enforce the "at least one hardened step" rule
- _wallet: generalize GetActiveHDPubKeys helper_ - extracts code from `gethdkeys` which `derivehdkey` needs
- _wallet: add GetExtKey helper_ - reconstruct an xprv from a wallet xpub (analog of `GetKey()`); behavior-preserving prep, also simplifies `gethdkeys`.
Meat and potatoes:
- _rpc: add derivehdkey_ - the RPC itself, plus the `UnusedKey` filter on `GetHDPubKeys` that drives key selection.
- _test: use derivehdkey in M-of-N multisig demo_ - rewrites the functional multisig test to use the RPC and `<0;1>` syntax.
- _doc: use derivehdkey in multisig tutorial_ - same for the prose tutorial.
ACKs for top commit:
pseudoramdom:
code review ACK c3945bfd2b
achow101:
ACK c3945bfd2b
w0xlt:
That being the case, ACK c3945bfd2b
Tree-SHA512: 661f17c9bfe26017eb14c27ba7af37093387100d3baa25f5d29bba9c1aedc40d19afe1bdfc126a18d018857bb02f1fc84386f10b8f4f4b8e9d6f4b0691d9e302
ae36e2ef79 rpc: avoid quadratic prevout resolution (Lőrinc)
da1eaeb350 rpc: preserve `gettxspendingprevout` order (Lőrinc)
f98753e762 refactor: identify prevouts by request index (Lőrinc)
221a3fe5cf test: cover mixed `gettxspendingprevout` order (Lőrinc)
Pull request description:
**Problem:** `gettxspendingprevout` erases each mempool result from a vector while holding `mempool.cs`, shifting the remaining requests every time and making large calls quadratic in the critical section.
For 10,000 mempool matches, an [operation-count model](https://godbolt.org/z/nzch7McPG) reaches nearly 50 million moves.
For mixed requests, the RPC returns mempool results before `txospenderindex` results instead of following request order.
#34749 introduced both regressions.
**Fix:** `gettxspendingprevout` stores each result at its request position and collects unresolved requests in a reserved worklist for the `txospenderindex` lookup.
The mempool pass is linear, the response follows request order, and Clang can verify the lock requirement on `GetConflictTx`.
**Benchmark:** The [functional benchmark](https://gist.github.com/l0rinc/c3231e287cacfdefd100dbf95cd0c3ad) sends mempool-only requests ranging from 8,000 to 128,000 entries ten times per size.
Using the same settings for the unfixed and fixed commits:
```text
AMD Ryzen 7 3700X (8 cores)
unfixed ██████████████████████████████ 90 s
fixed ███▒░░░░░░░░░░░░░░░░░░░░░░░░░░ 10 s (-80 s, 9.0x faster)
Raspberry Pi 5 (4 cores)
unfixed ██████████████████████████████ 685 s
fixed ▓░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 22 s (-663 s, 31.1x faster)
```
The unfixed run timed out after ~9 minutes on a Raspberry Pi 4 with 1 GB RAM.
<details><summary>Benchmark command</summary>
```bash
for commit in 963b061358 46e7173550a93cbe9d4e8ea28cfe7216286d8197; do \
git fetch origin "$commit" && git checkout --detach "$commit" && \
rm -rfd build && cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTS=OFF -DENABLE_WALLET=OFF >/dev/null 2>&1 && \
ninja -C build -j1 bitcoind >/dev/null 2>&1 && \
build/test/functional/test_runner.py rpc_gettxspendingprevout_quadratic.py --repeats=10 || break; \
done
```
</details>
ACKs for top commit:
andrewtoth:
ACK ae36e2ef79
sedited:
Re-ACK ae36e2ef79
Tree-SHA512: c7734cae481f6638228c8fd3cc6d4c3fbc26cec6981dae7902292af08eae37455d37ff196da2cca5c3694271c99154838431ff21c12855f81f5f10edf85a793a
`gettxspendingprevout` erases each mempool result from its worklist while holding `mempool.cs`, shifting the remaining requests every time and making the pass quadratic when it resolves many requests.
Collect unresolved requests in a reserved worklist so the mempool pass is linear and the compiler can verify the lock requirement on `GetConflictTx`.
Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
EraseTx calls LimitOrphans, which may evict announcements from a peer
that didn't announce the erased transaction, causing that peer's usage
to decrease. Relax the assertion in the EraseTx branch that claimed
usage of a non-announcer peer is unchanged. Also, add assertions for
the other cases.
fab74a0e92 refactor: Use C++14 digit separator for large int literals (MarcoFalke)
fae759be79 scripted-diff: Use inline constexpr over plain constexpr (MarcoFalke)
fa74f58a26 scripted-diff: Use inline const over (static) const (MarcoFalke)
fab1a62c87 refactor: Use inline constexpr for string literals in headers (MarcoFalke)
fa08bbed8d contrib: Adjust generate-seeds.py to write inline constexpr (MarcoFalke)
fad753611b scripted-diff: Use inline constexpr over (static) const (MarcoFalke)
faedb52583 refactor: Make CFeeRate(integral) ctor constexpr (MarcoFalke)
5555d5dcb5 scripted-diff: Use inline constexpr over static constexpr (MarcoFalke)
fa6e1a1e85 refactor: Remove static from constexpr functions in headers (MarcoFalke)
Pull request description:
Both are fine and this refactor doesn't change any behavior.
However, `inline constexpr` from C++17 will ensure each symbol has a single address
across all TU, making the release binary minimally smaller. (For me it is smaller by about 1kB)
ACKs for top commit:
l0rinc:
reACK fab74a0e92
rustaceanrob:
ACK fab74a0e92
hebasto:
ACK fab74a0e92, I have reviewed the code and it looks OK.
Tree-SHA512: 6ec94136c12bcbf696812d0661c9857318a69e367c79fc00b9ca0b4068f269d10e5548d95c9ba2070225308c12d7a54fe8cb8447de7e0979cba99f48892b35f9
8b5da677d7 common: remove ::runtime_error from RunCommandParseJSON (fanquake)
Pull request description:
I don't think there's a code path that can reach `RunCommandParseJSON` if we compile with `ENABLE_EXTERNAL_SIGNER=OFF`. If there is a reason for having the code this way, it could be good to document.
This also requires more workarounds in #35911.
ACKs for top commit:
stickies-v:
re-ACK 8b5da677d7
sedited:
ACK 8b5da677d7
willcl-ark:
ACK 8b5da677d7
Tree-SHA512: b0c50372fed35afe47713310851f0b58cd1803fbe87a3a5a75877772172a3881283394a91663251de22a9972f56b46d84ddc868686dec8b970474cfaf5dc0d32
34c03075a5 test: run generic baseindex tests against every index type (Martin Zumsande)
11b3e251c4 test: make baseindex flush test chain-length agnostic (Martin Zumsande)
8b959f4c6a test: move unclean_shutdown test to baseindex_tests (Martin Zumsande)
2232d6afbe test: move index_reorg_crash to baseindex_tests (Martin Zumsande)
a3597e2683 test: move BuildChain helper into test mining util (Martin Zumsande)
954985e6a3 test: simplify blockfilter test's BuildChain helper (Martin Zumsande)
Pull request description:
In #34897, the `baseindex_tests` unit test was introduced, meant for tests that test basic index functionality (e.g. reorg or unclean shutdown behavior) that should work regardless of the particular index type.
This PR moves two more of these tests (`index_reorg_crash`, `coinstatsindex_unclean_shutdown`) from test files of specific indexes into that folder.
In the second part, tests are executed sequentially for all index types instead of just one particular one, where applicable.
Before moving `index_reorg_crash` I extracted the `BuildChain` helper to `util/mining` so that it can be used by multiple tests. While doing that, I simplified the helper a bit.
ACKs for top commit:
jeanpablojp:
tACK 34c03075a5
sedited:
ACK 34c03075a5
Tree-SHA512: 1d7a43160a9b7ec3c75a8c806967f2031da4855fe449c9c8aac8e44b1940e5ee28fde9473406666e74a682cf135b87f8c0eb9ddba50fce156dce9fc54a7763eb
I don't think there's a code path that can reach RunCommandParseJSON if
we compile with `-DENABLE_EXTERNAL_SIGNER=OFF`. This also requires more
workarounds in #35911.
Co-authored-by: stickies-v <stickies-v@protonmail.com>
New databases never contain legacy ('t' + txid) entries.
Peek at the database before opening it, and if no legacy
entries are found, skip building bloom filters (hashed
entries are only read via iterators, which do not consult
them) and return early from lookups instead of checking
for legacy entries.
Use a 5-byte salted siphash to key txindex entries,
instead of the full 32-byte txid. Store the block sequence and tx position
after the hash in the key, so an iterator can scan
through any collisions and return the correct tx.
Fall back to the legacy key lookup if the tx is not found.
Co-authored-by: Pieter Wuille <pieter@wuille.net>
Co-authored-by: l0rinc <pap.lorinc@gmail.com>
Co-authored-by: Anthony Towns <aj@erisian.com.au>
Move the per-transaction position computation from CustomAppend into
DB::WriteTxs, so the DB layer receives the whole block instead of a
pre-built vector of positions. This is a non-functional refactor.
The hashed txindex entries cannot be found by older nodes. Record sync
progress under a new locator key so a downgraded node will not rely on
entries indexed by upgraded nodes, and instead continue syncing from the
legacy locator.
d055a3ab10 test: verify disallowed RPC clients are rejected upon `accept()` (winterrdog)
Pull request description:
this is a follow-up PR from a suggestion in this [comment](https://github.com/bitcoin/bitcoin/pull/35592#pullrequestreview-4823271031)
it adds a unit test that confirms that clients not permitted by
`-rpcallowip` are rejected immediately after `accept()`, before any
request bytes are read from the socket
specifically, the test checks that: no request is ever dispatched to the
server's request handler, the connection is closed without any response
to the client, no `HTTPRemoteClient` is ever registered for it, and the
client's request bytes are left completely unread in the socket's
receive buffer
ACKs for top commit:
achow101:
ACK d055a3ab10
pinheadmz:
ACK d055a3ab10
w0xlt:
reACK d055a3ab10
Tree-SHA512: 5b99eb8a795e302333549f3ea5c76118c380402e2ebac8256db0a06ac0b4c0739b0b94471aa2f278214e65574e84f321b416e2543b361bf4fd8eb6854768377a
