MTCannon: Fix AssertEVMReverts to correctly construct data#12200
Merged
Inphi merged 11 commits intoethereum-optimism:developfrom Oct 8, 2024
Merged
MTCannon: Fix AssertEVMReverts to correctly construct data#12200Inphi merged 11 commits intoethereum-optimism:developfrom
AssertEVMReverts to correctly construct data#12200Inphi merged 11 commits intoethereum-optimism:developfrom
Conversation
mbaxter
suggested changes
Sep 30, 2024
Contributor
mbaxter
left a comment
There was a problem hiding this comment.
Thanks for raising this issue - great find!
2. Added `ThreadProofEncoder` helper function to generate `ThreadProof`. 3. Updated `AssertEVMReverts` to include `statePCs` for multiple memory requirements, added `ProofData` parameter for passing `ThreadProof`, and introduced `expectedReason` parameter for more precise testing. 4. Revised `TestEVMFault` and `TestEVM_UnsupportedSyscall` test functions to be compatible with `AssertEVMReverts`.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #12200 +/- ##
===========================================
+ Coverage 64.10% 64.11% +0.01%
===========================================
Files 52 52
Lines 4312 4345 +33
===========================================
+ Hits 2764 2786 +22
- Misses 1374 1385 +11
Partials 174 174
Flags with carried forward coverage won't be shown. Click here to find out more.
|
mbaxter
reviewed
Oct 4, 2024
mbaxter
reviewed
Oct 7, 2024
Contributor
mbaxter
left a comment
There was a problem hiding this comment.
Looks good!! Just a few last comments to tidy things up a bit and looks like there's a merge conflict that needs to be resolved.
Nice catch! Co-authored-by: mbaxter <meredith.a.baxter@gmail.com>
mbaxter
approved these changes
Oct 7, 2024
Inphi
reviewed
Oct 8, 2024
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
First part of #12198
WithRegistersfor initializing state'sRegisters.ThreadProofEncoderhelper function to generateThreadProof.AssertEVMRevertsto includestatePCsfor multiple memory requirements, addedProofDataparameter for passingThreadProof, and introducedexpectedReasonparameter for more precise testing.TestEVMFaultandTestEVM_UnsupportedSyscalltest functions to be compatible withAssertEVMReverts.illegal instructionthat needsSecondMemoryProof