From d3adddf6213e1ac35d70e349893807eef1008e9b Mon Sep 17 00:00:00 2001 From: Nazarii Denha Date: Mon, 22 Jul 2024 10:50:47 +0200 Subject: [PATCH] add flag to precompile to determine whether it called form worker, add test --- core/state_transition.go | 2 + core/vm/contracts.go | 42 ++++++++++++++------- core/vm/interpreter.go | 12 +++++- miner/worker.go | 11 +++++- miner/worker_test.go | 79 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 130 insertions(+), 16 deletions(-) diff --git a/core/state_transition.go b/core/state_transition.go index 49d24768212c..e45b91f00a70 100644 --- a/core/state_transition.go +++ b/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 diff --git a/core/vm/contracts.go b/core/vm/contracts.go index 3d761d8ea339..7e7449829fa7 100644 --- a/core/vm/contracts.go +++ b/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} - } diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index aafe5cf25119..b4d1d8149277 100644 --- a/core/vm/interpreter.go +++ b/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 { diff --git a/miner/worker.go b/miner/worker.go index d3d925dcbff1..0d49d617c717 100644 --- a/miner/worker.go +++ b/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...) diff --git a/miner/worker_test.go b/miner/worker_test.go index e20dc00dae47..c998fe160dc5 100644 --- a/miner/worker_test.go +++ b/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 = ¶ms.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)