add flag to precompile to determine whether it called form worker, add test

Author: NazariiDenha

core/state_transition.go

@@ -413,6 +413,8 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
 		ret, st.gas, vmerr = st.evm.Call(sender, st.to(), st.data, st.gas, st.value)
 		stateTransitionEvmCallExecutionTimer.Update(time.Since(evmCallStart))
 	}
+
+	// This error can only be caught if CallerType in vm config is worker, worker will reinsert tx into txpool in case of this error
 	var errL1 *vm.ErrL1RPCError
 	if errors.As(vmerr, &errL1) {
 		return nil, vmerr

core/vm/contracts.go

@@ -22,6 +22,7 @@ import (
 	"encoding/binary"
 	"errors"
 	"math/big"
+	"time"
 
 	"github.com/scroll-tech/go-ethereum/common"
 	"github.com/scroll-tech/go-ethereum/common/math"
@@ -145,7 +146,7 @@ func PrecompiledContractsDescartes(cfg Config) map[common.Address]PrecompiledCon
 		common.BytesToAddress([]byte{8}): &bn256PairingIstanbul{},
 		common.BytesToAddress([]byte{9}): &blake2FDisabled{},
 		// TODO final contract address to be decided
-		common.BytesToAddress([]byte{1, 1}): &l1sload{l1Client: cfg.L1Client},
+		common.BytesToAddress([]byte{1, 1}): &l1sload{l1Client: cfg.L1Client, callerType: cfg.CallerType},
 	}
 }
 
@@ -1164,7 +1165,8 @@ func (c *bls12381MapG2) Run(state StateDB, input []byte) ([]byte, error) {
 
 // L1SLoad precompiled
 type l1sload struct {
-	l1Client L1Client
+	l1Client   L1Client
+	callerType CallerType
 }
 
 // RequiredGas returns the gas required to execute the pre-compiled contract.
@@ -1179,7 +1181,8 @@ func (c *l1sload) RequiredGas(input []byte) uint64 {
 }
 
 func (c *l1sload) Run(state StateDB, input []byte) ([]byte, error) {
-	const l1ClientRequestAttempts = 3
+	log.Info("l1sload", "input", input)
+	const l1ClientMaxRetries = 3
 
 	if c.l1Client == nil {
 		log.Error("No L1Client in the l1sload")
@@ -1200,16 +1203,29 @@ func (c *l1sload) Run(state StateDB, input []byte) ([]byte, error) {
 		keys[i] = common.BytesToHash(input[20+32*i : 52+32*i])
 	}
 
-	var res []byte
-	var err error
-	for i := 0; i < l1ClientRequestAttempts; i++ {
-		res, err = c.l1Client.StoragesAt(context.Background(), address, keys, block)
-		if err != nil {
-			continue
-		} else {
-			return res, nil
+	// if caller type is non-worker then we can retry request multiple times and return err, the tx will be reinserted in tx poll
+	// otherwise, we should retry requests forever
+	if c.callerType == CallerTypeNonWorker {
+		for {
+			res, err := c.l1Client.StoragesAt(context.Background(), address, keys, block)
+			if err == nil {
+				return res, nil
+			}
+			// wait before retrying
+			time.Sleep(100 * time.Millisecond)
+			log.Warn("L1 client request error", "err", err)
 		}
+	} else {
+		var innerErr error
+		for i := 0; i < l1ClientMaxRetries; i++ {
+			res, err := c.l1Client.StoragesAt(context.Background(), address, keys, block)
+			if err != nil {
+				innerErr = err
+				continue
+			} else {
+				return res, nil
+			}
+		}
+		return nil, &ErrL1RPCError{err: innerErr}
 	}
-	return nil, &ErrL1RPCError{err: err}
-
 }

core/vm/interpreter.go

@@ -44,9 +44,19 @@ type Config struct {
 
 	ExtraEips []int // Additional EIPS that are to be enabled
 
-	L1Client L1Client // L1 RPC client
+	L1Client   L1Client   // L1 RPC client
+	CallerType CallerType // caller type is used in L1Sload precompile to determine whether to retry RPC call forever in case of error
 }
 
+type CallerType int
+
+const (
+	// NonWorker
+	CallerTypeNonWorker CallerType = iota
+	// Worker
+	CallerTypeWorker
+)
+
 // ScopeContext contains the things that are per-call, such as stack and memory,
 // but not transients like pc and gas
 type ScopeContext struct {

miner/worker.go

@@ -1007,9 +1007,13 @@ func (w *worker) commitTransaction(tx *types.Transaction, coinbase common.Addres
 	// create new snapshot for `core.ApplyTransaction`
 	snap := w.current.state.Snapshot()
 
+	// todo: apply this changes to new worker when merged with upstream
+	// make a copy of vm config and change caller type to worker
+	var vmConf vm.Config = *w.chain.GetVMConfig()
+	vmConf.CallerType = vm.CallerTypeWorker
 	var receipt *types.Receipt
 	common.WithTimer(l2CommitTxApplyTimer, func() {
-		receipt, err = core.ApplyTransaction(w.chainConfig, w.chain, &coinbase, w.current.gasPool, w.current.state, w.current.header, tx, &w.current.header.GasUsed, *w.chain.GetVMConfig())
+		receipt, err = core.ApplyTransaction(w.chainConfig, w.chain, &coinbase, w.current.gasPool, w.current.state, w.current.header, tx, &w.current.header.GasUsed, vmConf)
 	})
 	if err != nil {
 		w.current.state.RevertToSnapshot(snap)
@@ -1169,10 +1173,13 @@ loop:
 			// Reorg notification data race between the transaction pool and miner, skip account =
 			log.Trace("Skipping account with hight nonce", "sender", from, "nonce", tx.Nonce())
 			txs.Pop()
+
 		case errors.As(err, &errL1):
-			// Skip the current transaction failed on L1Sload precompile with L1RpcError without shifting in the next from the account
+			// Skip the current transaction failed on L1Sload precompile with L1RpcError without shifting in the next from the account, this tx will be left in txpool and retried in future block
 			log.Trace("Skipping transaction failed on L1Sload precompile with L1RpcError", "sender", from)
+			atomic.AddInt32(&w.newTxs, int32(1))
 			txs.Pop()
+
 		case errors.Is(err, nil):
 			// Everything ok, collect the logs and shift in the next transaction from the same account
 			coalescedLogs = append(coalescedLogs, logs...)

miner/worker_test.go

@@ -17,6 +17,8 @@
 package miner
 
 import (
+	"context"
+	"errors"
 	"math"
 	"math/big"
 	"math/rand"
@@ -1198,6 +1200,83 @@ func TestPrioritizeOverflowTx(t *testing.T) {
 	}
 }
 
+type mockL1Client struct {
+	failList []bool
+}
+
+func (c *mockL1Client) StoragesAt(ctx context.Context, account common.Address, keys []common.Hash, blockNumber *big.Int) ([]byte, error) {
+	if len(c.failList) == 0 {
+		return common.Hash{}.Bytes(), nil
+	}
+	failed := c.failList[0]
+	c.failList = c.failList[1:]
+	if failed {
+		return nil, errors.New("error")
+	} else {
+		return common.Hash{}.Bytes(), nil
+	}
+}
+
+func TestL1SloadFailedTxReexecuted(t *testing.T) {
+	assert := assert.New(t)
+
+	var (
+		chainConfig = params.AllCliqueProtocolChanges
+		db          = rawdb.NewMemoryDatabase()
+		engine      = clique.New(chainConfig.Clique, db)
+	)
+
+	chainConfig.Clique = &params.CliqueConfig{Period: 1, Epoch: 30000}
+	chainConfig.LondonBlock = big.NewInt(0)
+	chainConfig.DescartesBlock = big.NewInt(0)
+
+	w, b := newTestWorker(t, chainConfig, engine, db, 0)
+	// GetStoragesAt will shouldn't fail 2 times on block tracing and should fail then on tx executing
+	w.chain.GetVMConfig().L1Client = &mockL1Client{failList: []bool{false, false, true, true, true}}
+	defer w.close()
+
+	// This test chain imports the mined blocks.
+	db2 := rawdb.NewMemoryDatabase()
+	b.genesis.MustCommit(db2)
+	chain, _ := core.NewBlockChain(db2, nil, b.chain.Config(), engine, vm.Config{
+		Debug:  true,
+		Tracer: vm.NewStructLogger(&vm.LogConfig{EnableMemory: true, EnableReturnData: true})}, nil, nil)
+	defer chain.Stop()
+	chain.GetVMConfig().L1Client = &mockL1Client{}
+
+	// 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()
+
+	// Define tx that calls L1Sload
+	l1SlaodAddress := common.BytesToAddress([]byte{1, 1})
+	input := make([]byte, 52)
+	tx, _ := types.SignTx(types.NewTransaction(b.txPool.Nonce(testBankAddress), l1SlaodAddress, big.NewInt(0), 25208, big.NewInt(10*params.InitialBaseFee), input), types.HomesteadSigner{}, testBankKey)
+
+	// Process 2 transactions with gas order: tx0 > tx1, tx1 will overflow.
+	b.txPool.AddRemote(tx)
+	// b.txPool.AddLocal(b.newRandomTx(false))
+	w.start()
+
+	select {
+	case ev := <-sub.Chan():
+		w.stop()
+		block := ev.Data.(core.NewMinedBlockEvent).Block
+		assert.Equal(1, len(block.Transactions()))
+		assert.Equal(tx.Hash(), block.Transactions()[0].Hash())
+		if _, err := chain.InsertChain([]*types.Block{block}); err != nil {
+			t.Fatalf("failed to insert new mined block %d: %v", block.NumberU64(), err)
+		}
+	case <-time.After(3 * time.Second): // Worker needs 1s to include new changes.
+		t.Fatalf("timeout")
+	}
+}
+
 func TestSkippedTransactionDatabaseEntries(t *testing.T) {
 	assert := assert.New(t)