-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat(invariants
): use SlotIdentifier
for identifying complex types and sampling them
#11450
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
Conversation
.filter_map(|type_info| { | ||
if type_info.encoding == "mapping" | ||
&& let Some(t_value) = type_info.value.as_ref() | ||
&& let Some(mapping_value) = t_value.strip_prefix("t_") | ||
{ | ||
DynSolType::parse(mapping_value).ok() | ||
} else { | ||
None | ||
} | ||
}) | ||
.collect(); |
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.
Note: Currently, we insert all mapping
value types regardless of whether the storage_slot
belongs to it or not.
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.
do I get this note right - that is clarifies how the PR improves collecting samples (we previously inserted all values regardless of type while the PR removes this limitation and inserts valued only for correct type)? Thanks!
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.
Not exactly - we didn't insert all values only values that decoded into any one of the mapping types.
We collect all mapping types here, and then attempt decoding of the incoming storage_value
here: #11450 (comment).
If decoding succeeded, we would insert it into the sample.
This was better than inserting all values, but not as good as SlotIdentifier
which identifies the exact slot type and inserts accordingly.
For example, with the non-SlotIdentifier
approach:
address superAdmin; // Slot 0
mapping(address admin => address spender); // Base Slot = 1
Even if the storage change occurred in slot 0
, we would insert the sample value for mappings as well, since the mapping value type (address spender)
matches the type of slot 0, and abi.decode(storage_value)
would succeed.
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.
On second thought, I guess this was fine -- and our approach more or less remained the same as it's ok to insert sample value of same types even though they may be originating from a different variable's storage_change.
What we get from the SlotIdentifier
is the ability to handle structs primarily and nested mappings.
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.
got it, that should improve things do you see any difference for simple DSChief test with this PR?
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.
Possible, but it's not breaking it on nightly
version as well:
forge --version
forge Version: 1.3.2-nightly
Commit SHA: e6098acb1a1a3de0dd4051f8c81f5b3ac6f75122
Build Timestamp: 2025-08-29T06:04:48.477884000Z (1756447488)
Build Profile: maxperf
I have a campaign running:
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment.
[⠊] Compiling...
No files changed, compilation skipped
---------------------------------------- 0/1 completed (with 8 threads)
⠁ test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
↪ invariant_check_dschief: [151764] Runs, ends at 09:15:29 2025-08-29 UTC
I'll revert back when this ends.
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.
Yeah it passed on nightly
forge t --show-progress
Warning: This is a nightly build of Foundry. It is recommended to use the latest stable version. To mute this warning set `FOUNDRY_DISABLE_NIGHTLY_WARNING` in your environment.
[⠊] Compiling...
No files changed, compilation skipped
test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
↪ Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 3671.87s (3671.87s CPU time)
Ran 1 test for test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
[PASS] invariant_check_dschief() (runs: 180841, calls: 90420500, reverts: 6766509)
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.
Hmm, Able to break it on nightly before #11440 i.e https://github.com/foundry-rs/foundry/tree/nightly-4e59f423070ddf6f316c44ee8cfe16e9c012b54c
Implementation in #11440 looks alright. Will need to investigate this deeper.
forge-pre-11440-nightly t --mt invariant_check_dschief --show-progress
[⠊] Compiling...
No files changed, compilation skipped
test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
↪ Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1063.10s (1063.09s CPU time)
Ran 1 test for test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
[FAIL: assertion failed]
[Sequence] (original: 120, shrunk: 5)
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteSlate(bytes32) args=[0xe169a59fb923f91b8fb1d1fc9258a999c5a19ad050199cd59889d6fa30d99753]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=lock(uint256) args=[231150035537531015466043543667015649 [2.311e35]]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteYays(address) args=[0x0000000000000000000000000000000000001474]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteYays(address) args=[0xc8453dd06678c8C3B09FeC20c26E737a4C22f820]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=checkInvariant() args=[]
invariant_check_dschief() (runs: 68524, calls: 34262000, reverts: 2542136)
╭---------------+----------------+---------+---------+----------╮
| Contract | Selector | Calls | Reverts | Discards |
+===============================================================+
| SimpleDSChief | checkInvariant | 5710314 | 0 | 0 |
|---------------+----------------+---------+---------+----------|
| SimpleDSChief | etch | 5708617 | 0 | 0 |
|---------------+----------------+---------+---------+----------|
| SimpleDSChief | free | 5710286 | 1134438 | 0 |
|---------------+----------------+---------+---------+----------|
| SimpleDSChief | lock | 5710955 | 554142 | 0 |
|---------------+----------------+---------+---------+----------|
| SimpleDSChief | voteSlate | 5710865 | 853009 | 0 |
|---------------+----------------+---------+---------+----------|
| SimpleDSChief | voteYays | 5711100 | 547 | 0 |
╰---------------+----------------+---------+---------+----------╯
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1063.10s (1063.09s CPU time)
Ran 1 test suite in 1063.12s (1063.10s CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
[FAIL: assertion failed]
[Sequence] (original: 120, shrunk: 5)
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteSlate(bytes32) args=[0xe169a59fb923f91b8fb1d1fc9258a999c5a19ad050199cd59889d6fa30d99753]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=lock(uint256) args=[231150035537531015466043543667015649 [2.311e35]]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteYays(address) args=[0x0000000000000000000000000000000000001474]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteYays(address) args=[0xc8453dd06678c8C3B09FeC20c26E737a4C22f820]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=checkInvariant() args=[]
invariant_check_dschief() (runs: 68524, calls: 34262000, reverts: 2542136)
Encountered a total of 1 failing tests, 0 tests succeeded
Running campaign once more on nightly-6e6341b8f6518f00b64c439714c91e14db2955cf to make sure #11440 is the issue
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.
Running campaign once more on nightly-6e6341b8f6518f00b64c439714c91e14db2955cf to make sure #11440 is the issue
Broke with #11440 included as well
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.
OK, ran with latest changes and successfully broke the invariant thrice:
First:
Encountered 1 failing test in test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
[FAIL: assertion failed]
[Sequence] (original: 177, shrunk: 4)
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteSlate(bytes32) args=[0xcc53d03fda58d9a098fd16c62dc974b0bc1c6e942fad55724f240f085e67485c]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=lock(uint256) args=[125813996375599159817140963330240011258305308995329 [1.258e50]]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=etch(address) args=[0x000000000000000000000000000000000000178c]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=checkInvariant() args=[]
invariant_check_dschief() (runs: 10540, calls: 5270000, reverts: 290526)
Second:
Failing tests:
Encountered 1 failing test in test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
[FAIL: assertion failed]
[Sequence] (original: 166, shrunk: 4)
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteSlate(bytes32) args=[0xe169a59fb923f91b8fb1d1fc9258a999c5a19ad050199cd59889d6fa30d99753]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=lock(uint256) args=[15464324525850824866417824659042904377 [1.546e37]]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=etch(address) args=[0x0000000000000000000000000000000000001474]
sender=0x0000000000000000000000000000000000020000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=checkInvariant() args=[]
invariant_check_dschief() (runs: 135824, calls: 67912000, reverts: 3681433)
Third:
Failing tests:
Encountered 1 failing test in test/SimpleDSChiefTest.t.sol:SimpleDSChiefTest
[FAIL: assertion failed]
[Sequence] (original: 127, shrunk: 6)
sender=0x0000000000000000000000000000000000010000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=lock(uint256) args=[254383755 [2.543e8]]
sender=0x0000000000000000000000000000000000010000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteSlate(bytes32) args=[0xcc53d03fda58d9a098fd16c62dc974b0bc1c6e942fad55724f240f085e67485c]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=voteYays(address) args=[0x000000000000000000000000000000000000178c]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=lock(uint256) args=[961]
sender=0x0000000000000000000000000000000000010000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=free(uint256) args=[35]
sender=0x0000000000000000000000000000000000030000 addr=[src/SimpleDSChief.sol:SimpleDSChief]0x5615dEB798BB3E4dFa0139dFa1b3D433Cc23b72f calldata=checkInvariant() args=[]
invariant_check_dschief() (runs: 79667, calls: 39833500, reverts: 2160730)
storage_value: &U256, | ||
) { | ||
for sol_type in mapping_types { | ||
match sol_type.abi_decode(storage_value.as_le_slice()) { |
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.
https://github.com/foundry-rs/foundry/pull/11450/files#r2309268582, then attempt decoding the value into the mapping type and insert it if successful
Marking as draft due to: #11450 (comment) |
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!
…s and sampling them (foundry-rs#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` (foundry-rs#11343) * add @onbjerg * add @0xrusowsky * resolve conflicts * fix: disable tx gas limit cap (foundry-rs#11347) * chore(deps): bump all dependencies (foundry-rs#11349) * chore: use get_or_calculate_hash better (foundry-rs#11350) * resolve more conflicts * fix(lint): 'unwrapped-modifier-logic' incorrectly marked with `Severity::Gas` (foundry-rs#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 (foundry-rs#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 (foundry-rs#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 <95942363+zerosnacks@users.noreply.github.com> * 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 <yash@Yashs-Laptop.local> Co-authored-by: zerosnacks <95942363+zerosnacks@users.noreply.github.com> Co-authored-by: Matthias Seitz <matthias.seitz@outlook.de> Co-authored-by: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> Co-authored-by: 0xrusowsky <90208954+0xrusowsky@users.noreply.github.com> Co-authored-by: grandizzy <38490174+grandizzy@users.noreply.github.com> Co-authored-by: grandizzy <grandizzy.the.egg@gmail.com>
Motivation
Closes #11334
Currently we are sampling only primitive types from storage changesets.
Solution
SlotIdentifier
introduced in refactor: moves state diff decoding to common #11413 to identify slots of more complex types such as arrays, structs, and mappingsCommits:
SlotIdentifier
for identiying primitive types, array and structs: 3a88f5dPR Checklist