Wp v2 #20340

almogdepaz · 2025-12-10T15:29:21Z

Purpose:

Implement new Weight Proof Protocol with MMR-based proofs and optimized sub-epoch challenge segment tracking.
this will allow smaller more efficient proofs that are more secure

Current Behavior:

use old Weight proofs protocol when syncing

New Behavior:

V3 Weight Proofs: we add MMR roots to the RewardChainBlock aggregating the header hashes of the previous
blocks using the MMR roots we will be able to prove block inclusion by header hash, we will later use this in the new
protocol to sample individual blocks and not full challenge slots like the current protocol.
the MMR root is hashed into the RewardChain VDF so it can't be manipulated without affecting the challenge chain.
we also add merkle roots committing to the number of blocks in each challenge slot in the sub-spoch so we can know
what slot each block in the sub epoch belongs to without downloading more then the actual block
MMR Infrastructure: New MMRManagerProtocol for tracking block header commitments
Sub-Epoch Challenge Tree infrastructure: Track and store challenge chain segments data
moved MerkleTree from wallet.util to chia.util

Testing Notes:

Note

Introduce MMR-based header commitments and sub-epoch challenge merkle roots (post-HF2), wiring them through consensus, full node/timelord flows, weight proofs, and tests.

Consensus/Core:
- MMR Infrastructure: Add MMRManagerProtocol, BlockchainMMRManager, and flat MMR (consensus/mmr.py); integrate mmr_manager into Blockchain, AugmentedBlockchain, BlockCache, and WalletBlockchain (copy/rollback/add/update, new get_mmr_root_for_block/get_current_mmr_root).
- Block Creation/Validation: Compute and embed reward_chain_block.header_mmr_root (via unfinished_block_to_full_block_with_mmr); validate MMR root in validate_finished_header_block after HARD_FORK2_HEIGHT; add post_hard_fork2() helper and skip_commitment_validation path for weight proofs.
- Sub‑Epoch Challenge Root: New consensus/challenge_tree.py; extend make_sub_epoch_summary to include challenge_merkle_root post-HF2; propagate through validation and SES handling.
Full Node & Timelord:
- Full node computes MMR root for unfinished blocks and includes current root in NewUnfinishedBlockTimelord; timelord uses provided header_mmr_root when finishing blocks.
- Use post_hard_fork2 to conditionally include SES challenge roots.
Weight Proofs:
- Include challenge_merkle_root in SubEpochData/reconstruction; skip commitment validation during WP verification.
Simulator/Tools:
- BlockTools maintains an BlockchainMMRManager during block generation; pass through to block building paths.
Tests:
- Add comprehensive MMR and commitment tests (test_mmr.py, test_block_commitments.py, sub-epoch summary tests); update fixtures for fork heights; adjust mocks to StubMMRManager; minor test updates for API changes.
Protocol/Types:
- Extend timelord_protocol.NewUnfinishedBlockTimelord with header_mmr_root; update test vectors/json accordingly.

^{Written by Cursor Bugbot for commit 87ebec0. This will update automatically on new commits. Configure here.}

chia/consensus/augmented_chain.py

chia/consensus/blockchain_mmr.py

chia/consensus/challenge_tree.py

chia/consensus/blockchain_mmr.py

arvidn

as far as I can tell, you don't need MerkleTree.py. I think you can revert those changes, and use compute_merkle_set_root() instead.

arvidn · 2025-12-16T01:27:59Z

chia/_tests/blockchain/test_block_commitments.py

+                    slot = block.finished_sub_slots[0].replace(
+                        challenge_chain=block.finished_sub_slots[0].challenge_chain.replace(subepoch_summary_hash=None)
+                    )
+                    block_no_challenge_root = block.replace(finished_sub_slots=([slot]))


are the parentheses deliberate? It they won't create a 1-tuple, right?

it has the same outcome but you are correct this is a mistake

arvidn · 2025-12-16T01:29:07Z

chia/_tests/blockchain/test_blockchain.py

+            "chia.consensus.blockchain_mmr.BlockchainMMRManager.add_block_to_mmr",
+            lambda *args, **kwargs: None,
+        )
+


I have a feeling this monkey patching will come back and bite us later

the problem here is that BlockchainMMRManager catches the error before blockHeader validation, i can change the expected error here, but this test is meant for header validation and blockchain and i dident want us to bypass, i can add the same test without the monkeypatch to check that the MMR manager catches this as well

arvidn · 2025-12-16T01:40:09Z

chia/consensus/blockchain_mmr.py

+
+    _mmr: MerkleMountainRange
+
+    _checkpoints: dict[int, MerkleMountainRange]  # height -> MMR snapshot


