You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(dash)!: bound partial merkle trees by Dash's block size - #1307
Follow-up to #1200. This gap was listed in the handoff comment on #1205 (#1205 (comment)): "extract_matches uses Bitcoin's 16,666-transaction bound where a Dash block holds about 33,333".
Problem
PartialMerkleTree::extract_matches refused a tree of more than MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT = 4,000,000 / 240 = 16,666 transactions. That is Bitcoin's BIP141 bound. A Dash block holds up to 2 MB over the 60 bytes of the smallest transaction. Dash Core's CPartialMerkleTree::ExtractMatches refuses only nTransactions > MaxBlockSize() / 60 = 33,333 (src/merkleblock.cpp, src/consensus/consensus.h).
Since #1200, every masternode list diff proves its coinbase with this tree. For a valid Dash block of 16,667–33,333 transactions, MnListDiff::coinbase_merkle_root fails with InvalidCoinbaseProof. That is one of the two refusals FeedError::blames_peer holds against the peer. So the honest peer is banned, then the next peer asked for that list, and masternode sync stops. Any miner can build such a block on purpose, with about 1 MB of 60-byte transactions.
#400 (SegWit/Taproot removal) left MAX_BLOCK_WEIGHT and MIN_TRANSACTION_WEIGHT in blockdata::constants. This was their only use.
Fix
extract_matches bounds the count by a private MAX_BLOCK_TRANSACTIONS = MAX_BLOCK_SIZE / 60, Dash Core's bound. blockdata::constants::MAX_BLOCK_SIZE is new: it is Dash Core's MAX_DIP0001_BLOCK_SIZE, 2,000,000 bytes. It replaces the BIP141 MAX_BLOCK_WEIGHT and MIN_TRANSACTION_WEIGHT, which nothing in the workspace used any more.
The other checks are unchanged:
no transactions;
more hashes than transactions;
fewer bits than hashes;
every bit and hash used up;
identical left/right refused.
The traversal stays bounded by the hashes and flag bits the proof carries. The depth is log2 of the count, so the higher bound adds no work. A security review found no new forgery, malleability, depth-confusion or DoS surface.
Rejected: a dependency on base-sdk's dash-primitives for MAX_DIP0001_BLOCK_SIZE, which adds a crate for one constant.
Red → green
a_partial_merkle_tree_spans_as_many_transactions_as_a_dash_block_holds (dash/src/merkle_tree/block.rs) was committed before the fix:
passes; 33,334 transactions are still refused with TooManyTransactions
Breaking
blockdata::constants::MAX_BLOCK_WEIGHT and MIN_TRANSACTION_WEIGHT are removed. Use MAX_BLOCK_SIZE, Dash's limit in bytes. dev already carries breaking dashcore changes since v0.45.0.
Checks (local)
At 1ca9924 (head), every check below passed in full: dashd_masternode 11/11 and dashd_sync 38/38. The 8 tests dev already ignores (3 #[ignore], 5 ignore doctests) were skipped.
cargo test --workspace --all-features --no-fail-fast with the dashd regtest env: everything passed, including dashd_masternode 11/11, except one test in dashd_sync.
dashd_sync ran while another session's dashd suite and builds held the machine at a load average of about 25.
First full run: 37/38. test_coinbase_matures_when_a_later_block_touches_the_wallet failed ("the newest coinbase is immature").
Second full run: 35/38. That test, test_coinbase_matures_without_later_wallet_activity and test_runtime_add_with_tip_advance_during_rescan failed.
The two coinbase tests pass alone, 3 runs of 3.
The full suite passes 38/38 with --test-threads=4.
These regtest blocks hold a handful of transactions, far below the old 16,666 bound, so this change cannot reach them.
Skipped: the 8 tests dev already ignores (3 #[ignore], 5 ignore doctests).
Notes
Out of scope (pre-existing):PartialMerkleTree::from_parts expands the flag bytes into a Vec<bool> before any bound check.
files with no dedicated owner (dash/src/blockdata/constants.rs, dash/src/merkle_tree/block.rs) — QuantumExplorer or ZocoLini or xdustinface
xdustinface requested changes — waiting for them to re-review or dismiss
When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.
Summary by CodeRabbit
Behavior Changes
Once DIP-0001 is active, the maximum serialized block size is 2 MB.
Transaction inclusion proofs now use a transaction-count limit based on that block-size cap. Proofs for up to 33,333 transactions are accepted; proofs for 33,334 or more are rejected.
A Dash block holds up to 33,333 transactions, its 2 MB size limit over the
60 bytes of the smallest transaction. Dash Core's
`CPartialMerkleTree::ExtractMatches` allows that many
(`nTransactions > MaxBlockSize() / 60`). `extract_matches` still uses
Bitcoin's BIP141 bound, `MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT` =
16,666, so it refuses the proof of any larger Dash block. That includes
the coinbase proof of a masternode list diff at that block, which
`coinbase_merkle_root` reports as `InvalidCoinbaseProof`, and that
refusal bans the peer.
This test fails on dev: 33,333 transactions are refused with
`TooManyTransactions`. The next commit fixes it.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
`PartialMerkleTree::extract_matches` refused a tree of more than
`MAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT` = 16,666 transactions. That
is Bitcoin's BIP141 bound. A Dash block holds up to 2 MB over the 60
bytes of the smallest transaction, so up to 33,333 transactions, and
Dash Core's `CPartialMerkleTree::ExtractMatches` allows exactly that
(`MaxBlockSize() / 60`, src/merkleblock.cpp).
So a valid Dash block above 16,666 transactions could not be proven. A
masternode list diff at that block failed its coinbase proof with
`InvalidCoinbaseProof`. That refusal bans the peer, so every honest peer
asked for that list was banned in turn, and masternode sync stopped. Any
miner can build such a block.
Raising the bound adds no work. The traversal is bounded by the hashes
and flag bits the proof carries, and the depth of the tree is log2 of
the count.
The BIP141 constants stay public in `blockdata::constants`. Nothing in
the crate uses them any more.
Test would have caught this in CI:
`a_partial_merkle_tree_spans_as_many_transactions_as_a_dash_block_holds`.
✖ before the fix (33,333 transactions refused with
`TooManyTransactions`), ✔ after.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
No actionable comments were generated in the recent review.
Walkthrough
The change adds a 2,000,000-byte serialized block-size limit and uses it to set the maximum transaction count accepted by partial Merkle proof extraction. Tests check the boundary at 33,333 and 33,334 transactions.
Adds MAX_BLOCK_SIZE and derives a private transaction-count limit from it. extract_matches rejects counts above that limit. Tests check acceptance at 33,333 transactions and rejection at 33,334.
The change permits larger valid proofs; no actionable merge-blocking risk remains identified.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main change: limiting partial merkle trees using Dash's block-size bound.
Docstring Coverage
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 2 files.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 2900e46 and 1ca9924.
Files selected for processing: 2
dash/src/blockdata/constants.rs
dash/src/merkle_tree/block.rs
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.
Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.
✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.16%. Comparing base (2900e46) to head (1ca9924). ⚠️ Report is 11 commits behind head on dev.
The reason will be displayed to describe this comment to others. Learn more.
Code
Put Dash's block size in blockdata::constants and drop MAX_BLOCK_WEIGHT and MIN_TRANSACTION_WEIGHT.
After this PR nothing in the workspace uses those two constants. They still describe Bitcoin's BIP141 limit, the one that caused this bug, while Dash's real limit sits as a bare 2_000_000 in merkle_tree/block.rs. Removing them costs nothing extra, because dev already carries unreleased breaking dashcore changes since v0.45.0 (#400, #1288). Replace both with pub const MAX_BLOCK_SIZE: u32 = 2_000_000; (Core's MAX_DIP0001_BLOCK_SIZE) in constants.rs and derive MAX_BLOCK_TRANSACTIONS from it. The whole workspace still compiles with that change.
…s block size
`MAX_BLOCK_WEIGHT` and `MIN_TRANSACTION_WEIGHT` described Bitcoin's
BIP141 limit, the bound that refused large Dash blocks' partial merkle
trees. Nothing in the workspace uses them any more, while Dash's block
size sat as a bare `2_000_000` in `merkle_tree/block.rs`.
`blockdata::constants::MAX_BLOCK_SIZE` is now Dash Core's
`MAX_DIP0001_BLOCK_SIZE`, 2,000,000 bytes, and the partial merkle tree
bound is derived from it. The bound stays at 33,333 transactions.
BREAKING CHANGE: `blockdata::constants::MAX_BLOCK_WEIGHT` and
`MIN_TRANSACTION_WEIGHT` are removed. Use `MAX_BLOCK_SIZE`, Dash's limit
in bytes.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
shumkov
changed the title
fix(dash): bound partial merkle trees by Dash's block size
fix(dash)!: bound partial merkle trees by Dash's block size
Nov 3, 2026
Waiting for bot review — coderabbitai not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.
Ready for review — files with no dedicated owner: QuantumExplorer or ZocoLini or xdustinface · re-review or resolve: xdustinface.
Full checklist in the description.
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
ready-for-humanBots have reported, the author has self-reviewed, and the build is green: this needs a human.
2 participants
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.
github-actions
Bot
added
the
waiting-bots
xdustinface
left a comment
shumkov
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Follow-up to #1200. This gap was listed in the handoff comment on #1205 (#1205 (comment)): "
extract_matchesuses Bitcoin's 16,666-transaction bound where a Dash block holds about 33,333".Problem
PartialMerkleTree::extract_matchesrefused a tree of more thanMAX_BLOCK_WEIGHT / MIN_TRANSACTION_WEIGHT= 4,000,000 / 240 = 16,666 transactions. That is Bitcoin's BIP141 bound. A Dash block holds up to 2 MB over the 60 bytes of the smallest transaction. Dash Core'sCPartialMerkleTree::ExtractMatchesrefuses onlynTransactions > MaxBlockSize() / 60= 33,333 (src/merkleblock.cpp, src/consensus/consensus.h).Since #1200, every masternode list diff proves its coinbase with this tree. For a valid Dash block of 16,667–33,333 transactions,
MnListDiff::coinbase_merkle_rootfails withInvalidCoinbaseProof. That is one of the two refusalsFeedError::blames_peerholds against the peer. So the honest peer is banned, then the next peer asked for that list, and masternode sync stops. Any miner can build such a block on purpose, with about 1 MB of 60-byte transactions.#400 (SegWit/Taproot removal) left
MAX_BLOCK_WEIGHTandMIN_TRANSACTION_WEIGHTinblockdata::constants. This was their only use.Fix
extract_matchesbounds the count by a privateMAX_BLOCK_TRANSACTIONS = MAX_BLOCK_SIZE / 60, Dash Core's bound.blockdata::constants::MAX_BLOCK_SIZEis new: it is Dash Core'sMAX_DIP0001_BLOCK_SIZE, 2,000,000 bytes. It replaces the BIP141MAX_BLOCK_WEIGHTandMIN_TRANSACTION_WEIGHT, which nothing in the workspace used any more.The other checks are unchanged:
The traversal stays bounded by the hashes and flag bits the proof carries. The depth is log2 of the count, so the higher bound adds no work. A security review found no new forgery, malleability, depth-confusion or DoS surface.
Rejected: a dependency on base-sdk's
dash-primitivesforMAX_DIP0001_BLOCK_SIZE, which adds a crate for one constant.Red → green
a_partial_merkle_tree_spans_as_many_transactions_as_a_dash_block_holds(dash/src/merkle_tree/block.rs) was committed before the fix:assertion failed: tree(33_333).extract_matches(&mut vec![], &mut vec![]).is_ok()TooManyTransactionsBreaking
blockdata::constants::MAX_BLOCK_WEIGHTandMIN_TRANSACTION_WEIGHTare removed. UseMAX_BLOCK_SIZE, Dash's limit in bytes. dev already carries breakingdashcorechanges since v0.45.0.Checks (local)
At 1ca9924 (head), every check below passed in full:
dashd_masternode11/11 anddashd_sync38/38. The 8 tests dev already ignores (3#[ignore], 5ignoredoctests) were skipped.At f5b94f7, before the constants change:
cargo fmt --all --check, and typos 1.40.0 on the changed files.cargo +1.89 check --workspace --all-features.cargo clippy --workspace --all-targets --all-features -- -D warnings.RUSTDOCFLAGS="-D warnings" cargo doc --workspace --all-features --no-deps.cargo test --workspace --all-features --no-fail-fastwith the dashd regtest env: everything passed, includingdashd_masternode11/11, except one test indashd_sync.dashd_syncran while another session's dashd suite and builds held the machine at a load average of about 25.test_coinbase_matures_when_a_later_block_touches_the_walletfailed ("the newest coinbase is immature").test_coinbase_matures_without_later_wallet_activityandtest_runtime_add_with_tip_advance_during_rescanfailed.--test-threads=4.#[ignore], 5ignoredoctests).Notes
PartialMerkleTree::from_partsexpands the flag bytes into aVec<bool>before any bound check.🤖 Generated with Claude Code
PR Hygiene ·
1ca9924dash/src/blockdata/constants.rs,dash/src/merkle_tree/block.rs) — QuantumExplorer or ZocoLini or xdustinfaceWhen every merge requirement is met, the
PR Hygienecheck passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.Summary by CodeRabbit