diff --git a/Cargo.lock b/Cargo.lock index 1899a30903..9f4ebe5b8c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3667,7 +3667,7 @@ dependencies = [ name = "fuel-core-global-merkle-root-storage" version = "0.41.7" dependencies = [ - "anyhow", + "derive_more 0.99.19", "enum-iterator", "fuel-core-global-merkle-root-storage", "fuel-core-storage", diff --git a/crates/proof_system/global_merkle_root/service/src/service.rs b/crates/proof_system/global_merkle_root/service/src/service.rs index 4d6550831b..35ee891e73 100644 --- a/crates/proof_system/global_merkle_root/service/src/service.rs +++ b/crates/proof_system/global_merkle_root/service/src/service.rs @@ -20,13 +20,13 @@ use crate::ports::{ pub struct Service(ServiceRunner>) where B: BlockStream + Send + 'static, - B::Error: std::error::Error + Send + Sync + 'static, + B::Error: core::error::Error + Send + Sync + 'static, S: ServiceStorage + Send + 'static; impl Service where B: BlockStream + Send + 'static, - B::Error: std::error::Error + Send + Sync + 'static, + B::Error: core::error::Error + Send + Sync + 'static, S: ServiceStorage + Send + 'static, { /// Construct a new service. @@ -40,7 +40,7 @@ where impl Deref for Service where B: BlockStream + Send + 'static, - B::Error: std::error::Error + Send + Sync + 'static, + B::Error: core::error::Error + Send + Sync + 'static, S: ServiceStorage + Send + 'static, { type Target = ServiceRunner>; @@ -72,7 +72,7 @@ impl UpdateMerkleRootTask { impl RunnableService for UpdateMerkleRootTask where B: BlockStream + Send, - B::Error: std::error::Error + Send + Sync + 'static, + B::Error: core::error::Error + Send + Sync + 'static, S: ServiceStorage + Send, { const NAME: &'static str = "MerkleRootService"; @@ -97,7 +97,7 @@ where impl RunnableTask for UpdateMerkleRootTask where B: BlockStream + Send, - B::Error: std::error::Error + Send + Sync + 'static, + B::Error: core::error::Error + Send + Sync + 'static, S: ServiceStorage + Send, { #[tracing::instrument(skip(self, watcher))] @@ -121,7 +121,7 @@ where impl UpdateMerkleRootTask where B: BlockStream, - B::Error: std::error::Error + Send + Sync + 'static, + B::Error: core::error::Error + Send + Sync + 'static, S: ServiceStorage, { #[tracing::instrument(skip(self))] diff --git a/crates/proof_system/global_merkle_root/storage/Cargo.toml b/crates/proof_system/global_merkle_root/storage/Cargo.toml index aec5c65785..32195dba13 100644 --- a/crates/proof_system/global_merkle_root/storage/Cargo.toml +++ b/crates/proof_system/global_merkle_root/storage/Cargo.toml @@ -11,7 +11,7 @@ repository = { workspace = true } version = { workspace = true } [dependencies] -anyhow = { workspace = true } +derive_more = { workspace = true } enum-iterator = { workspace = true } fuel-core-storage = { workspace = true, features = ["alloc"] } fuel-core-types = { workspace = true, default-features = false, features = [ @@ -43,7 +43,7 @@ rand = { workspace = true } [features] std = ["fuel-core-storage/std", "fuel-core-types/std"] -test-helpers = ["dep:rand"] +test-helpers = ["dep:rand", "std"] fault-proving = [ "fuel-core-types/fault-proving", "fuel-core-storage/fault-proving", diff --git a/crates/proof_system/global_merkle_root/storage/src/error.rs b/crates/proof_system/global_merkle_root/storage/src/error.rs new file mode 100644 index 0000000000..2dd8fb2877 --- /dev/null +++ b/crates/proof_system/global_merkle_root/storage/src/error.rs @@ -0,0 +1,71 @@ +use fuel_core_types::fuel_tx::{ + self, + TxId, + UtxoId, +}; + +/// Errors that can occur in the merkleized storage. +#[derive(Debug, derive_more::Display, derive_more::From)] +#[non_exhaustive] +pub enum Error { + /// Errors indicating that the block is invalid. + #[from] + InvalidBlock(InvalidBlock), + /// Errors originating from storage operations. + #[from] + Storage(fuel_core_storage::Error), + /// Errors that can only occur if there is a bug in the code. + #[from] + InvariantViolation(InvariantViolation), +} + +/// Error indicating that a block is invalid +#[derive(Debug, derive_more::Display, derive_more::From)] +#[non_exhaustive] +pub enum InvalidBlock { + /// The block has too many transactions. + TooManyTransactions, + /// A transaction has too many outputs. + TooManyOutputs, + /// An output contains an invalid contract input index. + InvalidContractInputIndex(u16), + /// A transaction tries to increment the consensus parameter version beyond the supported range. + ConsensusParametersVersionOutOfBounds, + /// A transaction tries to increment the state transition bytecode version beyond the supported range. + StateTransitionBytecodeVersionOutOfBounds, + /// A transaction already exists. + DuplicateTransaction(TxId), + /// A transaction is missing a referenced witness. + MissingWitness(usize), + /// A create transaction is missing a contract created output. + ContractCreatedOutputNotFound, + /// An upload transaction tries to upload code that has already been completely uploaded. + BytecodeAlreadyCompleted, + /// An upload transaction tries to upload the wrong subsection. + #[display(fmt = "wrong subsection index: expected {expected}, got {observed}")] + WrongSubsectionIndex { + /// The expected subsection. + expected: u16, + /// The observed subsection. + observed: u16, + }, + /// An upload transaction tries to upload a subsection index outside of the supported range. + SubsectionNumberOverflow, + /// A transaction doesn't satisfy the transaction validity rules + #[from] + TxValidityError(fuel_tx::ValidityError), +} + +/// Error indicating that there is a bug in the code. +#[derive(Debug, derive_more::Display, derive_more::From)] +#[non_exhaustive] +pub enum InvariantViolation { + /// We try to store a coin that already exists. + CoinAlreadyExists(UtxoId), +} + +impl core::error::Error for Error {} +impl core::error::Error for InvalidBlock {} + +/// Convenience alias for merkleized storage results +pub type Result = core::result::Result; diff --git a/crates/proof_system/global_merkle_root/storage/src/lib.rs b/crates/proof_system/global_merkle_root/storage/src/lib.rs index 8e76ab6561..6072c6c57e 100644 --- a/crates/proof_system/global_merkle_root/storage/src/lib.rs +++ b/crates/proof_system/global_merkle_root/storage/src/lib.rs @@ -21,6 +21,14 @@ pub mod structured_storage; /// Update logic pub mod update; +/// Error type +pub mod error; + +pub use error::{ + Error, + Result, +}; + /// Test helpers #[cfg(feature = "test-helpers")] pub mod test_helpers; diff --git a/crates/proof_system/global_merkle_root/storage/src/update.rs b/crates/proof_system/global_merkle_root/storage/src/update.rs index 1e2724cd06..a8e61893e3 100644 --- a/crates/proof_system/global_merkle_root/storage/src/update.rs +++ b/crates/proof_system/global_merkle_root/storage/src/update.rs @@ -1,5 +1,9 @@ use crate::{ column::Column, + error::{ + InvalidBlock, + InvariantViolation, + }, Blobs, Coins, ConsensusParametersVersions, @@ -15,7 +19,6 @@ use alloc::{ vec, vec::Vec, }; -use anyhow::anyhow; use fuel_core_storage::{ kv_store::KeyValueInspect, transactional::StorageTransaction, @@ -83,10 +86,6 @@ use fuel_core_types::{ ChainId, }, fuel_vm::UploadedBytecode, - services::executor::{ - Error as ExecutorError, - TransactionValidityError, - }, }; /// Main entrypoint to the update functionality @@ -96,7 +95,7 @@ pub trait UpdateMerkleizedTables { &mut self, chain_id: ChainId, block: &Block, - ) -> anyhow::Result<&mut Self>; + ) -> crate::Result<&mut Self>; } impl UpdateMerkleizedTables for StorageTransaction @@ -108,7 +107,7 @@ where &mut self, chain_id: ChainId, block: &Block, - ) -> anyhow::Result<&mut Self> { + ) -> crate::Result<&mut Self> { let mut update_transaction = UpdateMerkleizedTablesTransaction { chain_id, storage: self, @@ -137,14 +136,13 @@ impl<'a, Storage> UpdateMerkleizedTablesTransaction<'a, Storage> where Storage: KeyValueInspect, { - // TODO(#2588): Proper result type #[tracing::instrument(skip(self, block))] - pub fn process_block(&mut self, block: &Block) -> anyhow::Result<()> { + pub fn process_block(&mut self, block: &Block) -> crate::Result<()> { let block_height = *block.header().height(); for (tx_idx, tx) in block.transactions().iter().enumerate() { let tx_idx: u16 = - u16::try_from(tx_idx).map_err(|_| ExecutorError::TooManyTransactions)?; + u16::try_from(tx_idx).map_err(|_| InvalidBlock::TooManyTransactions)?; self.process_transaction(block_height, tx_idx, tx)?; } @@ -157,7 +155,7 @@ where block_height: BlockHeight, tx_idx: u16, tx: &Transaction, - ) -> anyhow::Result<()> { + ) -> crate::Result<()> { let tx_id = tx.id(&self.chain_id); let inputs = tx.inputs(); @@ -168,7 +166,7 @@ where let tx_pointer = TxPointer::new(block_height, tx_idx); for (output_index, output) in tx.outputs().iter().enumerate() { let output_index = - u16::try_from(output_index).map_err(|_| ExecutorError::TooManyOutputs)?; + u16::try_from(output_index).map_err(|_| InvalidBlock::TooManyOutputs)?; let utxo_id = UtxoId::new(tx_id, output_index); self.process_output(tx_pointer, utxo_id, &inputs, output)?; @@ -188,7 +186,7 @@ where } #[tracing::instrument(skip(self, input))] - fn process_input(&mut self, input: &Input) -> anyhow::Result<()> { + fn process_input(&mut self, input: &Input) -> crate::Result<()> { match input { Input::CoinSigned(CoinSigned { utxo_id, .. }) | Input::CoinPredicate(CoinPredicate { utxo_id, .. }) => { @@ -228,7 +226,7 @@ where utxo_id: UtxoId, inputs: &[Input], output: &Output, - ) -> anyhow::Result<()> { + ) -> crate::Result<()> { match output { Output::Coin { to, @@ -266,7 +264,7 @@ where } #[tracing::instrument(skip(self))] - fn store_processed_transaction(&mut self, tx_id: TxId) -> anyhow::Result<()> { + fn store_processed_transaction(&mut self, tx_id: TxId) -> crate::Result<()> { tracing::debug!("storing processed transaction"); let previous_tx = self .storage @@ -274,7 +272,7 @@ where .replace(&tx_id, &())?; if previous_tx.is_some() { - anyhow::bail!("duplicate transaction detected") + Err(InvalidBlock::DuplicateTransaction(tx_id))? }; Ok(()) @@ -288,7 +286,7 @@ where owner: Address, amount: Word, asset_id: AssetId, - ) -> anyhow::Result<()> { + ) -> crate::Result<()> { // Only insert a coin output if it has some amount. // This is because variable or transfer outputs won't have any value // if there's a revert or panic and shouldn't be added to the utxo set. @@ -302,12 +300,10 @@ where .into(); tracing::debug!("storing coin"); - let previous_coin = - self.storage.storage::().replace(&utxo_id, &coin)?; - - // We should never overwrite coins. - // TODO(#2588): Return error instead. - assert!(previous_coin.is_none()); + let None = self.storage.storage::().replace(&utxo_id, &coin)? else { + // We should never overwrite coins. + return Err(InvariantViolation::CoinAlreadyExists(utxo_id).into()) + }; } Ok(()) @@ -320,7 +316,7 @@ where utxo_id: UtxoId, inputs: &[Input], contract: output::contract::Contract, - ) -> anyhow::Result<()> { + ) -> crate::Result<()> { if let Some(Input::Contract(input::contract::Contract { contract_id, .. })) = inputs.get(contract.input_index as usize) { @@ -331,20 +327,22 @@ where )?; } else { tracing::warn!("invalid contract input index"); - Err(ExecutorError::TransactionValidity( - TransactionValidityError::InvalidContractInputIndex(utxo_id), + Err(InvalidBlock::InvalidContractInputIndex( + contract.input_index, ))?; } Ok(()) } #[tracing::instrument(skip(self, tx))] - fn process_create_transaction(&mut self, tx: &Create) -> anyhow::Result<()> { + fn process_create_transaction(&mut self, tx: &Create) -> crate::Result<()> { let bytecode_witness_index = tx.bytecode_witness_index(); let witnesses = tx.witnesses(); let bytecode = witnesses .get(usize::from(*bytecode_witness_index)) - .ok_or_else(|| anyhow!("invalid witness index {bytecode_witness_index}"))? + .ok_or_else(|| { + InvalidBlock::MissingWitness(usize::from(*bytecode_witness_index)) + })? .as_vec(); // The Fuel specs mandate that each create transaction has exactly one output of type `Output::ContractCreated`. @@ -354,7 +352,7 @@ where .iter() .find(|output| matches!(output, Output::ContractCreated { .. })) else { - anyhow::bail!("Create transaction does not have contract created output") + Err(InvalidBlock::ContractCreatedOutputNotFound)? }; tracing::debug!("storing contract code"); @@ -365,10 +363,12 @@ where } #[tracing::instrument(skip(self, tx))] - fn process_upgrade_transaction(&mut self, tx: &Upgrade) -> anyhow::Result<()> { + fn process_upgrade_transaction(&mut self, tx: &Upgrade) -> crate::Result<()> { let metadata = match tx.metadata() { Some(metadata) => metadata.body.clone(), - None => UpgradeMetadata::compute(tx).map_err(|e| anyhow::anyhow!(e))?, + None => { + UpgradeMetadata::compute(tx).map_err(InvalidBlock::TxValidityError)? + } }; match metadata { @@ -379,7 +379,7 @@ where let Some(next_consensus_parameters_version) = self.latest_consensus_parameters_version.checked_add(1) else { - return Err(anyhow::anyhow!("Invalid consensus parameters version")); + return Err(InvalidBlock::ConsensusParametersVersionOutOfBounds.into()) }; self.latest_consensus_parameters_version = next_consensus_parameters_version; @@ -400,9 +400,7 @@ where let Some(next_state_transition_bytecode_version) = self.latest_state_transition_bytecode_version.checked_add(1) else { - return Err(anyhow::anyhow!( - "Invalid state transition bytecode version" - )); + return Err(InvalidBlock::StateTransitionBytecodeVersionOutOfBounds.into()); }; self.latest_state_transition_bytecode_version = next_state_transition_bytecode_version; @@ -419,7 +417,7 @@ where } #[tracing::instrument(skip(self, tx))] - fn process_upload_transaction(&mut self, tx: &Upload) -> anyhow::Result<()> { + fn process_upload_transaction(&mut self, tx: &Upload) -> crate::Result<()> { let bytecode_root = *tx.bytecode_root(); let uploaded_bytecode = self .storage @@ -436,26 +434,30 @@ where uploaded_subsections_number, } = uploaded_bytecode else { - anyhow::bail!("expected uncompleted bytecode") + Err(InvalidBlock::BytecodeAlreadyCompleted)? }; let expected_subsection_index = uploaded_subsections_number; - if expected_subsection_index != tx.body().subsection_index { - anyhow::bail!("expected subsection index {expected_subsection_index}"); + let subsection_index = tx.body().subsection_index; + + if expected_subsection_index != subsection_index { + Err(InvalidBlock::WrongSubsectionIndex { + expected: expected_subsection_index, + observed: subsection_index, + })? }; let witness_index = usize::from(*tx.bytecode_witness_index()); let new_bytecode = tx .witnesses() .get(witness_index) - .ok_or_else(|| anyhow!("expected witness with bytecode"))?; + .ok_or_else(|| InvalidBlock::MissingWitness(witness_index))?; bytecode.extend_from_slice(new_bytecode.as_ref()); - let new_uploaded_subsections_number = - uploaded_subsections_number.checked_add(1).ok_or_else(|| { - anyhow!("overflow when incrementing uploaded subsection number") - })?; + let new_uploaded_subsections_number = uploaded_subsections_number + .checked_add(1) + .ok_or_else(|| InvalidBlock::SubsectionNumberOverflow)?; let new_uploaded_bytecode = if new_uploaded_subsections_number == tx.body().subsections_number { @@ -476,17 +478,17 @@ where } #[tracing::instrument(skip(self, tx))] - fn process_blob_transaction(&mut self, tx: &Blob) -> anyhow::Result<()> { + fn process_blob_transaction(&mut self, tx: &Blob) -> crate::Result<()> { let BlobBody { id: blob_id, witness_index, } = tx.body(); + let witness_index = usize::from(*witness_index); let blob = tx .witnesses() - .get(usize::from(*witness_index)) - // TODO(#2588): Proper error type - .ok_or_else(|| anyhow!("transaction should have blob payload"))?; + .get(witness_index) + .ok_or_else(|| InvalidBlock::MissingWitness(witness_index))?; tracing::debug!("storing blob"); self.storage