WIP: Reject double-spends of shielded nullifiers and UTXOs

zebra-consensus/src/script.rs

@@ -5,7 +5,7 @@ use tracing::Instrument;
 
 use zebra_chain::{parameters::NetworkUpgrade, transparent};
 use zebra_script::CachedFfiTransaction;
-use zebra_state::Utxo;
+use zebra_state::OrderedUtxo;
 
 use crate::BoxError;
 
@@ -59,7 +59,7 @@ pub struct Request {
     /// A set of additional UTXOs known in the context of this verification request.
     ///
     /// This allows specifying additional UTXOs that are not already known to the chain state.
-    pub known_utxos: Arc<HashMap<transparent::OutPoint, Utxo>>,
+    pub known_utxos: Arc<HashMap<transparent::OutPoint, OrderedUtxo>>,
     /// The network upgrade active in the context of this verification request.
     ///
     /// Because the consensus branch ID changes with each network upgrade,
@@ -111,7 +111,7 @@ where
                     tracing::trace!("awaiting outpoint lookup");
                     let utxo = if let Some(output) = known_utxos.get(&outpoint) {
                         tracing::trace!("UXTO in known_utxos, discarding query");
-                        output.clone()
+                        output.utxo.clone()
                     } else if let zebra_state::Response::Utxo(utxo) = query.await? {
                         utxo
                     } else {

zebra-consensus/src/transaction.rs

@@ -70,7 +70,7 @@ pub enum Request {
         /// The transaction itself.
         transaction: Arc<Transaction>,
         /// Additional UTXOs which are known at the time of verification.
-        known_utxos: Arc<HashMap<transparent::OutPoint, zs::Utxo>>,
+        known_utxos: Arc<HashMap<transparent::OutPoint, zs::OrderedUtxo>>,
         /// The height of the block containing this transaction.
         height: block::Height,
     },
@@ -100,7 +100,7 @@ impl Request {
     }
 
     /// The set of additional known unspent transaction outputs that's in this request.
-    pub fn known_utxos(&self) -> Arc<HashMap<transparent::OutPoint, zs::Utxo>> {
+    pub fn known_utxos(&self) -> Arc<HashMap<transparent::OutPoint, zs::OrderedUtxo>> {
         match self {
             Request::Block { known_utxos, .. } => known_utxos.clone(),
             Request::Mempool { .. } => HashMap::new().into(),

zebra-consensus/src/transaction/tests.rs

@@ -17,7 +17,7 @@ use zebra_chain::{
     },
     transparent::{self, CoinbaseData},
 };
-use zebra_state::Utxo;
+use zebra_state::OrderedUtxo;
 
 use super::{check, Request, Verifier};
 
@@ -823,23 +823,27 @@ fn v5_with_sapling_spends() {
 /// First, this creates a fake unspent transaction output from a fake transaction included in the
 /// specified `previous_utxo_height` block height. This fake [`Utxo`] also contains a simple script
 /// that can either accept or reject any spend attempt, depending on if `script_should_succeed` is
-/// `true` or `false`.
+/// `true` or `false`. Since the `tx_index_in_block` is irrelevant for blocks that have already
+/// been verified, it is set to `1`.
 ///
 /// Then, a [`transparent::Input::PrevOut`] is created that attempts to spend the previously created fake
-/// UTXO. A new UTXO is created with the [`transparent::Output`] resulting from the spend.
+/// UTXO to a new [`transparent::Output`].
 ///
 /// Finally, the initial fake UTXO is placed in a `known_utxos` [`HashMap`] so that it can be
 /// retrieved during verification.
 ///
 /// The function then returns the generated transparent input and output, as well as the
 /// `known_utxos` map.
+///
+/// Note: `known_utxos` is only intended to be used for UTXOs within the same block,
+/// so future verification changes might break this mocking function.
 fn mock_transparent_transfer(
     previous_utxo_height: block::Height,
     script_should_succeed: bool,
 ) -> (
     transparent::Input,
     transparent::Output,
-    HashMap<transparent::OutPoint, Utxo>,
+    HashMap<transparent::OutPoint, OrderedUtxo>,
 ) {
     // A script with a single opcode that accepts the transaction (pushes true on the stack)
     let accepting_script = transparent::Script::new(&[1, 1]);
@@ -863,11 +867,7 @@ fn mock_transparent_transfer(
         lock_script,
     };
 
-    let previous_utxo = Utxo {
-        output: previous_output,
-        height: previous_utxo_height,
-        from_coinbase: false,
-    };
+    let previous_utxo = OrderedUtxo::new(previous_output, previous_utxo_height, false, 1);
 
     // Use the `previous_outpoint` as input
     let input = transparent::Input::PrevOut {

zebra-state/Cargo.toml

@@ -10,23 +10,25 @@ edition = "2018"
 [dependencies]
 zebra-chain = { path = "../zebra-chain" }
 
-dirs = "3.0.2"
+chrono = "0.4.19"
 hex = "0.4.3"
+itertools = "0.10.1"
 lazy_static = "1.4.0"
 regex = "1"
 serde = { version = "1", features = ["serde_derive"] }
 
+displaydoc = "0.2.2"
 futures = "0.3.15"
 metrics = "0.13.0-alpha.8"
 tower = { version = "0.4", features = ["buffer", "util"] }
 tracing = "0.1"
 thiserror = "1.0.25"
 tokio = { version = "0.3.6", features = ["sync"] }
-displaydoc = "0.2.2"
+
+dirs = "3.0.2"
+rlimit = "0.5.4"
 rocksdb = "0.16.0"
 tempdir = "0.3.7"
-chrono = "0.4.19"
-rlimit = "0.5.4"
 
 [dev-dependencies]
 zebra-chain = { path = "../zebra-chain", features = ["proptest-impl"] }

zebra-state/src/lib.rs

@@ -36,4 +36,4 @@ pub use error::{BoxError, CloneError, CommitBlockError, ValidateContextError};
 pub use request::{FinalizedBlock, HashOrHeight, PreparedBlock, Request};
 pub use response::Response;
 pub use service::init;
-pub use utxo::{new_outputs, Utxo};
+pub use utxo::{new_outputs, OrderedUtxo, Utxo};

zebra-state/src/request.rs

@@ -5,7 +5,7 @@ use zebra_chain::{
     transaction, transparent,
 };
 
-use crate::Utxo;
+use crate::OrderedUtxo;
 
 // Allow *only* this unused import, so that rustdoc link resolution
 // will work with inline links.
@@ -73,7 +73,7 @@ pub struct PreparedBlock {
     /// Note: although these transparent outputs are newly created, they may not
     /// be unspent, since a later transaction in a block can spend outputs of an
     /// earlier transaction.
-    pub new_outputs: HashMap<transparent::OutPoint, Utxo>,
+    pub new_outputs: HashMap<transparent::OutPoint, OrderedUtxo>,
     /// A precomputed list of the hashes of the transactions in this block.
     pub transaction_hashes: Vec<transaction::Hash>,
     // TODO: add these parameters when we can compute anchors.
@@ -98,7 +98,7 @@ pub struct FinalizedBlock {
     /// Note: although these transparent outputs are newly created, they may not
     /// be unspent, since a later transaction in a block can spend outputs of an
     /// earlier transaction.
-    pub(crate) new_outputs: HashMap<transparent::OutPoint, Utxo>,
+    pub(crate) new_outputs: HashMap<transparent::OutPoint, OrderedUtxo>,
     /// A precomputed list of the hashes of the transactions in this block.
     pub(crate) transaction_hashes: Vec<transaction::Hash>,
 }

zebra-state/src/service/finalized_state.rs

@@ -1,10 +1,12 @@
 //! The primary implementation of the `zebra_state::Service` built upon rocksdb
 
+mod check;
 mod disk_format;
 
 #[cfg(test)]
 mod tests;
 
+use std::collections::HashSet;
 use std::{collections::HashMap, convert::TryInto, path::Path, sync::Arc};
 
 use zebra_chain::transparent;
@@ -39,6 +41,7 @@ pub struct FinalizedState {
 impl FinalizedState {
     pub fn new(config: &Config, network: Network) -> Self {
         let (path, db_options) = config.db_config(network);
+
         let column_families = vec![
             rocksdb::ColumnFamilyDescriptor::new("hash_by_height", db_options.clone()),
             rocksdb::ColumnFamilyDescriptor::new("height_by_hash", db_options.clone()),
@@ -49,6 +52,7 @@ impl FinalizedState {
             rocksdb::ColumnFamilyDescriptor::new("sapling_nullifiers", db_options.clone()),
             rocksdb::ColumnFamilyDescriptor::new("orchard_nullifiers", db_options.clone()),
         ];
+
         let db_result = rocksdb::DB::open_cf_descriptors(&db_options, &path, column_families);
 
         let db = match db_result {
@@ -227,6 +231,44 @@ impl FinalizedState {
             );
         }
 
+        let spent_transparent_outpoints;
+        let revealed_sprout_nullifiers;
+        let revealed_sapling_nullifiers;
+        let revealed_orchard_nullifiers;
+
+        // Consensus rule: transactions in the genesis block are ignored.
+        if block.header.previous_block_hash != GENESIS_PREVIOUS_BLOCK_HASH {
+            // Reject double-spends of transparent outputs and nullifers, where:
+            // - both are within this block, and
+            // - one is in this block, and the other was already committed to the finalized state.
+
+            spent_transparent_outpoints =
+                check::transparent_double_spends(&self.db, utxo_by_outpoint, &block, &new_outputs)?;
+
+            revealed_sprout_nullifiers = check::nullifier_double_spends(
+                &self.db,
+                sprout_nullifiers,
+                block.sprout_nullifiers().cloned().collect(),
+            )?;
+
+            revealed_sapling_nullifiers = check::nullifier_double_spends(
+                &self.db,
+                sapling_nullifiers,
+                block.sapling_nullifiers().cloned().collect(),
+            )?;
+
+            revealed_orchard_nullifiers = check::nullifier_double_spends(
+                &self.db,
+                orchard_nullifiers,
+                block.orchard_nullifiers().cloned().collect(),
+            )?;
+        } else {
+            spent_transparent_outpoints = HashSet::new();
+            revealed_sprout_nullifiers = HashSet::new();
+            revealed_sapling_nullifiers = HashSet::new();
+            revealed_orchard_nullifiers = HashSet::new();
+        }
+
         // We use a closure so we can use an early return for control flow in
         // the genesis case
         let prepare_commit = || -> rocksdb::WriteBatch {
@@ -246,17 +288,37 @@ impl FinalizedState {
             }
 
             // Index all new transparent outputs
-            for (outpoint, utxo) in new_outputs.into_iter() {
-                batch.zs_insert(utxo_by_outpoint, outpoint, utxo);
+            //
+            // Spends of outputs created by this block will be added here,
+            // then deleted in the next statement.
+            for (outpoint, ordered_utxo) in new_outputs.into_iter() {
+                batch.zs_insert(utxo_by_outpoint, outpoint, ordered_utxo.utxo);
+            }
+
+            // Mark all transparent inputs as spent
+            //
+            // Deletes of missing keys and multiple deletes of the same key are ignored,
+            // so we must check for double-spends separately above.
+            for spent_outpoint in spent_transparent_outpoints {
+                batch.delete_cf(utxo_by_outpoint, spent_outpoint.as_bytes());
+            }
+
+            // Mark sprout, sapling and orchard nullifiers as spent
+            //
+            // Multiple inserts of the same key are ignored,
+            // so we must check for double-spends separately above.
+            for sprout_nullifier in revealed_sprout_nullifiers {
+                batch.zs_insert(sprout_nullifiers, sprout_nullifier, ());
+            }
+            for sapling_nullifier in revealed_sapling_nullifiers {
+                batch.zs_insert(sapling_nullifiers, sapling_nullifier, ());
+            }
+            for orchard_nullifier in revealed_orchard_nullifiers {
+                batch.zs_insert(orchard_nullifiers, orchard_nullifier, ());
             }
 
             // Index each transaction, spent inputs, nullifiers
-            // TODO: move computation into FinalizedBlock as with transparent outputs
-            for (transaction_index, (transaction, transaction_hash)) in block
-                .transactions
-                .iter()
-                .zip(transaction_hashes.into_iter())
-                .enumerate()
+            for (transaction_index, transaction_hash) in transaction_hashes.into_iter().enumerate()
             {
                 let transaction_location = TransactionLocation {
                     height,
@@ -265,29 +327,6 @@ impl FinalizedState {
                         .expect("no more than 4 billion transactions per block"),
                 };
                 batch.zs_insert(tx_by_hash, transaction_hash, transaction_location);
-
-                // Mark all transparent inputs as spent
-                for input in transaction.inputs() {
-                    match input {
-                        transparent::Input::PrevOut { outpoint, .. } => {
-                            batch.delete_cf(utxo_by_outpoint, outpoint.as_bytes());
-                        }
-                        // Coinbase inputs represent new coins,
-                        // so there are no UTXOs to mark as spent.
-                        transparent::Input::Coinbase { .. } => {}
-                    }
-                }
-
-                // Mark sprout, sapling and orchard nullifiers as spent
-                for sprout_nullifier in transaction.sprout_nullifiers() {
-                    batch.zs_insert(sprout_nullifiers, sprout_nullifier, ());
-                }
-                for sapling_nullifier in transaction.sapling_nullifiers() {
-                    batch.zs_insert(sapling_nullifiers, sapling_nullifier, ());
-                }
-                for orchard_nullifier in transaction.orchard_nullifiers() {
-                    batch.zs_insert(orchard_nullifiers, orchard_nullifier, ());
-                }
             }
 
             batch