reworked execute finalized block flow...all node tests are passing ex…

…cept should_store_finalized_approvals; will investigate further prior to opening PR

execution_engine_testing/test_support/src/wasm_test_builder.rs

@@ -792,14 +792,7 @@ where
             authorization_keys,
             auction_method,
         );
-        let ret = self.data_access_layer().bidding(bidding_req);
-        if let BiddingResult::Success {
-            post_state_hash, ..
-        } = ret
-        {
-            self.post_state_hash = Some(post_state_hash);
-        }
-        ret
+        self.data_access_layer().bidding(bidding_req)
     }
 
     /// Runs an [`ExecuteRequest`].

execution_engine_testing/tests/src/test/system_contracts/auction/distribute.rs

@@ -998,6 +998,7 @@ fn should_distribute_rewards_after_restaking_delegated_funds() {
             },
         );
         assert!(undelegate_result.is_success(), "{:?}", undelegate_result);
+        builder.commit_transforms(builder.get_post_state_hash(), undelegate_result.effects());
         delegator_1_stake =
             get_delegator_staked_amount(&mut builder, VALIDATOR_1.clone(), DELEGATOR_1.clone());
 
@@ -1017,6 +1018,7 @@ fn should_distribute_rewards_after_restaking_delegated_funds() {
             },
         );
         assert!(updelegate_result.is_success(), "{:?}", updelegate_result);
+        builder.commit_transforms(builder.get_post_state_hash(), undelegate_result.effects());
         delegator_2_stake =
             get_delegator_staked_amount(&mut builder, VALIDATOR_1.clone(), DELEGATOR_2.clone());
 
@@ -1044,6 +1046,7 @@ fn should_distribute_rewards_after_restaking_delegated_funds() {
             auction_method,
         );
         assert!(bid_flip_result.is_success(), "{:?}", bid_flip_result);
+        builder.commit_transforms(builder.get_post_state_hash(), undelegate_result.effects());
         validator_stake = get_validator_bid(&mut builder, VALIDATOR_1.clone())
             .expect("should have validator bid")
             .staked_amount();

node/src/components/contract_runtime.rs

@@ -72,8 +72,8 @@ pub(crate) use operations::compute_execution_results_checksum;
 pub use operations::execute_finalized_block;
 use operations::speculatively_execute;
 pub(crate) use types::{
-    BlockAndExecutionResults, ExecutionArtifact, ExecutionPreState, SpeculativeExecutionState,
-    StepEffectsAndUpcomingEraValidators,
+    BlockAndExecutionArtifacts, ExecutionArtifact, ExecutionArtifacts, ExecutionPreState,
+    SpeculativeExecutionState, StepOutcome,
 };
 use utils::{exec_or_requeue, run_intensive_task};
 

node/src/components/contract_runtime/types.rs

@@ -2,15 +2,17 @@ use std::{collections::BTreeMap, sync::Arc};
 
 use datasize::DataSize;
 use serde::Serialize;
+use tracing::{debug, trace};
 
 use casper_storage::{
-    block_store::types::ApprovalsHashes, data_access_layer::EraValidatorsRequest,
+    block_store::types::ApprovalsHashes,
+    data_access_layer::{BalanceHoldResult, BiddingResult, EraValidatorsRequest, TransferResult},
 };
 use casper_types::{
     contract_messages::Messages,
-    execution::{Effects, ExecutionResult},
-    BlockHash, BlockHeaderV2, BlockV2, DeployHash, DeployHeader, Digest, EraId, ProtocolVersion,
-    PublicKey, Timestamp, TransactionHash, TransactionHeader, TransactionV1Hash,
+    execution::{Effects, ExecutionResult, ExecutionResultV2},
+    BlockHash, BlockHeaderV2, BlockV2, DeployHash, DeployHeader, Digest, EraId, InvalidTransaction,
+    ProtocolVersion, PublicKey, Timestamp, TransactionHash, TransactionHeader, TransactionV1Hash,
     TransactionV1Header, U512,
 };
 
