Implement Keccak-based MMR frontier #2245

Fumuran · 2026-01-08T22:09:21Z

This PR implements a new Keccak-based MMR frontier collection. See the details in the documentation in the related masm file.

As a related change the structure of the miden-agglayer crate was slightly updated: bridge and newly created collection modules were moved to the lib folder. That was done to make the creation of the agglayer library a bit more simple: that way we can compile the whole lib folder.

Closes: #2105

mmagician

LGTM, though it's not a full review yet.

Thanks for outlining the details of the computation so well ✅ very helpful

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm

mmagician · 2026-01-13T14:50:22Z

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm

+# this implementation this maximum height is set to 32), and the leaves equal to the MMR frontier
+# leaves or a zero hash (Keccak256::hash(&[0u8; 32])).


IIUC, the leaves of the SMT can't be represented by the leaves of the MMR frontier, because the frontier doesn't have all the leaves

I was struggling with how can I explain that, but the idea is that we need to compute the root of the SMT, which has all the leaves from the MMR and all other missing leaves are zeros. For example, if we have an MMR with leaves A, B, C, the leaves of the SMT which root we want to compute will look like A, B, C, ZERO_HASH_3, ..., ZERO_HASH_N.

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm

crates/miden-testing/tests/agglayer/mmr_frontier.rs

PhilippGackstatter

Looks great! My assumption for the PR review is the algorithm described in #2105 (comment).

I left mostly small comments. The only important question I have is about whether leaf values should be hashed or not.

PhilippGackstatter · 2026-01-14T15:50:45Z

crates/miden-agglayer/asm/bridge/mmr_frontier32_keccak.masm

+# relevant frontier values in the frontier array for the current height. For example, if we have 10
+# leaves (1010 in binary representation), relevant frontier values will be stored at frontier[1] and
+# frontier[3].


I think it would be good to spell out what a "frontier value" is. For height 0 it is the leaf (or maybe the hash of the leaf - see other comment) while for height > 0, it is the subtree root. If we end up hashing the leaves, I think "frontier hash" would be a more precise name.

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm

PhilippGackstatter · 2026-01-14T16:46:50Z

crates/miden-testing/tests/agglayer/mmr_frontier.rs

+    // Push the zero of height 0 to the zeros vec. This is done separately because it requires
+    // `Keccak256::hash` instead of `Keccak256::merge`
+    zeros_by_height.push(Keccak256::hash(&[0u8; 32]));


If I understand correctly, the canonical leaf with value 0 has the value hash(0) in the tree? If so, shouldn't we be hashing leaves as well in append_and_update_frontier? Otherwise, it seems like adding a 0 into an empty frontier via append_and_update_frontier would result in a different tree root than the canonical tree root.

I would also add a test for adding (many) zeros to a frontier and make sure it matches the canonical subtree roots.

I think the canonical zero at height 0 is actually just a "pure" 0u32 rather than hash(0u32): https://github.com/agglayer/agglayer-contracts/blob/e468f9b0967334403069aa650d9f1164b1731ebb/contracts/v2/lib/DepositContractBase.sol#L43

and when we add the leaf, it's already hashed, so no need to hash the value again, see https://github.com/agglayer/agglayer-contracts/blob/e468f9b0967334403069aa650d9f1164b1731ebb/contracts/v2/lib/DepositContractBase.sol#L66

crates/miden-testing/tests/agglayer/mmr_frontier.rs

PhilippGackstatter · 2026-01-14T16:50:46Z

crates/miden-testing/tests/agglayer/mmr_frontier.rs

