feat(batcher): multi-frame altda channels #310

gastonponti · 2025-02-05T19:22:03Z

palango · 2025-02-13T14:17:00Z

This looks solid to me

ezdac · 2025-02-13T17:09:27Z

I couldn't find anything immediately concerning in this PR, without understanding all the intricate details of the batcher
data and update-queues.

This needs a fresh rebase, there has been a small commit added that fixes a stale comment.

Also it seems like their upstream PR has some more commits regarding kurtosis etc. - is that relevant for us?

gastonponti · 2025-02-13T18:06:46Z

I'll add the commit.

And the kurtosis commit is from this PR Layr-Labs#25 which we could also pick.
But is based on other PR that is moving from circle to github actions which I think we don't want

I would add it as a different PR

piersy · 2025-02-13T18:44:31Z

I added the eigenda repo as a remote (with name eigenda) to my local optimism repo and used the following command to see how it differed from this cherry pick.

git diff --no-index <(d eigenda/eigenda-develop eigenda/feat--multiframe-altda-channel) <(d origin/celo-rebase-12 origin/eigenda/feat--multiframe-altda-channel)

This is the result

diff --git a/dev/fd/15 b/dev/fd/16
--- a/dev/fd/15
+++ b/dev/fd/16
@@ -1,17 +1,8 @@
 diff --git a/op-batcher/batcher/channel.go b/op-batcher/batcher/channel.go
-index 6b936c112d..486f1d1a69 100644
+index 6b936c112d..fdd09b7f7b 100644
 --- a/op-batcher/batcher/channel.go
 +++ b/op-batcher/batcher/channel.go
