Skip to content

feat: add batch-tx submitter custom encoding and test vectors#1728

Merged
mslipper merged 5 commits intoethereum-optimism:developfrom
cfromknecht:bss-encoding
Dec 2, 2021
Merged

feat: add batch-tx submitter custom encoding and test vectors#1728
mslipper merged 5 commits intoethereum-optimism:developfrom
cfromknecht:bss-encoding

Conversation

@cfromknecht
Copy link
Contributor

@cfromknecht cfromknecht commented Nov 9, 2021

Description
This PR implements the batch-tx calldata encoding and AppendSequencerBatchParams construction in the go/batch-submitter package. The serialization test vectors are also committed in JSON, which will offer a language-agnostic test suite for the expected behavior captured in the final batch submitter spec.

Additional Context
In order to import both L1 and L2 geth into the same binary, the l2geth package name had to be changed to avoid a naming collision. The first commit modifies the l2geth go.mod name, and updates the entire project's import paths. The batch-submitter's package name is also corrected, as the original had a typo.

Metadata

  • Fixes ENG-1483

@changeset-bot
Copy link

changeset-bot bot commented Nov 9, 2021

🦋 Changeset detected

Latest commit: 272d20d

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 added the A-op-batcher Area: op-batcher label Nov 9, 2021
@codecov-commenter
Copy link

codecov-commenter commented Nov 9, 2021

Codecov Report

Merging #1728 (272d20d) into develop (d4300c8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1728   +/-   ##
========================================
  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...272d20d. Read the comment docs.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should check to make sure there is a tx in the block to prevent a panic here, the genesis block won't have a tx in it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

afaict the genesis block is explicitly skipped by setting BLOCK_OFFSET=1, so we should never process it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Confirming that this is an empty transaction:

$ rlp decode 0xc9808080808080808080
[
  '', '', '', '', '',
  '', '', '', ''
]

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, i just used the most minimal txn possible for the test vectors. we could add more complicated ones, plus invalid ones for negative test vectors.

Copy link
Contributor

@tynes tynes left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, #1802 will be helpful to ensure correctness in the integration tests

@tynes
Copy link
Contributor

tynes commented Nov 29, 2021

Can you add changesets?

@cfromknecht cfromknecht force-pushed the bss-encoding branch 2 times, most recently from da42066 to 8706d07 Compare December 2, 2021 18:14
@mslipper mslipper merged commit b9ee2d7 into ethereum-optimism:develop Dec 2, 2021
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.

4 participants