diff --git a/CHANGELOG.md b/CHANGELOG.md index 170679d0bea..a958c0222d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ### Changed +- [#1886](https://github.com/FuelLabs/fuel-core/pull/1886): Use ref to `Block` in validation code - [#1876](https://github.com/FuelLabs/fuel-core/pull/1876): Updated benchmark to include the worst scenario for `CROO` opcode. Also include consensus parameters in bench output. ### Changed diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 1689d15b3da..5ba4e5b02d8 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -336,7 +336,7 @@ mod tests { .. } = producer.produce_and_commit(block.into()).unwrap(); - let validation_result = verifier.validate(block); + let validation_result = verifier.validate(&block); assert!(validation_result.is_ok()); assert!(skipped_transactions.is_empty()); } @@ -677,7 +677,7 @@ mod tests { let producer = create_executor(database.clone(), config.clone()); let ExecutionResult { - block: produced_block, + block, skipped_transactions, .. } = producer @@ -690,18 +690,15 @@ mod tests { .unwrap() .into_result(); assert!(skipped_transactions.is_empty()); - let produced_txs = produced_block.transactions().to_vec(); + let produced_txs = block.transactions().to_vec(); let mut validator = create_executor( Default::default(), // Use the same config as block producer config, ); - let ExecutionResult { - block: validated_block, - .. - } = validator.validate_and_commit(produced_block).unwrap(); - assert_eq!(validated_block.transactions(), produced_txs); + let _ = validator.validate_and_commit(&block).unwrap(); + assert_eq!(block.transactions(), produced_txs); let ContractBalance { asset_id, amount, .. } = validator @@ -822,7 +819,7 @@ mod tests { }, ); let validation_err = validator - .validate_and_commit(block) + .validate_and_commit(&block) .expect_err("Expected error because coinbase if invalid"); assert!(matches!( validation_err, @@ -848,7 +845,7 @@ mod tests { let mut validator = create_executor(Default::default(), Default::default()); let validation_err = validator - .validate_and_commit(block) + .validate_and_commit(&block) .expect_err("Expected error because coinbase if invalid"); assert!(matches!( validation_err, @@ -862,7 +859,7 @@ mod tests { let mut validator = create_executor(Default::default(), Default::default()); let validation_err = validator - .validate_and_commit(block) + .validate_and_commit(&block) .expect_err("Expected error because coinbase is missing"); assert!(matches!(validation_err, ExecutorError::MintMissing)); } @@ -884,7 +881,7 @@ mod tests { let mut validator = create_executor(Default::default(), Default::default()); let validation_err = validator - .validate_and_commit(block) + .validate_and_commit(&block) .expect_err("Expected error because coinbase if invalid"); assert!(matches!( @@ -919,7 +916,7 @@ mod tests { }; let mut validator = create_executor(Default::default(), config); let validation_err = validator - .validate_and_commit(block) + .validate_and_commit(&block) .expect_err("Expected error because coinbase if invalid"); assert!(matches!( @@ -947,7 +944,7 @@ mod tests { let mut validator = create_executor(Default::default(), Default::default()); let validation_err = validator - .validate_and_commit(block) + .validate_and_commit(&block) .expect_err("Expected error because coinbase if invalid"); assert!(matches!( validation_err, @@ -991,7 +988,7 @@ mod tests { let ExecutionResult { skipped_transactions, - block, + mut block, .. } = producer .produce_without_commit(block) @@ -1008,15 +1005,12 @@ mod tests { )); // Produced block is valid - let ExecutionResult { mut block, .. } = verifier - .validate_without_commit(block) - .unwrap() - .into_result(); + let _ = verifier.validate(&block).unwrap().into_result(); // Invalidate the block with Insufficient tx let len = block.transactions().len(); block.transactions_mut().insert(len - 1, tx); - let verify_result = verifier.validate_without_commit(block); + let verify_result = verifier.validate(&block); assert!(matches!( verify_result, Err(ExecutorError::InvalidTransaction( @@ -1043,7 +1037,7 @@ mod tests { let ExecutionResult { skipped_transactions, - block, + mut block, .. } = producer .produce_without_commit(block) @@ -1056,17 +1050,14 @@ mod tests { )); // Produced block is valid - let ExecutionResult { mut block, .. } = verifier - .validate_without_commit(block) - .unwrap() - .into_result(); + let _ = verifier.validate(&block).unwrap().into_result(); // Make the block invalid by adding of the duplicating transaction let len = block.transactions().len(); block .transactions_mut() .insert(len - 1, Transaction::default_test_tx()); - let verify_result = verifier.validate_without_commit(block); + let verify_result = verifier.validate(&block); assert!(matches!( verify_result, Err(ExecutorError::TransactionIdCollision(_)) @@ -1113,7 +1104,7 @@ mod tests { let ExecutionResult { skipped_transactions, - block, + mut block, .. } = producer .produce_without_commit(block) @@ -1128,15 +1119,12 @@ mod tests { )); // Produced block is valid - let ExecutionResult { mut block, .. } = verifier - .validate_without_commit(block) - .unwrap() - .into_result(); + let _ = verifier.validate(&block).unwrap().into_result(); // Invalidate block by adding transaction with not existing coin let len = block.transactions().len(); block.transactions_mut().insert(len - 1, tx); - let verify_result = verifier.validate_without_commit(block); + let verify_result = verifier.validate(&block); assert!(matches!( verify_result, Err(ExecutorError::TransactionValidity( @@ -1181,7 +1169,7 @@ mod tests { } // then - let err = verifier.validate_and_commit(block).unwrap_err(); + let err = verifier.validate_and_commit(&block).unwrap_err(); assert_eq!( err, ExecutorError::InvalidTransactionOutcome { transaction_id } @@ -1216,7 +1204,7 @@ mod tests { block.header_mut().set_transaction_root(rng.gen()); block.header_mut().recalculate_metadata(); - let err = verifier.validate_and_commit(block).unwrap_err(); + let err = verifier.validate_and_commit(&block).unwrap_err(); assert_eq!(err, ExecutorError::BlockMismatch) } @@ -1869,7 +1857,7 @@ mod tests { assert_eq!(tx.inputs()[0].state_root(), state_root); let _ = executor - .validate(block) + .validate(&block) .expect("Validation of block should be successful"); } @@ -2058,7 +2046,7 @@ mod tests { assert!(skipped_transactions.is_empty()); let verifier = create_executor(db, Default::default()); - let verify_result = verifier.validate_without_commit(second_block); + let verify_result = verifier.validate(&second_block); assert!(verify_result.is_ok()); } @@ -2133,7 +2121,7 @@ mod tests { } let verifier = create_executor(db, Default::default()); - let err = verifier.validate_without_commit(second_block).unwrap_err(); + let err = verifier.validate(&second_block).unwrap_err(); assert_eq!( err, @@ -2283,7 +2271,7 @@ mod tests { .expect("block execution failed unexpectedly"); make_executor(&[&message]) - .validate_and_commit(block) + .validate_and_commit(&block) .expect("block validation failed unexpectedly"); } @@ -2402,14 +2390,14 @@ mod tests { // Produced block is valid make_executor(&[]) // No messages in the db - .validate_and_commit(block.clone()) + .validate_and_commit(&block) .unwrap(); // Invalidate block by returning back `tx` with not existing message let index = block.transactions().len() - 1; block.transactions_mut().insert(index, tx); let res = make_executor(&[]) // No messages in the db - .validate_and_commit(block); + .validate_and_commit(&block); assert!(matches!( res, Err(ExecutorError::TransactionValidity( @@ -2444,13 +2432,13 @@ mod tests { // Produced block is valid make_executor(&[&message]) - .validate_and_commit(block.clone()) + .validate_and_commit(&block) .unwrap(); // Invalidate block by return back `tx` with not ready message. let index = block.transactions().len() - 1; block.transactions_mut().insert(index, tx); - let res = make_executor(&[&message]).validate_and_commit(block); + let res = make_executor(&[&message]).validate_and_commit(&block); assert!(matches!( res, Err(ExecutorError::TransactionValidity( @@ -2504,7 +2492,7 @@ mod tests { let exec = make_executor(&[&message]); let ExecutionResult { skipped_transactions, - block, + mut block, .. } = exec.produce_without_commit(block).unwrap().into_result(); // One of two transactions is skipped. @@ -2520,14 +2508,13 @@ mod tests { // Produced block is valid let exec = make_executor(&[&message]); - let ExecutionResult { mut block, .. } = - exec.validate_without_commit(block).unwrap().into_result(); + let _ = exec.validate(&block).unwrap().into_result(); // Invalidate block by return back `tx2` transaction skipped during production. let len = block.transactions().len(); block.transactions_mut().insert(len - 1, tx2); let exec = make_executor(&[&message]); - let res = exec.validate_without_commit(block); + let res = exec.validate(&block); assert!(matches!( res, Err(ExecutorError::TransactionValidity( @@ -2739,7 +2726,7 @@ mod tests { assert!(skipped_transactions.is_empty()); let validator = create_executor(db.clone(), config); - let result = validator.validate(block); + let result = validator.validate(&block); assert!(result.is_ok(), "{result:?}") } @@ -3292,13 +3279,10 @@ mod tests { let events = vec![event]; add_events_to_relayer(&mut verifier_relayer_db, da_height.into(), &events); let verifier = create_relayer_executor(verifyer_db, verifier_relayer_db); - let (result, _) = verifier - .validate_without_commit(produced_block) - .unwrap() - .into(); + let (result, _) = verifier.validate(&produced_block).unwrap().into(); // then - let txs = result.block.transactions(); + let txs = produced_block.transactions(); assert_eq!(txs.len(), 1); // and @@ -3450,7 +3434,7 @@ mod tests { let validator = create_relayer_executor(on_chain_db, relayer_db); // When - let result = validator.validate_without_commit(result.block).map(|_| ()); + let result = validator.validate(&result.block).map(|_| ()); // Then assert_eq!(Ok(()), result); diff --git a/crates/fuel-core/src/service/adapters/block_importer.rs b/crates/fuel-core/src/service/adapters/block_importer.rs index e6bcf8b9fbe..c4085ecddb7 100644 --- a/crates/fuel-core/src/service/adapters/block_importer.rs +++ b/crates/fuel-core/src/service/adapters/block_importer.rs @@ -44,7 +44,7 @@ use fuel_core_types::{ }, services::executor::{ Result as ExecutorResult, - UncommittedResult as UncommittedExecutionResult, + UncommittedValidationResult, }, }; use std::sync::Arc; @@ -103,8 +103,8 @@ impl ImporterDatabase for Database { impl Validator for ExecutorAdapter { fn validate( &self, - block: Block, - ) -> ExecutorResult> { + block: &Block, + ) -> ExecutorResult> { self.executor.validate(block) } } diff --git a/crates/services/executor/src/executor.rs b/crates/services/executor/src/executor.rs index cf99a575dae..e3c66bd932a 100644 --- a/crates/services/executor/src/executor.rs +++ b/crates/services/executor/src/executor.rs @@ -134,6 +134,8 @@ use fuel_core_types::{ TransactionExecutionStatus, TransactionValidityError, UncommittedResult, + UncommittedValidationResult, + ValidationResult, }, relayer::Event, }, @@ -247,24 +249,63 @@ 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)?; - Self::uncommitted_block_results(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( self, - block: Block, - ) -> ExecutorResult> { + block: &Block, + ) -> ExecutorResult> { 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)?; - Self::uncommitted_block_results(block, execution_data) + + let ExecutionData { + coinbase, + used_gas, + tx_status, + events, + changes, + .. + } = block_executor.validate_block(block, storage_tx)?; + + let finalized_block_id = block.id(); + + debug!( + "Block {:#x} fees: {} gas: {}", + finalized_block_id, coinbase, used_gas + ); + + let result = ValidationResult { tx_status, events }; + + Ok(UncommittedValidationResult::new(result, changes)) } fn into_executor( @@ -285,37 +326,6 @@ where let executor = BlockExecutor::new(self.relayer, self.options, consensus_params)?; Ok((executor, storage_tx)) } - - fn uncommitted_block_results( - block: Block, - data: ExecutionData, - ) -> ExecutorResult> { - let ExecutionData { - coinbase, - used_gas, - tx_status, - skipped_transactions, - events, - changes, - .. - } = 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)) - } } type BlockStorageTransaction = StorageTransaction; diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs index 874649803b9..cef50bc30a2 100644 --- a/crates/services/importer/src/importer.rs +++ b/crates/services/importer/src/importer.rs @@ -34,7 +34,7 @@ use fuel_core_types::{ UncommittedResult, }, executor, - executor::ExecutionResult, + executor::ValidationResult, Uncommitted, }, }; @@ -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,25 +396,11 @@ where return Err(Error::ExecuteGenesis) } - // TODO: Pass `block` into `ExecutionBlock::Validation` by ref - let ( - ExecutionResult { - block, - skipped_transactions, - tx_status, - events, - }, - changes, - ) = executor - .validate(block) + 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 08d1f4e7786..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::{ @@ -31,8 +30,9 @@ use fuel_core_types::{ }, executor::{ Error as ExecutorError, - ExecutionResult, Result as ExecutorResult, + UncommittedValidationResult, + ValidationResult, }, Uncommitted, }, @@ -67,12 +67,6 @@ fn u32_to_merkle_root(number: u32) -> MerkleRoot { MerkleRoot::from(root) } -#[derive(Clone, Debug)] -struct MockExecutionResult { - block: SealedBlock, - skipped_transactions: usize, -} - fn genesis(height: u32) -> SealedBlock { let mut block = Block::default(); block.header_mut().set_block_height(height.into()); @@ -145,11 +139,14 @@ fn storage_failure_error() -> Error { storage_failure::<()>().unwrap_err().into() } -fn ex_result(height: u32, skipped_transactions: usize) -> MockExecutionResult { - MockExecutionResult { - block: poa_block(height), - skipped_transactions, - } +fn ex_result() -> ExecutorResult> { + Ok(Uncommitted::new( + ValidationResult { + tx_status: vec![], + events: vec![], + }, + Default::default(), + )) } fn execution_failure() -> ExecutorResult { @@ -162,24 +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( - ExecutionResult { - block: mock_result.block.entity, - skipped_transactions, - tx_status: vec![], - events: vec![], - }, - Default::default(), - )) - }); + executor.expect_validate().return_once(move |_| result()); executor } @@ -329,7 +312,6 @@ async fn commit_result_and_execute_and_commit_poa( ) -> Result<(), Error> { // `execute_and_commit` and `commit_result` should have the same // validation rules(-> test cases) during committing the result. - let height = *sealed_block.entity.header().height(); let transaction = db_transaction(); let commit_result = commit_result_assert(sealed_block.clone(), underlying_db(), transaction).await; @@ -339,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(height.into(), 0))), + executor(ex_result), verifier(ok(())), ) .await; @@ -449,37 +431,27 @@ fn one_lock_at_the_same_time() { ///////// New block, Block After Execution, Verification result, commits ///////// #[test_case( - genesis(113), ok(ex_result(113, 0)), ok(()), 0 + genesis(113), ex_result, ok(()), 0 => Err(Error::ExecuteGenesis); "cannot execute genesis block" )] #[test_case( - poa_block(1), ok(ex_result(1, 0)), ok(()), 1 + poa_block(1), ex_result, ok(()), 1 => Ok(()); "commits block 1" )] #[test_case( - poa_block(113), ok(ex_result(113, 0)), ok(()), 1 + poa_block(113), ex_result, ok(()), 1 => Ok(()); "commits block 113" )] -#[test_case( - poa_block(113), ok(ex_result(114, 0)), ok(()), 0 - => Err(Error::BlockIdMismatch(poa_block(113).entity.id(), poa_block(114).entity.id())); - "execution result should match block height" -)] #[test_case( poa_block(113), execution_failure, ok(()), 0 => Err(execution_failure_error()); "commit fails if execution fails" )] #[test_case( - poa_block(113), ok(ex_result(113, 1)), ok(()), 0 - => Err(Error::SkippedTransactionsNotEmpty); - "commit fails if there are skipped transactions" -)] -#[test_case( - poa_block(113), ok(ex_result(113, 0)), verification_failure, 0 + poa_block(113), ex_result, verification_failure, 0 => Err(verification_failure_error()); "commit fails if verification fails" )] @@ -491,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 @@ -527,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( @@ -543,7 +518,7 @@ where fn verify_and_execute_allowed_when_locked() { let importer = Importer::default_config( MockDatabase::default(), - executor(ok(ex_result(13, 0))), + executor(ex_result), verifier(ok(())), ); diff --git a/crates/services/importer/src/ports.rs b/crates/services/importer/src/ports.rs index 3795749c2e6..cd5cbe02c42 100644 --- a/crates/services/importer/src/ports.rs +++ b/crates/services/importer/src/ports.rs @@ -35,7 +35,7 @@ use fuel_core_types::{ }, services::executor::{ Result as ExecutorResult, - UncommittedResult, + UncommittedValidationResult, }, }; @@ -44,7 +44,10 @@ use fuel_core_types::{ pub trait Validator: Send + Sync { /// Executes the block and returns the result of execution with uncommitted database /// transaction. - fn validate(&self, block: Block) -> ExecutorResult>; + fn validate( + &self, + block: &Block, + ) -> ExecutorResult>; } /// The trait indicates that the type supports storage transactions. diff --git a/crates/services/upgradable-executor/src/executor.rs b/crates/services/upgradable-executor/src/executor.rs index 75438fdd62b..742b0406fff 100644 --- a/crates/services/upgradable-executor/src/executor.rs +++ b/crates/services/upgradable-executor/src/executor.rs @@ -54,6 +54,7 @@ use fuel_core_storage::{ use fuel_core_types::blockchain::block::PartialFuelBlock; #[cfg(any(test, feature = "test-helpers"))] use fuel_core_types::services::executor::UncommittedResult; +use fuel_core_types::services::executor::ValidationResult; #[cfg(feature = "wasm-executor")] enum ExecutionStrategy { @@ -210,10 +211,9 @@ where /// Executes the block and commits the result of the execution into the inner `Database`. pub fn validate_and_commit( &mut self, - block: Block, - ) -> fuel_core_types::services::executor::Result { - let (result, changes) = self.validate_without_commit(block)?.into(); - + block: &Block, + ) -> fuel_core_types::services::executor::Result { + let (result, changes) = self.validate(block)?.into(); self.storage_view_provider.commit_changes(changes)?; Ok(result) } @@ -235,15 +235,6 @@ where self.produce_without_commit_with_coinbase(block, Default::default(), 0) } - /// Executes the block and returns the result of the execution with storage changes. - pub fn validate_without_commit( - &self, - block: Block, - ) -> fuel_core_types::services::executor::Result> { - let options = self.config.as_ref().into(); - self.validate_inner(block, options) - } - /// The analog of the [`Self::produce_without_commit`] method, /// but with the ability to specify the coinbase recipient and the gas price. pub fn produce_without_commit_with_coinbase( @@ -336,8 +327,8 @@ where pub fn validate( &self, - block: Block, - ) -> ExecutorResult> { + block: &Block, + ) -> ExecutorResult> { let options = self.config.as_ref().into(); self.validate_inner(block, options) } @@ -395,9 +386,9 @@ where #[cfg(feature = "wasm-executor")] fn validate_inner( &self, - block: Block, + block: &Block, options: ExecutionOptions, - ) -> ExecutorResult> { + ) -> ExecutorResult> { let block_version = block.header().state_transition_bytecode_version; let native_executor_version = self.native_executor_version(); if block_version == native_executor_version { @@ -427,9 +418,9 @@ where #[cfg(not(feature = "wasm-executor"))] fn validate_inner( &self, - block: Block, + block: &Block, options: ExecutionOptions, - ) -> ExecutorResult> { + ) -> ExecutorResult> { let block_version = block.header().state_transition_bytecode_version; let native_executor_version = self.native_executor_version(); if block_version == native_executor_version { @@ -493,9 +484,9 @@ where fn wasm_validate_inner( &self, module: &wasmtime::Module, - block: Block, + block: &Block, options: ExecutionOptions, - ) -> ExecutorResult> { + ) -> ExecutorResult> { let storage = self.storage_view_provider.latest_view(); let relayer = self.relayer_view_provider.latest_view(); @@ -508,7 +499,9 @@ where let output = instance.run(module)?; match output { - fuel_core_wasm_executor::utils::ReturnType::V1(result) => result, + fuel_core_wasm_executor::utils::ReturnType::V1(result) => { + Ok(result?.into_validation_result()) + } } } @@ -527,9 +520,9 @@ where fn native_validate_inner( &self, - block: Block, + block: &Block, options: ExecutionOptions, - ) -> ExecutorResult> { + ) -> ExecutorResult> { let instance = self.new_native_executor_instance(options); instance.validate_without_commit(block) } @@ -809,7 +802,7 @@ mod test { let block = valid_block(Executor::::VERSION); // When - let result = executor.validate_without_commit(block).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then assert_eq!(Ok(()), result); @@ -824,7 +817,7 @@ mod test { let block = valid_block(Executor::::VERSION); // When - let result = executor.validate_without_commit(block).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then assert_eq!(Ok(()), result); @@ -840,7 +833,7 @@ mod test { let block = valid_block(wrong_version); // When - let result = executor.validate_without_commit(block).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then result.expect_err("The validation should fail because of versions mismatch"); @@ -856,7 +849,7 @@ mod test { let block = valid_block(wrong_version); // When - let result = executor.validate_without_commit(block).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then result.expect_err("The validation should fail because of versions mismatch"); @@ -895,7 +888,7 @@ mod test { let block = valid_block(next_version); // When - let result = executor.validate_without_commit(block).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then assert_eq!(Ok(()), result); @@ -910,7 +903,7 @@ mod test { let block = valid_block(next_version); // When - let result = executor.validate_without_commit(block).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then assert_eq!(Ok(()), result); @@ -929,7 +922,7 @@ mod test { // When for _ in 0..1000 { - let result = executor.validate_without_commit(block.clone()).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then assert_eq!(Ok(()), result); @@ -949,7 +942,7 @@ mod test { // When for _ in 0..1000 { - let result = executor.validate_without_commit(block.clone()).map(|_| ()); + let result = executor.validate(&block).map(|_| ()); // Then assert_eq!(Ok(()), result); diff --git a/crates/services/upgradable-executor/src/instance.rs b/crates/services/upgradable-executor/src/instance.rs index 0fb8417b8af..3fade8a2efa 100644 --- a/crates/services/upgradable-executor/src/instance.rs +++ b/crates/services/upgradable-executor/src/instance.rs @@ -31,9 +31,9 @@ use fuel_core_types::{ use fuel_core_wasm_executor::utils::{ pack_exists_size_result, unpack_ptr_and_len, - InputType, + InputSerializationType, ReturnType, - WasmExecutionBlockTypes, + WasmSerializationBlockTypes, }; use std::{ collections::HashMap, @@ -459,8 +459,8 @@ impl Instance { components: Components<()>, options: ExecutionOptions, ) -> ExecutorResult> { - let input = InputType::V1 { - block: WasmExecutionBlockTypes::Production(components), + let input = InputSerializationType::V1 { + block: WasmSerializationBlockTypes::Production(components), options, }; self.add_input_data(input) @@ -471,26 +471,30 @@ impl Instance { components: Components<()>, options: ExecutionOptions, ) -> ExecutorResult> { - let input = InputType::V1 { - block: WasmExecutionBlockTypes::DryRun(components), + let input = InputSerializationType::V1 { + block: WasmSerializationBlockTypes::DryRun(components), options, }; self.add_input_data(input) } + // pub fn add_validation_input_data( self, - block: Block, + block: &Block, options: ExecutionOptions, ) -> ExecutorResult> { - let input = InputType::V1 { - block: WasmExecutionBlockTypes::Validation(block), + let input = InputSerializationType::V1 { + block: WasmSerializationBlockTypes::Validation(block), options, }; self.add_input_data(input) } - fn add_input_data(mut self, input: InputType) -> ExecutorResult> { + fn add_input_data( + mut self, + input: InputSerializationType, + ) -> ExecutorResult> { let encoded_input = postcard::to_allocvec(&input).map_err(|e| { ExecutorError::Other(format!( "Failed encoding of the input for `input` function: {}", diff --git a/crates/services/upgradable-executor/wasm-executor/src/ext.rs b/crates/services/upgradable-executor/wasm-executor/src/ext.rs index 666552a20ca..bafad827179 100644 --- a/crates/services/upgradable-executor/wasm-executor/src/ext.rs +++ b/crates/services/upgradable-executor/wasm-executor/src/ext.rs @@ -1,6 +1,6 @@ use crate::utils::{ unpack_exists_size_result, - InputType, + InputDeserializationType, }; use core::marker::PhantomData; use fuel_core_executor::ports::MaybeCheckedTransaction; @@ -131,7 +131,7 @@ mod host { pub struct ReturnResult(u16); /// Gets the `InputType` by using the host function. The `size` is the size of the encoded input. -pub fn input(size: usize) -> anyhow::Result { +pub fn input(size: usize) -> anyhow::Result { let mut encoded_block = vec![0u8; size]; let size = encoded_block.len(); unsafe { @@ -141,7 +141,7 @@ pub fn input(size: usize) -> anyhow::Result { ) }; - let input: InputType = postcard::from_bytes(&encoded_block) + let input: InputDeserializationType = postcard::from_bytes(&encoded_block) .map_err(|e| anyhow::anyhow!("Failed to decode the block: {:?}", e))?; Ok(input) diff --git a/crates/services/upgradable-executor/wasm-executor/src/main.rs b/crates/services/upgradable-executor/wasm-executor/src/main.rs index bdc72902183..6065cae9ea2 100644 --- a/crates/services/upgradable-executor/wasm-executor/src/main.rs +++ b/crates/services/upgradable-executor/wasm-executor/src/main.rs @@ -14,7 +14,10 @@ #![deny(warnings)] use crate as fuel_core_wasm_executor; -use crate::utils::WasmExecutionBlockTypes; +use crate::utils::{ + InputDeserializationType, + WasmDeserializationBlockTypes, +}; use fuel_core_executor::executor::ExecutionInstance; use fuel_core_storage::transactional::Changes; use fuel_core_types::{ @@ -35,7 +38,6 @@ use fuel_core_wasm_executor::{ tx_source::WasmTxSource, utils::{ pack_ptr_and_len, - InputType, ReturnType, }, }; @@ -65,16 +67,16 @@ pub fn execute_without_commit( .map_err(|e| ExecutorError::Other(e.to_string()))?; let (block, options) = match input { - InputType::V1 { block, options } => { + InputDeserializationType::V1 { block, options } => { let block = match block { - WasmExecutionBlockTypes::DryRun(c) => { - WasmExecutionBlockTypes::DryRun(use_wasm_tx_source(c)) + WasmDeserializationBlockTypes::DryRun(c) => { + WasmDeserializationBlockTypes::DryRun(use_wasm_tx_source(c)) } - WasmExecutionBlockTypes::Production(c) => { - WasmExecutionBlockTypes::Production(use_wasm_tx_source(c)) + WasmDeserializationBlockTypes::Production(c) => { + WasmDeserializationBlockTypes::Production(use_wasm_tx_source(c)) } - WasmExecutionBlockTypes::Validation(c) => { - WasmExecutionBlockTypes::Validation(c) + WasmDeserializationBlockTypes::Validation(c) => { + WasmDeserializationBlockTypes::Validation(c) } }; @@ -89,9 +91,9 @@ pub fn execute_without_commit( }; match block { - WasmExecutionBlockTypes::DryRun(c) => execute_dry_run(instance, c), - WasmExecutionBlockTypes::Production(c) => execute_production(instance, c), - WasmExecutionBlockTypes::Validation(c) => execute_validation(instance, c), + WasmDeserializationBlockTypes::DryRun(c) => execute_dry_run(instance, c), + WasmDeserializationBlockTypes::Production(c) => execute_production(instance, c), + WasmDeserializationBlockTypes::Validation(c) => execute_validation(instance, c), } } @@ -113,25 +115,13 @@ fn execute_validation( instance: ExecutionInstance, block: Block, ) -> ExecutorResult> { - let ( - ExecutionResult { - block, - skipped_transactions, - tx_status, - events, - }, - changes, - ) = instance.validate_without_commit(block)?.into(); - - Ok(Uncommitted::new( - ExecutionResult { - block, - skipped_transactions, - tx_status, - events, - }, - changes, - )) + 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(result) } fn use_wasm_tx_source(component: Components<()>) -> Components { diff --git a/crates/services/upgradable-executor/wasm-executor/src/utils.rs b/crates/services/upgradable-executor/wasm-executor/src/utils.rs index 6069c23a1bf..640c04c4d23 100644 --- a/crates/services/upgradable-executor/wasm-executor/src/utils.rs +++ b/crates/services/upgradable-executor/wasm-executor/src/utils.rs @@ -44,16 +44,34 @@ pub fn unpack_exists_size_result(val: u64) -> (bool, u32, u16) { /// The input type for the WASM executor. Enum allows handling different /// versions of the input without introducing new host functions. -#[derive(Debug, serde::Serialize, serde::Deserialize)] -pub enum InputType { +#[derive(Debug, serde::Serialize)] +pub enum InputSerializationType<'a> { V1 { - block: WasmExecutionBlockTypes<()>, + block: WasmSerializationBlockTypes<'a, ()>, options: ExecutionOptions, }, } -#[derive(Debug, serde::Serialize, serde::Deserialize)] -pub enum WasmExecutionBlockTypes { +#[derive(Debug, serde::Deserialize)] +pub enum InputDeserializationType { + V1 { + block: WasmDeserializationBlockTypes<()>, + options: ExecutionOptions, + }, +} + +#[derive(Debug, serde::Serialize)] +pub enum WasmSerializationBlockTypes<'a, TxSource> { + /// DryRun mode where P is being produced. + DryRun(Components), + /// Production mode where P is being produced. + Production(Components), + /// Validation of a produced block. + Validation(&'a Block), +} + +#[derive(Debug, serde::Deserialize)] +pub enum WasmDeserializationBlockTypes { /// DryRun mode where P is being produced. DryRun(Components), /// Production mode where P is being produced. diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index e8dce5937c0..e3fe02c9a59 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -36,11 +36,15 @@ use crate::{ /// The alias for executor result. pub type Result = core::result::Result; -/// The uncommitted result of the transaction execution. +/// The uncommitted result of the block production execution. pub type UncommittedResult = Uncommitted; -/// The result of transactions execution. +/// The uncommitted result of the block validation. +pub type UncommittedValidationResult = + Uncommitted; + +/// The result of transactions execution for block production. #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[derive(Debug)] pub struct ExecutionResult { @@ -55,6 +59,54 @@ pub struct ExecutionResult { pub events: Vec, } +/// The result of the validation of the block. +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[derive(Debug)] +pub struct ValidationResult { + /// 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. + pub events: Vec, +} + +impl UncommittedValidationResult { + /// Convert the `UncommittedValidationResult` into the `UncommittedResult`. + pub fn into_execution_result( + self, + block: Block, + skipped_transactions: Vec<(TxId, Error)>, + ) -> UncommittedResult { + let Self { + result: ValidationResult { tx_status, events }, + changes, + } = self; + UncommittedResult::new( + ExecutionResult { + block, + skipped_transactions, + tx_status, + events, + }, + changes, + ) + } +} + +impl UncommittedResult { + /// Convert the `UncommittedResult` into the `UncommittedValidationResult`. + pub fn into_validation_result( + self, + ) -> UncommittedValidationResult { + let Self { + result: ExecutionResult { + tx_status, events, .. + }, + changes, + } = self; + UncommittedValidationResult::new(ValidationResult { tx_status, events }, changes) + } +} + /// The event represents some internal state changes caused by the block execution. #[derive(Debug, Clone)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]