feat(Agglayer): add scaled down quotient to CLAIM note NoteStorage#2336
feat(Agglayer): add scaled down quotient to CLAIM note NoteStorage#2336partylikeits1983 wants to merge 63 commits intoagglayer-newfrom
CLAIM note NoteStorage#2336Conversation
Co-authored-by: igamigo <ignacio.amigo@lambdaclass.com>
Co-authored-by: Marti <marti@miden.team>
| # The slot in this component's storage layout where the bridge account ID is stored. | ||
| const BRIDGE_ID_SLOT = word("miden::agglayer::faucet") | ||
|
|
||
| # The slots where origin token address and scaling factor are stored | ||
| # AGGFAUCET_METADATA_SLOT_0: [tok0, tok1, tok2, tok3] | ||
| # AGGFAUCET_METADATA_SLOT_1: [tok4, scaling_factor, 0, 0] | ||
| const AGGFAUCET_METADATA_SLOT_0 = word("miden::agglayer::aggfaucet_metadata_0") | ||
| const AGGFAUCET_METADATA_SLOT_1 = word("miden::agglayer::aggfaucet_metadata_1") |
There was a problem hiding this comment.
A few comments on this:
- Would it not be more convenient to store the token address as 8 felts (each holding a
u32value)? I'm assuming here that this would allow us to re-use the native serialization of the AggLayer format. - I would probably separate token address from other metadata as I'm imagining we'd need more metadata for the faucet. Or are we using basic fungible faucet metadata slot for that?
- For slot names, we should make the convention consistent. Specifically, I'd use
miden::agglayer::faucetas the namespace and then add slot names within that namespace. - I would also maybe provide a high level description of the storage layout for the whole faucet, and after that would declare all constants (would make it a bit easier to read.
Overall, for slots, I'd probably have something like this:
const BRIDGE_ID_SLOT = word("miden::agglayer::faucet::bridge_account_id")
const FAUCET_METADATA = word("miden::agglayer::faucet::metadata")
const TOKEN_ADDRESS_SLOT_0 = word("miden::agglayer::faucet::token_address_0")
const TOKEN_ADDRESS_SLOT_1 = word("miden::agglayer::faucet::token_address_1")
There was a problem hiding this comment.
Ah - scratch the first point above. Token address is only 20 bytes, so, we already have u32 representation - and we don't need more than 5 elements for them.
| #! Outputs: [origin_token_addr[0], origin_token_addr[1], origin_token_addr[2], origin_token_addr[3], origin_token_addr[4]] | ||
| #! | ||
| #! Invocation: exec | ||
| proc get_origin_token_address |
There was a problem hiding this comment.
Maybe not for this PR, but I'd make this procedure a part of the public interface. Since public interface procedures need to be invoked via the call instruction, we'd need to have two procedures:
- This one, which I'd rename into something like
get_origin_token_address_internal. - The public procedure which would handle the padding and then call this procedure.
| mem_load.ORIGIN_TOKEN_ADDRESS_2 | ||
| mem_load.ORIGIN_TOKEN_ADDRESS_3 | ||
| mem_load.ORIGIN_TOKEN_ADDRESS_4 | ||
| # => [ORIGIN_TOKEN_ADDRESS_CLAIM[5]] |
There was a problem hiding this comment.
Relying on memory layout in this way feels a bit fragile to me. I'd try to make this procedure a bit more stateless. One approach is to pass in the memory pointer to where the token address is stored as an input - i.e., something like:
#! Inputs: [token_address_ptr]
#! Outputs: []
proc assert_origin_address_eq_storage_address
padw dup.4 mem_loadw_be
movup.4 add.4 mem_load
...
end
This would take a few more cycles, but I don't think that's an issue.
| #! Reads the origin token address from the aggfaucet metadata storage slots. | ||
| #! | ||
| #! Inputs: [] | ||
| #! Outputs: [origin_token_addr[0], origin_token_addr[1], origin_token_addr[2], origin_token_addr[3], origin_token_addr[4]] | ||
| #! | ||
| #! Invocation: exec | ||
| proc get_origin_token_address |
There was a problem hiding this comment.
These changes don't seem related to how CLAIM note handles scaling down - ideally would have been a separate PR
db9825d to
3ea2737
Compare
|
Closing in favor of #2460 |
This PR utilizes the
scale_u256_to_native_amountprocedure during the claim asset flow.Changes this PR introduces:
CLAIMnoteNoteStoragescale_u256_to_native_amountprocedureoriginTokenAddress[5]&scalingfactor toaggfaucetacross two slots.origin_token_addressin theCLAIMnoteNoteStoragematches theorigin_token_addressin the agglayer faucet metadata.Builds on: #2331
Resolves: #2011 (comment)
Partially resolves faucet registry issue: #2172