feat: index account/storage history #978
Conversation
rakita
force-pushed
the
rakita/history_indices
branch
from
7fd6b99 to
c1931ba
Compare
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #978 +/- ##
==========================================
+ Coverage 74.36% 74.88% +0.52%
==========================================
Files 309 312 +3
Lines 33312 34064 +752
==========================================
+ Hits 24773 25510 +737
- Misses 8539 8554 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
onbjerg
added
C-enhancement
| let to_block = | ||
| std::cmp::min(stage_progress + self.commit_threshold, previous_stage_progress); |
There was a problem hiding this comment.
this can be done with exec_or_return macro (see other stages)
There was a problem hiding this comment.
I didn't want to use macro it hides this simple intention, and as NOTE said it would be better to do bundles with transition id than a block number, so I did it like this.
Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
rakita
force-pushed
the
rakita/history_indices
branch
from
71658c3 to
aaf8318
Compare
|
I added an additional change to indices and set the last shard to be (Address, u64::MAX) it is a small change but would allow us faster fetching of shard that contains our transition id. Just using seek_exact would fetch us the |
| // Insert last list with u64::MAX | ||
| if let Some(last_list) = last_chunk { | ||
| tx.put::<tables::StorageHistory>( | ||
| StorageShardedKey::new(address, storage_key, u64::MAX), | ||
| TransitionList::new(last_list) | ||
| .expect("Indices are presorted and not empty"), | ||
| )?; | ||
| } |
There was a problem hiding this comment.
why the last list is inserted with u64::MAX? can we add more detailed description for struct IndexStorageHistoryStage explaining what it actually stores and how (with special cases like u64::MAX)?
There was a problem hiding this comment.
| tx.cursor_read::<tables::AccountChangeSet>()? | ||
| .walk(from_transition_rev)? | ||
| .take_while(|res| res.as_ref().map(|(k, _)| *k < to_transition_rev).unwrap_or_default()) | ||
| .collect::<Result<Vec<_>, _>>()? | ||
| .into_iter() | ||
| // reverse so we can get lowest transition id where we need to unwind account. | ||
| .rev() | ||
| // fold all account and get last transition index | ||
| .fold(BTreeMap::new(), |mut accounts: BTreeMap<Address, u64>, (index, account)| { | ||
| // we just need address and lowest transition id. | ||
| accounts.insert(account.address, index); | ||
| accounts | ||
| }) | ||
| .into_iter() | ||
| // try to unwind the index | ||
| .try_for_each(|(address, rem_index)| -> Result<(), StageError> { | ||
| let shard_part = | ||
| unwind_account_history_shards::<DB>(&mut cursor, address, rem_index)?; | ||
|
|
||
| // check last shard_part, if present, items needs to be reinserted. | ||
| if !shard_part.is_empty() { | ||
| // there are items in list | ||
| tx.put::<tables::AccountHistory>( | ||
| ShardedKey::new(address, u64::MAX), | ||
| TransitionList::new(shard_part) | ||
| .expect("There is at least one element in list and it is sorted."), | ||
| )?; | ||
| } | ||
| Ok(()) | ||
| })?; |
There was a problem hiding this comment.
can we flatten this (maybe already commented this)? it's quite difficult to read. same in IndexStorageHistoryStage
There was a problem hiding this comment.
ok, left already comment like this. what i mean by flatten is
// Collect all account changesets.
let account_changesets = tx.cursor_read::<tables::AccountChangeSet>()?
// ... walk, take_while
;
// Create a map with lowest transition ids
let account_map = BTreeMap::default();
for entry in account_changesets {
let (index, account) = entry?;
account_map.insert(account.address, index); // maybe we need some logic to determine the lowest
}
for (address, rem_index) in account_map {
// ...
}There was a problem hiding this comment.
wdym with flatten here, didn't see your comment when writing this.collect it and use for _ in _?
Will flatten it
There was a problem hiding this comment.
It is flatten :). To be honest i still prefer iter chains, but this is nice too
Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
Co-authored-by: Roman Krasiuk <[email protected]>
ref: #815