Check for transaction inputs duplicates (keep-starknet-strange#203)

packages/consensus/src/types/utxo_set.cairo

@@ -15,6 +15,10 @@ use core::poseidon::PoseidonTrait;
 use super::transaction::OutPoint;
 use super::utreexo::{UtreexoState, UtreexoAccumulator};
 
+pub const TX_OUTPUT_STATUS_NONE: u8 = 0;
+pub const TX_OUTPUT_STATUS_UNSPENT: u8 = 1;
+pub const TX_OUTPUT_STATUS_SPENT: u8 = 2;
+
 #[derive(Default, Destruct)]
 pub struct UtxoSet {
     /// Utreexo state.
@@ -24,7 +28,7 @@ pub struct UtxoSet {
     /// Hashes of UTXOs created within the current block(s).
     /// Note that to preserve the ordering, cache has to be updated right after a
     /// particular output is created or spent.
-    pub cache: Felt252Dict<bool>,
+    pub cache: Felt252Dict<u8>,
 }
 
 #[generate_trait]
@@ -33,13 +37,17 @@ pub impl UtxoSetImpl of UtxoSetTrait {
         UtxoSet { utreexo_state, ..Default::default() }
     }
 
-    fn add(ref self: UtxoSet, output: OutPoint) {
-        let outpoint_hash = PoseidonTrait::new().update_with(output).finalize();
+    fn add(ref self: UtxoSet, output: OutPoint) -> Result<(), ByteArray> {
+        let hash = PoseidonTrait::new().update_with(output).finalize();
         if output.data.cached {
-            self.cache.insert(outpoint_hash, true);
+            if self.cache.get(hash) != TX_OUTPUT_STATUS_NONE {
+                return Result::Err("The output has already been added");
+            }
+            self.cache.insert(hash, TX_OUTPUT_STATUS_UNSPENT);
         } else {
-            self.leaves_to_add.append(outpoint_hash);
+            self.leaves_to_add.append(hash);
         }
+        Result::Ok(())
     }
 
     fn utreexo_add(ref self: UtxoSet) {
@@ -48,14 +56,17 @@ pub impl UtxoSetImpl of UtxoSetTrait {
         }
     }
 
-    fn delete(ref self: UtxoSet, output: @OutPoint) {
-        if *output.data.cached {
-            let outpoint_hash = PoseidonTrait::new().update_with(*output).finalize();
+    fn spend(ref self: UtxoSet, output: @OutPoint) -> Result<(), ByteArray> {
+        let hash = PoseidonTrait::new().update_with(*output).finalize();
+        let status = self.cache.get(hash);
+        if status == TX_OUTPUT_STATUS_NONE {
             // Extra check that can be removed later.
-            assert(self.cache.get(outpoint_hash), 'output is not cached');
-            self.cache.insert(outpoint_hash, false);
-        } else { // TODO: update utreexo roots (+ verify inclusion)
-        // If batched proofs are used then do nothing.
+            assert(!*output.data.cached, 'output is not cached');
+            // TODO: verify inclusion and update utreexo roots
+        } else if status == TX_OUTPUT_STATUS_SPENT {
+            return Result::Err("The output has already been spent");
         }
+        self.cache.insert(hash, TX_OUTPUT_STATUS_SPENT);
+        Result::Ok(())
     }
 }

packages/consensus/src/validation/block.cairo

@@ -1,6 +1,6 @@
 //! Block validation helpers.
-use crate::types::utxo_set::UtxoSet;
-use crate::types::transaction::Transaction;
+use crate::types::utxo_set::{UtxoSet, UtxoSetTrait};
+use crate::types::transaction::{OutPoint, Transaction};
 use crate::codec::{Encode, TransactionCodec};
 use utils::{hash::Digest, merkle_tree::merkle_root, double_sha256::double_sha256_byte_array};
 use super::transaction::validate_transaction;
@@ -35,14 +35,10 @@ pub fn compute_and_validate_tx_data(
     let mut wtxids: Array<Digest> = array![];
     let mut total_fee = 0;
     let mut total_weight: u32 = 0;
-    let mut i = 0;
+    let mut inner_result = Result::Ok(());
+    let mut is_coinbase = true;
 
-    let validate_transactions: Result<(), ByteArray> = loop {
-        if i >= txs.len() {
-            break Result::Ok(());
-        }
-
-        let tx = txs[i];
+    for tx in txs {
         let tx_bytes_legacy = @tx.encode();
         let txid = double_sha256_byte_array(tx_bytes_legacy);
 
@@ -53,10 +49,10 @@ pub fn compute_and_validate_tx_data(
             /// The wTXID for the coinbase transaction must be set to all zeros. This is because
             /// it's eventually going to contain the commitment inside it
             /// see https://learnmeabitcoin.com/technical/transaction/wtxid/#commitment
-            let wtxid = if i != 0 {
-                double_sha256_byte_array(tx_bytes_segwit)
-            } else {
+            let wtxid = if is_coinbase {
                 Zero::zero()
+            } else {
+                double_sha256_byte_array(tx_bytes_segwit)
             };
 
             total_weight += 3 * tx_bytes_legacy.len()
@@ -70,19 +66,33 @@ pub fn compute_and_validate_tx_data(
 
         txids.append(txid);
 
-        // skipping the coinbase transaction
-        if i != 0 {
+        if (is_coinbase) {
+            let mut vout = 0;
+            for output in *tx
+                .outputs {
+                    let outpoint = OutPoint {
+                        txid, vout, data: *output, block_height, block_time, is_coinbase: true,
+                    };
+                    inner_result = utxo_set.add(outpoint);
+                    if inner_result.is_err() {
+                        break;
+                    }
+                    vout += 1;
+                };
+            is_coinbase = false;
+        } else {
             let fee = match validate_transaction(tx, block_height, block_time, txid, ref utxo_set) {
                 Result::Ok(fee) => fee,
-                Result::Err(err) => { break Result::Err(err); }
+                Result::Err(err) => {
+                    inner_result = Result::Err(err);
+                    break;
+                }
             };
             total_fee += fee;
         }
-
-        i += 1;
     };
 
-    validate_transactions?;
+    inner_result?;
     validate_block_weight(total_weight)?;
 
     let wtxid_root = if block_height >= SEGWIT_BLOCK {