Skip to content

fix(standards): bound the PSWAP lineage depth to u32 - #3777

Open
onurinanc wants to merge 7 commits into
nextfrom
fix-bound-pswap-depth
Open

fix(standards): bound the PSWAP lineage depth to u32#3777
onurinanc wants to merge 7 commits into
nextfrom
fix-bound-pswap-depth

Conversation

@onurinanc

Copy link
Copy Markdown
Collaborator

Summary

  • Bound the depth increment in get_current_depth to a u32, aborting with ERR_PSWAP_PARENT_DEPTH_NOT_U32 or ERR_PSWAP_DEPTH_OVERFLOW instead of wrapping the field.
  • Validate the PswapAttachment word through a new TryFrom<&NoteAttachment> for PswapNoteAttachment whenever a PswapNote is built or decoded.

Closes #3682.

@mmagician mmagician left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MASM change is ok, but I'd reword some of the comments to be more human-readable

Comment on lines +632 to +633
/// Returns an error if `attachment` was not stamped in a round after this note, or if the
/// fill amount is not a valid asset amount.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

attachment was not stamped in a round after this note

I don't understand what this means, could we simplify?

Comment on lines +639 to +640
// The payback of a round is derived from the note consumed in that round, whose serial
// sits one bump per preceding round after this note's.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

serial sits one bump per preceding round after this note's.

Again, this reads cryptic to me

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Smart Contract Diff Audit L-21: PSWAP Lineage Depth Tracking Diverges Between The On-Chain Script And The Rust Helpers

2 participants