fix: minor improvements to empty sequencer block handling#1814
Merged
mslipper merged 6 commits intoethereum-optimism:developfrom Dec 2, 2021
Merged
fix: minor improvements to empty sequencer block handling#1814mslipper merged 6 commits intoethereum-optimism:developfrom
mslipper merged 6 commits intoethereum-optimism:developfrom
Conversation
🦋 Changeset detectedLatest commit: 5febe10 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
Contributor
|
To prevent broken unit tests, sometimes we hide changes behind |
cfromknecht
force-pushed
the
deadlock-followup
branch
3 times, most recently
from
cd86746 to
59580b2
Compare
1 task
cfromknecht
requested review from
mslipper and
tynes
and removed request for
tynes
tynes
reviewed
Nov 29, 2021
Contributor
tynes
left a comment
tynes
left a comment
There was a problem hiding this comment.
Will approve when a changeset is added, then we can get this out on kovan
Contributor
Author
|
@tynes changeset added 👍 |
mslipper
approved these changes
Dec 2, 2021
When using PoW, geth begins mining on empty blocks before attempting to select which transactions to include. This optimization is not needed, as we require that every L2 block have exactly one transaction. This commit disables this behavior an modifies the tests accordingly. The test is also expanded to be stricter wrt to testing number of txs and uncles on all blocks.
As of the prior commit, all calls to the `commit` pass a noempty value of true, indicating that the sequencer should not start mining empty blocks. Therefore, we can remove all usage of noempty from the worker.
With empty block mining logic totally removed in the prior commit, worker.commit is now only called with update=true. We can now safely remove the argument and the affected codepaths.
This commit fixes a bug in the way txCount was used for detecting and aborting empty blocks. Previously, txCount was incremented immediately after popping off the new transaction but before it is executed. This was incorrect, as the transaction can still be rejected for nonce/gas errors later on. Instead, we modify the logic only count transactions that are successfully added, and utilize the existing w.current.tcount member to implement the detection.
Prior to this commit, an empty block would be submitted to the taskCh before sanity check whether or not it was empty. This assertion is moved up to fail before submitting the task. It seems the placement of the check before was done to get around the StreamUncle unit test, which would allow commit to submit an empty task before ultimately failing. Now that the test is modified to reflect the expected behavior of the sequencer, this placement is no longer necessary.
cfromknecht
force-pushed
the
deadlock-followup
branch
from
b943cb1 to
5febe10
Compare
theochap
added a commit
that referenced
this pull request
Dec 10, 2025
…g package (#1814) ## Description This PR fixes the package versions of the go e2e test folder. This fix ensures that the kurtosis CI job fails if the `kona-node` fails to start inside kurtosis. This will help preventing breaking the kurtosis deployment (at least at startup). Merging this PR means we'll be using the version of the monorepo from my fork `https://github.com/theochap/optimism`. This will be helpful to add features to the `devnet-sdk` without waiting for approval inside the `ethereum-optimism/optimism` repo. Once we stabilize the `devnet-sdk` we can remove the pointer to the forked version of the node.
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
3 participants
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.
Description
This PR makes some minor improvements to the sequencers empty block detection, specifically:
commitTransactionsto only count transactions if they are successfully executedcommitTransactionswhere an empty block can be produced when the execution is interruptedcommit, to avoid sending empty blocks onw.taskChNote, the empty block production in this case was never accepted by the chain, but this improves our detection of these cases locally instead of relying on the catch-all check in
commit.Metadata