From 3885aea6099a54f669757071a94bd0db167c73e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Fri, 11 Aug 2023 18:15:21 +0800 Subject: [PATCH 1/4] feat: include L1 message with insufficient balance --- core/state_transition.go | 2 +- miner/worker_test.go | 40 ++++++++++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index 3aeb75c02ed4..1793ea9ec4e9 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -359,7 +359,7 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { st.gas -= gas // Check clause 6 - if msg.Value().Sign() > 0 && !st.evm.Context.CanTransfer(st.state, msg.From(), msg.Value()) { + if msg.Value().Sign() > 0 && !st.evm.Context.CanTransfer(st.state, msg.From(), msg.Value()) && !msg.IsL1MessageTx() { return nil, fmt.Errorf("%w: address %v", ErrInsufficientFundsForTransfer, msg.From().Hex()) } diff --git a/miner/worker_test.go b/miner/worker_test.go index a28e9b5313c8..a71e02d0f0ea 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, callback func(i int, block *types.Block, db ethdb.Database, bc *core.BlockChain) bool) { var ( engine consensus.Engine chainConfig *params.ChainConfig @@ -784,8 +784,8 @@ func l1MessageTest(t *testing.T, msgs []types.L1MessageTx, callback func(i int, select { case ev := <-sub.Chan(): block := ev.Data.(core.NewMinedBlockEvent).Block - // TODO - if callback(ii, block, db) { + + if done := callback(ii, block, db, chain); done { return } @@ -805,7 +805,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, func(_ int, block *types.Block, db ethdb.Database, _ *core.BlockChain) bool { // skip #0, include #1 and #2 assert.Equal(2, len(block.Transactions())) @@ -834,7 +834,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, func(blockNum int, block *types.Block, db ethdb.Database, _ *core.BlockChain) bool { switch blockNum { case 1: // block #1 only includes 1 message @@ -876,7 +876,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, func(blockNum int, block *types.Block, db ethdb.Database, _ *core.BlockChain) bool { // include #0, #1 and #2 assert.Equal(3, len(block.Transactions())) @@ -895,3 +895,31 @@ func TestLargeL1MessageSkipPayloadCheck(t *testing.T) { return true }) } + +func TestL1MessageWithInsufficientBalanceNotSkipped(t *testing.T) { + assert := assert.New(t) + + // message #0 sends more funds than available in the sender account + msgs := []types.L1MessageTx{ + {QueueIndex: 0, Gas: 25100, To: &common.Address{1}, Data: make([]byte, 1025), Sender: common.Address{2}, Value: big.NewInt(1)}, + } + + l1MessageTest(t, msgs, func(blockNum int, block *types.Block, db ethdb.Database, bc *core.BlockChain) bool { + // include #0 + assert.Equal(1, len(block.Transactions())) + assert.True(block.Transactions()[0].IsL1MessageTx()) + assert.Equal(uint64(0), block.Transactions()[0].AsL1MessageTx().QueueIndex) + + // failing receipt is stored correctly + receipts := bc.GetReceiptsByHash(block.Hash()) + assert.Equal(1, len(receipts)) + assert.Equal(types.ReceiptStatusFailed, receipts[0].Status) + + // db is updated correctly + queueIndex := rawdb.ReadFirstQueueIndexNotInL2Block(db, block.Hash()) + assert.NotNil(queueIndex) + assert.Equal(uint64(1), *queueIndex) + + return true + }) +} From b8d60b268a4a52aed1e6fb16d271b3a5a00e4bb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Mon, 20 Jan 2025 15:35:49 +0100 Subject: [PATCH 2/4] fix tests --- miner/scroll_worker_test.go | 58 ++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 8 deletions(-) diff --git a/miner/scroll_worker_test.go b/miner/scroll_worker_test.go index aa207f1bd721..5f79902f0e15 100644 --- a/miner/scroll_worker_test.go +++ b/miner/scroll_worker_test.go @@ -708,10 +708,12 @@ func TestLargeL1MessageSkipPayloadCheck(t *testing.T) { 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)}, + // message #0 is skipped because of `GasLimit` + // (cannot happen in practice, this is checked in the contracts) + {QueueIndex: 0, Gas: 20000000, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}}, + + // messages #1 and #2 are correct {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}}, } @@ -742,15 +744,55 @@ func TestSkipMessageWithStrangeError(t *testing.T) { }) } +func TestL1MessageWithInsufficientBalanceNotSkipped(t *testing.T) { + assert := assert.New(t) + + msgs := []types.L1MessageTx{ + // message #0 sends more funds than available in the sender account + {QueueIndex: 0, Gas: 25100, To: &common.Address{1}, Data: make([]byte, 1025), Sender: common.Address{2}, Value: big.NewInt(1)}, + + // #message #1 is a correct msg + {QueueIndex: 1, Gas: 25100, To: &common.Address{1}, Data: make([]byte, 1025), Sender: common.Address{2}}, + } + + l1MessageTest(t, msgs, false, func(blockNum int, block *types.Block, db ethdb.Database, w *worker, bc *core.BlockChain) bool { + switch blockNum { + case 0: + return false + case 1: + // include both #0 and #1 + assert.Equal(2, len(block.Transactions())) + assert.True(block.Transactions()[0].IsL1MessageTx()) + assert.Equal(uint64(0), block.Transactions()[0].AsL1MessageTx().QueueIndex) + assert.True(block.Transactions()[1].IsL1MessageTx()) + assert.Equal(uint64(1), block.Transactions()[1].AsL1MessageTx().QueueIndex) + + // #0 fails, #1 succeeds + receipts := bc.GetReceiptsByHash(block.Hash()) + assert.Equal(2, len(receipts)) + assert.Equal(types.ReceiptStatusFailed, receipts[0].Status) + assert.Equal(types.ReceiptStatusSuccessful, receipts[1].Status) + + // db is updated correctly + queueIndex := rawdb.ReadFirstQueueIndexNotInL2Block(db, block.Hash()) + assert.NotNil(queueIndex) + assert.Equal(uint64(2), *queueIndex) + + return true + default: + 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 + // messages are skipped because of `GasLimit` 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)}, + {QueueIndex: 0, Gas: 20000000, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}}, + {QueueIndex: 1, Gas: 20000000, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{2}}, + {QueueIndex: 2, Gas: 20000000, To: &common.Address{1}, Data: []byte{0x01}, Sender: common.Address{3}}, } l1MessageTest(t, msgs, true, func(blockNum int, block *types.Block, db ethdb.Database, w *worker, bc *core.BlockChain) bool { From cd5d222f2e409332d03ef112aa0aa05d7d1859f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Mon, 20 Jan 2025 15:52:43 +0100 Subject: [PATCH 3/4] add comment --- core/state_transition.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/state_transition.go b/core/state_transition.go index 3f4138d4b468..5aeb7ca86898 100644 --- a/core/state_transition.go +++ b/core/state_transition.go @@ -374,6 +374,9 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { st.gas -= gas // Check clause 6 + // Note: If this is an L1MessageTx, we will not return a top-level ErrInsufficientFundsForTransfer. + // Instead, we return ErrInsufficientBalance from within st.evm.Call. This means that such transactions + // will revert but they can be included in a valid block. This is necessary for supporting enforced txs. if msg.Value().Sign() > 0 && !st.evm.Context.CanTransfer(st.state, msg.From(), msg.Value()) && !msg.IsL1MessageTx() { return nil, fmt.Errorf("%w: address %v", ErrInsufficientFundsForTransfer, msg.From().Hex()) } From e1d647319f331080eb6ec4acab0dd58f5f9ffe15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A9ter=20Garamv=C3=B6lgyi?= Date: Tue, 21 Jan 2025 08:42:30 +0100 Subject: [PATCH 4/4] bump version --- params/version.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/params/version.go b/params/version.go index d8971caf4ac0..9bffc4505f0f 100644 --- a/params/version.go +++ b/params/version.go @@ -24,7 +24,7 @@ import ( const ( VersionMajor = 5 // Major version component of the current release VersionMinor = 8 // Minor version component of the current release - VersionPatch = 0 // Patch version component of the current release + VersionPatch = 1 // Patch version component of the current release VersionMeta = "mainnet" // Version metadata to append to the version string )