-
Notifications
You must be signed in to change notification settings - Fork 106
feat: implement build_note_tag_for_local_account in masm
#2235
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
base: next
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -252,3 +252,62 @@ pub proc build_note_tag_for_network_account | |
| drop | ||
| # => [network_account_tag] | ||
| end | ||
|
|
||
| #! Constructs a LocalAny note tag from the given account_id. | ||
| #! | ||
| #! This procedure implements the same logic as the Rust NoteTag::from_local_account_id() | ||
| #! but assumes tag_len to be 14 bits. | ||
| #! | ||
| #! The tag is constructed as follows: | ||
| #! - The two most significant bits are set to `0b11` to indicate a LOCAL_ANY tag. | ||
| #! - The next 14 bits are set to the most significant bits of the account ID prefix. | ||
| #! - The remaining bits are set to zero. | ||
| #! | ||
| #! Inputs: [account_id_prefix, account_id_suffix] | ||
| #! Outputs: [local_account_tag] | ||
| #! | ||
partylikeits1983 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| #! Invocation: exec | ||
| pub proc build_note_tag_for_local_account | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As for the structure/naming, maybe it makes sense to introduce a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good idea! |
||
| # => [account_id_prefix, account_id_suffix] | ||
|
|
||
| # Drop the suffix as we only need the prefix | ||
| swap drop | ||
| # => [account_id_prefix] | ||
|
|
||
| # Convert prefix to u64 by splitting into high and low parts | ||
| u32split | ||
| # => [prefix_hi, prefix_lo] | ||
|
|
||
| # Shift the high bits of the account ID such that they are laid out as: | ||
| # [34 zero bits | remaining high bits (30 bits)] | ||
| push.2 | ||
| # => [2, prefix_hi, prefix_lo] | ||
partylikeits1983 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| exec.u64::shr | ||
| # => [shifted_hi, shifted_lo] | ||
|
|
||
| # This is equivalent to the following layout, interpreted as a u32: | ||
| # [2 zero bits | remaining high bits (30 bits)] | ||
| swap drop | ||
partylikeits1983 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| # => [high_bits] | ||
|
|
||
| # Select the top 14 bits of the account ID, i.e.: | ||
| # [2 zero bits | remaining high bits (14 bits) | (30 - 14) zero bits] | ||
| # Create mask: u32::MAX << (32 - 2 - 14) = u32::MAX << 16 | ||
| push.4294967295 | ||
| push.16 | ||
| # => [16, u32::MAX, high_bits] | ||
|
|
||
| u32shl | ||
| # => [mask, high_bits] | ||
|
|
||
| u32and | ||
| # => [masked_high_bits] | ||
|
|
||
| # Set the local execution tag in the two most significant bits | ||
| push.3221225472 # 0xc0000000 = LOCAL_ANY | ||
| # => [LOCAL_ANY, masked_high_bits] | ||
|
|
||
| u32or | ||
| # => [local_account_tag] | ||
| end | ||
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.
I don't think it makes sense to build on the
NoteTagformat fromnextthat will be outdated as soon as #2219 is merged, since that PR completely refactors the current format. This PR should rather build on top of #2219.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.
Makes sense! What is the timeline to merge #2219?
Once 2219 is merged I will update this PR
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.
Sounds good.
Sometime early next week if everything goes well.