-
Notifications
You must be signed in to change notification settings - Fork 2.1k
refactor: moves state diff decoding to common #11413
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor: moves state diff decoding to common #11413
Conversation
@@ -0,0 +1,685 @@ | |||
//! Storage slot identification and decoding utilities for Solidity storage layouts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@grandizzy this file contains the slot type identification and decoding logic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, left couple of nits, pls check
None | ||
} | ||
|
||
fn handle_struct( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as above, let's add a comment to this fn too. Do we need both handle_struct
and handle_struct_recursive
too or can we get rid of handle_struct
and rename handle_struct_recursive
to handle_struct
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crates/common/src/slot_identifier.rs
Outdated
) -> Option<SlotInfo> { | ||
// Limit recursion depth to prevent stack overflow | ||
const MAX_DEPTH: usize = 10; | ||
if depth > MAX_DEPTH { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
guess that's OK, wonder though how you choose 10 as max depth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chose it arbitrarily. Depth increases only when we encounter nested structs.
Open to increasing it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense 👍
46984c2
into
yash/decode-mappings-state-diffs
* feat(cheatcodes): decode mappings in state diffs * feat: decode nested mappings * assert vm.getStateDiff output * feat: add `keys` fields to `SlotInfo` in case of mappings * remove wrapper * refactor: moves state diff decoding to common (#11413) * refactor: storage decoder * cleanup * dedup MappingSlots by moving it to common * move decoding logic into SlotInfo * rename to SlotIndentifier * docs * fix: delegate identification according to encoding types * clippy + fmt * docs fix * fix * merge match arms * merge ifs * recurse handle_struct
…11331) * feat(`forge`): sample typed storage values * arc it * nit * clippy * nit * strip file prefixes * fmt * don't add adjacent values to sample * feat(cheatcodes): add contract identifier to AccountStateDiffs * forge fmt * doc nits * fix tests * feat(`cheatcodes`): include `SlotInfo` in SlotStateDiff * cleanup + identify slots of static arrays * nits * nit * nits * test + nits * docs * handle 2d arrays * use DynSolType * feat: decode storage values * doc nit * skip decoded serialization if none * nit * fmt * fix * fix * fix * feat(cheatcodes): decode structs in state diff output * fix * while decode * fix: show only decoded in plaintext / display output + test * feat: format slots to only significant bits in vm.getStateDiff output * encode_prefixed * nit * chore: add @onbjerg to `CODEOWNERS` (#11343) * add @onbjerg * add @0xrusowsky * resolve conflicts * fix: disable tx gas limit cap (#11347) * chore(deps): bump all dependencies (#11349) * chore: use get_or_calculate_hash better (#11350) * resolve more conflicts * fix(lint): 'unwrapped-modifier-logic' incorrectly marked with `Severity::Gas` (#11358) fix(lint): 'unwrapped-modifier-logic' incorrectly marked with Severity::Gas * feat: identify and decode nested structs * cleanup * decode structs and members recursively * cleanup * doc fix * feat(cheatcodes): decode mappings in state diffs (#11381) * feat(cheatcodes): decode mappings in state diffs * feat: decode nested mappings * assert vm.getStateDiff output * feat: add `keys` fields to `SlotInfo` in case of mappings * remove wrapper * refactor: moves state diff decoding to common (#11413) * refactor: storage decoder * cleanup * dedup MappingSlots by moving it to common * move decoding logic into SlotInfo * rename to SlotIndentifier * docs * fix: delegate identification according to encoding types * clippy + fmt * docs fix * fix * merge match arms * merge ifs * recurse handle_struct * dedup assertContains test util * fix * Update crates/common/src/slot_identifier.rs Co-authored-by: zerosnacks <[email protected]> * Review changes: simplify get or insert, use common fmt * alloy-dyn-abi.workspace * nits --------- Co-authored-by: Yash Atreya <[email protected]> Co-authored-by: zerosnacks <[email protected]> Co-authored-by: Matthias Seitz <[email protected]> Co-authored-by: DaniPopes <[email protected]> Co-authored-by: srdtrk <[email protected]> Co-authored-by: 0xrusowsky <[email protected]> Co-authored-by: grandizzy <[email protected]> Co-authored-by: grandizzy <[email protected]>
…s and sampling them (#11450) * feat(`forge`): sample typed storage values * arc it * nit * clippy * nit * strip file prefixes * fmt * don't add adjacent values to sample * feat(cheatcodes): add contract identifier to AccountStateDiffs * forge fmt * doc nits * fix tests * feat(`cheatcodes`): include `SlotInfo` in SlotStateDiff * cleanup + identify slots of static arrays * nits * nit * nits * test + nits * docs * handle 2d arrays * use DynSolType * feat: decode storage values * doc nit * skip decoded serialization if none * nit * fmt * fix * fix * fix * feat(cheatcodes): decode structs in state diff output * fix * while decode * fix: show only decoded in plaintext / display output + test * feat: format slots to only significant bits in vm.getStateDiff output * encode_prefixed * nit * chore: add @onbjerg to `CODEOWNERS` (#11343) * add @onbjerg * add @0xrusowsky * resolve conflicts * fix: disable tx gas limit cap (#11347) * chore(deps): bump all dependencies (#11349) * chore: use get_or_calculate_hash better (#11350) * resolve more conflicts * fix(lint): 'unwrapped-modifier-logic' incorrectly marked with `Severity::Gas` (#11358) fix(lint): 'unwrapped-modifier-logic' incorrectly marked with Severity::Gas * feat: identify and decode nested structs * cleanup * decode structs and members recursively * cleanup * doc fix * feat(cheatcodes): decode mappings in state diffs (#11381) * feat(cheatcodes): decode mappings in state diffs * feat: decode nested mappings * assert vm.getStateDiff output * feat: add `keys` fields to `SlotInfo` in case of mappings * remove wrapper * refactor: moves state diff decoding to common (#11413) * refactor: storage decoder * cleanup * dedup MappingSlots by moving it to common * move decoding logic into SlotInfo * rename to SlotIndentifier * docs * fix: delegate identification according to encoding types * clippy + fmt * docs fix * fix * merge match arms * merge ifs * recurse handle_struct * dedup assertContains test util * fix * Update crates/common/src/slot_identifier.rs Co-authored-by: zerosnacks <[email protected]> * Review changes: simplify get or insert, use common fmt * alloy-dyn-abi.workspace * identify slot types using `SlotIdentifier` * clippy * feat(`invariants`): record mapping keys and slots to identify their types for sampling * fix * only insert if value decodes * tracing::info logs * remove logs * nit * rm --------- Co-authored-by: Yash Atreya <[email protected]> Co-authored-by: zerosnacks <[email protected]> Co-authored-by: Matthias Seitz <[email protected]> Co-authored-by: DaniPopes <[email protected]> Co-authored-by: srdtrk <[email protected]> Co-authored-by: 0xrusowsky <[email protected]> Co-authored-by: grandizzy <[email protected]> Co-authored-by: grandizzy <[email protected]>
Motivation
Stacked on #11381
Towards #11334
After #11331 and #11381, the state diff identification and decoding logic had become tightly coupled with cheatcodes.
Slot type identification and decoding can be used in other places as well (e.g invariants)
Solution
SlotIdentifier
andSlotInfo
types incommon
SlotIdentifier
leverages theStorageLayout
to identify the slot types of raw storage slotsSlotInfo
for the slots it can identifySlotInfo
can then be used to decode slot valuesMappingSlots
has been moved fromcheatcodes
tocommon
as it is used inSlotIdentifier
for mapping slots.PR Checklist