fix(anvil): block dumps #8160
fix(anvil): block dumps #8160
Conversation
crates/anvil/src/eth/backend/db.rs
Outdated
| // TODO: only implementing here for now, need to upstream this | ||
| // can we make alloy_consensus::header Serializable/Deserializable? | ||
| /// this is the same struct, literally, with derive(Serialize, Deserialize) |
There was a problem hiding this comment.
Highlighting this. Copied a bunch of structs just to make them Serialize/Deserialize. We can clean this up by deriving Serialize/Deserialize upstream in alloy if needed.
There was a problem hiding this comment.
we should upstream this instead, looks like the serde impl for header is just missing
| let blocks = self.blockchain.storage.read().serialized_blocks(); | ||
| let state = self.db.read().await.dump_state(at, best_number, blocks)?; |
There was a problem hiding this comment.
Not sure if this is the best implementation. Reading the blocks and then passing them to the dump_state function in db to also include them in the dumped json.
Will want to also dump events soon, so the db dump_state function might become cluttered and coupled. An alternative design is to pass mut references to a serialized_state to db, storage, event services, and have them add whatever information they are dumping, so that we can partially construct the serialized_state?
|
|
||
| // verifies that blocks in BlockchainStorage remain the same when dumped and reloaded | ||
| #[test] | ||
| fn test_storage_dump_reload_cycle() { |
There was a problem hiding this comment.
added this unit test for storage, but now that we are dumping and reloading both the db and storage, we might need an integration test in mod.rs for the dumping/reloading of the entire serialized_state?
|
FYI you can just build and run in debug mode with |
|
@DaniPopes fixed the anvil related clippy errors from https://github.com/foundry-rs/foundry/actions/runs/9512891848/job/26223615034?pr=8160 in 036aeed |
crates/anvil/src/eth/backend/db.rs
Outdated
| // TODO: only implementing here for now, need to upstream this | ||
| // can we make alloy_consensus::header Serializable/Deserializable? | ||
| /// this is the same struct, literally, with derive(Serialize, Deserialize) |
There was a problem hiding this comment.
we should upstream this instead, looks like the serde impl for header is just missing
|
Hey @mattsse , tried to update this PR to alloy's commit b857ea5 but running into was signer-wallet renamed to signer-trezor? How can I fix this? Or can you guys make the upgrade and commit it so I can rebase my stuff off of it if it requires big changes? |
|
#8177 was just merged, pls just rebase |
samlaf
force-pushed
the
fix-7502-block-dumps
branch
from
036aeed
to
e495ffe
Compare
* implemented latest_block dump/load * update to dump/load all blocks instead of only latest * refactored state loading into storage.rs, and added load-dump cycle test * fix clippy errors for anvil * remove SerializableHeader and use Header (now serializable) * clippy happy --------- Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de>
Motivation
Fixes #7502
Solution
Dump all blocks as part of dumped json file.
built with
cargo install --path ./crates/anvil --profile local --force --lockedand tested by running:then closed anvil, and restarted with
And then dumped json db file looks like
Remaining bugs
seems like full block is not dumped?
Random comments
Was originally formatting with
cargo fmtwhich was creating a huge number of diffs (mostly adding;at end of lines). Realized you guys probably usecargo +nightly fmtsince it didn't create diffs. However it also doesn't enforce the styling, so I had to manually remove all the;added bycargo fmt, which was very annoying. Is there a way to enforce a single style fromcargo +nightly fmt? This way it would have removed (or added) all;, but not accept both and leave my git status in a weird state.