-@@ -125,21 +125,21 @@ func (c *channel) ID() derive.ChannelID {
- }
-
- // NextTxData dequeues the next frames from the channel and returns them encoded in a tx data packet.
--// If cfg.UseBlobs is false, it returns txData with a single frame.
--// If cfg.UseBlobs is true, it will read frames from its channel builder
-+// If cfg.DaType == DaTypeCalldata, it returns txData with a single frame.
-+// Else when cfg.DaType == DaTypeBlob or DaTypeAltDA, it will read frames from its channel builder
- // until it either doesn't have more frames or the target number of frames is reached.
- //
+@@ -132,14 +132,14 @@ func (c *channel) ID() derive.ChannelID {
  // NextTxData should only be called after HasTxData returned true.
  func (c *channel) NextTxData() txData {
    nf := c.cfg.MaxFramesPerTx()
@@ -179,7 +170,7 @@ index b36ce9311b..5e1a4414ad 100644
        CompressorConfig: compressor.Config{
            CompressionAlgo: derive.Zlib,
 diff --git a/op-batcher/batcher/driver.go b/op-batcher/batcher/driver.go
-index fb791d4d21..7540ab3e7b 100644
+index fe1874d190..a2625e295c 100644
 --- a/op-batcher/batcher/driver.go
 +++ b/op-batcher/batcher/driver.go
 @@ -780,14 +780,6 @@ func (l *BatchSubmitter) cancelBlockingTx(queue *txmgr.Queue[txRef], receiptsCh
@@ -248,10 +239,10 @@ index fb791d4d21..7540ab3e7b 100644

  func (l *BatchSubmitter) blobTxCandidate(data txData) (*txmgr.TxCandidate, error) {
 diff --git a/op-batcher/batcher/service.go b/op-batcher/batcher/service.go
-index f884c57b3e..03031d7a49 100644
+index 4f8a5dba44..0499e42c5c 100644
 --- a/op-batcher/batcher/service.go
 +++ b/op-batcher/batcher/service.go
-@@ -218,30 +218,40 @@ func (bs *BatcherService) initChannelConfig(cfg *CLIConfig) error {
+@@ -218,33 +218,39 @@ func (bs *BatcherService) initChannelConfig(cfg *CLIConfig) error {
        TargetNumFrames:       cfg.TargetNumFrames,
        SubSafetyMargin:       cfg.SubSafetyMargin,
diff --git a/dev/fd/15 b/dev/fd/16
--- a/dev/fd/15
+++ b/dev/fd/16
@@ -1,17 +1,8 @@
 diff --git a/op-batcher/batcher/channel.go b/op-batcher/batcher/channel.go
-index 6b936c112d..486f1d1a69 100644
+index 6b936c112d..fdd09b7f7b 100644
 --- a/op-batcher/batcher/channel.go
 +++ b/op-batcher/batcher/channel.go
-@@ -125,21 +125,21 @@ func (c *channel) ID() derive.ChannelID {
- }
-
- // NextTxData dequeues the next frames from the channel and returns them encoded in a tx data packet.
--// If cfg.UseBlobs is false, it returns txData with a single frame.
--// If cfg.UseBlobs is true, it will read frames from its channel builder
-+// If cfg.DaType == DaTypeCalldata, it returns txData with a single frame.
-+// Else when cfg.DaType == DaTypeBlob or DaTypeAltDA, it will read frames from its channel builder
- // until it either doesn't have more frames or the target number of frames is reached.
- //
+@@ -132,14 +132,14 @@ func (c *channel) ID() derive.ChannelID {
  // NextTxData should only be called after HasTxData returned true.
  func (c *channel) NextTxData() txData {
    nf := c.cfg.MaxFramesPerTx()
@@ -179,7 +170,7 @@ index b36ce9311b..5e1a4414ad 100644
        CompressorConfig: compressor.Config{
            CompressionAlgo: derive.Zlib,
 diff --git a/op-batcher/batcher/driver.go b/op-batcher/batcher/driver.go
-index fb791d4d21..7540ab3e7b 100644
+index fe1874d190..a2625e295c 100644
 --- a/op-batcher/batcher/driver.go
 +++ b/op-batcher/batcher/driver.go
 @@ -780,14 +780,6 @@ func (l *BatchSubmitter) cancelBlockingTx(queue *txmgr.Queue[txRef], receiptsCh
@@ -248,10 +239,10 @@ index fb791d4d21..7540ab3e7b 100644

  func (l *BatchSubmitter) blobTxCandidate(data txData) (*txmgr.TxCandidate, error) {
 diff --git a/op-batcher/batcher/service.go b/op-batcher/batcher/service.go
-index f884c57b3e..03031d7a49 100644
+index 4f8a5dba44..0499e42c5c 100644
 --- a/op-batcher/batcher/service.go
 +++ b/op-batcher/batcher/service.go
-@@ -218,30 +218,40 @@ func (bs *BatcherService) initChannelConfig(cfg *CLIConfig) error {
+@@ -218,33 +218,39 @@ func (bs *BatcherService) initChannelConfig(cfg *CLIConfig) error {
        TargetNumFrames:       cfg.TargetNumFrames,
        SubSafetyMargin:       cfg.SubSafetyMargin,
        BatchType:             cfg.BatchType,
@@ -268,19 +259,11 @@ index f884c57b3e..03031d7a49 100644
 +          cc.DaType = DaTypeAltDA
 +      } else {
 +          return fmt.Errorf("altDA is currently only supported with calldata DA Type")
-       }
--      cc.UseBlobs = true
--  case flags.CalldataType: // do nothing
--  default:
--      return fmt.Errorf("unknown data availability type: %v", cfg.DataAvailabilityType)
--  }
++      }
 +      if cc.MaxFrameSize > altda.MaxInputSize {
 +          return fmt.Errorf("max frame size %d exceeds altDA max input size %d", cc.MaxFrameSize, altda.MaxInputSize)
 +      }
 +  } else {
-
--  if bs.UseAltDA && cc.MaxFrameSize > altda.MaxInputSize {
--      return fmt.Errorf("max frame size %d exceeds altDA max input size %d", cc.MaxFrameSize, altda.MaxInputSize)
 +      switch cfg.DataAvailabilityType {
 +      case flags.BlobsType, flags.AutoType:
 +          if !cfg.TestUseMaxTxSizeForBlobs {
@@ -292,7 +275,18 @@ index f884c57b3e..03031d7a49 100644
 +          cc.DaType = DaTypeCalldata
 +      default:
 +          return fmt.Errorf("unknown data availability type: %v", cfg.DataAvailabilityType)
-+      }
+       }
+-      cc.UseBlobs = true
+-  case flags.CalldataType: // do nothing
+-  default:
+-      return fmt.Errorf("unknown data availability type: %v", cfg.DataAvailabilityType)
+-  }
+-
+-  if bs.UseAltDA && cfg.DataAvailabilityType != flags.CalldataType {
+-      return fmt.Errorf("cannot use Blobs with Alt DA")
+-  }
+-  if bs.UseAltDA && cc.MaxFrameSize > altda.MaxInputSize {
+-      return fmt.Errorf("max frame size %d exceeds altDA max input size %d", cc.MaxFrameSize, altda.MaxInputSize)
    }

    cc.InitCompressorConfig(cfg.ApproxComprRatio, cfg.Compressor, cfg.CompressionAlgo)
@@ -306,7 +300,7 @@ index f884c57b3e..03031d7a49 100644
        bs.Log.Warn("Ecotone upgrade is active, but batcher is not configured to use Blobs!")
    }

-@@ -273,7 +283,7 @@ func (bs *BatcherService) initChannelConfig(cfg *CLIConfig) error {
+@@ -276,7 +282,7 @@ func (bs *BatcherService) initChannelConfig(cfg *CLIConfig) error {
        calldataCC := cc
        calldataCC.TargetNumFrames = 1
        calldataCC.MaxFrameSize = 120_000
@@ -316,7 +310,7 @@ index f884c57b3e..03031d7a49 100644

        bs.ChannelConfig = NewDynamicEthChannelConfig(bs.Log, 10*time.Second, bs.TxManager, cc, calldataCC)
 diff --git a/op-batcher/batcher/test_batch_submitter.go b/op-batcher/batcher/test_batch_submitter.go
-index 93083aa0dc..f497a81209 100644
+index 3c3c71ff58..b14af91476 100644
 --- a/op-batcher/batcher/test_batch_submitter.go
 +++ b/op-batcher/batcher/test_batch_submitter.go
 @@ -28,7 +28,7 @@ func (l *TestBatchSubmitter) JamTxPool(ctx context.Context) error {
@@ -378,3 +372,27 @@ index dee068cdb8..405fa1fdf8 100644
        Value:   1,
        EnvVars: prefixEnvVars("TARGET_NUM_FRAMES"),
    }
+diff --git a/op-chain-ops/cmd/celo-migrate/main.go b/op-chain-ops/cmd/celo-migrate/main.go
+index 2546ab8ae8..ff6a8375cb 100644
+--- a/op-chain-ops/cmd/celo-migrate/main.go
++++ b/op-chain-ops/cmd/celo-migrate/main.go
+@@ -501,19 +501,6 @@ func runDBCheck(opts dbCheckOptions) (err error) {
+
+   log.Info("DB Continuity Check Started", "dbPath", opts.dbPath)
+
+-  // We want to open the db in readonly mode to take advantage of concurrency, but opening in readonly
+-  // mode will fail if the db is missing some files suffixed by ".meta". Our legacy celo db does not have
+-  // ".meta" files, so opening in readonly mode will fail. Opening the db in readwrite mode will create the
+-  // ".meta" files if they don't exist. So, we can open the db in readwrite mode and then close it so
+-  // that the ".meta" files are created. We then reopen the db in readonly mode to run the actual script.
+-  db, err := openDB(opts.dbPath, false)
+-  if err != nil {
+-      return fmt.Errorf("failed to open db in readwrite mode: %w", err)
+-  }
+-  if err = db.Close(); err != nil {
+-      return fmt.Errorf("failed to close db in readwrite mode: %w", err)
+-  }
+-
+   ancientDB, err := NewChainFreezer(filepath.Join(opts.dbPath, "ancient"), "", true)
+   if err != nil {
+       return fmt.Errorf("failed to open ancient db: %w", err)

piersy · 2025-02-13T18:59:38Z

@gastonponti so it seems you are missing a couple of comment adjustments, which I think @ezdac already mentioned and also another rebase is required to get rid of the celo-migrate changes.

…nnel

piersy · 2025-02-13T21:11:06Z

@gastonponti checked again and the only difference I can see between this and the original is the addition of fee currency params to core.IntrinsicGas so looking good on that front.

piersy · 2025-02-13T21:48:59Z

op-batcher/batcher/channel.go

-// If cfg.UseBlobs is false, it returns txData with a single frame.
-// If cfg.UseBlobs is true, it will read frames from its channel builder
+// If cfg.DaType == DaTypeCalldata, it returns txData with a single frame.
+// Else when cfg.DaType == DaTypeBlob or DaTypeAltDA, it will read frames from its channel builder
 // until it either doesn't have more frames or the target number of frames is reached.
 //
 // NextTxData should only be called after HasTxData returned true.
 func (c *channel) NextTxData() txData {
 	nf := c.cfg.MaxFramesPerTx()


Hey @gastonponti, I'm wondering how do you set target-num-frames aka MaxFramesPerTx to make sure that your frames don't exceed the max frame size of 128KB?

There's a flag that you can use to set that:

TargetNumFramesFlag = &cli.IntFlag{ Name: "target-num-frames", Usage: "The target number of frames to create per channel. " + "Controls number of blobs per blob tx, if using Blob DA, " + "or number of frames per blob, if using altDA.", Value: 1, EnvVars: prefixEnvVars("TARGET_NUM_FRAMES"), }

Here are two different things.

The frame size from one side, and the "tx for the L1 data size" on the other.

The frame size is also configurable, and we should reduce it. We strictly don't need to have it as 128kb because we are not switching to blobs on eth, but to txData on eth. But if we set it to 128kb, it will be easier to switch to a eth blob scenario if we want, because we won't need to think about that size. De downside of setting it to 128kb and not 200kb is that you will need more txs in a full blocks scenario, but this is a fallback scenario that we shouldn't see often if eigenda works as it should.

The maxFramesPerTx is used for all the other types that are NOT the txDataType (or DaTypeCalldata from this PR)

func (cc *ChannelConfig) MaxFramesPerTx() int { if cc.DaType == DaTypeCalldata { return 1 } return cc.TargetNumFrames }

with that, you are guaranteeing, that in the case of switching to ethDa, you will have only one frame in the tx, and you won't exceed the max L1 tx data size

So we need to set TARGET_NUM_FRAMES or do we want to change the default in that flag?

We only need to set that TARGET_NUM_FRAMES flag

Can we document those values somewhere?

The frames are capped by this flag:

MaxL1TxSizeBytesFlag = &cli.Uint64Flag{ Name: "max-l1-tx-size-bytes", Usage: "The maximum size of a batch tx submitted to L1. Ignored for blobs, where max blob size will be used.", Value: 120_000, // will be overwritten to max for blob da-type EnvVars: prefixEnvVars("MAX_L1_TX_SIZE_BYTES"), }

actually is MaxL1TxSizeByte-1

It's not a moving thing that depends on the channel. The channel depends on the Frame size x number of Frames

The ignored for blobs, is for the ethDa for blobs, which we are not using. So our frame cap is actually set in the configurations

Actually I think I've understood.

Channel contains whatever unsafe data from L2
Channels produce a stream of txData objects (as many as required)
TxData objects contain TARGET_NUM_FRAMES frames
Frames are up to MaxFrameSize big

But now I'm wondering, how is it enforced that the txdata is within the required size limits?

Yeah, that txData name is not the best to be honest.

But, with that txData, if it goes to the altDa, it builds a blob with N frames. But if it goes to ethda as tx-data (not blobs), it sends a tx per frame. So the frame size is the one limiting that

@piersy

optimism/op-batcher/batcher/channel_builder.go

Lines 363 to 383 in f90b97b

var buf bytes.Buffer

fn, err := c.co.OutputFrame(&buf, c.cfg.MaxFrameSize)

if err != io.EOF && err != nil {

return fmt.Errorf("writing frame[%d]: %w", fn, err)

}

// Mark as full if max index reached

// TODO: If there's still data in the compression pipeline of the channel out,

// we would miss it and the whole channel would be broken because the last

// frames would never be generated...

// Hitting the max index is impossible with current parameters, so ignore for

// now. Note that in order to properly catch this, we'd need to call Flush

// after every block addition to estimate how many more frames are coming.

if fn == math.MaxUint16 {

c.setFullErr(ErrMaxFrameIndex)

}

frame := frameData{

id: frameID{chID: c.co.ID(), frameNumber: fn},

data: buf.Bytes(),

}

Within the OutputFrame() the passed in size is used as the size of a buffer, where then data from the compression
stage is written to. The passed in buffer is then used as frameData in the batcher.

…nnel

gastonponti mentioned this pull request Feb 5, 2025

feat(batcher): op batcher altda failover to ethda #311

Merged

gastonponti force-pushed the eigenda/feat--multiframe-altda-channel branch from 6dc9e05 to ba78a9a Compare February 12, 2025 16:21

gastonponti changed the base branch from karlb/celo11-15 to karlb/celo11-16 February 12, 2025 16:21

gastonponti closed this Feb 12, 2025

gastonponti force-pushed the eigenda/feat--multiframe-altda-channel branch from ba78a9a to fbeb6c3 Compare February 12, 2025 16:32

samlaf added 2 commits February 12, 2025 13:38

feat(batcher): multi-frame altda channels

ed9ec99

docs(batcher): add documentation for DaType and txData.daType

dbd9ffb

gastonponti reopened this Feb 12, 2025

gastonponti changed the base branch from karlb/celo11-16 to celo-rebase-12 February 12, 2025 16:38

samlaf and others added 2 commits February 13, 2025 16:41

docs: fix NextTxData comment

0f4d7ef

Merge branch 'celo-rebase-12' into eigenda/feat--multiframe-altda-cha…

99d00f2

…nnel

piersy reviewed Feb 13, 2025

View reviewed changes

palango approved these changes Feb 14, 2025

View reviewed changes

Merge branch 'celo-rebase-12' into eigenda/feat--multiframe-altda-cha…

8f9c63d

…nnel

gastonponti merged commit 3dce54b into celo-rebase-12 Feb 14, 2025
3 of 21 checks passed

gastonponti deleted the eigenda/feat--multiframe-altda-channel branch February 14, 2025 13:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(batcher): multi-frame altda channels #310

feat(batcher): multi-frame altda channels #310

gastonponti commented Feb 5, 2025 •

edited

Loading

palango commented Feb 13, 2025

ezdac commented Feb 13, 2025

gastonponti commented Feb 13, 2025

piersy commented Feb 13, 2025

piersy commented Feb 13, 2025

piersy commented Feb 13, 2025

piersy Feb 13, 2025

gastonponti Feb 14, 2025

palango Feb 14, 2025

gastonponti Feb 14, 2025

palango Feb 14, 2025

gastonponti Feb 14, 2025

gastonponti Feb 14, 2025

piersy Feb 14, 2025

gastonponti Feb 14, 2025

ezdac Feb 14, 2025 •

edited

Loading

	var buf bytes.Buffer
	fn, err := c.co.OutputFrame(&buf, c.cfg.MaxFrameSize)
	if err != io.EOF && err != nil {
	return fmt.Errorf("writing frame[%d]: %w", fn, err)
	}

	// Mark as full if max index reached
	// TODO: If there's still data in the compression pipeline of the channel out,
	// we would miss it and the whole channel would be broken because the last
	// frames would never be generated...
	// Hitting the max index is impossible with current parameters, so ignore for
	// now. Note that in order to properly catch this, we'd need to call Flush
	// after every block addition to estimate how many more frames are coming.
	if fn == math.MaxUint16 {
	c.setFullErr(ErrMaxFrameIndex)
	}

	frame := frameData{
	id: frameID{chID: c.co.ID(), frameNumber: fn},
	data: buf.Bytes(),
	}

feat(batcher): multi-frame altda channels #310

feat(batcher): multi-frame altda channels #310

Conversation

gastonponti commented Feb 5, 2025 • edited Loading

palango commented Feb 13, 2025

ezdac commented Feb 13, 2025

gastonponti commented Feb 13, 2025

piersy commented Feb 13, 2025

piersy commented Feb 13, 2025

piersy commented Feb 13, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ezdac Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

gastonponti commented Feb 5, 2025 •

edited

Loading

ezdac Feb 14, 2025 •

edited

Loading