Skip to content

op-node: handle special engine RPC user-input error codes#3319

Merged
mergify[bot] merged 1 commit intodevelopfrom
fix-payload-building
Aug 26, 2022
Merged

op-node: handle special engine RPC user-input error codes#3319
mergify[bot] merged 1 commit intodevelopfrom
fix-payload-building

Conversation

@protolambda
Copy link
Contributor

Description

Testnet bugfix:

  • The testnet was started without the parent-hash checking (op-node/specs: refactor batch queue, add parent_hash to batches, extend testing #3221 )
  • Reorgs happened (on L1 Goerli testnet, 20+ block deep, well past confirmation distance)
  • Frame from data tx was missed again (twice actually). Due to valid reasons: it was just too old, from pre-reorg multiple minutes ago, likely bad L1 data in there too.
  • Batches after that frame were still derived, and not dropped, and insertion into engine was attempted
  • Nonce errors were hit, but this time we hit a different error path: we received back an RPC error code that we were not properly handling. So the nice payload-attributes validity error we got was assumed to be temporary.
  • And then it got stuck retrying those attributes over and over again. At least we have a backoff, but it essentially got stuck due to bad error handling

This PR:

  • Adds the missing error type codes that we need to handle. See https://github.com/ethereum/execution-apis/blob/main/src/engine/specification.md
  • Transforms the RPC error into an InputError so we can safely wrap it without losing the ability to distinguish errors
  • Handles forkchoice-update errors pre-block-insertion as a reason to reset to regain consistency, not a reason to drop sequencer transactions to try to force forward when the forkchoice is incoherent already.
  • Handles input-errors: transactions can be bad, and the payload processing status may not be reached if something is wrong about the input before EVM execution. Nonce values are one example of this.

Depends on #3318 for lint issue in CI.

Metadata

Fix ENG-2695

@changeset-bot
Copy link

changeset-bot bot commented Aug 26, 2022

⚠️ No Changeset found

Latest commit: 5692274

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@mergify
Copy link
Contributor

mergify bot commented Aug 26, 2022

This PR changes implementation code, but doesn't include a changeset. Did you forget to add one?

Copy link
Contributor

@tuxcanfly tuxcanfly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK.

@protolambda
Copy link
Contributor Author

Will rebase to include the linting fix on develop.

@protolambda protolambda force-pushed the fix-payload-building branch 2 times, most recently from 38df26a to d99edba Compare August 26, 2022 03:01
@protolambda
Copy link
Contributor Author

And fixed go-imports issue

@protolambda protolambda force-pushed the fix-payload-building branch from d99edba to 5692274 Compare August 26, 2022 03:03
@mergify
Copy link
Contributor

mergify bot commented Aug 26, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify mergify bot merged commit a1bea61 into develop Aug 26, 2022
@mergify mergify bot deleted the fix-payload-building branch August 26, 2022 03:07
@mergify
Copy link
Contributor

mergify bot commented Aug 26, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants