shielded-pool(spend): fix validation of dummy spends in spend proof #3871
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
Instead of short circuiting all checks in the spend circuit in the case of a dummy spend (a note with value 0), this PR changes the spend circuit so that it only skips the SCT inclusion proof check. This prevents a nullifier griefing bug reported by Zellic in #3859 where an attacker watching the mempool can race transactions with a 0-value transaction, copying the signatures, in order to prevent a that nullifier from ever being spent.
avahowell
force-pushed
the
dummy-spend-circuit-fix
branch
from
7721b49 to
abbe262
Compare
…e commitment proof with invalid anchor
cratelyn
added
the
zellic-component-remediated
erwanor
changed the title
cronokirby
approved these changes
Feb 23, 2024
avahowell
changed the title
2 tasks
hdevalence
added a commit
that referenced
this pull request
Feb 25, 2024
From looking at the CI history, it seems like deployments have been failing since #3871; this seems like it might be the cause, so I'll try reverting it and seeing if that fixes the problem.
hdevalence
added a commit
that referenced
this pull request
Feb 25, 2024
From looking at the CI history, it seems like deployments have been failing since #3871; this seems like it might be the cause, so I'll try reverting it and seeing if that fixes the problem.
Merged
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.
Instead of short circuiting all checks in the spend circuit in the case of a dummy spend (a note with value 0), this PR changes the spend circuit so that it only skips the SCT inclusion proof check. This addresses bug reported by Zellic in #3859 and ensures the integrity of spends with zero value.
I also added a test that reproduces the bug in main, and verifies that the proof verifier rejects dummy spends with invalid proofs.