Skip to content

Spend descriptions validation#2244

Merged
brentstone merged 16 commits intomainfrom
grarco/spend-description-validation
Dec 29, 2023
Merged

Spend descriptions validation#2244
brentstone merged 16 commits intomainfrom
grarco/spend-description-validation

Conversation

@grarco
Copy link
Copy Markdown
Collaborator

@grarco grarco commented Dec 4, 2023

Describe your changes

Addresses #1373.

Creates a note commitment tree in storage and modifies the masp tx to update this tree.
Changes finalize_block to update the anchor of this tree at the end of a block.
Updates the masp VP to verify the changes made to the tree (also checks that the tree capacity is not exceeded) and the validity of the anchor for each spend description.

Indicate on which release or other PRs this topic is based on

Commit 39e00c5 of #2240

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

@grarco grarco force-pushed the grarco/spend-description-validation branch from d1879d2 to 694b949 Compare December 6, 2023 10:59
@grarco grarco marked this pull request as ready for review December 6, 2023 11:26
@grarco grarco requested a review from murisi December 6, 2023 11:26
}

if !(self.valid_spend_descriptions_anchor(&shielded_tx)?
&& self.valid_nullifiers_reveal(keys_changed, &shielded_tx)?)
Copy link
Copy Markdown
Collaborator Author

@grarco grarco Dec 6, 2023

Choose a reason for hiding this comment

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

In case instead source != Address::Internal(Masp), should we add a check to verify that no shielded_spends, nor shielded_converts are attached to the Transaction?

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.

Yes, this sounds good to me, thanks! (Maybe you can also add this to the Boundary Conditions section in https://specs.namada.net/masp/ledger-integration ?)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For simplicity's sake I'm addressing this in #2248

Copy link
Copy Markdown
Collaborator

@murisi murisi left a comment

Choose a reason for hiding this comment

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

This looks good to me, thanks!

@grarco grarco mentioned this pull request Dec 11, 2023
Fraccaman pushed a commit that referenced this pull request Dec 13, 2023
* origin/grarco/spend-description-validation:
  Changelog #2244
  Updates comment
  Fixes masp vp benchmark
  Fixes commitment tree validation in masp vp. Adds a workaround to update tree from the tx
  Updates the merkle tree anchor only if the tree changed
  Updates commitment tree anchor only once per block
  Refactors masp nullifiers check in a separate function
  Updates masp vp to validate note commitment tree and anchor
  Updates masp tx with note commitment tree and anchor
brentstone added a commit that referenced this pull request Dec 29, 2023
* origin/grarco/spend-description-validation:
  Changelog #2244
  Updates comment
  Fixes masp vp benchmark
  Fixes commitment tree validation in masp vp. Adds a workaround to update tree from the tx
  Updates the merkle tree anchor only if the tree changed
  Updates commitment tree anchor only once per block
  Refactors masp nullifiers check in a separate function
  Updates masp vp to validate note commitment tree and anchor
  Updates masp tx with note commitment tree and anchor
@brentstone brentstone merged commit b9e1502 into main Dec 29, 2023
@brentstone brentstone deleted the grarco/spend-description-validation branch December 29, 2023 19:37
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.

3 participants