-
Notifications
You must be signed in to change notification settings - Fork 97
feat: implement u256 agglayer asset scaling up procedure #2141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements asset conversion functionality for Agglayer bridging operations, specifically adding a procedure to convert Miden asset amounts to U256 format with decimal scaling. The implementation includes a MASM procedure for computing scaled U256 values and Rust utilities for testing and integration.
Key changes:
- Adds
convert_to_u256_scaledMASM procedure to scale asset amounts by powers of 10 and convert to U256 format - Implements helper utilities for converting between Felt and U256 byte representations
- Provides comprehensive test coverage with two test cases validating scaling operations
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm | Core MASM implementation with pow10 helper and convert_to_u256_scaled procedure for asset scaling |
| crates/miden-lib/src/agglayer/mod.rs | Module definition providing asset conversion library initialization and component creation |
| crates/miden-lib/src/agglayer/utils.rs | Utility functions for converting between Felt values and U256 byte representations |
| crates/miden-testing/tests/agglayer/asset_conversion.rs | Test suite validating asset conversion with ETH and large amount scenarios |
| crates/miden-testing/tests/agglayer/mod.rs | Module declaration for agglayer tests |
| crates/miden-testing/tests/lib.rs | Adds agglayer test module to test suite |
| crates/miden-lib/src/lib.rs | Exposes agglayer module for testing/test features |
| crates/miden-lib/build.rs | Extends build script to compile agglayer account components |
| crates/miden-testing/Cargo.toml | Adds primitive-types dependency for U256 support in tests |
| Cargo.toml | Adds primitive-types to workspace dependencies |
| Cargo.lock | Locks new dependencies (primitive-types, byteorder, fixed-hash, hex, uint, static_assertions) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
…ion.masm Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ion.masm Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ion.masm Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
…ion.masm Co-authored-by: Marti <marti@miden.team>
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Renamed this PR to "feat: implement u256 agglayer asset scaling up procedure" since I think we should implement scaling down in a separate PR. |
| tokio = { default-features = false, features = ["sync"], version = "1" } | ||
| anyhow = { default-features = false, features = ["backtrace", "std"], version = "1.0" } | ||
| assert_matches = { default-features = false, version = "1.5" } | ||
| primitive-types = { default-features = false, version = "0.14" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Importing U256 type.
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
…ion.masm Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
bobbinth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you! I left a few comments inline - the main one is about using u64 multiplication to make the procedure much more efficient.
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
bobbinth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Thank you! I left a few more comments inline.
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
crates/miden-lib/asm/agglayer/account_components/asset_conversion.masm
Outdated
Show resolved
Hide resolved
|
Changed I think this is ready to merge @bobbinth @mmagician |
bobbinth
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All looks good! Thank you!
Asset Conversion Procedure Overview
This PR adds the functionality for converting between asset amounts with various decimals for Agglayer bridging operations.
Note:
Resolves: #2011
The
convert_to_u256_scaledprocedure converts Miden amounts to Agglayer compatibleu256format for bridging to agglayer chains.