From 9a4244867f8e8349b07be826bee0337b1ed15fc4 Mon Sep 17 00:00:00 2001 From: Matthias Seitz Date: Wed, 17 May 2023 11:00:55 +0200 Subject: [PATCH] refactor: cleanup Providererror (#2710) --- crates/consensus/beacon/src/engine/mod.rs | 2 +- crates/interfaces/src/provider.rs | 77 ++++++------------- crates/stages/src/pipeline/mod.rs | 4 +- crates/stages/src/stages/finish.rs | 7 +- crates/stages/src/stages/headers.rs | 12 +-- crates/stages/src/stages/total_difficulty.rs | 2 +- .../provider/src/providers/database.rs | 31 ++++---- crates/storage/provider/src/providers/mod.rs | 2 +- .../src/providers/state/historical.rs | 4 +- .../provider/src/providers/state/latest.rs | 2 +- .../storage/provider/src/test_utils/mock.rs | 2 +- crates/storage/provider/src/traits/state.rs | 6 +- crates/storage/provider/src/transaction.rs | 11 +-- 13 files changed, 64 insertions(+), 98 deletions(-) diff --git a/crates/consensus/beacon/src/engine/mod.rs b/crates/consensus/beacon/src/engine/mod.rs index 0faff51a0c..7aad32a1c8 100644 --- a/crates/consensus/beacon/src/engine/mod.rs +++ b/crates/consensus/beacon/src/engine/mod.rs @@ -470,7 +470,7 @@ where })?; let head = head.header.seal(update.head_block_hash); let head_td = self.blockchain.header_td_by_number(head.number)?.ok_or_else(|| { - Error::Provider(ProviderError::TotalDifficulty { number: head.number }) + Error::Provider(ProviderError::TotalDifficultyNotFound { number: head.number }) })?; self.sync_state_updater.update_status(Head { diff --git a/crates/interfaces/src/provider.rs b/crates/interfaces/src/provider.rs index 3caf0840be..e482819c04 100644 --- a/crates/interfaces/src/provider.rs +++ b/crates/interfaces/src/provider.rs @@ -1,31 +1,19 @@ -use reth_primitives::{Address, BlockHash, BlockNumber, TxNumber, H256}; +use reth_primitives::{Address, BlockHash, BlockHashOrNumber, BlockNumber, TxNumber, H256}; /// Bundled errors variants thrown by various providers. #[allow(missing_docs)] #[derive(Debug, thiserror::Error, PartialEq, Eq, Clone)] pub enum ProviderError { - /// The header hash is missing from the database. - #[error("Block number {block_number} does not exist in database")] - CanonicalHeader { block_number: BlockNumber }, - /// A header body is missing from the database. - #[error("No header for block #{number}")] - Header { - /// The block number key - number: BlockNumber, - }, /// The header number was not found for the given block hash. - #[error("Block hash {block_hash:?} does not exist in Headers table")] - BlockHash { block_hash: BlockHash }, + #[error("Block hash {0:?} does not exist in Headers table")] + BlockHashNotFound(BlockHash), /// A block body is missing. - #[error("Block meta not found for block #{number}")] - BlockBodyIndices { number: BlockNumber }, - /// The block transition id for a certain block number is missing. - #[error("Block transition id does not exist for block #{block_number}")] - BlockTransition { block_number: BlockNumber }, + #[error("Block meta not found for block #{0}")] + BlockBodyIndicesNotFound(BlockNumber), /// The transition id was found for the given address and storage key, but the changeset was /// not found. #[error("Storage ChangeSet address: ({address:?} key: {storage_key:?}) for block:#{block_number} does not exist")] - StorageChangeset { + StorageChangesetNotFound { /// The block number found for the address and storage key block_number: BlockNumber, /// The account address @@ -35,7 +23,7 @@ pub enum ProviderError { }, /// The block number was found for the given address, but the changeset was not found. #[error("Account {address:?} ChangeSet for block #{block_number} does not exist")] - AccountChangeset { + AccountChangesetNotFound { /// Block number found for the address block_number: BlockNumber, /// The account address @@ -43,40 +31,19 @@ pub enum ProviderError { }, /// The total difficulty for a block is missing. #[error("Total difficulty not found for block #{number}")] - TotalDifficulty { number: BlockNumber }, - /// The transaction is missing - #[error("Transaction #{id} not found")] - Transaction { - /// The transaction id - id: TxNumber, - }, - /// A ommers are missing. - #[error("Block ommers not found for block #{number}")] - Ommers { - /// The block number key - number: BlockNumber, - }, - /// There is a gap in the transaction table, at a missing transaction number. - #[error("Gap in transaction table. Missing tx number #{missing}.")] - TransactionsGap { missing: TxNumber }, - /// There is a gap in the senders table, at a missing transaction number. - #[error("Gap in transaction signer table. Missing tx number #{missing}.")] - TransactionsSignerGap { missing: TxNumber }, - /// Reached the end of the transaction table. - #[error("Got to the end of transaction table")] - EndOfTransactionTable, - /// Reached the end of the transaction sender table. - #[error("Got to the end of the transaction sender table")] - EndOfTransactionSenderTable, - /// Missing block hash in BlockchainTree - #[error("Missing block hash for block #{block_number:?} in blockchain tree")] - BlockchainTreeBlockHash { block_number: BlockNumber }, - /// Some error occurred while interacting with the state tree. - #[error("Unknown error occurred while interacting with the state trie.")] - StateTrie, + TotalDifficultyNotFound { number: BlockNumber }, /// Thrown when required header related data was not found but was required. - #[error("requested data not found")] - HeaderNotFound, + #[error("No header found for {0:?}")] + HeaderNotFound(BlockHashOrNumber), + /// Thrown we were unable to find the best block + #[error("Best block does not exist")] + BestBlockNotFound, + /// Thrown we were unable to find the finalized block + #[error("Finalized block does not exist")] + FinalizedBlockNotFound, + /// Thrown we were unable to find the safe block + #[error("Safe block does not exist")] + SafeBlockNotFound, /// Mismatch of sender and transaction #[error("Mismatch of sender and transaction id {tx_id}")] MismatchOfTransactionAndSenderId { tx_id: TxNumber }, @@ -86,12 +53,12 @@ pub enum ProviderError { /// Thrown when the cache service task dropped #[error("cache service task stopped")] CacheServiceUnavailable, - /// Thrown when the gas oracle task dropped - #[error("gas oracle task stopped")] - GasPriceOracleServiceUnavailable, /// Thrown when we failed to lookup a block for the pending state #[error("Unknown block hash: {0:}")] UnknownBlockHash(H256), + /// Thrown when we were unable to find a state for a block hash + #[error("No State found for block hash: {0:}")] + StateForHashNotFound(H256), /// Unable to compute state root on top of historical block #[error("Unable to compute state root on top of historical block")] StateRootNotAvailableForHistoricalBlock, diff --git a/crates/stages/src/pipeline/mod.rs b/crates/stages/src/pipeline/mod.rs index a2cb1df03b..d5aa4aad4b 100644 --- a/crates/stages/src/pipeline/mod.rs +++ b/crates/stages/src/pipeline/mod.rs @@ -708,14 +708,14 @@ mod tests { let db = test_utils::create_test_db::(EnvKind::RW); let mut pipeline = Pipeline::builder() .add_stage(TestStage::new(StageId("Fatal")).add_exec(Err( - StageError::DatabaseIntegrity(ProviderError::BlockBodyIndices { number: 5 }), + StageError::DatabaseIntegrity(ProviderError::BlockBodyIndicesNotFound(5)), ))) .build(db); let result = pipeline.run().await; assert_matches!( result, Err(PipelineError::Stage(StageError::DatabaseIntegrity( - ProviderError::BlockBodyIndices { number: 5 } + ProviderError::BlockBodyIndicesNotFound(5) ))) ); } diff --git a/crates/stages/src/stages/finish.rs b/crates/stages/src/stages/finish.rs index 2027c9adbc..5e6d4ee6ee 100644 --- a/crates/stages/src/stages/finish.rs +++ b/crates/stages/src/stages/finish.rs @@ -47,16 +47,11 @@ mod tests { stage_test_suite_ext!(FinishTestRunner, finish); + #[derive(Default)] struct FinishTestRunner { tx: TestTransaction, } - impl Default for FinishTestRunner { - fn default() -> Self { - FinishTestRunner { tx: TestTransaction::default() } - } - } - impl StageTestRunner for FinishTestRunner { type S = FinishStage; diff --git a/crates/stages/src/stages/headers.rs b/crates/stages/src/stages/headers.rs index 3c38985283..3381bf23cb 100644 --- a/crates/stages/src/stages/headers.rs +++ b/crates/stages/src/stages/headers.rs @@ -69,7 +69,7 @@ where let mut header_cursor = tx.cursor_read::()?; let (head_num, _) = header_cursor .seek_exact(stage_progress)? - .ok_or(ProviderError::CanonicalHeader { block_number: stage_progress })?; + .ok_or_else(|| ProviderError::HeaderNotFound(stage_progress.into()))?; // Check if the next entry is congruent Ok(header_cursor.next()?.map(|(next_num, _)| head_num + 1 == next_num).unwrap_or_default()) } @@ -89,12 +89,12 @@ where // Get head hash and reposition the cursor let (head_num, head_hash) = cursor .seek_exact(stage_progress)? - .ok_or(ProviderError::CanonicalHeader { block_number: stage_progress })?; + .ok_or_else(|| ProviderError::HeaderNotFound(stage_progress.into()))?; // Construct head let (_, head) = header_cursor .seek_exact(head_num)? - .ok_or(ProviderError::Header { number: head_num })?; + .ok_or_else(|| ProviderError::HeaderNotFound(head_num.into()))?; let local_head = head.seal(head_hash); // Look up the next header @@ -103,7 +103,7 @@ where .map(|(next_num, next_hash)| -> Result { let (_, next) = header_cursor .seek_exact(next_num)? - .ok_or(ProviderError::Header { number: next_num })?; + .ok_or_else(|| ProviderError::HeaderNotFound(next_num.into()))?; Ok(next.seal(next_hash)) }) .transpose()?; @@ -494,8 +494,8 @@ mod tests { // Empty database assert_matches!( stage.get_sync_gap(&tx, stage_progress).await, - Err(StageError::DatabaseIntegrity(ProviderError::CanonicalHeader { block_number })) - if block_number == stage_progress + Err(StageError::DatabaseIntegrity(ProviderError::HeaderNotFound(block_number))) + if block_number.as_number().unwrap() == stage_progress ); // Checkpoint and no gap diff --git a/crates/stages/src/stages/total_difficulty.rs b/crates/stages/src/stages/total_difficulty.rs index 4f3c242e10..ba701c5ff6 100644 --- a/crates/stages/src/stages/total_difficulty.rs +++ b/crates/stages/src/stages/total_difficulty.rs @@ -66,7 +66,7 @@ impl Stage for TotalDifficultyStage { let last_header_number = input.stage_progress.unwrap_or_default(); let last_entry = cursor_td .seek_exact(last_header_number)? - .ok_or(ProviderError::TotalDifficulty { number: last_header_number })?; + .ok_or(ProviderError::TotalDifficultyNotFound { number: last_header_number })?; let mut td: U256 = last_entry.1.into(); debug!(target: "sync::stages::total_difficulty", ?td, block_number = last_header_number, "Last total difficulty entry"); diff --git a/crates/storage/provider/src/providers/database.rs b/crates/storage/provider/src/providers/database.rs index 535b490b7e..2aca06848b 100644 --- a/crates/storage/provider/src/providers/database.rs +++ b/crates/storage/provider/src/providers/database.rs @@ -74,7 +74,7 @@ impl ShareableDatabase { // get block number let mut block_number = tx .get::(block_hash)? - .ok_or(ProviderError::BlockHash { block_hash })?; + .ok_or(ProviderError::BlockHashNotFound(block_hash))?; if is_latest_block_number(&tx, block_number)? { return Ok(Box::new(LatestStateProvider::new(tx))) @@ -182,7 +182,7 @@ impl BlockProvider for ShareableDatabase { let tx = self.db.tx()?; let transactions = self .transactions_by_block(id)? - .ok_or(ProviderError::BlockBodyIndices { number })?; + .ok_or(ProviderError::BlockBodyIndicesNotFound(number))?; let ommers = tx.get::(header.number)?.map(|o| o.ommers); let withdrawals = self.withdrawals_by_block(id, header.timestamp)?; @@ -421,8 +421,8 @@ impl EvmEnvProvider for ShareableDatabase { block_env: &mut BlockEnv, at: BlockHashOrNumber, ) -> Result<()> { - let hash = self.convert_number(at)?.ok_or(ProviderError::HeaderNotFound)?; - let header = self.header(&hash)?.ok_or(ProviderError::HeaderNotFound)?; + let hash = self.convert_number(at)?.ok_or(ProviderError::HeaderNotFound(at))?; + let header = self.header(&hash)?.ok_or(ProviderError::HeaderNotFound(at))?; self.fill_env_with_header(cfg, block_env, &header) } @@ -432,22 +432,24 @@ impl EvmEnvProvider for ShareableDatabase { block_env: &mut BlockEnv, header: &Header, ) -> Result<()> { - let total_difficulty = - self.header_td_by_number(header.number)?.ok_or(ProviderError::HeaderNotFound)?; + let total_difficulty = self + .header_td_by_number(header.number)? + .ok_or_else(|| ProviderError::HeaderNotFound(header.number.into()))?; fill_cfg_and_block_env(cfg, block_env, &self.chain_spec, header, total_difficulty); Ok(()) } fn fill_block_env_at(&self, block_env: &mut BlockEnv, at: BlockHashOrNumber) -> Result<()> { - let hash = self.convert_number(at)?.ok_or(ProviderError::HeaderNotFound)?; - let header = self.header(&hash)?.ok_or(ProviderError::HeaderNotFound)?; + let hash = self.convert_number(at)?.ok_or(ProviderError::HeaderNotFound(at))?; + let header = self.header(&hash)?.ok_or(ProviderError::HeaderNotFound(at))?; self.fill_block_env_with_header(block_env, &header) } fn fill_block_env_with_header(&self, block_env: &mut BlockEnv, header: &Header) -> Result<()> { - let total_difficulty = - self.header_td_by_number(header.number)?.ok_or(ProviderError::HeaderNotFound)?; + let total_difficulty = self + .header_td_by_number(header.number)? + .ok_or_else(|| ProviderError::HeaderNotFound(header.number.into()))?; let spec_id = revm_spec( &self.chain_spec, Head { @@ -465,14 +467,15 @@ impl EvmEnvProvider for ShareableDatabase { } fn fill_cfg_env_at(&self, cfg: &mut CfgEnv, at: BlockHashOrNumber) -> Result<()> { - let hash = self.convert_number(at)?.ok_or(ProviderError::HeaderNotFound)?; - let header = self.header(&hash)?.ok_or(ProviderError::HeaderNotFound)?; + let hash = self.convert_number(at)?.ok_or(ProviderError::HeaderNotFound(at))?; + let header = self.header(&hash)?.ok_or(ProviderError::HeaderNotFound(at))?; self.fill_cfg_env_with_header(cfg, &header) } fn fill_cfg_env_with_header(&self, cfg: &mut CfgEnv, header: &Header) -> Result<()> { - let total_difficulty = - self.header_td_by_number(header.number)?.ok_or(ProviderError::HeaderNotFound)?; + let total_difficulty = self + .header_td_by_number(header.number)? + .ok_or_else(|| ProviderError::HeaderNotFound(header.number.into()))?; fill_cfg_env(cfg, &self.chain_spec, header, total_difficulty); Ok(()) } diff --git a/crates/storage/provider/src/providers/mod.rs b/crates/storage/provider/src/providers/mod.rs index 29fd8bdeb6..df3a011b0b 100644 --- a/crates/storage/provider/src/providers/mod.rs +++ b/crates/storage/provider/src/providers/mod.rs @@ -69,7 +69,7 @@ where let best = database.chain_info()?; match database.header_by_number(best.best_number)? { Some(header) => Ok(Self::with_latest(database, tree, header.seal(best.best_hash))), - None => Err(Error::Provider(ProviderError::Header { number: best.best_number })), + None => Err(Error::Provider(ProviderError::HeaderNotFound(best.best_number.into()))), } } } diff --git a/crates/storage/provider/src/providers/state/historical.rs b/crates/storage/provider/src/providers/state/historical.rs index 3719313bd1..b453926a89 100644 --- a/crates/storage/provider/src/providers/state/historical.rs +++ b/crates/storage/provider/src/providers/state/historical.rs @@ -59,7 +59,7 @@ impl<'a, 'b, TX: DbTx<'a>> AccountProvider for HistoricalStateProviderRef<'a, 'b .cursor_dup_read::()? .seek_by_key_subkey(changeset_block_number, address)? .filter(|acc| acc.address == address) - .ok_or(ProviderError::AccountChangeset { + .ok_or(ProviderError::AccountChangesetNotFound { block_number: changeset_block_number, address, })?; @@ -120,7 +120,7 @@ impl<'a, 'b, TX: DbTx<'a>> StateProvider for HistoricalStateProviderRef<'a, 'b, .cursor_dup_read::()? .seek_by_key_subkey((changeset_block_number, address).into(), storage_key)? .filter(|entry| entry.key == storage_key) - .ok_or(ProviderError::StorageChangeset { + .ok_or(ProviderError::StorageChangesetNotFound { block_number: changeset_block_number, address, storage_key, diff --git a/crates/storage/provider/src/providers/state/latest.rs b/crates/storage/provider/src/providers/state/latest.rs index d9bda462b9..63d264c468 100644 --- a/crates/storage/provider/src/providers/state/latest.rs +++ b/crates/storage/provider/src/providers/state/latest.rs @@ -90,7 +90,7 @@ impl<'a, 'b, TX: DbTx<'a>> StateProvider for LatestStateProviderRef<'a, 'b, TX> .db .cursor_read::()? .last()? - .ok_or(ProviderError::Header { number: 0 })? + .ok_or_else(|| ProviderError::HeaderNotFound(0.into()))? .1 .state_root; diff --git a/crates/storage/provider/src/test_utils/mock.rs b/crates/storage/provider/src/test_utils/mock.rs index 20ec5bfbb4..59b4d9b1a8 100644 --- a/crates/storage/provider/src/test_utils/mock.rs +++ b/crates/storage/provider/src/test_utils/mock.rs @@ -252,7 +252,7 @@ impl BlockNumProvider for MockEthProvider { .iter() .max_by_key(|h| h.1.number) .map(|(_, header)| header.number) - .ok_or(ProviderError::HeaderNotFound)?) + .ok_or(ProviderError::BestBlockNotFound)?) } fn block_number(&self, hash: H256) -> Result> { diff --git a/crates/storage/provider/src/traits/state.rs b/crates/storage/provider/src/traits/state.rs index e5ed7e5fcc..e1fac70a5a 100644 --- a/crates/storage/provider/src/traits/state.rs +++ b/crates/storage/provider/src/traits/state.rs @@ -119,7 +119,7 @@ pub trait StateProviderFactory: BlockIdProvider + Send + Sync { // we can only get the finalized state by hash, not by num let hash = match self.finalized_block_hash()? { Some(hash) => hash, - None => return Err(ProviderError::HeaderNotFound.into()), + None => return Err(ProviderError::FinalizedBlockNotFound.into()), }; self.state_by_block_hash(hash) @@ -128,7 +128,7 @@ pub trait StateProviderFactory: BlockIdProvider + Send + Sync { // we can only get the safe state by hash, not by num let hash = match self.safe_block_hash()? { Some(hash) => hash, - None => return Err(ProviderError::HeaderNotFound.into()), + None => return Err(ProviderError::SafeBlockNotFound.into()), }; self.state_by_block_hash(hash) @@ -182,7 +182,7 @@ pub trait BlockchainTreePendingStateProvider: Send + Sync { ) -> Result> { Ok(self .find_pending_state_provider(block_hash) - .ok_or(ProviderError::UnknownBlockHash(block_hash))?) + .ok_or(ProviderError::StateForHashNotFound(block_hash))?) } /// Returns state provider if a matching block exists. diff --git a/crates/storage/provider/src/transaction.rs b/crates/storage/provider/src/transaction.rs index 1de5823940..ac1d8ee7d0 100644 --- a/crates/storage/provider/src/transaction.rs +++ b/crates/storage/provider/src/transaction.rs @@ -140,7 +140,7 @@ where pub fn get_block_hash(&self, block_number: BlockNumber) -> Result { let hash = self .get::(block_number)? - .ok_or(ProviderError::CanonicalHeader { block_number })?; + .ok_or_else(|| ProviderError::HeaderNotFound(block_number.into()))?; Ok(hash) } @@ -151,7 +151,7 @@ where ) -> Result { let body = self .get::(number)? - .ok_or(ProviderError::BlockBodyIndices { number })?; + .ok_or(ProviderError::BlockBodyIndicesNotFound(number))?; Ok(body) } @@ -169,8 +169,9 @@ where /// Query the block header by number pub fn get_header(&self, number: BlockNumber) -> Result { - let header = - self.get::(number)?.ok_or(ProviderError::Header { number })?; + let header = self + .get::(number)? + .ok_or_else(|| ProviderError::HeaderNotFound(number.into()))?; Ok(header) } @@ -178,7 +179,7 @@ where pub fn get_td(&self, block: BlockNumber) -> Result { let td = self .get::(block)? - .ok_or(ProviderError::TotalDifficulty { number: block })?; + .ok_or(ProviderError::TotalDifficultyNotFound { number: block })?; Ok(td.into()) }