Only enforce block_spend_limit in V15 during finalization

Author: vicsn

synthesizer/src/vm/finalize.rs

@@ -61,7 +61,7 @@ type AbortReason = String;
 type ComputeSpend = u64;
 /// An intermediary speculation artifact indicating whether a transaction should
 /// be aborted or finalized with a certain compute_spend.
-enum PrepareSpeculateResult {
+enum ShouldAbortResult {
     Abort(AbortReason),
     Finalize(ComputeSpend),
 }
@@ -402,7 +402,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
             // Accumulate per-block uniqueness checks.
             let mut candidate_transaction_details = CandidateTransactionDetails::<N>::default();
             // Accumulate per-block spend.
-            let mut block_spend = 0;
+            let mut block_spend = 0u64;
             // Determine the transaction spend limit. These unwraps are safe, see tests in consensus_heights.rs
             let consensus_version = N::CONSENSUS_VERSION(state.block_height()).unwrap();
             let transaction_spend_limit =
@@ -425,19 +425,29 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
                 match self.should_abort_transaction(
                     &transaction,
                     &candidate_transaction_details,
-                    block_spend,
                     transaction_spend_limit,
-                    block_spend_limit,
                     consensus_version,
                 ) {
-                    PrepareSpeculateResult::Abort(abort_reason) => {
+                    ShouldAbortResult::Abort(abort_reason) => {
                         // Store the aborted transaction.
                         aborted.push((transaction.clone(), abort_reason));
                         // Continue to the next transaction.
                         continue 'outer;
                     }
-                    PrepareSpeculateResult::Finalize(compute_spend) => {
+                    ShouldAbortResult::Finalize(compute_spend) => {
+                        // If the consensus version is >= V15, ensure that the
+                        // transaction is not exceeding block spend limits.
                         if consensus_version >= ConsensusVersion::V15 {
+                            if let Some(block_spend_limit) = block_spend_limit {
+                                if block_spend.saturating_add(compute_spend) > block_spend_limit {
+                                    aborted.push((
+                                        transaction.clone(),
+                                        format!("Exceeds the block spend limit with compute_spend: '{compute_spend}'"),
+                                    ));
+                                    // Continue to the next transaction.
+                                    continue 'outer;
+                                }
+                            }
                             // Track the compute_spend used so far.
                             block_spend = block_spend.saturating_add(compute_spend);
                         }
@@ -937,7 +947,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
         finalize_result
     }
 
-    /// Returns PrepareSpeculateResult.
+    /// Returns ShouldAbortResult.
     ///
     /// The transaction will be aborted if any of the following conditions are met:
     /// - The transaction is producing a duplicate transition
@@ -954,14 +964,12 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
         &self,
         transaction: &Transaction<N>,
         candidate_transaction_details: &CandidateTransactionDetails<N>,
-        block_spend: u64,
         transaction_spend_limit: u64,
-        block_spend_limit: Option<u64>,
         consensus_version: ConsensusVersion,
-    ) -> PrepareSpeculateResult {
+    ) -> ShouldAbortResult {
         // Ensure that the transaction is not a fee transaction.
         if let Transaction::Fee(..) = transaction {
-            return PrepareSpeculateResult::Abort("Fee transactions are not allowed in speculate".to_string());
+            return ShouldAbortResult::Abort("Fee transactions are not allowed in speculate".to_string());
         }
 
         // Ensure that:
@@ -974,11 +982,11 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
             if candidate_transaction_details.transition_ids.contains(transition_id)
                 || self.transition_store().contains_transition_id(transition_id).unwrap_or(true)
             {
-                return PrepareSpeculateResult::Abort(format!("Duplicate transition {transition_id}"));
+                return ShouldAbortResult::Abort(format!("Duplicate transition {transition_id}"));
             }
             // If the transition's program is being deployed or redeployed in this block, abort the transaction.
             if candidate_transaction_details.deployments.contains(transition.program_id()) {
-                return PrepareSpeculateResult::Abort(format!(
+                return ShouldAbortResult::Abort(format!(
                     "Program {} is being deployed or redeployed in this block",
                     transition.program_id()
                 ));
@@ -991,7 +999,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
             if candidate_transaction_details.input_ids.contains(input_id)
                 || self.transition_store().contains_input_id(input_id).unwrap_or(true)
             {
-                return PrepareSpeculateResult::Abort(format!("Double-spending input {input_id}"));
+                return ShouldAbortResult::Abort(format!("Double-spending input {input_id}"));
             }
         }
 
@@ -1001,7 +1009,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
             if candidate_transaction_details.output_ids.contains(output_id)
                 || self.transition_store().contains_output_id(output_id).unwrap_or(true)
             {
-                return PrepareSpeculateResult::Abort(format!("Duplicate output {output_id}"));
+                return ShouldAbortResult::Abort(format!("Duplicate output {output_id}"));
             }
         }
 
@@ -1012,7 +1020,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
             if candidate_transaction_details.tpks.contains(tpk)
                 || self.transition_store().contains_tpk(tpk).unwrap_or(true)
             {
-                return PrepareSpeculateResult::Abort(format!("Duplicate transition public key {tpk}"));
+                return ShouldAbortResult::Abort(format!("Duplicate transition public key {tpk}"));
             }
         }
 
@@ -1021,7 +1029,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
             // If any public deployment payer has already deployed in this block, abort the transaction.
             if let Some(payer) = fee.payer() {
                 if candidate_transaction_details.deployment_payers.contains(&payer) {
-                    return PrepareSpeculateResult::Abort(format!(
+                    return ShouldAbortResult::Abort(format!(
                         "Another deployment in the block from the same public fee payer {payer}"
                     ));
                 }
@@ -1030,7 +1038,7 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
 
         // Before V15, we return without tracking any compute spend.
         if consensus_version < ConsensusVersion::V15 {
-            PrepareSpeculateResult::Finalize(0)
+            ShouldAbortResult::Finalize(0)
         // If the consensus version is >= V15, ensure that the transaction is not exceeding spend limits.
         } else {
             // Compute microcredit spend from deployment or execution cost details.
@@ -1039,37 +1047,28 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
                     match deployment_cost(self.process(), deployment, consensus_version) {
                         Ok((_, cost_details)) => deploy_compute_cost_in_microcredits(cost_details, consensus_version),
                         Err(e) => {
-                            return PrepareSpeculateResult::Abort(format!(
-                                "Failed to compute the deployment cost: {e}"
-                            ));
+                            return ShouldAbortResult::Abort(format!("Failed to compute the deployment cost: {e}"));
                         }
                     }
                 }
                 Transaction::Execute(_, _, execution, _) => {
                     match execution_cost(self.process(), execution, consensus_version) {
                         Ok((_, cost_details)) => execute_compute_cost_in_microcredits(cost_details, consensus_version),
                         Err(e) => {
-                            return PrepareSpeculateResult::Abort(format!("Failed to compute the execution cost: {e}"));
+                            return ShouldAbortResult::Abort(format!("Failed to compute the execution cost: {e}"));
                         }
                     }
                 }
                 Transaction::Fee(..) => 0, // Fee transactions are already aborted above and don't contribute compute spend.
             };
 
             if compute_spend > transaction_spend_limit {
-                return PrepareSpeculateResult::Abort(format!(
+                return ShouldAbortResult::Abort(format!(
                     "Exceeds the transaction spend limit with compute_spend: '{compute_spend}'"
                 ));
             }
-            if let Some(block_spend_limit) = block_spend_limit {
-                if block_spend.saturating_add(compute_spend) > block_spend_limit {
-                    return PrepareSpeculateResult::Abort(format!(
-                        "Exceeds the block spend limit with compute_spend: '{compute_spend}'"
-                    ));
-                }
-            }
 
-            PrepareSpeculateResult::Finalize(compute_spend)
+            ShouldAbortResult::Finalize(compute_spend)
         }
     }
 
@@ -1092,34 +1091,26 @@ impl<N: Network, C: ConsensusStorage<N>> VM<N, C> {
 
         // Accumulate per-block uniqueness checks.
         let mut candidate_transaction_details = CandidateTransactionDetails::<N>::default();
-        // Accumulate per-block spend.
-        let mut block_spend = 0;
         // Determine the transaction spend limit. These unwraps are safe, see tests in consensus_heights.rs
         let consensus_version = N::CONSENSUS_VERSION(state.block_height()).unwrap();
         let transaction_spend_limit =
             consensus_config_value_by_version!(N, TRANSACTION_SPEND_LIMIT, consensus_version).unwrap();
-        // Determine the block spend limit.
-        let block_spend_limit = state.block_spend_limit();
 
         // Abort duplicate, overspending, invalid, or disallowed transactions before verification.
         for transaction in transactions.iter() {
             match self.should_abort_transaction(
                 transaction,
                 &candidate_transaction_details,
-                block_spend,
                 transaction_spend_limit,
-                block_spend_limit,
                 consensus_version,
             ) {
-                PrepareSpeculateResult::Abort(abort_reason) => {
+                ShouldAbortResult::Abort(abort_reason) => {
                     // Store the aborted transaction.
                     aborted_transactions.push((*transaction, abort_reason));
                 }
-                PrepareSpeculateResult::Finalize(compute_spend) => {
-                    if consensus_version >= ConsensusVersion::V15 {
-                        // Track the compute_spend used so far.
-                        block_spend = block_spend.saturating_add(compute_spend);
-                    }
+                // We do not further track the compute spend here, to not count
+                // aborted transactions towards the block spend limit.
+                ShouldAbortResult::Finalize(_compute_spend) => {
                     // Track the accepted transaction details.
                     candidate_transaction_details.record_accepted_transaction(transaction);
                     // Mark the transaction ready to verify.