Skip to content

feat(stages): add table checkpoint to AccountHashing and StorageHashing#1667

Merged
gakonst merged 25 commits intomainfrom
joshie/hashing-checkpoint
Mar 14, 2023
Merged

feat(stages): add table checkpoint to AccountHashing and StorageHashing#1667
gakonst merged 25 commits intomainfrom
joshie/hashing-checkpoint

Conversation

@joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Mar 8, 2023

Similar to/blocked by/branch out of #1656

Removes tx.commit from the hashing stages and uses checkpoints through tables::SyncStageProgress

@joshieDo joshieDo added S-blocked This cannot more forward until something else changes A-staged-sync Related to staged sync (pipelines and stages) labels Mar 8, 2023
@codecov-commenter
Copy link

codecov-commenter commented Mar 8, 2023

Codecov Report

Merging #1667 (360f7e1) into main (ee3a8af) will decrease coverage by 0.02%.
The diff coverage is 67.47%.

📣 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    #1667      +/-   ##
==========================================
- Coverage   72.65%   72.63%   -0.02%     
==========================================
  Files         392      393       +1     
  Lines       47168    47281     +113     
==========================================
+ Hits        34269    34342      +73     
- Misses      12899    12939      +40     
Flag Coverage Δ
integration-tests 20.11% <0.00%> (-0.04%) ⬇️
unit-tests 67.43% <67.47%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
bin/reth/src/dump_stage/hashing_account.rs 0.00% <0.00%> (ø)
bin/reth/src/dump_stage/hashing_storage.rs 0.00% <0.00%> (ø)
crates/primitives/src/lib.rs 100.00% <ø> (ø)
crates/primitives/src/proofs.rs 97.13% <ø> (-0.04%) ⬇️
crates/stages/src/stages/hashing_account.rs 87.35% <70.96%> (-3.68%) ⬇️
crates/stages/src/stages/hashing_storage.rs 94.72% <94.50%> (-1.22%) ⬇️
crates/primitives/src/checkpoints.rs 100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@joshieDo joshieDo marked this pull request as ready for review March 14, 2023 07:35
@joshieDo joshieDo requested a review from onbjerg as a code owner March 14, 2023 07:35
@joshieDo joshieDo requested a review from rkrasiuk as a code owner March 14, 2023 07:35
Comment on lines +127 to +137
// Address caching for the first iteration when current_key
// is None
let keccak_address =
if let Some(keccak_address) = keccak_address {
keccak_address
} else {
keccak256(address)
};

// TODO cache map keccak256(slot.key) ?
((keccak_address, keccak256(slot.key)), slot.value)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah probably right solution here is just to have a LRU maps for map<address, keccak256(address)> and map<h256, keccak256(h256)> to avoid redundant hashing, but doubt it'll be a bottleneck

@gakonst gakonst merged commit 46c96a1 into main Mar 14, 2023
@gakonst gakonst deleted the joshie/hashing-checkpoint branch March 14, 2023 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-staged-sync Related to staged sync (pipelines and stages) S-blocked This cannot more forward until something else changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants