From d650299a36d88f7d8c2592c04d797e41e72ac5c4 Mon Sep 17 00:00:00 2001 From: HAOYUatHZ <37070449+HAOYUatHZ@users.noreply.github.com> Date: Sun, 27 Aug 2023 00:08:04 +0800 Subject: [PATCH] feat(miner): consider l1msg in txCount (#486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(miner): consider l1msg in txCount * refactor * tweak * fix comments * fix unit test * set chainConfig.Scroll.MaxTxPerBlock in l1 message test * add a skipped tx in TestAcceptableTxlimit * add l1 skipped msg in test * fix unit test * fix unit test * Update miner/worker.go Co-authored-by: Péter Garamvölgyi * revert some test changes * tweak * bump version --------- Co-authored-by: colinlyguo Co-authored-by: colin <102356659+colinlyguo@users.noreply.github.com> Co-authored-by: Péter Garamvölgyi --- core/block_validator.go | 2 +- miner/worker.go | 9 ++--- miner/worker_test.go | 76 ++++++++++++++++++++++++++++++++++++++--- params/config.go | 4 +-- params/version.go | 2 +- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/core/block_validator.go b/core/block_validator.go index 2038bd4f0d..ab0be766f0 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -70,7 +70,7 @@ func (v *BlockValidator) ValidateBody(block *types.Block) error { if v.bc.HasBlockAndState(block.Hash(), block.NumberU64()) { return ErrKnownBlock } - if !v.config.Scroll.IsValidL2TxCount(block.CountL2Tx()) { + if !v.config.Scroll.IsValidTxCount(len(block.Transactions())) { return consensus.ErrInvalidTxCount } // Check if block payload size is smaller than the max size diff --git a/miner/worker.go b/miner/worker.go index 5c99c2be6f..3742d4421e 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -978,12 +978,9 @@ loop: break } // If we have collected enough transactions then we're done - l2TxCount := w.current.tcount - w.current.l1TxCount - if !tx.IsL1MessageTx() { // If the next tx is not L1MessageTx type then +1. - l2TxCount++ - } - if !w.chainConfig.Scroll.IsValidL2TxCount(l2TxCount) { - log.Trace("Transaction count limit reached", "have", w.current.tcount-w.current.l1TxCount, "want", w.chainConfig.Scroll.MaxTxPerBlock) + // Originally we only limit l2txs count, but now strictly limit total txs number. + if !w.chainConfig.Scroll.IsValidTxCount(w.current.tcount + 1) { + log.Trace("Transaction count limit reached", "have", w.current.tcount, "want", w.chainConfig.Scroll.MaxTxPerBlock) break } if tx.IsL1MessageTx() && tx.AsL1MessageTx().QueueIndex != w.current.nextL1MsgIndex { diff --git a/miner/worker_test.go b/miner/worker_test.go index a215a60ec9..d462310975 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -606,7 +606,7 @@ func testGenerateBlockWithL1Msg(t *testing.T, isClique bool) { } } -func TestExcludeL1MsgFromTxlimit(t *testing.T) { +func TestAcceptableTxlimit(t *testing.T) { assert := assert.New(t) var ( engine consensus.Engine @@ -617,8 +617,72 @@ func TestExcludeL1MsgFromTxlimit(t *testing.T) { chainConfig.Clique = ¶ms.CliqueConfig{Period: 1, Epoch: 30000} engine = clique.New(chainConfig.Clique, db) - // Set maxTxPerBlock = 2 and NumL1MessagesPerBlock = 2 - maxTxPerBlock := 2 + // Set maxTxPerBlock = 4, which >= non-l1msg + non-skipped l1msg txs + maxTxPerBlock := 4 + chainConfig.Scroll.MaxTxPerBlock = &maxTxPerBlock + chainConfig.Scroll.L1Config = ¶ms.L1Config{ + NumL1MessagesPerBlock: 3, + } + + // Insert 3 l1msgs, with one be skipped. + l1msgs := []types.L1MessageTx{ + {QueueIndex: 0, Gas: 10000000, To: &common.Address{3}, Data: []byte{0x01}, Sender: common.Address{4}}, // over gas limit + {QueueIndex: 1, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{4}}, + {QueueIndex: 2, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}}} + rawdb.WriteL1Messages(db, l1msgs) + + chainConfig.LondonBlock = big.NewInt(0) + w, b := newTestWorker(t, chainConfig, engine, db, 0) + defer w.close() + + // This test chain imports the mined blocks. + b.genesis.MustCommit(db) + chain, _ := core.NewBlockChain(db, nil, b.chain.Config(), engine, vm.Config{ + Debug: true, + Tracer: vm.NewStructLogger(&vm.LogConfig{EnableMemory: true, EnableReturnData: true})}, nil, nil, false) + defer chain.Stop() + + // Ignore empty commit here for less noise. + w.skipSealHook = func(task *task) bool { + return len(task.receipts) == 0 + } + + // Wait for mined blocks. + sub := w.mux.Subscribe(core.NewMinedBlockEvent{}) + defer sub.Unsubscribe() + + // Insert 2 non-l1msg txs + b.txPool.AddLocal(b.newRandomTx(true)) + b.txPool.AddLocal(b.newRandomTx(false)) + + // Start mining! + w.start() + + select { + case ev := <-sub.Chan(): + block := ev.Data.(core.NewMinedBlockEvent).Block + if _, err := chain.InsertChain([]*types.Block{block}); err != nil { + t.Fatalf("failed to insert new mined block %d: %v", block.NumberU64(), err) + } + assert.Equal(4, len(block.Transactions())) + case <-time.After(3 * time.Second): + t.Fatalf("timeout") + } +} + +func TestUnacceptableTxlimit(t *testing.T) { + assert := assert.New(t) + var ( + engine consensus.Engine + chainConfig *params.ChainConfig + db = rawdb.NewMemoryDatabase() + ) + chainConfig = params.AllCliqueProtocolChanges + chainConfig.Clique = ¶ms.CliqueConfig{Period: 1, Epoch: 30000} + engine = clique.New(chainConfig.Clique, db) + + // Set maxTxPerBlock = 3, which < non-l1msg + l1msg txs + maxTxPerBlock := 3 chainConfig.Scroll.MaxTxPerBlock = &maxTxPerBlock chainConfig.Scroll.L1Config = ¶ms.L1Config{ NumL1MessagesPerBlock: 2, @@ -663,7 +727,7 @@ func TestExcludeL1MsgFromTxlimit(t *testing.T) { if _, err := chain.InsertChain([]*types.Block{block}); err != nil { t.Fatalf("failed to insert new mined block %d: %v", block.NumberU64(), err) } - assert.Equal(4, len(block.Transactions())) + assert.Equal(3, len(block.Transactions())) case <-time.After(3 * time.Second): t.Fatalf("timeout") } @@ -680,6 +744,8 @@ func TestL1MsgCorrectOrder(t *testing.T) { chainConfig.Clique = ¶ms.CliqueConfig{Period: 1, Epoch: 30000} engine = clique.New(chainConfig.Clique, db) + maxTxPerBlock := 4 + chainConfig.Scroll.MaxTxPerBlock = &maxTxPerBlock chainConfig.Scroll.L1Config = ¶ms.L1Config{ NumL1MessagesPerBlock: 10, } @@ -744,6 +810,8 @@ func l1MessageTest(t *testing.T, msgs []types.L1MessageTx, withL2Tx bool, callba chainConfig = params.AllCliqueProtocolChanges chainConfig.Clique = ¶ms.CliqueConfig{Period: 1, Epoch: 30000} engine = clique.New(chainConfig.Clique, db) + maxTxPerBlock := 4 + chainConfig.Scroll.MaxTxPerBlock = &maxTxPerBlock maxPayload := 1024 chainConfig.Scroll.MaxTxPayloadBytesPerBlock = &maxPayload diff --git a/params/config.go b/params/config.go index 012f88bc3a..134aee48c6 100644 --- a/params/config.go +++ b/params/config.go @@ -552,9 +552,9 @@ func (s ScrollConfig) String() string { s.UseZktrie, maxTxPerBlock, maxTxPayloadBytesPerBlock, s.FeeVaultAddress, s.EnableEIP2718, s.EnableEIP1559, s.L1Config.String()) } -// IsValidL2TxCount returns whether the given block's L2 transaction count is below the limit. +// IsValidTxCount returns whether the given block's transaction count is below the limit. // This limit corresponds to the number of ECDSA signature checks that we can fit into the zkEVM. -func (s ScrollConfig) IsValidL2TxCount(count int) bool { +func (s ScrollConfig) IsValidTxCount(count int) bool { return s.MaxTxPerBlock == nil || count <= *s.MaxTxPerBlock } diff --git a/params/version.go b/params/version.go index 4b67f0af26..e2ec33324e 100644 --- a/params/version.go +++ b/params/version.go @@ -24,7 +24,7 @@ import ( const ( VersionMajor = 4 // Major version component of the current release VersionMinor = 3 // Minor version component of the current release - VersionPatch = 53 // Patch version component of the current release + VersionPatch = 54 // Patch version component of the current release VersionMeta = "sepolia" // Version metadata to append to the version string )