Skip to content

Develop -> Master PR#1853

Merged
tynes merged 27 commits intomasterfrom
develop
Dec 3, 2021
Merged

Develop -> Master PR#1853
tynes merged 27 commits intomasterfrom
develop

Conversation

@tynes
Copy link
Contributor

@tynes tynes commented Dec 3, 2021

Description
Merge Develop into Master

tynes and others added 27 commits November 30, 2021 10:46
Compute the gas used in an epoch based on the
actual amount of gas used instead of assuming
that the full block's worth of gas was used in
each block.
Convert the average block gas limit to a `uint64` in the
codebase instead of it being used as a `float64`.
feat: more accurate gas-oracle gas metering
Updates the github action to properly reference
the `proxyd` dockerfile. The logic is copied from
the canary publish github action.
Slither github action fix and ignore database
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.
fix: minor improvements to empty sequencer block handling
feat: add batch-tx submitter custom encoding and test vectors
@changeset-bot
Copy link

changeset-bot bot commented Dec 3, 2021

🦋 Changeset detected

Latest commit: b9ee2d7

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

This PR includes changesets to release 3 packages
Name Type
@eth-optimism/proxyd Patch
@eth-optimism/gas-oracle Patch
@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

@github-actions github-actions bot added 2-reviewers M-ci Meta: ci related work labels Dec 3, 2021
@codecov-commenter
Copy link

codecov-commenter commented Dec 3, 2021

Codecov Report

Merging #1853 (b9ee2d7) into master (5b4d65d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1853   +/-   ##
=======================================
  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.

Impacted Files Coverage Δ
.../contracts/contracts/standards/L2StandardERC20.sol 100.00% <0.00%> (ø)
...ontracts/contracts/L1/verification/BondManager.sol 100.00% <0.00%> (ø)
...ontracts/contracts/libraries/rlp/Lib_RLPReader.sol 96.87% <0.00%> (ø)
...ntracts/contracts/chugsplash/L1ChugSplashProxy.sol 100.00% <0.00%> (ø)
...tracts/contracts/L1/deployment/AddressDictator.sol 0.00% <0.00%> (ø)
...tracts/contracts/L1/messaging/L1StandardBridge.sol 97.05% <0.00%> (ø)
...tracts/contracts/L2/messaging/L2StandardBridge.sol 95.45% <0.00%> (ø)
...tracts/contracts/libraries/trie/Lib_MerkleTrie.sol 98.30% <0.00%> (ø)
...racts/contracts/L1/rollup/StateCommitmentChain.sol 87.50% <0.00%> (ø)
...acts/contracts/L1/rollup/ChainStorageContainer.sol 70.00% <0.00%> (ø)
... and 10 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 5b4d65d...b9ee2d7. Read the comment docs.

@tynes tynes merged commit e3d68e1 into master Dec 3, 2021
theochap pushed a commit that referenced this pull request Dec 10, 2025
## Overview

Alternative version of #1851, which keeps the `BinaryHeap` priority
queue and achieves the same goal of not starving the engine actor
control loop.
theochap added a commit that referenced this pull request Dec 10, 2025
## Description

This PR adds a test to ensure that nodes sync properly the safe heads
and have a consistent view of the state.
For each kona-node, this test:
- Gathers the node's safe head gossip over 10sec
- Checks that all the other nodes that have seen all these safe heads
have a consistent view with the gossiped safe heads.

For now, we only have positive tests so we're lacking some coverage
(what happens with rogue nodes, network failures, reorgs, ...). These
cases will come once we adapt the integration testing suite to use
`in-memory` mode.

## Note

Adds back the real "large network configuration" to CI now that we have
fixed CPU usage of the kona nodes #1853

## Related issues

Close #1463
theochap added a commit that referenced this pull request Jan 14, 2026
## Description

This PR adds a test to ensure that nodes sync properly the safe heads
and have a consistent view of the state.
For each kona-node, this test:
- Gathers the node's safe head gossip over 10sec
- Checks that all the other nodes that have seen all these safe heads
have a consistent view with the gossiped safe heads.

For now, we only have positive tests so we're lacking some coverage
(what happens with rogue nodes, network failures, reorgs, ...). These
cases will come once we adapt the integration testing suite to use
`in-memory` mode.

## Note

Adds back the real "large network configuration" to CI now that we have
fixed CPU usage of the kona nodes #1853

## Related issues

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

Labels

A-cannon Area: cannon M-ci Meta: ci related work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants