feat: add Teleportr service by cfromknecht · Pull Request #2233 · ethereum-optimism/optimism

cfromknecht · 2022-02-25T02:11:35Z

Description
This PR adds the Teleportr service implemented as a bss-core driver. The general principal is the same the batch submitter, except here we are pulling data from L1 and posting to L2. There are however some notable differences:

The teleportr driver is backed by a Postgres instance for ingesting Deposit events and holding metadata about the progression of the deposit until it is disbursed. The ingestion happens inside each call to Driver.GetBatchBlockRange, and overwrites deposits with the same deposit ID to handle reorgs up to the NumDepositConfirmations window.
L2 doesn't support EIP-1559, so when submitting batch transactions we rely on a call to EstimateGas inside the Driver.UpdateGasPrice callback to set a proper L2 fee rate. Note that this also differs from the prior approach where the fees were linearly scaled between a min and max configuration. This eliminates the need for adding the additional configuration arguments and needing to further modify the bss-core interface to support the old methodology, seeing as we can start using EIP-1559 for Teleportr L2 txs in the near future.
There is no need to support ClearPendingTxs since L2Geth doesn't have a mempool where prior transactions can be stuck.

TODO:

Record disbursement success/fail by inspecting receipt in Postgres
~~Figure out how to deploy Teleportr contracts on itests and address manager~~ (bumped to follow up)
~~Add itests~~ (bumped to followup)

Blocked by:

interface bss-core metrics to allow for driver-specific metrics extensions #2235

Metadata

Fixes ENG-1932

changeset-bot · 2022-02-25T02:11:37Z

🦋 Changeset detected

Latest commit: 2910a3c

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/batch-submitter-service	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

codecov-commenter · 2022-02-25T02:23:02Z

Codecov Report

Merging #2233 (1480c92) into develop (c1619de) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2233   +/-   ##
========================================
  Coverage    73.04%   73.04%           
========================================
  Files           86       86           
  Lines         2846     2846           
  Branches       486      486           
========================================
  Hits          2079     2079           
  Misses         767      767

Flag	Coverage Δ
batch-submitter	`62.63% <ø> (ø)`
contracts	`90.88% <ø> (ø)`
core-utils	`87.80% <ø> (ø)`
data-transport-layer	`36.89% <ø> (ø)`
sdk	`57.71% <ø> (ø)`

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 c1619de...1480c92. Read the comment docs.

go/batch-submitter/.gitignore

go/bss-core/.gitignore

go/teleportr/.gitignore

go/teleportr/drivers/disburser/driver.go

mslipper · 2022-02-25T19:06:23Z

go/teleportr/drivers/disburser/driver.go

