feat(storage): account for pruned account/storage history #4092

shekhirin · 2023-08-07T11:07:39Z

Tested with requesting the trace_transaction for beacon deposit contract creation transaction https://etherscan.io/tx/0xe75fb554e433e03763a1560646ee22dcb74e5274b34c5ad644e7c0f619a7e1d0.

Before:

ubuntu@reth1:~/reth-alexey$ cast rpc trace_transaction 0xe75fb554e433e03763a1560646ee22dcb74e5274b34c5ad644e7c0f619a7e1d0
Error:
(code: -32003, message: nonce too high, data: None)

After:

ubuntu@reth1:~/reth-alexey$ cast rpc trace_transaction 0xe75fb554e433e03763a1560646ee22dcb74e5274b34c5ad644e7c0f619a7e1d0
Error:
(code: -32603, message: State at block #11052984 is pruned, data: None)

Same for other methods that use the historical state provider:

ubuntu@reth1:~/reth-alexey$ cast rpc eth_getStorageAt 0x00000000219ab540356cBB839Cbe05303d7705Fa 0x0000000000000000000000000000000000000000000000000000000000000000 0x2
Error:
(code: -32603, message: State at block #3 is pruned, data: None)

joshieDo · 2023-08-07T11:34:00Z

i don't quite like that i'm getting a provider for a block that i didn't request. I'd rather it fail (or None ?), and handle the error somewhere else

codecov · 2023-08-07T11:41:35Z

Codecov Report

Merging #4092 (c307a94) into main (aaf2d2c) will increase coverage by 0.07%.
Report is 29 commits behind head on main.
The diff coverage is 78.19%.

Files Changed	Coverage Δ
crates/interfaces/src/provider.rs	`100.00% <ø> (+100.00%)`	⬆️
...tes/storage/provider/src/providers/database/mod.rs	`35.78% <62.06%> (+3.80%)`	⬆️
...storage/provider/src/providers/state/historical.rs	`87.04% <82.69%> (-1.28%)`	⬇️

... and 59 files with indirect coverage changes

Flag	Coverage Δ
integration-tests	`16.85% <0.00%> (+0.34%)`	⬆️
unit-tests	`63.95% <78.19%> (-0.07%)`	⬇️

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

Components	Coverage Δ
reth binary	`25.28% <ø> (-0.23%)`	⬇️
blockchain tree	`82.83% <ø> (-0.22%)`	⬇️
pipeline	`90.07% <ø> (ø)`
storage (db)	`74.72% <78.19%> (+0.16%)`	⬆️
trie	`94.71% <ø> (+<0.01%)`	⬆️
txpool	`47.59% <ø> (-0.78%)`	⬇️
networking	`77.39% <ø> (-0.05%)`	⬇️
rpc	`58.67% <ø> (+1.22%)`	⬆️
consensus	`63.76% <ø> (-0.32%)`	⬇️
revm	`32.26% <ø> (-0.28%)`	⬇️
payload builder	`6.57% <ø> (-0.02%)`	⬇️
primitives	`87.93% <ø> (-0.08%)`	⬇️

mattsse

agree with @joshieDo

if the requested block's state is pruned this should just fail.

this looks way too complex

shekhirin · 2023-08-07T13:42:16Z

@joshieDo @mattsse agree, made it return an error instead. But I'd still prefer to check account and storage history restrictions separately, as e.g. eth_getStorageAt doesn't require account history, and a user can have separate account/storage history pruning configurations.

crates/storage/provider/src/providers/state/historical.rs

mattsse

we definitely need this.

left some suggestions,
a helper wrapper type could be nice with functions to ensure target block number is present

mattsse · 2023-08-09T16:17:08Z

crates/storage/provider/src/providers/database/mod.rs

+        let account_history_prune_checkpoint =
+            provider.get_prune_checkpoint(PrunePart::AccountHistory)?;
+        let storage_history_prune_checkpoint =
+            provider.get_prune_checkpoint(PrunePart::StorageHistory)?;


this makes sense and is the correct way, should be negligible

mattsse · 2023-08-09T16:18:34Z

crates/storage/provider/src/providers/state/historical.rs

+        lowest_account_history_block_number: Option<BlockNumber>,
+        lowest_storage_history_block_number: Option<BlockNumber>,
+        block_number: BlockNumber,


lets flip the order

block_number first then the options

agree, fixed

mattsse · 2023-08-09T16:19:15Z

crates/storage/provider/src/providers/state/historical.rs

+    /// Lowest block number at which the account history is available.
+    lowest_account_history_block_number: Option<BlockNumber>,
+    /// Lowest block number at which the storage history is available.
+    lowest_storage_history_block_number: Option<BlockNumber>,


I'd like to add some docs here that we have this because data could be pruned

perhaps consider adding a helper type for this, maybe we need more things in the future?

added docs, helper struct and also tests

mattsse

lgtm

shekhirin added 2 commits August 7, 2023 12:05

feat(storage): account for pruned account/storage history

ef02024

fix borrow

01018bb

mattsse requested changes Aug 7, 2023

View reviewed changes

shekhirin closed this Aug 7, 2023

return error on pruned block

0473aee

shekhirin reopened this Aug 7, 2023

fix lint

40f5a69

shekhirin mentioned this pull request Aug 7, 2023

Pruning Testing Checklist #3958

Closed

shekhirin requested a review from mattsse August 7, 2023 13:40

shekhirin marked this pull request as ready for review August 7, 2023 13:40

shekhirin requested review from rakita, joshieDo and gakonst as code owners August 7, 2023 13:40

shekhirin added A-rpc Related to the RPC implementation A-pruning Related to pruning or full node labels Aug 7, 2023

joshieDo reviewed Aug 7, 2023

View reviewed changes

crates/storage/provider/src/providers/state/historical.rs Outdated Show resolved Hide resolved

mattsse requested changes Aug 9, 2023

View reviewed changes

shekhirin added 2 commits August 9, 2023 18:01

reorder fields, add helper struct, add tests

30cc002

add explanations to test

c307a94

shekhirin requested a review from mattsse August 9, 2023 17:13

mattsse approved these changes Aug 9, 2023

View reviewed changes

joshieDo approved these changes Aug 9, 2023

View reviewed changes

shekhirin added this pull request to the merge queue Aug 9, 2023

Merged via the queue into main with commit a8a2cfa Aug 9, 2023
24 checks passed

shekhirin deleted the alexey/historical-state-prune branch August 9, 2023 18:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(storage): account for pruned account/storage history #4092

feat(storage): account for pruned account/storage history #4092

shekhirin commented Aug 7, 2023 •

edited

Loading

joshieDo commented Aug 7, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading

mattsse left a comment

shekhirin commented Aug 7, 2023

mattsse left a comment

mattsse Aug 9, 2023

mattsse Aug 9, 2023

shekhirin Aug 9, 2023

mattsse Aug 9, 2023

shekhirin Aug 9, 2023

mattsse left a comment

feat(storage): account for pruned account/storage history #4092

feat(storage): account for pruned account/storage history #4092

Conversation

shekhirin commented Aug 7, 2023 • edited Loading

joshieDo commented Aug 7, 2023 • edited Loading

codecov bot commented Aug 7, 2023 • edited Loading

Codecov Report

mattsse left a comment

Choose a reason for hiding this comment

shekhirin commented Aug 7, 2023

mattsse left a comment

Choose a reason for hiding this comment

mattsse Aug 9, 2023

Choose a reason for hiding this comment

mattsse Aug 9, 2023

Choose a reason for hiding this comment

shekhirin Aug 9, 2023

Choose a reason for hiding this comment

mattsse Aug 9, 2023

Choose a reason for hiding this comment

shekhirin Aug 9, 2023

Choose a reason for hiding this comment

mattsse left a comment

Choose a reason for hiding this comment

shekhirin commented Aug 7, 2023 •

edited

Loading

joshieDo commented Aug 7, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading