Use ref instead of owned Block in validation (#1886)

#1882 ## Checklist - [ ] Breaking changes are clearly marked as such in the PR description and changelog - [ ] New behavior is reflected in tests - [ ] [The specification](https://github.com/FuelLabs/fuel-specs/) matches the implemented behavior (link update PR if changes are needed) ### Before requesting review - [x] I have reviewed the code myself - [ ] I have created follow-up issues caused by this PR and linked them here ### After merging, notify other teams [Add or remove entries as needed] - [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/) - [ ] [Sway compiler](https://github.com/FuelLabs/sway/) - [ ] [Platform documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+) (for out-of-organization contributors, the person merging the PR will do this) - [ ] Someone else? --------- Co-authored-by: xgreenx <xgreenx9999@gmail.com>
FuelLabs · May 8, 2024 · 32600db · 32600db
1 parent 9cc93b1
commit 32600db
Show file tree

Hide file tree

Showing 13 changed files with 260 additions and 248 deletions.
diff --git 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
@@ -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
@@ -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<UncommittedExecutionResult<Changes>> {
+        block: &Block,
+    ) -> ExecutorResult<UncommittedValidationResult<Changes>> {
         self.executor.validate(block)
     }
 }