Skip to content

Commit

Permalink
Removed skipped_transactions from ValidationResult
Browse files Browse the repository at this point in the history
  • Loading branch information
xgreenx committed May 8, 2024
1 parent dbba4a6 commit c9728b2
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 140 deletions.
88 changes: 45 additions & 43 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ use fuel_core_types::{
executor::{
Error as ExecutorError,
Event as ExecutorEvent,
ExecutionResult,
ForcedTransactionFailure,
Result as ExecutorResult,
TransactionExecutionResult,
Expand Down Expand Up @@ -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(
Expand All @@ -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<R>, StorageTransaction<D>)> {
let storage_tx = self
.database
.into_transaction()
.with_policy(ConflictPolicy::Overwrite);
let consensus_params = storage_tx
.storage::<ConsensusParametersVersions>()
.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<Changes> {
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();

Expand All @@ -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<Changes> {
let result = Self::uncommitted_validation_result(&block, data);
result.into_execution_result(block)
fn into_executor(
self,
consensus_params_version: ConsensusParametersVersion,
) -> ExecutorResult<(BlockExecutor<R>, StorageTransaction<D>)> {
let storage_tx = self
.database
.into_transaction()
.with_policy(ConflictPolicy::Overwrite);
let consensus_params = storage_tx
.storage::<ConsensusParametersVersions>()
.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))
}
}

Expand Down
18 changes: 1 addition & 17 deletions crates/services/importer/src/importer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}.")]
Expand Down Expand Up @@ -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.
Expand Down
60 changes: 22 additions & 38 deletions crates/services/importer/src/importer/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use fuel_core_types::{
consensus::Consensus,
SealedBlock,
},
fuel_tx::TxId,
fuel_types::BlockHeight,
services::{
block_importer::{
Expand All @@ -32,6 +31,7 @@ use fuel_core_types::{
executor::{
Error as ExecutorError,
Result as ExecutorResult,
UncommittedValidationResult,
ValidationResult,
},
Uncommitted,
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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<UncommittedValidationResult<Changes>> {
Ok(Uncommitted::new(
ValidationResult {
tx_status: vec![],
events: vec![],
},
Default::default(),
))
}

fn execution_failure<T>() -> ExecutorResult<T> {
Expand All @@ -160,23 +159,10 @@ fn execution_failure_error() -> Error {

fn executor<R>(result: R) -> MockValidator
where
R: Fn() -> ExecutorResult<MockExecutionResult> + Send + 'static,
R: Fn() -> ExecutorResult<UncommittedValidationResult<Changes>> + 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
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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"
)]
Expand All @@ -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"
)]
Expand All @@ -482,7 +463,10 @@ async fn execute_and_commit_and_verify_and_execute_block_poa<V, P>(
commits: usize,
) -> Result<(), Error>
where
P: Fn() -> ExecutorResult<MockExecutionResult> + Send + Clone + 'static,
P: Fn() -> ExecutorResult<UncommittedValidationResult<Changes>>
+ Send
+ Clone
+ 'static,
V: Fn() -> anyhow::Result<()> + Send + Clone + 'static,
{
// `execute_and_commit` and `verify_and_execute_block` should have the same
Expand Down Expand Up @@ -518,7 +502,7 @@ fn verify_and_execute_assert<P, V>(
verifier_result: V,
) -> Result<(), Error>
where
P: Fn() -> ExecutorResult<MockExecutionResult> + Send + 'static,
P: Fn() -> ExecutorResult<UncommittedValidationResult<Changes>> + Send + 'static,
V: Fn() -> anyhow::Result<()> + Send + 'static,
{
let importer = Importer::default_config(
Expand All @@ -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(())),
);

Expand Down
22 changes: 4 additions & 18 deletions crates/services/upgradable-executor/wasm-executor/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ use fuel_core_types::{
Error as ExecutorError,
ExecutionResult,
Result as ExecutorResult,
ValidationResult,
},
Uncommitted,
},
Expand Down Expand Up @@ -116,26 +115,13 @@ fn execute_validation(
instance: ExecutionInstance<WasmRelayer, WasmStorage>,
block: Block,
) -> ExecutorResult<Uncommitted<ExecutionResult, Changes>> {
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<WasmTxSource> {
Expand Down
Loading

0 comments on commit c9728b2

Please sign in to comment.