From 393b5a7aa5f6a50f0d6448f19554ee5ca7225f5f Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 14:19:37 +0000 Subject: [PATCH 01/33] - Added the main logic of the block importer. - Added `debug_assert_eq` to verify that the block header id is an identifier of the block. And that it uses the last hash of the application header. No unit tests right now. --- Cargo.lock | 4 +- crates/fuel-core/src/executor.rs | 23 +- crates/fuel-core/src/graphql_api.rs | 13 +- .../fuel-core/src/query/message_proof/test.rs | 1 - crates/fuel-core/src/schema/block.rs | 1 - crates/fuel-core/src/service/genesis.rs | 1 - crates/services/importer/Cargo.toml | 4 +- crates/services/importer/src/config.rs | 12 +- crates/services/importer/src/importer.rs | 275 ++++++++++++++++++ crates/services/importer/src/lib.rs | 5 +- crates/services/importer/src/ports.rs | 72 +++++ crates/services/importer/src/service.rs | 40 --- .../services/producer/src/block_producer.rs | 1 - .../producer/src/block_producer/tests.rs | 1 - crates/storage/src/lib.rs | 23 ++ crates/storage/src/test_helpers.rs | 38 +++ crates/types/src/blockchain/block.rs | 7 +- crates/types/src/blockchain/consensus.rs | 3 +- crates/types/src/blockchain/header.rs | 47 ++- crates/types/src/services.rs | 45 +++ crates/types/src/services/block_importer.rs | 22 ++ crates/types/src/services/executor.rs | 64 +--- tests/tests/blocks.rs | 58 +--- 23 files changed, 569 insertions(+), 191 deletions(-) create mode 100644 crates/services/importer/src/importer.rs create mode 100644 crates/services/importer/src/ports.rs delete mode 100644 crates/services/importer/src/service.rs create mode 100644 crates/storage/src/test_helpers.rs create mode 100644 crates/types/src/services/block_importer.rs diff --git a/Cargo.lock b/Cargo.lock index 505612b0cb0..4125c88ab5c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2350,8 +2350,10 @@ name = "fuel-core-importer" version = "0.15.1" dependencies = [ "anyhow", - "parking_lot 0.12.1", + "fuel-core-storage", + "fuel-core-types", "tokio", + "tracing", ] [[package]] diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 4465260704c..27c2cd27231 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -204,8 +204,6 @@ impl Executor { ) -> ExecutorResult>> { // Compute the block id before execution if there is one. let pre_exec_block_id = block.id(); - // Get the transaction root before execution if there is one. - let pre_exec_txs_root = block.txs_root(); // If there is full fuel block for validation then map it into // a partial header. @@ -230,13 +228,6 @@ impl Executor { .map(|b: PartialFuelBlock| b.generate(&message_ids[..])) .into_inner(); - // check transaction commitment - if let Some(pre_exec_txs_root) = pre_exec_txs_root { - if block.header().transactions_root != pre_exec_txs_root { - return Err(ExecutorError::InvalidTransactionRoot) - } - } - let finalized_block_id = block.id(); debug!( @@ -247,6 +238,7 @@ impl Executor { // check if block id doesn't match proposed block id if let Some(pre_exec_block_id) = pre_exec_block_id { + // The block id comparison compares the whole blocks including all fields. if pre_exec_block_id != finalized_block_id { // In theory this shouldn't happen since any deviance in the block should've already // been checked by now. @@ -1855,6 +1847,7 @@ mod tests { let mut block = Block::default(); *block.transactions_mut() = vec![mint.into()]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -1875,6 +1868,7 @@ mod tests { let mut block = Block::default(); *block.transactions_mut() = vec![mint.into()]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -1897,6 +1891,7 @@ mod tests { let mut block = Block::default(); *block.transactions_mut() = vec![mint.into()]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -1923,6 +1918,7 @@ mod tests { let mut block = Block::default(); *block.transactions_mut() = vec![mint.into()]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -1950,6 +1946,7 @@ mod tests { let mut block = Block::default(); *block.transactions_mut() = vec![mint.into()]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -1973,6 +1970,7 @@ mod tests { let mut block = Block::default(); *block.transactions_mut() = vec![mint.into()]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -2002,6 +2000,7 @@ mod tests { ) .into(), ]; + block.header_mut().recalculate_metadata(); let validator = Executor { database: Default::default(), @@ -2423,14 +2422,12 @@ mod tests { // randomize transaction commitment block.header_mut().application.generated.transactions_root = rng.gen(); + block.header_mut().recalculate_metadata(); let verify_result = verifier.execute_and_commit(ExecutionBlock::Validation(block)); - assert!(matches!( - verify_result, - Err(ExecutorError::InvalidTransactionRoot) - )) + assert!(matches!(verify_result, Err(ExecutorError::InvalidBlockId))) } // invalidate a block if a tx is missing at least one coin input diff --git a/crates/fuel-core/src/graphql_api.rs b/crates/fuel-core/src/graphql_api.rs index 7614a0989a3..c04e094744e 100644 --- a/crates/fuel-core/src/graphql_api.rs +++ b/crates/fuel-core/src/graphql_api.rs @@ -1,4 +1,7 @@ -use fuel_core_storage::Error as StorageError; +use fuel_core_storage::{ + Error as StorageError, + IsNotFound, +}; use fuel_core_types::{ blockchain::primitives::SecretKeyWrapper, fuel_tx::ConsensusParameters, @@ -35,10 +38,10 @@ impl IntoApiResult for Result { NewT: From, E: From, { - match self { - Ok(t) => Ok(Some(t.into())), - Err(StorageError::NotFound(_, _)) => Ok(None), - Err(err) => Err(err.into()), + if self.is_not_found() { + Ok(None) + } else { + Ok(Some(self?.into())) } } } diff --git a/crates/fuel-core/src/query/message_proof/test.rs b/crates/fuel-core/src/query/message_proof/test.rs index a0283a2701b..02768b34be1 100644 --- a/crates/fuel-core/src/query/message_proof/test.rs +++ b/crates/fuel-core/src/query/message_proof/test.rs @@ -154,7 +154,6 @@ async fn can_build_message_proof() { time: Tai64::UNIX_EPOCH, generated: Default::default(), }, - metadata: None, }; data.expect_block() .once() diff --git a/crates/fuel-core/src/schema/block.rs b/crates/fuel-core/src/schema/block.rs index f078fc82e4b..061a6c5874e 100644 --- a/crates/fuel-core/src/schema/block.rs +++ b/crates/fuel-core/src/schema/block.rs @@ -337,7 +337,6 @@ impl BlockMutation { da_height: Default::default(), generated: Default::default(), }, - metadata: Default::default(), }, vec![], ); diff --git a/crates/fuel-core/src/service/genesis.rs b/crates/fuel-core/src/service/genesis.rs index a58559423f7..b0f3ca51a3d 100644 --- a/crates/fuel-core/src/service/genesis.rs +++ b/crates/fuel-core/src/service/genesis.rs @@ -119,7 +119,6 @@ fn add_genesis_block(config: &Config, database: &mut Database) -> anyhow::Result time: fuel_core_types::tai64::Tai64::UNIX_EPOCH, generated: Empty, }, - metadata: None, }, // Genesis block doesn't have any transaction. vec![], diff --git a/crates/services/importer/Cargo.toml b/crates/services/importer/Cargo.toml index 0f3b2914568..a9f1670f7c4 100644 --- a/crates/services/importer/Cargo.toml +++ b/crates/services/importer/Cargo.toml @@ -11,5 +11,7 @@ description = "Fuel Block Importer" [dependencies] anyhow = "1.0" -parking_lot = "0.12" +fuel-core-storage = { path = "../../storage" } +fuel-core-types = { path = "../../types" } tokio = { version = "1.21", features = ["full"] } +tracing = "0.1" diff --git a/crates/services/importer/src/config.rs b/crates/services/importer/src/config.rs index 476292ea8d8..ddb17391427 100644 --- a/crates/services/importer/src/config.rs +++ b/crates/services/importer/src/config.rs @@ -1,4 +1,14 @@ -#[derive(Default, Debug, Clone)] +#[derive(Debug, Clone)] pub struct Config { + pub max_block_notify_buffer: usize, pub metrics: bool, } + +impl Default for Config { + fn default() -> Self { + Self { + max_block_notify_buffer: 1 << 10, + metrics: false, + } + } +} diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs new file mode 100644 index 00000000000..e403f1c414a --- /dev/null +++ b/crates/services/importer/src/importer.rs @@ -0,0 +1,275 @@ +use crate::{ + ports::{ + BlockVerifier, + Executor, + ExecutorDatabase, + ImporterDatabase, + }, + Config, +}; +use anyhow::anyhow; +use fuel_core_storage::{ + transactional::StorageTransaction, + IsNotFound, +}; +use fuel_core_types::{ + blockchain::{ + consensus::{ + Consensus, + Sealed, + }, + primitives::BlockHeight, + SealedBlock, + }, + fuel_vm::crypto::ephemeral_merkle_root, + services::{ + block_importer::{ + ImportResult, + UncommittedResult, + }, + executor::{ + ExecutionBlock, + ExecutionResult, + }, + Uncommitted, + }, +}; +use std::sync::Arc; +use tokio::sync::broadcast; + +pub struct Importer { + database: D, + executor: E, + verifier: V, + broadcast: broadcast::Sender>, + guard: tokio::sync::Semaphore, +} + +impl Importer { + pub fn new(config: Config, database: D, executor: E, verifier: V) -> Self { + let (broadcast, _) = broadcast::channel(config.max_block_notify_buffer); + Self { + database, + executor, + verifier, + broadcast, + guard: tokio::sync::Semaphore::new(1), + } + } + + pub fn subscribe(&self) -> broadcast::Receiver> { + self.broadcast.subscribe() + } + + fn lock(&self) -> anyhow::Result { + let guard = self.guard.try_acquire(); + match guard { + Ok(permit) => Ok(permit), + Err(err) => { + tracing::error!( + "The semaphore was acquired before. It is a problem \ + because the current architecture doesn't expect that." + ); + return Err(anyhow!("The commit is already in the progress: {}", err)) + } + } + } +} + +impl Importer +where + IDatabase: ImporterDatabase, +{ + /// The method commits the result of the block execution attaching the consensus data. + /// It expects that the `UncommittedResult` contains the result of the block + /// execution(It includes the block itself), but not more. + /// + /// It doesn't do any checks regarding block validity(execution, fields, signing, etc.). + /// It only checks the validity of the database. + /// + /// After the commit into the database notifies about a new imported block. + /// + /// Only one commit may be in progress at the time. All other calls will be fail. + pub fn commit_result( + &self, + result: UncommittedResult>, + ) -> anyhow::Result<()> + where + EDatabase: ExecutorDatabase, + { + let _guard = self.lock()?; + self._commit_result(result) + } + + fn _commit_result( + &self, + result: UncommittedResult>, + ) -> anyhow::Result<()> + where + EDatabase: ExecutorDatabase, + { + let (result, mut db_tx) = result.into(); + let block = &result.sealed_block.entity; + let consensus = &result.sealed_block.consensus; + let block_id = block.id(); + let actual_next_height = *block.header().height(); + + // During importing of the genesis block, the database should not be initialized + // and the genesis block defines the next height. + // During the production of the non-genesis block, the next height should be underlying + // database height + 1. + let expected_next_height = match consensus { + Consensus::Genesis(_) => { + let result = self.database.latest_block_height(); + let found = !result.is_not_found(); + // Because the genesis block is not committed, it should return non found error. + // If we find the latest height, something is wrong with the state of the database. + if found { + return Err(anyhow!("The wrong state of database during insertion of the genesis block")) + } + actual_next_height + } + Consensus::PoA(_) => { + let last_db_height = self.database.latest_block_height()?; + last_db_height + .checked_add(1u32) + .ok_or_else(|| { + anyhow!("An overflow during the calculation of the next height") + })? + .into() + } + }; + + if expected_next_height != actual_next_height { + return Err(anyhow!( + "The actual height is {}, when the next expected height is {}", + actual_next_height.as_usize(), + expected_next_height.as_usize(), + )) + } + + let db_after_execution = db_tx.as_mut(); + + // Importer expects that `UncommittedResult` contains the result of block + // execution(It includes the block itself). + if db_after_execution.latest_block_height()? != actual_next_height { + return Err(anyhow!( + "`UncommittedResult` doesn't have the block at height {}", + actual_next_height + )) + } + + db_after_execution + .seal_block(&block_id, &result.sealed_block.consensus)? + .should_be_unique(&actual_next_height)?; + + // TODO: This should use a proper BMT MMR. Based on peaks stored in the database, + // we need to calculate a new root. The data type that will do that should live + // in the `fuel-core-storage` or `fuel-merkle` crate. + let root = ephemeral_merkle_root( + vec![*block.header().prev_root(), block_id.into()].iter(), + ); + db_after_execution + .insert_block_header_merkle_root(&actual_next_height, &root)? + .should_be_unique(&actual_next_height)?; + + db_tx.commit()?; + + let _ = self.broadcast.send(Arc::new(result)); + Ok(()) + } +} + +impl Importer +where + IDatabase: ImporterDatabase, + E: Executor, + V: BlockVerifier, +{ + /// Performs all checks required to commit the block, it includes the execution of + /// the block(As a result returns the uncommitted state). + /// + /// It validates only the `Block` execution rules and the `Block` fields' validity. + /// The validity of the `SealedBlock` and seal information is not the concern of this function. + /// + /// The method doesn't require synchronous access, so it could be called in a + /// concurrent environment. + /// + /// Returns `Err` if the block is invalid for committing. Otherwise, it returns the + /// `Ok` with the uncommitted state. + pub fn verify_block( + &self, + sealed_block: SealedBlock, + ) -> anyhow::Result>> { + let consensus = sealed_block.consensus; + let block = sealed_block.entity; + let sealed_block_id = block.id(); + + let result_of_verification = + self.verifier.verify_block_fields(&consensus, &block); + if let Err(err) = result_of_verification { + return Err(anyhow!("Some of the block fields are not valid: {}", err)) + } + + // TODO: Pass `block` into `ExecutionBlock::Validation` by ref + let ( + ExecutionResult { + block, + skipped_transactions, + tx_status, + }, + db_tx, + ) = self + .executor + .execute_without_commit(ExecutionBlock::Validation(block))? + .into(); + + // If we skipped transaction, it means that the block is invalid. + if !skipped_transactions.is_empty() { + return Err(anyhow!( + "It is not possible to skip transactions during importing of the block" + )) + } + + if block.id() != sealed_block_id { + // It should not be possible because, during validation, we don't touch the block. + // But while we pass it by value, let's check it. + return Err(anyhow!("Got another id after validation of the block")) + } + + let import_result = ImportResult { + sealed_block: Sealed { + entity: block, + consensus, + }, + tx_status, + }; + + Ok(Uncommitted::new(import_result, db_tx)) + } + + /// The method validates the `Block` fields and commits the `SealedBlock`. + /// It is a combination of the [`Importer::verify_block`] and [`Importer::commit_result`]. + pub fn execute_and_commit(&self, sealed_block: SealedBlock) -> anyhow::Result<()> { + let _guard = self.lock()?; + let result = self.verify_block(sealed_block)?; + self._commit_result(result) + } +} + +trait ShouldBeUnique { + fn should_be_unique(&self, height: &BlockHeight) -> anyhow::Result<()>; +} + +impl ShouldBeUnique for Option { + fn should_be_unique(&self, height: &BlockHeight) -> anyhow::Result<()> { + if self.is_some() { + return Err(anyhow!( + "The database already contains the data at the height {}", + height, + )) + } else { + Ok(()) + } + } +} diff --git a/crates/services/importer/src/lib.rs b/crates/services/importer/src/lib.rs index 456016fb48f..4ed7f57fb4e 100644 --- a/crates/services/importer/src/lib.rs +++ b/crates/services/importer/src/lib.rs @@ -1,7 +1,8 @@ #![deny(unused_crate_dependencies)] pub mod config; -pub mod service; +pub mod importer; +pub mod ports; pub use config::Config; -pub use service::Service; +pub use importer::Importer; diff --git a/crates/services/importer/src/ports.rs b/crates/services/importer/src/ports.rs new file mode 100644 index 00000000000..944a07a6a4d --- /dev/null +++ b/crates/services/importer/src/ports.rs @@ -0,0 +1,72 @@ +use fuel_core_storage::{ + transactional::StorageTransaction, + Result as StorageResult, +}; +use fuel_core_types::{ + blockchain::{ + block::Block, + consensus::Consensus, + primitives::{ + BlockHeight, + BlockId, + }, + }, + fuel_types::Bytes32, + services::executor::{ + ExecutionBlock, + Result as ExecutorResult, + UncommittedResult, + }, +}; + +/// The executors port. +pub trait Executor: Send + Sync { + /// The database used by the executor. + type Database: ExecutorDatabase; + + /// Executes the block and returns the result of execution with uncommitted database + /// transaction. + fn execute_without_commit( + &self, + block: ExecutionBlock, + ) -> ExecutorResult>>; +} + +/// The database port used by the block importer. +pub trait ImporterDatabase { + /// Returns the latest block height. + fn latest_block_height(&self) -> StorageResult; +} + +/// The port for returned database from the executor. +pub trait ExecutorDatabase: ImporterDatabase { + /// Assigns the `Consensus` data to the block under the `block_id`. + /// Return the previous value at the `height`, if any. + fn seal_block( + &mut self, + block_id: &BlockId, + consensus: &Consensus, + ) -> StorageResult>; + + /// Assigns the block header BMT MMR root to the block at the height `height`. + /// Return the previous value at the `height`, if any. + fn insert_block_header_merkle_root( + &mut self, + height: &BlockHeight, + root: &Bytes32, + ) -> StorageResult>; +} + +/// The verifier of the block. +pub trait BlockVerifier { + /// Verifies the consistency of the block fields for the block's height. + /// It includes the verification of **all** fields, it includes the consensus rules for + /// the corresponding height. + /// + /// Return an error if the verification failed, otherwise `Ok(())`. + fn verify_block_fields( + &self, + consensus: &Consensus, + block: &Block, + ) -> anyhow::Result<()>; +} diff --git a/crates/services/importer/src/service.rs b/crates/services/importer/src/service.rs deleted file mode 100644 index 3709bc9ace8..00000000000 --- a/crates/services/importer/src/service.rs +++ /dev/null @@ -1,40 +0,0 @@ -use crate::Config; -use parking_lot::Mutex; -use tokio::{ - sync::mpsc, - task::JoinHandle, -}; - -pub struct Service { - join: Mutex>>, - sender: mpsc::Sender<()>, -} - -impl Service { - pub async fn new(_config: &Config, _db: ()) -> anyhow::Result { - let (sender, _receiver) = mpsc::channel(100); - Ok(Self { - sender, - join: Mutex::new(None), - }) - } - - pub async fn start(&self) { - let mut join = self.join.lock(); - if join.is_none() { - *join = Some(tokio::spawn(async {})); - } - } - - pub async fn stop(&self) -> Option> { - let join = self.join.lock().take(); - if join.is_some() { - let _ = self.sender.send(()); - } - join - } - - pub fn sender(&self) -> &mpsc::Sender<()> { - &self.sender - } -} diff --git a/crates/services/producer/src/block_producer.rs b/crates/services/producer/src/block_producer.rs index 59484739697..f46a4877967 100644 --- a/crates/services/producer/src/block_producer.rs +++ b/crates/services/producer/src/block_producer.rs @@ -186,7 +186,6 @@ where time: Tai64::now(), generated: Default::default(), }, - metadata: None, }) } diff --git a/crates/services/producer/src/block_producer/tests.rs b/crates/services/producer/src/block_producer/tests.rs index 80355accb74..ffbbc6d0b5b 100644 --- a/crates/services/producer/src/block_producer/tests.rs +++ b/crates/services/producer/src/block_producer/tests.rs @@ -138,7 +138,6 @@ async fn cant_produce_if_previous_block_da_height_too_high() { height: prev_height, ..Default::default() }, - ..Default::default() }, transactions: vec![], } diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 3fab5ec4e55..7f0d3438ff8 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -17,6 +17,8 @@ pub use fuel_vm_private::{ pub mod iter; pub mod tables; +#[cfg(feature = "test-helpers")] +pub mod test_helpers; pub mod transactional; /// The storage result alias. @@ -53,6 +55,27 @@ impl From for ExecutorError { } } +/// The helper trait to work with storage errors. +pub trait IsNotFound { + /// Return `true` if the error is [`Error::NotFound`]. + fn is_not_found(&self) -> bool; +} + +impl IsNotFound for Error { + fn is_not_found(&self) -> bool { + matches!(self, Error::NotFound(_, _)) + } +} + +impl IsNotFound for Result { + fn is_not_found(&self) -> bool { + match self { + Err(err) => err.is_not_found(), + _ => false, + } + } +} + /// Creates `StorageError::NotFound` error with file and line information inside. /// /// # Examples diff --git a/crates/storage/src/test_helpers.rs b/crates/storage/src/test_helpers.rs new file mode 100644 index 00000000000..c7263b46d75 --- /dev/null +++ b/crates/storage/src/test_helpers.rs @@ -0,0 +1,38 @@ +//! The module to help with tests. + +use crate::{ + transactional::{ + StorageTransaction, + Transaction, + Transactional, + }, + Result, +}; + +/// The empty transactional storage. +#[derive(Default, Clone, Copy)] +pub struct EmptyStorage; + +impl AsRef for EmptyStorage { + fn as_ref(&self) -> &EmptyStorage { + self + } +} + +impl AsMut for EmptyStorage { + fn as_mut(&mut self) -> &mut EmptyStorage { + self + } +} + +impl Transactional for EmptyStorage { + fn transaction(&self) -> StorageTransaction { + StorageTransaction::new(EmptyStorage) + } +} + +impl Transaction for EmptyStorage { + fn commit(&mut self) -> Result<()> { + Ok(()) + } +} diff --git a/crates/types/src/blockchain/block.rs b/crates/types/src/blockchain/block.rs index 0bfbac77a39..556280a4283 100644 --- a/crates/types/src/blockchain/block.rs +++ b/crates/types/src/blockchain/block.rs @@ -128,6 +128,12 @@ impl CompressedBlock { impl Block { /// Get the hash of the header. pub fn id(&self) -> BlockId { + // The `Block` can be created only via the `Block::new` method, which calculates the + // identifier based on the header. So the block is immutable and can't change its + // identifier on the fly. + // + // This assertion is a double-checks that this behavior is not changed. + debug_assert_eq!(self.header.id(), self.header.hash()); self.header.id() } @@ -209,7 +215,6 @@ impl From for PartialFuelBlock { time, generated: Empty {}, }, - metadata: None, }, transactions, } diff --git a/crates/types/src/blockchain/consensus.rs b/crates/types/src/blockchain/consensus.rs index 85d8d874b6e..bd4ba171ab6 100644 --- a/crates/types/src/blockchain/consensus.rs +++ b/crates/types/src/blockchain/consensus.rs @@ -57,13 +57,14 @@ pub enum ConsensusType { PoA, } +/// A sealed entity with consensus info. #[derive(Clone, Debug)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] -/// A sealed entity with consensus info. pub struct Sealed { /// The actual value pub entity: Entity, + // TODO: Rename to `seal` /// Consensus info pub consensus: Consensus, } diff --git a/crates/types/src/blockchain/header.rs b/crates/types/src/blockchain/header.rs index ec8a4ddf2a4..085b03ae4e3 100644 --- a/crates/types/src/blockchain/header.rs +++ b/crates/types/src/blockchain/header.rs @@ -18,19 +18,18 @@ use crate::{ }; use tai64::Tai64; -#[derive(Clone, Debug)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] /// A fuel block header that has all the fields generated because it /// has been executed. +#[derive(Clone, Debug)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct BlockHeader { /// The application header. pub application: ApplicationHeader, /// The consensus header. pub consensus: ConsensusHeader, - /// Header Metadata - #[cfg_attr(feature = "serde", serde(skip))] - pub metadata: Option, + /// The header metadata calculated during creation. + /// The field is private to enforce the use of the [`PartialBlockHeader::generate`] method. + metadata: Option, } #[derive(Clone, Debug)] @@ -42,8 +41,6 @@ pub struct PartialBlockHeader { pub application: ApplicationHeader, /// The consensus header. pub consensus: ConsensusHeader, - /// Header Metadata - pub metadata: Option, } #[derive(Clone, Debug)] @@ -113,6 +110,19 @@ pub struct BlockHeaderMetadata { id: BlockId, } +#[cfg(any(test, feature = "test-helpers"))] +impl Default for BlockHeader { + fn default() -> Self { + let mut default = Self { + application: Default::default(), + consensus: Default::default(), + metadata: None, + }; + default.recalculate_metadata(); + default + } +} + // Accessors for the consensus header. impl BlockHeader { /// Merkle root of all previous block header hashes. @@ -161,11 +171,19 @@ impl PartialBlockHeader { impl BlockHeader { /// Re-generate the header metadata. pub fn recalculate_metadata(&mut self) { + let application_hash = self.application.hash(); + self.consensus.generated.application_hash = application_hash; self.metadata = Some(BlockHeaderMetadata { id: self.hash() }); } /// Get the hash of the fuel header. pub fn hash(&self) -> BlockId { + // The `BlockHeader` can be created only via the [`PartialBlockHeader::generate`] method, + // which calculates the hash of the `ApplicationHeader`. So the block header is immutable + // and can't change its final hash on the fly. + // + // This assertion is a double-checks that this behavior is not changed. + debug_assert_eq!(self.consensus.application_hash, self.application.hash()); // This internally hashes the hash of the application header. self.consensus.hash() } @@ -226,20 +244,21 @@ impl PartialBlockHeader { }, }; - // Generate the hash of the complete application header. - let application_hash = application.hash(); let mut header = BlockHeader { application, consensus: ConsensusHeader { prev_root: self.consensus.prev_root, height: self.consensus.height, time: self.consensus.time, - generated: GeneratedConsensusFields { application_hash }, + generated: GeneratedConsensusFields { + // Calculates it inside of `BlockHeader::recalculate_metadata`. + application_hash: Default::default(), + }, }, metadata: None, }; - // cache the hash. + // Cache the hash. header.recalculate_metadata(); header } @@ -256,7 +275,7 @@ fn generate_txns_root(transactions: &[Vec]) -> Bytes32 { impl ApplicationHeader { /// Hash the application header. - fn hash(&self) -> Bytes32 { + pub fn hash(&self) -> Bytes32 { // Order matters and is the same as the spec. let mut hasher = crate::fuel_crypto::Hasher::default(); hasher.input(&self.da_height.to_bytes()[..]); @@ -270,7 +289,7 @@ impl ApplicationHeader { impl ConsensusHeader { /// Hash the consensus header. - fn hash(&self) -> BlockId { + pub fn hash(&self) -> BlockId { // Order matters and is the same as the spec. let mut hasher = crate::fuel_crypto::Hasher::default(); hasher.input(self.prev_root.as_ref()); diff --git a/crates/types/src/services.rs b/crates/types/src/services.rs index a2cb62d622f..c54ed432d6b 100644 --- a/crates/types/src/services.rs +++ b/crates/types/src/services.rs @@ -1,8 +1,53 @@ //! Types for specific services +pub mod block_importer; pub mod executor; pub mod graphql_api; pub mod p2p; pub mod txpool; // TODO: Define a one common error for all services like + +/// The uncommitted `Result` of some action with database transaction. +/// The user should commit the result by itself. +#[derive(Debug)] +pub struct Uncommitted { + /// The result of the action. + result: Result, + /// The database transaction with not committed state. + database_transaction: DatabaseTransaction, +} + +impl Uncommitted { + /// Create a new instance of `Uncommitted`. + pub fn new(result: Result, database_transaction: DatabaseTransaction) -> Self { + Self { + result, + database_transaction, + } + } + + /// Returns a reference to the `Result`. + pub fn result(&self) -> &Result { + &self.result + } + + /// Return the result and database transaction. + /// + /// The caller can unpack the `Uncommitted`, apply some changes and pack it again into + /// `UncommittedResult`. Because `commit` of the database transaction consumes `self`, + /// after committing it is not possible create `Uncommitted`. + pub fn into(self) -> (Result, DatabaseTransaction) { + (self.result, self.database_transaction) + } + + /// Discards the database transaction and returns only the result of the action. + pub fn into_result(self) -> Result { + self.result + } + + /// Discards the result and return database transaction. + pub fn into_transaction(self) -> DatabaseTransaction { + self.database_transaction + } +} diff --git a/crates/types/src/services/block_importer.rs b/crates/types/src/services/block_importer.rs new file mode 100644 index 00000000000..c31375df8da --- /dev/null +++ b/crates/types/src/services/block_importer.rs @@ -0,0 +1,22 @@ +//! Types related to block importer service. + +use crate::{ + blockchain::SealedBlock, + services::{ + executor::TransactionExecutionStatus, + Uncommitted, + }, +}; + +/// The uncommitted result of the block importing. +pub type UncommittedResult = + Uncommitted; + +/// The result of the block import. +#[derive(Debug)] +pub struct ImportResult { + /// Imported sealed block. + pub sealed_block: SealedBlock, + /// The status of the transactions execution included into the block. + pub tx_status: Vec, +} diff --git a/crates/types/src/services/executor.rs b/crates/types/src/services/executor.rs index 17e46978218..54723d815ca 100644 --- a/crates/types/src/services/executor.rs +++ b/crates/types/src/services/executor.rs @@ -24,11 +24,15 @@ use crate::{ InterpreterError, ProgramState, }, + services::Uncommitted, }; use std::error::Error as StdError; /// The alias for executor result. pub type Result = core::result::Result; +/// The uncommitted result of teh transaction execution. +pub type UncommittedResult = + Uncommitted; /// The result of transactions execution. #[derive(Debug)] @@ -42,8 +46,8 @@ pub struct ExecutionResult { pub tx_status: Vec, } -#[derive(Debug, Clone)] /// The status of a transaction after it is executed. +#[derive(Debug, Clone)] pub struct TransactionExecutionStatus { /// The id of the transaction. pub id: Bytes32, @@ -51,8 +55,8 @@ pub struct TransactionExecutionStatus { pub result: TransactionExecutionResult, } -#[derive(Debug, Clone)] /// The result of transaction execution. +#[derive(Debug, Clone)] pub enum TransactionExecutionResult { /// Transaction was successfully executed. Success { @@ -68,53 +72,9 @@ pub enum TransactionExecutionResult { }, } -/// The uncommitted result of transactions execution with database transaction. -/// The caller should commit the result by itself. -#[derive(Debug)] -pub struct UncommittedResult { - /// The execution result. - result: ExecutionResult, - /// The database transaction with not committed state. - database_transaction: DbTransaction, -} - -impl UncommittedResult { - /// Create a new instance of `UncommittedResult`. - pub fn new(result: ExecutionResult, database_transaction: DbTransaction) -> Self { - Self { - result, - database_transaction, - } - } - - /// Returns a reference to the `ExecutionResult`. - pub fn result(&self) -> &ExecutionResult { - &self.result - } - - /// Return the result and database transaction. - /// - /// The service can unpack the `UncommittedResult`, apply some changes and pack it again into - /// `UncommittedResult`. Because `commit` of the database transaction consumes `self`, - /// after committing it is not possible create `UncommittedResult`. - pub fn into(self) -> (ExecutionResult, DbTransaction) { - (self.result, self.database_transaction) - } - - /// Discards the database transaction and returns only the result of execution. - pub fn into_result(self) -> ExecutionResult { - self.result - } - - /// Discards the result and return database transaction. - pub fn into_transaction(self) -> DbTransaction { - self.database_transaction - } -} - -#[derive(Debug, Clone, Copy)] /// Execution wrapper where the types /// depend on the type of execution. +#[derive(Debug, Clone, Copy)] pub enum ExecutionTypes { /// Production mode where P is being produced. Production(P), @@ -134,14 +94,6 @@ impl ExecutionBlock { ExecutionTypes::Validation(v) => Some(v.id()), } } - - /// Get the transaction root from the full [`FuelBlock`] if validating. - pub fn txs_root(&self) -> Option { - match self { - ExecutionTypes::Production(_) => None, - ExecutionTypes::Validation(v) => Some(v.header().transactions_root), - } - } } // TODO: Move `ExecutionType` and `ExecutionKind` into `fuel-core-executor` @@ -310,8 +262,6 @@ pub enum Error { Backtrace(Box), #[error("Transaction doesn't match expected result: {transaction_id:#x}")] InvalidTransactionOutcome { transaction_id: Bytes32 }, - #[error("Transaction root is invalid")] - InvalidTransactionRoot, #[error("The amount of charged fees is invalid")] InvalidFeeAmount, #[error("Block id is invalid")] diff --git a/tests/tests/blocks.rs b/tests/tests/blocks.rs index 4a8fe1eb814..43b3eb20f2f 100644 --- a/tests/tests/blocks.rs +++ b/tests/tests/blocks.rs @@ -16,21 +16,9 @@ use fuel_core_client::client::{ PageDirection, PaginationRequest, }; -use fuel_core_storage::{ - tables::{ - FuelBlocks, - SealedBlockConsensus, - }, - StorageAsMut, -}; use fuel_core_types::{ - blockchain::{ - block::CompressedBlock, - consensus::Consensus, - }, fuel_tx::*, secrecy::ExposeSecret, - tai64::Tai64, }; use itertools::{ rev, @@ -41,23 +29,15 @@ use std::ops::Deref; #[tokio::test] async fn block() { - // setup test data in the node - let block = CompressedBlock::default(); - let id = block.id(); - let mut db = Database::default(); - db.storage::().insert(&id, &block).unwrap(); - db.storage::() - .insert(&id, &Consensus::PoA(Default::default())) - .unwrap(); - // setup server & client - let srv = FuelService::from_database(db, Config::local_node()) + let srv = FuelService::from_database(Database::default(), Config::local_node()) .await .unwrap(); let client = FuelClient::from(srv.bound_address); // run test - let id_bytes: Bytes32 = id.into(); + let block = client.block_by_height(0).await.unwrap().unwrap(); + let id_bytes: Bytes32 = block.id.into(); let block = client .block(BlockId::from(id_bytes).to_string().as_str()) .await @@ -295,38 +275,16 @@ async fn block_connection_5( #[values(PageDirection::Forward, PageDirection::Backward)] pagination_direction: PageDirection, ) { - // blocks - let blocks = (0..10u32) - .map(|i| { - CompressedBlock::test( - fuel_core_types::blockchain::header::BlockHeader { - consensus: fuel_core_types::blockchain::header::ConsensusHeader { - height: i.into(), - time: Tai64(i.into()), - ..Default::default() - }, - ..Default::default() - }, - vec![], - ) - }) - .collect_vec(); - - // setup test data in the node - let mut db = Database::default(); - for block in blocks { - let id = block.id(); - db.storage::().insert(&id, &block).unwrap(); - db.storage::() - .insert(&id, &Consensus::PoA(Default::default())) - .unwrap(); - } + let mut config = Config::local_node(); + config.manual_blocks_enabled = true; // setup server & client - let srv = FuelService::from_database(db, Config::local_node()) + let srv = FuelService::from_database(Default::default(), config) .await .unwrap(); let client = FuelClient::from(srv.bound_address); + // setup test data in the node + client.produce_blocks(9, None).await.unwrap(); // run test let blocks = client From a6d835e54fc4f2ff60ae5255bac288f80c436db8 Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 22:58:31 +0000 Subject: [PATCH 02/33] Tested `commit_result` --- Cargo.lock | 3 + crates/services/importer/Cargo.toml | 10 +- crates/services/importer/src/importer.rs | 135 ++++-- crates/services/importer/src/importer/test.rs | 443 ++++++++++++++++++ crates/services/importer/src/ports.rs | 2 + crates/types/src/blockchain/block.rs | 2 +- crates/types/src/blockchain/consensus.rs | 8 +- crates/types/src/blockchain/consensus/poa.rs | 2 +- crates/types/src/blockchain/header.rs | 12 +- 9 files changed, 562 insertions(+), 55 deletions(-) create mode 100644 crates/services/importer/src/importer/test.rs diff --git a/Cargo.lock b/Cargo.lock index 4125c88ab5c..3c877514791 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2352,6 +2352,9 @@ dependencies = [ "anyhow", "fuel-core-storage", "fuel-core-types", + "mockall", + "test-case", + "thiserror", "tokio", "tracing", ] diff --git a/crates/services/importer/Cargo.toml b/crates/services/importer/Cargo.toml index a9f1670f7c4..87b61be7adf 100644 --- a/crates/services/importer/Cargo.toml +++ b/crates/services/importer/Cargo.toml @@ -11,7 +11,13 @@ description = "Fuel Block Importer" [dependencies] anyhow = "1.0" -fuel-core-storage = { path = "../../storage" } -fuel-core-types = { path = "../../types" } +fuel-core-storage = { path = "../../storage", version = "0.15.1" } +fuel-core-types = { path = "../../types", version = "0.15.1" } +thiserror = "1.0" tokio = { version = "1.21", features = ["full"] } tracing = "0.1" + +[dev-dependencies] +fuel-core-types = { path = "../../types", features = ["test-helpers"] } +mockall = "0.11" +test-case = "2.2" diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs index e403f1c414a..eaa3be0df83 100644 --- a/crates/services/importer/src/importer.rs +++ b/crates/services/importer/src/importer.rs @@ -7,9 +7,9 @@ use crate::{ }, Config, }; -use anyhow::anyhow; use fuel_core_storage::{ transactional::StorageTransaction, + Error as StorageError, IsNotFound, }; use fuel_core_types::{ @@ -18,7 +18,10 @@ use fuel_core_types::{ Consensus, Sealed, }, - primitives::BlockHeight, + primitives::{ + BlockHeight, + BlockId, + }, SealedBlock, }, fuel_vm::crypto::ephemeral_merkle_root, @@ -27,6 +30,7 @@ use fuel_core_types::{ ImportResult, UncommittedResult, }, + executor, executor::{ ExecutionBlock, ExecutionResult, @@ -35,7 +39,55 @@ use fuel_core_types::{ }, }; use std::sync::Arc; -use tokio::sync::broadcast; +use tokio::sync::{ + broadcast, + TryAcquireError, +}; + +#[cfg(test)] +pub mod test; + +#[derive(thiserror::Error, Debug)] +pub enum Error { + #[error("The commit is already in the progress: {0}.")] + SemaphoreError(TryAcquireError), + #[error("The wrong state of database during insertion of the genesis block.")] + InvalidUnderlyingDatabaseGenesisState, + #[error( + "The wrong state of database after execution of the block.\ + The actual height is {1}, when the next expected height is {0}." + )] + InvalidDatabaseStateAfterExecution(BlockHeight, BlockHeight), + #[error("Got overflow during increasing the height.")] + Overflow, + #[error("The non-generic block can't have zero height.")] + ZeroNonGenericHeight, + #[error("The actual height is {1}, when the next expected height is {0}.")] + IncorrectBlockHeight(BlockHeight, BlockHeight), + #[error( + "Got another block id after validation of the block. Expected {0} != Actual {1}" + )] + BlockIdMismatch(BlockId, BlockId), + #[error("Some of the block fields are not valid: {0}.")] + FailedVerification(anyhow::Error), + #[error("The execution of the block failed: {0}.")] + FailedExecution(executor::Error), + #[error("It is not possible to skip transactions during importing of the block.")] + SkippedTransactionsNotEmpty, + #[error("It is not possible to execute the genesis block.")] + ExecuteGenesis, + #[error("The database already contains the data at the height {0}.")] + NotUnique(BlockHeight), + #[error(transparent)] + StorageError(#[from] StorageError), +} + +#[cfg(test)] +impl PartialEq for Error { + fn eq(&self, other: &Self) -> bool { + format!("{:?}", self) == format!("{:?}", other) + } +} pub struct Importer { database: D, @@ -61,7 +113,7 @@ impl Importer { self.broadcast.subscribe() } - fn lock(&self) -> anyhow::Result { + pub(crate) fn lock(&self) -> Result { let guard = self.guard.try_acquire(); match guard { Ok(permit) => Ok(permit), @@ -70,7 +122,7 @@ impl Importer { "The semaphore was acquired before. It is a problem \ because the current architecture doesn't expect that." ); - return Err(anyhow!("The commit is already in the progress: {}", err)) + Err(Error::SemaphoreError(err)) } } } @@ -93,7 +145,7 @@ where pub fn commit_result( &self, result: UncommittedResult>, - ) -> anyhow::Result<()> + ) -> Result<(), Error> where EDatabase: ExecutorDatabase, { @@ -104,7 +156,7 @@ where fn _commit_result( &self, result: UncommittedResult>, - ) -> anyhow::Result<()> + ) -> Result<(), Error> where EDatabase: ExecutorDatabase, { @@ -125,26 +177,27 @@ where // Because the genesis block is not committed, it should return non found error. // If we find the latest height, something is wrong with the state of the database. if found { - return Err(anyhow!("The wrong state of database during insertion of the genesis block")) + return Err(Error::InvalidUnderlyingDatabaseGenesisState) } actual_next_height } Consensus::PoA(_) => { + if actual_next_height == BlockHeight::from(0u32) { + return Err(Error::ZeroNonGenericHeight) + } + let last_db_height = self.database.latest_block_height()?; last_db_height .checked_add(1u32) - .ok_or_else(|| { - anyhow!("An overflow during the calculation of the next height") - })? + .ok_or(Error::Overflow)? .into() } }; if expected_next_height != actual_next_height { - return Err(anyhow!( - "The actual height is {}, when the next expected height is {}", - actual_next_height.as_usize(), - expected_next_height.as_usize(), + return Err(Error::IncorrectBlockHeight( + expected_next_height, + actual_next_height, )) } @@ -152,16 +205,17 @@ where // Importer expects that `UncommittedResult` contains the result of block // execution(It includes the block itself). - if db_after_execution.latest_block_height()? != actual_next_height { - return Err(anyhow!( - "`UncommittedResult` doesn't have the block at height {}", - actual_next_height + let actual_height = db_after_execution.latest_block_height()?; + if expected_next_height != actual_height { + return Err(Error::InvalidDatabaseStateAfterExecution( + expected_next_height, + actual_height, )) } db_after_execution .seal_block(&block_id, &result.sealed_block.consensus)? - .should_be_unique(&actual_next_height)?; + .should_be_unique(&expected_next_height)?; // TODO: This should use a proper BMT MMR. Based on peaks stored in the database, // we need to calculate a new root. The data type that will do that should live @@ -170,8 +224,8 @@ where vec![*block.header().prev_root(), block_id.into()].iter(), ); db_after_execution - .insert_block_header_merkle_root(&actual_next_height, &root)? - .should_be_unique(&actual_next_height)?; + .insert_block_header_merkle_root(&expected_next_height, &root)? + .should_be_unique(&expected_next_height)?; db_tx.commit()?; @@ -197,10 +251,10 @@ where /// /// Returns `Err` if the block is invalid for committing. Otherwise, it returns the /// `Ok` with the uncommitted state. - pub fn verify_block( + pub fn verify_and_execute_block( &self, sealed_block: SealedBlock, - ) -> anyhow::Result>> { + ) -> Result>, Error> { let consensus = sealed_block.consensus; let block = sealed_block.entity; let sealed_block_id = block.id(); @@ -208,7 +262,11 @@ where let result_of_verification = self.verifier.verify_block_fields(&consensus, &block); if let Err(err) = result_of_verification { - return Err(anyhow!("Some of the block fields are not valid: {}", err)) + return Err(Error::FailedVerification(err)) + } + + if let Consensus::Genesis(_) = consensus { + return Err(Error::ExecuteGenesis) } // TODO: Pass `block` into `ExecutionBlock::Validation` by ref @@ -221,20 +279,20 @@ where db_tx, ) = self .executor - .execute_without_commit(ExecutionBlock::Validation(block))? + .execute_without_commit(ExecutionBlock::Validation(block)) + .map_err(Error::FailedExecution)? .into(); // If we skipped transaction, it means that the block is invalid. if !skipped_transactions.is_empty() { - return Err(anyhow!( - "It is not possible to skip transactions during importing of the block" - )) + return Err(Error::SkippedTransactionsNotEmpty) } - if block.id() != sealed_block_id { + 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. // But while we pass it by value, let's check it. - return Err(anyhow!("Got another id after validation of the block")) + return Err(Error::BlockIdMismatch(sealed_block_id, actual_block_id)) } let import_result = ImportResult { @@ -249,25 +307,22 @@ where } /// The method validates the `Block` fields and commits the `SealedBlock`. - /// It is a combination of the [`Importer::verify_block`] and [`Importer::commit_result`]. - pub fn execute_and_commit(&self, sealed_block: SealedBlock) -> anyhow::Result<()> { + /// It is a combination of the [`Importer::verify_and_execute_block`] and [`Importer::commit_result`]. + pub fn execute_and_commit(&self, sealed_block: SealedBlock) -> Result<(), Error> { let _guard = self.lock()?; - let result = self.verify_block(sealed_block)?; + let result = self.verify_and_execute_block(sealed_block)?; self._commit_result(result) } } trait ShouldBeUnique { - fn should_be_unique(&self, height: &BlockHeight) -> anyhow::Result<()>; + fn should_be_unique(&self, height: &BlockHeight) -> Result<(), Error>; } impl ShouldBeUnique for Option { - fn should_be_unique(&self, height: &BlockHeight) -> anyhow::Result<()> { + fn should_be_unique(&self, height: &BlockHeight) -> Result<(), Error> { if self.is_some() { - return Err(anyhow!( - "The database already contains the data at the height {}", - height, - )) + Err(Error::NotUnique(*height)) } else { Ok(()) } diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs new file mode 100644 index 00000000000..e00e0fb5cf1 --- /dev/null +++ b/crates/services/importer/src/importer/test.rs @@ -0,0 +1,443 @@ +use crate::{ + importer::Error, + ports::{ + ExecutorDatabase, + ImporterDatabase, + MockBlockVerifier, + MockExecutor, + }, + Importer, +}; +use anyhow::anyhow; +use fuel_core_storage::{ + not_found, + transactional::{ + StorageTransaction, + Transaction, + }, + Error as StorageError, + Result as StorageResult, +}; +use fuel_core_types::{ + blockchain::{ + block::Block, + consensus::Consensus, + primitives::{ + BlockHeight, + BlockId, + }, + SealedBlock, + }, + fuel_types::Bytes32, + services::{ + block_importer::{ + ImportResult, + UncommittedResult, + }, + executor::{ + ExecutionResult, + Result as ExecutorResult, + }, + Uncommitted, + }, +}; +use test_case::test_case; +use tokio::sync::{ + broadcast::error::TryRecvError, + TryAcquireError, +}; + +mockall::mock! { + pub Database {} + + impl ImporterDatabase for Database { + fn latest_block_height(&self) -> StorageResult; + } + + impl ExecutorDatabase for Database { + fn seal_block( + &mut self, + block_id: &BlockId, + consensus: &Consensus, + ) -> StorageResult>; + + fn insert_block_header_merkle_root( + &mut self, + height: &BlockHeight, + root: &Bytes32, + ) -> StorageResult>; + } + + impl Transaction for Database { + fn commit(&mut self) -> StorageResult<()>; + } +} + +impl AsMut for MockDatabase { + fn as_mut(&mut self) -> &mut MockDatabase { + self + } +} + +impl AsRef for MockDatabase { + fn as_ref(&self) -> &MockDatabase { + self + } +} + +fn genesis(height: u32) -> SealedBlock { + let mut block = Block::default(); + block.header_mut().consensus.height = height.into(); + block.header_mut().recalculate_metadata(); + + SealedBlock { + entity: block, + consensus: Consensus::Genesis(Default::default()), + } +} + +fn poa_block(height: u32) -> SealedBlock { + let mut block = Block::default(); + block.header_mut().consensus.height = height.into(); + block.header_mut().recalculate_metadata(); + + SealedBlock { + entity: block, + consensus: Consensus::PoA(Default::default()), + } +} + +fn underlying_db(result: R) -> impl Fn() -> MockDatabase +where + R: Fn() -> StorageResult + Send + Clone + 'static, +{ + move || { + let result = result.clone(); + let mut db = MockDatabase::default(); + db.expect_latest_block_height() + .returning(move || result().map(Into::into)); + db + } +} + +fn executor_db( + height: H, + seal: S, + root: R, + commit: usize, +) -> impl Fn() -> MockDatabase +where + H: Fn() -> StorageResult + Send + Clone + 'static, + S: Fn() -> StorageResult> + Send + Clone + 'static, + R: Fn() -> StorageResult> + Send + Clone + 'static, +{ + move || { + let height = height.clone(); + let seal = seal.clone(); + let root = root.clone(); + let mut db = MockDatabase::default(); + db.expect_latest_block_height() + .returning(move || height().map(Into::into)); + db.expect_seal_block().returning(move |_, _| seal()); + db.expect_insert_block_header_merkle_root() + .returning(move |_, _| root()); + db.expect_commit().times(commit).returning(|| Ok(())); + + db + } +} + +fn ok(entity: T) -> impl Fn() -> Result + Clone { + move || Ok(entity.clone()) +} + +fn not_found() -> StorageResult { + Err(not_found!("Not found")) +} + +fn failure() -> StorageResult { + Err(StorageError::Other(anyhow!("Some failure"))) +} + +fn storage_failure_error() -> Error { + Error::StorageError(StorageError::Other(anyhow!("Some failure"))) +} + +fn executor(block: B, database: MockDatabase) -> MockExecutor +where + B: Fn() -> ExecutorResult + Send + 'static, +{ + let mut executor = MockExecutor::default(); + executor + .expect_execute_without_commit() + .return_once(move |_| { + Ok(Uncommitted::new( + ExecutionResult { + block: block()?, + skipped_transactions: vec![], + tx_status: vec![], + }, + StorageTransaction::new(database), + )) + }); + + executor +} + +fn verifier(result: R) -> MockBlockVerifier +where + R: Fn() -> anyhow::Result<()> + Send + 'static, +{ + let mut verifier = MockBlockVerifier::default(); + verifier + .expect_verify_block_fields() + .return_once(move |_, _| result()); + + verifier +} + +//////////////// SealedBlock, UnderlyingDB, ExecutionDB /////////////// +//////////////// //////////// Genesis Block /////////// //////////////// +#[test_case( + genesis(0), + underlying_db(not_found), + executor_db(ok(0), ok(None), ok(None), 1) + => Ok(()) +)] +#[test_case( + genesis(113), + underlying_db(not_found), + executor_db(ok(113), ok(None), ok(None), 1) + => Ok(()) +)] +#[test_case( + genesis(0), + underlying_db(failure), + executor_db(ok(0), ok(None), ok(None), 0) + => Err(Error::InvalidUnderlyingDatabaseGenesisState) +)] +#[test_case( + genesis(0), + underlying_db(ok(0)), + executor_db(ok(0), ok(None), ok(None), 0) + => Err(Error::InvalidUnderlyingDatabaseGenesisState) +)] +#[test_case( + genesis(1), + underlying_db(not_found), + executor_db(ok(0), ok(None), ok(None), 0) + => Err(Error::InvalidDatabaseStateAfterExecution(1u32.into(), 0u32.into())) +)] +#[test_case( + genesis(0), + underlying_db(not_found), + executor_db(ok(0), ok(Some(Default::default())), ok(None), 0) + => Err(Error::NotUnique(0u32.into())) +)] +#[test_case( + genesis(0), + underlying_db(not_found), + executor_db(ok(0), ok(None), ok(Some(Default::default())), 0) + => Err(Error::NotUnique(0u32.into())) +)] +fn commit_result_genesis( + sealed_block: SealedBlock, + underlying_db: impl Fn() -> MockDatabase, + executor_db: impl Fn() -> MockDatabase, +) -> Result<(), Error> { + commit_result_assert(sealed_block.clone(), underlying_db(), executor_db()) +} + +//////////////////////////// PoA Block //////////////////////////// +#[test_case( + poa_block(1), + underlying_db(ok(0)), + executor_db(ok(1), ok(None), ok(None), 1) + => Ok(()) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(ok(113), ok(None), ok(None), 1) + => Ok(()) +)] +#[test_case( + poa_block(0), + underlying_db(ok(0)), + executor_db(ok(1), ok(None), ok(None), 0) + => Err(Error::ZeroNonGenericHeight) +)] +#[test_case( + poa_block(113), + underlying_db(ok(111)), + executor_db(ok(113), ok(None), ok(None), 0) + => Err(Error::IncorrectBlockHeight(112u32.into(), 113u32.into())) +)] +#[test_case( + poa_block(113), + underlying_db(ok(114)), + executor_db(ok(113), ok(None), ok(None), 0) + => Err(Error::IncorrectBlockHeight(115u32.into(), 113u32.into())) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(ok(114), ok(None), ok(None), 0) + => Err(Error::InvalidDatabaseStateAfterExecution(113u32.into(), 114u32.into())) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(failure, ok(None), ok(None), 0) + => Err(storage_failure_error()) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(ok(113), ok(Some(Default::default())), ok(None), 0) + => Err(Error::NotUnique(113u32.into())) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(ok(113), ok(None), ok(Some(Default::default())), 0) + => Err(Error::NotUnique(113u32.into())) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(ok(113), failure, ok(None), 0) + => Err(storage_failure_error()) +)] +#[test_case( + poa_block(113), + underlying_db(ok(112)), + executor_db(ok(113), ok(None), failure, 0) + => Err(storage_failure_error()) +)] +fn commit_result_poa( + sealed_block: SealedBlock, + underlying_db: impl Fn() -> MockDatabase, + executor_db: impl Fn() -> MockDatabase, +) -> Result<(), Error> { + // `execute_and_commit` and `commit_result` should have the same + // validation rules(-> test cases) during committing the result. + let commit_result = + commit_result_assert(sealed_block.clone(), underlying_db(), executor_db()); + let execute_and_commit_result = execute_and_commit_assert( + sealed_block.clone(), + underlying_db(), + executor(ok(sealed_block.entity.clone()), executor_db()), + verifier(ok(())), + ); + assert_eq!(commit_result, execute_and_commit_result); + commit_result +} + +fn commit_result_assert( + sealed_block: SealedBlock, + underlying_db: MockDatabase, + executor_db: MockDatabase, +) -> Result<(), Error> { + let expected_to_broadcast = sealed_block.clone(); + let importer = Importer::new(Default::default(), underlying_db, (), ()); + let uncommitted_result = UncommittedResult::new( + ImportResult { + sealed_block, + tx_status: vec![], + }, + StorageTransaction::new(executor_db), + ); + + let mut imported_blocks = importer.subscribe(); + let result = importer.commit_result(uncommitted_result); + + if result.is_ok() { + let actual_sealed_block = imported_blocks.try_recv().unwrap(); + assert_eq!(actual_sealed_block.sealed_block, expected_to_broadcast); + assert_eq!( + imported_blocks + .try_recv() + .expect_err("We should broadcast only one block"), + TryRecvError::Empty + ) + } + + result +} + +fn execute_and_commit_assert( + sealed_block: SealedBlock, + underlying_db: MockDatabase, + executor: MockExecutor, + verifier: MockBlockVerifier, +) -> Result<(), Error> { + let expected_to_broadcast = sealed_block.clone(); + let importer = Importer::new(Default::default(), underlying_db, executor, verifier); + + let mut imported_blocks = importer.subscribe(); + let result = importer.execute_and_commit(sealed_block); + + if result.is_ok() { + let actual_sealed_block = imported_blocks.try_recv().unwrap(); + assert_eq!(actual_sealed_block.sealed_block, expected_to_broadcast); + assert_eq!( + imported_blocks + .try_recv() + .expect_err("We should broadcast only one block"), + TryRecvError::Empty + ) + } + + result +} + +#[test] +fn commit_result_fail_when_locked() { + let importer = Importer::new(Default::default(), MockDatabase::default(), (), ()); + let uncommitted_result = UncommittedResult::new( + ImportResult { + sealed_block: Default::default(), + tx_status: vec![], + }, + StorageTransaction::new(MockDatabase::default()), + ); + + let _guard = importer.lock(); + assert_eq!( + importer.commit_result(uncommitted_result), + Err(Error::SemaphoreError(TryAcquireError::NoPermits)) + ); +} + +#[test] +fn execute_and_commit_fail_when_locked() { + let importer = Importer::new( + Default::default(), + MockDatabase::default(), + MockExecutor::default(), + MockBlockVerifier::default(), + ); + + let _guard = importer.lock(); + assert_eq!( + importer.execute_and_commit(Default::default()), + Err(Error::SemaphoreError(TryAcquireError::NoPermits)) + ); +} + +#[test] +fn one_lock_at_the_same_time() { + let importer = Importer::new( + Default::default(), + MockDatabase::default(), + MockExecutor::default(), + MockBlockVerifier::default(), + ); + + let _guard = importer.lock(); + assert_eq!( + importer.lock().map(|_| ()), + Err(Error::SemaphoreError(TryAcquireError::NoPermits)) + ); +} diff --git a/crates/services/importer/src/ports.rs b/crates/services/importer/src/ports.rs index 944a07a6a4d..b51fb440877 100644 --- a/crates/services/importer/src/ports.rs +++ b/crates/services/importer/src/ports.rs @@ -19,6 +19,7 @@ use fuel_core_types::{ }, }; +#[cfg_attr(test, mockall::automock(type Database = crate::importer::test::MockDatabase;))] /// The executors port. pub trait Executor: Send + Sync { /// The database used by the executor. @@ -57,6 +58,7 @@ pub trait ExecutorDatabase: ImporterDatabase { ) -> StorageResult>; } +#[cfg_attr(test, mockall::automock)] /// The verifier of the block. pub trait BlockVerifier { /// Verifies the consistency of the block fields for the block's height. diff --git a/crates/types/src/blockchain/block.rs b/crates/types/src/blockchain/block.rs index 556280a4283..95d38e10cc0 100644 --- a/crates/types/src/blockchain/block.rs +++ b/crates/types/src/blockchain/block.rs @@ -26,7 +26,7 @@ use crate::{ }; /// Fuel block with all transaction data included -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] pub struct Block { diff --git a/crates/types/src/blockchain/consensus.rs b/crates/types/src/blockchain/consensus.rs index bd4ba171ab6..905124f1a50 100644 --- a/crates/types/src/blockchain/consensus.rs +++ b/crates/types/src/blockchain/consensus.rs @@ -18,7 +18,7 @@ pub mod poa; use poa::PoAConsensus; -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] /// The consensus related data that doesn't live on the /// header. @@ -43,7 +43,6 @@ impl Consensus { } } -#[cfg(any(test, feature = "test-helpers"))] impl Default for Consensus { fn default() -> Self { Consensus::PoA(Default::default()) @@ -58,9 +57,8 @@ pub enum ConsensusType { } /// A sealed entity with consensus info. -#[derive(Clone, Debug)] +#[derive(Default, Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] pub struct Sealed { /// The actual value pub entity: Entity, @@ -87,7 +85,7 @@ pub struct ConsensusVote { /// network - contracts states, contracts balances, unspent coins, and messages. It also contains /// the hash on the initial config of the network that defines the consensus rules for following /// blocks. -#[derive(Clone, Debug, Default)] +#[derive(Clone, Debug, Default, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct Genesis { /// The chain config define what consensus type to use, what settlement layer to use, diff --git a/crates/types/src/blockchain/consensus/poa.rs b/crates/types/src/blockchain/consensus/poa.rs index 7d3abd12b5b..92192f43ad8 100644 --- a/crates/types/src/blockchain/consensus/poa.rs +++ b/crates/types/src/blockchain/consensus/poa.rs @@ -2,7 +2,7 @@ use crate::fuel_crypto::Signature; -#[derive(Clone, Debug, Default)] +#[derive(Default, Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] /// The consensus related data that doesn't live on the /// header. diff --git a/crates/types/src/blockchain/header.rs b/crates/types/src/blockchain/header.rs index 085b03ae4e3..1ae4acf2719 100644 --- a/crates/types/src/blockchain/header.rs +++ b/crates/types/src/blockchain/header.rs @@ -20,7 +20,7 @@ use tai64::Tai64; /// A fuel block header that has all the fields generated because it /// has been executed. -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct BlockHeader { /// The application header. @@ -43,7 +43,7 @@ pub struct PartialBlockHeader { pub consensus: ConsensusHeader, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] /// The fuel block application header. @@ -60,7 +60,7 @@ pub struct ApplicationHeader { pub generated: Generated, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] /// Concrete generated application header fields. @@ -76,7 +76,7 @@ pub struct GeneratedApplicationFields { pub output_messages_root: Bytes32, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] /// The fuel block consensus header. /// This contains fields related to consensus plus @@ -92,7 +92,7 @@ pub struct ConsensusHeader { pub generated: Generated, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] /// Concrete generated consensus header fields. @@ -102,7 +102,7 @@ pub struct GeneratedConsensusFields { pub application_hash: Bytes32, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] /// Extra data that is not actually part of the header. pub struct BlockHeaderMetadata { From 0b71b7f0f76e7ae9dd9e994e2c22d9e8a7c138ae Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 22:59:17 +0000 Subject: [PATCH 03/33] Make clippy happy --- crates/services/importer/src/importer/test.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index e00e0fb5cf1..8f7f5a3a97c 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -245,7 +245,7 @@ fn commit_result_genesis( underlying_db: impl Fn() -> MockDatabase, executor_db: impl Fn() -> MockDatabase, ) -> Result<(), Error> { - commit_result_assert(sealed_block.clone(), underlying_db(), executor_db()) + commit_result_assert(sealed_block, underlying_db(), executor_db()) } //////////////////////////// PoA Block //////////////////////////// @@ -327,7 +327,7 @@ fn commit_result_poa( let execute_and_commit_result = execute_and_commit_assert( sealed_block.clone(), underlying_db(), - executor(ok(sealed_block.entity.clone()), executor_db()), + executor(ok(sealed_block.entity), executor_db()), verifier(ok(())), ); assert_eq!(commit_result, execute_and_commit_result); From d7d7a6c03a2ec1c0b8240ffe75465f2f188a8fdb Mon Sep 17 00:00:00 2001 From: green Date: Thu, 19 Jan 2023 00:31:45 +0000 Subject: [PATCH 04/33] Added tests for `verify_and_execute_block` part of the importer --- crates/services/importer/src/importer.rs | 11 +- crates/services/importer/src/importer/test.rs | 151 ++++++++++++++++-- 2 files changed, 145 insertions(+), 17 deletions(-) diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs index eaa3be0df83..229637b8623 100644 --- a/crates/services/importer/src/importer.rs +++ b/crates/services/importer/src/importer.rs @@ -236,7 +236,6 @@ where impl Importer where - IDatabase: ImporterDatabase, E: Executor, V: BlockVerifier, { @@ -265,6 +264,9 @@ where return Err(Error::FailedVerification(err)) } + // The current code has a separate function X to process `StateConfig`. + // It is not possible to execute it via `Executor`. + // Maybe we need consider to move the function X here, if that possible. if let Consensus::Genesis(_) = consensus { return Err(Error::ExecuteGenesis) } @@ -305,7 +307,14 @@ where Ok(Uncommitted::new(import_result, db_tx)) } +} +impl Importer +where + IDatabase: ImporterDatabase, + E: Executor, + V: BlockVerifier, +{ /// The method validates the `Block` fields and commits the `SealedBlock`. /// It is a combination of the [`Importer::verify_and_execute_block`] and [`Importer::commit_result`]. pub fn execute_and_commit(&self, sealed_block: SealedBlock) -> Result<(), Error> { diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index 8f7f5a3a97c..5222f209f25 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -13,7 +13,7 @@ use fuel_core_storage::{ not_found, transactional::{ StorageTransaction, - Transaction, + Transaction as TransactionTrait, }, Error as StorageError, Result as StorageResult, @@ -28,6 +28,7 @@ use fuel_core_types::{ }, SealedBlock, }, + fuel_tx::Transaction, fuel_types::Bytes32, services::{ block_importer::{ @@ -35,6 +36,7 @@ use fuel_core_types::{ UncommittedResult, }, executor::{ + Error as ExecutorError, ExecutionResult, Result as ExecutorResult, }, @@ -68,7 +70,7 @@ mockall::mock! { ) -> StorageResult>; } - impl Transaction for Database { + impl TransactionTrait for Database { fn commit(&mut self) -> StorageResult<()>; } } @@ -85,6 +87,12 @@ impl AsRef for MockDatabase { } } +#[derive(Clone, Debug)] +struct MockExecutionResult { + block: SealedBlock, + skipped_transactions: usize, +} + fn genesis(height: u32) -> SealedBlock { let mut block = Block::default(); block.header_mut().consensus.height = height.into(); @@ -124,7 +132,7 @@ fn executor_db( height: H, seal: S, root: R, - commit: usize, + commits: usize, ) -> impl Fn() -> MockDatabase where H: Fn() -> StorageResult + Send + Clone + 'static, @@ -141,7 +149,7 @@ where db.expect_seal_block().returning(move |_, _| seal()); db.expect_insert_block_header_merkle_root() .returning(move |_, _| root()); - db.expect_commit().times(commit).returning(|| Ok(())); + db.expect_commit().times(commits).returning(|| Ok(())); db } @@ -155,7 +163,7 @@ fn not_found() -> StorageResult { Err(not_found!("Not found")) } -fn failure() -> StorageResult { +fn storage_failure() -> StorageResult { Err(StorageError::Other(anyhow!("Some failure"))) } @@ -163,18 +171,37 @@ fn storage_failure_error() -> Error { Error::StorageError(StorageError::Other(anyhow!("Some failure"))) } -fn executor(block: B, database: MockDatabase) -> MockExecutor +fn ex_result(height: u32, skipped_transactions: usize) -> MockExecutionResult { + MockExecutionResult { + block: poa_block(height), + skipped_transactions, + } +} + +fn execution_failure() -> ExecutorResult { + Err(ExecutorError::InvalidBlockId) +} + +fn execution_failure_error() -> Error { + Error::FailedExecution(ExecutorError::InvalidBlockId) +} + +fn executor(result: R, database: MockDatabase) -> MockExecutor where - B: Fn() -> ExecutorResult + Send + 'static, + R: Fn() -> ExecutorResult + Send + 'static, { let mut executor = MockExecutor::default(); executor .expect_execute_without_commit() .return_once(move |_| { + let mock_result = result()?; + let skipped_transactions: Vec<_> = (0..mock_result.skipped_transactions) + .map(|_| (Transaction::default(), ExecutorError::InvalidBlockId)) + .collect(); Ok(Uncommitted::new( ExecutionResult { - block: block()?, - skipped_transactions: vec![], + block: mock_result.block.entity, + skipped_transactions, tx_status: vec![], }, StorageTransaction::new(database), @@ -184,6 +211,14 @@ where executor } +fn verification_failure() -> anyhow::Result { + Err(anyhow!("Not verified")) +} + +fn verification_failure_error() -> Error { + Error::FailedVerification(anyhow!("Not verified")) +} + fn verifier(result: R) -> MockBlockVerifier where R: Fn() -> anyhow::Result<()> + Send + 'static, @@ -212,7 +247,7 @@ where )] #[test_case( genesis(0), - underlying_db(failure), + underlying_db(storage_failure), executor_db(ok(0), ok(None), ok(None), 0) => Err(Error::InvalidUnderlyingDatabaseGenesisState) )] @@ -288,7 +323,7 @@ fn commit_result_genesis( #[test_case( poa_block(113), underlying_db(ok(112)), - executor_db(failure, ok(None), ok(None), 0) + executor_db(storage_failure, ok(None), ok(None), 0) => Err(storage_failure_error()) )] #[test_case( @@ -306,28 +341,29 @@ fn commit_result_genesis( #[test_case( poa_block(113), underlying_db(ok(112)), - executor_db(ok(113), failure, ok(None), 0) + executor_db(ok(113), storage_failure, ok(None), 0) => Err(storage_failure_error()) )] #[test_case( poa_block(113), underlying_db(ok(112)), - executor_db(ok(113), ok(None), failure, 0) + executor_db(ok(113), ok(None), storage_failure, 0) => Err(storage_failure_error()) )] -fn commit_result_poa( +fn commit_result_and_execute_and_commit_poa( sealed_block: SealedBlock, underlying_db: impl Fn() -> MockDatabase, executor_db: impl Fn() -> MockDatabase, ) -> 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 commit_result = commit_result_assert(sealed_block.clone(), underlying_db(), executor_db()); let execute_and_commit_result = execute_and_commit_assert( - sealed_block.clone(), + sealed_block, underlying_db(), - executor(ok(sealed_block.entity), executor_db()), + executor(ok(ex_result(height.into(), 0)), executor_db()), verifier(ok(())), ); assert_eq!(commit_result, execute_and_commit_result); @@ -441,3 +477,86 @@ fn one_lock_at_the_same_time() { Err(Error::SemaphoreError(TryAcquireError::NoPermits)) ); } + +///////// New block, Block After Execution, Verification result, commits ///////// +#[test_case( + genesis(113), ok(ex_result(113, 0)), ok(()), 0 + => Err(Error::ExecuteGenesis) +)] +#[test_case( + poa_block(1), ok(ex_result(1, 0)), ok(()), 1 + => Ok(()) +)] +#[test_case( + poa_block(113), ok(ex_result(113, 0)), ok(()), 1 + => Ok(()) +)] +#[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())) +)] +#[test_case( + poa_block(113), execution_failure, ok(()), 0 + => Err(execution_failure_error()) +)] +#[test_case( + poa_block(113), ok(ex_result(113, 1)), ok(()), 0 + => Err(Error::SkippedTransactionsNotEmpty) +)] +#[test_case( + poa_block(113), ok(ex_result(113, 0)), verification_failure, 0 + => Err(verification_failure_error()) +)] +fn execute_and_commit_and_verify_and_execute_block_poa( + sealed_block: SealedBlock, + block_after_execution: P, + verifier_result: V, + commits: usize, +) -> Result<(), Error> +where + 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 + // validation rules(-> test cases) during verification. + let verify_and_execute_result = verify_and_execute_assert( + sealed_block.clone(), + block_after_execution.clone(), + verifier_result.clone(), + ); + + // We tested commit part in the `commit_result_and_execute_and_commit_poa` so setup the + // databases to always pass the committing part. + let expected_height: u32 = sealed_block.entity.header().consensus.height.into(); + let previous_height = expected_height.checked_sub(1).unwrap_or_default(); + let execute_and_commit_result = execute_and_commit_assert( + sealed_block, + underlying_db(ok(previous_height))(), + executor( + block_after_execution, + executor_db(ok(expected_height), ok(None), ok(None), commits)(), + ), + verifier(verifier_result), + ); + assert_eq!(verify_and_execute_result, execute_and_commit_result); + execute_and_commit_result +} + +fn verify_and_execute_assert( + sealed_block: SealedBlock, + block_after_execution: P, + verifier_result: V, +) -> Result<(), Error> +where + P: Fn() -> ExecutorResult + Send + 'static, + V: Fn() -> anyhow::Result<()> + Send + 'static, +{ + let importer = Importer::new( + Default::default(), + MockDatabase::default(), + executor(block_after_execution, MockDatabase::default()), + verifier(verifier_result), + ); + + importer.verify_and_execute_block(sealed_block).map(|_| ()) +} From d60971a0eedfa3172f343df72c5e0d68094d160d Mon Sep 17 00:00:00 2001 From: green Date: Thu, 19 Jan 2023 00:34:52 +0000 Subject: [PATCH 05/33] Added test for lock for `verify_and_execute_block` --- crates/services/importer/src/importer/test.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index 5222f209f25..67e4d9c9b46 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -560,3 +560,16 @@ where importer.verify_and_execute_block(sealed_block).map(|_| ()) } + +#[test] +fn verify_and_execute_allowed_when_locked() { + let importer = Importer::new( + Default::default(), + MockDatabase::default(), + executor(ok(ex_result(13, 0)), MockDatabase::default()), + verifier(ok(())), + ); + + let _guard = importer.lock(); + assert!(importer.verify_and_execute_block(poa_block(13)).is_ok()); +} From aa720f3f683109e7ec02a402559b678e6e778f2e Mon Sep 17 00:00:00 2001 From: Green Baneling Date: Thu, 19 Jan 2023 13:04:36 +0000 Subject: [PATCH 06/33] Update crates/services/importer/src/importer.rs Co-authored-by: Tom --- crates/services/importer/src/importer.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs index 229637b8623..78af829ddf1 100644 --- a/crates/services/importer/src/importer.rs +++ b/crates/services/importer/src/importer.rs @@ -142,6 +142,8 @@ where /// After the commit into the database notifies about a new imported block. /// /// Only one commit may be in progress at the time. All other calls will be fail. + /// # Concurrency + /// Returns an error if called while another call is in progress. pub fn commit_result( &self, result: UncommittedResult>, From 7e3ba96dac6c10ea9273666c3f94ccae1c2c634d Mon Sep 17 00:00:00 2001 From: green Date: Thu, 19 Jan 2023 13:14:49 +0000 Subject: [PATCH 07/33] Make fmt happy --- crates/services/importer/src/importer.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/services/importer/src/importer.rs b/crates/services/importer/src/importer.rs index 78af829ddf1..c325bfc07c6 100644 --- a/crates/services/importer/src/importer.rs +++ b/crates/services/importer/src/importer.rs @@ -141,9 +141,10 @@ where /// /// After the commit into the database notifies about a new imported block. /// - /// Only one commit may be in progress at the time. All other calls will be fail. /// # Concurrency - /// Returns an error if called while another call is in progress. + /// + /// Only one commit may be in progress at the time. All other calls will be fail. + /// Returns an error if called while another call is in progress. pub fn commit_result( &self, result: UncommittedResult>, From 3be7239e4c5fdd0fc025c81d9b440fca6c6c5ac1 Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 15:23:26 +0000 Subject: [PATCH 08/33] Added verifeir for the block --- Cargo.lock | 17 ++- Cargo.toml | 1 + bin/fuel-core/src/cli/run.rs | 23 +++ crates/chain-config/Cargo.toml | 2 +- crates/chain-config/src/config/chain.rs | 39 ++++- crates/fuel-core/Cargo.toml | 2 + crates/fuel-core/src/service/config.rs | 51 ++++++- crates/services/consensus_module/Cargo.toml | 17 +++ .../services/consensus_module/poa/Cargo.toml | 6 +- .../consensus_module/poa/src/config.rs | 33 +---- .../services/consensus_module/poa/src/lib.rs | 1 + .../consensus_module/poa/src/service.rs | 2 + .../consensus_module/poa/src/verifier.rs | 135 ++++++++++++++++++ .../consensus_module/src/block_verifier.rs | 88 ++++++++++++ .../src/block_verifier/config.rs | 32 +++++ crates/services/consensus_module/src/lib.rs | 7 + tests/tests/trigger_integration/hybrid.rs | 9 +- tests/tests/tx/txn_status_subscription.rs | 12 +- 18 files changed, 429 insertions(+), 48 deletions(-) create mode 100644 crates/services/consensus_module/Cargo.toml create mode 100644 crates/services/consensus_module/poa/src/verifier.rs create mode 100644 crates/services/consensus_module/src/block_verifier.rs create mode 100644 crates/services/consensus_module/src/block_verifier/config.rs create mode 100644 crates/services/consensus_module/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index 3c877514791..4071c79dc90 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2203,8 +2203,10 @@ dependencies = [ "derive_more", "enum-iterator", "fuel-core-chain-config", + "fuel-core-consensus-module", "fuel-core-database", "fuel-core-executor", + "fuel-core-importer", "fuel-core-metrics", "fuel-core-p2p", "fuel-core-poa", @@ -2279,10 +2281,10 @@ version = "0.15.1" dependencies = [ "anyhow", "bincode", - "fuel-core-poa", "fuel-core-storage", "fuel-core-types", "hex", + "humantime-serde", "insta", "itertools", "rand 0.8.5", @@ -2325,6 +2327,16 @@ dependencies = [ "tokio", ] +[[package]] +name = "fuel-core-consensus-module" +version = "0.15.1" +dependencies = [ + "anyhow", + "fuel-core-chain-config", + "fuel-core-poa", + "fuel-core-types", +] + [[package]] name = "fuel-core-database" version = "0.15.1" @@ -2427,13 +2439,12 @@ version = "0.15.1" dependencies = [ "anyhow", "async-trait", + "fuel-core-chain-config", "fuel-core-services", "fuel-core-storage", "fuel-core-types", - "humantime-serde", "mockall", "rand 0.8.5", - "serde", "tokio", "tokio-stream", "tracing", diff --git a/Cargo.toml b/Cargo.toml index b1868e5b5aa..20b4a0df6ec 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,6 +12,7 @@ members = [ "crates/fuel-core", "crates/metrics", "crates/services", + "crates/services/consensus_module", "crates/services/consensus_module/bft", "crates/services/consensus_module/poa", "crates/services/executor", diff --git a/bin/fuel-core/src/cli/run.rs b/bin/fuel-core/src/cli/run.rs index afd7bc305be..64c57519cf6 100644 --- a/bin/fuel-core/src/cli/run.rs +++ b/bin/fuel-core/src/cli/run.rs @@ -51,6 +51,13 @@ mod p2p; #[cfg(feature = "relayer")] mod relayer; +#[derive(clap::ArgEnum, Default, Debug, Clone)] +pub enum NodeRole { + #[default] + Producer, + Validator, +} + #[derive(Debug, Clone, Parser)] pub struct Command { #[clap(long = "ip", default_value = "127.0.0.1", parse(try_from_str))] @@ -96,6 +103,10 @@ pub struct Command { #[clap(long = "consensus-key")] pub consensus_key: Option, + /// The role of the node. + #[clap(arg_enum, long = "node-role", default_value = "producer")] + pub node_role: NodeRole, + /// Use a default insecure consensus key for testing purposes. /// This will not be enabled by default in the future. #[clap(long = "dev-keys", default_value = "true")] @@ -132,6 +143,7 @@ impl Command { utxo_validation, min_gas_price, consensus_key, + node_role, consensus_dev_key, coinbase_recipient, #[cfg(feature = "relayer")] @@ -177,6 +189,7 @@ impl Command { Ok(Config { addr, + node_role: node_role.into(), database_path, database_type, chain_conf: chain_conf.clone(), @@ -192,6 +205,7 @@ impl Command { metrics, }, block_executor: Default::default(), + block_importer: Default::default(), #[cfg(feature = "relayer")] relayer: relayer_args.into(), #[cfg(feature = "p2p")] @@ -233,3 +247,12 @@ fn load_consensus_key( Ok(None) } } + +impl From for fuel_core::service::config::NodeRole { + fn from(node_role: NodeRole) -> Self { + match node_role { + NodeRole::Producer => fuel_core::service::config::NodeRole::Producer, + NodeRole::Validator => fuel_core::service::config::NodeRole::Validator, + } + } +} diff --git a/crates/chain-config/Cargo.toml b/crates/chain-config/Cargo.toml index 20721cd595d..2fe90328532 100644 --- a/crates/chain-config/Cargo.toml +++ b/crates/chain-config/Cargo.toml @@ -13,13 +13,13 @@ description = "Fuel Chain config types" [dependencies] anyhow = "1.0" bincode = "1.3" -fuel-core-poa = { path = "../services/consensus_module/poa", version = "0.15.1" } fuel-core-storage = { path = "../storage", version = "0.15.1" } fuel-core-types = { path = "../types", version = "0.15.1", features = [ "serde", "random", ] } hex = { version = "0.4", features = ["serde"] } +humantime-serde = "1.1.1" itertools = "0.10" rand = "0.8" serde = { version = "1.0", features = ["derive"] } diff --git a/crates/chain-config/src/config/chain.rs b/crates/chain-config/src/config/chain.rs index 3a7c01972c0..48e4424facf 100644 --- a/crates/chain-config/src/config/chain.rs +++ b/crates/chain-config/src/config/chain.rs @@ -22,6 +22,7 @@ use std::{ io::ErrorKind, path::PathBuf, str::FromStr, + time::Duration, }; use crate::GenesisCommitment; @@ -52,7 +53,7 @@ impl Default for ChainConfig { Self { chain_name: "local".into(), block_production: BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Instant, + trigger: PoABlockProduction::Instant, }, block_gas_limit: ConsensusParameters::DEFAULT.max_gas_per_tx * 10, /* TODO: Pick a sensible default */ transaction_parameters: ConsensusParameters::DEFAULT, @@ -155,6 +156,40 @@ pub enum BlockProduction { /// Proof-of-authority modes ProofOfAuthority { #[serde(flatten)] - trigger: fuel_core_poa::Trigger, + trigger: PoABlockProduction, + }, +} + +/// Block production of the PoA consensus. +#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +#[serde(tag = "trigger")] +pub enum PoABlockProduction { + /// A new block is produced instantly when transactions are available. + #[default] + Instant, + /// A new block is produced periodically. Used to simulate consensus block delay. + Interval { + #[serde(with = "humantime_serde")] + block_time: Duration, + }, + /// A new block will be produced when the timer runs out. + /// Set to `max_block_time` when the txpool is empty, otherwise + /// `min(max_block_time, max_tx_idle_time)`. If it expires, + /// but minimum block time hasn't expired yet, then the deadline + /// is set to `last_block_created + min_block_time`. + /// See https://github.com/FuelLabs/fuel-core/issues/50#issuecomment-1241895887 + /// Requires `min_block_time` <= `max_tx_idle_time` <= `max_block_time`. + Hybrid { + /// Minimum time between two blocks, even if there are more txs available + #[serde(with = "humantime_serde")] + min_block_time: Duration, + /// If there are txs available, but not enough for a full block, + /// this is how long the block is waiting for more txs + #[serde(with = "humantime_serde")] + max_tx_idle_time: Duration, + /// Time after which a new block is produced, even if it's empty + #[serde(with = "humantime_serde")] + max_block_time: Duration, }, } diff --git a/crates/fuel-core/Cargo.toml b/crates/fuel-core/Cargo.toml index cb8958efba3..d5359511aa9 100644 --- a/crates/fuel-core/Cargo.toml +++ b/crates/fuel-core/Cargo.toml @@ -21,8 +21,10 @@ bincode = "1.3" derive_more = { version = "0.99" } enum-iterator = "1.2" fuel-core-chain-config = { path = "../chain-config", version = "0.15.1" } +fuel-core-consensus-module = { path = "../../crates/services/consensus_module", version = "0.15.1" } fuel-core-database = { path = "../database", version = "0.15.1" } fuel-core-executor = { path = "../services/executor", version = "0.15.1" } +fuel-core-importer = { path = "../services/importer", version = "0.15.1" } fuel-core-metrics = { path = "../metrics", version = "0.15.1", optional = true } fuel-core-p2p = { path = "../services/p2p", version = "0.15.1", optional = true } fuel-core-poa = { path = "../services/consensus_module/poa", version = "0.15.1" } diff --git a/crates/fuel-core/src/service/config.rs b/crates/fuel-core/src/service/config.rs index 96997bc3e8b..34462ab44e1 100644 --- a/crates/fuel-core/src/service/config.rs +++ b/crates/fuel-core/src/service/config.rs @@ -1,4 +1,8 @@ -use fuel_core_chain_config::ChainConfig; +use fuel_core_chain_config::{ + BlockProduction, + ChainConfig, + PoABlockProduction, +}; use fuel_core_types::{ blockchain::primitives::SecretKeyWrapper, fuel_vm::SecretKey, @@ -22,10 +26,18 @@ use fuel_core_p2p::config::{ Config as P2PConfig, NotInitialized, }; +use fuel_core_poa::Trigger; + +#[derive(Clone, Debug)] +pub enum NodeRole { + Producer, + Validator, +} #[derive(Clone, Debug)] pub struct Config { pub addr: SocketAddr, + pub node_role: NodeRole, pub database_path: PathBuf, pub database_type: DbType, pub chain_conf: ChainConfig, @@ -36,6 +48,7 @@ pub struct Config { pub txpool: fuel_core_txpool::Config, pub block_producer: fuel_core_producer::Config, pub block_executor: fuel_core_executor::Config, + pub block_importer: fuel_core_importer::Config, #[cfg(feature = "relayer")] pub relayer: fuel_core_relayer::Config, #[cfg(feature = "p2p")] @@ -50,6 +63,7 @@ impl Config { let min_gas_price = 0; Self { addr: SocketAddr::new(Ipv4Addr::new(127, 0, 0, 1).into(), 0), + node_role: NodeRole::Producer, database_path: Default::default(), database_type: DbType::InMemory, chain_conf: chain_conf.clone(), @@ -63,6 +77,7 @@ impl Config { ), block_producer: Default::default(), block_executor: Default::default(), + block_importer: Default::default(), #[cfg(feature = "relayer")] relayer: Default::default(), #[cfg(feature = "p2p")] @@ -70,6 +85,40 @@ impl Config { consensus_key: Some(Secret::new(default_consensus_dev_key().into())), } } + + pub fn poa_config(&self) -> anyhow::Result { + let BlockProduction::ProofOfAuthority { trigger } = + self.chain_conf.block_production.clone(); + + let trigger = match self.node_role { + NodeRole::Producer => match trigger { + PoABlockProduction::Instant => Trigger::Instant, + PoABlockProduction::Interval { + block_time: average_block_time, + .. + } => Trigger::Interval { + block_time: average_block_time, + }, + PoABlockProduction::Hybrid { + min_block_time, + max_block_time, + max_tx_idle_time, + } => Trigger::Hybrid { + min_block_time, + max_block_time, + max_tx_idle_time, + }, + }, + NodeRole::Validator => Trigger::Never, + }; + + Ok(fuel_core_poa::Config { + trigger, + block_gas_limit: self.chain_conf.block_gas_limit, + signing_key: self.consensus_key.clone(), + metrics: false, + }) + } } #[derive(Clone, Debug, Default)] diff --git a/crates/services/consensus_module/Cargo.toml b/crates/services/consensus_module/Cargo.toml new file mode 100644 index 00000000000..71b4978378e --- /dev/null +++ b/crates/services/consensus_module/Cargo.toml @@ -0,0 +1,17 @@ +[package] +name = "fuel-core-consensus-module" +version = "0.15.1" +authors = ["Fuel Labs "] +edition = "2021" +homepage = "https://fuel.network/" +keywords = ["blockchain", "fuel", "consensus"] +license = "BUSL-1.1" +repository = "https://github.com/FuelLabs/fuel-core" +description = "The common code for fuel core consensuses." + +[dependencies] +anyhow = "1.0" +fuel-core-chain-config = { version = "0.15.1", path = "../../chain-config" } +fuel-core-poa = { version = "0.15.1", path = "poa" } +fuel-core-types = { version = "0.15.1", path = "../../types" } + diff --git a/crates/services/consensus_module/poa/Cargo.toml b/crates/services/consensus_module/poa/Cargo.toml index cc6e49bc030..a92c50b285b 100644 --- a/crates/services/consensus_module/poa/Cargo.toml +++ b/crates/services/consensus_module/poa/Cargo.toml @@ -12,16 +12,16 @@ version = "0.15.1" [dependencies] anyhow = "1.0" async-trait = "0.1" -fuel-core-services = { path = "../.." } +fuel-core-chain-config = { path = "../../../chain-config", version = "0.15.1" } +fuel-core-services = { path = "../..", version = "0.15.1" } fuel-core-storage = { path = "../../../storage", version = "0.15.1" } fuel-core-types = { path = "../../../types", version = "0.15.1" } -humantime-serde = "1.1.1" -serde = { version = "1.0", features = ["derive"] } tokio = { version = "1.21", features = ["full"] } tokio-stream = "0.1" tracing = "0.1" [dev-dependencies] +fuel-core-storage = { path = "../../../storage", features = ["test-helpers"] } fuel-core-types = { path = "../../../types", features = ["test-helpers"] } mockall = "0.11" rand = "0.8" diff --git a/crates/services/consensus_module/poa/src/config.rs b/crates/services/consensus_module/poa/src/config.rs index 974838d86c5..f257ab995c5 100644 --- a/crates/services/consensus_module/poa/src/config.rs +++ b/crates/services/consensus_module/poa/src/config.rs @@ -3,10 +3,6 @@ use fuel_core_types::{ fuel_asm::Word, secrecy::Secret, }; -use serde::{ - Deserialize, - Serialize, -}; use tokio::time::Duration; #[derive(Default, Debug, Clone)] @@ -18,38 +14,19 @@ pub struct Config { } /// Block production trigger for PoA operation -#[derive(Clone, Copy, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "kebab-case")] -#[serde(tag = "trigger")] +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub enum Trigger { - /// A new block is produced instantly when transactions are available. - /// This is useful for some test cases. + /// See [`fuel_core_chain_config::PoABlockProduction::Instant`]. #[default] Instant, /// This node doesn't produce new blocks. Used for passive listener nodes. Never, - /// A new block is produced periodically. Used to simulate consensus block delay. - Interval { - #[serde(with = "humantime_serde")] - block_time: Duration, - }, - /// A new block will be produced when the timer runs out. - /// Set to `max_block_time` when the txpool is empty, otherwise - /// `min(max_block_time, max_tx_idle_time)`. If it expires, - /// but minimum block time hasn't expired yet, then the deadline - /// is set to `last_block_created + min_block_time`. - /// See https://github.com/FuelLabs/fuel-core/issues/50#issuecomment-1241895887 - /// Requires `min_block_time` <= `max_tx_idle_time` <= `max_block_time`. + /// See [`fuel_core_chain_config::PoABlockProduction::Interval`]. + Interval { block_time: Duration }, + /// See [`fuel_core_chain_config::PoABlockProduction::Hybrid`]. Hybrid { - /// Minimum time between two blocks, even if there are more txs available - #[serde(with = "humantime_serde")] min_block_time: Duration, - /// If there are txs available, but not enough for a full block, - /// this is how long the block is waiting for more txs - #[serde(with = "humantime_serde")] max_tx_idle_time: Duration, - /// Time after which a new block is produced, even if it's empty - #[serde(with = "humantime_serde")] max_block_time: Duration, }, } diff --git a/crates/services/consensus_module/poa/src/lib.rs b/crates/services/consensus_module/poa/src/lib.rs index 29d21640b03..af98e73bb45 100644 --- a/crates/services/consensus_module/poa/src/lib.rs +++ b/crates/services/consensus_module/poa/src/lib.rs @@ -9,6 +9,7 @@ mod service_test; pub mod config; pub mod ports; pub mod service; +pub mod verifier; pub use config::{ Config, diff --git a/crates/services/consensus_module/poa/src/service.rs b/crates/services/consensus_module/poa/src/service.rs index e4b9215c3fe..1d630435359 100644 --- a/crates/services/consensus_module/poa/src/service.rs +++ b/crates/services/consensus_module/poa/src/service.rs @@ -75,6 +75,8 @@ pub struct Task { /// a bit, but doesn't cause any other issues. pub(crate) last_block_created: Instant, pub(crate) trigger: Trigger, + // TODO: Consider that the creation of the block takes some time, and maybe we need to + // patch the timer to generate the block earlier. /// Deadline clock, used by the triggers pub(crate) timer: DeadlineClock, } diff --git a/crates/services/consensus_module/poa/src/verifier.rs b/crates/services/consensus_module/poa/src/verifier.rs new file mode 100644 index 00000000000..b60a28bbfb1 --- /dev/null +++ b/crates/services/consensus_module/poa/src/verifier.rs @@ -0,0 +1,135 @@ +use anyhow::ensure; +use fuel_core_chain_config::PoABlockProduction; +use fuel_core_storage::Result as StorageResult; +use fuel_core_types::{ + blockchain::{ + block::Block, + header::BlockHeader, + primitives::BlockHeight, + }, + fuel_types::Bytes32, + tai64::Tai64N, +}; +use std::ops::Add; + +/// The config of the block verifier. +pub struct Config { + /// If the manual block is enabled, skip verification of some fields. + pub enabled_manual_blocks: bool, + /// This configuration exists because right now consensus module uses the `Tai64::now` + /// during the generation of the block. That means if the node(block producer) was + /// offline for a while, it would not use the expected block time after + /// restarting(it should generate blocks with old timestamps). + pub perform_strict_time_rules: bool, + /// The configuration of the network. + pub production: PoABlockProduction, +} + +/// The port for the database. +pub trait Database { + /// Gets the block header at `height`. + fn block_header(&self, height: &BlockHeight) -> StorageResult; + + /// Gets the block header BMT MMR root at `height`. + fn block_header_merkle_root(&self, height: &BlockHeight) -> StorageResult; +} + +pub fn verify_poa_block_fields( + config: &Config, + database: &D, + block: &Block, +) -> anyhow::Result<()> { + let height = *block.header().height(); + ensure!( + height != 0u32.into(), + "The PoA block can't have the zero height" + ); + + let prev_height = height - 1u32.into(); + let prev_root = database.block_header_merkle_root(&prev_height)?; + let header = block.header(); + ensure!( + header.prev_root() == &prev_root, + "The genesis time should be unix epoch time" + ); + + let prev_header = database.block_header(&prev_height)?; + + ensure!( + header.da_height >= prev_header.da_height, + "The `da_height` of the next block can't be lower" + ); + + // Skip the verification of the time if it is possible to produce blocks manually. + if !config.enabled_manual_blocks { + if config.perform_strict_time_rules { + match config.production { + PoABlockProduction::Instant => { + ensure!( + header.time() >= prev_header.time(), + "The `time` of the next block can't be lower" + ); + } + PoABlockProduction::Interval { + block_time: average_block_time, + } => { + let previous_block_time = Tai64N::from(prev_header.time()); + + // If the `average_block_time` is 15 seconds, the block should be in the range + // [15..15 + 15/2] -> [15..23] + let average_block_time = average_block_time; + let half_average_block_time = average_block_time / 2; + let lower_bound = previous_block_time.add(average_block_time).0; + let upper_bound = previous_block_time + .add(average_block_time + half_average_block_time) + .0; + let block_time = Tai64N::from(header.time()).0; + ensure!( + lower_bound <= block_time && block_time <= upper_bound, + "The `time` of the next should be more than {:?} but less than {:?}", + lower_bound, + upper_bound, + ); + } + PoABlockProduction::Hybrid { + min_block_time, + max_block_time, + .. + } => { + let previous_block_time = Tai64N::from(prev_header.time()); + // The block should be in the range + // [min..max + (max - min)/2] + let half = (max_block_time + min_block_time) / 2; + let lower_bound = previous_block_time.add(min_block_time).0; + let upper_bound = previous_block_time.add(max_block_time + half).0; + let block_time = Tai64N::from(header.time()).0; + ensure!( + lower_bound <= block_time && block_time <= upper_bound, + "The `time` of the next should be more than {:?} but less than {:?}", + lower_bound, + upper_bound, + ); + } + }; + } else { + ensure!( + header.time() >= prev_header.time(), + "The `time` of the next block can't be lower" + ); + } + } + + ensure!( + header.consensus.application_hash == header.application.hash(), + "The application hash mismatch." + ); + + // TODO: We can check the root of the transactions and the root of the messages here. + // But we do the same in the executor right now during validation mode. I will not check + // it for now. But after merge of the https://github.com/FuelLabs/fuel-core/pull/889 it + // is should be easy to do with the `validate_transactions` method. And maybe we want + // to remove this check from the executor and replace it with check that transaction + // id is not modified during the execution. + + Ok(()) +} diff --git a/crates/services/consensus_module/src/block_verifier.rs b/crates/services/consensus_module/src/block_verifier.rs new file mode 100644 index 00000000000..71dc80a4193 --- /dev/null +++ b/crates/services/consensus_module/src/block_verifier.rs @@ -0,0 +1,88 @@ +//! The module provides the functionality that verifies the blocks and headers based +//! on the used consensus. + +pub mod config; + +use crate::block_verifier::config::Config; +use anyhow::ensure; +use fuel_core_poa::verifier::{ + verify_poa_block_fields, + Database as PoAVerifierDatabase, +}; +use fuel_core_types::{ + blockchain::{ + block::Block, + consensus::Consensus, + }, + fuel_types::Bytes32, + tai64::Tai64, +}; + +/// Verifier is responsible for validation of the blocks and headers. +pub struct Verifier { + config: Config, + database: D, + _relayer: R, +} + +impl Verifier { + /// Creates a new instance of the verifier. + pub fn new(config: Config, database: D, _relayer: R) -> Self { + Self { + config, + database, + _relayer, + } + } +} + +impl Verifier +where + D: PoAVerifierDatabase, +{ + /// Verifies **all** fields of the block based on used consensus to produce a block. + /// + /// Return an error if the verification failed, otherwise `Ok(())`. + pub fn verify_block_fields( + &self, + consensus: &Consensus, + block: &Block, + ) -> anyhow::Result<()> { + match consensus { + Consensus::Genesis(_) => self.verify_genesis_block_fields(block), + Consensus::PoA(_) => { + verify_poa_block_fields(&self.config.poa, &self.database, block) + } + } + } + + fn verify_genesis_block_fields(&self, block: &Block) -> anyhow::Result<()> { + let expected_genesis_height = self + .config + .chain_config + .initial_state + .as_ref() + .map(|config| config.height.unwrap_or_else(|| 0u32.into())) + .unwrap_or_else(|| 0u32.into()); + let actual_genesis_height = *block.header().height(); + + ensure!( + block.header().prev_root() == &Bytes32::zeroed(), + "The genesis previous root should be zeroed" + ); + ensure!( + block.header().time() == Tai64::UNIX_EPOCH, + "The genesis time should be unix epoch time" + ); + ensure!( + // TODO: Set `da_height` based on the chain config. + block.header().da_height == Default::default(), + "The genesis `da_height` is not as expected" + ); + ensure!( + expected_genesis_height == actual_genesis_height, + "The genesis height is not as expected" + ); + Ok(()) + } +} diff --git a/crates/services/consensus_module/src/block_verifier/config.rs b/crates/services/consensus_module/src/block_verifier/config.rs new file mode 100644 index 00000000000..08d9ef61fad --- /dev/null +++ b/crates/services/consensus_module/src/block_verifier/config.rs @@ -0,0 +1,32 @@ +//! The config of the block verifier. + +use fuel_core_chain_config::{ + BlockProduction, + ChainConfig, +}; +use fuel_core_poa::verifier::Config as PoAVerifierConfig; + +/// The config of the block verifier. +pub struct Config { + /// The chain configuration. + pub chain_config: ChainConfig, + /// The config of verifier for the PoA. + pub poa: PoAVerifierConfig, +} + +impl Config { + /// Creates the verifier config for all possible consensuses. + pub fn new(chain_config: ChainConfig, enabled_manual_blocks: bool) -> Self { + let BlockProduction::ProofOfAuthority { trigger } = + chain_config.block_production.clone(); + Self { + chain_config, + poa: PoAVerifierConfig { + enabled_manual_blocks, + // TODO: Make it `true`. See the comment of the `perform_strict_time_rules`. + perform_strict_time_rules: false, + production: trigger, + }, + } + } +} diff --git a/crates/services/consensus_module/src/lib.rs b/crates/services/consensus_module/src/lib.rs new file mode 100644 index 00000000000..6a8523a10df --- /dev/null +++ b/crates/services/consensus_module/src/lib.rs @@ -0,0 +1,7 @@ +//! Common traits and logic for managing the lifecycle of services +#![deny(unused_crate_dependencies)] +#![deny(missing_docs)] + +extern crate core; + +pub mod block_verifier; diff --git a/tests/tests/trigger_integration/hybrid.rs b/tests/tests/trigger_integration/hybrid.rs index 8261aed6e34..ffc5fba3582 100644 --- a/tests/tests/trigger_integration/hybrid.rs +++ b/tests/tests/trigger_integration/hybrid.rs @@ -1,5 +1,8 @@ use fuel_core::{ - chain_config::BlockProduction, + chain_config::{ + BlockProduction, + PoABlockProduction, + }, database::Database, service::{ Config, @@ -35,7 +38,7 @@ async fn poa_hybrid_produces_empty_blocks_at_correct_rate() { let mut config = Config::local_node(); config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); config.chain_conf.block_production = BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Hybrid { + trigger: PoABlockProduction::Hybrid { max_block_time: Duration::new(round_time_seconds, 0), max_tx_idle_time: Duration::new(5, 0), min_block_time: Duration::new(2, 0), @@ -104,7 +107,7 @@ async fn poa_hybrid_produces_nonempty_blocks_at_correct_rate() { let mut config = Config::local_node(); config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); config.chain_conf.block_production = BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Hybrid { + trigger: PoABlockProduction::Hybrid { max_block_time: Duration::new(30, 0), max_tx_idle_time: Duration::new(5, 0), min_block_time: Duration::new(round_time_seconds, 0), diff --git a/tests/tests/tx/txn_status_subscription.rs b/tests/tests/tx/txn_status_subscription.rs index f6f3871b813..a7053974d46 100644 --- a/tests/tests/tx/txn_status_subscription.rs +++ b/tests/tests/tx/txn_status_subscription.rs @@ -14,15 +14,13 @@ use futures::StreamExt; #[tokio::test] async fn subscribe_txn_status() { - use fuel_core_poa::Trigger; let mut config = Config::local_node(); - match &mut config.chain_conf.block_production { - fuel_core::chain_config::BlockProduction::ProofOfAuthority { trigger } => { - *trigger = Trigger::Interval { + config.chain_conf.block_production = + fuel_core::chain_config::BlockProduction::ProofOfAuthority { + trigger: fuel_core::chain_config::PoABlockProduction::Interval { block_time: Duration::from_secs(2), - } - } - }; + }, + }; let srv = FuelService::new_node(config).await.unwrap(); let client = FuelClient::from(srv.bound_address); From f90e0263b72386cae4e360901f3aaf8cb106d6a2 Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 15:28:21 +0000 Subject: [PATCH 09/33] Fixed merge conflicts --- Cargo.lock | 1 - crates/fuel-core/Cargo.toml | 1 - crates/fuel-core/src/service/sub_services.rs | 28 +++++++------------- tests/tests/trigger_integration/instant.rs | 7 +++-- tests/tests/trigger_integration/interval.rs | 9 ++++--- tests/tests/trigger_integration/never.rs | 6 ++--- 6 files changed, 23 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4071c79dc90..de9e50947d7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2203,7 +2203,6 @@ dependencies = [ "derive_more", "enum-iterator", "fuel-core-chain-config", - "fuel-core-consensus-module", "fuel-core-database", "fuel-core-executor", "fuel-core-importer", diff --git a/crates/fuel-core/Cargo.toml b/crates/fuel-core/Cargo.toml index d5359511aa9..1d88009ba4d 100644 --- a/crates/fuel-core/Cargo.toml +++ b/crates/fuel-core/Cargo.toml @@ -21,7 +21,6 @@ bincode = "1.3" derive_more = { version = "0.99" } enum-iterator = "1.2" fuel-core-chain-config = { path = "../chain-config", version = "0.15.1" } -fuel-core-consensus-module = { path = "../../crates/services/consensus_module", version = "0.15.1" } fuel-core-database = { path = "../database", version = "0.15.1" } fuel-core-executor = { path = "../services/executor", version = "0.15.1" } fuel-core-importer = { path = "../services/importer", version = "0.15.1" } diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index f0a29c0949a..39790846f00 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -1,7 +1,6 @@ #![allow(clippy::let_unit_value)] use super::adapters::P2PAdapter; use crate::{ - chain_config::BlockProduction, database::Database, fuel_core_graphql_api::Config as GraphQLConfig, schema::{ @@ -103,23 +102,16 @@ pub fn init_sub_services( dry_run_semaphore: Semaphore::new(max_dry_run_concurrency), }); - let poa = match &config.chain_conf.block_production { - BlockProduction::ProofOfAuthority { trigger } => fuel_core_poa::new_service( - fuel_core_poa::Config { - trigger: *trigger, - block_gas_limit: config.chain_conf.block_gas_limit, - signing_key: config.consensus_key.clone(), - metrics: false, - }, - TxPoolAdapter::new(txpool.shared.clone()), - // TODO: Pass Importer - importer_adapter.tx, - BlockProducerAdapter { - block_producer: block_producer.clone(), - }, - database.clone(), - ), - }; + let poa = fuel_core_poa::new_service( + config.poa_config()?, + TxPoolAdapter::new(txpool.shared.clone()), + // TODO: Pass Importer + importer_adapter.tx, + BlockProducerAdapter { + block_producer: block_producer.clone(), + }, + database.clone(), + ); // TODO: Figure out on how to move it into `fuel-core-graphql-api`. let schema = dap::init(build_schema(), config.chain_conf.transaction_parameters) diff --git a/tests/tests/trigger_integration/instant.rs b/tests/tests/trigger_integration/instant.rs index 6cb7702b1ae..2292beded08 100644 --- a/tests/tests/trigger_integration/instant.rs +++ b/tests/tests/trigger_integration/instant.rs @@ -1,5 +1,8 @@ use fuel_core::{ - chain_config::BlockProduction, + chain_config::{ + BlockProduction, + PoABlockProduction, + }, database::Database, service::{ Config, @@ -30,7 +33,7 @@ async fn poa_instant_trigger_is_produces_instantly() { let mut config = Config::local_node(); config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); config.chain_conf.block_production = BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Instant, + trigger: PoABlockProduction::Instant, }; let srv = FuelService::from_database(db.clone(), config) diff --git a/tests/tests/trigger_integration/interval.rs b/tests/tests/trigger_integration/interval.rs index 1f00abd432c..18f24c5667d 100644 --- a/tests/tests/trigger_integration/interval.rs +++ b/tests/tests/trigger_integration/interval.rs @@ -1,5 +1,8 @@ use fuel_core::{ - chain_config::BlockProduction, + chain_config::{ + BlockProduction, + PoABlockProduction, + }, database::Database, service::{ Config, @@ -35,7 +38,7 @@ async fn poa_interval_produces_empty_blocks_at_correct_rate() { let mut config = Config::local_node(); config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); config.chain_conf.block_production = BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Interval { + trigger: PoABlockProduction::Interval { block_time: Duration::new(round_time_seconds, 0), }, }; @@ -99,7 +102,7 @@ async fn poa_interval_produces_nonempty_blocks_at_correct_rate() { let mut config = Config::local_node(); config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); config.chain_conf.block_production = BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Interval { + trigger: PoABlockProduction::Interval { block_time: Duration::new(round_time_seconds, 0), }, }; diff --git a/tests/tests/trigger_integration/never.rs b/tests/tests/trigger_integration/never.rs index f0c85347fb5..e40afc1fdee 100644 --- a/tests/tests/trigger_integration/never.rs +++ b/tests/tests/trigger_integration/never.rs @@ -1,7 +1,7 @@ use fuel_core::{ - chain_config::BlockProduction, database::Database, service::{ + config::NodeRole, Config, FuelService, }, @@ -27,9 +27,7 @@ async fn poa_never_trigger_doesnt_produce_blocks() { let mut rng = StdRng::seed_from_u64(10); let db = Database::default(); let mut config = Config::local_node(); - config.chain_conf.block_production = BlockProduction::ProofOfAuthority { - trigger: fuel_core_poa::Trigger::Never, - }; + config.node_role = NodeRole::Producer; config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); let srv = FuelService::from_database(db.clone(), config) .await From 7d21d6bfea55548e4a77c615ed47367cc8601c0c Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 15:31:19 +0000 Subject: [PATCH 10/33] Fix test to use Validator role --- tests/tests/trigger_integration/never.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tests/trigger_integration/never.rs b/tests/tests/trigger_integration/never.rs index e40afc1fdee..ba8e016eab9 100644 --- a/tests/tests/trigger_integration/never.rs +++ b/tests/tests/trigger_integration/never.rs @@ -27,7 +27,7 @@ async fn poa_never_trigger_doesnt_produce_blocks() { let mut rng = StdRng::seed_from_u64(10); let db = Database::default(); let mut config = Config::local_node(); - config.node_role = NodeRole::Producer; + config.node_role = NodeRole::Validator; config.consensus_key = Some(Secret::new(SecretKey::random(&mut rng).into())); let srv = FuelService::from_database(db.clone(), config) .await From d9cade30d4f29462e18ed8dff077e4a4e2a081aa Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 19 Jan 2023 12:50:06 +1100 Subject: [PATCH 11/33] add unit tests to verify genesis --- Cargo.lock | 1 + crates/services/consensus_module/Cargo.toml | 11 +-- .../consensus_module/src/block_verifier.rs | 68 +++++++++++-------- .../src/block_verifier/tests.rs | 59 ++++++++++++++++ 4 files changed, 106 insertions(+), 33 deletions(-) create mode 100644 crates/services/consensus_module/src/block_verifier/tests.rs diff --git a/Cargo.lock b/Cargo.lock index de9e50947d7..e840c869c01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2334,6 +2334,7 @@ dependencies = [ "fuel-core-chain-config", "fuel-core-poa", "fuel-core-types", + "test-case", ] [[package]] diff --git a/crates/services/consensus_module/Cargo.toml b/crates/services/consensus_module/Cargo.toml index 71b4978378e..4e1a15fba81 100644 --- a/crates/services/consensus_module/Cargo.toml +++ b/crates/services/consensus_module/Cargo.toml @@ -1,13 +1,13 @@ [package] -name = "fuel-core-consensus-module" -version = "0.15.1" authors = ["Fuel Labs "] +description = "The common code for fuel core consensuses." edition = "2021" homepage = "https://fuel.network/" -keywords = ["blockchain", "fuel", "consensus"] +keywords = ["blockchain", "consensus", "fuel"] license = "BUSL-1.1" +name = "fuel-core-consensus-module" repository = "https://github.com/FuelLabs/fuel-core" -description = "The common code for fuel core consensuses." +version = "0.15.1" [dependencies] anyhow = "1.0" @@ -15,3 +15,6 @@ fuel-core-chain-config = { version = "0.15.1", path = "../../chain-config" } fuel-core-poa = { version = "0.15.1", path = "poa" } fuel-core-types = { version = "0.15.1", path = "../../types" } +[dev-dependencies] +fuel-core-types = { path = "../../types", features = ["test-helpers"] } +test-case = "2.2" diff --git a/crates/services/consensus_module/src/block_verifier.rs b/crates/services/consensus_module/src/block_verifier.rs index 71dc80a4193..090b4148a47 100644 --- a/crates/services/consensus_module/src/block_verifier.rs +++ b/crates/services/consensus_module/src/block_verifier.rs @@ -3,6 +3,9 @@ pub mod config; +#[cfg(test)] +mod tests; + use crate::block_verifier::config::Config; use anyhow::ensure; use fuel_core_poa::verifier::{ @@ -13,6 +16,8 @@ use fuel_core_types::{ blockchain::{ block::Block, consensus::Consensus, + header::BlockHeader, + primitives::BlockHeight, }, fuel_types::Bytes32, tai64::Tai64, @@ -49,40 +54,45 @@ where block: &Block, ) -> anyhow::Result<()> { match consensus { - Consensus::Genesis(_) => self.verify_genesis_block_fields(block), + Consensus::Genesis(_) => { + let expected_genesis_height = self + .config + .chain_config + .initial_state + .as_ref() + .map(|config| config.height.unwrap_or_else(|| 0u32.into())) + .unwrap_or_else(|| 0u32.into()); + verify_genesis_block_fields(expected_genesis_height, block.header()) + } Consensus::PoA(_) => { verify_poa_block_fields(&self.config.poa, &self.database, block) } } } +} - fn verify_genesis_block_fields(&self, block: &Block) -> anyhow::Result<()> { - let expected_genesis_height = self - .config - .chain_config - .initial_state - .as_ref() - .map(|config| config.height.unwrap_or_else(|| 0u32.into())) - .unwrap_or_else(|| 0u32.into()); - let actual_genesis_height = *block.header().height(); +fn verify_genesis_block_fields( + expected_genesis_height: BlockHeight, + header: &BlockHeader, +) -> anyhow::Result<()> { + let actual_genesis_height = *header.height(); - ensure!( - block.header().prev_root() == &Bytes32::zeroed(), - "The genesis previous root should be zeroed" - ); - ensure!( - block.header().time() == Tai64::UNIX_EPOCH, - "The genesis time should be unix epoch time" - ); - ensure!( - // TODO: Set `da_height` based on the chain config. - block.header().da_height == Default::default(), - "The genesis `da_height` is not as expected" - ); - ensure!( - expected_genesis_height == actual_genesis_height, - "The genesis height is not as expected" - ); - Ok(()) - } + ensure!( + header.prev_root() == &Bytes32::zeroed(), + "The genesis previous root should be zeroed" + ); + ensure!( + header.time() == Tai64::UNIX_EPOCH, + "The genesis time should be unix epoch time" + ); + ensure!( + // TODO: Set `da_height` based on the chain config. + header.da_height == Default::default(), + "The genesis `da_height` is not as expected" + ); + ensure!( + expected_genesis_height == actual_genesis_height, + "The genesis height is not as expected" + ); + Ok(()) } diff --git a/crates/services/consensus_module/src/block_verifier/tests.rs b/crates/services/consensus_module/src/block_verifier/tests.rs new file mode 100644 index 00000000000..c4357bb10a3 --- /dev/null +++ b/crates/services/consensus_module/src/block_verifier/tests.rs @@ -0,0 +1,59 @@ +use super::*; +use test_case::test_case; + +#[test_case( + { + let mut h = BlockHeader::default(); + h.consensus.prev_root = Bytes32::zeroed(); + h.consensus.time = Tai64::UNIX_EPOCH; + h.consensus.height = 0u32.into(); + h + }, + 0 => matches Ok(_) ; "Correct header" +)] +#[test_case( + { + let mut h = BlockHeader::default(); + h.consensus.prev_root = Bytes32::zeroed(); + h.consensus.time = Tai64::UNIX_EPOCH; + h.consensus.height = 0u32.into(); + h + }, + 10 => matches Err(_) ; "wrong expected height" +)] +#[test_case( + { + let mut h = BlockHeader::default(); + h.consensus.prev_root = Bytes32::zeroed(); + h.consensus.time = Tai64::UNIX_EPOCH; + h.consensus.height = 5u32.into(); + h + }, + 0 => matches Err(_) ; "wrong header height" +)] +#[test_case( + { + let mut h = BlockHeader::default(); + h.consensus.prev_root = Bytes32::zeroed(); + h.consensus.time = Tai64(0); + h.consensus.height = 0u32.into(); + h + }, + 0 => matches Err(_) ; "wrong time" +)] +#[test_case( + { + let mut h = BlockHeader::default(); + h.consensus.prev_root = Bytes32::from([1u8; 32]); + h.consensus.time = Tai64::UNIX_EPOCH; + h.consensus.height = 0u32.into(); + h + }, + 0 => matches Err(_) ; "wrong root" +)] +fn test_verify_genesis_block_fields( + header: BlockHeader, + expected_genesis_height: u32, +) -> anyhow::Result<()> { + verify_genesis_block_fields(expected_genesis_height.into(), &header) +} From d67e72927e5b76fc40ac5f7991e4425d2b6f0b1d Mon Sep 17 00:00:00 2001 From: Tom Date: Thu, 19 Jan 2023 14:29:57 +1100 Subject: [PATCH 12/33] add unit tests to verify poa block --- Cargo.lock | 1 + .../services/consensus_module/poa/Cargo.toml | 1 + .../consensus_module/poa/src/verifier.rs | 23 ++- .../poa/src/verifier/tests.rs | 195 ++++++++++++++++++ 4 files changed, 211 insertions(+), 9 deletions(-) create mode 100644 crates/services/consensus_module/poa/src/verifier/tests.rs diff --git a/Cargo.lock b/Cargo.lock index e840c869c01..3101d86f5ea 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2445,6 +2445,7 @@ dependencies = [ "fuel-core-types", "mockall", "rand 0.8.5", + "test-case", "tokio", "tokio-stream", "tracing", diff --git a/crates/services/consensus_module/poa/Cargo.toml b/crates/services/consensus_module/poa/Cargo.toml index a92c50b285b..3be34fc74f5 100644 --- a/crates/services/consensus_module/poa/Cargo.toml +++ b/crates/services/consensus_module/poa/Cargo.toml @@ -25,4 +25,5 @@ fuel-core-storage = { path = "../../../storage", features = ["test-helpers"] } fuel-core-types = { path = "../../../types", features = ["test-helpers"] } mockall = "0.11" rand = "0.8" +test-case = "2.2" tokio = { version = "1.21", features = ["full", "test-util"] } diff --git a/crates/services/consensus_module/poa/src/verifier.rs b/crates/services/consensus_module/poa/src/verifier.rs index b60a28bbfb1..c7d5b562e48 100644 --- a/crates/services/consensus_module/poa/src/verifier.rs +++ b/crates/services/consensus_module/poa/src/verifier.rs @@ -12,6 +12,9 @@ use fuel_core_types::{ }; use std::ops::Add; +#[cfg(test)] +mod tests; + /// The config of the block verifier. pub struct Config { /// If the manual block is enabled, skip verification of some fields. @@ -25,6 +28,7 @@ pub struct Config { pub production: PoABlockProduction, } +#[cfg_attr(test, mockall::automock)] /// The port for the database. pub trait Database { /// Gets the block header at `height`. @@ -79,16 +83,16 @@ pub fn verify_poa_block_fields( // [15..15 + 15/2] -> [15..23] let average_block_time = average_block_time; let half_average_block_time = average_block_time / 2; - let lower_bound = previous_block_time.add(average_block_time).0; + let lower_bound = previous_block_time.add(average_block_time); let upper_bound = previous_block_time - .add(average_block_time + half_average_block_time) - .0; - let block_time = Tai64N::from(header.time()).0; + .add(average_block_time + half_average_block_time); + let block_time = Tai64N::from(header.time()); ensure!( lower_bound <= block_time && block_time <= upper_bound, - "The `time` of the next should be more than {:?} but less than {:?}", + "The `time` of the next should be more than {:?} but less than {:?} but got {:?}", lower_bound, upper_bound, + block_time, ); } PoABlockProduction::Hybrid { @@ -100,14 +104,15 @@ pub fn verify_poa_block_fields( // The block should be in the range // [min..max + (max - min)/2] let half = (max_block_time + min_block_time) / 2; - let lower_bound = previous_block_time.add(min_block_time).0; - let upper_bound = previous_block_time.add(max_block_time + half).0; - let block_time = Tai64N::from(header.time()).0; + let lower_bound = previous_block_time.add(min_block_time); + let upper_bound = previous_block_time.add(max_block_time + half); + let block_time = Tai64N::from(header.time()); ensure!( lower_bound <= block_time && block_time <= upper_bound, - "The `time` of the next should be more than {:?} but less than {:?}", + "The `time` of the next should be more than {:?} but less than {:?} but got {:?}", lower_bound, upper_bound, + block_time, ); } }; diff --git a/crates/services/consensus_module/poa/src/verifier/tests.rs b/crates/services/consensus_module/poa/src/verifier/tests.rs new file mode 100644 index 00000000000..7c59aeba245 --- /dev/null +++ b/crates/services/consensus_module/poa/src/verifier/tests.rs @@ -0,0 +1,195 @@ +use super::*; +use fuel_core_types::{ + blockchain::header::{ + ApplicationHeader, + ConsensusHeader, + GeneratedApplicationFields, + GeneratedConsensusFields, + }, + tai64::Tai64, +}; +use std::time::Duration; +use test_case::test_case; + +struct Input { + c: Config, + block_header_merkle_root: [u8; 32], + prev_header_time: Tai64, + prev_header_da_height: u64, + ch: ConsensusHeader, + ah: ApplicationHeader, +} + +fn app_hash(da_height: u64) -> Bytes32 { + ApplicationHeader { + da_height: da_height.into(), + ..Default::default() + } + .hash() +} + +fn correct() -> Input { + Input { + c: Config { + enabled_manual_blocks: false, + perform_strict_time_rules: false, + production: PoABlockProduction::Instant, + }, + block_header_merkle_root: [2u8; 32], + prev_header_time: Tai64(2), + prev_header_da_height: 2, + ch: ConsensusHeader { + prev_root: [2u8; 32].into(), + height: 2u32.into(), + time: Tai64(2), + generated: GeneratedConsensusFields { + application_hash: app_hash(2), + }, + }, + ah: ApplicationHeader { + da_height: 2u64.into(), + ..Default::default() + }, + } +} + +#[test_case(correct() => matches Ok(_) ; "Correct block")] +#[test_case( + { + let mut i = correct(); + i.ch.height = 0u32.into(); + i + } => matches Err(_) ; "Height 0" +)] +#[test_case( + { + let mut i = correct(); + i.ch.prev_root = [3u8; 32].into(); + i + } => matches Err(_) ; "Prev root mis-match" +)] +#[test_case( + { + let mut i = correct(); + i.ah.da_height = 1u64.into(); + i + } => matches Err(_) ; "da height lower then prev header" +)] +#[test_case( + { + let mut i = correct(); + i.ch.generated.application_hash = [0u8; 32].into(); + i + } => matches Err(_) ; "application hash mis-match" +)] +#[test_case( + { + let mut i = correct(); + i.ch.time = Tai64(1); + i + } => matches Err(_) ; "time before prev header" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i + } => matches Ok(_) ; "Strict rules with correct block" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.ch.time = Tai64(1); + i + } => matches Err(_) ; "time before prev header with strict rules" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.c.production = PoABlockProduction::Interval { block_time: Duration::from_secs(4) }; + i.ch.time = Tai64(7); + i + } => matches Ok(_) ; "interval time ok" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.c.production = PoABlockProduction::Interval { block_time: Duration::from_secs(4) }; + i.ch.time = Tai64(5); + i + } => matches Err(_) ; "interval time too early" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.c.production = PoABlockProduction::Interval { block_time: Duration::from_secs(4) }; + i.ch.time = Tai64(9); + i + } => matches Err(_) ; "interval time too late" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.c.production = PoABlockProduction::Hybrid { + min_block_time: Duration::from_secs(4), + max_tx_idle_time: Duration::from_secs(5), + max_block_time: Duration::from_secs(6), + }; + i.ch.time = Tai64(7); + i + } => matches Ok(_) ; "hybrid time ok" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.c.production = PoABlockProduction::Hybrid { + min_block_time: Duration::from_secs(4), + max_tx_idle_time: Duration::from_secs(5), + max_block_time: Duration::from_secs(6), + }; + i.ch.time = Tai64(5); + i + } => matches Err(_) ; "hybrid time too early" +)] +#[test_case( + { + let mut i = correct(); + i.c.perform_strict_time_rules = true; + i.c.production = PoABlockProduction::Hybrid { + min_block_time: Duration::from_secs(4), + max_tx_idle_time: Duration::from_secs(5), + max_block_time: Duration::from_secs(6), + }; + i.ch.time = Tai64(14); + i + } => matches Err(_) ; "hybrid time too late" +)] +fn test_verify_genesis_block_fields(input: Input) -> anyhow::Result<()> { + let Input { + c, + block_header_merkle_root, + prev_header_time, + prev_header_da_height, + ch, + ah, + } = input; + let mut d = MockDatabase::default(); + d.expect_block_header_merkle_root() + .returning(move |_| Ok(block_header_merkle_root.into())); + d.expect_block_header().returning(move |_| { + let mut h = BlockHeader::default(); + h.consensus.time = prev_header_time; + h.application.da_height = prev_header_da_height.into(); + Ok(h) + }); + let mut b = Block::default(); + b.header_mut().consensus = ch; + b.header_mut().application = ah; + verify_poa_block_fields(&c, &d, &b) +} From 5a16bc0bb263cedcbea8dd478042525596749315 Mon Sep 17 00:00:00 2001 From: green Date: Thu, 19 Jan 2023 13:47:10 +0000 Subject: [PATCH 13/33] Linked TODO to the issue --- crates/services/consensus_module/poa/src/service.rs | 1 + crates/services/consensus_module/poa/src/verifier.rs | 10 +++++++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/crates/services/consensus_module/poa/src/service.rs b/crates/services/consensus_module/poa/src/service.rs index 1d630435359..44b7c11cb17 100644 --- a/crates/services/consensus_module/poa/src/service.rs +++ b/crates/services/consensus_module/poa/src/service.rs @@ -77,6 +77,7 @@ pub struct Task { pub(crate) trigger: Trigger, // TODO: Consider that the creation of the block takes some time, and maybe we need to // patch the timer to generate the block earlier. + // https://github.com/FuelLabs/fuel-core/issues/918 /// Deadline clock, used by the triggers pub(crate) timer: DeadlineClock, } diff --git a/crates/services/consensus_module/poa/src/verifier.rs b/crates/services/consensus_module/poa/src/verifier.rs index c7d5b562e48..0d666d3530b 100644 --- a/crates/services/consensus_module/poa/src/verifier.rs +++ b/crates/services/consensus_module/poa/src/verifier.rs @@ -23,6 +23,8 @@ pub struct Config { /// during the generation of the block. That means if the node(block producer) was /// offline for a while, it would not use the expected block time after /// restarting(it should generate blocks with old timestamps). + /// + /// https://github.com/FuelLabs/fuel-core/issues/918 pub perform_strict_time_rules: bool, /// The configuration of the network. pub production: PoABlockProduction, @@ -66,6 +68,7 @@ pub fn verify_poa_block_fields( // Skip the verification of the time if it is possible to produce blocks manually. if !config.enabled_manual_blocks { + // See comment of the `Config.perform_strict_time_rules` if config.perform_strict_time_rules { match config.production { PoABlockProduction::Instant => { @@ -81,6 +84,7 @@ pub fn verify_poa_block_fields( // If the `average_block_time` is 15 seconds, the block should be in the range // [15..15 + 15/2] -> [15..23] + // https://github.com/FuelLabs/fuel-core/issues/918 let average_block_time = average_block_time; let half_average_block_time = average_block_time / 2; let lower_bound = previous_block_time.add(average_block_time); @@ -102,10 +106,10 @@ pub fn verify_poa_block_fields( } => { let previous_block_time = Tai64N::from(prev_header.time()); // The block should be in the range - // [min..max + (max - min)/2] - let half = (max_block_time + min_block_time) / 2; + // [min..max] + // https://github.com/FuelLabs/fuel-core/issues/918 let lower_bound = previous_block_time.add(min_block_time); - let upper_bound = previous_block_time.add(max_block_time + half); + let upper_bound = previous_block_time.add(max_block_time); let block_time = Tai64N::from(header.time()); ensure!( lower_bound <= block_time && block_time <= upper_bound, From 838a7160c24a6347c4f54cfcbb4d7fb258e45b90 Mon Sep 17 00:00:00 2001 From: green Date: Thu, 19 Jan 2023 13:49:55 +0000 Subject: [PATCH 14/33] Added one more test for geensis --- .../consensus_module/src/block_verifier/tests.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/services/consensus_module/src/block_verifier/tests.rs b/crates/services/consensus_module/src/block_verifier/tests.rs index c4357bb10a3..2c7bc090597 100644 --- a/crates/services/consensus_module/src/block_verifier/tests.rs +++ b/crates/services/consensus_module/src/block_verifier/tests.rs @@ -9,7 +9,17 @@ use test_case::test_case; h.consensus.height = 0u32.into(); h }, - 0 => matches Ok(_) ; "Correct header" + 0 => matches Ok(_) ; "Correct header at `0`" +)] +#[test_case( + { + let mut h = BlockHeader::default(); + h.consensus.prev_root = Bytes32::zeroed(); + h.consensus.time = Tai64::UNIX_EPOCH; + h.consensus.height = 113u32.into(); + h + }, + 113 => matches Ok(_) ; "Correct header at `113`" )] #[test_case( { From e7e183525acc6cffe7bf7f74b57e3726b3d9e865 Mon Sep 17 00:00:00 2001 From: green Date: Wed, 18 Jan 2023 15:38:04 +0000 Subject: [PATCH 15/33] - Removed `CHAIN_HEIGHT_KEY`, because now we have a genesis block to track this information. - Added `manual_produce_block` into `PoA` and used it in the `GraphQL` to have the one source of the block production. There are no unit tests(help is appreciated). - Connected `PoA` and `BlockImporter` via ports. Added `BlockImporterAdapter`. - Added the `FuelBlockRoots` column with the corresponding table. It is used to track BMT MMR roots for fuel blocks. The implementation adds new `DatabaseColumn` and `IntoDatabaseKey` traits to automate the storage trait implementation for some tables. - Moved implementation of database ports into the corresponding file in the `fuel_core::service::adapter`. - Removed the `Database` generic from `PoA` at all. Now we must pass the `last_height` while creating the `Task`. Updated tests to work with it. - Block producer now accepts the `block_time` along with `block_height`. --- Cargo.lock | 1 + crates/chain-config/src/config/state.rs | 4 +- crates/fuel-core/Cargo.toml | 1 + crates/fuel-core/src/database.rs | 110 +----- crates/fuel-core/src/database/block.rs | 22 +- crates/fuel-core/src/database/metadata.rs | 14 - crates/fuel-core/src/database/p2p.rs | 2 +- crates/fuel-core/src/database/sealed_block.rs | 2 +- crates/fuel-core/src/database/storage.rs | 90 +++++ crates/fuel-core/src/database/vm_database.rs | 4 +- crates/fuel-core/src/graphql_api/ports.rs | 11 +- crates/fuel-core/src/graphql_api/service.rs | 15 +- crates/fuel-core/src/query/block.rs | 2 +- crates/fuel-core/src/schema/block.rs | 88 ++--- crates/fuel-core/src/schema/dap.rs | 5 +- crates/fuel-core/src/schema/tx.rs | 1 + crates/fuel-core/src/service.rs | 2 +- crates/fuel-core/src/service/adapters.rs | 28 +- .../src/service/adapters/block_importer.rs | 99 ++++++ .../src/service/adapters/consensus_module.rs | 61 ++++ .../adapters/{ => consensus_module}/poa.rs | 31 +- .../src/service/adapters/graphql_api.rs | 4 +- .../src/service/adapters/producer.rs | 41 +++ .../fuel-core/src/service/adapters/txpool.rs | 80 ++++- crates/fuel-core/src/service/genesis.rs | 55 ++- crates/fuel-core/src/service/sub_services.rs | 74 ++-- .../consensus_module/poa/src/ports.rs | 45 +-- .../consensus_module/poa/src/service.rs | 209 +++++++---- .../consensus_module/poa/src/service_test.rs | 329 ++++++------------ .../poa/src/service_test/trigger_tests.rs | 48 ++- crates/services/p2p/src/ports.rs | 2 +- crates/services/p2p/src/service.rs | 4 +- .../services/producer/src/block_producer.rs | 41 ++- .../producer/src/block_producer/tests.rs | 24 +- crates/services/producer/src/mocks.rs | 21 +- crates/services/producer/src/ports.rs | 15 +- crates/services/txpool/src/ports.rs | 18 +- crates/services/txpool/src/service.rs | 17 +- .../txpool/src/service/test_helpers.rs | 18 +- crates/services/txpool/src/txpool.rs | 2 +- crates/storage/src/tables.rs | 15 +- tests/tests/blocks.rs | 10 +- 42 files changed, 961 insertions(+), 704 deletions(-) create mode 100644 crates/fuel-core/src/database/storage.rs create mode 100644 crates/fuel-core/src/service/adapters/block_importer.rs create mode 100644 crates/fuel-core/src/service/adapters/consensus_module.rs rename crates/fuel-core/src/service/adapters/{ => consensus_module}/poa.rs (69%) diff --git a/Cargo.lock b/Cargo.lock index 3101d86f5ea..2b0456b35f9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2203,6 +2203,7 @@ dependencies = [ "derive_more", "enum-iterator", "fuel-core-chain-config", + "fuel-core-consensus-module", "fuel-core-database", "fuel-core-executor", "fuel-core-importer", diff --git a/crates/chain-config/src/config/state.rs b/crates/chain-config/src/config/state.rs index 4aad9d4bfd6..01f5c136978 100644 --- a/crates/chain-config/src/config/state.rs +++ b/crates/chain-config/src/config/state.rs @@ -44,7 +44,7 @@ impl StateConfig { coins: db.get_coin_config()?, contracts: db.get_contract_config()?, messages: db.get_message_config()?, - height: db.get_block_height()?, + height: Some(db.get_block_height()?), }) } } @@ -57,5 +57,5 @@ pub trait ChainConfigDb { /// Returns *all* unspent message configs available in the database. fn get_message_config(&self) -> StorageResult>>; /// Returns the last available block height. - fn get_block_height(&self) -> StorageResult>; + fn get_block_height(&self) -> StorageResult; } diff --git a/crates/fuel-core/Cargo.toml b/crates/fuel-core/Cargo.toml index 1d88009ba4d..d5359511aa9 100644 --- a/crates/fuel-core/Cargo.toml +++ b/crates/fuel-core/Cargo.toml @@ -21,6 +21,7 @@ bincode = "1.3" derive_more = { version = "0.99" } enum-iterator = "1.2" fuel-core-chain-config = { path = "../chain-config", version = "0.15.1" } +fuel-core-consensus-module = { path = "../../crates/services/consensus_module", version = "0.15.1" } fuel-core-database = { path = "../database", version = "0.15.1" } fuel-core-executor = { path = "../services/executor", version = "0.15.1" } fuel-core-importer = { path = "../services/importer", version = "0.15.1" } diff --git a/crates/fuel-core/src/database.rs b/crates/fuel-core/src/database.rs index bb7caf2b299..da7152918af 100644 --- a/crates/fuel-core/src/database.rs +++ b/crates/fuel-core/src/database.rs @@ -13,33 +13,20 @@ use fuel_core_chain_config::{ MessageConfig, }; use fuel_core_executor::refs::ContractStorageTrait; -use fuel_core_poa::ports::BlockDb; -use fuel_core_producer::ports::BlockProducerDatabase; use fuel_core_storage::{ - not_found, - tables::{ - FuelBlocks, - SealedBlockConsensus, + transactional::{ + StorageTransaction, + Transactional, }, Error as StorageError, Result as StorageResult, - StorageAsMut, - StorageAsRef, -}; -use fuel_core_types::blockchain::{ - block::CompressedBlock, - consensus::Consensus, - primitives::{ - BlockHeight, - BlockId, - }, }; +use fuel_core_types::blockchain::primitives::BlockHeight; use serde::{ de::DeserializeOwned, Serialize, }; use std::{ - borrow::Cow, fmt::{ self, Debug, @@ -59,23 +46,6 @@ type DatabaseResult = Result; #[cfg(feature = "rocksdb")] use crate::state::rocks_db::RocksDb; -use fuel_core_storage::tables::{ - Coins, - ContractsRawCode, - Messages, -}; -use fuel_core_txpool::ports::TxPoolDb; -use fuel_core_types::{ - entities::{ - coin::CompressedCoin, - message::Message, - }, - fuel_tx::UtxoId, - fuel_types::{ - ContractId, - MessageId, - }, -}; #[cfg(feature = "rocksdb")] use std::path::Path; #[cfg(feature = "rocksdb")] @@ -99,6 +69,7 @@ mod state; pub mod balances; pub mod metadata; // TODO: Rename in a separate PR into `transaction` +pub mod storage; pub mod transactional; pub mod transactions; pub mod vm_database; @@ -143,6 +114,8 @@ pub enum Column { OwnedMessageIds = 15, /// The column that stores the consensus metadata associated with a finalized fuel block FuelBlockConsensus = 16, + /// The column that stores the BMT MMR roots of the block headers + BlockHeaderMerkle = 17, } #[derive(Clone, Debug)] @@ -286,6 +259,12 @@ impl Database { } } +impl Transactional for Database { + fn transaction(&self) -> StorageTransaction { + StorageTransaction::new(self.transaction()) + } +} + impl AsRef for Database { fn as_ref(&self) -> &Database { self @@ -328,65 +307,6 @@ impl Default for Database { } } -impl BlockDb for Database { - fn block_height(&self) -> anyhow::Result { - Ok(self.latest_height()?.unwrap_or_default()) - } - - fn seal_block( - &mut self, - block_id: BlockId, - consensus: Consensus, - ) -> anyhow::Result<()> { - self.storage::() - .insert(&block_id, &consensus) - .map(|_| ()) - .map_err(Into::into) - } -} - -impl TxPoolDb for Database { - fn utxo(&self, utxo_id: &UtxoId) -> StorageResult> { - self.storage::() - .get(utxo_id) - .map(|t| t.map(|t| t.as_ref().clone())) - } - - fn contract_exist(&self, contract_id: &ContractId) -> StorageResult { - self.storage::().contains_key(contract_id) - } - - fn message(&self, message_id: &MessageId) -> StorageResult> { - self.storage::() - .get(message_id) - .map(|t| t.map(|t| t.as_ref().clone())) - } - - fn current_block_height(&self) -> StorageResult { - self.latest_height() - .map(|h| h.unwrap_or_default()) - .map_err(Into::into) - } -} - -impl BlockProducerDatabase for Database { - fn get_block( - &self, - fuel_height: BlockHeight, - ) -> StorageResult>> { - let id = self - .get_block_id(fuel_height)? - .ok_or(not_found!("BlockId"))?; - self.storage::().get(&id).map_err(Into::into) - } - - fn current_block_height(&self) -> StorageResult { - self.latest_height() - .map(|h| h.unwrap_or_default()) - .map_err(Into::into) - } -} - /// Implement `ChainConfigDb` so that `Database` can be passed to /// `StateConfig's` `generate_state_config()` method impl ChainConfigDb for Database { @@ -402,7 +322,7 @@ impl ChainConfigDb for Database { Self::get_message_config(self).map_err(Into::into) } - fn get_block_height(&self) -> StorageResult> { - Self::latest_height(self).map_err(Into::into) + fn get_block_height(&self) -> StorageResult { + Self::latest_height(self) } } diff --git a/crates/fuel-core/src/database/block.rs b/crates/fuel-core/src/database/block.rs index 271f67d13b3..c31688d4a29 100644 --- a/crates/fuel-core/src/database/block.rs +++ b/crates/fuel-core/src/database/block.rs @@ -1,5 +1,6 @@ use crate::{ database::{ + storage::IntoDatabaseKey, Column, Database, Error as DatabaseError, @@ -85,15 +86,10 @@ impl StorageMutate for Database { } impl Database { - pub fn latest_height(&self) -> DatabaseResult> { - let id = self.ids_of_latest_block()?; - // if no blocks, check if chain was configured with a base height - let id = match id { - Some((id, _)) => Some(id), - None => self.get_starting_chain_height()?, - }; - - Ok(id) + pub fn latest_height(&self) -> StorageResult { + self.ids_of_latest_block()? + .map(|(height, _)| height) + .ok_or(not_found!("BlockHeight")) } /// Get the current block at the head of the chain. @@ -105,7 +101,7 @@ impl Database { } } - pub fn block_time(&self, height: BlockHeight) -> StorageResult { + pub fn block_time(&self, height: &BlockHeight) -> StorageResult { let id = self.get_block_id(height)?.unwrap_or_default(); let block = self .storage::() @@ -114,9 +110,9 @@ impl Database { Ok(block.header().time().to_owned()) } - pub fn get_block_id(&self, height: BlockHeight) -> StorageResult> { - Database::get(self, &height.to_bytes()[..], Column::FuelBlockIds) - .map_err(Into::into) + pub fn get_block_id(&self, height: &BlockHeight) -> StorageResult> { + let key = height.database_key(); + Database::get(self, key.as_ref(), Column::FuelBlockIds).map_err(Into::into) } pub fn all_block_ids( diff --git a/crates/fuel-core/src/database/metadata.rs b/crates/fuel-core/src/database/metadata.rs index 514fadd63bb..8632c6728ea 100644 --- a/crates/fuel-core/src/database/metadata.rs +++ b/crates/fuel-core/src/database/metadata.rs @@ -5,11 +5,9 @@ use crate::database::{ Result as DatabaseResult, }; use fuel_core_chain_config::ChainConfig; -use fuel_core_types::blockchain::primitives::BlockHeight; pub(crate) const DB_VERSION_KEY: &[u8] = b"version"; pub(crate) const CHAIN_NAME_KEY: &[u8] = b"chain_name"; -pub(crate) const CHAIN_HEIGHT_KEY: &[u8] = b"chain_height"; #[cfg(feature = "relayer")] pub(crate) const FINALIZED_DA_HEIGHT_KEY: &[u8] = b"finalized_da_height"; @@ -28,23 +26,11 @@ impl Database { } })?; - let chain_height = config - .initial_state - .as_ref() - .and_then(|c| c.height) - .unwrap_or_default(); - let _: Option = self.insert(DB_VERSION_KEY, Column::Metadata, DB_VERSION)?; - let _: Option = - self.insert(CHAIN_HEIGHT_KEY, Column::Metadata, chain_height)?; Ok(()) } pub fn get_chain_name(&self) -> DatabaseResult> { self.get(CHAIN_NAME_KEY, Column::Metadata) } - - pub fn get_starting_chain_height(&self) -> DatabaseResult> { - self.get(CHAIN_HEIGHT_KEY, Column::Metadata) - } } diff --git a/crates/fuel-core/src/database/p2p.rs b/crates/fuel-core/src/database/p2p.rs index 40e49de53f7..49ce13f9958 100644 --- a/crates/fuel-core/src/database/p2p.rs +++ b/crates/fuel-core/src/database/p2p.rs @@ -11,7 +11,7 @@ use fuel_core_types::blockchain::{ impl P2pDb for Database { async fn get_sealed_block( &self, - height: BlockHeight, + height: &BlockHeight, ) -> StorageResult> { self.get_sealed_block_by_height(height) } diff --git a/crates/fuel-core/src/database/sealed_block.rs b/crates/fuel-core/src/database/sealed_block.rs index 5839adcc747..eb98ce29d8d 100644 --- a/crates/fuel-core/src/database/sealed_block.rs +++ b/crates/fuel-core/src/database/sealed_block.rs @@ -84,7 +84,7 @@ impl Database { /// Reusable across different trait implementations pub fn get_sealed_block_by_height( &self, - height: BlockHeight, + height: &BlockHeight, ) -> StorageResult> { let block_id = self.get_block_id(height)?.ok_or(not_found!("BlockId"))?; self.get_sealed_block_by_id(&block_id) diff --git a/crates/fuel-core/src/database/storage.rs b/crates/fuel-core/src/database/storage.rs new file mode 100644 index 00000000000..3b009972008 --- /dev/null +++ b/crates/fuel-core/src/database/storage.rs @@ -0,0 +1,90 @@ +use crate::database::{ + Column, + Database, +}; +use fuel_core_storage::{ + tables::FuelBlockRoots, + Error as StorageError, + Mappable, + Result as StorageResult, + StorageInspect, + StorageMutate, +}; +use fuel_core_types::blockchain::primitives::BlockHeight; +use serde::{ + de::DeserializeOwned, + Serialize, +}; +use std::borrow::Cow; + +impl DatabaseColumn for FuelBlockRoots { + fn column() -> Column { + Column::BlockHeaderMerkle + } +} + +/// The table has a corresponding column in the database. +trait DatabaseColumn { + /// The column of the table. + fn column() -> Column; +} + +impl StorageInspect for Database +where + T: Mappable + DatabaseColumn, + T::Key: IntoDatabaseKey, + T::GetValue: DeserializeOwned, +{ + type Error = StorageError; + + fn get(&self, key: &T::Key) -> StorageResult>> { + let key = key.database_key(); + self.get(key.as_ref(), T::column()).map_err(Into::into) + } + + fn contains_key(&self, key: &T::Key) -> StorageResult { + let key = key.database_key(); + self.exists(key.as_ref(), T::column()).map_err(Into::into) + } +} + +impl StorageMutate for Database +where + T: Mappable + DatabaseColumn, + T::Key: IntoDatabaseKey, + T::SetValue: Serialize, + T::GetValue: DeserializeOwned, +{ + fn insert( + &mut self, + key: &T::Key, + value: &T::SetValue, + ) -> StorageResult> { + let key = key.database_key(); + Database::insert(self, key.as_ref(), T::column(), value).map_err(Into::into) + } + + fn remove(&mut self, key: &T::Key) -> StorageResult> { + let key = key.database_key(); + Database::remove(self, key.as_ref(), T::column()).map_err(Into::into) + } +} + +// TODO: Implement this trait for all keys and use `type Type = MultiKey` for tuples. +// -> After replace all common implementation with blanket, if possible. +/// Some keys requires pre-processing that could change their type. +pub trait IntoDatabaseKey { + /// A new type of prepared database key that can be converted into bytes. + type Type: AsRef<[u8]>; + + /// Coverts the key into database key that supports byte presentation. + fn database_key(&self) -> Self::Type; +} + +impl IntoDatabaseKey for BlockHeight { + type Type = [u8; 4]; + + fn database_key(&self) -> Self::Type { + self.to_bytes() + } +} diff --git a/crates/fuel-core/src/database/vm_database.rs b/crates/fuel-core/src/database/vm_database.rs index 56339a78390..7e54107fc2c 100644 --- a/crates/fuel-core/src/database/vm_database.rs +++ b/crates/fuel-core/src/database/vm_database.rs @@ -136,7 +136,7 @@ impl InterpreterStorage for VmDatabase { return Err(anyhow!("block height too high for timestamp").into()) } height if height == self.current_block_height => self.current_timestamp, - height => self.database.block_time(height.into())?, + height => self.database.block_time(&height.into())?, }; Ok(timestamp.0) } @@ -149,7 +149,7 @@ impl InterpreterStorage for VmDatabase { } else { // this will return 0x00**32 for block height 0 as well self.database - .get_block_id(block_height.into())? + .get_block_id(&block_height.into())? .ok_or(not_found!("BlockId")) .map(Into::into) } diff --git a/crates/fuel-core/src/graphql_api/ports.rs b/crates/fuel-core/src/graphql_api/ports.rs index 2e7958ad9c3..ccbf250d8ee 100644 --- a/crates/fuel-core/src/graphql_api/ports.rs +++ b/crates/fuel-core/src/graphql_api/ports.rs @@ -38,6 +38,7 @@ use fuel_core_types::{ graphql_api::ContractBalance, txpool::TransactionStatus, }, + tai64::Tai64, }; /// The database port expected by GraphQL API service. @@ -58,7 +59,7 @@ pub trait DatabaseBlocks: StorageInspect + StorageInspect { - fn block_id(&self, height: BlockHeight) -> StorageResult; + fn block_id(&self, height: &BlockHeight) -> StorageResult; fn blocks_ids( &self, @@ -130,3 +131,11 @@ pub trait DatabaseChain { fn base_chain_height(&self) -> StorageResult; } + +#[async_trait::async_trait] +pub trait ConsensusModulePort: Send + Sync { + async fn manual_produce_block( + &self, + block_times: Vec>, + ) -> anyhow::Result<()>; +} diff --git a/crates/fuel-core/src/graphql_api/service.rs b/crates/fuel-core/src/graphql_api/service.rs index 2ad61df8686..d2cd7e897cb 100644 --- a/crates/fuel-core/src/graphql_api/service.rs +++ b/crates/fuel-core/src/graphql_api/service.rs @@ -1,5 +1,8 @@ use crate::{ - fuel_core_graphql_api::ports::DatabasePort, + fuel_core_graphql_api::ports::{ + ConsensusModulePort, + DatabasePort, + }, graphql_api::Config, schema::{ CoreSchema, @@ -56,7 +59,6 @@ use std::{ TcpListener, }, pin::Pin, - sync::Arc, }; use tokio_stream::StreamExt; use tower_http::{ @@ -67,11 +69,10 @@ use tower_http::{ pub type Service = fuel_core_services::ServiceRunner; pub type Database = Box; -// TODO: When the port for `Executor` will exist we need to replace it with `Box -pub type Executor = crate::service::adapters::ExecutorAdapter; +pub type ConsensusModule = Box; // TODO: When the port of BlockProducer will exist we need to replace it with // `Box -pub type BlockProducer = Arc>; +pub type BlockProducer = crate::service::adapters::BlockProducerAdapter; // TODO: When the port of TxPool will exist we need to replace it with // `Box. In the future GraphQL should not be aware of `TxPool`. It should // use only `Database` to receive all information about @@ -140,7 +141,7 @@ pub fn new_service( schema: CoreSchemaBuilder, producer: BlockProducer, txpool: TxPool, - executor: Executor, + consensus_module: ConsensusModule, ) -> anyhow::Result { let network_addr = config.addr; let schema = schema @@ -148,7 +149,7 @@ pub fn new_service( .data(database) .data(producer) .data(txpool) - .data(executor) + .data(consensus_module) .extension(Tracing) .finish(); diff --git a/crates/fuel-core/src/query/block.rs b/crates/fuel-core/src/query/block.rs index 0f1642831d5..42ed509d1d8 100644 --- a/crates/fuel-core/src/query/block.rs +++ b/crates/fuel-core/src/query/block.rs @@ -36,7 +36,7 @@ impl BlockQueryContext<'_> { Ok(block) } - pub fn block_id(&self, height: BlockHeight) -> StorageResult { + pub fn block_id(&self, height: &BlockHeight) -> StorageResult { self.0.block_id(height) } diff --git a/crates/fuel-core/src/schema/block.rs b/crates/fuel-core/src/schema/block.rs index 061a6c5874e..e5b769796a9 100644 --- a/crates/fuel-core/src/schema/block.rs +++ b/crates/fuel-core/src/schema/block.rs @@ -4,7 +4,7 @@ use super::scalars::{ }; use crate::{ fuel_core_graphql_api::{ - service::Executor, + service::ConsensusModule, Config as GraphQLConfig, IntoApiResult, }, @@ -34,30 +34,16 @@ use async_graphql::{ SimpleObject, Union, }; -use fuel_core_poa::service::seal_block; -use fuel_core_producer::ports::Executor as ExecutorTrait; use fuel_core_storage::{ iter::IntoBoxedIter, Result as StorageResult, }; use fuel_core_types::{ blockchain::{ - block::{ - CompressedBlock, - PartialFuelBlock, - }, - header::{ - ApplicationHeader, - BlockHeader, - ConsensusHeader, - PartialBlockHeader, - }, + block::CompressedBlock, + header::BlockHeader, }, fuel_types, - services::executor::{ - ExecutionBlock, - ExecutionResult, - }, tai64::Tai64, }; @@ -210,7 +196,7 @@ impl BlockQuery { (None, Some(height)) => { let height: u64 = height.into(); let height: u32 = height.try_into()?; - data.block_id(height.into()) + data.block_id(&height.into()) } (None, None) => { return Err(async_graphql::Error::new("Missing either id or height")) @@ -308,7 +294,7 @@ impl BlockMutation { time: Option, ) -> async_graphql::Result { let query = BlockQueryContext(ctx.data_unchecked()); - let executor = ctx.data_unchecked::().clone(); + let consensus_module = ctx.data_unchecked::(); let config = ctx.data_unchecked::().clone(); if !config.manual_blocks_enabled { @@ -316,42 +302,10 @@ impl BlockMutation { anyhow!("Manual Blocks must be enabled to use this endpoint").into(), ) } - // todo!("trigger block production manually"); let latest_block = query.latest_block()?; - let block_time = get_time_closure(&latest_block, time, blocks_to_produce.0)?; - - for idx in 0..blocks_to_produce.0 { - let current_height = query.latest_block_height().unwrap_or_default(); - let new_block_height = current_height + 1u32.into(); - - let block = PartialFuelBlock::new( - PartialBlockHeader { - consensus: ConsensusHeader { - height: new_block_height, - time: block_time(idx), - prev_root: Default::default(), - generated: Default::default(), - }, - application: ApplicationHeader { - da_height: Default::default(), - generated: Default::default(), - }, - }, - vec![], - ); - - // TODO: Instead of using raw `Executor` here, we need to use `CM` - Consensus Module. - // It will guarantee that the block is entirely valid and all information is stored. - // For that, we need to manually trigger block production and reset/ignore timers - // inside CM for `blocks_to_produce` blocks. Also in this case `CM` will notify - // `TxPool` about a new block. - let (ExecutionResult { block, .. }, mut db_transaction) = executor - .execute_without_commit(ExecutionBlock::Production(block))? - .into(); - seal_block(&config.consensus_key, &block, db_transaction.as_mut())?; - db_transaction.commit()?; - } + let block_times = get_time_closure(&latest_block, time, blocks_to_produce.0)?; + consensus_module.manual_produce_block(block_times).await?; query .latest_block_height() @@ -364,21 +318,27 @@ fn get_time_closure( latest_block: &CompressedBlock, time_parameters: Option, blocks_to_produce: u64, -) -> anyhow::Result Tai64 + Send>> { +) -> anyhow::Result>> { if let Some(params) = time_parameters { - check_start_after_latest_block(latest_block, params.start_time.0)?; + let start_time = params.start_time.into(); + check_start_after_latest_block(latest_block, start_time)?; check_block_time_overflow(¶ms, blocks_to_produce)?; - - return Ok(Box::new(move |idx: u64| { - let (timestamp, _) = params - .start_time - .0 - .overflowing_add(params.block_time_interval.0.overflowing_mul(idx).0); - Tai64(timestamp) - })) + let interval: u64 = params.block_time_interval.into(); + + let vec = (0..blocks_to_produce) + .into_iter() + .map(|idx| { + let (timestamp, _) = params + .start_time + .0 + .overflowing_add(interval.overflowing_mul(idx).0); + Some(Tai64(timestamp)) + }) + .collect(); + return Ok(vec) }; - Ok(Box::new(|_| Tai64::now())) + Ok(vec![None; blocks_to_produce as usize]) } fn check_start_after_latest_block( diff --git a/crates/fuel-core/src/schema/dap.rs b/crates/fuel-core/src/schema/dap.rs index 2cb02679d7f..e427e2dd4dd 100644 --- a/crates/fuel-core/src/schema/dap.rs +++ b/crates/fuel-core/src/schema/dap.rs @@ -351,10 +351,7 @@ impl DapMutation { let db = locked.db.get(&id).ok_or("Invalid debugging session ID")?; let checked_tx = tx - .into_checked_basic( - db.latest_height()?.unwrap_or_default().into(), - &locked.params, - )? + .into_checked_basic(db.latest_height()?.into(), &locked.params)? .into(); let vm = locked diff --git a/crates/fuel-core/src/schema/tx.rs b/crates/fuel-core/src/schema/tx.rs index 6f6c936437e..13519a2e0ac 100644 --- a/crates/fuel-core/src/schema/tx.rs +++ b/crates/fuel-core/src/schema/tx.rs @@ -31,6 +31,7 @@ use async_graphql::{ Object, Subscription, }; +use fuel_core_poa::ports::BlockProducer as BlockProducerTrait; use fuel_core_storage::Result as StorageResult; use fuel_core_types::{ fuel_tx::{ diff --git a/crates/fuel-core/src/service.rs b/crates/fuel-core/src/service.rs index 332511bac25..6bce27d9f03 100644 --- a/crates/fuel-core/src/service.rs +++ b/crates/fuel-core/src/service.rs @@ -175,7 +175,7 @@ impl Task { /// Private inner method for initializing the fuel service task pub fn new(database: Database, config: Config) -> anyhow::Result { // initialize state - genesis::initialize_state(&config, &database)?; + genesis::maybe_initialize_state(&config, &database)?; // initialize sub services let (services, shared) = sub_services::init_sub_services(&config, &database)?; diff --git a/crates/fuel-core/src/service/adapters.rs b/crates/fuel-core/src/service/adapters.rs index c91df9b581d..97a3bd62c43 100644 --- a/crates/fuel-core/src/service/adapters.rs +++ b/crates/fuel-core/src/service/adapters.rs @@ -2,25 +2,22 @@ use crate::{ database::Database, service::Config, }; +use fuel_core_consensus_module::block_verifier::Verifier; use fuel_core_txpool::service::SharedState as TxPoolSharedState; -use fuel_core_types::blockchain::SealedBlock; use std::sync::Arc; -use tokio::sync::broadcast::Sender; +pub mod block_importer; +pub mod consensus_module; pub mod graphql_api; -pub mod poa; pub mod producer; pub mod txpool; -/// This is used to get block import events from coordinator source -/// and pass them to the txpool. #[derive(Clone)] -pub struct BlockImportAdapter { - // TODO: We should use `fuel_core_poa::Service here but for that we need to fix - // the `start` of the process and store the task inside of the `Service`. - pub tx: Sender, +pub struct PoAAdapter { + shared_state: fuel_core_poa::service::SharedState, } +#[derive(Clone)] pub struct TxPoolAdapter { service: TxPoolSharedState, } @@ -37,16 +34,29 @@ pub struct ExecutorAdapter { pub config: Config, } +#[derive(Clone)] +pub struct VerifierAdapter { + pub block_verifier: Arc>, +} + +#[derive(Clone)] pub struct MaybeRelayerAdapter { pub database: Database, #[cfg(feature = "relayer")] pub relayer_synced: Option, } +#[derive(Clone)] pub struct BlockProducerAdapter { pub block_producer: Arc>, } +#[derive(Clone)] +pub struct BlockImporterAdapter { + pub block_importer: + Arc>, +} + #[cfg(feature = "p2p")] #[derive(Clone)] pub struct P2PAdapter { diff --git a/crates/fuel-core/src/service/adapters/block_importer.rs b/crates/fuel-core/src/service/adapters/block_importer.rs new file mode 100644 index 00000000000..3bb8c4efbf6 --- /dev/null +++ b/crates/fuel-core/src/service/adapters/block_importer.rs @@ -0,0 +1,99 @@ +use crate::{ + database::Database, + service::adapters::{ + BlockImporterAdapter, + ExecutorAdapter, + VerifierAdapter, + }, +}; +use fuel_core_importer::{ + ports::{ + BlockVerifier, + ExecutorDatabase, + ImporterDatabase, + }, + Config, + Importer, +}; +use fuel_core_storage::{ + tables::{ + FuelBlockRoots, + SealedBlockConsensus, + }, + transactional::StorageTransaction, + Result as StorageResult, + StorageAsMut, +}; +use fuel_core_types::{ + blockchain::{ + block::Block, + consensus::Consensus, + primitives::{ + BlockHeight, + BlockId, + }, + }, + fuel_tx::Bytes32, + services::block_importer::UncommittedResult, +}; +use std::sync::Arc; + +impl BlockImporterAdapter { + pub fn new( + config: Config, + database: Database, + executor: ExecutorAdapter, + verifier: VerifierAdapter, + ) -> Self { + Self { + block_importer: Arc::new(Importer::new(config, database, executor, verifier)), + } + } +} + +impl fuel_core_poa::ports::BlockImporter for BlockImporterAdapter { + type Database = Database; + + fn commit_result( + &self, + result: UncommittedResult>, + ) -> anyhow::Result<()> { + self.block_importer.commit_result(result) + } +} + +impl BlockVerifier for VerifierAdapter { + fn verify_block_fields( + &self, + consensus: &Consensus, + block: &Block, + ) -> anyhow::Result<()> { + self.block_verifier.verify_block_fields(consensus, block) + } +} + +impl ImporterDatabase for Database { + fn latest_block_height(&self) -> StorageResult { + self.latest_height() + } +} + +impl ExecutorDatabase for Database { + fn seal_block( + &mut self, + block_id: &BlockId, + consensus: &Consensus, + ) -> StorageResult> { + self.storage::() + .insert(block_id, consensus) + .map_err(Into::into) + } + + fn insert_block_header_merkle_root( + &mut self, + height: &BlockHeight, + root: &Bytes32, + ) -> StorageResult> { + self.storage::().insert(height, root) + } +} diff --git a/crates/fuel-core/src/service/adapters/consensus_module.rs b/crates/fuel-core/src/service/adapters/consensus_module.rs new file mode 100644 index 00000000000..590499187cf --- /dev/null +++ b/crates/fuel-core/src/service/adapters/consensus_module.rs @@ -0,0 +1,61 @@ +use crate::{ + database::Database, + service::{ + adapters::{ + MaybeRelayerAdapter, + VerifierAdapter, + }, + Config, + }, +}; +use fuel_core_consensus_module::block_verifier::{ + config::Config as VerifierConfig, + Verifier, +}; +use fuel_core_producer::ports::BlockProducerDatabase; +use fuel_core_storage::{ + not_found, + tables::FuelBlockRoots, + Result as StorageResult, + StorageAsRef, +}; +use fuel_core_types::{ + blockchain::{ + header::BlockHeader, + primitives::BlockHeight, + }, + fuel_tx::Bytes32, +}; +use std::{ + borrow::Cow, + sync::Arc, +}; + +pub mod poa; + +impl VerifierAdapter { + pub fn new( + config: &Config, + database: Database, + relayer: MaybeRelayerAdapter, + ) -> Self { + let config = + VerifierConfig::new(config.chain_conf.clone(), config.manual_blocks_enabled); + Self { + block_verifier: Arc::new(Verifier::new(config, database, relayer)), + } + } +} + +impl fuel_core_poa::verifier::Database for Database { + fn block_header(&self, height: &BlockHeight) -> StorageResult { + Ok(self.get_block(height)?.header().clone()) + } + + fn block_header_merkle_root(&self, height: &BlockHeight) -> StorageResult { + self.storage::() + .get(height)? + .ok_or(not_found!(FuelBlockRoots)) + .map(Cow::into_owned) + } +} diff --git a/crates/fuel-core/src/service/adapters/poa.rs b/crates/fuel-core/src/service/adapters/consensus_module/poa.rs similarity index 69% rename from crates/fuel-core/src/service/adapters/poa.rs rename to crates/fuel-core/src/service/adapters/consensus_module/poa.rs index e4a2886cefe..32fbf3116b4 100644 --- a/crates/fuel-core/src/service/adapters/poa.rs +++ b/crates/fuel-core/src/service/adapters/consensus_module/poa.rs @@ -1,11 +1,16 @@ use crate::{ database::Database, + fuel_core_graphql_api::ports::ConsensusModulePort, service::adapters::{ BlockProducerAdapter, + PoAAdapter, TxPoolAdapter, }, }; -use fuel_core_poa::ports::TransactionPool; +use fuel_core_poa::{ + ports::TransactionPool, + service::SharedState, +}; use fuel_core_services::stream::BoxStream; use fuel_core_storage::transactional::StorageTransaction; use fuel_core_types::{ @@ -23,8 +28,25 @@ use fuel_core_types::{ TxStatus, }, }, + tai64::Tai64, }; +impl PoAAdapter { + pub fn new(shared_state: SharedState) -> Self { + Self { shared_state } + } +} + +#[async_trait::async_trait] +impl ConsensusModulePort for PoAAdapter { + async fn manual_produce_block( + &self, + block_times: Vec>, + ) -> anyhow::Result<()> { + self.shared_state.manually_produce_block(block_times).await + } +} + impl TransactionPool for TxPoolAdapter { fn pending_number(&self) -> usize { self.service.pending_number() @@ -51,14 +73,17 @@ impl TransactionPool for TxPoolAdapter { } #[async_trait::async_trait] -impl fuel_core_poa::ports::BlockProducer for BlockProducerAdapter { +impl fuel_core_poa::ports::BlockProducer for BlockProducerAdapter { + type Database = Database; + async fn produce_and_execute_block( &self, height: BlockHeight, + block_time: Option, max_gas: Word, ) -> anyhow::Result>> { self.block_producer - .produce_and_execute_block(height, max_gas) + .produce_and_execute_block(height, block_time, max_gas) .await } diff --git a/crates/fuel-core/src/service/adapters/graphql_api.rs b/crates/fuel-core/src/service/adapters/graphql_api.rs index 323e9dc6c68..14aedd07ed2 100644 --- a/crates/fuel-core/src/service/adapters/graphql_api.rs +++ b/crates/fuel-core/src/service/adapters/graphql_api.rs @@ -48,9 +48,9 @@ use fuel_core_types::{ }; impl DatabaseBlocks for Database { - fn block_id(&self, height: BlockHeight) -> StorageResult { + fn block_id(&self, height: &BlockHeight) -> StorageResult { self.get_block_id(height) - .and_then(|heigh| heigh.ok_or(not_found!("BlockId"))) + .and_then(|height| height.ok_or(not_found!("BlockId"))) } fn blocks_ids( diff --git a/crates/fuel-core/src/service/adapters/producer.rs b/crates/fuel-core/src/service/adapters/producer.rs index 29fa034ce32..21b3168dc70 100644 --- a/crates/fuel-core/src/service/adapters/producer.rs +++ b/crates/fuel-core/src/service/adapters/producer.rs @@ -2,6 +2,7 @@ use crate::{ database::Database, executor::Executor, service::adapters::{ + BlockProducerAdapter, ExecutorAdapter, MaybeRelayerAdapter, TxPoolAdapter, @@ -9,15 +10,23 @@ use crate::{ }; use fuel_core_producer::ports::TxPool; use fuel_core_storage::{ + not_found, + tables::{ + FuelBlockRoots, + FuelBlocks, + }, transactional::StorageTransaction, Result as StorageResult, + StorageAsRef, }; use fuel_core_types::{ blockchain::{ + block::CompressedBlock, primitives, primitives::BlockHeight, }, fuel_tx::Receipt, + fuel_types::Bytes32, services::{ executor::{ ExecutionBlock, @@ -27,6 +36,18 @@ use fuel_core_types::{ txpool::ArcPoolTx, }, }; +use std::{ + borrow::Cow, + sync::Arc, +}; + +impl BlockProducerAdapter { + pub fn new(block_producer: fuel_core_producer::Producer) -> Self { + Self { + block_producer: Arc::new(block_producer), + } + } +} #[async_trait::async_trait] impl TxPool for TxPoolAdapter { @@ -85,3 +106,23 @@ impl fuel_core_producer::ports::Relayer for MaybeRelayerAdapter { } } } + +impl fuel_core_producer::ports::BlockProducerDatabase for Database { + fn get_block(&self, height: &BlockHeight) -> StorageResult> { + let id = self.get_block_id(height)?.ok_or(not_found!("BlockId"))?; + self.storage::() + .get(&id)? + .ok_or(not_found!(FuelBlocks)) + } + + fn block_header_merkle_root(&self, height: &BlockHeight) -> StorageResult { + self.storage::() + .get(height)? + .ok_or(not_found!(FuelBlocks)) + .map(Cow::into_owned) + } + + fn current_block_height(&self) -> StorageResult { + self.latest_height() + } +} diff --git a/crates/fuel-core/src/service/adapters/txpool.rs b/crates/fuel-core/src/service/adapters/txpool.rs index d79577366b4..66c30e03c82 100644 --- a/crates/fuel-core/src/service/adapters/txpool.rs +++ b/crates/fuel-core/src/service/adapters/txpool.rs @@ -1,34 +1,54 @@ -use crate::service::adapters::{ - BlockImportAdapter, - P2PAdapter, +use crate::{ + database::Database, + service::adapters::{ + BlockImporterAdapter, + P2PAdapter, + }, }; use fuel_core_services::stream::BoxStream; -use fuel_core_txpool::ports::BlockImport; +use fuel_core_storage::{ + tables::{ + Coins, + ContractsRawCode, + Messages, + }, + Result as StorageResult, + StorageAsRef, +}; +use fuel_core_txpool::ports::BlockImporter; use fuel_core_types::{ - blockchain::SealedBlock, - fuel_tx::Transaction, - services::p2p::{ - GossipsubMessageAcceptance, - TransactionGossipData, + blockchain::primitives::BlockHeight, + entities::{ + coin::CompressedCoin, + message::Message, + }, + fuel_tx::{ + Transaction, + UtxoId, + }, + fuel_types::{ + ContractId, + MessageId, + }, + services::{ + block_importer::ImportResult, + p2p::{ + GossipsubMessageAcceptance, + TransactionGossipData, + }, }, }; use std::sync::Arc; -use tokio::sync::broadcast::Sender; -impl BlockImportAdapter { - pub fn new(tx: Sender) -> Self { - Self { tx } - } -} - -impl BlockImport for BlockImportAdapter { - fn block_events(&self) -> BoxStream { +impl BlockImporter for BlockImporterAdapter { + fn block_events(&self) -> BoxStream> { use tokio_stream::{ wrappers::BroadcastStream, StreamExt, }; Box::pin( - BroadcastStream::new(self.tx.subscribe()).filter_map(|result| result.ok()), + BroadcastStream::new(self.block_importer.subscribe()) + .filter_map(|result| result.ok()), ) } } @@ -85,3 +105,25 @@ impl fuel_core_txpool::ports::PeerToPeer for P2PAdapter { Ok(()) } } + +impl fuel_core_txpool::ports::TxPoolDb for Database { + fn utxo(&self, utxo_id: &UtxoId) -> StorageResult> { + self.storage::() + .get(utxo_id) + .map(|t| t.map(|t| t.as_ref().clone())) + } + + fn contract_exist(&self, contract_id: &ContractId) -> StorageResult { + self.storage::().contains_key(contract_id) + } + + fn message(&self, message_id: &MessageId) -> StorageResult> { + self.storage::() + .get(message_id) + .map(|t| t.map(|t| t.as_ref().clone())) + } + + fn current_block_height(&self) -> StorageResult { + self.latest_height() + } +} diff --git a/crates/fuel-core/src/service/genesis.rs b/crates/fuel-core/src/service/genesis.rs index b0f3ca51a3d..06691b85489 100644 --- a/crates/fuel-core/src/service/genesis.rs +++ b/crates/fuel-core/src/service/genesis.rs @@ -9,7 +9,7 @@ use fuel_core_chain_config::{ StateConfig, }; use fuel_core_executor::refs::ContractRef; -use fuel_core_poa::ports::BlockDb; +use fuel_core_importer::Importer; use fuel_core_storage::{ tables::{ Coins, @@ -21,7 +21,7 @@ use fuel_core_storage::{ FuelBlocks, Messages, }, - transactional::Transaction, + transactional::Transactional, MerkleRoot, StorageAsMut, }; @@ -38,6 +38,7 @@ use fuel_core_types::{ PartialBlockHeader, }, primitives::Empty, + SealedBlock, }, entities::{ coin::{ @@ -57,30 +58,34 @@ use fuel_core_types::{ Bytes32, ContractId, }, + services::block_importer::{ + ImportResult, + UncommittedResult as UncommittedImportResult, + }, }; use itertools::Itertools; /// Loads state from the chain config into database -pub(crate) fn initialize_state( +pub(crate) fn maybe_initialize_state( config: &Config, database: &Database, ) -> anyhow::Result<()> { // check if chain is initialized if database.get_chain_name()?.is_none() { - // start a db transaction for bulk-writing - let mut import_tx = database.transaction(); - let database = import_tx.as_mut(); - - add_genesis_block(config, database)?; - - // Write transaction to db - import_tx.commit()?; + import_genesis_block(config, database)?; } Ok(()) } -fn add_genesis_block(config: &Config, database: &mut Database) -> anyhow::Result<()> { +fn import_genesis_block( + config: &Config, + original_database: &Database, +) -> anyhow::Result<()> { + // start a db transaction for bulk-writing + let mut database_transaction = Transactional::transaction(original_database); + + let database = database_transaction.as_mut(); // Initialize the chain id and height. database.init(&config.chain_conf)?; @@ -102,6 +107,7 @@ fn add_genesis_block(config: &Config, database: &mut Database) -> anyhow::Result let block = Block::new( PartialBlockHeader { application: ApplicationHeader:: { + // TODO: Set `da_height` based on the chain config. da_height: Default::default(), generated: Empty, }, @@ -125,12 +131,30 @@ fn add_genesis_block(config: &Config, database: &mut Database) -> anyhow::Result &message_ids, ); - let seal = Consensus::Genesis(genesis); let block_id = block.id(); database .storage::() .insert(&block_id, &block.compress())?; - database.seal_block(block_id, seal) + let consensus = Consensus::Genesis(genesis); + let block = SealedBlock { + entity: block, + consensus, + }; + + let importer = Importer::new( + config.block_importer.clone(), + original_database.clone(), + (), + (), + ); + importer.commit_result(UncommittedImportResult::new( + ImportResult { + sealed_block: block, + tx_status: vec![], + }, + database_transaction, + ))?; + Ok(()) } fn init_coin_state( @@ -408,7 +432,6 @@ mod tests { assert_eq!( test_height, db.latest_height() - .unwrap() .expect("Expected a block height to be set") ) } @@ -572,7 +595,7 @@ mod tests { let db = &Database::default(); - initialize_state(&config, db).unwrap(); + maybe_initialize_state(&config, db).unwrap(); let expected_msg: Message = msg.into(); diff --git a/crates/fuel-core/src/service/sub_services.rs b/crates/fuel-core/src/service/sub_services.rs index 39790846f00..79c5bfe1c7d 100644 --- a/crates/fuel-core/src/service/sub_services.rs +++ b/crates/fuel-core/src/service/sub_services.rs @@ -9,27 +9,27 @@ use crate::{ }, service::{ adapters::{ - BlockImportAdapter, + BlockImporterAdapter, BlockProducerAdapter, ExecutorAdapter, MaybeRelayerAdapter, + PoAAdapter, TxPoolAdapter, + VerifierAdapter, }, Config, SharedState, SubServices, }, }; -use fuel_core_txpool::service::TxStatusChange; use std::sync::Arc; use tokio::sync::{ - broadcast, Mutex, Semaphore, }; pub type PoAService = - fuel_core_poa::Service; + fuel_core_poa::Service; #[cfg(feature = "relayer")] pub type RelayerService = fuel_core_relayer::Service; #[cfg(feature = "p2p")] @@ -41,8 +41,9 @@ pub fn init_sub_services( config: &Config, database: &Database, ) -> anyhow::Result<(SubServices, SharedState)> { + let last_height = database.latest_height()?; #[cfg(feature = "relayer")] - let relayer = if config.relayer.eth_client.is_some() { + let relayer_service = if config.relayer.eth_client.is_some() { Some(fuel_core_relayer::new_service( database.clone(), config.relayer.clone(), @@ -51,7 +52,11 @@ pub fn init_sub_services( None }; - let (block_import_tx, _) = broadcast::channel(16); + let relayer_adapter = MaybeRelayerAdapter { + database: database.clone(), + #[cfg(feature = "relayer")] + relayer_synced: relayer_service.as_ref().map(|r| r.shared.clone()), + }; #[cfg(feature = "p2p")] let network = { @@ -70,48 +75,51 @@ pub fn init_sub_services( let p2p_adapter = p2p_adapter; - let importer_adapter = BlockImportAdapter::new(block_import_tx); + let executor = ExecutorAdapter { + database: database.clone(), + config: config.clone(), + }; + + let verifier = + VerifierAdapter::new(config, database.clone(), relayer_adapter.clone()); + + let importer_adapter = BlockImporterAdapter::new( + config.block_importer.clone(), + database.clone(), + executor.clone(), + verifier, + ); let txpool = fuel_core_txpool::new_service( config.txpool.clone(), database.clone(), - TxStatusChange::new(100), importer_adapter.clone(), p2p_adapter, ); - - let executor = ExecutorAdapter { - database: database.clone(), - config: config.clone(), - }; + let tx_pool_adapter = TxPoolAdapter::new(txpool.shared.clone()); // restrict the max number of concurrent dry runs to the number of CPUs // as execution in the worst case will be CPU bound rather than I/O bound. let max_dry_run_concurrency = num_cpus::get(); - let block_producer = Arc::new(fuel_core_producer::Producer { + let block_producer = fuel_core_producer::Producer { config: config.block_producer.clone(), db: database.clone(), - txpool: Box::new(TxPoolAdapter::new(txpool.shared.clone())), - executor: Arc::new(executor.clone()), - relayer: Box::new(MaybeRelayerAdapter { - database: database.clone(), - #[cfg(feature = "relayer")] - relayer_synced: relayer.as_ref().map(|r| r.shared.clone()), - }), + txpool: Box::new(tx_pool_adapter.clone()), + executor: Arc::new(executor), + relayer: Box::new(relayer_adapter), lock: Mutex::new(()), dry_run_semaphore: Semaphore::new(max_dry_run_concurrency), - }); + }; + let producer_adapter = BlockProducerAdapter::new(block_producer); let poa = fuel_core_poa::new_service( + last_height, config.poa_config()?, - TxPoolAdapter::new(txpool.shared.clone()), - // TODO: Pass Importer - importer_adapter.tx, - BlockProducerAdapter { - block_producer: block_producer.clone(), - }, - database.clone(), + tx_pool_adapter, + producer_adapter.clone(), + importer_adapter, ); + let poa_adapter = PoAAdapter::new(poa.shared.clone()); // TODO: Figure out on how to move it into `fuel-core-graphql-api`. let schema = dap::init(build_schema(), config.chain_conf.transaction_parameters) @@ -130,9 +138,9 @@ pub fn init_sub_services( }, Box::new(database.clone()), schema, - block_producer, + producer_adapter, txpool.clone(), - executor, + Box::new(poa_adapter), )?; let shared = SharedState { @@ -140,7 +148,7 @@ pub fn init_sub_services( #[cfg(feature = "p2p")] network: network.shared.clone(), #[cfg(feature = "relayer")] - relayer: relayer.as_ref().map(|r| r.shared.clone()), + relayer: relayer_service.as_ref().map(|r| r.shared.clone()), graph_ql: graph_ql.shared.clone(), }; @@ -154,7 +162,7 @@ pub fn init_sub_services( ]; #[cfg(feature = "relayer")] - if let Some(relayer) = relayer { + if let Some(relayer) = relayer_service { services.push(Box::new(relayer)); } diff --git a/crates/services/consensus_module/poa/src/ports.rs b/crates/services/consensus_module/poa/src/ports.rs index dffb64f1346..e0de9cf04be 100644 --- a/crates/services/consensus_module/poa/src/ports.rs +++ b/crates/services/consensus_module/poa/src/ports.rs @@ -1,13 +1,7 @@ use fuel_core_services::stream::BoxStream; use fuel_core_storage::transactional::StorageTransaction; use fuel_core_types::{ - blockchain::{ - consensus::Consensus, - primitives::{ - BlockHeight, - BlockId, - }, - }, + blockchain::primitives::BlockHeight, fuel_asm::Word, fuel_tx::{ Receipt, @@ -15,14 +9,17 @@ use fuel_core_types::{ TxId, }, services::{ - executor::UncommittedResult, + block_importer::UncommittedResult as UncommittedImportResult, + executor::UncommittedResult as UncommittedExecutionResult, txpool::{ ArcPoolTx, TxStatus, }, }, + tai64::Tai64, }; +#[cfg_attr(test, mockall::automock)] pub trait TransactionPool: Send + Sync { /// Returns the number of pending transactions in the `TxPool`. fn pending_number(&self) -> usize; @@ -34,26 +31,20 @@ pub trait TransactionPool: Send + Sync { fn transaction_status_events(&self) -> BoxStream; } -pub trait BlockDb: Send + Sync { - fn block_height(&self) -> anyhow::Result; - - // Returns error if already sealed - fn seal_block( - &mut self, - block_id: BlockId, - consensus: Consensus, - ) -> anyhow::Result<()>; -} +#[cfg(test)] +use fuel_core_storage::test_helpers::EmptyStorage; +#[cfg_attr(test, mockall::automock(type Database=EmptyStorage;))] #[async_trait::async_trait] -pub trait BlockProducer: Send + Sync { - // TODO: Right now production and execution of the block is one step, but in the future, - // `produce_block` should only produce a block without affecting the blockchain state. +pub trait BlockProducer: Send + Sync { + type Database; + async fn produce_and_execute_block( &self, height: BlockHeight, + block_time: Option, max_gas: Word, - ) -> anyhow::Result>>; + ) -> anyhow::Result>>; async fn dry_run( &self, @@ -62,3 +53,13 @@ pub trait BlockProducer: Send + Sync { utxo_validation: Option, ) -> anyhow::Result>; } + +#[cfg_attr(test, mockall::automock(type Database=EmptyStorage;))] +pub trait BlockImporter: Send + Sync { + type Database; + + fn commit_result( + &self, + result: UncommittedImportResult>, + ) -> anyhow::Result<()>; +} diff --git a/crates/services/consensus_module/poa/src/service.rs b/crates/services/consensus_module/poa/src/service.rs index 44b7c11cb17..2e9b8806f0c 100644 --- a/crates/services/consensus_module/poa/src/service.rs +++ b/crates/services/consensus_module/poa/src/service.rs @@ -4,7 +4,7 @@ use crate::{ OnConflict, }, ports::{ - BlockDb, + BlockImporter, BlockProducer, TransactionPool, }, @@ -17,7 +17,6 @@ use anyhow::{ }; use fuel_core_services::{ stream::BoxStream, - EmptyShared, RunnableService, RunnableTask, ServiceRunner, @@ -45,92 +44,153 @@ use fuel_core_types::{ Secret, }, services::{ + block_importer::ImportResult, executor::{ ExecutionResult, - UncommittedResult, + UncommittedResult as UncommittedExecutionResult, }, txpool::TxStatus, + Uncommitted, }, + tai64::Tai64, }; use std::ops::Deref; use tokio::{ - sync::broadcast, + sync::{ + mpsc, + oneshot, + }, time::Instant, }; use tokio_stream::StreamExt; use tracing::error; -pub type Service = ServiceRunner>; +pub type Service = ServiceRunner>; + +#[derive(Clone)] +pub struct SharedState { + request_sender: mpsc::Sender, +} + +impl SharedState { + pub async fn manually_produce_block( + &self, + block_times: Vec>, + ) -> anyhow::Result<()> { + let (sender, receiver) = oneshot::channel(); + + self.request_sender + .send(Request::ManualBlocks((block_times, sender))) + .await?; + receiver.await? + } +} -pub struct Task { - pub(crate) block_gas_limit: Word, - pub(crate) signing_key: Option>, - pub(crate) db: D, - pub(crate) block_producer: B, - pub(crate) txpool: T, - pub(crate) tx_status_update_stream: BoxStream, - pub(crate) import_block_events_tx: broadcast::Sender, +/// Requests accepted by the task. +enum Request { + /// Manually produces the next blocks with `Tai64` block timestamp. + /// The block timestamp should be higher than previous one. + ManualBlocks((Vec>, oneshot::Sender>)), +} + +impl core::fmt::Debug for Request { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "Request") + } +} + +pub struct Task { + block_gas_limit: Word, + signing_key: Option>, + block_producer: B, + block_importer: I, + txpool: T, + tx_status_update_stream: BoxStream, + request_receiver: mpsc::Receiver, + shared_state: SharedState, + last_height: BlockHeight, /// Last block creation time. When starting up, this is initialized /// to `Instant::now()`, which delays the first block on startup for /// a bit, but doesn't cause any other issues. - pub(crate) last_block_created: Instant, - pub(crate) trigger: Trigger, + last_block_created: Instant, + trigger: Trigger, // TODO: Consider that the creation of the block takes some time, and maybe we need to // patch the timer to generate the block earlier. // https://github.com/FuelLabs/fuel-core/issues/918 /// Deadline clock, used by the triggers - pub(crate) timer: DeadlineClock, + timer: DeadlineClock, } -impl Task +impl Task where T: TransactionPool, { pub fn new( + last_height: BlockHeight, config: Config, txpool: T, - import_block_events_tx: broadcast::Sender, block_producer: B, - db: D, + block_importer: I, ) -> Self { let tx_status_update_stream = txpool.transaction_status_events(); + let (request_sender, request_receiver) = mpsc::channel(100); Self { block_gas_limit: config.block_gas_limit, signing_key: config.signing_key, - db, - block_producer, txpool, + block_producer, + block_importer, tx_status_update_stream, + request_receiver, + shared_state: SharedState { request_sender }, + last_height, last_block_created: Instant::now(), - import_block_events_tx, trigger: config.trigger, timer: DeadlineClock::new(), } } + + fn next_height(&self) -> BlockHeight { + self.last_height + 1u32.into() + } } -impl Task +impl Task where - D: BlockDb, T: TransactionPool, - B: BlockProducer, + B: BlockProducer, + I: BlockImporter, { // Request the block producer to make a new block, and return it when ready async fn signal_produce_block( &self, - ) -> anyhow::Result>> { - let current_height = self - .db - .block_height() - .map_err(|err| anyhow::format_err!("db error {err:?}"))?; - let height = BlockHeight::from(current_height.as_usize() + 1); - + height: BlockHeight, + block_time: Option, + ) -> anyhow::Result>> { self.block_producer - .produce_and_execute_block(height, self.block_gas_limit) + .produce_and_execute_block(height, block_time, self.block_gas_limit) .await } - pub(crate) async fn produce_block(&mut self) -> anyhow::Result<()> { + pub(crate) async fn produce_next_block(&mut self) -> anyhow::Result<()> { + self.produce_block(self.next_height(), None).await + } + + pub(crate) async fn produce_manual_blocks( + &mut self, + block_times: Vec>, + ) -> anyhow::Result<()> { + for block_time in block_times { + self.produce_block(self.next_height(), block_time).await?; + } + Ok(()) + } + + pub(crate) async fn produce_block( + &mut self, + height: BlockHeight, + block_time: Option, + ) -> anyhow::Result<()> { // verify signing key is set if self.signing_key.is_none() { return Err(anyhow!("unable to produce blocks without a consensus key")) @@ -141,14 +201,10 @@ where ExecutionResult { block, skipped_transactions, - .. + tx_status, }, - mut db_transaction, - ) = self.signal_produce_block().await?.into(); - - // sign the block and seal it - let seal = seal_block(&self.signing_key, &block, db_transaction.as_mut())?; - db_transaction.commit()?; + db_transaction, + ) = self.signal_produce_block(height, block_time).await?.into(); let mut tx_ids_to_remove = Vec::with_capacity(skipped_transactions.len()); for (tx, err) in skipped_transactions { @@ -160,17 +216,23 @@ where } self.txpool.remove_txs(tx_ids_to_remove); - // Send the block back to the txpool - // TODO: this probably must be done differently with multi-node configuration - let sealed_block = SealedBlock { + // Sign the block and seal it + let seal = seal_block(&self.signing_key, &block)?; + let block = SealedBlock { entity: block, consensus: seal, }; - self.import_block_events_tx - .send(sealed_block) - .expect("Failed to import the generated block"); + // Import the sealed block + self.block_importer.commit_result(Uncommitted::new( + ImportResult { + sealed_block: block, + tx_status, + }, + db_transaction, + ))?; // Update last block time + self.last_height = height; self.last_block_created = Instant::now(); // Set timer for the next block @@ -222,7 +284,7 @@ where let pending_number = self.txpool.pending_number(); // skip production if there are no pending transactions if pending_number > 0 { - self.produce_block().await?; + self.produce_next_block().await?; } Ok(()) } @@ -239,7 +301,7 @@ where if consumable_gas > self.block_gas_limit && self.last_block_created + min_block_time < Instant::now() { - self.produce_block().await?; + self.produce_next_block().await?; } else if self.txpool.pending_number() > 0 { // We have at least one transaction, so tx_max_idle_time is the limit self.timer @@ -271,7 +333,7 @@ where // 3. max_block_time expired // => we produce a new block in any case Trigger::Interval { .. } | Trigger::Hybrid { .. } => { - self.produce_block().await?; + self.produce_next_block().await?; Ok(()) } } @@ -279,17 +341,17 @@ where } #[async_trait::async_trait] -impl RunnableService for Task +impl RunnableService for Task where Self: RunnableTask, { const NAME: &'static str = "PoA"; - type SharedData = EmptyShared; - type Task = Task; + type SharedData = SharedState; + type Task = Task; fn shared_data(&self) -> Self::SharedData { - EmptyShared + self.shared_state.clone() } async fn into_task(self, _: &StateWatcher) -> anyhow::Result { @@ -311,11 +373,11 @@ where } #[async_trait::async_trait] -impl RunnableTask for Task +impl RunnableTask for Task where - D: BlockDb, T: TransactionPool, - B: BlockProducer, + B: BlockProducer, + I: BlockImporter, { async fn run(&mut self, watcher: &mut StateWatcher) -> anyhow::Result { let should_continue; @@ -323,6 +385,19 @@ where _ = watcher.while_started() => { should_continue = false; } + request = self.request_receiver.recv() => { + if let Some(request) = request { + match request { + Request::ManualBlocks((block_times, response)) => { + let result = self.produce_manual_blocks(block_times).await; + let _ = response.send(result); + } + } + should_continue = true; + } else { + unreachable!("The task is the holder of the `Sender` too") + } + } // TODO: This should likely be refactored to use something like tokio::sync::Notify. // Otherwise, if a bunch of txs are submitted at once and all the txs are included // into the first block production trigger, we'll still call the event handler @@ -346,31 +421,30 @@ where } } -pub fn new_service( +pub fn new_service( + last_height: BlockHeight, config: Config, txpool: T, - import_block_events_tx: broadcast::Sender, block_producer: B, - db: D, -) -> Service + block_importer: I, +) -> Service where T: TransactionPool + 'static, - D: BlockDb + 'static, - B: BlockProducer + 'static, + B: BlockProducer + 'static, + I: BlockImporter + 'static, { Service::new(Task::new( + last_height, config, txpool, - import_block_events_tx, block_producer, - db, + block_importer, )) } -pub fn seal_block( +fn seal_block( signing_key: &Option>, block: &Block, - database: &mut dyn BlockDb, ) -> anyhow::Result { if let Some(key) = signing_key { let block_hash = block.id(); @@ -381,7 +455,6 @@ pub fn seal_block( let poa_signature = Signature::sign(signing_key, &message); let seal = Consensus::PoA(PoAConsensus::new(poa_signature)); - database.seal_block(block_hash, seal.clone())?; Ok(seal) } else { Err(anyhow!("no PoA signing key configured")) diff --git a/crates/services/consensus_module/poa/src/service_test.rs b/crates/services/consensus_module/poa/src/service_test.rs index f24cc00730e..7d4c09409a8 100644 --- a/crates/services/consensus_module/poa/src/service_test.rs +++ b/crates/services/consensus_module/poa/src/service_test.rs @@ -1,10 +1,9 @@ use crate::{ - deadline_clock::DeadlineClock, new_service, ports::{ - BlockDb, - BlockProducer, - TransactionPool, + MockBlockImporter, + MockBlockProducer, + MockTransactionPool, }, service::Task, Config, @@ -12,25 +11,17 @@ use crate::{ Trigger, }; use fuel_core_services::{ - stream::{ - pending, - BoxStream, - }, + stream::pending, Service as StorageTrait, }; use fuel_core_storage::{ - transactional::{ - StorageTransaction, - Transaction as StorageTransactionTrait, - }, - Result as StorageResult, + test_helpers::EmptyStorage, + transactional::StorageTransaction, }; use fuel_core_types::{ blockchain::{ - consensus::Consensus, primitives::{ BlockHeight, - BlockId, SecretKeyWrapper, }, SealedBlock, @@ -49,7 +40,6 @@ use fuel_core_types::{ UncommittedResult, }, txpool::{ - ArcPoolTx, Error as TxPoolError, TxStatus, }, @@ -75,121 +65,95 @@ use tokio::{ watch, }, time, - time::Instant, }; mod trigger_tests; +// TODO: Add test for manual block produce +// TODO: Add testing of `block_time` passing. + struct TestContextBuilder { - mock_db: Option, - producer: Option, - txpool: Option, config: Option, + txpool: Option, + importer: Option, } impl TestContextBuilder { fn new() -> Self { Self { - mock_db: None, - producer: None, - txpool: None, config: None, + txpool: None, + importer: None, } } - fn with_txpool(&mut self, txpool: MockTxPool) -> &mut Self { + fn with_config(&mut self, config: Config) -> &mut Self { + self.config = Some(config); + self + } + + fn with_txpool(&mut self, txpool: MockTransactionPool) -> &mut Self { self.txpool = Some(txpool); self } - fn with_config(&mut self, config: Config) -> &mut Self { - self.config = Some(config); + fn with_importer(&mut self, importer: MockBlockImporter) -> &mut Self { + self.importer = Some(importer); self } fn build(self) -> TestContext { - let (block_import_tx, _) = broadcast::channel(100); let config = self.config.unwrap_or_default(); - let producer = self.producer.unwrap_or_else(|| { - let mut producer = MockBlockProducer::default(); - producer - .expect_produce_and_execute_block() - .returning(|_, _| { - let mut db = MockDatabase::default(); - db.expect_as_mut().returning(move || { - let mut tx_db = MockDatabase::default(); - tx_db.expect_seal_block().returning(|_, _| Ok(())); - tx_db - }); - db.expect_commit().returning(|| Ok(())); - - Ok(UncommittedResult::new( - ExecutionResult { - block: Default::default(), - skipped_transactions: Default::default(), - tx_status: Default::default(), - }, - StorageTransaction::new(db), - )) - }); - producer - }); - let txpool = self.txpool.unwrap_or_else(MockTxPool::no_tx_updates); - let mock_db = self.mock_db.unwrap_or_else(|| { - // default db - let mut mock_db = MockDatabase::default(); - mock_db.expect_block_height().returning(|| Ok(1u64.into())); - mock_db + let mut producer = MockBlockProducer::default(); + producer + .expect_produce_and_execute_block() + .returning(|_, _, _| { + Ok(UncommittedResult::new( + ExecutionResult { + block: Default::default(), + skipped_transactions: Default::default(), + tx_status: Default::default(), + }, + StorageTransaction::new(EmptyStorage), + )) + }); + + let importer = self.importer.unwrap_or_else(|| { + let mut importer = MockBlockImporter::default(); + importer.expect_commit_result().returning(|_| Ok(())); + importer }); + let txpool = self + .txpool + .unwrap_or_else(MockTransactionPool::no_tx_updates); + let service = - new_service(config, txpool, block_import_tx.clone(), producer, mock_db); + new_service(BlockHeight::from(1u64), config, txpool, producer, importer); service.start().unwrap(); - TestContext { - block_import_tx, - service, - } + TestContext { service } } } struct TestContext { - block_import_tx: broadcast::Sender, - service: Service, + service: Service, } impl TestContext { - fn subscribe_import(&self) -> broadcast::Receiver { - self.block_import_tx.subscribe() - } - async fn stop(&self) { - let _ = self.service.stop_and_await().await.unwrap(); + self.service.stop_and_await().await.unwrap(); } } -mockall::mock! { - TxPool {} - - impl TransactionPool for TxPool { - fn pending_number(&self) -> usize; - - fn total_consumable_gas(&self) -> u64; - - fn remove_txs(&self, tx_ids: Vec) -> Vec; - - fn transaction_status_events(&self) -> BoxStream; - } -} - -struct TxPoolContext { - pub txpool: MockTxPool, +pub struct TxPoolContext { + pub txpool: MockTransactionPool, pub txs: Arc>>, pub status_sender: Arc>>, } -impl MockTxPool { - pub fn no_tx_updates() -> Self { - let mut txpool = MockTxPool::default(); +impl MockTransactionPool { + fn no_tx_updates() -> Self { + let mut txpool = MockTransactionPool::default(); txpool .expect_transaction_status_events() .returning(|| Box::pin(pending())); @@ -197,7 +161,7 @@ impl MockTxPool { } pub fn new_with_txs(txs: Vec