-
Notifications
You must be signed in to change notification settings - Fork 759
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
handle an edge case in newpayload block execution #3131
Conversation
Codecov Report
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. |
// this error can come if the block tries to load a previous block yet not in the chain | ||
// error coding of the evm errors should be a better way to handle this | ||
// | ||
// at Blockchain.getBlock (/home/gajinder/ethereumjs-monorepo/packages/blockchain/src/blockchain.ts:763:15) |
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.
Do we need this entire stack trace in the logs?
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.
if the issue clear can remove the trace
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.
Generally looks good. Do we need a specific test for this edge case or is this covered by existing tests?
this particular edge case might not be covered ... i think we would need to have a tx that does BLOCKHASH opcode, could add this |
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.
I would really really like to get things prepared for a release round tomorrow, at least if things work on a somewhat descent level (just re-running Holesky again right now).
I would therefore merge here, maybe we can add an eventual test later on.
Thanks! 🙏
handle an edgecase noticed during holesky sync