-
Notifications
You must be signed in to change notification settings - Fork 865
skip LastResultsHash check if giga executor is on #2866
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2866 +/- ##
===========================================
+ Coverage 48.32% 57.07% +8.75%
===========================================
Files 671 2090 +1419
Lines 50576 171162 +120586
===========================================
+ Hits 24439 97696 +73257
- Misses 23998 64775 +40777
- Partials 2139 8691 +6552
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| if err != nil { | ||
| // Check if this is a LastResultsHash mismatch and log detailed info | ||
| if !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { | ||
| if !types.SkipLastResultsHashValidation.Load() && !bytes.Equal(block.LastResultsHash, state.LastResultsHash) { |
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.
Does this actually work? I think you're just skipping the log, but still returning the error here, so I don't think SkipLastResultsHashValidation actually does anything.
Describe your changes and provide context
Giga executors may yield different gas usage than v2 executors due to:
Since we only care about state consistency, which is covered by AppHash, we don't need to require lastResultsHash on nodes that enabled giga.
Testing performed to validate your change
patch on giga-enabled RPC nodes