Added skipping of invalid transactions during block production (#746)

* Added skipping of invalid transactions during block production with unit tests.

* Add one more test to check that skipped transaction didn't change the state.

* Make clippy happy

* Update fuel-core/src/executor.rs

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

* Update fuel-core/src/executor.rs

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

* Use `into_iter().filter_map` instead of `while` loop

* Make clippy happy

* Added test to check the order after skipping

* Remove double `tx_index += 1`

* Added different name for inner index

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>

fuel-block-producer/src/block_producer.rs

@@ -29,8 +29,8 @@ use fuel_core_interfaces::{
         fuel_types::Bytes32,
     },
     executor::{
-        Error,
         ExecutionBlock,
+        ExecutionResult,
         Executor,
     },
     model::{
@@ -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)
     }

fuel-block-producer/src/mocks.rs

@@ -19,13 +19,13 @@ use fuel_core_interfaces::{
     executor::{
         Error as ExecutorError,
         ExecutionBlock,
+        ExecutionResult,
         Executor,
     },
     model::{
         ArcPoolTx,
         BlockHeight,
         DaBlockHeight,
-        FuelBlock,
         FuelBlockDb,
         Message,
     },
@@ -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(
@@ -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![],
+            })
         }
     }
 

fuel-core-interfaces/src/executor.rs

@@ -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 {
@@ -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,