Merge pull request #2332 from cfromknecht/bss-plaintext-tx-size-enforcement

feat: bss plaintext tx size enforcement

.changeset/stale-ladybugs-smell.md

+---
+'@eth-optimism/batch-submitter-service': patch
+---
+
+Enforce min/max tx size on plaintext batch encoding

go/batch-submitter/drivers/sequencer/batch.go

@@ -78,7 +78,6 @@ func GenSequencerBatchParams(
 	shouldStartAtElement uint64,
 	blockOffset uint64,
 	batch []BatchElement,
-	batchType BatchType,
 ) (*AppendSequencerBatchParams, error) {
 
 	var (
@@ -189,6 +188,5 @@ func GenSequencerBatchParams(
 		TotalElementsToAppend: uint64(len(batch)),
 		Contexts:              contexts,
 		Txs:                   txs,
-		Type:                  batchType,
 	}, nil
 }

go/batch-submitter/drivers/sequencer/driver.go

@@ -199,39 +199,57 @@ func (d *Driver) CraftBatchTx(
 	var pruneCount int
 	for {
 		batchParams, err := GenSequencerBatchParams(
-			shouldStartAt, d.cfg.BlockOffset, batchElements, d.cfg.BatchType,
+			shouldStartAt, d.cfg.BlockOffset, batchElements,
 		)
 		if err != nil {
 			return nil, err
 		}
 
-		batchArguments, err := batchParams.Serialize()
+		// Use plaintext encoding to enforce size constraints.
+		plaintextBatchArguments, err := batchParams.Serialize(BatchTypeLegacy)
 		if err != nil {
 			return nil, err
 		}
 
 		appendSequencerBatchID := d.ctcABI.Methods[appendSequencerBatchMethodName].ID
-		batchCallData := append(appendSequencerBatchID, batchArguments...)
+		plaintextCalldata := append(appendSequencerBatchID, plaintextBatchArguments...)
 
-		// Continue pruning until calldata size is less than configured max.
-		calldataSize := uint64(len(batchCallData))
-		if calldataSize > d.cfg.MaxTxSize {
+		// Continue pruning until plaintext calldata size is less than
+		// configured max.
+		plaintextCalldataSize := uint64(len(plaintextCalldata))
+		if plaintextCalldataSize > d.cfg.MaxTxSize {
 			oldLen := len(batchElements)
 			newBatchElementsLen := (oldLen * 9) / 10
 			batchElements = batchElements[:newBatchElementsLen]
-			log.Info(name+" pruned batch", "old_num_txs", oldLen, "new_num_txs", newBatchElementsLen)
+			log.Info(name+" pruned batch",
+				"plaintext_size", plaintextCalldataSize,
+				"max_tx_size", d.cfg.MaxTxSize,
+				"old_num_txs", oldLen,
+				"new_num_txs", newBatchElementsLen)
 			pruneCount++
 			continue
-		} else if calldataSize < d.cfg.MinTxSize {
+		} else if plaintextCalldataSize < d.cfg.MinTxSize {
 			log.Info(name+" batch tx size below minimum",
-				"size", calldataSize, "min_tx_size", d.cfg.MinTxSize)
+				"plaintext_size", plaintextCalldataSize,
+				"min_tx_size", d.cfg.MinTxSize,
+				"num_txs", len(batchElements))
 			return nil, nil
 		}
 
 		d.metrics.NumElementsPerBatch().Observe(float64(len(batchElements)))
 		d.metrics.BatchPruneCount.Set(float64(pruneCount))
 
-		log.Info(name+" batch constructed", "num_txs", len(batchElements), "length", len(batchCallData))
+		// Finally, encode the batch using the configured batch type.
+		var calldata = plaintextCalldata
+		if d.cfg.BatchType != BatchTypeLegacy {
+			batchArguments, err := batchParams.Serialize(d.cfg.BatchType)
+			if err != nil {
+				return nil, err
+			}
+			calldata = append(appendSequencerBatchID, batchArguments...)
+		}
+
+		log.Info(name+" batch constructed", "num_txs", len(batchElements), "length", len(calldata))
 
 		opts, err := bind.NewKeyedTransactorWithChainID(
 			d.cfg.PrivKey, d.cfg.ChainID,
@@ -243,7 +261,7 @@ func (d *Driver) CraftBatchTx(
 		opts.Nonce = nonce
 		opts.NoSend = true
 
-		tx, err := d.rawCtcContract.RawTransact(opts, batchCallData)
+		tx, err := d.rawCtcContract.RawTransact(opts, calldata)
 		switch {
 		case err == nil:
 			return tx, nil
@@ -258,7 +276,7 @@ func (d *Driver) CraftBatchTx(
 			log.Warn(d.cfg.Name + " eth_maxPriorityFeePerGas is unsupported " +
 				"by current backend, using fallback gasTipCap")
 			opts.GasTipCap = drivers.FallbackGasTipCap
-			return d.rawCtcContract.RawTransact(opts, batchCallData)
+			return d.rawCtcContract.RawTransact(opts, calldata)
 
 		default:
 			return nil, err

go/batch-submitter/drivers/sequencer/encoding.go

@@ -47,6 +47,25 @@ type BatchContext struct {
 	BlockNumber uint64 `json:"block_number"`
 }
 
+// IsMarkerContext returns true if the BatchContext is a marker context used to
+// specify the encoding format. This is only valid if called on the first
+// BatchContext in the calldata.
+func (c BatchContext) IsMarkerContext() bool {
+	return c.Timestamp == 0
+}
+
+// MarkerBatchType returns the BatchType specified by a marker BatchContext.
+// The return value is only valid if called on the first BatchContext in the
+// calldata and IsMarkerContext returns true.
+func (c BatchContext) MarkerBatchType() BatchType {
+	switch c.BlockNumber {
+	case 0:
+		return BatchTypeZlib
+	default:
+		return BatchTypeLegacy
+	}
+}
+
 // Write encodes the BatchContext into a 16-byte stream using the following
 // encoding:
 //  - num_sequenced_txs:        3 bytes
@@ -83,13 +102,34 @@ func (c *BatchContext) Read(r io.Reader) error {
 	return readUint64(r, &c.BlockNumber, 5)
 }
 
-// BatchType represents the type of batch being
-// submitted. When the first context in the batch
-// has a timestamp of 0, the blocknumber is interpreted
-// as an enum that represets the type
+// BatchType represents the type of batch being submitted. When the first
+// context in the batch has a timestamp of 0, the blocknumber is interpreted as
+// an enum that represets the type.
 type BatchType int8
 
-// Implements the Stringer interface for BatchType
+const (
+	// BatchTypeLegacy represets the legacy batch type.
+	BatchTypeLegacy BatchType = -1
+
+	// BatchTypeZlib represents a batch type where the transaction data is
+	// compressed using zlib.
+	BatchTypeZlib BatchType = 0
+)
+
+// BatchTypeFromString returns the BatchType enum based on a human readable
+// string.
+func BatchTypeFromString(s string) BatchType {
+	switch s {
+	case "zlib", "ZLIB":
+		return BatchTypeZlib
+	case "legacy", "LEGACY":
+		return BatchTypeLegacy
+	default:
+		return BatchTypeLegacy
+	}
+}
+
+// String implements the Stringer interface for BatchType.
 func (b BatchType) String() string {
 	switch b {
 	case BatchTypeLegacy:
@@ -101,27 +141,26 @@ func (b BatchType) String() string {
 	}
 }
 
-// BatchTypeFromString returns the BatchType
-// enum based on a human readable string
-func BatchTypeFromString(s string) BatchType {
-	switch s {
-	case "zlib", "ZLIB":
-		return BatchTypeZlib
-	case "legacy", "LEGACY":
-		return BatchTypeLegacy
+// MarkerContext returns the marker context, if any, for the given batch type.
+func (b BatchType) MarkerContext() *BatchContext {
+	switch b {
+
+	// No marker context for legacy encoding.
+	case BatchTypeLegacy:
+		return nil
+
+	// Zlib marker context sets block number equal to zero.
+	case BatchTypeZlib:
+		return &BatchContext{
+			Timestamp:   0,
+			BlockNumber: 0,
+		}
+
 	default:
-		return BatchTypeLegacy
+		return nil
 	}
 }
 
-const (
-	// BatchTypeLegacy represets the legacy batch type
-	BatchTypeLegacy BatchType = -1
-	// BatchTypeZlib represents a batch type where the
-	// transaction data is compressed using zlib
-	BatchTypeZlib BatchType = 0
-)
-
 // AppendSequencerBatchParams holds the raw data required to submit a batch of
 // L2 txs to L1 CTC contract. Rather than encoding the objects using the
 // standard ABI encoding, a custom encoding is and provided in the call data to
@@ -146,9 +185,6 @@ type AppendSequencerBatchParams struct {
 	// Txs contains all sequencer txs that will be recorded in the L1 CTC
 	// contract.
 	Txs []*CachedTx
-
-	// The type of the batch
-	Type BatchType
 }
 
 // Write encodes the AppendSequencerBatchParams using the following format:
@@ -173,7 +209,11 @@ type AppendSequencerBatchParams struct {
 //
 // Note that writing to a bytes.Buffer cannot
 // error, so errors are ignored here
-func (p *AppendSequencerBatchParams) Write(w *bytes.Buffer) error {
+func (p *AppendSequencerBatchParams) Write(
+	w *bytes.Buffer,
+	batchType BatchType,
+) error {
+
 	_ = writeUint64(w, p.ShouldStartAtElement, 5)
 	_ = writeUint64(w, p.TotalElementsToAppend, 3)
 
@@ -190,10 +230,10 @@ func (p *AppendSequencerBatchParams) Write(w *bytes.Buffer) error {
 	// copy the contexts as to not malleate the struct
 	// when it is a typed batch
 	contexts := make([]BatchContext, 0, len(p.Contexts)+1)
-	if p.Type == BatchTypeZlib {
-		// All zero values for the single batch context
-		// is desired here as blocknumber 0 means it is a zlib batch
-		contexts = append(contexts, BatchContext{})
+	// Add the marker context, if any, for non-legacy encodings.
+	markerContext := batchType.MarkerContext()
+	if markerContext != nil {
+		contexts = append(contexts, *markerContext)
 	}
 	contexts = append(contexts, p.Contexts...)
 
@@ -203,7 +243,7 @@ func (p *AppendSequencerBatchParams) Write(w *bytes.Buffer) error {
 		context.Write(w)
 	}
 
-	switch p.Type {
+	switch batchType {
 	case BatchTypeLegacy:
 		// Write each length-prefixed tx.
 		for _, tx := range p.Txs {
@@ -225,7 +265,7 @@ func (p *AppendSequencerBatchParams) Write(w *bytes.Buffer) error {
 		}
 
 	default:
-		return fmt.Errorf("Unknown batch type: %s", p.Type)
+		return fmt.Errorf("Unknown batch type: %s", batchType)
 	}
 
 	return nil
@@ -233,9 +273,12 @@ func (p *AppendSequencerBatchParams) Write(w *bytes.Buffer) error {
 
 // Serialize performs the same encoding as Write, but returns the resulting
 // bytes slice.
-func (p *AppendSequencerBatchParams) Serialize() ([]byte, error) {
+func (p *AppendSequencerBatchParams) Serialize(
+	batchType BatchType,
+) ([]byte, error) {
+
 	var buf bytes.Buffer
-	if err := p.Write(&buf); err != nil {
+	if err := p.Write(&buf, batchType); err != nil {
 		return nil, err
 	}
 	return buf.Bytes(), nil
@@ -266,6 +309,9 @@ func (p *AppendSequencerBatchParams) Read(r io.Reader) error {
 		return err
 	}
 
+	// Assume that it is a legacy batch at first, this will be overwrritten if
+	// we detect a marker context.
+	var batchType = BatchTypeLegacy
 	// Ensure that contexts is never nil
 	p.Contexts = make([]BatchContext, 0)
 	for i := uint64(0); i < numContexts; i++ {
@@ -274,31 +320,34 @@ func (p *AppendSequencerBatchParams) Read(r io.Reader) error {
 			return err
 		}
 
+		if i == 0 && batchContext.IsMarkerContext() {
+			batchType = batchContext.MarkerBatchType()
+			continue
+		}
+
 		p.Contexts = append(p.Contexts, batchContext)
 	}
 
-	// Assume that it is a legacy batch at first
-	p.Type = BatchTypeLegacy
+	// Define a closure to clean up the reader used by the specified encoding.
+	var closeReader func() error
+	switch batchType {
 
-	// Handle backwards compatible batch types
-	if len(p.Contexts) > 0 && p.Contexts[0].Timestamp == 0 {
-		switch p.Contexts[0].BlockNumber {
-		case 0:
-			// zlib compressed transaction data
-			p.Type = BatchTypeZlib
-			// remove the first dummy context
-			p.Contexts = p.Contexts[1:]
-			numContexts--
+	// The legacy serialization does not require clsing, so we instatiate a
+	// dummy closure.
+	case BatchTypeLegacy:
+		closeReader = func() error { return nil }
 
+	// The zlib serialization requires decompression before reading the
+	// plaintext bytes, and also requires proper cleanup.
+	case BatchTypeZlib:
 		zr, err := zlib.NewReader(r)
 		if err != nil {
 			return err
 		}
-			defer zr.Close()
+		closeReader = zr.Close
 
 		r = bufio.NewReader(zr)
 	}
-	}
 
 	// Deserialize any transactions. Since the number of txs is ommitted
 	// from the encoding, loop until the stream is consumed.
@@ -315,7 +364,7 @@ func (p *AppendSequencerBatchParams) Read(r io.Reader) error {
 			if len(p.Txs) == 0 && len(p.Contexts) != 0 {
 				return ErrMalformedBatch
 			}
-			return nil
+			return closeReader()
 		} else if err != nil {
 			return err
 		}
@@ -327,7 +376,6 @@ func (p *AppendSequencerBatchParams) Read(r io.Reader) error {
 
 		p.Txs = append(p.Txs, NewCachedTx(tx))
 	}
-
 }
 
 // writeUint64 writes a the bottom `n` bytes of `val` to `w`.

go/batch-submitter/drivers/sequencer/encoding_test.go

@@ -119,7 +119,6 @@ func testAppendSequencerBatchParamsEncodeDecode(
 		TotalElementsToAppend: test.TotalElementsToAppend,
 		Contexts:              test.Contexts,
 		Txs:                   nil,
-		Type:                  sequencer.BatchTypeLegacy,
 	}
 
 	// Decode the batch from the test string.
@@ -133,7 +132,6 @@ func testAppendSequencerBatchParamsEncodeDecode(
 	} else {
 		require.Nil(t, err)
 	}
-	require.Equal(t, params.Type, sequencer.BatchTypeLegacy)
 
 	// Assert that the decoded params match the expected params. The
 	// transactions are compared serparetly (via hash), since the internal
@@ -149,7 +147,7 @@ func testAppendSequencerBatchParamsEncodeDecode(
 
 	// Finally, encode the decoded object and assert it matches the original
 	// hex string.
-	paramsBytes, err := params.Serialize()
+	paramsBytes, err := params.Serialize(sequencer.BatchTypeLegacy)
 
 	// Return early when testing error cases, no need to reserialize again
 	if test.Error {
@@ -161,17 +159,14 @@ func testAppendSequencerBatchParamsEncodeDecode(
 	require.Equal(t, test.HexEncoding, hex.EncodeToString(paramsBytes))
 
 	// Serialize the batches in compressed form
-	params.Type = sequencer.BatchTypeZlib
-	compressedParamsBytes, err := params.Serialize()
+	compressedParamsBytes, err := params.Serialize(sequencer.BatchTypeZlib)
 	require.Nil(t, err)
 
 	// Deserialize the compressed batch
 	var paramsCompressed sequencer.AppendSequencerBatchParams
 	err = paramsCompressed.Read(bytes.NewReader(compressedParamsBytes))
 	require.Nil(t, err)
-	require.Equal(t, paramsCompressed.Type, sequencer.BatchTypeZlib)
 
-	expParams.Type = sequencer.BatchTypeZlib
 	decompressedTxs := paramsCompressed.Txs
 	paramsCompressed.Txs = nil
 
@@ -189,3 +184,71 @@ func compareTxs(t *testing.T, a []*l2types.Transaction, b []*sequencer.CachedTx)
 		require.Equal(t, txA.Hash(), b[i].Tx().Hash())
 	}
 }
+
+// TestMarkerContext asserts that each batch type returns the correct marker
+// context.
+func TestMarkerContext(t *testing.T) {
+	batchTypes := []sequencer.BatchType{
+		sequencer.BatchTypeLegacy,
+		sequencer.BatchTypeZlib,
+	}
+
+	for _, batchType := range batchTypes {
+		t.Run(batchType.String(), func(t *testing.T) {
+			markerContext := batchType.MarkerContext()
+			if batchType == sequencer.BatchTypeLegacy {
+				require.Nil(t, markerContext)
+			} else {
+				require.NotNil(t, markerContext)
+
+				// All marker contexts MUST have a zero timestamp.
+				require.Equal(t, uint64(0), markerContext.Timestamp)
+
+				// Currently all other fields besides block number are defined
+				// as zero.
+				require.Equal(t, uint64(0), markerContext.NumSequencedTxs)
+				require.Equal(t, uint64(0), markerContext.NumSubsequentQueueTxs)
+
+				// Assert that the block number for each batch type is set to
+				// the correct constant.
+				switch batchType {
+				case sequencer.BatchTypeZlib:
+					require.Equal(t, uint64(0), markerContext.BlockNumber)
+				default:
+					t.Fatalf("unknown batch type")
+				}
+
+				// Ensure MarkerBatchType produces the expected BatchType.
+				require.Equal(t, batchType, markerContext.MarkerBatchType())
+			}
+		})
+	}
+}
+
+// TestIsMarkerContext asserts that IsMarkerContext returns true iff the
+// timestamp is zero.
+func TestIsMarkerContext(t *testing.T) {
+	batchContext := sequencer.BatchContext{
+		NumSequencedTxs:       1,
+		NumSubsequentQueueTxs: 2,
+		Timestamp:             3,
+		BlockNumber:           4,
+	}
+	require.False(t, batchContext.IsMarkerContext())
+
+	batchContext = sequencer.BatchContext{
+		NumSequencedTxs:       0,
+		NumSubsequentQueueTxs: 0,
+		Timestamp:             3,
+		BlockNumber:           0,
+	}
+	require.False(t, batchContext.IsMarkerContext())
+
+	batchContext = sequencer.BatchContext{
+		NumSequencedTxs:       1,
+		NumSubsequentQueueTxs: 2,
+		Timestamp:             0,
+		BlockNumber:           4,
+	}
+	require.True(t, batchContext.IsMarkerContext())
+}