Commit d42fc0b9 authored by zhiqiangxu's avatar zhiqiangxu Committed by GitHub

op-batcher,op-e2e: replace magic numbers like 6 with consts, eg MaxBlobsPerBlobTx (#11842)

* remove some magic numbers

* also check TargetNumFrames <= 6 for auto DA

* define eth.MaxBlobsPerBlobTx and replace magic 6

* also change the original params.MaxBlobGasPerBlock/params.BlobTxBlobGasPerBlob
parent 4a608f64
...@@ -160,7 +160,7 @@ func TestChannel_NextTxData_singleFrameTx(t *testing.T) { ...@@ -160,7 +160,7 @@ func TestChannel_NextTxData_singleFrameTx(t *testing.T) {
func TestChannel_NextTxData_multiFrameTx(t *testing.T) { func TestChannel_NextTxData_multiFrameTx(t *testing.T) {
require := require.New(t) require := require.New(t)
const n = 6 const n = eth.MaxBlobsPerBlobTx
lgr := testlog.Logger(t, log.LevelWarn) lgr := testlog.Logger(t, log.LevelWarn)
ch, err := newChannel(lgr, metrics.NoopMetrics, ChannelConfig{ ch, err := newChannel(lgr, metrics.NoopMetrics, ChannelConfig{
UseBlobs: true, UseBlobs: true,
......
...@@ -12,6 +12,7 @@ import ( ...@@ -12,6 +12,7 @@ import (
"github.com/ethereum-optimism/optimism/op-batcher/compressor" "github.com/ethereum-optimism/optimism/op-batcher/compressor"
"github.com/ethereum-optimism/optimism/op-batcher/flags" "github.com/ethereum-optimism/optimism/op-batcher/flags"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive" "github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum-optimism/optimism/op-service/eth"
oplog "github.com/ethereum-optimism/optimism/op-service/log" oplog "github.com/ethereum-optimism/optimism/op-service/log"
opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics" opmetrics "github.com/ethereum-optimism/optimism/op-service/metrics"
"github.com/ethereum-optimism/optimism/op-service/oppprof" "github.com/ethereum-optimism/optimism/op-service/oppprof"
...@@ -135,18 +136,19 @@ func (c *CLIConfig) Check() error { ...@@ -135,18 +136,19 @@ func (c *CLIConfig) Check() error {
if !derive.ValidCompressionAlgo(c.CompressionAlgo) { if !derive.ValidCompressionAlgo(c.CompressionAlgo) {
return fmt.Errorf("invalid compression algo %v", c.CompressionAlgo) return fmt.Errorf("invalid compression algo %v", c.CompressionAlgo)
} }
if c.BatchType > 1 { if c.BatchType > derive.SpanBatchType {
return fmt.Errorf("unknown batch type: %v", c.BatchType) return fmt.Errorf("unknown batch type: %v", c.BatchType)
} }
if c.CheckRecentTxsDepth > 128 { if c.CheckRecentTxsDepth > 128 {
return fmt.Errorf("CheckRecentTxsDepth cannot be set higher than 128: %v", c.CheckRecentTxsDepth) return fmt.Errorf("CheckRecentTxsDepth cannot be set higher than 128: %v", c.CheckRecentTxsDepth)
} }
if c.DataAvailabilityType == flags.BlobsType && c.TargetNumFrames > 6 {
return errors.New("too many frames for blob transactions, max 6")
}
if !flags.ValidDataAvailabilityType(c.DataAvailabilityType) { if !flags.ValidDataAvailabilityType(c.DataAvailabilityType) {
return fmt.Errorf("unknown data availability type: %q", c.DataAvailabilityType) return fmt.Errorf("unknown data availability type: %q", c.DataAvailabilityType)
} }
// we want to enforce it for both blobs and auto
if c.DataAvailabilityType != flags.CalldataType && c.TargetNumFrames > eth.MaxBlobsPerBlobTx {
return fmt.Errorf("too many frames for blob transactions, max %d", eth.MaxBlobsPerBlobTx)
}
if err := c.MetricsConfig.Check(); err != nil { if err := c.MetricsConfig.Check(); err != nil {
return err return err
} }
......
package batcher_test package batcher_test
import ( import (
"fmt"
"testing" "testing"
"time" "time"
...@@ -8,6 +9,7 @@ import ( ...@@ -8,6 +9,7 @@ import (
"github.com/ethereum-optimism/optimism/op-batcher/compressor" "github.com/ethereum-optimism/optimism/op-batcher/compressor"
"github.com/ethereum-optimism/optimism/op-batcher/flags" "github.com/ethereum-optimism/optimism/op-batcher/flags"
"github.com/ethereum-optimism/optimism/op-node/rollup/derive" "github.com/ethereum-optimism/optimism/op-node/rollup/derive"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/log" "github.com/ethereum-optimism/optimism/op-service/log"
"github.com/ethereum-optimism/optimism/op-service/metrics" "github.com/ethereum-optimism/optimism/op-service/metrics"
"github.com/ethereum-optimism/optimism/op-service/oppprof" "github.com/ethereum-optimism/optimism/op-service/oppprof"
...@@ -98,12 +100,12 @@ func TestBatcherConfig(t *testing.T) { ...@@ -98,12 +100,12 @@ func TestBatcherConfig(t *testing.T) {
errString: "TargetNumFrames must be at least 1", errString: "TargetNumFrames must be at least 1",
}, },
{ {
name: "larger 6 TargetNumFrames for blobs", name: fmt.Sprintf("larger %d TargetNumFrames for blobs", eth.MaxBlobsPerBlobTx),
override: func(c *batcher.CLIConfig) { override: func(c *batcher.CLIConfig) {
c.TargetNumFrames = 7 c.TargetNumFrames = eth.MaxBlobsPerBlobTx + 1
c.DataAvailabilityType = flags.BlobsType c.DataAvailabilityType = flags.BlobsType
}, },
errString: "too many frames for blob transactions, max 6", errString: fmt.Sprintf("too many frames for blob transactions, max %d", eth.MaxBlobsPerBlobTx),
}, },
{ {
name: "invalid compr ratio for ratio compressor", name: "invalid compr ratio for ratio compressor",
......
...@@ -14,6 +14,7 @@ import ( ...@@ -14,6 +14,7 @@ import (
batcherFlags "github.com/ethereum-optimism/optimism/op-batcher/flags" batcherFlags "github.com/ethereum-optimism/optimism/op-batcher/flags"
"github.com/ethereum-optimism/optimism/op-e2e/e2eutils" "github.com/ethereum-optimism/optimism/op-e2e/e2eutils"
"github.com/ethereum-optimism/optimism/op-node/rollup/sync" "github.com/ethereum-optimism/optimism/op-node/rollup/sync"
"github.com/ethereum-optimism/optimism/op-service/eth"
"github.com/ethereum-optimism/optimism/op-service/testlog" "github.com/ethereum-optimism/optimism/op-service/testlog"
) )
...@@ -104,10 +105,10 @@ func TestEIP4844MultiBlobs(gt *testing.T) { ...@@ -104,10 +105,10 @@ func TestEIP4844MultiBlobs(gt *testing.T) {
sequencer.ActBuildToL1Head(t) sequencer.ActBuildToL1Head(t)
// submit all new L2 blocks // submit all new L2 blocks
batcher.ActSubmitAllMultiBlobs(t, 6) batcher.ActSubmitAllMultiBlobs(t, eth.MaxBlobsPerBlobTx)
batchTx := batcher.LastSubmitted batchTx := batcher.LastSubmitted
require.Equal(t, uint8(types.BlobTxType), batchTx.Type(), "batch tx must be blob-tx") require.Equal(t, uint8(types.BlobTxType), batchTx.Type(), "batch tx must be blob-tx")
require.Len(t, batchTx.BlobTxSidecar().Blobs, 6) require.Len(t, batchTx.BlobTxSidecar().Blobs, eth.MaxBlobsPerBlobTx)
// new L1 block with L2 batch // new L1 block with L2 batch
miner.ActL1StartBlock(12)(t) miner.ActL1StartBlock(12)(t)
......
...@@ -346,8 +346,8 @@ func (s *L2Batcher) ActL2BatchSubmitMultiBlob(t Testing, numBlobs int) { ...@@ -346,8 +346,8 @@ func (s *L2Batcher) ActL2BatchSubmitMultiBlob(t Testing, numBlobs int) {
if s.l2BatcherCfg.DataAvailabilityType != batcherFlags.BlobsType { if s.l2BatcherCfg.DataAvailabilityType != batcherFlags.BlobsType {
t.InvalidAction("ActL2BatchSubmitMultiBlob only available for Blobs DA type") t.InvalidAction("ActL2BatchSubmitMultiBlob only available for Blobs DA type")
return return
} else if numBlobs > 6 || numBlobs < 1 { } else if numBlobs > eth.MaxBlobsPerBlobTx || numBlobs < 1 {
t.InvalidAction("invalid number of blobs %d, must be within [1,6]", numBlobs) t.InvalidAction("invalid number of blobs %d, must be within [1,%d]", numBlobs, eth.MaxBlobsPerBlobTx)
} }
// Don't run this action if there's no data to submit // Don't run this action if there's no data to submit
......
...@@ -2,6 +2,7 @@ package da ...@@ -2,6 +2,7 @@ package da
import ( import (
"context" "context"
"fmt"
"math/big" "math/big"
"math/rand" "math/rand"
"testing" "testing"
...@@ -50,12 +51,12 @@ func testSystem4844E2E(t *testing.T, multiBlob bool, daType batcherFlags.DataAva ...@@ -50,12 +51,12 @@ func testSystem4844E2E(t *testing.T, multiBlob bool, daType batcherFlags.DataAva
cfg.BatcherBatchType = derive.SpanBatchType cfg.BatcherBatchType = derive.SpanBatchType
cfg.DeployConfig.L1GenesisBlockBaseFeePerGas = (*hexutil.Big)(big.NewInt(7000)) cfg.DeployConfig.L1GenesisBlockBaseFeePerGas = (*hexutil.Big)(big.NewInt(7000))
const maxBlobs = 6 const maxBlobs = eth.MaxBlobsPerBlobTx
var maxL1TxSize int var maxL1TxSize int
if multiBlob { if multiBlob {
cfg.BatcherTargetNumFrames = 6 cfg.BatcherTargetNumFrames = eth.MaxBlobsPerBlobTx
cfg.BatcherUseMaxTxSizeForBlobs = true cfg.BatcherUseMaxTxSizeForBlobs = true
// leads to 6 blobs for an L2 block with a user tx with 400 random bytes // leads to eth.MaxBlobsPerBlobTx blobs for an L2 block with a user tx with 400 random bytes
// while all other L2 blocks take 1 blob (deposit tx) // while all other L2 blocks take 1 blob (deposit tx)
maxL1TxSize = derive.FrameV0OverHeadSize + 100 maxL1TxSize = derive.FrameV0OverHeadSize + 100
cfg.BatcherMaxL1TxSizeBytes = uint64(maxL1TxSize) cfg.BatcherMaxL1TxSizeBytes = uint64(maxL1TxSize)
...@@ -129,7 +130,7 @@ func testSystem4844E2E(t *testing.T, multiBlob bool, daType batcherFlags.DataAva ...@@ -129,7 +130,7 @@ func testSystem4844E2E(t *testing.T, multiBlob bool, daType batcherFlags.DataAva
opts.Value = big.NewInt(1_000_000_000) opts.Value = big.NewInt(1_000_000_000)
opts.Nonce = 1 // Already have deposit opts.Nonce = 1 // Already have deposit
opts.ToAddr = &common.Address{0xff, 0xff} opts.ToAddr = &common.Address{0xff, 0xff}
// put some random data in the tx to make it fill up 6 blobs (multi-blob case) // put some random data in the tx to make it fill up eth.MaxBlobsPerBlobTx blobs (multi-blob case)
opts.Data = testutils.RandomData(rand.New(rand.NewSource(420)), 400) opts.Data = testutils.RandomData(rand.New(rand.NewSource(420)), 400)
opts.Gas, err = core.IntrinsicGas(opts.Data, nil, false, true, true, false) opts.Gas, err = core.IntrinsicGas(opts.Data, nil, false, true, true, false)
require.NoError(t, err) require.NoError(t, err)
...@@ -207,7 +208,7 @@ func testSystem4844E2E(t *testing.T, multiBlob bool, daType batcherFlags.DataAva ...@@ -207,7 +208,7 @@ func testSystem4844E2E(t *testing.T, multiBlob bool, daType batcherFlags.DataAva
if !multiBlob { if !multiBlob {
require.NotZero(t, numBlobs, "single-blob: expected to find L1 blob tx") require.NotZero(t, numBlobs, "single-blob: expected to find L1 blob tx")
} else { } else {
require.Equal(t, maxBlobs, numBlobs, "multi-blob: expected to find L1 blob tx with 6 blobs") require.Equal(t, maxBlobs, numBlobs, fmt.Sprintf("multi-blob: expected to find L1 blob tx with %d blobs", eth.MaxBlobsPerBlobTx))
// blob tx should have filled up all but last blob // blob tx should have filled up all but last blob
bcl := sys.L1BeaconHTTPClient() bcl := sys.L1BeaconHTTPClient()
hashes := toIndexedBlobHashes(blobTx.BlobHashes()...) hashes := toIndexedBlobHashes(blobTx.BlobHashes()...)
...@@ -255,7 +256,7 @@ func TestBatcherAutoDA(t *testing.T) { ...@@ -255,7 +256,7 @@ func TestBatcherAutoDA(t *testing.T) {
cfg.DeployConfig.L1GenesisBlockGasLimit = 2_500_000 // low block gas limit to drive up gas price more quickly cfg.DeployConfig.L1GenesisBlockGasLimit = 2_500_000 // low block gas limit to drive up gas price more quickly
t.Logf("L1BlockTime: %d, L2BlockTime: %d", cfg.DeployConfig.L1BlockTime, cfg.DeployConfig.L2BlockTime) t.Logf("L1BlockTime: %d, L2BlockTime: %d", cfg.DeployConfig.L1BlockTime, cfg.DeployConfig.L2BlockTime)
cfg.BatcherTargetNumFrames = 6 cfg.BatcherTargetNumFrames = eth.MaxBlobsPerBlobTx
sys, err := cfg.Start(t) sys, err := cfg.Start(t)
require.NoError(t, err, "Error starting up system") require.NoError(t, err, "Error starting up system")
......
...@@ -9,6 +9,7 @@ import ( ...@@ -9,6 +9,7 @@ import (
"github.com/ethereum/go-ethereum/common" "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/hexutil" "github.com/ethereum/go-ethereum/common/hexutil"
"github.com/ethereum/go-ethereum/crypto/kzg4844" "github.com/ethereum/go-ethereum/crypto/kzg4844"
"github.com/ethereum/go-ethereum/params"
) )
const ( const (
...@@ -17,6 +18,7 @@ const ( ...@@ -17,6 +18,7 @@ const (
EncodingVersion = 0 EncodingVersion = 0
VersionOffset = 1 // offset of the version byte in the blob encoding VersionOffset = 1 // offset of the version byte in the blob encoding
Rounds = 1024 // number of encode/decode rounds Rounds = 1024 // number of encode/decode rounds
MaxBlobsPerBlobTx = params.MaxBlobGasPerBlock / params.BlobTxBlobGasPerBlob
) )
var ( var (
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment