feat(AggLayer): Store hash(GER) in the bridge storage and implement assert_valid_ger#2388
feat(AggLayer): Store hash(GER) in the bridge storage and implement assert_valid_ger#2388mmagician merged 24 commits intoagglayer-newfrom
hash(GER) in the bridge storage and implement assert_valid_ger#2388Conversation
This reverts commit 61852a9.
| ger_lower.reverse(); | ||
| ger_upper.reverse(); |
There was a problem hiding this comment.
It seems like to use Hasher::merge(...) we need to reverse the inputs - is this expected?
cc @adr1anh
There was a problem hiding this comment.
Pull request overview
This PR refactors the Global Exit Root (GER) storage mechanism in the AggLayer bridge to use a map-based approach instead of storing GERs in separate value slots. It stores hash(GER) as the map key with a flag value to indicate the GER is known, and implements the assert_valid_ger procedure to verify GERs against storage.
Changes:
- Changed GER storage from two value slots (ger_upper, ger_lower) to a single map slot that stores
hash(GER) -> flag - Implemented
assert_valid_gerprocedure that checks if a GER exists in storage by verifying the flag value - Updated tests to use Solidity-generated test vectors for better cross-platform verification
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/miden-agglayer/asm/bridge/bridge_in.masm | Implements update_ger to store GER hash in map and assert_valid_ger to verify GER exists |
| crates/miden-agglayer/asm/note_scripts/UPDATE_GER.masm | Changes memory loading from BE to LE for GER data (part of byte conversion fix) |
| crates/miden-agglayer/src/lib.rs | Updates bridge account builder to use map storage slot instead of two value slots |
| crates/miden-agglayer/src/errors/agglayer.rs | Adds ERR_GER_NOT_FOUND error message |
| crates/miden-testing/tests/agglayer/update_ger.rs | Refactors tests to use Solidity test vectors and verifies GER hash storage |
| crates/miden-testing/tests/agglayer/test_utils.rs | Updates test inputs to include global_exit_root with Solidity test vector values |
| crates/miden-testing/tests/agglayer/bridge_in.rs | Updates claim test to execute update_ger before claim to populate GER storage |
| crates/miden-agglayer/solidity-compat/test/ExitRoots.t.sol | Adds Solidity test to generate exit root test vectors from mainnet transactions |
| crates/miden-agglayer/solidity-compat/test-vectors/exit_roots.json | Contains test vectors generated by Solidity for GER computation verification |
| CHANGELOG.md | Documents the storage change and assert_valid_ger implementation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
partylikeits1983
left a comment
There was a problem hiding this comment.
Tests and changes look good to me. I just think we should refactor this to use keccak256 as opposed to using rpo256 since we will eventually have to change this anyways later once RPO is phased out. Also it would be easier for gateway to use. But then you'd need to employ the Key -> [Word, Word] mapping logic you implemented in a different PR.
partylikeits1983
left a comment
There was a problem hiding this comment.
Looks great! I think once there is a definitive answer as to why the inputs need to be reversed when using Hasher::merge(), we should add a comment why we need to call .reverse() would be good.
Otherwise everything looks good, your answer to my question regarding rpo256 usage makes sense
… `assert_valid_ger` (0xMiden#2388) * chore: unit test for compute_ger * fix: use LE byte<>felt conversion * chore: inline bytes32_to_felts into callers * fixup! fix: use LE byte<>felt conversion * Update crates/miden-testing/tests/agglayer/update_ger.rs * chore: unify underlying repr, use type aliases * feat: store GER hash in a map * feat: check GER in storage * chore: rename to GER_KNOWN_FLAG * Apply suggestions from code review * chore: reduce inline comments * changelog * chore: use constants on Felt * fix: reverse felt order within word after hashing * chore: generate GER test vectors * chore: integrate GER vectors to compute_ger test * chore: add expected GER in bridge-in test * Revert "fix: reverse felt order within word after hashing" This reverts commit 61852a9. * fix: reverse felt order before Hasher::merge * Update crates/miden-agglayer/solidity-compat/test/ExitRoots.t.sol * chore: add comments re: rpo256:merge stack ordering
Changes:
hash(GER) -> flagin a map slot, instead of two value slotsassert_valid_gerwhich checks if the flag == 1This PR is incremental on top of #2387
fixes #2341