we usually use uint32 for heights. If there's a special reason to be able to use -1 (or some other value that doesn't fit in uint32) it would probably warrant a comment

Suggested change

_checkpoints: dict[int, MerkleMountainRange] # height -> MMR snapshot

_checkpoints: dict[uint32, MerkleMountainRange] # height -> MMR snapshot

I can imagine this being more efficient, in the future:

We could increase the distance between checkpoints the farther away from the peak we are

We could reuse nodes between the checkpoints, as they probably have a lot of overlap

i am looking into this
https://commonware.xyz/blogs/mmr its purposely built for blockchain use cases i need to explore the api to see if it has exactly what we need but basically the whole deal is an MMR database with rolleback capabilities

actually, my understanding of MMR is that you append new items. So rolling back should just be a matter of popping the most recent items. I don't think you need to store a history of previous MMRs.

In fact, you could even provide a view onto an MMR for an earlier peak, by just pretending the most recent items aren't there.

arvidn · 2025-12-16T01:42:27Z

chia/consensus/blockchain_mmr.py

+            self._max_checkpoints = max_checkpoints
+
+    def copy(self) -> BlockchainMMRManager:
+        return BlockchainMMRManager(self)


it requires some work to make sure this really copies the content. I think it does, but wouldn't making this a dataclass support copying in a built-in manner?

dataclass doesnt provide deep copy, but i agree that using dataclass is more like what we usually do and i will change and use copy.deepcopy instead of the custom logic

chia/consensus/blockchain_mmr.py

arvidn · 2025-12-16T13:13:16Z

chia/simulator/block_tools.py

                        blocks_added_this_sub_slot += 1
                        blocks[full_block.header_hash] = block_record
+                        # Add block to MMR manager for proper MMR computation
+                        self.mmr_manager.add_block_to_mmr(


and here, should you really do this for heights < constants.HARD_FORK2_HEIGHT?

oh theres a nuance here we include the mmr root only above the fork, question is since when do we add blocks to the mmr

currently this assumes from genesis, which is probably not what we want to do (although we wont be able to prove inclusion for prefork blocks), this relates to the what Bram said about doing the checkpoint, we can just opt to start adding from the fork ill do that and will make sure with Bram

this is only relevant to the mmr commitment since the other one is per sub epoch

in any case still need to add tests that start an already synced node to make sure that this works not only when syncing from 0 but also when restarting a node, this scenario is not tested yet

chia/simulator/block_tools.py

arvidn · 2025-12-16T13:15:37Z

chia/util/block_cache.py

        hh = block.header_hash
        self._block_records[hh] = block
        self._height_to_hash[block.height] = hh
+        self.mmr_manager.add_block_to_mmr(block.header_hash, block.prev_hash, block.height)


one thing we need to be careful with here is the consistency with the blockchain database. Right now, the coin store, the full blocks, peak and height-to-hash all needs to stay in sync, even with a power outage or kernel panic. The MMR manager is similar to the coin store in that it's tied to a specific peak, with a specific chain.

If the database update fails, we may need to roll back this. Or perhaps this only happens after the DB update succeeds.

True we need to add checks when starting up that the peak in the mmr and the peak in the block store are the same

chia/util/merkle_tree.py

chia/consensus/challenge_tree.py

cursor · 2025-12-17T01:47:28Z

chia/consensus/block_header_validation.py

+        prev_b_hash=header_block.prev_header_hash,
+        sp_index=header_block.reward_chain_block.signage_point_index,
+        first_in_sub_slot=len(header_block.finished_sub_slots) > 0,
+    )


Bug: Duplicate calculation of pre_sp_tx_height in validation

The function pre_sp_tx_block_height() is called twice with identical parameters within validate_finished_header_block() - once at lines 119-125 and again at lines 1076-1082. Since this function traverses block records to find the pre-signage-point transaction block height, calling it twice is wasteful. The variable pre_sp_tx_height from the first calculation at line 119 is already in scope and can be reused at line 1076.

Additional Locations (1)

chia/consensus/block_header_validation.py#L118-L125

arvidn · 2025-12-17T10:09:33Z

chia/consensus/blockchain_mmr.py

+
+    _mmr: MerkleMountainRange
+
+    _checkpoints: dict[int, MerkleMountainRange]  # height -> MMR snapshot


actually, my understanding of MMR is that you append new items. So rolling back should just be a matter of popping the most recent items. I don't think you need to store a history of previous MMRs.

In fact, you could even provide a view onto an MMR for an earlier peak, by just pretending the most recent items aren't there.

