-
Notifications
You must be signed in to change notification settings - Fork 39
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
Enclave: no panic for bad rlp format #1679
Conversation
WalkthroughThe changes involve enhancing error handling and logging within the Changes
TipsChat with CodeRabbit Bot (
|
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.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- go/enclave/rpc_server.go (2 hunks)
Additional comments: 4
go/enclave/rpc_server.go (4)
110-123: The changes in the
SubmitL1Block
method correctly implement error handling for the decoding of blocks and receipts, logging errors instead of causing a panic, which aligns with the PR's objective to improve robustness and prevent system outages.450-456: The
decodeBlock
method has been correctly updated to return an error along with the block, which allows for proper error handling in theSubmitL1Block
method.459-468: The
decodeReceipts
method has been correctly updated to return an error along with the receipts, which allows for proper error handling in theSubmitL1Block
method.447-448: The
EnclavePublicConfig
method does not include any changes related to error handling or logging, which is inconsistent with the PR's objective to improve robustness by preventing panics due to badly formatted requests. Please verify if this method should also include error handling improvements.
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.
lgtm
(cherry picked from commit 4551c81)
Why this change is needed
We don't want to fail for badly formatted requests, we want to log the error and allow chance to recover.
Currently we panic and this recently took down the sepolia testnet. (Investigation of the root cause of that is ongoing).
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks