Adding min/max Canonical Tx Chain Batch Calldata #262

willmeister · 2020-09-17T15:03:32Z

Description

Changes Canonical Transaction Chain Batch Creator config from accepting a min / max number of transactions to accepting a min / max number of transaction calldata bytes. This allows it to control the ultimate size of the L1 tx to be posted.

Also fixes:

Issue where L1 block subscription does not upsert re-orgs when it should
Issue where microservices will not start if only running L1 and/or L2 chain data persisters and no consumers

Metadata

Fixes

Fixes # YAS-677

Contributing Agreement

I have read and understood the Optimism Contributing Guide and Code of Conduct and am following those guidelines in this pull request.

…max rollup calldata size instead of number of transactions

tynes

Overall makes sense, left a few comments for you

tynes · 2020-09-17T17:16:18Z

packages/rollup-core/src/app/data/data-service.ts

+    maxBatchCalldataBytes: number
  ): Promise<number> {
    // TODO: ADD SOME SIZE LIMIT
    const txRes = await this.rdb.select(


Verifying that I understand this change: calldata is stored in the db with the 0x prefix, so LENGTH(calldata)-2 accounts for the prefix. The division by 2 here: (LENGTH(calldata)-2) / 2 is converting from string size to byte size. Is there any chance that an odd size of calldata could end up in the database? I saw that ethers sometimes uses 0x0 as 0, the chainId is returned as a single character following 0x. That is a different codepath but noting it here just in case. Empty calldata should be 0x, if LENGTH(calldata) is 0 then there would be a bug here.

The SUM statement will add the calldata size (in bytes) of the 2 earliest transactions (ORDER BY block_timestamp ASC)

calldata is stored in the db with the 0x prefix, so LENGTH(calldata)-2 accounts for the prefix

Yep!

The division by 2 here: (LENGTH(calldata)-2) / 2 is converting from string size to byte size.

Yep!

Is there any chance that an odd size of calldata could end up in the database? I saw that ethers sometimes uses 0x0 as 0, the chainId is returned as a single character following 0x.

Yes and no. Yes, bad data can get into the DB (at which point we have way bigger problems -- we commit fraud), but no, calldata is always bytes, and bytes are always an even number of hex chars. Shortcuts used in JSON representation of some fields should not apply here.

Empty calldata should be 0x, if LENGTH(calldata) is 0 then there would be a bug here.

Even empty calldata should be 0x in the DB, but I'll make it GREATEST(LENGTH(calldata)-2, 0) just in case.

The SUM statement will add the calldata size (in bytes) of the 2 earliest transactions (ORDER BY block_timestamp ASC)

This is the block timestamp of our L2 node, which we update infrequently, so there will be many txs with the same block timestamp. Two txs with different timestamps cannot be in the same rollup batch, though. So if this query returns 2 results, we know the first batch cannot get any bigger and must be submitted. Regardless, this query is summing up all of the calldata of all of the txs that could possibly be in each batch (divided by block timestamp).

packages/rollup-core/src/app/data/data-service.ts

tynes · 2020-09-17T17:22:06Z

packages/rollup-core/src/app/data/data-service.ts

+    let lastId = -1
+    for (const row of res) {
+      totalCalldataBytes += parseInt(row['calldata_bytes'], 10)
+      if (totalCalldataBytes > maxBatchCalldataBytes) {


This codepath wouldn't be impacted from a ton of transactions being included in a batch? The batch size would be selected by the sequencer? So users submitting tons of txs wouldn't be able to trigger this?

It's possible that we'd handle a ton of txs with the same block timestamp, at which point this collection would be very large, but we'll eagerly exit this loop as soon as our max gets hit, so we should be good. Might be good to add an arbitrary [but large] limit to the query, though, just in case we have a burst of txs with 0 calldata.

Worth noting, I created a query that does this calc in the query itself, but it's slow and would get slower as more records get added (stupid CROSS JOIN 😆 )

tynes

Might be good to add an arbitrary [but large] limit to the query, though, just in case we have a burst of txs with 0 calldata.

This sounds like a nice addition and would be a good safety precaution. If you want to add it, I'd be on board with it.

Looks good to me 23884b2

… size

…y L1 and/or L2 chain data persisters are configured to run. Updating documentation / comments

Co-authored-by: Mark Tyneway <[email protected]>

* feat: add shared lockbox (#126) * feat: create shared lockbox contract with its interface and unit tests * chore: polish tests and interfaces * chore: run pre-pr * chore: improve natspec * chore: run pre-pr * chore: update compiler version * feat: integrate portal to lockbox (#139) * feat: integrate portal to lockbox * fix: pr fixes * test: refactor assert * feat: add liquidity migrator contract with its unit test and interface (#128) * feat: create shared lockbox contract with its interface and unit tests * chore: polish tests and interfaces * chore: run pre-pr * chore: improve natspec * chore: run pre-pr * feat: add liqudity migrator contract with its unit test and interface * chore: remove underscore on stack var * chore: add todo * chore: run pre-pr * chore: add contract title natspec and proxied * refactor: integrate testing suite with common test * chore: pre-pr * chore: add spec test * feat: integrate system config with superchain config (#140) * feat: integrate portal to lockbox * fix: pr fixes * test: refactor assert * feat: integrate system config with superchain config * fix: remove OPCM interop * test: add dependency counter test * feat: manage dependency set on superchain config (#138) * chore: add zero dependencies check (#142) * fix: pre pr * feat: Add pause check (#145) * feat: Add pause check Co-authored-by: 0xParticle <[email protected]> Co-authored-by: gotzenx <[email protected]> Co-authored-by: Joxess <[email protected]> * test: add tests natspecs --------- Co-authored-by: 0xParticle <[email protected]> Co-authored-by: gotzenx <[email protected]> Co-authored-by: Joxess <[email protected]> * fix: pre pr and interfaces imports * feat: add upgrader role to superchain config (#163) * feat: use superchain config lockbox in portal (#164) * feat: use superchain config lockbox in portal * test: add new sharedlockbox test * fix: pre pr * feat: liquidity migrator deployment (#166) * feat: liquidity migrator deployment * test: fix comment * test: fix internal variables names * feat: dependency set refactor (#170) * feat: dependency set refactor * fix: deploy script variable name * fix: pr * fix: pr * fix: pre pr * fix: semgrep * fix: merge conflict * [WIP] feat: new lockbox (#192) * chore: partial implementation comments * feat: new lockbox * feat: introduce dependency manager predeploy * feat: remove timestamp check from CrossL2Inbox * feat: introduce cluster manager role and remove immutables * fix: remove unnecessary code, fix tests and setup * feat: use unstructured storage and OZ v5 * fix: L2ToL2CDM dependency set check * fix: dependency manager gas limit * feat: refactor interop feature contracts (#200) * feat: refactor interop feature contracts * fix: add noops comment * feat: adds OptimismPortal migrated flag * test: add missing tests * fix: portal interop storage naming --------- Co-authored-by: Skeletor Spaceman <[email protected]> * fix: pre pr, setup and tests * fix: remove system config interop and add interop portal target check (#205) * fix: remove system config interop and add interop portal check * fix: interop portal target check order * fix: remove wrong comment * fix: refactor portal noops function (#206) * test: add dependency manager and portal interop tests (#209) * feat: initialize shared lockbox in interop portal (#211) * feat: initialize shared lockbox in interop portal * fix: refactor shared lockbox storage getter * fix: lockbox pr fixes (#214) * fix: pre pr * fix: semver lock * fix: semver lock * feat: descope dependency manager (#242) * feat: descope dependency manager * test: fix tests * test: fix tests * chore: improve eth liquidity test (#248) * fix: internal review fixes (#243) * fix: I-0 * fix: I-1 * fix: I-2 * fix: I-3 * fix: I-6 * fix: I-7 * fix: I-9 * fix: pre pr * fix: pre pr * fix: portal withdrawal checks (#255) * fix: portal withdrawal checks * fix: include current withdrawal check * fix: remove unused interop contracts (#256) * test: fix flake tests (#257) * fix: adjust op-deployer interop scripts (#262) * fix: pre pr * fix op-deployer tests * remove dependency code * fix lint * use Bob account --------- Co-authored-by: Disco <[email protected]> Co-authored-by: AgusDuha <[email protected]> Co-authored-by: agusduha <[email protected]> Co-authored-by: 0xParticle <[email protected]> Co-authored-by: gotzenx <[email protected]> Co-authored-by: Joxess <[email protected]> Co-authored-by: Skeletor Spaceman <[email protected]>

We advise this restriction to be imposed solely when the implementation is being set to an _alive status of true, ensuring that a "deleted" implementation (such as an EIP-7702 delegated address) can still be removed.

Closes #262 --------- Co-authored-by: Matthias Seitz <[email protected]>

changing canonical tx chain batch building config to specify a min / …

1ac397c

…max rollup calldata size instead of number of transactions

tynes reviewed Sep 17, 2020

View reviewed changes

PR Feedback

23884b2

tynes approved these changes Sep 17, 2020

View reviewed changes

willmeister added 3 commits September 17, 2020 17:17

PR feedback and testing changes

a2764d5

exactly calculating rollup tx size instead of just using its calldata…

46de2ef

… size

fixing issue in services.ts where microservices will not start if onl…

6cbbe56

…y L1 and/or L2 chain data persisters are configured to run. Updating documentation / comments

willmeister merged commit 753e2cd into master Sep 18, 2020

willmeister deleted the YAS-677/RollupBlockCalldataLimit branch September 18, 2020 15:08

snario pushed a commit that referenced this pull request Apr 14, 2021

Tiny cleanup to SafetyChecker (#262)

8b4a26a

Co-authored-by: Mark Tyneway <[email protected]>

mslipper pushed a commit that referenced this pull request Feb 6, 2025

fix: adjust op-deployer interop scripts (#262)

2b0abfe

xibao-nr pushed a commit to node-real/combo-optimism that referenced this pull request Feb 17, 2025

doc: add eip 7702 item to v0.5.2 changelog (ethereum-optimism#262)

b5671a9

Zena-park added a commit to tokamak-network/optimism that referenced this pull request Dec 30, 2025

[DAP-01M] Improper Imposition of Restriction ethereum-optimism#262

e61105d

theochap pushed a commit that referenced this pull request Jan 15, 2026

feat: add serde for OpTxType (#317)

c8ba171

Closes #262 --------- Co-authored-by: Matthias Seitz <[email protected]>

theochap pushed a commit that referenced this pull request Jan 21, 2026

feat: add serde for OpTxType (alloy-rs/op-alloy#317)

1495ab8

Closes #262 --------- Co-authored-by: Matthias Seitz <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding min/max Canonical Tx Chain Batch Calldata #262

Adding min/max Canonical Tx Chain Batch Calldata #262

Uh oh!

willmeister commented Sep 17, 2020 •

edited

Loading

Uh oh!

tynes left a comment

Uh oh!

tynes Sep 17, 2020

Uh oh!

willmeister Sep 17, 2020 •

edited

Loading

Uh oh!

Uh oh!

tynes Sep 17, 2020

Uh oh!

willmeister Sep 17, 2020

Uh oh!

willmeister Sep 17, 2020

Uh oh!

tynes left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Adding min/max Canonical Tx Chain Batch Calldata #262

Adding min/max Canonical Tx Chain Batch Calldata #262

Uh oh!

Conversation

willmeister commented Sep 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Metadata

Fixes

Contributing Agreement

Uh oh!

tynes left a comment

Choose a reason for hiding this comment

Uh oh!

tynes Sep 17, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Sep 17, 2020 • 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!

Uh oh!

tynes Sep 17, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Sep 17, 2020

Choose a reason for hiding this comment

Uh oh!

willmeister Sep 17, 2020

Choose a reason for hiding this comment

Uh oh!

tynes left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

willmeister commented Sep 17, 2020 •

edited

Loading

willmeister Sep 17, 2020 •

edited

Loading