+    format!(
+        r#"
+            # load the provided leaf onto the stack
+            push.{LEAF}


Suggested change

push.{LEAF}

push.[{LEAF}]

Nit: Might be worth adapting to avoid a breakage once 0xMiden/miden-vm#2572 is implemented. I think this requires dropping the rev in the stringify function, but please double check.

AFAIK, we can support square brackets only for the word literals, but the LEAF here is a double word value, so I'm not sure that we can use them directly. Though it is a good idea to adapt the code preliminarily, so I just need to update the way LEAF is generated.

Edit: it seems like right now push.[a, b, c, d] pushes values on the stack in the same order as push.a.b.c.d, so now it works the same way, but after mentioned PR will be merged we will have to just un-reverse the resulting string.

PhilippGackstatter · 2026-01-14T16:52:59Z

crates/miden-testing/tests/agglayer/mmr_frontier.rs

+    // create a leaf from a random hex
+    let first_leaf = Keccak256Digest::try_from(
+        "0x110527e2a134fcb367f3bc770acc0d75b9c47bb4c5a78f0de02c80143340df62",
+    )
+    .unwrap();
+    let first_root = mmr_frontier.append_and_update_frontier(first_leaf);
+    let first_leaf_count = mmr_frontier.leaves_num;


I think ideally this would test many more rounds (16 or 32 or something like that), but at least 4, so we also test the case when the number of MMR peaks is "shrinking". With three leaves, the number only grows.

So could we rewrite this to a test loop that generates a couple of frontier updates?

Ideally we can create a small reusable test setup where we can run the MASM code and then write two concrete tests where one is similar to the current one, with more values, and the other one tests that adding a bunch of zeros to the frontier results in the expected canonical zero subtree roots.

Assertion of all empty subtree roots will require to add 2^32 - 1 zero leaves to the MMR (that way we can compare the frontier with the zero hashes), but it will require too many calculations.
Instead I implemented the test in which I add 32 zero leaves and check that after each insertion the MMR root remains to be equal to the empty MMR root. I don't check subtree roots directly, but probably checking the MMR root should be sufficient?

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm

partylikeits1983 · 2026-01-14T18:40:46Z

crates/miden-agglayer/asm/bridge/mmr_frontier32_keccak.masm

+    # => [num_leaves_ptr, num_leaves+1, num_leaves, mmr_frontier_ptr]
+
+    mem_store
+    # => [num_leaves, mmr_frontier_ptr]


Is the stack comment here correct?

Shouldn't it be:

# => [num_leaves+1, num_leaves, mmr_frontier_ptr]

It seems correct, since the mem_store instruction removes both the address and the value it stores from the stack: docs.

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm

PhilippGackstatter

Looks good to me!

PhilippGackstatter · 2026-01-16T14:15:02Z

crates/miden-agglayer/asm/bridge/mmr_frontier32_keccak.masm

+    u32assert.err=ERR_MMR_FRONTIER_LEAVES_NUM_EXCEED_LIMIT
+    # => [num_leaves, mmr_frontier_ptr]


Nit: May be worth adding to the Panics if section of the procedure.

PhilippGackstatter · 2026-01-16T14:22:59Z

crates/miden-agglayer/asm/bridge/mmr_frontier32_keccak.masm

+    # make sure that the MMR is not full yet and we still can store the new leaf
+    #
+    # MMR is full when the number of leaves is equal to the 2^TREE_HEIGHT, and in case of the
+    # current implementation this number equals to 2^32. This value doesn't fit into the u32
+    # anymore, so we can use only the `u32assert` instruction to assert that the MMR is not full.
+    u32assert.err=ERR_MMR_FRONTIER_LEAVES_NUM_EXCEED_LIMIT
+    # => [num_leaves, mmr_frontier_ptr]


My other comment was incorrect re: max number of leaves. Due to TREE_HEIGHT = 32, the max number of leaves is u32::MAX = 2^32 - 1. So, the last time we can call the procedure is when the number of leaves is 2^32 - 2, because we will then add the leaf and increase the number of leaves to 2^32 - 1. So, I think we have to u32assert and u32lte.0xfffffffe assert. If this is correct, then the error message should also be updated to note the correct number of max leaves with which this procedure can be called.

Due to TREE_HEIGHT = 32, the max number of leaves is u32::MAX = 2^32 - 1

Good catch - this is indeed the case, as per the Solidity counterpart.

Do we still need u32assert though? If we have u32lte.0xfffffffe assert, we will never get past the point of u32::MAX.

updated in fix: update max leaves constants and comments, lmk if this works?

PhilippGackstatter · 2026-01-16T14:31:50Z

crates/miden-testing/tests/agglayer/mmr_frontier.rs

+    let mut source = "use miden::agglayer::collections::mmr_frontier32_keccak begin".to_string();
+
+    for round in 1..=32 {
+        // check that pushing the zero leaves into the MMR doesn't change its root
+        source.push_str(&leaf_assertion_code(zero_leaf, empty_mmr_root, round));
+    }
+
+    source.push_str("end");


Nit: mmr_frontier_ptr is never mentioned or explicitly initialized anywhere, and while that is technically correct due to "empty memory being a valid mmr frontier", this is very subtle and could make it harder to understand the test in the future.

mmagician

The code looks great, well documented and nicely structured.
One thing that is missing here are the Solidity compatibility tests. So I quickly built few test vectors locally with foundry and verified that things check out 1-to-1 ✅

I'll make a separate PR against the agglayer branch that adds the Solidity-compat tests. I think a small foundry project will be a good addition under the miden-agglayer crate. It will be useful not just for these MMR frontier tests, but also for addLeafValue that @partylikeits1983 was working on and verifyLeaf that I added.

This one is ready to merge from my perspective.
We could still iron out the terminology (MMR/frontier/SMT) but let's do that in a separate PR, I'll open a tracking issue.

crates/miden-testing/tests/agglayer/mmr_frontier.rs

mmagician · 2026-01-19T10:33:43Z

crates/miden-testing/tests/agglayer/mmr_frontier.rs

+    let mut source = "use miden::agglayer::collections::mmr_frontier32_keccak begin".to_string();
+
+    for round in 0..32 {
+        // construct the leaf from the hex representation of the round number
+        let leaf = Keccak256Digest::try_from(format!("{:#066x}", round).as_str()).unwrap();
+        let root = mmr_frontier.append_and_update_frontier(leaf);
+        let num_leaves = mmr_frontier.num_leaves;
+
+        source.push_str(&leaf_assertion_code(leaf, root, num_leaves));
+    }


Very neat ✅

in leaf assertion tests

mmagician · 2026-01-19T14:35:50Z

bridge and newly created collection modules were moved to the lib folder.

I think this was an overall improvement, but since we'll be restructuring the agglayer directories further as per #2294, let's tackle it all in one go when the time comes. For now I've reverted the directory structure changes as part of chore: revert to using old dir structure

Fumuran mentioned this pull request Jan 8, 2026

Implement Keccak-based MMR frontier #2202

Closed

3 tasks

feat: impl fist frontier version, add (yet buggy) test

fb09b70

Fumuran force-pushed the andrew-keccak-based-mmr-frontier branch from 7b40afe to fb09b70 Compare January 9, 2026 21:12

Fumuran added 3 commits January 11, 2026 22:28

test: fix the test, fix the bug in algorithm

1d71ed5

chore: update changelog

645b3ad

docs: add docs for the MMR frontier in the masm file

13d756e

Fumuran marked this pull request as ready for review January 11, 2026 21:16

Fumuran requested review from PhilippGackstatter and mmagician January 11, 2026 21:16

bobbinth requested a review from partylikeits1983 January 12, 2026 18:21

mmagician reviewed Jan 13, 2026

View reviewed changes

Fumuran added 2 commits January 13, 2026 23:59

refactor: update the doc comments, slightly update code

dec8cfa

Merge branch 'next' into andrew-keccak-based-mmr-frontier

5820f55

PhilippGackstatter reviewed Jan 14, 2026

View reviewed changes

partylikeits1983 reviewed Jan 14, 2026

View reviewed changes

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm Outdated Show resolved Hide resolved

partylikeits1983 reviewed Jan 14, 2026

View reviewed changes

crates/miden-agglayer/asm/lib/collections/mmr_frontier32_keccak.masm Outdated Show resolved Hide resolved

partylikeits1983 approved these changes Jan 14, 2026

View reviewed changes

Fumuran added 4 commits January 15, 2026 17:36

refactor: update docs and comments, add overflow check, update test

32d7b4c

test: add more leaves

1252871

test: add zero root test

ee5564f

Merge branch 'agglayer' into andrew-keccak-based-mmr-frontier

fe7bc87

Base automatically changed from agglayer to next January 16, 2026 08:48

PhilippGackstatter approved these changes Jan 16, 2026

View reviewed changes

mmagician approved these changes Jan 19, 2026

View reviewed changes

mmagician mentioned this pull request Jan 19, 2026

Ensure consistent naming on MMR frontier code #2308

Open

chore: rename root -> expected_root

fc86060

in leaf assertion tests

mmagician changed the base branch from next to agglayer January 19, 2026 11:06

mmagician added 2 commits January 19, 2026 11:47

Merge branch 'agglayer' into andrew-keccak-based-mmr-frontier

53e1788

chore: lint

f015d31

chore: revert to using old dir structure

089eafd

mmagician mentioned this pull request Jan 19, 2026

feat(AggLayer): Solidity compatibility tests for MMR frontier code #2312

Open

mmagician added 2 commits January 19, 2026 16:30

fix: update max leaves constants and comments

9528a3a

chore: regen errors file

6b129b0

		# this implementation this maximum height is set to 32), and the leaves equal to the MMR frontier
		# leaves or a zero hash (Keccak256::hash(&[0u8; 32])).

		u32assert.err=ERR_MMR_FRONTIER_LEAVES_NUM_EXCEED_LIMIT
		# => [num_leaves, mmr_frontier_ptr]

Implement Keccak-based MMR frontier #2245

Are you sure you want to change the base?

Implement Keccak-based MMR frontier #2245

Uh oh!

Conversation

Fumuran commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

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!

PhilippGackstatter left a comment

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!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fumuran Jan 15, 2026 • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

PhilippGackstatter 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!

mmagician left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mmagician commented Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Fumuran commented Jan 8, 2026 •

edited

Loading

Fumuran Jan 15, 2026 •

edited

Loading