From f577e147807a783438a3f16aad968b4396274483 Mon Sep 17 00:00:00 2001 From: Peter Davies Date: Thu, 27 Jul 2023 13:25:49 +0100 Subject: [PATCH] refactor(storage): historical state lookup (better comments) (#3867) Co-authored-by: Alexey Shekhirin Co-authored-by: Matthias Seitz --- .../src/providers/state/historical.rs | 81 ++++++++++--------- 1 file changed, 42 insertions(+), 39 deletions(-) diff --git a/crates/storage/provider/src/providers/state/historical.rs b/crates/storage/provider/src/providers/state/historical.rs index f62ab7b98c..458a4c6421 100644 --- a/crates/storage/provider/src/providers/state/historical.rs +++ b/crates/storage/provider/src/providers/state/historical.rs @@ -5,8 +5,10 @@ use crate::{ use reth_db::{ cursor::{DbCursorRO, DbDupCursorRO}, models::{storage_sharded_key::StorageShardedKey, ShardedKey}, + table::Table, tables, transaction::DbTx, + BlockNumberList, }; use reth_interfaces::Result; use reth_primitives::{ @@ -17,11 +19,11 @@ use std::marker::PhantomData; /// State provider for a given transition id which takes a tx reference. /// /// Historical state provider reads the following tables: -/// [tables::AccountHistory] -/// [tables::Bytecodes] -/// [tables::StorageHistory] -/// [tables::AccountChangeSet] -/// [tables::StorageChangeSet] +/// - [tables::AccountHistory] +/// - [tables::Bytecodes] +/// - [tables::StorageHistory] +/// - [tables::AccountChangeSet] +/// - [tables::StorageChangeSet] pub struct HistoricalStateProviderRef<'a, 'b, TX: DbTx<'a>> { /// Transaction tx: &'b TX, @@ -32,7 +34,7 @@ pub struct HistoricalStateProviderRef<'a, 'b, TX: DbTx<'a>> { } pub enum HistoryInfo { - NotWritten, + NotYetWritten, InChangeset(u64), InPlainState, } @@ -47,24 +49,7 @@ impl<'a, 'b, TX: DbTx<'a>> HistoricalStateProviderRef<'a, 'b, TX> { pub fn account_history_lookup(&self, address: Address) -> Result { // history key to search IntegerList of block number changesets. let history_key = ShardedKey::new(address, self.block_number); - let mut cursor = self.tx.cursor_read::()?; - - if let Some(chunk) = - cursor.seek(history_key)?.filter(|(key, _)| key.key == address).map(|x| x.1 .0) - { - let chunk = chunk.enable_rank(); - let rank = chunk.rank(self.block_number as usize); - if rank == 0 && !cursor.prev()?.is_some_and(|(key, _)| key.key == address) { - return Ok(HistoryInfo::NotWritten) - } - if rank < chunk.len() { - Ok(HistoryInfo::InChangeset(chunk.select(rank) as u64)) - } else { - Ok(HistoryInfo::InPlainState) - } - } else { - Ok(HistoryInfo::NotWritten) - } + self.history_info::(history_key, |key| key.key == address) } /// Lookup a storage key in the StorageHistory table @@ -75,29 +60,47 @@ impl<'a, 'b, TX: DbTx<'a>> HistoricalStateProviderRef<'a, 'b, TX> { ) -> Result { // history key to search IntegerList of block number changesets. let history_key = StorageShardedKey::new(address, storage_key, self.block_number); - let mut cursor = self.tx.cursor_read::()?; + self.history_info::(history_key, |key| { + key.address == address && key.sharded_key.key == storage_key + }) + } - if let Some(chunk) = cursor - .seek(history_key)? - .filter(|(key, _)| key.address == address && key.sharded_key.key == storage_key) - .map(|x| x.1 .0) - { + fn history_info(&self, key: K, key_filter: impl Fn(&K) -> bool) -> Result + where + T: Table, + { + let mut cursor = self.tx.cursor_read::()?; + + // Lookup the history chunk in the history index. If they key does not appear in the + // index, the first chunk for the next key will be returned so we filter out chunks that + // have a different key. + if let Some(chunk) = cursor.seek(key)?.filter(|(key, _)| key_filter(key)).map(|x| x.1 .0) { let chunk = chunk.enable_rank(); + + // Get the rank of the first entry after our block. let rank = chunk.rank(self.block_number as usize); - if rank == 0 && - !cursor.prev()?.is_some_and(|(key, _)| { - key.address == address && key.sharded_key.key == storage_key - }) - { - return Ok(HistoryInfo::NotWritten) + + // If our block is before the first entry in the index chunk, it might be before + // the first write ever. To check, we look at the previous entry and check if the + // key is the same. + // This check is worth it, the `cursor.prev()` check is rarely triggered (the if will + // short-circuit) and when it passes we save a full seek into the changeset/plain state + // table. + if rank == 0 && !cursor.prev()?.is_some_and(|(key, _)| key_filter(&key)) { + // The key is written to, but only after our block. + return Ok(HistoryInfo::NotYetWritten) } if rank < chunk.len() { + // The chunk contains an entry for a write after our block, return it. Ok(HistoryInfo::InChangeset(chunk.select(rank) as u64)) } else { + // The chunk does not contain an entry for a write after our block. This can only + // happen if this is the last chunk and so we need to look in the plain state. Ok(HistoryInfo::InPlainState) } } else { - Ok(HistoryInfo::NotWritten) + // The key has not been written to at all. + Ok(HistoryInfo::NotYetWritten) } } } @@ -106,7 +109,7 @@ impl<'a, 'b, TX: DbTx<'a>> AccountReader for HistoricalStateProviderRef<'a, 'b, /// Get basic account information. fn basic_account(&self, address: Address) -> Result> { match self.account_history_lookup(address)? { - HistoryInfo::NotWritten => Ok(None), + HistoryInfo::NotYetWritten => Ok(None), HistoryInfo::InChangeset(changeset_block_number) => Ok(self .tx .cursor_dup_read::()? @@ -152,7 +155,7 @@ impl<'a, 'b, TX: DbTx<'a>> StateProvider for HistoricalStateProviderRef<'a, 'b, /// Get storage. fn storage(&self, address: Address, storage_key: StorageKey) -> Result> { match self.storage_history_lookup(address, storage_key)? { - HistoryInfo::NotWritten => Ok(None), + HistoryInfo::NotYetWritten => Ok(None), HistoryInfo::InChangeset(changeset_block_number) => Ok(Some( self.tx .cursor_dup_read::()?