Repository navigation
RFC: measurements of the three announced policy changes against v31.1 (issues are disabled, hence a draft PR) - #1
Draft
tialkan wants to merge 3 commits into
Conversation
…tants MAX_STANDARD_TX_WEIGHT alone does not compile: packages.h asserts MAX_PACKAGE_WEIGHT >= MAX_STANDARD_TX_WEIGHT, and then MAX_PACKAGE_WEIGHT <= DEFAULT_CLUSTER_SIZE_LIMIT_KVB * WITNESS_SCALE_FACTOR * 1000. All three have to move together. This commit is the minimal change that builds and produces the intended runtime ceiling (3,894,045 WU accepted, 3,906,445 rejected as tx-size on regtest). It deliberately does NOT update the tests it breaks; those are five test cases, three of which raise behaviour questions rather than expectation updates. See https://github.com/tialkan/bitcoin-dust-utxo-lab for measurements. Not proposed for merge as-is.
test_IsStandard built its boundary transaction from hardcoded numbers: 2438 inputs and a 19 byte OP_RETURN payload, summing to exactly 400,000 weight units. Raising the constant broke it, and hardcoding the new numbers would break it again on the next change. Compute the input count and payload size from the constant instead, so the transaction lands on whatever the ceiling happens to be. Verified to pass at both 400,000 (on upstream master) and 3,900,000. This is the only one of the five unit test cases broken by the weight change that is a mechanical fix. The remaining four encode assumptions about behaviour at the old ceiling and need a decision rather than an update; they are left failing deliberately.
This was referenced Aug 28, 2026
… label The runners/record-frozen-commit jobs ran on runs-on: ubuntu-slim, a custom Warp runner image scoped to the upstream bitcoin/bitcoin org. This fork has no access to that runner group, so GitHub failed the whole workflow before scheduling any job (0 jobs, conclusion failure, "workflow file issue" - e.g. run 33050282126 on dogmode/tx-weight-3900k). Both jobs only run shell steps, so a standard GitHub-hosted ubuntu-latest works regardless of provider.
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
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.
Filing this as a draft PR because Issues and Discussions are both disabled on this repository, so there is no other way to raise it. This is not a merge request. The commit deliberately does not update the tests it breaks. It exists so the analysis below has something concrete to point at, and I will close it on request.
I implemented the three announced changes against v31.1 and measured what happens. Harness, raw JSON and full write-ups: https://github.com/tialkan/bitcoin-dust-utxo-lab
Everything below is reproducible on regtest with a stdlib-only Python driver.
1. The dust change appears not to need code
-dustrelayfeeis registered ininit.cppwithALLOW_ANY | DEBUG_ONLYand, unlike-acceptnonstdtxna dozen lines below it in the same function, it is not gated onchainparams.IsTestChain(). It works on mainnet.Smallest output value stock v31.1 does not reject as
dust, binary-searched viatestmempoolaccept:If the intent is a different default rather than a new capability, shipping it as a default change plus release notes keeps the diff against upstream smaller, which matters directly for the rebase burden on every Core release.
Minor: the "294-546 sat" range that gets quoted is incomplete. P2TR is 330, P2SH-P2WPKH is 540.
2. The weight change is three coupled constants
MAX_STANDARD_TX_WEIGHT{3900000}alone does not compile:Raising
MAX_PACKAGE_WEIGHThits the next assertion four lines down,MAX_PACKAGE_WEIGHT <= DEFAULT_CLUSTER_SIZE_LIMIT_KVB * WITNESS_SCALE_FACTOR * 1000, which requiresDEFAULT_CLUSTER_SIZE_LIMIT_KVB >= 975.MAX_STANDARD_TX_WEIGHTpolicy/policy.hMAX_PACKAGE_WEIGHTpolicy/packages.hDEFAULT_CLUSTER_SIZE_LIMIT_KVBpolicy/policy.hThat is the commit in this PR. Verified at runtime on regtest, single transaction with no unconfirmed parents: 3,894,045 WU accepted, 3,906,445 WU rejected as
tx-size.Two consequences that follow mechanically and are worth deciding on rather than inheriting:
MAX_OP_RETURN_RELAYisMAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR(policy.h:83) and feeds the defaultmax_datacarrier_bytes(kernel/mempool_options.h:53). The weight change silently raises the default OP_RETURN limit from 100,000 to 975,000 bytes.Flatten()intxmempool.cpprequires-maxmempool >= cluster_size_vbytes * 40, moving the enforced floor from about 4 MB to 39 MB. I first wrote that this was "not a blocker" since it sits under the 300 MB default. That was wrong, and section 4 below shows why:Flatten()treats it as a fatal configuration error rather than clamping, so anything pinning a smaller mempool stops booting.Note also that v31.1 replaced ancestor/descendant size limits with cluster size limits.
-limitancestorsizeand-limitdescendantsizeare now no-ops that emit a startup warning.3. The measured cost of a 1-sat output
450,000 1-sat outputs created and mined on regtest. Marginal chainstate cost per UTXO, checkpoint to checkpoint:
63.97, 42.67, 42.65, 42.65, 42.65, 42.65bytes. The first interval carries LevelDB's initial allocation; after that it is flat.A block whose 1,000,000 vB is entirely 1-sat dust holds about 32,219 outputs:
These outputs are permanent, not transient. An output is economically spendable only if its value exceeds the fee cost of the input spending it. A P2TR key path input is 57.5 vB, so a 1-sat output would need a feerate below 0.017 sat/vB to be worth spending, far under the 1 sat/vB minimum relay floor.
This is the part I would most like to see addressed. If these outputs are intended never to be spent and to function as markers, then the framing that follows is "how much do we grow the UTXO set permanently, and who pays for it", rather than "how much value is unlocked". Both halves are now quantified.
4. Test impact
Two builds, identical configuration. Stock v31.1:
*** No errors detected. With the three-constant patch: 19 assertion failures across five test cases.transaction_tests/test_IsStandardtest/transaction_tests.cpptxpackage_tests/package_sanitization_teststest/txpackage_tests.cpptxpackage_tests/package_validation_teststest/txpackage_tests.cpporphanage_tests/DoS_mapOrphanstest/orphanage_tests.cppcoinselector_tests/check_max_selection_weightwallet/test/coinselector_tests.cppSince opening this,
transaction_tests/test_IsStandardis fixed in the branch: it built its boundary transaction from hardcoded numbers (2438 inputs, 19 byte payload, summing to 400,000), so it now derives the input count and payload from the constant instead. Verified to pass at both 400,000 on upstream master and 3,900,000 here. Four unit test cases remain.That one was the only mechanical fix. The rest are behaviour questions. For example
package-too-largestops being reachable because the package count limit trips first:That is not a string to update; it is a package relay semantics change someone has to decide on. I did not want to rewrite those tests and thereby quietly pick an answer. To be concrete about what each needs:
txpackage_tests/package_sanitization_testsandpackage_validation_tests:package-too-largestops being reachable. Whether that is acceptable is a package relay semantics decision.Withdrawn. I measured this afterwards and it is wrong: the per-peer memory reservation binds before that check, so the largest orphan actually retained is ~400,000 WU on stock and on a patched build alike. The memory a peer can pin does not change. This one is a mechanical fix, deriving the hardcodedorphanage_tests/DoS_mapOrphans: the orphan pool's DoS bounds are sized against the standard transaction ceiling. Raising it by 9.75x changes the memory an attacker can pin. That needs a deliberate answer, not a new expected number.tx.vin.resize(2777)from the constant. Numbers and method in finding 7.coinselector_tests/check_max_selection_weight: all three scenarios are built around a 400,000 ceiling and a 49.5 BTC target, with coin counts chosen so the transaction would overflow it. At 3,900,000 the scenarios no longer test what they were written to test. Restoring them means scaling coin counts, amounts and the target together, which encodes an assumption about intent that is not mine to make.Functional tests, added after the initial version of this post.
Same method, both builds,
-j10, 267 files. The stock baseline is not clean on macOS: two environmental failures (interface_http.py,tool_utxo_to_sqlite.py) and 21 skips. Against that baseline, 21 functional failures are attributable to the patch.Eleven of them are not test failures. The node will not start:
feature_fee_estimation,mempool_limit,mempool_package_rbf,p2p_addr_relay,p2p_blocksonly,p2p_compactblocks_blocksonly,p2p_feefilter,p2p_opportunistic_1p1c,p2p_sendtxrcncl,p2p_tx_download,rpc_packages. Each starts a node with a small-maxmempool. This is a migration problem rather than a test problem, and it is invisible in the constant diff.The other ten are genuine policy behaviour changes:
mempool_accept,mempool_datacarrier,mempool_package_limits,mempool_sigoplimit,p2p_segwit,rpc_psbt,wallet_fundrawtransaction,wallet_miniscript,wallet_send,wallet_sendall.mempool_datacarrierfailing is direct confirmation of theMAX_OP_RETURN_RELAYcoupling described in section 2. The four wallet failures show the change reaches past relay policy into coin selection and funding.So the revised total against a three line diff is five unit test cases and 21 functional tests.
A suggestion on the eleven that will not start
The eleven failures above are all the same thing:
Flatten()intxmempool.cpptreats a-maxmempoolbelowcluster_size_vbytes * 40as a fatal configuration error.Changing eleven test files to pass a larger
-maxmempoolwould make the suite green while leaving the real problem in place. Any node, deployment script or CI job that pins a small mempool stops booting after upgrading, with no migration path other than editing configuration on every host.Clamping instead of erroring fixes both at once: raise the effective value to the minimum, log a warning saying it was raised and why, and start. The tests then pass unmodified, and operators get a warning in the log rather than a node that will not come up. If the fatal error is deliberate, it is worth saying so in release notes, because it is a breaking configuration change and it is not visible in the constant diff.
I have not implemented this, since it is a judgement call about how much operator configuration a policy change is allowed to invalidate, and that is yours rather than mine.
What I am not claiming
Happy to do the test rework and open a real PR if you tell me how you want the three behaviour questions in section 4 answered. Corrections on the methodology are welcome; the harness is MIT and every raw run is committed.