Skip to content

feat: add v0 go batch submitter and gh actions integration#1802

Merged
smartcontracts merged 7 commits intoethereum-optimism:developfrom
cfromknecht:bss-v0
Dec 13, 2021
Merged

feat: add v0 go batch submitter and gh actions integration#1802
smartcontracts merged 7 commits intoethereum-optimism:developfrom
cfromknecht:bss-v0

Conversation

@cfromknecht
Copy link
Contributor

@cfromknecht cfromknecht commented Nov 23, 2021

Description
This PR contains the v0 go batch submitter, which integrates the prior PRs and passes the integration test suite. Currently this version only submits one tx per L2 block, so no batching is implemented yet. Now that a baseline has been established, further optimizations, e.g. batching, EIP-1559 support, etc, will be added once more of the operational concerns are fleshed out

Since there will be some time while both the Typescript and Go batch submitters exist within the codebase, the docker-compose set up has been modified to support selecting either. This allows us to set up a build matrix on Github Actions that concurrently runs two versions of the integration tests, with one testing the Typescript version and one testing the Go version.

Pushing this up now to test out the new changes to the build system and integration tests. The following items are in progress:

  • Improve the robustness and detail of logging
  • Port over existing prometheus metrics
  • Add unit tests for asserting internal driver behavior

Depends On

Metadata

  • Fixes ENG-1484
  • Fixes ENG-1672

@changeset-bot
Copy link

changeset-bot bot commented Nov 23, 2021

⚠️ No Changeset found

Latest commit: fb31230

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

@github-actions github-actions bot added 2-reviewers M-ci Meta: ci related work A-cannon Area: cannon A-ops Area: ops labels Nov 23, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 23, 2021

Codecov Report

Merging #1802 (cb3f9b5) into develop (b9ee2d7) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1802   +/-   ##
========================================
  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 b9ee2d7...cb3f9b5. Read the comment docs.

@cfromknecht cfromknecht force-pushed the bss-v0 branch 2 times, most recently from b123d9f to 2f6b799 Compare November 23, 2021 21:34
Copy link
Contributor

Choose a reason for hiding this comment

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

You are probably aware of this, but when you move to posting more than a single batch element per batch this could end up in an infinite crash loop

Copy link
Contributor

Choose a reason for hiding this comment

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

Was this causing build issues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, pulling in secp256k1 from geth into l2geth caused this dependency to update. unfortunately the newer version had an incompatible API, but the changes were pretty minimal

Copy link
Contributor

Choose a reason for hiding this comment

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

Something that we could do that would be a nice to have to eliminate one extra config option would be to only pass in the address manager address and have the batch submitter resolve both the ctc and scc. It could also resolve OVM_Sequencer and OVM_Proposer to make sure that its own keys match these values, proposer is the entity that has the scc key and sequencer is the entity with the ctc key

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previously we discussed removing the address resolution and passing in the addresses manually. i'm open do doing this if it's what we want, we should get some input @karlfloersch

@cfromknecht cfromknecht force-pushed the bss-v0 branch 2 times, most recently from 8feaae7 to 1c220aa Compare December 13, 2021 17:07
Attempting to compile L1 and L2 get in the same binary produces a
symbole collision since the CGO names for the library methods are
identical. To avoid this, we simply reexport the L1 secp256k1 members
that rely on CGO.
For some reason the log level environment variables aren't being picked
up, and neither is the default value for the flag being applied to the
final binary when run in docker. Remove this commit once a fix is
established.
@smartcontracts smartcontracts merged commit 2b74367 into ethereum-optimism:develop Dec 13, 2021
@cfromknecht cfromknecht deleted the bss-v0 branch December 13, 2021 21:57
theochap pushed a commit that referenced this pull request Dec 10, 2025
## Overview

Fixes the lint job by using the stable toolchain for the `fmt + lint`
job. `cargo fmt` still uses nightly, but clippy uses stable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants