Skip to content
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

Stop double-spends by checking nullifiers in the finalized state #2230

Closed
5 of 9 tasks
teor2345 opened this issue Jun 1, 2021 · 2 comments
Closed
5 of 9 tasks

Stop double-spends by checking nullifiers in the finalized state #2230

teor2345 opened this issue Jun 1, 2021 · 2 comments
Assignees
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) NU-1 Sapling Network Upgrade: Sapling specific tasks NU-5 Network Upgrade: NU5 specific tasks

Comments

@teor2345
Copy link
Contributor

teor2345 commented Jun 1, 2021

Motivation

Zebra needs to check the double-spend consensus rule for shielded spends in the finalized state.

Consensus Rule

Nullifiers

A nullifier MUST NOT repeat either within a transaction, or across transactions in a valid blockchain. Sprout and Sapling and Orchard nullifiers are considered disjoint, even if they have the same bit pattern.

https://zips.z.cash/protocol/nu5.pdf#nullifierset

A transaction is not valid if it would have added a nullifier to the nullifier set that already exists in the set

https://zips.z.cash/protocol/nu5.pdf#commitmentsandnullifiers

Existing Design

pub(super) fn queue_and_commit_finalized_blocks(&mut self, queued_block: QueuedBlock)
...
Iterate over the enumerated transactions in the block. For each transaction:
...
For each JoinSplit description in the transaction, insert (nullifiers[0],()) and (nullifiers[1],()) into sprout_nullifiers.
For each Spend description in the transaction, insert (nullifier,()) into sapling_nullifiers.
For each Orchard description in the transaction, insert (nullifier,()) into orchard_nullifiers.

https://github.com/ZcashFoundation/zebra/blob/main/book/src/dev/rfcs/0005-state-updates.md#pubsuper-fn-queue_and_commit_finalized_blocksmut-self-queued_block-queuedblock

Solution

Design Change

  • Inserts into RocksDB column families should return an error if the key already exists.

This design change enforces the double-spend rules for each shielded pool.

It also enforces single writes of the other 8 column families in the data structure (as of 1 June 2021):
https://github.com/ZcashFoundation/zebra/blob/main/book/src/dev/rfcs/0005-state-updates.md#rocksdb-data-structures

Implementation Change

Here's the RocksDB merge feature documentation:
https://github.com/facebook/rocksdb/wiki/Merge-Operator#what

Testing

  • Test that duplicate sprout, sapling, and orchard nullifiers are rejected by the finalized state
  • Test that duplicate spends of UTXOs are rejected by the finalized state

Optional:

  • Proptest the merge operator function

Alternatives

We could try to fetch each value before writing it, but that's error-prone. We'd probably forget to do the check somewhere in the code.

@teor2345 teor2345 added A-consensus Area: Consensus rule updates NU-1 Sapling Network Upgrade: Sapling specific tasks NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement S-needs-triage Status: A bug report needs triage NU-5 Network Upgrade: NU5 specific tasks P-Medium labels Jun 1, 2021
@mpguerra mpguerra mentioned this issue Jun 1, 2021
53 tasks
@mpguerra mpguerra removed the S-needs-triage Status: A bug report needs triage label Jun 4, 2021
@teor2345 teor2345 changed the title Stop double-spends by checking nullifiers in the finalized state Stop double-spends by checking nullifiers and UTXOs in the finalized state Jun 24, 2021
@teor2345 teor2345 changed the title Stop double-spends by checking nullifiers and UTXOs in the finalized state Stop double-spends by checking nullifiers in the finalized state Jun 24, 2021
@teor2345 teor2345 changed the title Stop double-spends by checking nullifiers in the finalized state Stop double-spends by checking nullifiers and UTXO spends in the finalized state Jun 24, 2021
@teor2345
Copy link
Contributor Author

This ticket is blocked by #2301, because they both want to return errors from the UpdateWith trait.

@teor2345 teor2345 self-assigned this Jun 30, 2021
@teor2345 teor2345 changed the title Stop double-spends by checking nullifiers and UTXO spends in the finalized state Stop double-spends by checking nullifiers in the finalized state Jun 30, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Jul 2, 2021

This ticket is actually a duplicate of #2231 - if we check for duplicates when adding a block to a non-finalized chain, we don't need to check again in the finalized state.

@teor2345 teor2345 closed this as completed Jul 2, 2021
@teor2345 teor2345 removed this from the 2021 Sprint 13 milestone Jul 2, 2021
mergify bot pushed a commit that referenced this issue May 23, 2023
* ZIPs were updated to remove ambiguity, this was tracked in #1267.

* #2105 was fixed by #3039 and #2379 was closed by #3069

* #2230 was a duplicate of #2231 which was closed by #2511

* #3235 was obsoleted by #2156 which was fixed by #3505

* #1850 was fixed by #2944, #1851 was fixed by #2961 and #2902 was fixed by #2969

* We migrated to Rust 2021 edition in Jan 2022 with #3332

* #1631 was closed as not needed

* #338 was fixed by #3040 and #1162 was fixed by #3067

* #2079 was fixed by #2445

* #4794 was fixed by #6122

* #1678 stopped being an issue

* #3151 was fixed by #3934

* #3204 was closed as not needed

* #1213 was fixed by #4586

* #1774 was closed as not needed

* #4633 was closed as not needed

* Clarify behaviour of difficulty spacing

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour

Co-authored-by: teor <teor@riseup.net>

* Update comment to reflect implemented behaviour when retrying block downloads

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify when we might want to fix

Co-authored-by: teor <teor@riseup.net>

* Update `TODO` to remove closed issue and clarify what we might want to change in future

Co-authored-by: teor <teor@riseup.net>

* Clarify benefits of how we do block verification

Co-authored-by: teor <teor@riseup.net>

* Fix rustfmt errors

---------

Co-authored-by: teor <teor@riseup.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-consensus Area: Consensus rule updates A-rust Area: Updates to Rust code C-enhancement Category: This is an improvement NU Sprout Network Upgrade: Sprout specific tasks (before Overwinter) NU-1 Sapling Network Upgrade: Sapling specific tasks NU-5 Network Upgrade: NU5 specific tasks
Projects
None yet
Development

No branches or pull requests

2 participants