Skip to content

Commit

Permalink
feat: remove PartialEq+Eq from ProviderError and all others affected (#…
Browse files Browse the repository at this point in the history
…13592)

Co-authored-by: Matthias Seitz <[email protected]>
  • Loading branch information
tnv1 and mattsse authored Jan 4, 2025
1 parent 91d09de commit 86399e2
Show file tree
Hide file tree
Showing 12 changed files with 129 additions and 135 deletions.
4 changes: 2 additions & 2 deletions crates/blockchain-tree-api/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub enum BlockchainTreeError {
}

/// Canonical Errors
#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
#[derive(thiserror::Error, Debug, Clone)]
pub enum CanonicalError {
/// Error originating from validation operations.
#[error(transparent)]
Expand Down Expand Up @@ -167,7 +167,7 @@ impl std::fmt::Debug for InsertBlockError {
}

#[derive(thiserror::Error, Debug)]
#[error("Failed to insert block (hash={}, number={}, parent_hash={}): {kind}",
#[error("Failed to insert block (hash={}, number={}, parent_hash={}): {kind}",
.block.hash(),
.block.number,
.block.parent_hash)]
Expand Down
8 changes: 4 additions & 4 deletions crates/ethereum/evm/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -421,10 +421,10 @@ mod tests {
"Executing cancun block without parent beacon block root field should fail",
);

assert_eq!(
assert!(matches!(
err.as_validation().unwrap().clone(),
BlockValidationError::MissingParentBeaconBlockRoot
);
));

