Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added skipping of invalid transactions during block production #746

Merged
merged 11 commits into from
Nov 3, 2022
29 changes: 12 additions & 17 deletions fuel-block-producer/src/block_producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ use fuel_core_interfaces::{
fuel_types::Bytes32,
},
executor::{
Error,
ExecutionBlock,
ExecutionResult,
Executor,
},
model::{
Expand Down Expand Up @@ -98,28 +98,23 @@ impl Trait for Producer {
"Failed to produce block {:?} due to execution failure",
block
);
let result = self
let ExecutionResult {
block,
skipped_transactions,
} = self
.executor
.execute(ExecutionBlock::Production(block))
.await;

if let Err(
Error::VmExecution { transaction_id, .. }
| Error::TransactionIdCollision(transaction_id),
) = &result
{
// TODO: if block execution fails due to any transaction validity errors,
// should those txs be removed from the txpool? While this
// theoretically shouldn't happen due to txpool validation rules,
// it is a possibility.
.await
.context(context_string)?;

for (tx, err) in skipped_transactions {
// TODO: Removed from `TxPool` invalid transactions
error!(
"faulty tx prevented block production: {:#x}",
transaction_id
"During block production got invalid transaction {:?} with error {:?}",
tx, err
);
}

let block = result.context(context_string)?;

debug!("Produced block: {:?}", &block);
Ok(block)
}
Expand Down
29 changes: 21 additions & 8 deletions fuel-block-producer/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ use fuel_core_interfaces::{
executor::{
Error as ExecutorError,
ExecutionBlock,
ExecutionResult,
Executor,
},
model::{
ArcPoolTx,
BlockHeight,
DaBlockHeight,
FuelBlock,
FuelBlockDb,
Message,
},
Expand Down Expand Up @@ -72,15 +72,21 @@ pub struct MockExecutor(pub MockDb);

#[async_trait]
impl Executor for MockExecutor {
async fn execute(&self, block: ExecutionBlock) -> Result<FuelBlock, ExecutorError> {
async fn execute(
&self,
block: ExecutionBlock,
) -> Result<ExecutionResult, ExecutorError> {
let block = match block {
ExecutionBlock::Production(block) => block.generate(&[]),
ExecutionBlock::Validation(block) => block,
};
// simulate executor inserting a block
let mut block_db = self.0.blocks.lock().unwrap();
block_db.insert(*block.header().height(), block.to_db_block());
Ok(block)
Ok(ExecutionResult {
block,
skipped_transactions: vec![],
})
}

async fn dry_run(
Expand All @@ -96,16 +102,23 @@ pub struct FailingMockExecutor(pub Mutex<Option<ExecutorError>>);

#[async_trait]
impl Executor for FailingMockExecutor {
async fn execute(&self, block: ExecutionBlock) -> Result<FuelBlock, ExecutorError> {
async fn execute(
&self,
block: ExecutionBlock,
) -> Result<ExecutionResult, ExecutorError> {
// simulate an execution failure
let mut err = self.0.lock().unwrap();
if let Some(err) = err.take() {
Err(err)
} else {
match block {
ExecutionBlock::Production(b) => Ok(b.generate(&[])),
ExecutionBlock::Validation(b) => Ok(b),
}
let block = match block {
ExecutionBlock::Production(b) => b.generate(&[]),
ExecutionBlock::Validation(b) => b,
};
Ok(ExecutionResult {
block,
skipped_transactions: vec![],
})
}
}

Expand Down
12 changes: 11 additions & 1 deletion fuel-core-interfaces/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ pub enum ExecutionTypes<P, V> {
Validation(V),
}

/// The result of transactions execution.
#[derive(Debug)]
pub struct ExecutionResult {
/// Created block during the execution of transactions. It contains only valid transactions.
pub block: FuelBlock,
/// 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<(Transaction, Error)>,
}

#[derive(Debug, Clone, Copy)]
/// The kind of execution.
pub enum ExecutionKind {
Expand All @@ -54,7 +64,7 @@ pub enum ExecutionKind {

#[async_trait]
pub trait Executor: Sync + Send {
async fn execute(&self, block: ExecutionBlock) -> Result<FuelBlock, Error>;
async fn execute(&self, block: ExecutionBlock) -> Result<ExecutionResult, Error>;

async fn dry_run(
&self,
Expand Down
Loading