Skip to content
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

catch step panic in Guard #12202

Merged
merged 3 commits into from
Feb 18, 2025
Merged

Conversation

zhiqiangxu
Copy link
Contributor

@zhiqiangxu zhiqiangxu commented Sep 30, 2024

It seems the error return value of vm.Step is always nil, the exceptional cases are handled by panic directly(e.g., HandleBranch/HandleJump/HandleRd).

So what actually needs to be guarded is the panic.

Copy link

codecov bot commented Sep 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.97%. Comparing base (d340262) to head (c64d07c).
Report is 1043 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #12202      +/-   ##
===========================================
- Coverage    75.19%   74.97%   -0.22%     
===========================================
  Files           49       49              
  Lines         3656     3656              
===========================================
- Hits          2749     2741       -8     
- Misses         734      743       +9     
+ Partials       173      172       -1     
Flag Coverage Δ
cannon-go-tests 74.97% <ø> (-0.22%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@protolambda protolambda requested a review from Inphi December 16, 2024 12:30
@protolambda protolambda added T-proofs Team: Proofs A-cannon Area: cannon A-op-program Area: op-program labels Feb 5, 2025
@Inphi
Copy link
Contributor

Inphi commented Feb 5, 2025

@zhiqiangxu Please rebase this on develop so CI checks can pass.

@zhiqiangxu
Copy link
Contributor Author

@zhiqiangxu Please rebase this on develop so CI checks can pass.

Done in 76bc53f.

@Inphi
Copy link
Contributor

Inphi commented Feb 13, 2025

/ci authorize 76bc53f

@Inphi Inphi enabled auto-merge February 13, 2025 20:42
@zhiqiangxu
Copy link
Contributor Author

@Inphi , looks like there's a random CI failure irrelevant to this PR..

@Inphi
Copy link
Contributor

Inphi commented Feb 14, 2025

@zhiqiangxu Could you please do another rebase on develop? The tests seem flaky and I've repeatedly re-ran the tests to no avail. Perhaps the flakes are fixed on develop.

@zhiqiangxu
Copy link
Contributor Author

@zhiqiangxu Could you please do another rebase on develop? The tests seem flaky and I've repeatedly re-ran the tests to no avail. Perhaps the flakes are fixed on develop.

Done.

@Inphi
Copy link
Contributor

Inphi commented Feb 18, 2025

/ci authorize 6458246

@Inphi Inphi added this pull request to the merge queue Feb 18, 2025
Merged via the queue into ethereum-optimism:develop with commit 03b66de Feb 18, 2025
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-cannon Area: cannon A-op-program Area: op-program T-proofs Team: Proofs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants