Suggestions for #140 #145
Merged
Suggestions for #140 #145
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
Contributor
nategraf
commented
Jun 7, 2024
nategraf
commented
- rename DeployEstopVerifier and change vm.startBroadcast() and vm.broadcast()
- move common functions to base contract
- simulate scheduled calls to avoid finding out they'll fail later
- add a codeblock language
- add --broadcast flag
- fix simulation, error on unset env var
|
@intoverflow feel free to press the merge button if these suggestions look good to you. |
intoverflow
reviewed
Jun 11, 2024
Comment on lines
+98
to
+105
| /// @notice Simulates a call to check if it will succeed, given the current EVM state. | ||
| function simulate(address dest, bytes memory data) internal { | ||
| uint256 snapshot = vm.snapshot(); | ||
| vm.prank(address(timelockController())); | ||
| (bool success,) = dest.call(data); | ||
| require(success, "simulation of transaction to schedule failed"); | ||
| vm.revertTo(snapshot); | ||
| } |
|
This is terrific. I've tested the entire flow against Anvil and it works as advertised. I've also confirmed that every scheduled update gets simulated (using vm.prank()) before being scheduled, and that the |
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.