op-node: fix invalid payload insertion error handling#3180
Merged
Conversation
|
Contributor
|
This PR changes implementation code, but doesn't include a changeset. Did you forget to add one? |
mslipper
approved these changes
Aug 5, 2022
Contributor
mslipper
left a comment
There was a problem hiding this comment.
LGTM, just fix the lint error.
roninjin10
pushed a commit
that referenced
this pull request
Aug 26, 2022
* op-node: fix invalid payload insertion error handling * fix goimports Co-authored-by: Matthew Slipper <me@matthewslipper.com>
maurelian
pushed a commit
that referenced
this pull request
Sep 15, 2022
* op-node: fix invalid payload insertion error handling * fix goimports Co-authored-by: Matthew Slipper <me@matthewslipper.com>
sam-goldman
pushed a commit
that referenced
this pull request
Sep 15, 2022
* op-node: fix invalid payload insertion error handling * fix goimports Co-authored-by: Matthew Slipper <me@matthewslipper.com>
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
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.
This PR relies on ethereum-optimism/reference-optimistic-geth#40 to return
INVALIDwhen the payload attributes cannot be processed.When payload attributes cannot be processed due to a true payload-error (i.e. rpc errors, syncing state, etc. don't count) then we first re-attempt with only the deposits, and then return a critical error if we cannot even process a block with just the deposits.
I've chosen this approach, instead of looking for alternative batches, to keep the batch derivation simple (i.e. allow multiple batches to be queued up with the eager-batch-derivation logic, instead of having to check them 1 by 1 with the engine).
When the typed error handling lands we can handle the deposit processing error as critical, and stop the syncing entirely. Safety over liveness, deposits always have to work.
During goerli devnet testing there was a batch with invalid nonce usage, and so we have to drop the user transaction(s) from that batch to continue rollup processing, or we get stuck. This is attributable to the batch-submitter, who should not have included the invalid transaction. So we drop the batch transactions as a whole, instead of finding a single invalid transaction, to keep things simple.