Skip to content

feat: surface low-level sequencer execution errors, fix itest flakes#1816

Merged
mslipper merged 3 commits intoethereum-optimism:developfrom
cfromknecht:surface-sequencer-execution-errors
Dec 9, 2021
Merged

feat: surface low-level sequencer execution errors, fix itest flakes#1816
mslipper merged 3 commits intoethereum-optimism:developfrom
cfromknecht:surface-sequencer-execution-errors

Conversation

@cfromknecht
Copy link
Contributor

@cfromknecht cfromknecht commented Nov 25, 2021

Description
After surfacing the errors as part of the deadlock fix, the itests now exhibit a flake caused by these unexpected errors being returned. Internally, the harness detects and reattempts expected errors (currently rejected at the txpool level), however, the specific lower-level execution errors are still not surfaced. To fix the itests, we need to bubble up the specific execution errors (rather than the generic "Cannot commit transaction in miner") to the rpc caller, which will allow the test harness to proceed as usual.

Builds on:

Metadata

@changeset-bot
Copy link

changeset-bot bot commented Nov 25, 2021

🦋 Changeset detected

Latest commit: c62c486

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@eth-optimism/l2geth Patch

Not sure what this means? Click here to learn what changesets are.

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

@cfromknecht cfromknecht changed the title feat: surface low-level sequencer execution errors, fix itests flakes feat: surface low-level sequencer execution errors, fix itest flakes Nov 25, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 25, 2021

Codecov Report

Merging #1816 (6e7edc6) into develop (d4300c8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1816   +/-   ##
========================================
  Coverage    71.99%   71.99%           
========================================
  Files           70       70           
  Lines         2321     2321           
  Branches       346      346           
========================================
  Hits          1671     1671           
  Misses         650      650           
Flag Coverage Δ
batch-submitter 62.07% <ø> (ø)
contracts 87.96% <ø> (ø)
core-utils 57.50% <ø> (ø)
data-transport-layer 38.64% <ø> (ø)
message-relayer 70.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4300c8...6e7edc6. Read the comment docs.

@mslipper
Copy link
Collaborator

Mind walking us through these changes during tomorrow's infra sync?

@cfromknecht cfromknecht force-pushed the surface-sequencer-execution-errors branch from 35bdcb3 to 6e7edc6 Compare December 2, 2021 21:04
@cfromknecht cfromknecht force-pushed the surface-sequencer-execution-errors branch 2 times, most recently from 1006f78 to 7f0c739 Compare December 9, 2021 00:43
@cfromknecht cfromknecht force-pushed the surface-sequencer-execution-errors branch from 7f0c739 to e55b93c Compare December 9, 2021 18:42
@cfromknecht cfromknecht force-pushed the surface-sequencer-execution-errors branch from e55b93c to c62c486 Compare December 9, 2021 18:48
@cfromknecht
Copy link
Contributor Author

@mslipper @tynes PR is ready for merge. ErrNonceTooHigh is mapped to ErrNonceTooLow for now to match the mempool—the mapping will be removed in a follow-up once the change has been communicated to integrators. The follow up should be merged at the same time as #1882.

Copy link
Collaborator

@mslipper mslipper left a comment

Choose a reason for hiding this comment

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

LGTM

@mslipper mslipper merged commit 158743b into ethereum-optimism:develop Dec 9, 2021
theochap pushed a commit that referenced this pull request Dec 10, 2025
## Overview

Forwards derivation reset events to the engine. In the case of the
`HoloceneActivation` reset, the reset can be handled internal to the
pipeline without informing the engine.

Additionally, adds functionality to block the derivation actor while it
is waiting for a signal to be sent back. This will prevent it from
attempting to advance while the engine is resetting.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-cannon Area: cannon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flaky integration test ("Cannot commit transaction in miner")

3 participants