fix after-execution validation failure case

tari-project · Jun 27, 2024 · 946c6f2 · 946c6f2
1 parent cddca26
commit 946c6f2
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 25 deletions.
diff --git a/applications/tari_dan_wallet_daemon/src/handlers/validator.rs b/applications/tari_dan_wallet_daemon/src/handlers/validator.rs
@@ -108,18 +108,17 @@ pub async fn handle_claim_validator_fees(
 
     let finalized = wait_for_result(&mut events, tx_id).await?;
 
-    if let Some(reject) = finalized.finalize.result.reject() {
+    if let Some(reject) = finalized.finalize.reject() {
         return Err(anyhow::anyhow!("Fee transaction rejected: {}", reject));
     }
-    if let Some(reason) = finalized.finalize.reject() {
+    if let Some(reason) = finalized.finalize.full_reject() {
         return Err(anyhow::anyhow!(
-            "Fee transaction succeeded (fees charged) however the transaction failed: {}",
-            reason
+            "Fee transaction succeeded (fees charged) however the transaction failed: {reason}",
         ));
     }
     info!(
         target: LOG_TARGET,
-        "✅ Transfer transaction {} finalized. Fee: {}",
+        "✅ Claim fee transaction {} finalized. Fee: {}",
         finalized.transaction_id,
         finalized.final_fee
     );

diff --git a/dan_layer/storage/src/consensus_models/executed_transaction.rs b/dan_layer/storage/src/consensus_models/executed_transaction.rs
@@ -12,7 +12,7 @@ use std::{
 use indexmap::IndexSet;
 use serde::{Deserialize, Serialize};
 use tari_dan_common_types::{optional::Optional, SubstateAddress};
-use tari_engine_types::commit_result::{ExecuteResult, FinalizeResult, RejectReason};
+use tari_engine_types::commit_result::ExecuteResult;
 use tari_transaction::{Transaction, TransactionId, VersionedSubstateId};
 
 use crate::{
@@ -128,24 +128,7 @@ impl ExecutedTransaction {
     }
 
     pub fn into_final_result(self) -> Option<ExecuteResult> {
-        self.final_decision().map(|d| {
-            if d.is_commit() {
-                self.result
-            } else {
-                // TODO: We preserve the original result mainly for debugging purposes, but this is a little hacky
-                ExecuteResult {
-                    finalize: FinalizeResult::new_rejected(
-                        self.result.finalize.transaction_hash,
-                        RejectReason::ShardRejected(format!(
-                            "Validators decided to abort: {}",
-                            self.abort_details
-                                .as_deref()
-                                .unwrap_or("<invalid state, no abort details>")
-                        )),
-                    ),
-                }
-            }
-        })
+        TransactionRecord::from(self).into_final_result()
     }
 
     pub fn into_result(self) -> ExecuteResult {

diff --git a/dan_layer/storage/src/consensus_models/transaction.rs b/dan_layer/storage/src/consensus_models/transaction.rs
@@ -131,9 +131,14 @@ impl TransactionRecord {
         //       result. These results are independent of each other.
         self.final_decision().and_then(|d| {
             if d.is_commit() {
+                // Is is expected that the result is ACCEPT.
+                // TODO: Handle (elsewhere) the edge-case where our execution failed but the committee decided to COMMIT
+                // (fetch the state transitions from a peer)
                 self.result
             } else {
-                let finalize_result = self.result.map(|r| r.finalize);
+                // Only use rejected results for the transaction. If execution ACCEPTed but the final decision is ABORT,
+                // then use abort_details (which should have been set in this case).
+                let finalize_result = self.result.map(|r| r.finalize).filter(|f| !f.result.is_accept());
                 Some(ExecuteResult {
                     finalize: finalize_result.unwrap_or_else(|| {
                         FinalizeResult::new_rejected(

diff --git a/integration_tests/tests/features/claim_fees.feature b/integration_tests/tests/features/claim_fees.feature
@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 @claim_fees
 Feature: Claim Fees
+
   @serial @fixed
   Scenario: Claim validator fees
     # Initialize a base node, wallet, miner and VN