Spend descriptions validation #2244
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ate tree from the tx
grarco
force-pushed
the
grarco/spend-description-validation
branch
from
d1879d2 to
694b949
Compare
grarco
commented
Dec 6, 2023
grarco
commented
Dec 6, 2023
| @@ -161,6 +322,18 @@ where | |||
| return Ok(false); | |||
| } | |||
| } | |||
|
|
|||
| if !(self.valid_spend_descriptions_anchor(&shielded_tx)? | |||
| && self.valid_nullifiers_reveal(keys_changed, &shielded_tx)?) | |||
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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 ?)
There was a problem hiding this comment.
For simplicity's sake I'm addressing this in #2248
2 tasks
murisi
approved these changes
Dec 9, 2023
Closed
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
Fraccaman
pushed a commit
that referenced
this pull request
Dec 13, 2023
grarco
added a commit
that referenced
this pull request
Dec 15, 2023
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
added a commit
that referenced
this pull request
Dec 29, 2023
brentstone
added a commit
that referenced
this pull request
Dec 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
Addresses #1373.
Creates a note commitment tree in storage and modifies the masp tx to update this tree.
Changes
finalize_blockto 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