-
Notifications
You must be signed in to change notification settings - Fork 0
refactor(pswap): follow-ups for #3000 (AssetAmount + NonZeroU32, parsing, MASM asserts, file split, zero-amount fix) #4
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 8 commits
b8d78bf
cf8fee4
a29e336
54d07fa
a1b07eb
6bafbf2
259c048
0565c0f
ca43a6f
420bc00
5ccde28
0682994
157eef5
c816401
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 |
|---|---|---|
|
|
@@ -96,6 +96,7 @@ const EXEC_AMT_REQUESTED_NOTE_FILL = 9 | |
| # write the word at PARENT_ATTACHMENT_PTR, read back only the depth at DEPTH_OFFSET. | ||
| const PARENT_ATTACHMENT_PTR = 0 | ||
| const PARENT_ATTACHMENT_DEPTH_OFFSET = 2 | ||
| const PSWAP_ATTACHMENT_NUM_WORDS = 1 | ||
|
|
||
| # ERRORS | ||
| # ================================================================================================= | ||
|
|
@@ -106,6 +107,8 @@ const ERR_PSWAP_FILL_EXCEEDS_REQUESTED="PSWAP fill amount exceeds requested amou | |
| const ERR_PSWAP_FILL_SUM_OVERFLOW="PSWAP account_fill + note_fill overflows u64" | ||
| const ERR_PSWAP_NOT_VALID_ASSET_AMOUNT="PSWAP computed amount exceeds max fungible asset amount" | ||
| const ERR_PSWAP_PAYOUT_OVERFLOW="PSWAP payout quotient does not fit in u64" | ||
| const ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS="PSWAP attachment must encode exactly one word" | ||
| const ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32="PSWAP attachment depth must fit in u32" | ||
|
|
||
| # U64 VALIDATION | ||
| # ================================================================================================= | ||
|
|
@@ -493,9 +496,13 @@ proc get_order_id | |
| end | ||
|
|
||
| #! Returns current_depth = parent_depth + 1, where parent_depth is the depth carried in the | ||
| #! consumed PSWAP note's PswapAttachment word at offset PARENT_ATTACHMENT_DEPTH_OFFSET if such an | ||
| #! attachment exists, or 0 if not (i.e., the parent is the original PSWAP, with scheme none() | ||
| #! or NetworkAccountTarget). | ||
| #! consumed PSWAP note's PswapAttachment word at offset PARENT_ATTACHMENT_DEPTH_OFFSET if such | ||
| #! an attachment exists, or 0 if not. | ||
| #! | ||
| #! The initial (depth-0) PSWAP carries no PSWAP_ATTACHMENT_SCHEME attachment; only payback P2IDs | ||
| #! and remainder PSWAPs do. So `is_found = false` here means we are consuming the original PSWAP | ||
| #! (or one carrying only an unrelated scheme like NetworkAccountTarget) and the next round is | ||
| #! depth 1. | ||
| #! | ||
| #! Inputs: [] | ||
| #! Outputs: [current_depth] | ||
|
|
@@ -514,11 +521,19 @@ proc get_current_depth | |
| exec.active_note::write_attachment_to_memory | ||
| # => [num_words] | ||
|
|
||
| drop | ||
| eq.PSWAP_ATTACHMENT_NUM_WORDS | ||
| assert.err=ERR_PSWAP_ATTACHMENT_WRONG_NUM_WORDS | ||
| # => [] | ||
|
|
||
| loc_load.PARENT_ATTACHMENT_DEPTH_OFFSET | ||
| # => [parent_depth] | ||
|
|
||
| # Manual hi==0 check rather than u32assert.err=... because the VM wraps u32assert's | ||
|
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. remove this comment.... |
||
| # message at runtime, which would defeat exact-match assertions on the named error. | ||
|
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. remove this above line |
||
| dup u32split drop | ||
| # => [hi, parent_depth] | ||
| assertz.err=ERR_PSWAP_ATTACHMENT_DEPTH_NOT_U32 | ||
|
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. line after # => |
||
| # => [parent_depth] | ||
|
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. Remove this long comment |
||
| else | ||
| drop push.0 | ||
| # => [0] | ||
|
|
||
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.
The attachment-length guard runs after the unsafe write.
write_attachment_to_memoryhas already copied the payload into a 4-local buffer beforenum_words == 1is asserted, so a forged multi-word PSWAP attachment can still clobber adjacent local memory and only then fail. If there is no length-only probe, this needs a scratch region sized for the maximum attachment length or another pre-write bound check; the current check does not provide the protection described in the comment.🤖 Prompt for AI Agents