arvidn · 2025-12-17T10:24:28Z

chia/consensus/blockchain_mmr.py

+                self._last_header_hash = block_record.header_hash
+                self._last_height = uint32(height)
+            except Exception as e:
+                log.warning(f"Could not find block at height {height} during MMR rollback: {e}")


Suggested change

log.warning(f"Could not find block at height {height} during MMR rollback: {e}")

log.exception(f"Could not find block at height {height} during MMR rollback")

Feel free to resolve this comment if you prefer warning().

this should never happen so your right, we should log.exception

arvidn · 2025-12-17T10:26:05Z

chia/consensus/blockchain_mmr.py

+    _mmr: MerkleMountainRange = field(default_factory=MerkleMountainRange)
+    _last_header_hash: bytes32 | None = None
+    _last_height: uint32 | None = None
+    _checkpoints: dict[uint32, MerkleMountainRange] = field(default_factory=dict)


I don't actually think you need the checkpoints. Since new items are appended to an MMR (and new parents are created on-demand), you can pretty easily just pop items as well, to restore to an earlier state.

arvidn · 2025-12-17T10:27:06Z

chia/consensus/mmr.py

+    """
+
+    nodes: list[bytes32]
+    size: uint32


if this isn't just len(nodes), I think it warrants a comment.
I suppose this is the number of leaves in the MMR. That should still be computable from len(nodes), I would expect.

yes, ill add a comment, this is the number of blocks we aggregated

arvidn · 2025-12-17T10:29:57Z

chia/consensus/mmr.py

+    Flat MMR implementation.
+    """
+
+    nodes: list[bytes32]


I think it would be an important performance improvement to make this a bytearray instead. But these kinds of optimizations can wait.

arvidn · 2025-12-17T11:19:53Z

chia/consensus/mmr.py

+    return peaks
+
+
+def leaf_index_to_pos(leaf_index: int) -> int:


I think this function should have a unit test with the interesting edge cases, and a human-readable illustration of the shape of the MMR and what the expected result is, for the various cases.

I imagine we would cover a single complete tree, two peaks and 3 peaks.

arvidn · 2025-12-17T11:30:59Z

chia/_tests/core/consensus/test_mmr.py

+    assert mmr.get_root() == expected_root
+
+
+def test_flat_mmr_peak_positions() -> None:


I think it would really help this test to have an ASCII-art illustration of the MMR.

Something like this one:

Height 3 14 / \ / \ / \ / \ 2 6 13 / / \ 1 2 9 12 17 / \ / / \ / 0 0 1 7 10 11 15 18

I also think it would be helpful to remind the reader of what the indices refer to. The input index is the leaf, not MMR index, right?. The return values are indices if the peaks, so not given numbers on the illustration above.

arvidn · 2025-12-17T11:35:06Z

chia/_tests/core/consensus/test_mmr.py

+@pytest.mark.skip("Height calculation not needed - we store heights directly")
+def test_flat_mmr_height_calculation() -> None:
+    """Test height calculation from position"""
+    assert get_height(0) == 0  # Leaf


I think this test also would benefit of an ASCII-art illustration of the tree

arvidn · 2025-12-17T11:43:58Z

chia/_tests/core/consensus/test_mmr.py

+def random_bytes32() -> bytes32:
+    return bytes32(os.urandom(32))


you can also use bytes32.random(), and we have a test fixture that provides a random.Random context, which give you deterministic values

arvidn · 2025-12-17T11:45:29Z

chia/_tests/core/consensus/test_mmr.py

+    root2 = mmr2.get_root()
+    for i in range(10):
+        assert mmr.nodes[i] == mmr2.nodes[i]
+    assert root2 == root


in addition to these round-trip tests for proofs, I think you should have a few concrete test cases that check that the resulting proof has exactly the nodes we expect it to have. Just to make sure the result is valid, not just that it validates in the validation function

cursor · 2025-12-17T21:19:24Z

chia/consensus/blockchain_mmr.py

+            block = blocks.height_to_block_record(uint32(height))
+            mmr.append(block.header_hash)
+
+        return mmr.get_root()


Bug: MMR root rebuild requires uncached history

BlockchainMMRManager._build_mmr_to_block() rebuilds the MMR by iterating from height 0 and calling blocks.height_to_block_record(). In Blockchain, height_to_block_record() only works for in-memory cached BlockRecords, so after startup (only last cache window loaded) or after cache eviction this can raise or produce incorrect header_mmr_root, breaking post-fork block creation/validation.

Additional Locations (1)

chia/consensus/blockchain.py#L171-L186

cursor · 2025-12-17T21:19:24Z

