l2geth: Sync from Backend Queue by Inphi · Pull Request #2296 · ethereum-optimism/optimism

Inphi · 2022-03-09T16:12:12Z

Description

Introducing a new Backend verifiers can use to synchronize transactions from an external message queue.
To facilitate failover recovery without causing a chainsplit. The sequencer can be configured to log transactions externally prior to being added to the next block.

To facilitate failover recovery without causing a chainsplit.

changeset-bot · 2022-03-09T16:12:15Z

🦋 Changeset detected

Latest commit: f061820

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-service	Patch
@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

codecov-commenter · 2022-03-09T16:45:42Z

Codecov Report

Merging #2296 (f061820) into develop (4bfddfb) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2296   +/-   ##
========================================
  Coverage    80.14%   80.14%           
========================================
  Files           77       77           
  Lines         2458     2458           
  Branches       450      450           
========================================
  Hits          1970     1970           
  Misses         488      488

Flag	Coverage Δ
contracts	`99.29% <ø> (ø)`
core-utils	`86.77% <ø> (ø)`
data-transport-layer	`49.72% <ø> (ø)`
sdk	`55.90% <ø> (ø)`

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 4bfddfb...f061820. Read the comment docs.

tynes · 2022-03-09T17:53:09Z

l2geth/rollup/sync_service.go