+			"blockNumber", receipt.BlockNumber,
+			"blockTimestamp", blockTimestamp)
+
+		err = d.cfg.Database.UpsertDisbursement(


If a disbursement fails to be marked as successful, what happens?

it would show up in the db as confirmed but not disbursed, and would require manual intervention to add the disbursement. the best way to get around this IMO is to add a database that tracks txhash => (startId, endId) and then we can recover after startups or in GetBatchBlockRange. this is a little more involved though, as we would need a call back from within the txmgr before it publishes, but it's definitely the safer route

ah, actually we can just do the tracking in the SendTransaction iface method 😎

mslipper · 2022-02-25T19:08:31Z

go/teleportr/drivers/disburser/driver.go

+		// are missing a disbursement for this deposit even though the contract
+		// beleves it was disbursed.
+		case deposit.ID < startID:
+			log.Warn("Filtering deposit with missing disbursement",


Should have a metric for this as well.

go/teleportr/drivers/disburser/driver.go

tynes · 2022-02-25T20:40:21Z

go/batch-submitter/dial_l2_client.go

+// provided URL. If the dial doesn't complete within dial.DefaultTimeout seconds,
 // this method will return an error.
-func L2EthClientWithTimeout(ctx context.Context, url string, disableHTTP2 bool) (
+func DialL2EthClientWithTimeout(ctx context.Context, url string, disableHTTP2 bool) (


Is there any reason as to why we are using the l2geth deps here? I feel like we could use the upstream stuff for things like rpc or utils, especially now that the http2 fix is in: 6f2ea19

yes, upstream doesn't expose the L2 metadata https://github.com/ethereum-optimism/optimism/blob/develop/go/batch-submitter/drivers/sequencer/batch.go#L53L54

fwiw we do use upstream for L2 in this driver since we don't need to access any non-standard info

tynes · 2022-02-25T20:45:16Z

go/teleportr/drivers/disburser/driver.go

+// stateRootSize is the size in bytes of a state root.
+const stateRootSize = 32
+
+var bigOne = new(big.Int).SetUint64(1) //nolint:unused


Did you mean to leave this even though its unused?

good catch, was left over from copying in from the BSS driver as a template.

go/teleportr/drivers/disburser/driver.go

tynes · 2022-02-25T20:53:53Z

go/teleportr/drivers/disburser/driver.go

+				BlockNumber:    event.Raw.BlockNumber,
+				BlockTimestamp: time.Unix(int64(header.Time), 0),
+				Address:        event.Emitter,
+				Amount:         event.Amount,


There have been hacks in the past where a bridge uses tx.Value instead of a value emitted from an event which could cause problems if the tx calls a loop, it would count the value multiple times. I believe we are safe from this kind of attack but perhaps its good to add a unit test to the contract tests before this goes into production

tynes · 2022-02-25T20:58:25Z

ops/docker-compose.yml

      ENABLE_GAS_REPORT: 1
      NO_NETWORK: 1

+  teleportr_postgres:


Are you planning on adding integration tests for teleportr? Eventually the docker compose setup will become unwieldy for local development using too many resources and we don't have a good way to define sets of integration tests that bring up parts of the stack to specifically test. I think adding teleportr is fine now fwiw

Oh I see that teleportr has replicas: 0 but the postgres does not

good catch, yes i'm planning to add itests but will do so in a follow up. this pr will just have the bulk of the teleportr service

This avoids having l2geth as a dependency for other projects building on bss-core. Speicifically this avoids having to copy l2geth into docker builds.

Less casting.

Currently the teleportr database supports a LatestDeposit method, which returns the highest block number observed, if any. The intent was to use this as the starting point for syncing, however, this isn't super useful as there may be long periods of inactivity that we have already scanned. Instead, we now store the last processed block in a separate table, and pass the end of the ingestion block range as argument to UpsertDeposits. The list of deposits and last processed block are written atomically to avoid consistency issues. The value can be retrieved using the LastProcessedBlock getter.

cfromknecht · 2022-03-02T21:26:40Z

Pushed a small change to record success/failures for every database operation

## Overview Small cleanup to the `RollupNodeService::start` function. --------- Co-authored-by: refcell <abigger87@gmail.com>

github-actions bot added 2-reviewers A-ops Area: ops labels Feb 25, 2022

cfromknecht force-pushed the teleportr-service branch from 8841d49 to 3fd27ab Compare February 25, 2022 02:34

mslipper requested changes Feb 25, 2022

View reviewed changes

tynes reviewed Feb 25, 2022

View reviewed changes

go/teleportr/drivers/disburser/driver.go Outdated Show resolved Hide resolved

tynes reviewed Feb 25, 2022

View reviewed changes

cfromknecht force-pushed the teleportr-service branch from 3fd27ab to abb86d2 Compare March 2, 2022 19:37

github-actions bot added the M-ci Meta: ci related work label Mar 2, 2022

cfromknecht added 10 commits March 2, 2022 12:39

feat: move L2 dial logic out of bss-core

6af67df

This avoids having l2geth as a dependency for other projects building on bss-core. Speicifically this avoids having to copy l2geth into docker builds.

feat: use uint64 for deposit ids and block numbers

59ff146

Less casting.

feat: add vim swap files to gitignore

8a24ce3

feat: add teleportr flags and config

014976d

feat: add teleportr binary and Makefile skeleton

d8780d8

feat: add teleportr deposit + disburser bindings

4077920

feat: add FindFilterStartBlockNumber helper

44d2dc9

feat: add PendingTx tracking to teleportr db

2ece217

feat: add success field to teleportr disbursement table

11fe70a

cfromknecht force-pushed the teleportr-service branch from 51a677f to 04405d4 Compare March 2, 2022 20:40

github-actions bot removed 2-reviewers M-ci Meta: ci related work A-ops Area: ops labels Mar 2, 2022

cfromknecht force-pushed the teleportr-service branch from 04405d4 to 3017f2a Compare March 2, 2022 21:25

cfromknecht requested a review from tynes March 2, 2022 21:29

cfromknecht requested a review from mslipper March 2, 2022 21:29

cfromknecht force-pushed the teleportr-service branch from 3017f2a to 1480c92 Compare March 2, 2022 21:57

cfromknecht added 2 commits March 2, 2022 14:01

feat: add teleportr driver

a800608

fix: teleportr CI env vars (round 2)

2910a3c

cfromknecht force-pushed the teleportr-service branch from 1480c92 to 2910a3c Compare March 2, 2022 23:11

github-actions bot added 2-reviewers M-ci Meta: ci related work labels Mar 2, 2022

mslipper approved these changes Mar 2, 2022

View reviewed changes

mslipper merged commit 7dc259c into ethereum-optimism:develop Mar 2, 2022

cfromknecht deleted the teleportr-service branch March 2, 2022 23:20

theochap pushed a commit that referenced this pull request Dec 10, 2025

chore(node/service): Cleanup RollupNodeService::start (#2233)

7c0194f

## Overview Small cleanup to the `RollupNodeService::start` function. --------- Co-authored-by: refcell <abigger87@gmail.com>

Conversation

cfromknecht commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

codecov-commenter commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

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!

Choose a reason for hiding this comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

tynes Feb 25, 2022 • 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!

cfromknecht commented Mar 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cfromknecht commented Feb 25, 2022 •

edited

Loading

changeset-bot bot commented Feb 25, 2022 •

edited

Loading

codecov-commenter commented Feb 25, 2022 •

edited

Loading

tynes Feb 25, 2022 •

edited

Loading