// fix header, set a gas limit
header.parent_beacon_block_root = Some(B256::with_last_byte(0x69));
Expand Down Expand Up @@ -1098,13 +1098,13 @@ mod tests {
// Check if the execution result is an error and assert the specific error type
match exec_result {
Ok(_) => panic!("Expected block gas limit error"),
Err(err) => assert_eq!(
Err(err) => assert!(matches!(
*err.as_validation().unwrap(),
BlockValidationError::TransactionGasLimitMoreThanAvailableBlockGas {
transaction_gas_limit: 2_500_000,
block_available_gas: 1_500_000,
}
),
)),
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/evm/execution-errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub mod trie;
pub use trie::*;

/// Transaction validation errors
#[derive(Error, PartialEq, Eq, Clone, Debug)]
#[derive(Error, Clone, Debug)]
pub enum BlockValidationError {
/// EVM error with transaction hash and message
#[error("EVM reported invalid transaction ({hash}): {error}")]
Expand Down
2 changes: 1 addition & 1 deletion crates/net/p2p/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ impl From<oneshot::error::RecvError> for RequestError {
pub type DownloadResult<T> = Result<T, DownloadError>;

/// The downloader error type
#[derive(Debug, Clone, PartialEq, Eq, Display, Error)]
#[derive(Debug, Clone, Display, Error)]
pub enum DownloadError {
/* ==================== HEADER ERRORS ==================== */
/// Header validation failed.
Expand Down
58 changes: 20 additions & 38 deletions crates/stages/stages/src/stages/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,9 @@ mod tests {
previous_checkpoint,
);

assert_eq!(stage_checkpoint, Ok(previous_stage_checkpoint));
assert!(
matches!(stage_checkpoint, Ok(checkpoint) if checkpoint == previous_stage_checkpoint)
);
}

#[test]
Expand Down Expand Up @@ -938,28 +940,21 @@ mod tests {
};

// assert accounts
assert_eq!(
provider.basic_account(&account1),
Ok(Some(account1_info)),
"Post changed of a account"
assert!(
matches!(provider.basic_account(&account1), Ok(Some(acc)) if acc == account1_info)
);
assert_eq!(
provider.basic_account(&account2),
Ok(Some(account2_info)),
"Post changed of a account"
assert!(
matches!(provider.basic_account(&account2), Ok(Some(acc)) if acc == account2_info)
);
assert_eq!(
provider.basic_account(&account3),
Ok(Some(account3_info)),
"Post changed of a account"
assert!(
matches!(provider.basic_account(&account3), Ok(Some(acc)) if acc == account3_info)
);
// assert storage
// Get on dupsort would return only first value. This is good enough for this test.
assert_eq!(
assert!(matches!(
provider.tx_ref().get::<tables::PlainStorageState>(account1),
Ok(Some(StorageEntry { key: B256::with_last_byte(1), value: U256::from(2) })),
"Post changed of a account"
);
Ok(Some(entry)) if entry.key == B256::with_last_byte(1) && entry.value == U256::from(2)
));

let mut provider = factory.database_provider_rw().unwrap();
let mut stage = stage();
Expand Down Expand Up @@ -1074,25 +1069,13 @@ mod tests {
} if total == block.gas_used);

// assert unwind stage
assert_eq!(
provider.basic_account(&acc1),
Ok(Some(acc1_info)),
"Pre changed of a account"
);
assert_eq!(
provider.basic_account(&acc2),
Ok(Some(acc2_info)),
"Post changed of a account"
);
assert!(matches!(provider.basic_account(&acc1), Ok(Some(acc)) if acc == acc1_info));
assert!(matches!(provider.basic_account(&acc2), Ok(Some(acc)) if acc == acc2_info));

let miner_acc = address!("2adc25665018aa1fe0e6bc666dac8fc2697ff9ba");
assert_eq!(
provider.basic_account(&miner_acc),
Ok(None),
"Third account should be unwound"
);
assert!(matches!(provider.basic_account(&miner_acc), Ok(None)));

assert_eq!(provider.receipt(0), Ok(None), "First receipt should be unwound");
assert!(matches!(provider.receipt(0), Ok(None)));
}
}

Expand Down Expand Up @@ -1173,13 +1156,12 @@ mod tests {

// assert unwind stage
let provider = test_db.factory.database_provider_rw().unwrap();
assert_eq!(provider.basic_account(&destroyed_address), Ok(None), "Account was destroyed");
assert!(matches!(provider.basic_account(&destroyed_address), Ok(None)));

assert_eq!(
assert!(matches!(
provider.tx_ref().get::<tables::PlainStorageState>(destroyed_address),
Ok(None),
"There is storage for destroyed account"
);
Ok(None)
));
// drops tx so that it returns write privilege to test_tx
drop(provider);
let plain_accounts = test_db.table::<tables::PlainAccountState>().unwrap();
Expand Down
22 changes: 11 additions & 11 deletions crates/stages/stages/src/stages/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,11 @@ mod tests {
// writer for the first time.
let mut static_file_provider = db.factory.static_file_provider();
static_file_provider = StaticFileProvider::read_write(static_file_provider.path()).unwrap();
assert_eq!(
assert!(matches!(
static_file_provider
.check_consistency(&db.factory.database_provider_ro().unwrap(), is_full_node,),
Ok(expected)
);
Ok(e) if e == expected
));
}

/// Saves a checkpoint with `checkpoint_block_number` and compare the check consistency result
Expand All @@ -338,12 +338,12 @@ mod tests {
.unwrap();
provider_rw.commit().unwrap();

assert_eq!(
assert!(matches!(
db.factory
.static_file_provider()
.check_consistency(&db.factory.database_provider_ro().unwrap(), false,),
Ok(expected)
);
Ok(e) if e == expected
));
}

/// Inserts a dummy value at key and compare the check consistency result against the expected
Expand All @@ -360,23 +360,23 @@ mod tests {
cursor.insert(key, Default::default()).unwrap();
provider_rw.commit().unwrap();

assert_eq!(
assert!(matches!(
db.factory
.static_file_provider()
.check_consistency(&db.factory.database_provider_ro().unwrap(), false),
Ok(expected)
);
Ok(e) if e == expected
));
}

#[test]
fn test_consistency() {
let db = seed_data(90).unwrap();
let db_provider = db.factory.database_provider_ro().unwrap();

assert_eq!(
assert!(matches!(
db.factory.static_file_provider().check_consistency(&db_provider, false),
Ok(None)
);
));
}

#[test]
Expand Down
6 changes: 3 additions & 3 deletions crates/storage/db-common/src/init.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ pub const AVERAGE_COUNT_ACCOUNTS_PER_GB_STATE_DUMP: usize = 285_228;
const SOFT_LIMIT_COUNT_FLUSHED_UPDATES: usize = 1_000_000;

/// Storage initialization error type.
#[derive(Debug, thiserror::Error, PartialEq, Eq, Clone)]
#[derive(Debug, thiserror::Error, Clone)]
pub enum InitStorageError {
/// Genesis header found on static files but the database is empty.
#[error("static files found, but the database is uninitialized. If attempting to re-syncing, delete both.")]
Expand Down Expand Up @@ -688,13 +688,13 @@ mod tests {
static_file_provider,
));

assert_eq!(
assert!(matches!(
genesis_hash.unwrap_err(),
InitStorageError::GenesisHashMismatch {
chainspec_hash: MAINNET_GENESIS_HASH,
storage_hash: SEPOLIA_GENESIS_HASH
}
)
))
}

#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/errors/src/provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use reth_static_file_types::StaticFileSegment;
pub type ProviderResult<Ok> = Result<Ok, ProviderError>;

/// Bundled errors variants thrown by various providers.
#[derive(Clone, Debug, PartialEq, Eq, thiserror::Error)]
#[derive(Clone, Debug, thiserror::Error)]
pub enum ProviderError {
/// Database error.
#[error(transparent)]
Expand Down
8 changes: 4 additions & 4 deletions crates/storage/provider/src/providers/blockchain_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2740,14 +2740,14 @@ mod tests {

// Even though the block exists, given the order of provider queries done in the method
// above, we do not see it.
assert_eq!(
assert!(matches!(
old_transaction_hash_fn(
to_be_persisted_tx.hash(),
provider.canonical_in_memory_state(),
provider.database.clone()
),
Ok(None)
);
));
}

// CORRECT BEHAVIOUR
Expand All @@ -2757,14 +2757,14 @@ mod tests {
persist_block_after_db_tx_creation(provider.clone(), in_memory_blocks[1].number);
let to_be_persisted_tx = in_memory_blocks[1].body().transactions[0].clone();

assert_eq!(
assert!(matches!(
correct_transaction_hash_fn(
to_be_persisted_tx.hash(),
provider.canonical_in_memory_state(),
provider.database
),
Ok(Some(to_be_persisted_tx))
);
));
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/provider/src/providers/database/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -778,7 +778,7 @@ mod tests {
);

let db_senders = provider.senders_by_tx_range(range);
assert_eq!(db_senders, Ok(vec![]));
assert!(matches!(db_senders, Ok(ref v) if v.is_empty()));
}
}

Expand Down
Loading

0 comments on commit 86399e2

Please sign in to comment.