feat: autogenerate decoded state diffs for validations #607
+541
−1
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Comment on lines
215
to
384
| summary: "Batch inbox address", | ||
| detail: "Unstructured storage slot for the address of the BatchInbox proxy." | ||
| }); | ||
| } | ||
| if (_slot == START_BLOCK_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "uint256", | ||
| oldValue: toUint(_oldValue), | ||
| newValue: toUint(_newValue), | ||
| summary: "Start block", | ||
| detail: "Unstructured storage slot for the start block number." | ||
| }); | ||
| } | ||
| if (_slot == DISPUTE_GAME_FACTORY_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "address", | ||
| oldValue: toAddress(_oldValue), | ||
| newValue: toAddress(_newValue), | ||
| summary: "DisputeGameFactory proxy address", | ||
| detail: "Unstructured storage slot for the address of the DisputeGameFactory proxy." | ||
| }); | ||
| } | ||
| if (_slot == L2_OUTPUT_ORACLE_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "address", | ||
| oldValue: toAddress(_oldValue), | ||
| newValue: toAddress(_newValue), | ||
| summary: "L2OutputOracle proxy address", | ||
| detail: "Unstructured storage slot for the address of the L2OutputOracle proxy." | ||
| }); | ||
| } | ||
|
|
||
| // SuperchainConfig. | ||
| if (_slot == PAUSED_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "bool", | ||
| oldValue: toBool(_oldValue), | ||
| newValue: toBool(_newValue), | ||
| summary: "Superchain pause status", | ||
| detail: "Unstructured storage slot for the pause status of the superchain." | ||
| }); | ||
| } | ||
| if (_slot == GUARDIAN_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "address", | ||
| oldValue: toAddress(_oldValue), | ||
| newValue: toAddress(_newValue), | ||
| summary: "Guardian address", | ||
| detail: "Unstructured storage slot for the address of the superchain guardian." | ||
| }); | ||
| } | ||
|
|
||
| // ProtocolVersions. | ||
| if (_slot == REQUIRED_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "uint256", | ||
| oldValue: toUint(_oldValue), | ||
| newValue: toUint(_newValue), | ||
| summary: "Required protocol version", | ||
| detail: "Unstructured storage slot for the required protocol version." | ||
| }); | ||
| } | ||
| if (_slot == RECOMMENDED_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "uint256", | ||
| oldValue: toUint(_oldValue), | ||
| newValue: toUint(_newValue), | ||
| summary: "Recommended protocol version", | ||
| detail: "Unstructured storage slot for the recommended protocol version." | ||
| }); | ||
| } | ||
|
|
||
| // Gas paying token slots. | ||
| if (_slot == GAS_PAYING_TOKEN_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "address", | ||
| oldValue: toAddress(_oldValue), | ||
| newValue: toAddress(_newValue), | ||
| summary: "Gas paying token address", | ||
| detail: "Unstructured storage slot for the address of the gas paying token." | ||
| }); | ||
| } | ||
| if (_slot == GAS_PAYING_TOKEN_NAME_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "string", | ||
| oldValue: vm.toString(_oldValue), | ||
| newValue: vm.toString(_newValue), | ||
| summary: "Gas paying token name", | ||
| detail: "Unstructured storage slot for the name of the gas paying token." | ||
| }); | ||
| } | ||
| if (_slot == GAS_PAYING_TOKEN_SYMBOL_SLOT) { | ||
| return DecodedSlot({ | ||
| kind: "string", | ||
| oldValue: vm.toString(_oldValue), | ||
| newValue: vm.toString(_newValue), | ||
| summary: "Gas paying token symbol", | ||
| detail: "Unstructured storage slot for the symbol of the gas paying token." | ||
| }); | ||
| } |
There was a problem hiding this comment.
It would be really great if the new values for addresses were checked to make sure they are contracts where applicable.
There was a problem hiding this comment.
That feels like a coupling of concerns, this file is purely decoding of state diffs, and we have existing approaches to handle validating the correctness of storage writes
|
|
||
| string memory label = layout[i]._label; | ||
| return DecodedSlot({kind: kind, oldValue: oldValue, newValue: newValue, summary: label, detail: ""}); | ||
| } |
There was a problem hiding this comment.
will this work for mappings?
There was a problem hiding this comment.
This does not support mappings yet, that will come later
mds1
commented
Feb 14, 2025
| @@ -116,6 +118,8 @@ contract NestedSignFromJson is OriginalNestedSignFromJson, CouncilFoundationNest | |||
| checkDisputeGameFactory(); | |||
| checkPermissionedDisputeGame(); | |||
| console.log("All assertions passed!"); | |||
|
|
|||
| StateDiffDecoder.decode(accesses); | |||
There was a problem hiding this comment.
This file shows example usage, this feature is not used by default
mds1
commented
Feb 14, 2025
Comment on lines
444
to
478
| // Read the entire JSON file. | ||
| string memory jsonData = vm.readFile(filePath); | ||
|
|
||
| // Get all the top-level keys, which are the chain IDs | ||
| string[] memory chainIds = vm.parseJsonKeys(jsonData, "$"); | ||
| for (uint256 i = 0; i < chainIds.length; i++) { | ||
| string memory chainId = chainIds[i]; | ||
|
|
||
| // Get all the keys of the nested object under currentTopKey. | ||
| string memory key = string.concat("$.", chainId); | ||
| string[] memory contractNames = vm.parseJsonKeys(jsonData, key); | ||
| for (uint256 j = 0; j < contractNames.length; j++) { | ||
| string memory contractName = contractNames[j]; | ||
|
|
||
| // Build the JSON path: e.g. ".10.Guardian" | ||
| string memory path = string.concat(".", chainId, ".", contractName); | ||
| address foundAddress = vm.parseJsonAddress(jsonData, path); | ||
|
|
||
| if (foundAddress == target) { | ||
| // If the contract name ends with "Proxy", strip it. | ||
| if (contractName.endsWith("Proxy")) { | ||
| contractName = contractName.slice(0, bytes(contractName).length - 5); | ||
| } | ||
| // If the contract name is "OptimismPortal", change it to "OptimismPortal2". | ||
| if (contractName.eq("OptimismPortal")) { | ||
| contractName = "OptimismPortal2"; | ||
| } | ||
| // We make a call to see if the contract is a Safe. | ||
| if (isGnosisSafe(target)) { | ||
| contractName = string.concat(contractName, " (GnosisSafe)"); | ||
| } | ||
|
|
||
| return (vm.parseUint(chainId), contractName); | ||
| } | ||
| } |
There was a problem hiding this comment.
@ElliotFriedman @blmalone @prat-gpt when we implement this decoded logging into the new version, we can replace this block with a lookup in the address registry using the getAddressInfo method that @prat-gpt added
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Autogenerates decoded state diffs to the terminal, requires the change in ethereum-optimism/optimism#14378 which is why the submodule is updated. Functionality works but doesn't decode mappings yet
This will also be easy to port to the new superchain-ops approach