feat(storage): add AccountsHistory RocksDB consistency check #20594
Conversation
Add consistency checking for the AccountsHistory table in RocksDB, following the same pattern as StoragesHistory. Changes: - Add check_accounts_history() call in check_consistency() - Add check_accounts_history() and prune_accounts_history_above() functions - Add comprehensive tests for all edge cases
yongkangc
requested review from
joshieDo,
rakita and
shekhirin
as code owners
There was a problem hiding this comment.
Pull request overview
This PR adds a missing RocksDB consistency check for the AccountsHistory table, completing the consistency check coverage alongside the existing TransactionHashNumbers and StoragesHistory checks.
- Implements
check_accounts_history()method following the established pattern for history table invariant checking - Adds
prune_accounts_history_above()helper for healing inconsistencies when RocksDB is ahead of checkpoints - Integrates the new check into the main
check_consistency()flow with proper conditional execution
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// TODO(<https://github.com/paradigmxyz/reth/issues/20417>): this iterates the whole table, | ||
| /// which is inefficient. Use changeset-based pruning instead. | ||
| fn prune_accounts_history_above(&self, max_block: BlockNumber) -> ProviderResult<()> { |
There was a problem hiding this comment.
I see, so rn this requires scanning the entire table?
There was a problem hiding this comment.
yeah we need changesets in SF to redo this, kinda as a placeholder here + testing asap other components ig
… consistency check StoragesHistory was missing the case where RocksDB data is behind the checkpoint, which should trigger an unwind. This aligns with the AccountsHistory implementation.
joshieDo
left a comment
joshieDo
left a comment
There was a problem hiding this comment.
feel like we could dedup storage/account history here, but we can do it another time
|
|
||
| // Find the max highest_block_number (excluding u64::MAX sentinel) across all | ||
| // entries | ||
| let mut max_highest_block = 0u64; |
There was a problem hiding this comment.
i feel like there was already some kind of rocksdb checkpoint block somewhere?
closes #20595
RocksDB consistency checks only cover TransactionHashNumbers and StoragesHistory, but AccountsHistory was missing.
Add
check_accounts_history()invariant check following the same pattern asStoragesHistory.
Related