chia/consensus/blockchain_mmr.py

+            log.exception(f"Could not find block at height {target_height} during MMR rollback: {e}")
+
+        self._last_header_hash = target_block.header_hash
+        self._last_height = uint32(target_height)


Bug: MMR rollback can crash on genesis

rollback_to_height() catches failures from blocks.height_to_block_record() but still dereferences target_block, which can be unassigned, and it doesn’t handle target_height < 0. Callers pass height - 1 (e.g. removing height 0 gives -1), which can trigger an exception and leave the MMR manager in a bad state.

Additional Locations (1)

chia/consensus/blockchain.py#L998-L1005

almogdepaz added 6 commits December 10, 2025 17:24

add mmr manager

50d90f6

sub epoch summary challenge tree

7c67160

validation/block creation

abb78db

Timelord/old weight proofs

cbaee62

move merkle ro util

f97d12d

handle tests

e492a3c

almogdepaz added full_node Added Required label for PR that categorizes merge commit message as "Added" for changelog labels Dec 10, 2025

almogdepaz added 3 commits December 15, 2025 12:35

move HF check out of SE creation

911ba4b

remove type check skip

b88e9a4

missing test param

8c2b660

almogdepaz marked this pull request as ready for review December 15, 2025 11:02

almogdepaz requested a review from a team as a code owner December 15, 2025 11:02

cursor bot reviewed Dec 15, 2025

View reviewed changes

chia/consensus/augmented_chain.py Outdated Show resolved Hide resolved

chia/consensus/blockchain_mmr.py Show resolved Hide resolved

chia/consensus/challenge_tree.py Outdated Show resolved Hide resolved

github-actions bot added the merge_conflict Branch has conflicts that prevent merge to main label Dec 15, 2025

fix off by one and SE bounderies

0b3fc97

cursor bot reviewed Dec 16, 2025

View reviewed changes

chia/consensus/blockchain_mmr.py Outdated Show resolved Hide resolved

arvidn reviewed Dec 16, 2025

View reviewed changes

better handle ses bounderies

c5701d4

cursor bot reviewed Dec 16, 2025

View reviewed changes

chia/consensus/challenge_tree.py Show resolved Hide resolved

almogdepaz added 2 commits December 16, 2025 20:44

use compute_merkle_set_root, revert MerkleTree mv

bdeef45

pr comments

0af6190

almogdepaz force-pushed the WP_v2 branch from 5752b75 to 0af6190 Compare December 16, 2025 23:53

Merge branch 'main' into WP_v2

3fb597b

github-actions bot removed the merge_conflict Branch has conflicts that prevent merge to main label Dec 16, 2025

cursor bot reviewed Dec 17, 2025

View reviewed changes

chia/consensus/challenge_tree.py Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

add test, minor fixes

80210ca

almogdepaz force-pushed the WP_v2 branch from a20fbfa to 80210ca Compare December 17, 2025 01:36

cursor bot reviewed Dec 17, 2025

View reviewed changes

arvidn reviewed Dec 17, 2025

View reviewed changes

mmr rolleback no checkpoints, some pr comments

87ebec0

cursor bot reviewed Dec 17, 2025

View reviewed changes


		_mmr: MerkleMountainRange

		_checkpoints: dict[int, MerkleMountainRange] # height -> MMR snapshot

	_checkpoints: dict[int, MerkleMountainRange] # height -> MMR snapshot
	_checkpoints: dict[uint32, MerkleMountainRange] # height -> MMR snapshot

	log.warning(f"Could not find block at height {height} during MMR rollback: {e}")
	log.exception(f"Could not find block at height {height} during MMR rollback")

		assert mmr.get_root() == expected_root


		def test_flat_mmr_peak_positions() -> None:

		def random_bytes32() -> bytes32:
		return bytes32(os.urandom(32))

Wp v2 #20340

Are you sure you want to change the base?

Wp v2 #20340

Conversation

almogdepaz commented Dec 10, 2025 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose:

Current Behavior:

New Behavior:

Testing Notes:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

arvidn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

almogdepaz Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

almogdepaz Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

almogdepaz Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

cursor bot Dec 17, 2025

Choose a reason for hiding this comment

Bug: Duplicate calculation of pre_sp_tx_height in validation

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

almogdepaz commented Dec 10, 2025 •

edited by cursor bot

Loading

almogdepaz Dec 16, 2025 •

edited

Loading

almogdepaz Dec 16, 2025 •

edited

Loading

almogdepaz Dec 16, 2025 •

edited

Loading

Bug: Duplicate calculation of `pre_sp_tx_height` in validation