feat: raw transaction batch submitter by karlfloersch · Pull Request #527 · ethereum-optimism/optimism

karlfloersch · 2021-04-21T00:00:48Z

Description
Re-introduce raw tx based batch submitter first implemented here: ethereum-optimism/batch-submitter#56

Additional context
This contributes to #496

changeset-bot · 2021-04-21T00:00:51Z

🦋 Changeset detected

Latest commit: 2a17bcb

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

This PR includes changesets to release 2 packages

Name	Type
@eth-optimism/batch-submitter	Minor
@eth-optimism/core-utils	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

Fix tests Add chainset

tynes · 2021-04-21T00:47:42Z

packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts

@@ -202,9 +202,10 @@ export class StateBatchSubmitter extends BatchSubmitter {
      [...Array(blockRange).keys()],


If blockRange is negative, an error will be thrown

that should never happen since we do this check in _getBatchStartAndEnd:

optimism/packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts

Lines 131 to 140 in 8c825d4

if (startBlock >= endBlock) {

if (startBlock > endBlock) {

this.log.error(

'State commitment chain is larger than transaction chain. This should never happen!'

)

}

this.log.info(

'No state commitments to submit. Skipping batch submission...'

)

return

packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

packages/batch-submitter/src/utils.ts

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

annieke

reviewed this with the original PR, everything else looks good!

annieke · 2021-04-21T18:21:01Z

packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts

@@ -202,9 +202,10 @@ export class StateBatchSubmitter extends BatchSubmitter {
      [...Array(blockRange).keys()],


that should never happen since we do this check in _getBatchStartAndEnd:

optimism/packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts

Lines 131 to 140 in 8c825d4

if (startBlock >= endBlock) {

if (startBlock > endBlock) {

this.log.error(

'State commitment chain is larger than transaction chain. This should never happen!'

)

}

this.log.info(

'No state commitments to submit. Skipping batch submission...'

)

return

packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

packages/batch-submitter/src/utils.ts

annieke · 2021-04-21T19:05:48Z

packages/batch-submitter/test/batch-submitter/mockchain-provider.ts

+      case 'eth_getBlockByNumber':
+        if (params.length === 0) {
+          throw new Error(`Invalid params for ${endpoint}`)
+        }
+        const blockNumber = BigNumber.from((params as any)[0]).toNumber()
+        return this.mockBlocks[blockNumber]


we might be able to remove this if injectL2Context lets us call getBlockWithTransactions

Mockchain provider refactor is going to be a beast -- thinking that we should just lump that into the big testing overhaul

…r.ts Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

tynes · 2021-04-21T23:54:29Z

Who owns pushing this over the finishline? @karlfloersch @annieke

karlfloersch · 2021-04-22T01:08:12Z

Who owns pushing this over the finishline? @karlfloersch @annieke

@tynes I'll own it

Update: Looking for re-review! @gakonst @annieke

tynes · 2021-04-22T02:10:18Z

packages/batch-submitter/src/types.ts

-/* Internal Imports */
-import { EIP155TxData, TxType } from '@eth-optimism/core-utils'
-
 export enum QueueOrigin {


Potentially out of scope of this PR but this enum should be moved to core-utils

I notice there is another change to core-utils as part of this PR

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

tynes · 2021-04-22T02:15:06Z

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

-  QueueOrigin,
-  queueOriginPlainText,
-} from '..'
+import { L2Block, BatchElement, Batch, QueueOrigin } from '..'


These types could all go in core-utils, not opinionated on if its as part of this PR or not

Agree they should definitely go into core-utils. That said there's a bunch of type & utility cleanup that we could do and so I'd lean against doing a potentially big refactor in the same PR that we're adding this new feature.

^happy to help with the utility cleanup!

🙏🙏🙏

tynes · 2021-04-22T02:16:13Z

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

@@ -1,20 +1,13 @@
 /* External Imports */
 import { Promise as bPromise } from 'bluebird'


What do you think of removing bluebird as a dependency in the future? Concurrent async calls are easy with Promise.all

I had the same thought when I read that code before, here's a post providing good reasoning on "why bluebird" https://blog.kuzzle.io/bluebird-native-async_await-javascript-promises-performances-2020

.changeset/sharp-worms-battle.md

tynes

The big one is adding the changeset for core-utils

gakonst

Code changes LGTM - assuming they are not breaking Kovan or Mainnet.

gakonst · 2021-04-22T09:03:08Z

.changeset/witty-buses-worry.md

+"@eth-optimism/core-utils": patch
+---
+
+- Use raw transaction in batch submitter -- incompatible with L2Geth v0.1.2.1


What is geth 0.1.2.1? Is it deployed / being used anywhere right now?

Ah it is the version of Geth that we were running before our last regenesis ~2/3 weeks ago. This should not be breaking Kovan or Mainnet now that we've got rawTransaction in mainnet

annieke

looks good! yay for deleted code, only concern is whether isSequencerTx is set correctly

annieke · 2021-04-22T16:17:33Z

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

-  QueueOrigin,
-  queueOriginPlainText,
-} from '..'
+import { L2Block, BatchElement, Batch, QueueOrigin } from '..'


^happy to help with the utility cleanup!

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts

annieke · 2021-04-22T16:19:11Z

packages/batch-submitter/test/batch-submitter/mockchain-provider.ts

+      case 'eth_getBlockByNumber':
+        if (params.length === 0) {
+          throw new Error(`Invalid params for ${endpoint}`)
+        }
+        const blockNumber = BigNumber.from((params as any)[0]).toNumber()
+        return this.mockBlocks[blockNumber]


…r.ts Co-authored-by: Annie Ke <annieke8@gmail.com>

tynes

Looks good to me!

* feat: use raw transaction Fix tests Add chainset * Update packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts Co-authored-by: Georgios Konstantopoulos <me@gakonst.com> * Remove raw eth_getBlock, delete dead code, & lint * Remove txTypePlaintext and queueOriginPlaintext * style: return promise; no await * Update changeset & fix lint error * Remove stray newline * Update packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts Co-authored-by: Annie Ke <annieke8@gmail.com> Co-authored-by: Annie Ke <annie@address.com> Co-authored-by: Georgios Konstantopoulos <me@gakonst.com> Co-authored-by: Annie Ke <annieke8@gmail.com>

chore(cgt): sync develop

these will allow us to offload this impl entirely https://github.com/paradigmxyz/reth/blob/4df1425fcf860572331c682cf868d0a94129536f/crates/optimism/rpc/src/eth/transaction.rs#L93-L97 ref paradigmxyz/reth#16451

Closes #527

karlfloersch requested a review from annieke as a code owner April 21, 2021 00:00

feat: use raw transaction

8c825d4

Fix tests Add chainset

karlfloersch force-pushed the feat/raw_transaction_batch_submitter branch from f146a12 to 8c825d4 Compare April 21, 2021 00:08

tynes reviewed Apr 21, 2021

View reviewed changes

packages/batch-submitter/src/batch-submitter/state-batch-submitter.ts Outdated Show resolved Hide resolved

gakonst suggested changes Apr 21, 2021

View reviewed changes

annieke suggested changes Apr 21, 2021

View reviewed changes

Update packages/batch-submitter/src/batch-submitter/tx-batch-submitte…

a56bf23

…r.ts Co-authored-by: Georgios Konstantopoulos <me@gakonst.com>

Remove raw eth_getBlock, delete dead code, & lint

181efea

karlfloersch requested review from ben-chain and smartcontracts as code owners April 22, 2021 00:02

Remove txTypePlaintext and queueOriginPlaintext

f43dd4f

karlfloersch force-pushed the feat/raw_transaction_batch_submitter branch from ecd05f8 to f43dd4f Compare April 22, 2021 01:09

tynes reviewed Apr 22, 2021

View reviewed changes

packages/batch-submitter/src/batch-submitter/tx-batch-submitter.ts Outdated Show resolved Hide resolved

tynes reviewed Apr 22, 2021

View reviewed changes

.changeset/sharp-worms-battle.md Outdated Show resolved Hide resolved

tynes requested changes Apr 22, 2021

View reviewed changes

style: return promise; no await

7e9babf

karlfloersch force-pushed the feat/raw_transaction_batch_submitter branch 2 times, most recently from edeaf80 to 5f68d9a Compare April 22, 2021 04:31

Update changeset & fix lint error

fc22647

karlfloersch force-pushed the feat/raw_transaction_batch_submitter branch from 5f68d9a to fc22647 Compare April 22, 2021 04:46

Remove stray newline

c40b00b

gakonst approved these changes Apr 22, 2021

View reviewed changes

annieke approved these changes Apr 22, 2021

View reviewed changes

Update packages/batch-submitter/src/batch-submitter/tx-batch-submitte…

2a17bcb

…r.ts Co-authored-by: Annie Ke <annieke8@gmail.com>

tynes approved these changes Apr 22, 2021

View reviewed changes

karlfloersch merged commit 5077441 into master Apr 22, 2021

karlfloersch deleted the feat/raw_transaction_batch_submitter branch April 22, 2021 18:30

annieke mentioned this pull request Apr 24, 2021

[cleanup] batch submitter unused code + types to core-utils #614

Merged

OptimismBot pushed a commit that referenced this pull request Sep 4, 2025

Merge pull request #527 from defi-wonderland/chore/cgt-sync-develop

1026df6

chore(cgt): sync develop

emhane pushed a commit that referenced this pull request Feb 4, 2026

chore(trie): replace eyre with OpProofsStorageError (#564)

7ab8246

Closes #527

		@@ -202,9 +202,10 @@ export class StateBatchSubmitter extends BatchSubmitter {
		[...Array(blockRange).keys()],

	if (startBlock >= endBlock) {
	if (startBlock > endBlock) {
	this.log.error(
	'State commitment chain is larger than transaction chain. This should never happen!'
	)
	}
	this.log.info(
	'No state commitments to submit. Skipping batch submission...'
	)
	return

		@@ -1,20 +1,13 @@
		/* External Imports */
		import { Promise as bPromise } from 'bluebird'

Comments

Conversation

karlfloersch commented Apr 21, 2021

Uh oh!

changeset-bot bot commented Apr 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

annieke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes commented Apr 21, 2021

Uh oh!

karlfloersch commented Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

karlfloersch Apr 22, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tynes left a comment

Choose a reason for hiding this comment

Uh oh!

gakonst left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

annieke left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tynes left a comment

Choose a reason for hiding this comment

changeset-bot bot commented Apr 21, 2021 •

edited

Loading

karlfloersch commented Apr 22, 2021 •

edited

Loading

karlfloersch Apr 22, 2021 •

edited

Loading