diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index 340d85529d9..e3c66bd932a 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -127,6 +127,7 @@ use fuel_core_types::{ executor::{ Error as ExecutorError, Event as ExecutorEvent, + ExecutionResult, ForcedTransactionFailure, Result as ExecutorResult, TransactionExecutionResult, @@ -248,13 +249,34 @@ where let ExecutionData { message_ids, event_inbox_root, + changes, + events, + tx_status, + skipped_transactions, + coinbase, + used_gas, .. - } = &execution_data; + } = execution_data; let block = partial_block - .generate(&message_ids[..], *event_inbox_root) + .generate(&message_ids[..], event_inbox_root) .map_err(ExecutorError::BlockHeaderError)?; - Ok(Self::uncommitted_block_result(block, execution_data)) + + let finalized_block_id = block.id(); + + debug!( + "Block {:#x} fees: {} gas: {}", + finalized_block_id, coinbase, used_gas + ); + + let result = ExecutionResult { + block, + skipped_transactions, + tx_status, + events, + }; + + Ok(UncommittedResult::new(result, changes)) } pub fn validate_without_commit( @@ -264,42 +286,15 @@ where let consensus_params_version = block.header().consensus_parameters_version; let (block_executor, storage_tx) = self.into_executor(consensus_params_version)?; - let execution_data = block_executor.validate_block(block, storage_tx)?; - Ok(Self::uncommitted_validation_result(block, execution_data)) - } - - fn into_executor( - self, - consensus_params_version: ConsensusParametersVersion, - ) -> ExecutorResult<(BlockExecutor, StorageTransaction)> { - let storage_tx = self - .database - .into_transaction() - .with_policy(ConflictPolicy::Overwrite); - let consensus_params = storage_tx - .storage::() - .get(&consensus_params_version)? - .ok_or(ExecutorError::ConsensusParametersNotFound( - consensus_params_version, - ))? - .into_owned(); - let executor = BlockExecutor::new(self.relayer, self.options, consensus_params)?; - Ok((executor, storage_tx)) - } - fn uncommitted_validation_result( - block: &Block, - data: ExecutionData, - ) -> UncommittedValidationResult { let ExecutionData { coinbase, used_gas, tx_status, - skipped_transactions, events, changes, .. - } = data; + } = block_executor.validate_block(block, storage_tx)?; let finalized_block_id = block.id(); @@ -308,21 +303,28 @@ where finalized_block_id, coinbase, used_gas ); - let result = ValidationResult { - skipped_transactions, - tx_status, - events, - }; + let result = ValidationResult { tx_status, events }; - UncommittedValidationResult::new(result, changes) + Ok(UncommittedValidationResult::new(result, changes)) } - fn uncommitted_block_result( - block: Block, - data: ExecutionData, - ) -> UncommittedResult { - let result = Self::uncommitted_validation_result(&block, data); - result.into_execution_result(block) + fn into_executor( + self, + consensus_params_version: ConsensusParametersVersion, + ) -> ExecutorResult<(BlockExecutor, StorageTransaction)> { + let storage_tx = self + .database + .into_transaction() + .with_policy(ConflictPolicy::Overwrite); + let consensus_params = storage_tx + .storage::() + .get(&consensus_params_version)? + .ok_or(ExecutorError::ConsensusParametersNotFound( + consensus_params_version, + ))? + .into_owned(); + let executor = BlockExecutor::new(self.relayer, self.options, consensus_params)?; + Ok((executor, storage_tx)) } } diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs index 60bbced956e..cef50bc30a2 100644 --- a/crates/services/importer/src/importer.rs +++ b/crates/services/importer/src/importer.rs @@ -87,10 +87,6 @@ pub enum Error { FailedVerification(anyhow::Error), #[display(fmt = "The execution of the block failed: {_0}.")] FailedExecution(executor::Error), - #[display( - fmt = "It is not possible to skip transactions during importing of the block." - )] - SkippedTransactionsNotEmpty, #[display(fmt = "It is not possible to execute the genesis block.")] ExecuteGenesis, #[display(fmt = "The database already contains the data at the height {_0}.")] @@ -400,23 +396,11 @@ where return Err(Error::ExecuteGenesis) } - let ( - ValidationResult { - skipped_transactions, - tx_status, - events, - }, - changes, - ) = executor + let (ValidationResult { tx_status, events }, changes) = executor .validate(&block) .map_err(Error::FailedExecution)? .into(); - // If we skipped transaction, it means that the block is invalid. - if !skipped_transactions.is_empty() { - return Err(Error::SkippedTransactionsNotEmpty) - } - let actual_block_id = block.id(); if actual_block_id != sealed_block_id { // It should not be possible because, during validation, we don't touch the block. diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index c8a2efacc88..3b07d82ebbe 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -22,7 +22,6 @@ use fuel_core_types::{ consensus::Consensus, SealedBlock, }, - fuel_tx::TxId, fuel_types::BlockHeight, services::{ block_importer::{ @@ -32,6 +31,7 @@ use fuel_core_types::{ executor::{ Error as ExecutorError, Result as ExecutorResult, + UncommittedValidationResult, ValidationResult, }, Uncommitted, @@ -67,11 +67,6 @@ fn u32_to_merkle_root(number: u32) -> MerkleRoot { MerkleRoot::from(root) } -#[derive(Clone, Debug)] -struct MockExecutionResult { - skipped_transactions: usize, -} - fn genesis(height: u32) -> SealedBlock { let mut block = Block::default(); block.header_mut().set_block_height(height.into()); @@ -144,10 +139,14 @@ fn storage_failure_error() -> Error { storage_failure::<()>().unwrap_err().into() } -fn ex_result(skipped_transactions: usize) -> MockExecutionResult { - MockExecutionResult { - skipped_transactions, - } +fn ex_result() -> ExecutorResult> { + Ok(Uncommitted::new( + ValidationResult { + tx_status: vec![], + events: vec![], + }, + Default::default(), + )) } fn execution_failure() -> ExecutorResult { @@ -160,23 +159,10 @@ fn execution_failure_error() -> Error { fn executor(result: R) -> MockValidator where - R: Fn() -> ExecutorResult + Send + 'static, + R: Fn() -> ExecutorResult> + Send + 'static, { let mut executor = MockValidator::default(); - executor.expect_validate().return_once(move |_| { - let mock_result = result()?; - let skipped_transactions: Vec<_> = (0..mock_result.skipped_transactions) - .map(|_| (TxId::zeroed(), ExecutorError::BlockMismatch)) - .collect(); - Ok(Uncommitted::new( - ValidationResult { - skipped_transactions, - tx_status: vec![], - events: vec![], - }, - Default::default(), - )) - }); + executor.expect_validate().return_once(move |_| result()); executor } @@ -335,7 +321,7 @@ async fn commit_result_and_execute_and_commit_poa( let execute_and_commit_result = execute_and_commit_assert( sealed_block, db, - executor(ok(ex_result(0))), + executor(ex_result), verifier(ok(())), ) .await; @@ -445,17 +431,17 @@ fn one_lock_at_the_same_time() { ///////// New block, Block After Execution, Verification result, commits ///////// #[test_case( - genesis(113), ok(ex_result(0)), ok(()), 0 + genesis(113), ex_result, ok(()), 0 => Err(Error::ExecuteGenesis); "cannot execute genesis block" )] #[test_case( - poa_block(1), ok(ex_result(0)), ok(()), 1 + poa_block(1), ex_result, ok(()), 1 => Ok(()); "commits block 1" )] #[test_case( - poa_block(113), ok(ex_result(0)), ok(()), 1 + poa_block(113), ex_result, ok(()), 1 => Ok(()); "commits block 113" )] @@ -465,12 +451,7 @@ fn one_lock_at_the_same_time() { "commit fails if execution fails" )] #[test_case( - poa_block(113), ok(ex_result(1)), ok(()), 0 - => Err(Error::SkippedTransactionsNotEmpty); - "commit fails if there are skipped transactions" -)] -#[test_case( - poa_block(113), ok(ex_result(0)), verification_failure, 0 + poa_block(113), ex_result, verification_failure, 0 => Err(verification_failure_error()); "commit fails if verification fails" )] @@ -482,7 +463,10 @@ async fn execute_and_commit_and_verify_and_execute_block_poa( commits: usize, ) -> Result<(), Error> where - P: Fn() -> ExecutorResult + Send + Clone + 'static, + P: Fn() -> ExecutorResult> + + Send + + Clone + + 'static, V: Fn() -> anyhow::Result<()> + Send + Clone + 'static, { // `execute_and_commit` and `verify_and_execute_block` should have the same @@ -518,7 +502,7 @@ fn verify_and_execute_assert( verifier_result: V, ) -> Result<(), Error> where - P: Fn() -> ExecutorResult + Send + 'static, + P: Fn() -> ExecutorResult> + Send + 'static, V: Fn() -> anyhow::Result<()> + Send + 'static, { let importer = Importer::default_config( @@ -534,7 +518,7 @@ where fn verify_and_execute_allowed_when_locked() { let importer = Importer::default_config( MockDatabase::default(), - executor(ok(ex_result(0))), + executor(ex_result), verifier(ok(())), ); diff --git a/crates/services/upgradable-executor/wasm-executor/src/main.rs b/crates/services/upgradable-executor/wasm-executor/src/main.rs index 792cf88073b..6065cae9ea2 100644 --- a/crates/services/upgradable-executor/wasm-executor/src/main.rs +++ b/crates/services/upgradable-executor/wasm-executor/src/main.rs @@ -28,7 +28,6 @@ use fuel_core_types::{ Error as ExecutorError, ExecutionResult, Result as ExecutorResult, - ValidationResult, }, Uncommitted, }, @@ -116,26 +115,13 @@ fn execute_validation( instance: ExecutionInstance, block: Block, ) -> ExecutorResult> { - let ( - ValidationResult { - skipped_transactions, - tx_status, - events, - }, - changes, - ) = instance.validate_without_commit(&block)?.into(); + let result = instance + .validate_without_commit(&block)? + .into_execution_result(block, vec![]); // TODO: Modify return type to differentiate between validation and production results // https://github.com/FuelLabs/fuel-core/issues/1887 - Ok(Uncommitted::new( - ExecutionResult { - block, - skipped_transactions, - tx_status, - events, - }, - changes, - )) + Ok(result) } fn use_wasm_tx_source(component: Components<()>) -> Components { diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index 8f1bf4a0301..e3fe02c9a59 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -63,9 +63,6 @@ pub struct ExecutionResult { #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Debug)] pub struct ValidationResult { - /// The list of skipped transactions with corresponding errors. Those transactions were - /// not included in the block and didn't affect the state of the blockchain. - pub skipped_transactions: Vec<(TxId, Error)>, /// The status of the transactions execution included into the block. pub tx_status: Vec, /// The list of all events generated during the execution of the block. @@ -77,14 +74,10 @@ impl UncommittedValidationResult { pub fn into_execution_result( self, block: Block, + skipped_transactions: Vec<(TxId, Error)>, ) -> UncommittedResult { let Self { - result: - ValidationResult { - skipped_transactions, - tx_status, - events, - }, + result: ValidationResult { tx_status, events }, changes, } = self; UncommittedResult::new( @@ -105,23 +98,12 @@ impl UncommittedResult { self, ) -> UncommittedValidationResult { let Self { - result: - ExecutionResult { - skipped_transactions, - tx_status, - events, - .. - }, - changes, - } = self; - UncommittedValidationResult::new( - ValidationResult { - skipped_transactions, - tx_status, - events, + result: ExecutionResult { + tx_status, events, .. }, changes, - ) + } = self; + UncommittedValidationResult::new(ValidationResult { tx_status, events }, changes) } }