PR Comments

axelKingsley · axelKingsley · commit 1a9e3e7f59ec · 2024-04-04T11:10:09.000-05:00
diff --git a/op-batcher/batcher/channel_builder.go b/op-batcher/batcher/channel_builder.go
@@ -83,13 +83,17 @@ func NewChannelBuilder(cfg ChannelConfig, rollupCfg rollup.Config, latestL1Origi
 	if err != nil {
 		return nil, err
 	}
-	var spanBatch *derive.SpanBatch
+	var co derive.ChannelOut
 	if cfg.BatchType == derive.SpanBatchType {
-		spanBatch = derive.NewSpanBatch(rollupCfg.Genesis.L2Time, rollupCfg.L2ChainID)
-	}
-	co, err := derive.NewChannelOut(cfg.BatchType, c, spanBatch)
-	if err != nil {
-		return nil, err
+		co, err = derive.NewSpanChannelOut(rollupCfg.Genesis.L2Time, rollupCfg.L2ChainID, cfg.CompressorConfig.TargetOutputSize)
+		if err != nil {
+			return nil, err
+		}
+	} else {
+		co, err = derive.NewChannelOut(c)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	cb := &ChannelBuilder{
diff --git a/op-batcher/batcher/channel_builder_test.go b/op-batcher/batcher/channel_builder_test.go
@@ -365,7 +365,7 @@ func TestChannelBuilder_OutputWrongFramePanic(t *testing.T) {
 	// to construct a single frame
 	c, err := channelConfig.CompressorConfig.NewCompressor()
 	require.NoError(t, err)
-	co, err := derive.NewChannelOut(derive.SingularBatchType, c, nil)
+	co, err := derive.NewChannelOut(c)
 	require.NoError(t, err)
 	var buf bytes.Buffer
 	fn, err := co.OutputFrame(&buf, channelConfig.MaxFrameSize)
diff --git a/op-batcher/compressor/blind_compressor.go b/op-batcher/compressor/blind_compressor.go
diff --git a/op-batcher/compressor/blind_compressor_test.go b/op-batcher/compressor/blind_compressor_test.go
diff --git a/op-batcher/compressor/compressors.go b/op-batcher/compressor/compressors.go
@@ -10,7 +10,6 @@ const (
 	RatioKind  = "ratio"
 	ShadowKind = "shadow"
 	NoneKind   = "none"
-	BlindKind  = "blind"
 
 	// CloseOverheadZlib is the number of final bytes a [zlib.Writer] call writes
 	// to the output buffer.
@@ -21,7 +20,6 @@ var Kinds = map[string]FactoryFunc{
 	RatioKind:  NewRatioCompressor,
 	ShadowKind: NewShadowCompressor,
 	NoneKind:   NewNonCompressor,
-	BlindKind:  NewBlindCompressor,
 }
 
 var KindKeys []string
diff --git a/op-batcher/compressor/non_compressor.go b/op-batcher/compressor/non_compressor.go
@@ -36,10 +36,6 @@ func NewNonCompressor(config Config) (derive.Compressor, error) {
 	return c, nil
 }
 
-func (t *NonCompressor) TargetOutputSize() uint64 {
-	return t.config.TargetOutputSize
-}
-
 func (t *NonCompressor) Write(p []byte) (int, error) {
 	if err := t.compress.Flush(); err != nil {
 		return 0, err
diff --git a/op-batcher/compressor/ratio_compressor.go b/op-batcher/compressor/ratio_compressor.go
@@ -34,10 +34,6 @@ func NewRatioCompressor(config Config) (derive.Compressor, error) {
 	return c, nil
 }
 
-func (t *RatioCompressor) TargetOutputSize() uint64 {
-	return t.config.TargetOutputSize
-}
-
 func (t *RatioCompressor) Write(p []byte) (int, error) {
 	if err := t.FullErr(); err != nil {
 		return 0, err
diff --git a/op-batcher/compressor/shadow_compressor.go b/op-batcher/compressor/shadow_compressor.go
@@ -58,10 +58,6 @@ func NewShadowCompressor(config Config) (derive.Compressor, error) {
 	return c, nil
 }
 
-func (t *ShadowCompressor) TargetOutputSize() uint64 {
-	return t.config.TargetOutputSize
-}
-
 func (t *ShadowCompressor) Write(p []byte) (int, error) {
 	if t.fullErr != nil {
 		return 0, t.fullErr
diff --git a/op-e2e/actions/l2_batcher.go b/op-e2e/actions/l2_batcher.go
@@ -189,24 +189,21 @@ func (s *L2Batcher) Buffer(t Testing) error {
 		if s.l2BatcherCfg.GarbageCfg != nil {
 			ch, err = NewGarbageChannelOut(s.l2BatcherCfg.GarbageCfg)
 		} else {
-			c, e := compressor.NewBlindCompressor(compressor.Config{
-				TargetOutputSize: batcher.MaxDataSize(1, s.l2BatcherCfg.MaxL1TxSize),
+			target := batcher.MaxDataSize(1, s.l2BatcherCfg.MaxL1TxSize)
+			c, e := compressor.NewShadowCompressor(compressor.Config{
+				TargetOutputSize: target,
 			})
 			require.NoError(t, e, "failed to create compressor")
 
-			var batchType uint = derive.SingularBatchType
-			var spanBatch *derive.SpanBatch
-
 			if s.l2BatcherCfg.ForceSubmitSingularBatch && s.l2BatcherCfg.ForceSubmitSpanBatch {
 				t.Fatalf("ForceSubmitSingularBatch and ForceSubmitSpanBatch cannot be set to true at the same time")
 			} else if s.l2BatcherCfg.ForceSubmitSingularBatch {
 				// use SingularBatchType
+				ch, err = derive.NewChannelOut(c)
 			} else if s.l2BatcherCfg.ForceSubmitSpanBatch || s.rollupCfg.IsDelta(block.Time()) {
 				// If both ForceSubmitSingularBatch and ForceSubmitSpanbatch are false, use SpanBatch automatically if Delta HF is activated.
-				batchType = derive.SpanBatchType
-				spanBatch = derive.NewSpanBatch(s.rollupCfg.Genesis.L2Time, s.rollupCfg.L2ChainID)
+				ch, err = derive.NewSpanChannelOut(s.rollupCfg.Genesis.L2Time, s.rollupCfg.L2ChainID, target)
 			}
-			ch, err = derive.NewChannelOut(batchType, c, spanBatch)
 		}
 		require.NoError(t, err, "failed to create channel")
 		s.l2ChannelOut = ch
diff --git a/op-e2e/actions/sync_test.go b/op-e2e/actions/sync_test.go
@@ -7,7 +7,6 @@ import (
 	"testing"
 	"time"
 
-	"github.com/ethereum-optimism/optimism/op-batcher/compressor"
 	"github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
 	"github.com/ethereum-optimism/optimism/op-node/rollup/derive"
 	"github.com/ethereum-optimism/optimism/op-node/rollup/sync"
@@ -27,12 +26,7 @@ import (
 )
 
 func newSpanChannelOut(t StatefulTesting, e e2eutils.SetupData) derive.ChannelOut {
-	c, err := compressor.NewBlindCompressor(compressor.Config{
-		TargetOutputSize: 128_000,
-	})
-	require.NoError(t, err)
-	spanBatch := derive.NewSpanBatch(e.RollupCfg.Genesis.L2Time, e.RollupCfg.L2ChainID)
-	channelOut, err := derive.NewChannelOut(derive.SpanBatchType, c, spanBatch)
+	channelOut, err := derive.NewSpanChannelOut(e.RollupCfg.Genesis.L2Time, e.RollupCfg.L2ChainID, 128_000)
 	require.NoError(t, err)
 	return channelOut
 }
diff --git a/op-node/benchmarks/batchbuilding_test.go b/op-node/benchmarks/batchbuilding_test.go
@@ -13,39 +13,34 @@ import (
 )
 
 var (
+	// a really large target output size to ensure that the compressors are never full
+	targetOutput_huge = uint64(100_000_000_000)
+	// this target size was determiend by the devnet sepolia batcher's configuration
+	targetOuput_real = uint64(780120)
 
 	// compressors used in the benchmark
 	rc, _ = compressor.NewRatioCompressor(compressor.Config{
-		TargetOutputSize: 100_000_000_000,
+		TargetOutputSize: targetOutput_huge,
 		ApproxComprRatio: 0.4,
 	})
 	sc, _ = compressor.NewShadowCompressor(compressor.Config{
-		TargetOutputSize: 100_000_000_000,
+		TargetOutputSize: targetOutput_huge,
 	})
 	nc, _ = compressor.NewNonCompressor(compressor.Config{
-		TargetOutputSize: 100_000_000_000,
-	})
-	bc, _ = compressor.NewBlindCompressor(compressor.Config{
-		TargetOutputSize: 100_000_000_000,
+		TargetOutputSize: targetOutput_huge,
 	})
 	realsc, _ = compressor.NewShadowCompressor(compressor.Config{
-		// this target size was determiend by the devnet sepolia batcher's configuration
-		TargetOutputSize: 780120,
-	})
-	realbc, _ = compressor.NewBlindCompressor(compressor.Config{
-		// this target size was determiend by the devnet sepolia batcher's configuration
-		TargetOutputSize: 780120,
+		TargetOutputSize: targetOuput_real,
 	})
 
-	compressors = map[string]derive.Compressor{
-		"BlindCompressor":      bc,
-		"NonCompressor":        nc,
-		"RatioCompressor":      rc,
-		"ShadowCompressor":     sc,
-		"RealBlindCompressor":  realbc,
-		"RealShadowCompressor": realsc,
+	// compressors used in the benchmark mapped by their name
+	// they come paired with a target output size so span batches can use the target size directly
+	compressors = map[string]compressorAndTarget{
+		"NonCompressor":        {nc, targetOutput_huge},
+		"RatioCompressor":      {rc, targetOutput_huge},
+		"ShadowCompressor":     {sc, targetOutput_huge},
+		"RealShadowCompressor": {realsc, targetOuput_real},
 	}
-
 	// batch types used in the benchmark
 	batchTypes = []uint{
 		derive.SpanBatchType,
@@ -55,6 +50,23 @@ var (
 	}
 )
 
+type compressorAndTarget struct {
+	compressor   derive.Compressor
+	targetOutput uint64
+}
+
+// channelOutByType returns a channel out of the given type as a helper for the benchmarks
+func channelOutByType(batchType uint, compKey string) (derive.ChannelOut, error) {
+	chainID := big.NewInt(333)
+	if batchType == derive.SingularBatchType {
+		return derive.NewChannelOut(compressors[compKey].compressor)
+	}
+	if batchType == derive.SpanBatchType {
+		return derive.NewSpanChannelOut(0, chainID, compressors[compKey].targetOutput)
+	}
+	return nil, fmt.Errorf("unsupported batch type: %d", batchType)
+}
+
 // a test case for the benchmark controls the number of batches and transactions per batch,
 // as well as the batch type and compressor used
 type BatchingBenchmarkTC struct {
@@ -120,9 +132,8 @@ func BenchmarkFinalBatchChannelOut(b *testing.B) {
 			for bn := 0; bn < b.N; bn++ {
 				// don't measure the setup time
 				b.StopTimer()
-				compressors[tc.compKey].Reset()
-				spanBatch := derive.NewSpanBatch(uint64(0), chainID)
-				cout, _ := derive.NewChannelOut(tc.BatchType, compressors[tc.compKey], spanBatch)
+				compressors[tc.compKey].compressor.Reset()
+				cout, _ := channelOutByType(tc.BatchType, tc.compKey)
 				// add all but the final batch to the channel out
 				for i := 0; i < tc.BatchCount-1; i++ {
 					_, err := cout.AddSingularBatch(batches[i], 0)
@@ -149,11 +160,11 @@ func BenchmarkIncremental(b *testing.B) {
 	// use batchCount as the number of batches to add in each benchmark iteration
 	// and use txPerBatch as the number of transactions per batch
 	tcs := []BatchingBenchmarkTC{
-		{derive.SpanBatchType, 100, 1, "RealBlindCompressor"},
-		{derive.SingularBatchType, 100, 1, "RealShadowCompressor"},
+		{derive.SpanBatchType, 5, 1, "RealBlindCompressor"},
+		//{derive.SingularBatchType, 100, 1, "RealShadowCompressor"},
 	}
 	for _, tc := range tcs {
-		cout, err := derive.NewChannelOut(tc.BatchType, compressors[tc.compKey], derive.NewSpanBatch(0, chainID))
+		cout, err := channelOutByType(tc.BatchType, tc.compKey)
 		if err != nil {
 			b.Fatal(err)
 		}
@@ -230,9 +241,8 @@ func BenchmarkAllBatchesChannelOut(b *testing.B) {
 			for bn := 0; bn < b.N; bn++ {
 				// don't measure the setup time
 				b.StopTimer()
-				compressors[tc.compKey].Reset()
-				spanBatch := derive.NewSpanBatch(0, chainID)
-				cout, _ := derive.NewChannelOut(tc.BatchType, compressors[tc.compKey], spanBatch)
+				compressors[tc.compKey].compressor.Reset()
+				cout, _ := channelOutByType(tc.BatchType, tc.compKey)
 				b.StartTimer()
 				// add all batches to the channel out
 				for i := 0; i < tc.BatchCount; i++ {
diff --git a/op-node/rollup/derive/channel_out.go b/op-node/rollup/derive/channel_out.go
@@ -48,8 +48,6 @@ type Compressor interface {
 	// calls to Write will fail if an error is returned from this method, but calls to Write
 	// can still return ErrCompressorFull even if this does not.
 	FullErr() error
-	// TargetOutputSize returns the target size of the compressed output
-	TargetOutputSize() uint64
 }
 
 type ChannelOut interface {
@@ -65,15 +63,8 @@ type ChannelOut interface {
 	OutputFrame(*bytes.Buffer, uint64) (uint16, error)
 }
 
-func NewChannelOut(batchType uint, compress Compressor, spanBatch *SpanBatch) (ChannelOut, error) {
-	switch batchType {
-	case SingularBatchType:
-		return NewSingularChannelOut(compress)
-	case SpanBatchType:
-		return NewSpanChannelOut(spanBatch.GenesisTimestamp, spanBatch.ChainID, compress.TargetOutputSize())
-	default:
-		return nil, fmt.Errorf("unrecognized batch type: %d", batchType)
-	}
+func NewChannelOut(compress Compressor) (ChannelOut, error) {
+	return NewSingularChannelOut(compress)
 }
 
 type SingularChannelOut struct {
diff --git a/op-node/rollup/derive/channel_out_test.go b/op-node/rollup/derive/channel_out_test.go
diff --git a/op-node/rollup/derive/span_channel_out.go b/op-node/rollup/derive/span_channel_out.go
