validator/verification: Return an Error if there are failing txs in verifying sets

src/validator/consensus.rs

@@ -34,7 +34,7 @@ use crate::{
         pid::slot_pid_output,
         pow::PoWModule,
         utils::{best_forks_indexes, block_rank, find_extended_fork_index, previous_slot_info},
-        verify_block, verify_proposal, verify_transactions,
+        verify_block, verify_proposal, verify_transactions, TxVerifyFailed,
     },
     Error, Result,
 };
@@ -527,10 +527,13 @@ impl Fork {
         let overlay = self.overlay.lock().unwrap().full_clone()?;
 
         // Verify transactions
-        let erroneous_txs =
-            verify_transactions(&overlay, time_keeper, &unproposed_txs, false).await?;
-        if !erroneous_txs.is_empty() {
-            unproposed_txs.retain(|x| !erroneous_txs.contains(x));
+        if let Err(e) = verify_transactions(&overlay, time_keeper, &unproposed_txs, false).await {
+            match e {
+                crate::Error::TxVerifyFailed(TxVerifyFailed::ErroneousTxs(erroneous_txs)) => {
+                    unproposed_txs.retain(|x| !erroneous_txs.contains(x))
+                }
+                _ => return Err(e),
+            }
         }
 
         Ok(unproposed_txs)

src/validator/mod.rs

@@ -220,10 +220,12 @@ impl Validator {
             let overlay = fork.overlay.lock().unwrap().full_clone()?;
 
             // Verify transaction
-            let erroneous_txs = verify_transactions(&overlay, &time_keeper, &tx_vec, false).await?;
-            if !erroneous_txs.is_empty() {
-                continue
+            match verify_transactions(&overlay, &time_keeper, &tx_vec, false).await {
+                Ok(_) => {}
+                Err(crate::Error::TxVerifyFailed(TxVerifyFailed::ErroneousTxs(_))) => continue,
+                Err(e) => return Err(e),
             }
+
             valid = true;
 
             // Store transaction hash in forks' mempool
@@ -232,9 +234,13 @@ impl Validator {
 
         // Verify transaction against canonical state
         let overlay = BlockchainOverlay::new(&self.blockchain)?;
-        let erroneous_txs = verify_transactions(&overlay, &time_keeper, &tx_vec, false).await?;
-        if erroneous_txs.is_empty() {
-            valid = true
+        let mut erroneous_txs = vec![];
+        match verify_transactions(&overlay, &time_keeper, &tx_vec, false).await {
+            Ok(_) => valid = true,
+            Err(crate::Error::TxVerifyFailed(TxVerifyFailed::ErroneousTxs(etx))) => {
+                erroneous_txs = etx
+            }
+            Err(e) => return Err(e),
         }
 
         // Drop forks lock
@@ -282,11 +288,13 @@ impl Validator {
                 let overlay = fork.overlay.lock().unwrap().full_clone()?;
 
                 // Verify transaction
-                let erroneous_txs =
-                    verify_transactions(&overlay, &time_keeper, &tx_vec, false).await?;
-                if erroneous_txs.is_empty() {
-                    valid = true;
-                    continue
+                match verify_transactions(&overlay, &time_keeper, &tx_vec, false).await {
+                    Ok(_) => {
+                        valid = true;
+                        continue
+                    }
+                    Err(crate::Error::TxVerifyFailed(TxVerifyFailed::ErroneousTxs(_))) => {}
+                    Err(e) => return Err(e),
                 }
 
                 // Remove erroneous transaction from forks' mempool
@@ -295,9 +303,11 @@ impl Validator {
 
             // Verify transaction against canonical state
             let overlay = BlockchainOverlay::new(&self.blockchain)?;
-            let erroneous_txs = verify_transactions(&overlay, &time_keeper, &tx_vec, false).await?;
-            if erroneous_txs.is_empty() {
-                valid = true
+
+            match verify_transactions(&overlay, &time_keeper, &tx_vec, false).await {
+                Ok(_) => valid = true,
+                Err(crate::Error::TxVerifyFailed(TxVerifyFailed::ErroneousTxs(_))) => {}
+                Err(e) => return Err(e),
             }
 
             // Remove pending transaction if it's not valid for canonical or any fork
@@ -480,15 +490,13 @@ impl Validator {
         );
 
         // Verify all transactions and get erroneous ones
-        let erroneous_txs =
-            verify_transactions(&overlay, &time_keeper, txs, self.verify_fees).await?;
-
+        let e = verify_transactions(&overlay, &time_keeper, txs, self.verify_fees).await;
         let lock = overlay.lock().unwrap();
         let mut overlay = lock.overlay.lock().unwrap();
-        if !erroneous_txs.is_empty() {
-            warn!(target: "validator::add_transactions", "Erroneous transactions found in set");
+
+        if let Err(e) = e {
             overlay.purge_new_trees()?;
-            return Err(TxVerifyFailed::ErroneousTxs(erroneous_txs).into())
+            return Err(e)
         }
 
         if !write {

src/validator/verification.rs

@@ -111,11 +111,13 @@ pub async fn verify_genesis_block(
 
     // Verify transactions, exluding producer(first) one
     let txs = &block.txs[1..];
-    let erroneous_txs = verify_transactions(overlay, time_keeper, txs, false).await?;
-    if !erroneous_txs.is_empty() {
-        warn!(target: "validator::verification::verify_genesis_block", "Erroneous transactions found in set");
+    if let Err(e) = verify_transactions(overlay, time_keeper, txs, false).await {
+        warn!(
+            target: "validator::verification::verify_genesis_block",
+            "[VALIDATOR] Erroneous transactions found in set",
+        );
         overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-        return Err(TxVerifyFailed::ErroneousTxs(erroneous_txs).into())
+        return Err(e)
     }
 
     // Insert block
@@ -177,11 +179,14 @@ pub async fn verify_block(
 
     // Verify transactions, exluding producer(first) one
     let txs = &block.txs[1..];
-    let erroneous_txs = verify_transactions(overlay, time_keeper, txs, false).await?;
-    if !erroneous_txs.is_empty() {
-        warn!(target: "validator::verification::verify_block", "Erroneous transactions found in set");
+    let e = verify_transactions(overlay, time_keeper, txs, false).await;
+    if let Err(e) = e {
+        warn!(
+            target: "validator::verification::verify_block",
+            "[VALIDATOR] Erroneous transactions found in set",
+        );
         overlay.lock().unwrap().overlay.lock().unwrap().purge_new_trees()?;
-        return Err(TxVerifyFailed::ErroneousTxs(erroneous_txs).into())
+        return Err(e)
     }
 
     // Insert block
@@ -503,16 +508,16 @@ pub async fn verify_transaction(
         gas_used += runtime.gas_used();
     }
 
-    if verify_fee {
-        // The signature fee is tx_size + fixed_sig_fee * n_signatures
-        gas_used += (PALLAS_SCHNORR_SIGNATURE_FEE * tx.signatures.len() as u64) +
-            serialize_async(tx).await.len() as u64;
+    // The signature fee is tx_size + fixed_sig_fee * n_signatures
+    gas_used += (PALLAS_SCHNORR_SIGNATURE_FEE * tx.signatures.len() as u64) +
+        serialize_async(tx).await.len() as u64;
 
-        // The ZK circuit fee is calculated using a function in validator/fees.rs
-        for zkbin in circuits_to_verify.iter() {
-            gas_used += circuit_gas_use(zkbin);
-        }
+    // The ZK circuit fee is calculated using a function in validator/fees.rs
+    for zkbin in circuits_to_verify.iter() {
+        gas_used += circuit_gas_use(zkbin);
+    }
 
+    if verify_fee {
         // Deserialize the fee call to find the paid fee
         let fee: u64 = match deserialize_async(&tx.calls[fee_call_idx].data.data[1..9]).await {
             Ok(v) => v,
@@ -543,44 +548,53 @@ pub async fn verify_transaction(
     // verify any accompanying ZK proofs.
     debug!(target: "validator::verification::verify_transaction", "Verifying signatures for transaction {}", tx_hash);
     if sig_table.len() != tx.signatures.len() {
-        error!(target: "validator::verification::verify_transaction", "Incorrect number of signatures in tx {}", tx_hash);
+        error!(
+            target: "validator::verification::verify_transaction",
+            "[VALIDATOR] Incorrect number of signatures in tx {}", tx_hash,
+        );
         return Err(TxVerifyFailed::MissingSignatures.into())
     }
 
     if let Err(e) = tx.verify_sigs(sig_table) {
-        error!(target: "validator::verification::verify_transaction", "Signature verification for tx {} failed: {}", tx_hash, e);
+        error!(
+            target: "validator::verification::verify_transaction",
+            "[VALIDATOR] Signature verification for tx {} failed: {}", tx_hash, e,
+        );
         return Err(TxVerifyFailed::InvalidSignature.into())
     }
-
     debug!(target: "validator::verification::verify_transaction", "Signature verification successful");
 
     debug!(target: "validator::verification::verify_transaction", "Verifying ZK proofs for transaction {}", tx_hash);
     if let Err(e) = tx.verify_zkps(verifying_keys, zkp_table).await {
-        error!(target: "validator::verification::verify_transaction", "ZK proof verification for tx {} failed: {}", tx_hash, e);
+        error!(
+            target: "validator::verification::verify_transaction",
+            "[VALIDATOR] ZK proof verification for tx {} failed: {}", tx_hash, e,
+        );
         return Err(TxVerifyFailed::InvalidZkProof.into())
     }
-
     debug!(target: "validator::verification::verify_transaction", "ZK proof verification successful");
-    debug!(target: "validator::verification::verify_transaction", "Transaction {} verified successfully", tx_hash);
 
+    debug!(target: "validator::verification::verify_transaction", "Transaction {} verified successfully", tx_hash);
     Ok(gas_used)
 }
 
 /// Verify a set of [`Transaction`] in sequence and apply them if all are valid.
-/// In case any of the transactions fail, they will be returned to the caller.
+/// In case any of the transactions fail, they will be returned to the caller as an error.
+/// If all transactions are valid, the function will return the accumulated gas used from
+/// all the transactions.
 pub async fn verify_transactions(
     overlay: &BlockchainOverlayPtr,
     time_keeper: &TimeKeeper,
     txs: &[Transaction],
     verify_fees: bool,
-) -> Result<Vec<Transaction>> {
+) -> Result<u64> {
     debug!(target: "validator::verification::verify_transactions", "Verifying {} transactions", txs.len());
 
     // Tracker for failed txs
     let mut erroneous_txs = vec![];
 
     // Gas accumulator
-    let mut _gas_used = 0;
+    let mut gas_used = 0;
 
     // Map of ZK proof verifying keys for the current transaction batch
     let mut vks: HashMap<[u8; 32], HashMap<String, VerifyingKey>> = HashMap::new();
@@ -596,7 +610,7 @@ pub async fn verify_transactions(
     for tx in txs {
         overlay.lock().unwrap().checkpoint();
         match verify_transaction(overlay, time_keeper, tx, &mut vks, verify_fees).await {
-            Ok(gas) => _gas_used += gas,
+            Ok(gas) => gas_used += gas,
             Err(e) => {
                 warn!(target: "validator::verification::verify_transactions", "Transaction verification failed: {}", e);
                 erroneous_txs.push(tx.clone());
@@ -611,7 +625,11 @@ pub async fn verify_transactions(
         }
     }
 
-    Ok(erroneous_txs)
+    if erroneous_txs.is_empty() {
+        Ok(gas_used)
+    } else {
+        Err(TxVerifyFailed::ErroneousTxs(erroneous_txs).into())
+    }
 }
 
 /// Verify given [`Proposal`] against provided consensus state