Compute claimed global index chain hash by Fumuran · Pull Request #2516 · 0xMiden/protocol

Fumuran · 2026-02-25T17:30:15Z

This PR:

Modifies the bridge_in.masm file to add the CGI chain hash computation in the verify_leaf_bridge procedure. For now it is more like in a "template" state, since for now it is impossible to compute this value: to do so we need to read the previous CGI hash value, but currently it is not store anywhere (yet).
Adds a Solidity contract to generate the data required for the CGI chain hash computation: leaf value, global index, previous (old) CGI chain hash and a new (expected) CGI chain hash. This contract was generated with AI help, but in my layman's opinion this code works correctly, though I still have some questions about it. I beg for the masters of the Solidity @partylikeits1983 and @mmagician to check its correctness. It should be fine, since the test is passing, but nevertheless.
Adds a new test to check that the CGI chain hash value computed in the verify_leaf_bridge procedure (or, to be precise, the value, computed using the same algorithm: I can't reuse the compute_cgi_hash_chain procedure itself since it is private and it uses some memory values which are not available during the test) is correct.

We still need to store the computed value in memory, but this approach is blocked until we change the way SWAP note it consumed. Storing the CGI hash will be done in a separate PR.

Part of #2386

partylikeits1983 · 2026-03-02T12:12:28Z

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

+    # save the leaf value to the local memory to reuse it during the computation of the CGI chain
+    # hash 


Nit:

Suggested change

# save the leaf value to the local memory to reuse it during the computation of the CGI chain

# hash

# save the leaf value to the local memory to reuse it during the computation

# of the CGI hash chain

partylikeits1983 · 2026-03-02T12:15:32Z

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

+    # TODO: once the CLAIM note will be consumed against the bridge contract (and not FPIed against
+    # it), store the computed value. Notice that this value is used during the computation of the
+    # next CGI chain hash, so the `compute_cgi_hash_chain` procedure should be updated.
+    dropw dropw


This functionality is available here: #2528

Probably best to add the functionality which stores the CGI hash chain in a separate PR.

I'll need to update this procedure to test it anyway, so it won't be a big deal to store the CGI hash chain into the memory. But I'm not sure where actually to store it.

It should be stored somewhere in the Bridge account storage, right? But I can't see a storage slot for that. Or is it not created yet, and I need to add it to the From<AggLayerBridge> for AccountComponent implementation?

a new slot needs to be created to hold it

partylikeits1983

Looks great!

I just think we should reuse the pure function test helpers: test_utils::execute_program_with_default_host. I think using execute_program_with_default_host will clean up the test, and also make it possible to not have to have a "copy" of the compute_cgi_hash_chain procedure in the test.

Also lets update references to "chain hash" to "hash chain". Otherwise everything looks great!

partylikeits1983 · 2026-03-02T12:22:41Z

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

+        proc compute_cgi_hash_chain_copy
+            # load the global index onto the stack
+            push.GLOBAL_INDEX_PTR exec.utils::mem_load_double_word
+            # => [GLOBAL_INDEX[8], LEAF_VALUE[8]]
+
+            exec.keccak256::merge
+            # => [Keccak256(GLOBAL_INDEX, LEAF_VALUE), pad(8)]
+
+            # load the old CGI chain hash
+            push.OLD_CGI_CHAIN_HASH_PTR exec.utils::mem_load_double_word
+            # => [OLD_CGI_CHAIN_HASH[8], Keccak256(GLOBAL_INDEX, LEAF_VALUE), pad(8)]
+
+            # compute the new CGI chain hash
+            exec.keccak256::merge
+            # => [NEW_CGI_CHAIN_HASH[8], pad(8)]
+        end


I'd actually call the real procedure, not a copy of it. I think the asset conversion tests are a good example which show how to do this so that you don't have to run a full transaction.

partylikeits1983 · 2026-03-02T12:24:54Z

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

+    "#
+    );
+
+    let tx_script = CodeBuilder::new()


I think it would be cleaner if we had the assert_eq! in Rust as opposed to running the test logic in masm. I think it reads easier if you call the masm procedure, get the returned values off the stack in Rust, then check the two Rust variable values are equal. As noted above, we have this testing functions available in the agglayer crate, check out the asset_conversion.rs file to see how they are used

I recall that we had a discussion about whether we should use asserts in the Rust code vs MASM code, but IIRC we agreed to follow @PhilippGackstatter 's proposal to use asserts in the MASM code, but honestly I don't remember why. Probably to make the code self-contained and, which is more important, to check the error messages correctness in case the testing code uses some of the internal kernel/protocol procedures.

I think we discussed it at some point, but don't recall the outcome. I think there are pros and cons. Multiple asserts in MASM read relatively nicely and can be done sequentially, as done in this test.

On the other hand, asserts in Rust generally result in a better error when they fail. The downside is if you need to assert the correctness of multiple values, you sometimes need to write extra MASM to arrange the output stack appropriately and then also extract multiple stack values in the right order from the ExecutionOutput, which is messy and incurs unnecessary mental load to review its correctness.

So, I'd use whichever approach results in a more readable and easily maintainble test and one that provides good errors when it fails.

mmagician · 2026-03-02T23:43:03Z

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