@@ -56,13 +58,218 @@ impl From<ValidatorWeightsByEraIdRequest> for EraValidatorsRequest {
 
 /// Effects from running step and the next era validators that are gathered when an era ends.
 #[derive(Clone, Debug, DataSize)]
-pub(crate) struct StepEffectsAndUpcomingEraValidators {
+pub(crate) struct StepOutcome {
     /// Validator sets for all upcoming eras that have already been determined.
     pub(crate) upcoming_era_validators: BTreeMap<EraId, BTreeMap<PublicKey, U512>>,
     /// An [`Effects`] created by an era ending.
     pub(crate) step_effects: Effects,
 }
 
+pub(crate) struct ExecutionArtifacts {
+    artifacts: Vec<ExecutionArtifact>,
+}
+
+pub(crate) enum ExecutionArtifactOutcome {
+    RootNotFound,
+    Failure,
+    Success(Effects),
+}
+
+impl ExecutionArtifacts {
+    pub fn with_capacity(capacity: usize) -> Self {
+        let artifacts = Vec::with_capacity(capacity);
+        ExecutionArtifacts { artifacts }
+    }
+
+    pub fn push(&mut self, execution_artifact: ExecutionArtifact) {
+        self.artifacts.push(execution_artifact);
+    }
+
+    pub fn push_invalid_transaction(
+        &mut self,
+        transaction_hash: TransactionHash,
+        transaction_header: TransactionHeader,
+        invalid_transaction: InvalidTransaction,
+    ) {
+        self.push_failure(
+            transaction_hash,
+            transaction_header,
+            format!("{:?}", invalid_transaction),
+        );
+    }
+
+    pub fn push_auction_method_failure(
+        &mut self,
+        transaction_hash: TransactionHash,
+        transaction_header: TransactionHeader,
+        cost: U512,
+    ) {
+        let msg = "failed to resolve auction method".to_string();
+        let artifact = ExecutionArtifact::new(
+            transaction_hash,
+            transaction_header,
+            ExecutionResult::V2(ExecutionResultV2::Failure {
+                effects: Effects::new(),
+                transfers: vec![],
+                error_message: msg.clone(),
+                cost,
+            }),
+            Messages::default(),
+        );
+        debug!(%transaction_hash, "{:?}", msg);
+        self.artifacts.push(artifact);
+    }
+
+    pub fn push_transfer_result(
+        &mut self,
+        transaction_hash: TransactionHash,
+        transaction_header: TransactionHeader,
+        transfer_result: TransferResult,
+        cost: U512,
+    ) -> ExecutionArtifactOutcome {
+        trace!(
+            ?transaction_hash,
+            ?transfer_result,
+            "native transfer result"
+        );
+        match transfer_result {
+            TransferResult::RootNotFound => ExecutionArtifactOutcome::RootNotFound,
+            TransferResult::Failure(transfer_error) => {
+                self.push_failure(
+                    transaction_hash,
+                    transaction_header,
+                    format!("{:?}", transfer_error),
+                );
+                debug!(%transfer_error);
+                ExecutionArtifactOutcome::Failure
+            }
+            TransferResult::Success {
+                effects: transfer_effects,
+                transfers,
+            } => {
+                self.artifacts.push(ExecutionArtifact::new(
+                    transaction_hash,
+                    transaction_header,
+                    ExecutionResult::V2(ExecutionResultV2::Success {
+                        effects: transfer_effects.clone(),
+                        cost,
+                        transfers,
+                    }),
+                    Messages::default(),
+                ));
+                ExecutionArtifactOutcome::Success(transfer_effects)
+            }
+        }
+    }
+
+    pub fn push_bidding_result(
+        &mut self,
+        transaction_hash: TransactionHash,
+        transaction_header: TransactionHeader,
+        bidding_result: BiddingResult,
+        cost: U512,
+    ) -> ExecutionArtifactOutcome {
+        trace!(?transaction_hash, ?bidding_result, "bidding result");
+        match bidding_result {
+            BiddingResult::RootNotFound => ExecutionArtifactOutcome::RootNotFound,
+            BiddingResult::Failure(tce) => {
+                self.artifacts.push(ExecutionArtifact::new(
+                    transaction_hash,
+                    transaction_header,
+                    ExecutionResult::V2(ExecutionResultV2::Failure {
+                        effects: Effects::new(),
+                        cost,
+                        transfers: vec![],
+                        error_message: format!("{:?}", tce),
+                    }),
+                    Messages::default(),
+                ));
+                debug!(%tce);
+                ExecutionArtifactOutcome::Failure
+            }
+            BiddingResult::Success {
+                effects: bidding_effects,
+                ..
+            } => {
+                self.artifacts.push(ExecutionArtifact::new(
+                    transaction_hash,
+                    transaction_header,
+                    ExecutionResult::V2(ExecutionResultV2::Success {
+                        effects: bidding_effects.clone(),
+                        cost,
+                        transfers: vec![],
+                    }),
+                    Messages::default(),
+                ));
+                ExecutionArtifactOutcome::Success(bidding_effects)
+            }
+        }
+    }
+
+    pub fn push_hold_result(
+        &mut self,
+        transaction_hash: TransactionHash,
+        transaction_header: TransactionHeader,
+        hold_result: BalanceHoldResult,
+    ) -> ExecutionArtifactOutcome {
+        trace!(?transaction_hash, ?hold_result, "balance hold result");
+        if hold_result.is_root_not_found() {
+            return ExecutionArtifactOutcome::RootNotFound;
+        }
+        if !hold_result.is_fully_covered() {
+            let error_message = hold_result.error_message();
+            self.push_failure(transaction_hash, transaction_header, error_message.clone());
+            debug!(%error_message);
+            ExecutionArtifactOutcome::Failure
+        } else {
+            let hold_cost = U512::zero(); // we don't charge for the hold itself.
+            let hold_effects = hold_result.effects();
+            self.artifacts.push(ExecutionArtifact::new(
+                transaction_hash,
+                transaction_header,
+                ExecutionResult::V2(ExecutionResultV2::Success {
+                    effects: hold_effects.clone(),
+                    transfers: vec![],
+                    cost: hold_cost,
+                }),
+                Messages::default(),
+            ));
+            ExecutionArtifactOutcome::Success(hold_effects)
+        }
+    }
+
+    fn push_failure(
+        &mut self,
+        transaction_hash: TransactionHash,
+        transaction_header: TransactionHeader,
+        error_message: String,
+    ) {
+        let execution_artifact = ExecutionArtifact::new(
+            transaction_hash,
+            transaction_header,
+            ExecutionResult::V2(ExecutionResultV2::Failure {
+                effects: Effects::new(),
+                cost: U512::zero(),
+                transfers: vec![],
+                error_message,
+            }),
+            Messages::default(),
+        );
+        self.artifacts.push(execution_artifact);
+    }
+
+    pub fn execution_results(&self) -> Vec<&ExecutionResult> {
+        self.artifacts
+            .iter()
+            .map(|artifact| &artifact.execution_result)
+            .collect::<Vec<&ExecutionResult>>()
+    }
+
+    pub fn take(self) -> Vec<ExecutionArtifact> {
+        self.artifacts
+    }
+}
+
 #[derive(Clone, Debug, DataSize, PartialEq, Eq, Serialize)]
 pub(crate) struct ExecutionArtifact {
     pub(crate) transaction_hash: TransactionHash,
@@ -120,16 +327,15 @@ impl ExecutionArtifact {
 /// A [`Block`] that was the result of execution in the `ContractRuntime` along with any execution
 /// effects it may have.
 #[derive(Clone, Debug, DataSize)]
-pub struct BlockAndExecutionResults {
+pub struct BlockAndExecutionArtifacts {
     /// The [`Block`] the contract runtime executed.
     pub(crate) block: Arc<BlockV2>,
     /// The [`ApprovalsHashes`] for the deploys in this block.
     pub(crate) approvals_hashes: Box<ApprovalsHashes>,
-    /// The results from executing the deploys in the block.
-    pub(crate) execution_results: Vec<ExecutionArtifact>,
+    /// The results from executing the transactions in the block.
+    pub(crate) execution_artifacts: Vec<ExecutionArtifact>,
     /// The [`Effects`] and the upcoming validator sets determined by the `step`
-    pub(crate) maybe_step_effects_and_upcoming_era_validators:
-        Option<StepEffectsAndUpcomingEraValidators>,
+    pub(crate) step_outcome: Option<StepOutcome>,
 }
 
 #[derive(DataSize, Debug, Clone, Serialize)]

node/src/components/contract_runtime/utils.rs

@@ -3,7 +3,7 @@ use crate::{
         exec_queue::{ExecQueue, QueueItem},
         execute_finalized_block,
         metrics::Metrics,
-        rewards, BlockAndExecutionResults, ExecutionPreState, StepEffectsAndUpcomingEraValidators,
+        rewards, BlockAndExecutionArtifacts, ExecutionPreState, StepOutcome,
     },
     effect::{
         announcements::{ContractRuntimeAnnouncement, FatalAnnouncement, MetaBlockAnnouncement},
@@ -99,11 +99,11 @@ pub(super) async fn exec_or_requeue<REv>(
         });
     }
 
-    let BlockAndExecutionResults {
+    let BlockAndExecutionArtifacts {
         block,
         approvals_hashes,
-        execution_results,
-        maybe_step_effects_and_upcoming_era_validators,
+        execution_artifacts: execution_results,
+        step_outcome: maybe_step_effects_and_upcoming_era_validators,
     } = match run_intensive_task(move || {
         debug!("ContractRuntime: execute_finalized_block");
         execute_finalized_block(
@@ -149,7 +149,7 @@ pub(super) async fn exec_or_requeue<REv>(
 
     let current_era_id = block.era_id();
 
-    if let Some(StepEffectsAndUpcomingEraValidators {
+    if let Some(StepOutcome {
         step_effects,
         mut upcoming_era_validators,
     }) = maybe_step_effects_and_upcoming_era_validators

node/src/reactor/main_reactor/tests.rs

@@ -1311,11 +1311,11 @@ async fn should_store_finalized_approvals() {
     // Run until the transaction gets executed.
     let has_stored_exec_results = |nodes: &Nodes| {
         nodes.values().all(|runner| {
-            runner
+            let read = runner
                 .main_reactor()
                 .storage()
-                .read_execution_result(&transaction_hash)
-                .is_some()
+                .read_execution_result(&transaction_hash);
+            read.is_some()
         })
     };
     fixture.run_until(has_stored_exec_results, ONE_MIN).await;

storage/src/data_access_layer/balance.rs

@@ -290,4 +290,14 @@ impl BalanceResult {
             _ => None,
         }
     }
+
+    /// Is the available balance sufficient to cover the cost?
+    pub fn is_sufficient(&self, cost: U512) -> bool {
+        match self {
+            BalanceResult::RootNotFound | BalanceResult::Failure(_) => false,
+            BalanceResult::Success {
+                available_balance, ..
+            } => available_balance >= &cost,
+        }
+    }
 }

storage/src/data_access_layer/balance_hold.rs

@@ -236,6 +236,7 @@ impl Display for BalanceHoldError {
 }
 
 /// Result enum that represents all possible outcomes of a balance hold request.
+#[derive(Debug)]
 pub enum BalanceHoldResult {
     /// Returned if a passed state root hash is not found.
     RootNotFound,