feat(pruning): prune storage history by contract or slots

[alessandromazza98]

Closes #4350

This is my attempt to solve that issue. I would like to receive some review and feedback on it please. @shekhirin

I tried to also add a test for it but it's incomplete for now because I was not able to properly test StorageHistory table.
As of now my test only tests that, after pruning specifying a contract address (and not a slot), inside StorageChangeSet table there are only changes related to the specified address or that are unprunable (tip - 128).

I took a lot of inspiration from the previous pruning_receipts_by_log feature.

[shekhirin]

Sorry for the delay in review, been busy with working on new features. Will take a look today!

[shekhirin]

I had one pass, will take another look. Also would appreciate @joshieDo's review because the logic is similar to receipts by logs pruning.

Overall seems ok, I wonder if we can somehow unify the lowest_block_with_distance and group_by_block logics between receipts and storages? Because we will also probably add the same pruning for accounts, and it'll be the 3rd implementation of almost the same functionality.

[joshieDo]

Overall seems ok, I wonder if we can somehow unify the lowest_block_with_distance and group_by_block logics between receipts and storages
Because we will also probably add the same pruning for accounts

They seem the same. Maybe we can make a common trait (given that we will add even more), add a default impl that takes the PrunePart

[alessandromazza98]

They seem the same. Maybe we can make a common trait (given that we will add even more), add a default impl that takes the PrunePart

Yes, sure we can add a common trait.

Maybe I can wait for some other reviews / feedbacks and then work on it all together. Let me know.
Thanks in advance for your reviews.

[codecov]

Codecov Report

Attention: 66 lines in your changes are missing coverage. Please review.

Comparison is base (4dc15c3) 26.07% compared to head (0b97420) 68.06%.
Report is 592 commits behind head on main.

❗ Current head 0b97420 differs from pull request most recent head 02c39d5. Consider uploading reports for the commit 02c39d5 to get more accurate results

Additional details and impacted files

Files	Coverage Δ
crates/primitives/src/lib.rs	100.00% <ø> (ø)
crates/primitives/src/prune/part.rs	100.00% <ø> (ø)
crates/primitives/src/prune/target.rs	56.52% <0.00%> (+27.95%)	⬆️
crates/primitives/src/prune/mod.rs	77.38% <75.55%> (+75.15%)	⬆️
bin/reth/src/args/pruning_args.rs	14.63% <0.00%> (-6.80%)	⬇️
crates/prune/src/pruner.rs	79.48% <76.74%> (+79.48%)	⬆️

... and 521 files with indirect coverage changes

Flag	Coverage Δ
integration-tests	16.70% <0.00%> (-9.37%)	⬇️
unit-tests	63.38% <71.55%> (?)

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

Components	Coverage Δ
reth binary	32.10% <0.00%> (+6.32%)	⬆️
blockchain tree	83.75% <ø> (+55.28%)	⬆️
pipeline	88.54% <ø> (+83.49%)	⬆️
storage (db)	73.01% <ø> (+43.04%)	⬆️
trie	94.73% <ø> (+72.20%)	⬆️
txpool	49.44% <ø> (+8.06%)	⬆️
networking	77.10% <ø> (+46.20%)	⬆️
rpc	57.81% <ø> (+31.33%)	⬆️
consensus	63.25% <ø> (+38.18%)	⬆️
revm	28.16% <ø> (+18.31%)	⬆️
payload builder	8.45% <ø> (-5.71%)	⬇️
primitives	86.49% <72.34%> (+57.32%)	⬆️

[shekhirin]

is it possible to set this configuration from TOML config file? IIUC it needs to have a map with AddressAndSlots as a key, but TOML doesn't support non-string keys.

I think it would be better to stick to a format like so

pub struct AddressAndSlots {
    pub address: Address,
    pub slots: Vec<StorageKey>,
}

pub struct StorageHistoryPruneAddressConfig {
    pub mode: Option<PruneMode>,
    pub slots: BTreeMap<StorageKey, PruneMode>,
}

pub struct StorageHistoryPruneConfig(pub BTreeMap<Address, StorageHistoryPruneAddressConfig>);

to be able to set the configuration according to the format I've mentioned in #4350 (comment)

[shekhirin]

I'm going to sync a node on this PR by changing the default configuration for this segment, and see if the expected contract storages or storage slots are preserved.

[gakonst]

Hi @alessandromazza98 what's your plan here for next steps? Would love to get it in, or else would like to close anything stale

[alessandromazza98]

Hei @gakonst , thanks for pinging me. I'd like to go ahead with it. I'm gonna think what are best next steps and do them. I'll update the issue very shortly, thanks.

[alessandromazza98]

I tested with a TOML file like this:

[prune.parts.storage_history_filter."0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48"]
mode = { before = 10091092 }
slots = {}

[prune.parts.storage_history_filter."0x00000000000000adc04c56bf30ac9d3c0aaf14dc"]
slots = { "0x0000000000000000000000000000000000000000000000000000000000000003" = { before = 10291092 } }

Those are two contracts on Ethereum mainnet (USDC the first one).

I also added another test to verify for the pruning of a contract AND a storage slot.

Basically you can filter like this in the TOML file:

1. filter only by contract: saving every slot of this contract

[prune.parts.storage_history_filter.<CONTRACT_ADDRESS>]
mode = { PRUNE_MODE }
slots = {}

2. filter by contract AND slots of the contract (here you do not need to add mode because you have to specify the prune mode for every slot)

[prune.parts.storage_history_filter.<CONTRACT_ADDRESS>]
slots = { SLOT_KEY = { PRUNE_MODE} }