From 626ae08ebf39004b04c42cc09a760f4c98f83d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Thu, 17 Aug 2023 19:55:56 +0200 Subject: [PATCH] fix: update worker nextL1MsgIndex for unknown errors (#464) * fix: update worker nextL1MsgIndex for strange errors * more tests * bump version * refactor: add more context to tracing failure (#466) * bump version * bump version --- miner/worker.go | 2 ++ miner/worker_test.go | 75 ++++++++++++++++++++++++++++++++++++++++---- params/version.go | 2 +- 3 files changed, 72 insertions(+), 7 deletions(-) diff --git a/miner/worker.go b/miner/worker.go index 57144697f7..5c1aeec754 100644 --- a/miner/worker.go +++ b/miner/worker.go @@ -978,6 +978,7 @@ loop: "expected", w.current.nextL1MsgIndex, "got", tx.AsL1MessageTx().QueueIndex, ) + break } if !tx.IsL1MessageTx() && !w.chainConfig.Scroll.IsValidBlockSize(w.current.blockSize+tx.Size()) { log.Trace("Block size limit reached", "have", w.current.blockSize, "want", w.chainConfig.Scroll.MaxTxPayloadBytesPerBlock, "tx", tx.Size()) @@ -1093,6 +1094,7 @@ loop: if tx.IsL1MessageTx() { queueIndex := tx.AsL1MessageTx().QueueIndex log.Info("Skipping L1 message", "queueIndex", queueIndex, "tx", tx.Hash().String(), "block", w.current.header.Number, "reason", "strange error", "err", err) + w.current.nextL1MsgIndex = queueIndex + 1 } txs.Shift() } diff --git a/miner/worker_test.go b/miner/worker_test.go index a28e9b5313..0ac7d23482 100644 --- a/miner/worker_test.go +++ b/miner/worker_test.go @@ -732,7 +732,7 @@ func TestL1MsgCorrectOrder(t *testing.T) { } } -func l1MessageTest(t *testing.T, msgs []types.L1MessageTx, callback func(i int, block *types.Block, db ethdb.Database) bool) { +func l1MessageTest(t *testing.T, msgs []types.L1MessageTx, withL2Tx bool, callback func(i int, block *types.Block, db ethdb.Database) bool) { var ( engine consensus.Engine chainConfig *params.ChainConfig @@ -770,13 +770,19 @@ func l1MessageTest(t *testing.T, msgs []types.L1MessageTx, callback func(i int, sub := w.mux.Subscribe(core.NewMinedBlockEvent{}) defer sub.Unsubscribe() + if withL2Tx { + b.txPool.AddLocal(b.newRandomTx(false)) + } + // Start mining! w.start() + // timeout for all blocks + globalTimeout := time.After(3 * time.Second) + for ii := 1; true; ii++ { - // timeout for all blocks select { - case <-time.After(3 * time.Second): + case <-globalTimeout: t.Fatalf("timeout") default: } @@ -805,7 +811,7 @@ func TestL1SingleMessageOverGasLimit(t *testing.T) { {QueueIndex: 2, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{3}}, // different sender } - l1MessageTest(t, msgs, func(_i int, block *types.Block, db ethdb.Database) bool { + l1MessageTest(t, msgs, false, func(_i int, block *types.Block, db ethdb.Database) bool { // skip #0, include #1 and #2 assert.Equal(2, len(block.Transactions())) @@ -834,7 +840,7 @@ func TestL1CombinedMessagesOverGasLimit(t *testing.T) { {QueueIndex: 2, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{3}}, // different sender } - l1MessageTest(t, msgs, func(blockNum int, block *types.Block, db ethdb.Database) bool { + l1MessageTest(t, msgs, false, func(blockNum int, block *types.Block, db ethdb.Database) bool { switch blockNum { case 1: // block #1 only includes 1 message @@ -876,7 +882,7 @@ func TestLargeL1MessageSkipPayloadCheck(t *testing.T) { {QueueIndex: 2, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{3}}, // different sender } - l1MessageTest(t, msgs, func(blockNum int, block *types.Block, db ethdb.Database) bool { + l1MessageTest(t, msgs, false, func(blockNum int, block *types.Block, db ethdb.Database) bool { // include #0, #1 and #2 assert.Equal(3, len(block.Transactions())) @@ -895,3 +901,60 @@ func TestLargeL1MessageSkipPayloadCheck(t *testing.T) { return true }) } + +func TestSkipMessageWithStrangeError(t *testing.T) { + assert := assert.New(t) + + // message #0 is skipped because of `Value` + // TODO: trigger skipping in some other way after this behaviour is changed + msgs := []types.L1MessageTx{ + {QueueIndex: 0, Gas: 25100, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}, Value: big.NewInt(1)}, + {QueueIndex: 1, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}}, + {QueueIndex: 2, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{3}}, + } + + l1MessageTest(t, msgs, false, func(blockNum int, block *types.Block, db ethdb.Database) bool { + // skip #0, include #1 and #2 + assert.Equal(2, len(block.Transactions())) + assert.True(block.Transactions()[0].IsL1MessageTx()) + + assert.True(block.Transactions()[0].IsL1MessageTx()) + assert.Equal(uint64(1), block.Transactions()[0].AsL1MessageTx().QueueIndex) + assert.True(block.Transactions()[1].IsL1MessageTx()) + assert.Equal(uint64(2), block.Transactions()[1].AsL1MessageTx().QueueIndex) + + // db is updated correctly + queueIndex := rawdb.ReadFirstQueueIndexNotInL2Block(db, block.Hash()) + assert.NotNil(queueIndex) + assert.Equal(uint64(3), *queueIndex) + + return true + }) +} + +func TestSkipAllL1MessagesInBlock(t *testing.T) { + assert := assert.New(t) + + // messages are skipped because of `Value` + // TODO: trigger skipping in some other way after this behaviour is changed + msgs := []types.L1MessageTx{ + {QueueIndex: 0, Gas: 25100, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}, Value: big.NewInt(1)}, + {QueueIndex: 1, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}, Value: big.NewInt(1)}, + {QueueIndex: 2, Gas: 21016, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{3}, Value: big.NewInt(1)}, + } + + l1MessageTest(t, msgs, true, func(blockNum int, block *types.Block, db ethdb.Database) bool { + // skip all 3 L1 messages, include 1 L2 tx + assert.Equal(1, len(block.Transactions())) + assert.False(block.Transactions()[0].IsL1MessageTx()) + + // db is updated correctly + // note: this should return 0 but on the signer we store 3 instead so + // that we do not process the same messages again for the next block. + queueIndex := rawdb.ReadFirstQueueIndexNotInL2Block(db, block.Hash()) + assert.NotNil(queueIndex) + assert.Equal(uint64(3), *queueIndex) + + return true + }) +} diff --git a/params/version.go b/params/version.go index 5e5478ea75..12bcc73e65 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 = 37 // Patch version component of the current release + VersionPatch = 38 // Patch version component of the current release VersionMeta = "sepolia" // Version metadata to append to the version string )