mempool: dust thresholds, the ephemeral rules, and the template floors Core applies (#661, #670) - #709
Open
bkeroack wants to merge 7 commits into
Open
mempool: dust thresholds, the ephemeral rules, and the template floors Core applies (#661, #670)#709bkeroack wants to merge 7 commits into
bkeroack wants to merge 7 commits into
Conversation
This was referenced Sep 9, 2026
Every `apt-get update` in CI runs under `set -euo pipefail`, so a failure on *any* configured repository fails the step. The GitHub runner image ships Google Chrome and Microsoft apt sources that none of satd's build dependencies come from, and a hash-sum mismatch on the Chrome one took down all five canary jobs — twice, including on a rerun. Remove those sources before updating. The canaries still gate the PR on exactly what they gated before; they just stop depending on a repository they never install from. Applied at all fourteen sites rather than only the canary's, because the same step exists in CI, the Core-functional run, the differential fuzzer, the e2e flake gate and the release workflow, and there is nothing canary-specific about the failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
`dust_threshold_with_rate` charged a 68-vbyte spend for witness outputs where Core charges 67, a 107-vbyte spend for P2SH where Core charges 148, and truncated a fee Core rounds up. Every threshold was wrong: P2WPKH 297 against Core's 294, P2TR 333 against 330 — the number Lightning anchor outputs are built against — and P2SH 417 against 540, a 123-satoshi window in which satd relayed outputs Core calls dust. Branch on `IsWitnessProgram` as Core does, rather than on the three named witness types, so an unknown witness version and pay-to-anchor are priced as witness spends and a bare script is priced as a legacy one. Treat a script longer than `MAX_SCRIPT_SIZE` as unspendable alongside `OP_RETURN`, and size the length prefix. Core's own table is pinned as golden vectors. The dust *rules* were wrong too. Core's `IsStandardTx` permits one dust output — `MAX_DUST_OUTPUTS_PER_TX` — so that a zero-fee ephemeral-dust parent stays relayable; the zero-fee requirement is `PreCheckEphemeralTx` and the requirement that a child sweep it is `CheckEphemeralSpends`. satd refused every dust output on the single-transaction path and enforced the zero-fee rule only inside `accept_package`, with the strings written out by hand. `MempoolError::EphemeralDustFee` existed but was never constructed. Extract `pre_check_ephemeral` and call it from both paths; it reads the `prioritisetransaction` delta as well as the base fee, as Core's `base_fee != 0 || mod_fee != 0` does. The two branches share the reject reason `dust` and differ only in the detail, so the new tests assert the detail, and the ephemeral fixtures move to a configuration with a real relay floor where the generic fallback says something else. `-dustrelayfee` now reaches every dust decision: `tx_has_dust_outputs`, `count_dust_outputs` and `dust_output_indices` took the constant, so `prioritisetransaction`, the package path and the stranded-parent unwind ignored the operator's setting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
A spend of an immature coinbase can sit in the mempool after a reorg — `remove_for_reorg` cannot cover one accepted between the reorg and the sweep — and `connect_block` answers `bad-txns-premature-spend-of-coinbase` for the whole block, so one such transaction costs the miner every fee in the template. The assembler re-checks maturity against the height it is building, alongside the BIP 68 re-check it already did. `-blockmintxfee` was parsed, documented and restart-only, and read nowhere in the node: the template floor did not exist. It is applied on the *package* feerate, as Core's `addPackageTxs` compares `chunk_feerate_vsize` against `blockMinFeeRate`, so a zero-fee parent still rides in on a child that pays for both. At the default that subsumes the ad-hoc "a zero-fee transaction needs a paying descendant" rule it replaces, and `-blockmintxfee=0` now really does mine free transactions. The floor is a process value set once at startup rather than a parameter on eight mining entry points, since the option is restart-only; `create_template_with_floor` takes it explicitly so tests neither depend on nor disturb process state. Also documents `reorg` as a mempool eviction reason. The node has emitted it on every carrier since the reorg sweep landed while the wire spec and the Operator Manual published a shorter list; the wire-string test is now exhaustive over the enum, so adding a reason without documenting it does not compile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
…w set Core gates `PreCheckEphemeralTx` on `require_standard`, alongside `IsStandardTx` (`validation.cpp`), so `-acceptnonstdtxn` lifts it. It also belongs to the exemptable standardness set an `allow` rule can forgive (§6.2): without that a policy written to relay a dust shape could no longer relay one that pays a fee, which is every dust shape an operator would write a rule for. An earlier deferred failure still wins, because Core reports `IsStandardTx`'s verdict first. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
satd listed six output shapes by hand where Core accepts every type `Solver` names. Two were missing. Bare P2PK — `<pubkey> OP_CHECKSIG` — was refused outright, so a payment to the output type of the earliest coinbases would not relay. Witness programs at versions satd has no predicate for (v2 through v16, and pay-to-anchor) were refused too, which defeats the point of the version space: a node that will not relay them cannot relay the next soft fork's transactions. Not "any witness program", though: a v0 program at a length Core does not recognise falls through `Solver` to NONSTANDARD, so v0 stays pinned to its two sizes and only other versions are open. Bare multisig gains Core's x-of-3 bound, which `Script::is_multisig` does not check. `testmempoolaccept` also runs the ephemeral-dust pre-check now. Core reaches `PreCheckEphemeralTx` from `PreChecks`, which both RPCs go through, and satd's separate `test_accept` had no equivalent — so it answered `allowed: true` for a dusty fee-paying transaction `sendrawtransaction` refuses. Flips `mempool_dust.py` and `mempool_spend_coinbase.py` in the Core-functional inventory. The second was unblocked by the -25/-26 fix already on master; this is the measurement that confirms it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
… floor satd's `-blockmintxfee` default was 1000 sat/kvB — the same value as the default `-minrelaytxfee`. That is harmless only while the two agree. Lower the relay floor, which Bitcoin Core's own functional tests do, and every transaction between the two floors enters the mempool and is never mined: `generate` stops draining the mempool at all, and `feature_rbf.py` and `feature_bip68_sequence.py` both hang on it. Core's `DEFAULT_BLOCK_MIN_TX_FEE` is 1 sat/kvB, three orders of magnitude below the relay floor, and deliberately so: the floor exists to keep a template from being padded with transactions that pay *nothing*, not to second-guess relay policy. Nothing depended on the old default, because the option was read nowhere until the commit that applied it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
bkeroack
force-pushed
the
fix/661-670-dust-and-template-floors
branch
from
September 9, 2026 22:31
478d020 to
9dab8b4
Compare
Contributor
Author
|
Rebased so that the CI fix from #715 is now the first commit on this branch ( Why it had to move here rather than merge separately: a This branch's own content is unchanged: its tree differs from the previous tip by exactly the seven files in that commit, with the six workflow files byte-identical to the original. 🤖 Generated with Claude Code |
…ee rates `blockmintxfee=0.00001` in a bitcoin.conf went through `.parse::<u64>().ok()`, so Core's spelling of the option fell through to the default without a word — on the one PR that makes the option live. It now resolves through the same `file_fee_rate` path as minrelaytxfee/dustrelayfee/incrementalrelayfee: a decimal is BTC/kvB, a bare integer is sat/kvB, and garbage stops the node. The CLI arg gets the same value_parser. Manual row corrected (default is 1 sat/kvB, not 1000). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL
Contributor
Author
|
Independent review pass — one fix landed on this branch.
Verified against Core v31.1 🤖 Generated with Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #661. Closes #670.
Bottom of a four-PR stack; merge in order: this → #663 → #662 → #660-bounds.
Dust thresholds were wrong for every script type
Core's
GetDustThresholdadds 67 vbytes to spend any witness program —32 + 4 + 1 + 107/4 + 4— and 148 for anything else, then rounds the fee up. satd added 68 for the three witness types it knew by name, 107 for P2SH, 68 for everything else, and truncated.The 330 row is the one that bites: Lightning anchor outputs are built against it, and a node that calls 330 dust refuses a commitment transaction its peers relay. The P2SH row is a 123-satoshi window in which satd relayed outputs Core drops.
The classification now branches on
IsWitnessProgramas Core's does, so pay-to-anchor and future witness versions are priced as witness spends. Core's own table is pinned as golden vectors.The dust rules were wrong too
Core splits them three ways:
IsStandardTxpermits one dust output (MAX_DUST_OUTPUTS_PER_TX),PreCheckEphemeralTxrequires a dusty transaction to pay nothing at all, andCheckEphemeralSpendsrequires a child to sweep it. satd refused every dust output on the single-transaction path and enforced the zero-fee rule only insidesubmitpackage, with the strings written out by hand —MempoolError::EphemeralDustFeeexisted but was never constructed.Both paths now share one
pre_check_ephemeral, which reads theprioritisetransactiondelta as well as the base fee (Core'sbase_fee != 0 || mod_fee != 0). It is gated onrequire_standardas Core gates it, and joins the exemptable standardness set anallowrule can forgive.The two branches share the reject reason
dustand differ only in the detail, so the tests assert the detail and the ephemeral fixtures move to a configuration with a real relay floor where the generic fallback says something else — the trap from #700.Two standard output types satd did not relay
Bare P2PK —
<pubkey> OP_CHECKSIG— was refused outright, and so were witness programs at versions satd has no predicate for (v2–v16 and pay-to-anchor), which defeats the point of the version space. Not "any witness program", though: a v0 program at an unrecognised length isNONSTANDARDto Core'sSolverand stays refused. Bare multisig gains Core's x-of-3 bound.-dustrelayfeereached only one of four dust decisionsprioritisetransaction, the package classifier and the stranded-parent unwind all took the built-in rate, so-dustrelayfee=0changed the admission check and nothing else.Two template floors
A spend of an immature coinbase can sit in the mempool after a reorg, and
connect_blockrejects the whole block for it. The assembler re-checks maturity for the height it is building, as Core's does.-blockmintxfeewas parsed, documented and restart-only — and read nowhere. It is applied now on the package feerate, as Core'saddPackageTxsdoes, so a zero-fee parent still rides in on its child.Its default changes from 1000 sat/kvB to Core's 1. The old value matched the default
-minrelaytxfee, which is harmless only while the two agree: lower the relay floor and every transaction between them enters the mempool and is never mined.feature_rbf.pyandfeature_bip68_sequence.pyboth catch this, and a unit test fails if the default is put back.Already fixed on master
For the record, since these issues predate them. #670's reorg sweep is
Mempool::remove_for_reorg, landed in #657 with coinbase maturity, BIP 68, missing coins, locktime and the quarantine no-LeaveEvictedrule;pending_priorityis gone. #661's child-sideCheckEphemeralSpendsruns on the single-transaction path since #700, and stranded parents are unwound. What is left of both issues is in this PR.Verification
Three cargo gates green. Every guard perturbation-proved by a named failing test: the witness/legacy spend estimates, the
IsWitnessProgrambranch,div_ceil, theMAX_SCRIPT_SIZEhalf ofIsUnspendable, the compact-size prefix,pre_check_ephemeralon the single-tx path, its modified-fee term, the at-most-one-dust rule, the configured dust rate incount_dust_outputsand inprioritisetransaction, bare P2PK, unknown witness versions, the multisig bound, template coinbase maturity, the-blockmintxfeefloor, the package judgement, and the default floor.Core-functional:
mempool_dust.pyflips torun(measured failing before, passing after), andmempool_spend_coinbase.pyflips too — it was unblocked by the -25/-26 fix already on master and this is the measurement. The whole 37-row run set passes.🤖 Generated with Claude Code
https://claude.ai/code/session_011H5HUWaezLTYJCZDpgaioL