+	if err := s.txLogger.Publish(s.ctx, encodedTx.Bytes()); err != nil {
+		pubTxDropCounter.Inc(1)
+		log.Error("Failed to publish transaction to log", "msg", err)
+		return fmt.Errorf("internal error: transaction logging failed")


I don't love the idea of returning an error here because that means we tightly couple googles uptime with our ability to accept transactions. Not really sure of a good solution

Yeah, I don't like quite like it either but it's the best option. With the way infra is setup right now, I don't think this will worsen failure rate of transaction submissions. i.e. the sequencer is more likely to fail than google pubsub. And going by actual historical incidents, even less likely to fail.

My biggest concern with this approach is the additional latency overhead used per tx for logging. The median time to publish a transaction is about 35ms. Not bad right now, but it's something to watch out if tx rate increases via gas fee reductions or by having more users.

If we use this logic in two places as per my other comment, we should wrap this in a helper method in rcfg/pub.

mslipper · 2022-03-09T19:46:35Z

I think you'll need to add similar TX logging functionality to the RPC handler for eth_sendRawTransaction.

mslipper · 2022-03-09T19:47:53Z

l2geth/rollup/pub/google.go

+	p.topic.ResumePublish(messageOrderingKey)
+	result := p.topic.Publish(ctx, &pmsg)


Is this thread safe?

good catch. Publish by itself is but not with the ResumePublish prior call.

mslipper · 2022-03-09T19:52:11Z

l2geth/rollup/sync_service.go

+	if err := s.txLogger.Publish(s.ctx, encodedTx.Bytes()); err != nil {
+		pubTxDropCounter.Inc(1)
+		log.Error("Failed to publish transaction to log", "msg", err)
+		return fmt.Errorf("internal error: transaction logging failed")


If we use this logic in two places as per my other comment, we should wrap this in a helper method in rcfg/pub.

Inphi · 2022-03-09T20:20:05Z

I think you'll need to add similar TX logging functionality to the RPC handler for eth_sendRawTransaction.

@mslipper eth_sendRawTransaction goes through the same code path. So we only need to log txs in one location.

l2geth/cmd/utils/flags.go

l2geth/rollup/pub/google.go

The BackendQueue can be used by verifiers to sync transactions from Google PubSub

Inphi · 2022-03-17T16:00:10Z

l2geth/rollup/sync_service.go

 	return s.applyTransaction(tx)
 }
+
+type QueuedTransactionMeta struct {


Removed a bunch of required fields from types.Transaction. The subscriber will assert that the necessary fields are set.

l2geth/rollup/sync_service.go

tynes · 2022-03-18T00:45:17Z

l2geth/core/types/transaction_meta.go

+func (q QueueOrigin) MarshalJSON() ([]byte, error) {
+	switch q {
+	case QueueOriginSequencer:
+		return []byte("\"sequencer\""), nil


Could you get rid of the need to the escapes by using a backtick string?

certainly. thanks for pointing that out.

tynes · 2022-03-18T00:48:05Z

l2geth/rollup/sync_service.go

+			return
+		}
+
+		if err := s.applyTransactionToTip(&tx); err != nil {


Do we have an ordering guarantee? And what about a delivery guarantee?

Yeah. An ordering is enforced by PubSub via external configuration. The transaction ordering relates to the block that contains it. Transactions publishing itself by a Sequencer is ordered - since a SendTx fails if we're unable to log it in PubSub.

Delivery guarantees are also handled by PubSub via external configuration. On infra, published messages will be available for at least several days. Which is enough time for a failover. Messages may be delivered more than once, so we check the db to see if we've already applied the transaction. If so, we acknowledge the message (advance the subscription queue offset) and move on.

We can assert that infra is setup correctly in code and the PubSub subscription has message ordering enabled. But this requires extra GCP permissions than needed to subscribe to messages.

and use l1 block params during tx apply

Inphi · 2022-03-18T03:54:03Z

l2geth/rollup/sync_service.go

 	// requirement of the remote server being up.
-	if service.enable {
+	// If we're syncing from the Queue, then we can skip all this and rely on L2 published transactions
+	if service.enable && service.backend != BackendQueue {


Yay or nay? Side effect of this is that IsSyncing always returns false when we're syncing from the Queue. IsSyncing is only used to prevent transactions from being submitted to the node. But only verifiers will use the BackendQueue, so I don't think this would be an issue.

I think this is OK - I don't expect any replica operators to use BackendQueue.

mslipper · 2022-03-18T13:25:32Z

Approval is conditioned on fixing the go.sum - I believe that's what's causing the test errors.

tynes · 2022-03-18T21:15:04Z

l2geth/rollup/sync_service_test.go

+	tx := mockTx()
+	tx.GetMeta().RawTransaction = nil
+	tests := map[string][]byte{
+		//"good txmeta":                      []byte("{\"l1BlockNumber\":0,\"l1Timestamp\":1647549225,\"l1MessageSender\":\"0x1487ef4dd5b0ca7610b85964371c1d8ab7c468eb\",\"queueOrigin\":\"sequencer\",\"index\":0,\"queueIndex\":0,\"rawTransaction\":\"34CAgJQrz3UmBr9M0373farCJhgNfaGiVICCAACAgIA=\"}"),


Does this test fail?

If it's uncommented then the test case fails. It's not meant to be tested. The comment is only a reference transaction log I used to construct the other failure scenarios.

Closes #2090

Inphi added 2 commits March 7, 2022 11:15

l2geth: Publish transactions to external msg queue

3e016ae

To facilitate failover recovery without causing a chainsplit.

recover from pub errors

935782f

github-actions bot added 2-reviewers A-cannon Area: cannon labels Mar 9, 2022

tynes reviewed Mar 9, 2022

View reviewed changes

mslipper suggested changes Mar 9, 2022

View reviewed changes

synchronize Publish with ResumePublish

f876add

Inphi commented Mar 11, 2022

View reviewed changes

l2geth/cmd/utils/flags.go Show resolved Hide resolved

Inphi commented Mar 11, 2022

View reviewed changes

l2geth/rollup/pub/google.go Show resolved Hide resolved

Inphi and others added 3 commits March 14, 2022 01:03

lint

36991e6

Merge branch 'develop' into feat/publish-tx

a11c4c4

add txpublisher.enable flag

9f05c87

Inphi marked this pull request as ready for review March 17, 2022 02:33

Inphi requested review from cfromknecht and smartcontracts as code owners March 17, 2022 02:33

Introduce new Backend Queue

783252e

The BackendQueue can be used by verifiers to sync transactions from Google PubSub

Inphi changed the title ~~l2geth: Publish transactions to external msg queue~~ l2geth: Sync from Backend Queue Mar 17, 2022

Inphi commented Mar 17, 2022

View reviewed changes

l2geth/rollup/sync_service.go Outdated Show resolved Hide resolved

Inphi added 2 commits March 17, 2022 12:49

log errors in sub cb

fe47425

add BackendQueue tests

3e7b5ab

tynes reviewed Mar 18, 2022

View reviewed changes

Inphi added 2 commits March 17, 2022 23:33

fix flags

6810613

and use l1 block params during tx apply

nits

de1ea95

Inphi commented Mar 18, 2022

View reviewed changes

assert Tx Meta equals in test

8e64fd9

mslipper approved these changes Mar 18, 2022

View reviewed changes

Inphi and others added 2 commits March 18, 2022 09:42

Fix build; add changeset

f834886

Merge branch 'develop' into feat/publish-tx

f061820

Inphi merged commit 32d6919 into ethereum-optimism:develop Mar 18, 2022

tynes reviewed Mar 18, 2022

View reviewed changes

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

feat: block processing metrics (#2296)

215b37a

Closes #2090

		p.topic.ResumePublish(messageOrderingKey)
		result := p.topic.Publish(ctx, &pmsg)

Conversation

Inphi commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

🦋 Changeset detected

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Inphi Mar 9, 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!

Choose a reason for hiding this comment

Uh oh!

mslipper commented Mar 9, 2022 • 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!

Choose a reason for hiding this comment

Uh oh!

Inphi commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

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!

Inphi Mar 18, 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!

Inphi Mar 18, 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!

mslipper commented Mar 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Inphi Mar 18, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Inphi commented Mar 9, 2022 •

edited

Loading

changeset-bot bot commented Mar 9, 2022 •

edited

Loading

codecov-commenter commented Mar 9, 2022 •

edited

Loading

Inphi Mar 9, 2022 •

edited

Loading

mslipper commented Mar 9, 2022 •

edited

Loading

Inphi commented Mar 9, 2022 •

edited

Loading

Inphi Mar 18, 2022 •

edited

Loading

Inphi Mar 18, 2022 •

edited

Loading

Inphi Mar 18, 2022 •

edited

Loading