fix: address Joshi's review comments

- Revert 'allow RocksDB unwind to 0' - keep panic on unwind to 0 as it would
  leave MDBX with huge free list size
- Remove checked_add overflow protection (not needed in practice)
- Created issue #20506 for refactoring StaticFileProvider::check_consistency

Amp-Thread-ID: https://ampcode.com/threads/T-019b3076-f7ed-700d-8916-779712facb8e
This commit is contained in:
yongkangc
2025-12-19 01:45:42 +00:00
parent d358d62353
commit 5dc4b5248e
2 changed files with 8 additions and 26 deletions

View File

@@ -529,32 +529,19 @@ where
};
if let Some(unwind_block) = unwind_target {
// Highly unlikely to happen, and given its destructive nature, it's better to panic
// instead. Unwinding to 0 would leave MDBX with a huge free list size.
let inconsistency_source = match (static_file_unwind, rocksdb_unwind) {
(Some(_), Some(_)) => "static file <> database and RocksDB <> database",
(Some(_), None) => "static file <> database",
(None, Some(_)) => "RocksDB <> database",
(None, None) => unreachable!(),
};
// Unwind to block 0 from static file inconsistency is destructive and likely
// indicates corruption - panic to prevent data loss.
// However, RocksDB returning unwind to 0 is expected when migrating an existing
// node to use RocksDB flags - allow this case to proceed with a full resync.
if unwind_block == 0 && static_file_unwind == Some(0) {
panic!(
"A {inconsistency_source} inconsistency was found that requires unwind to \
block 0. This likely indicates database corruption."
);
}
if unwind_block == 0 {
warn!(
target: "reth::cli",
%inconsistency_source,
"Consistency check requires unwind to block 0. This is expected when \
migrating an existing node to use RocksDB. A full resync will be performed."
);
}
assert_ne!(
unwind_block,
0,
"A {inconsistency_source} inconsistency was found that would trigger an unwind to block 0"
);
let unwind_target = PipelineTarget::Unwind(unwind_block);

View File

@@ -127,12 +127,7 @@ impl RocksDBProvider {
highest_static_tx = highest_tx,
"Static files ahead of MDBX, pruning TransactionHashNumbers excess data"
);
// Use checked_add to prevent overflow when mdbx_tx == u64::MAX
if let Some(start) = mdbx_tx.checked_add(1) {
if start <= highest_tx {
self.prune_transaction_hash_numbers_in_range(provider, start..=highest_tx)?;
}
}
self.prune_transaction_hash_numbers_in_range(provider, (mdbx_tx + 1)..=highest_tx)?;
// After pruning, check if MDBX is behind checkpoint
if checkpoint > mdbx_block {