feat(agglayer): store block number with each registered GER#2579
feat(agglayer): store block number with each registered GER#2579
Conversation
Store the transaction's reference block number alongside the GER flag in the bridge's GER map, changing the value from [1, 0, 0, 0] to [1, block_num, 0, 0]. This enables recovery scenarios that need to know when each GER was registered. The assert_valid_ger procedure now checks only the flag element instead of comparing the full word, since the block_num element is variable. Closes #2578 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address review feedback: reorder stack operations so the flag is pushed before getting the block number, eliminating the need for a swap. Also add changelog entry. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the AggLayer bridge GER registry to persist the transaction reference block number alongside the “known” flag, enabling recovery workflows to determine when a GER was registered.
Changes:
- Store
[GER_KNOWN_FLAG, block_num, 0, 0]as the GER map value (instead of a fixed sentinel). - Update
assert_valid_gerto validate only the flag element, sinceblock_numvaries. - Update tests and spec/docs to reflect the new GER value layout.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| crates/miden-testing/tests/agglayer/update_ger.rs | Updates storage assertion to include the stored reference block number. |
| crates/miden-agglayer/asm/agglayer/bridge/bridge_config.masm | Writes block_num into GER map values and adjusts GER validation logic accordingly. |
| crates/miden-agglayer/SPEC.md | Documents the updated GER map value encoding and meaning. |
| CHANGELOG.md | Notes the new feature in the upcoming release changelog. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mmagician
added
agglayer
mmagician
force-pushed
the
mmagician-claude/store-block-num-with-ger
branch
3 times, most recently
from
4355e05 to
7149c9e
Compare
Resolve conflicts in update_ger.rs: use the new AggLayerBridge helpers instead of direct storage access. Update is_ger_registered to check only the flag element (since the value is now [1, block_num, 0, 0]), and add get_ger_block_number to retrieve the registration block number. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mmagician
force-pushed
the
mmagician-claude/store-block-num-with-ger
branch
from
7149c9e to
0689bf8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ock number validation - Change `is_ger_registered` and `get_ger_block_number` to take `&Account` instead of `Account` for consistency with `read_local_exit_root` - Add `u32::try_from` range check on stored block number instead of silent `as u32` truncation - Add `InvalidBlockNumber` error variant to `AgglayerBridgeError` Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/miden-agglayer/src/bridge.rs
Outdated
| ) -> Result<bool, AgglayerBridgeError> { | ||
| // check that the provided account is a bridge account | ||
| Self::assert_bridge_account(&bridge_account)?; | ||
| Self::assert_bridge_account(bridge_account)?; |
There was a problem hiding this comment.
I would probably move this check into the get_ger_value method. That way we be able to reduce the amount of code (we can remove this check here and also from the get_ger_block_number), and also it seems to be a bit better form the encapsulation standpoint: an error can be raised during the storage access, so we're doing both check and access inside one procedure
There was a problem hiding this comment.
Move the bridge account validation into the shared get_ger_value helper so callers (is_ger_registered, get_ger_block_number) don't need to repeat it. Better encapsulation - validation lives with storage access. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
[1, 0, 0, 0]to[1, block_num, 0, 0]assert_valid_gerto check only the flag element (instead of full word comparison), since block_num is now variableTest plan
BUILD_GENERATED_FILES_IN_SRC=1 make test name=update_ger- all 3 tests passmake lint- cleanCloses #2578
🤖 Generated with Claude Code