+    # TODO: once the CLAIM note will be consumed against the bridge contract (and not FPIed against
+    # it), store the computed value. Notice that this value is used during the computation of the
+    # next CGI chain hash, so the `compute_cgi_hash_chain` procedure should be updated.
+    dropw dropw


a new slot needs to be created to hold it

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

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

…ure in test

…hain-hash

mmagician

LGTM, thanks, with small nits only.

Once #2528 is ready and merged into agglayer, we can merge this one too.

Currently, the test here is a unit test, but I would still like to see an integration test (insofar as we can call them integration tests in protocol repo) that checks the expected state of the CGI storage slots after a CLAIM note has been processed. But this could be done either as a follow-up if you want to get unblocked, or as part of this PR - whichever is easier (but it should be done before we release another alpha version)

mmagician · 2026-03-04T17:51:55Z

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

+    # load the old CGI chain hash
+    push.CGI_CHAIN_HASH_HI_SLOT_NAME[0..2]
+    exec.active_account::get_item
+    # => [OLD_CGI_CHAIN_HASH_HI, pad(16)]
+
+    push.CGI_CHAIN_HASH_LO_SLOT_NAME[0..2]
+    exec.active_account::get_item
+    # => [OLD_CGI_CHAIN_HASH[8], pad(16)]
+
+    # load the pointer to the leaf value onto the stack
+    locaddr.0
+    # => [leaf_value_ptr, OLD_CGI_CHAIN_HASH[8], pad(16)]

    # compute the claimed global index chain hash
    exec.compute_cgi_hash_chain
-    # => [NEW_CGI_CHAIN_HASH[8], pad(8)]
+    # => [NEW_CGI_CHAIN_HASH[8], pad(16)]

    # store the computed CGI chain hash
-    #
-    # TODO: once the CLAIM note will be consumed against the bridge contract (and not FPIed against
-    # it), store the computed value. Notice that this value is used during the computation of the
-    # next CGI chain hash, so the `compute_cgi_hash_chain` procedure should be updated.
-    dropw dropw
+    exec.store_cgi_hash_chain


this works, but it would be neater to encapsulate this logic into:

proc update_cgi_hash_chain # load # compute new # store the updated chain hash end

mmagician · 2026-03-04T18:08:04Z

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

+            # push the expected hash onto the stack
+            push.{expected_cgi_hash_hi} exec.word::reverse
+            push.{expected_cgi_hash_lo} exec.word::reverse
+            # => [EXPECTED_CGI_HASH[8]]


nit: I would only push the expected values onto the stack after we've computed the computed values.

mmagician · 2026-03-04T18:10:04Z

.gitmodules

+[submodule "crates/miden-agglayer/solidity-compat/lib/openzeppelin-contracts"]
+	path = crates/miden-agglayer/solidity-compat/lib/openzeppelin-contracts
+	url = https://github.com/OpenZeppelin/openzeppelin-contracts.git
+	branch = release-v5.0
+[submodule "crates/miden-agglayer/solidity-compat/lib/openzeppelin-contracts-upgradeable5"]
+	path = crates/miden-agglayer/solidity-compat/lib/openzeppelin-contracts-upgradeable5
+	url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable.git
+	branch = release-v5.0


nit: it seems we have a v4 and v5 of the same submodule, is it possible to just have the latest version?

Fumuran added the no changelog This PR does not require an entry in the `CHANGELOG.md` file label Feb 25, 2026

Fumuran marked this pull request as ready for review February 25, 2026 17:50

Fumuran requested review from mmagician and partylikeits1983 February 25, 2026 17:50

mmagician added agglayer PRs or issues related to AggLayer bridging integration pr-from-maintainers PRs that come from internal contributors or integration partners. They should be given priority labels Mar 1, 2026

partylikeits1983 reviewed Mar 2, 2026

View reviewed changes

partylikeits1983 requested changes Mar 2, 2026

View reviewed changes

Fumuran added 5 commits March 2, 2026 18:16

feat: add CGI chain hash computation

a0d43bf

test: [WiP-1] first iteration, not building

1e5f5aa

test: [WiP-2] building state

8c3dcdf

test: updated script, failing

5f707de

test: update script (passing)

558257d

Fumuran force-pushed the andrew-compute-cgi-chain-hash branch from 50ab9b1 to 558257d Compare March 2, 2026 15:17

Fumuran changed the base branch from agglayer to ajl-reorient-claim-note-flow March 2, 2026 15:17

mmagician reviewed Mar 3, 2026

View reviewed changes

Fumuran added 2 commits March 4, 2026 15:48

refactor: create ctorage slots for CGI chain hash, use actiual proced…

ef477d5

…ure in test

Merge branch 'ajl-reorient-claim-note-flow' into andrew-compute-cgi-c…

2d5149b

…hain-hash

Fumuran mentioned this pull request Mar 4, 2026

Update hash aliaces #2542

Open

mmagician approved these changes Mar 4, 2026

View reviewed changes

		# save the leaf value to the local memory to reuse it during the computation of the CGI chain
		# hash

Conversation

Fumuran commented Feb 25, 2026

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!

partylikeits1983 left a comment • 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!

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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

partylikeits1983 left a comment •

edited

Loading