feat: migrate miden-agglayer to VM 0.21 and crypto 0.22#2546
feat: migrate miden-agglayer to VM 0.21 and crypto 0.22#2546
Conversation
Re-enable the miden-agglayer crate in the workspace and apply all necessary API and convention changes from the VM 0.21 migration: Rust changes: - Remove FieldElement trait imports (removed from miden-core) - Felt::as_int() -> Felt::as_canonical_u64() - Program import moved to miden_core::program::Program - bytes_to_packed_u32_felts -> bytes_to_packed_u32_elements (miden_core::utils) - AccountId::try_from([Felt;2]) -> AccountId::try_from_elements(suffix, prefix) - Serializable/Deserializable from miden_assembly::serde (was ::utils) MASM changes: - rpo256 -> poseidon2 (hash function migration) - asset::mem_store/mem_load -> asset::store/load (procedure renames) - u64::overflowing_mul -> u64::widening_mul - u128_sub_no_underflow rewritten for LE convention (u64 procedures now use [lo, hi] input/output order) - verify_u128_to_native_amount_conversion updated for LE result order Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Re-enable agglayer tests in miden-testing and fix all compilation and runtime errors from the VM 0.21 migration: Test compilation fixes: - FieldElement import removal, as_int -> as_canonical_u64 - AuthScheme::Falcon512Rpo -> Falcon512Poseidon2 - AdviceInputs path: miden_processor::advice::AdviceInputs - FastProcessor::new_debug -> new().with_advice().with_debugging() - StackInputs::new(vec![]) -> new(&[]) - bytes_to_packed_u32_felts -> bytes_to_packed_u32_elements - AccountId::try_from -> try_from_elements MASM runtime fixes: - eth_address.masm: fix u32split LE output order in build_felt verification (movup.4 -> movup.3 for lo/hi comparison) - bridge_out.masm: fix create_burn_note note_idx corruption - loc_loadw_be overwrites top 4 stack elements including both copies from dup; save note_idx to local instead (pre-existing bug that only manifested with multiple output notes) - bridge_out.masm: fix num_leaves storage LE ordering - push new_leaf_count to stack top for Word[0] storage, use mem_storew_le instead of mem_storew_be for loading - bridge_config.masm: update GER hash from Rpo256 to Poseidon2 - canonical_zeros: remove .rev() from build.rs generation, swap push order for LE memory layout - Word element ordering fixes for bridge admin, GER manager, faucet registry keys, and conversion metadata Test expectation fixes: - Rpo256 -> Poseidon2 for GER hash computation - Removed word reversal in root/proof reading (LE convention) - Fixed expected storage value ordering - mem_storew_be -> mem_storew_le in test MASM code All 39 agglayer tests pass. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| # Arrange args for u64::overflowing_sub as: | ||
| # [y1, y0, x1, x0] | ||
| # Low 64 bits: (x0,x1) - (y0,y1) | ||
| # Arrange args for u64::overflowing_sub (LE convention) as: |
There was a problem hiding this comment.
did overflowing_sub change its expected inputs?
clarify, and as before, if docs are wrong, update the docs in the relevant repo
- Extract `create_id_key(id: AccountId) -> Word` helper and reuse for bridge_admin, ger_manager, bridge_account_id, and faucet_registry_key - Replace `felts_to_bytes` with re-export of `packed_u32_elements_to_bytes` from miden-core (identical implementation) - Remove stale comment in config_note.rs - Use `Felt::ONE` instead of `Felt::new(1)` in config_bridge test - Add TODO comments referencing issue #2548 for getter helpers - Simplify note_idx handling: use `padw` before `loc_loadw_le` instead of saving to a local variable; revert @Locals(15) to @Locals(14) - Switch `loc_storew_be`/`loc_loadw_be` to `_le` variants in create_burn_note - Add stack state comments before third overflowing_sub in u128_sub_no_underflow Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
87cc819 to
b1a0b6f
Compare
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Review in progress, left some questions.
Add convention to masm_doc_comment_fmt.md: - `value` = single felt - `value(N)` = N felts (non-word) - `VALUE` = word (4 felts, no (4) suffix needed) Apply throughout agglayer MASM: drop redundant (4) from ASSET_KEY, ASSET_VALUE, SERIAL_NUM, SCRIPT_ROOT, RECIPIENT, NOTE_ATTACHMENT, PROC_MAST_ROOT, OLD_VALUE, VALUE, U256_LO, U256_HI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Left a few comments regarding the big-endian -> little-endian migration.
crates/miden-agglayer/asm/agglayer/common/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-agglayer/asm/agglayer/common/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-testing/tests/agglayer/solidity_miden_address_conversion.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
4339785 to
05ae95c
Compare
Change GER_KNOWN_FLAG and IS_FAUCET_REGISTERED_FLAG to be stored at word[0] instead of word[3]. Under LE, `push.0.0.0.FLAG` puts FLAG at stack top = word[0], matching the documented layout [1, 0, 0, 0]. Previously `push.FLAG.0.0.0` placed FLAG at stack bottom = word[3], resulting in Word([0, 0, 0, 1]) which contradicted the docs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PhilippGackstatter
left a comment
There was a problem hiding this comment.
Looks good to me!
There is one TODO(bele) that would be good to do in this PR in scripts/check-features.sh.
crates/miden-testing/tests/agglayer/solidity_miden_address_conversion.rs
Show resolved
Hide resolved
Co-authored-by: Philipp Gackstatter <PhilippGackstatter@users.noreply.github.com>
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I focused mostly on non-test code and left just one comment inline (and also pushed a couple of small commits).
Once @PhilippGackstatter's comments are addressed, this should be good to merge.
Now that miden-agglayer has been migrated, add it back to the check-features.sh loop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All other bridge/faucet MASM procedures use [suffix, prefix] (suffix on top) for account IDs on the stack. Align to_account_id with this convention by adding a swap at the end, and update all callers: - faucet/mod.masm: update get_destination_account_id_data, claim, and build_p2id_output_note to handle the new stack order - solidity_miden_address_conversion test: swap stack index expectations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
miden-agglayercrate in the workspace (was commented out during the VM 0.21 migration)FieldElementremoval,as_int()->as_canonical_u64(), serde trait path changes,Programimport path,bytes_to_packed_u32_elementsrename,AccountIdconstructor updateu64::widening_mul, rewrittenu128_sub_no_underflow, updatedverify_u128_to_native_amount_conversionasset::mem_store/mem_loadtoasset::store/loadeth_address.masmbuild_feltu32split verification for LE output ordercreate_burn_notenote_idx corruption (pre-existing bug:loc_loadw_bedestroyed both copies fromdup; now uses local variable)Test plan
cargo clippy -p miden-agglayer --all-features -- -D warningspassescargo nextest run -p miden-testing -E 'test(agglayer)')make test-dev)🤖 Generated with Claude Code