From 818379d999c947b8075331e2e1b9434b80f5b2b7 Mon Sep 17 00:00:00 2001 From: Roshan <48975233+Pythonberg1997@users.noreply.github.com> Date: Thu, 25 Jan 2024 19:40:26 +0800 Subject: [PATCH] fix: upgrade system contracts twice issue in `traceBlockParallel` (#2178) * fix: upgrade system contracts twice issue in `traceBlockParallel` * fix review comments --- eth/state_accessor.go | 36 +++++++--- eth/tracers/api.go | 158 ++++++++++++++++++++++-------------------- 2 files changed, 109 insertions(+), 85 deletions(-) diff --git a/eth/state_accessor.go b/eth/state_accessor.go index 0971ef246..b82eb077d 100644 --- a/eth/state_accessor.go +++ b/eth/state_accessor.go @@ -33,6 +33,7 @@ import ( "github.com/ethereum/go-ethereum/eth/tracers" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/trie" + "github.com/ethereum/go-ethereum/core/systemcontracts" ) // noopReleaser is returned in case there is no operation expected @@ -239,12 +240,37 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, if err != nil { return nil, vm.BlockContext{}, nil, nil, err } + // upgrade build-in system contract before normal txs if Feynman is not enabled + if !eth.blockchain.Config().IsFeynman(block.Number(), block.Time()) { + systemcontracts.UpgradeBuildInSystemContract(eth.blockchain.Config(), block.Number(), parent.Time(), block.Time(), statedb) + } if txIndex == 0 && len(block.Transactions()) == 0 { return nil, vm.BlockContext{}, statedb, release, nil } // Recompute transactions up to the target index. - signer := types.MakeSigner(eth.blockchain.Config(), block.Number(), block.Time()) + var ( + signer = types.MakeSigner(eth.blockchain.Config(), block.Number(), block.Time()) + beforeSystemTx = true + ) for idx, tx := range block.Transactions() { + // upgrade build-in system contract before system txs if Feynman is enabled + if beforeSystemTx { + if posa, ok := eth.Engine().(consensus.PoSA); ok { + if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { + balance := statedb.GetBalance(consensus.SystemAddress) + if balance.Cmp(common.Big0) > 0 { + statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) + statedb.AddBalance(block.Header().Coinbase, balance) + } + + if eth.blockchain.Config().IsFeynman(block.Number(), block.Time()) { + systemcontracts.UpgradeBuildInSystemContract(eth.blockchain.Config(), block.Number(), parent.Time(), block.Time(), statedb) + } + beforeSystemTx = false + } + } + } + // Assemble the transaction call message and return if the requested offset msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee()) txContext := core.NewEVMTxContext(msg) @@ -254,14 +280,6 @@ func (eth *Ethereum) stateAtTransaction(ctx context.Context, block *types.Block, } // Not yet the searched for transaction, execute on top of the current state vmenv := vm.NewEVM(context, txContext, statedb, eth.blockchain.Config(), vm.Config{}) - if posa, ok := eth.Engine().(consensus.PoSA); ok && msg.From == context.Coinbase && - posa.IsSystemContract(msg.To) && msg.GasPrice.Cmp(big.NewInt(0)) == 0 { - balance := statedb.GetBalance(consensus.SystemAddress) - if balance.Cmp(common.Big0) > 0 { - statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) - statedb.AddBalance(context.Coinbase, balance) - } - } statedb.SetTxContext(tx.Hash(), idx) if _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(tx.Gas())); err != nil { return nil, vm.BlockContext{}, nil, nil, fmt.Errorf("transaction %#x failed: %v", tx.Hash(), err) diff --git a/eth/tracers/api.go b/eth/tracers/api.go index bfa8a3849..975c49140 100644 --- a/eth/tracers/api.go +++ b/eth/tracers/api.go @@ -188,6 +188,7 @@ type txTraceResult struct { // being traced. type blockTraceTask struct { statedb *state.StateDB // Intermediate state prepped for tracing + parent *types.Block // Parent block of the trace block block *types.Block // Block to trace the transactions from release StateReleaseFunc // The function to release the held resource for this task results []*txTraceResult // Trace results produced by the task @@ -204,8 +205,9 @@ type blockTraceResult struct { // txTraceTask represents a single transaction trace task when an entire block // is being traced. type txTraceTask struct { - statedb *state.StateDB // Intermediate state prepped for tracing - index int // Transaction offset in the block + statedb *state.StateDB // Intermediate state prepped for tracing + index int // Transaction offset in the block + isSystemTx bool // Whether the transaction is a system transaction } // TraceChain returns the structured logs created during the execution of EVM @@ -268,11 +270,30 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed // Fetch and execute the block trace taskCh for task := range taskCh { var ( - signer = types.MakeSigner(api.backend.ChainConfig(), task.block.Number(), task.block.Time()) - blockCtx = core.NewEVMBlockContext(task.block.Header(), api.chainContext(ctx), nil) + signer = types.MakeSigner(api.backend.ChainConfig(), task.block.Number(), task.block.Time()) + blockCtx = core.NewEVMBlockContext(task.block.Header(), api.chainContext(ctx), nil) + beforeSystemTx = true ) // Trace all the transactions contained within for i, tx := range task.block.Transactions() { + // upgrade build-in system contract before system txs if Feynman is enabled + if beforeSystemTx { + if posa, ok := api.backend.Engine().(consensus.PoSA); ok { + if isSystem, _ := posa.IsSystemTransaction(tx, task.block.Header()); isSystem { + balance := task.statedb.GetBalance(consensus.SystemAddress) + if balance.Cmp(common.Big0) > 0 { + task.statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) + task.statedb.AddBalance(blockCtx.Coinbase, balance) + } + + if api.backend.ChainConfig().IsFeynman(task.block.Number(), task.block.Time()) { + systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), task.block.Number(), task.parent.Time(), task.block.Time(), task.statedb) + } + beforeSystemTx = false + } + } + } + msg, _ := core.TransactionToMessage(tx, signer, task.block.BaseFee()) txctx := &Context{ BlockHash: task.block.Hash(), @@ -280,7 +301,7 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed TxIndex: i, TxHash: tx.Hash(), } - res, err := api.traceTx(ctx, msg, txctx, blockCtx, task.statedb, config) + res, err := api.traceTx(ctx, msg, txctx, blockCtx, task.statedb, config, !beforeSystemTx) if err != nil { task.results[i] = &txTraceResult{TxHash: tx.Hash(), Error: err.Error()} log.Warn("Tracing failed", "hash", tx.Hash(), "block", task.block.NumberU64(), "err", err) @@ -387,10 +408,15 @@ func (api *API) traceChain(start, end *types.Block, config *TraceConfig, closed // may fail if we release too early. tracker.callReleases() + // upgrade build-in system contract before normal txs if Feynman is not enabled + if !api.backend.ChainConfig().IsFeynman(next.Number(), next.Time()) { + systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), next.Number(), block.Time(), next.Time(), statedb) + } + // Send the block over to the concurrent tracers (if not in the fast-forward phase) txs := next.Transactions() select { - case taskCh <- &blockTraceTask{statedb: statedb.Copy(), block: next, release: release, results: make([]*txTraceResult, len(txs))}: + case taskCh <- &blockTraceTask{statedb: statedb.Copy(), parent: block, block: next, release: release, results: make([]*txTraceResult, len(txs))}: case <-closed: tracker.releaseState(number, release) return @@ -523,7 +549,7 @@ func (api *API) IntermediateRoots(ctx context.Context, hash common.Hash, config } defer release() - // upgrade build-in system contract before tracing non-system tx if Feynman is not enabled + // upgrade build-in system contract before normal txs if Feynman is not enabled if !api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) } @@ -614,7 +640,7 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac } defer release() - // upgrade build-in system contract before tracing non-system tx if Feynman is not enabled + // upgrade build-in system contract before normal txs if Feynman is not enabled if !api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) } @@ -639,10 +665,16 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac beforeSystemTx = true ) for i, tx := range txs { - // upgrade build-in system contract before tracing system tx if Feynman is enabled - if posa, ok := api.backend.Engine().(consensus.PoSA); ok { - if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { - if beforeSystemTx { + // upgrade build-in system contract before system txs if Feynman is enabled + if beforeSystemTx { + if posa, ok := api.backend.Engine().(consensus.PoSA); ok { + if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { + balance := statedb.GetBalance(consensus.SystemAddress) + if balance.Cmp(common.Big0) > 0 { + statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) + statedb.AddBalance(blockCtx.Coinbase, balance) + } + if api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) } @@ -653,14 +685,13 @@ func (api *API) traceBlock(ctx context.Context, block *types.Block, config *Trac // Generate the next state snapshot fast without tracing msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee()) - txctx := &Context{ BlockHash: blockHash, BlockNumber: block.Number(), TxIndex: i, TxHash: tx.Hash(), } - res, err := api.traceTx(ctx, msg, txctx, blockCtx, statedb, config) + res, err := api.traceTx(ctx, msg, txctx, blockCtx, statedb, config, !beforeSystemTx) if err != nil { return nil, err } @@ -704,7 +735,7 @@ func (api *API) traceBlockParallel(ctx context.Context, block *types.Block, stat TxIndex: task.index, TxHash: txs[task.index].Hash(), } - res, err := api.traceTx(ctx, msg, txctx, blockCtx, task.statedb, config) + res, err := api.traceTx(ctx, msg, txctx, blockCtx, task.statedb, config, task.isSystemTx) if err != nil { results[task.index] = &txTraceResult{TxHash: txs[task.index].Hash(), Error: err.Error()} continue @@ -714,14 +745,10 @@ func (api *API) traceBlockParallel(ctx context.Context, block *types.Block, stat }) } - // upgrade build-in system contract before tracing non-system tx if Feynman is not enabled parent, err := api.blockByNumberAndHash(ctx, rpc.BlockNumber(block.NumberU64()-1), block.ParentHash()) if err != nil { return nil, err } - if !api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { - systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) - } // Feed the transactions into the tracers and return var ( @@ -730,12 +757,16 @@ func (api *API) traceBlockParallel(ctx context.Context, block *types.Block, stat ) txloop: for i, tx := range txs { - var isSystem bool - // upgrade build-in system contract before tracing system tx if Feynman is enabled - if posa, ok := api.backend.Engine().(consensus.PoSA); ok { - isSystem, _ = posa.IsSystemTransaction(tx, block.Header()) - if isSystem { - if beforeSystemTx { + // upgrade build-in system contract before system txs if Feynman is enabled + if beforeSystemTx { + if posa, ok := api.backend.Engine().(consensus.PoSA); ok { + if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { + balance := statedb.GetBalance(consensus.SystemAddress) + if balance.Cmp(common.Big0) > 0 { + statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) + statedb.AddBalance(block.Header().Coinbase, balance) + } + if api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) } @@ -745,7 +776,7 @@ txloop: } // Send the trace task over for execution - task := &txTraceTask{statedb: statedb.Copy(), index: i} + task := &txTraceTask{statedb: statedb.Copy(), index: i, isSystemTx: !beforeSystemTx} select { case <-ctx.Done(): failed = ctx.Err() @@ -755,13 +786,6 @@ txloop: // Generate the next state snapshot fast without tracing msg, _ := core.TransactionToMessage(tx, signer, block.BaseFee()) - if isSystem { - balance := statedb.GetBalance(consensus.SystemAddress) - if balance.Cmp(common.Big0) > 0 { - statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) - statedb.AddBalance(block.Header().Coinbase, balance) - } - } statedb.SetTxContext(tx.Hash(), i) vmenv := vm.NewEVM(blockCtx, core.NewEVMTxContext(msg), statedb, api.backend.ChainConfig(), vm.Config{}) if _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.GasLimit)); err != nil { @@ -810,7 +834,7 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block } defer release() - // upgrade build-in system contract before tracing non-system tx if Feynman is not enabled + // upgrade build-in system contract before normal txs if Feynman is not enabled if !api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) } @@ -828,11 +852,12 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block // Execute transaction, either tracing all or just the requested one var ( - dumps []string - signer = types.MakeSigner(api.backend.ChainConfig(), block.Number(), block.Time()) - chainConfig = api.backend.ChainConfig() - vmctx = core.NewEVMBlockContext(block.Header(), api.chainContext(ctx), nil) - canon = true + dumps []string + signer = types.MakeSigner(api.backend.ChainConfig(), block.Number(), block.Time()) + chainConfig = api.backend.ChainConfig() + vmctx = core.NewEVMBlockContext(block.Header(), api.chainContext(ctx), nil) + canon = true + beforeSystemTx = true ) // Check if there are any overrides: the caller may wish to enable a future // fork when executing this block. Note, such overrides are only applicable to the @@ -844,14 +869,17 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block chainConfig, canon = overrideConfig(chainConfig, config.Overrides) } - beforeSystemTx := true for i, tx := range block.Transactions() { - // upgrade build-in system contract before tracing system tx if Feynman is enabled - var isSystem bool - if posa, ok := api.backend.Engine().(consensus.PoSA); ok { - isSystem, _ = posa.IsSystemTransaction(tx, block.Header()) - if isSystem { - if beforeSystemTx { + // upgrade build-in system contract before system txs if Feynman is enabled + if beforeSystemTx { + if posa, ok := api.backend.Engine().(consensus.PoSA); ok { + if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { + balance := statedb.GetBalance(consensus.SystemAddress) + if balance.Cmp(common.Big0) > 0 { + statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) + statedb.AddBalance(vmctx.Coinbase, balance) + } + if api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) } @@ -891,13 +919,6 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block } // Execute the transaction and flush any traces to disk vmenv := vm.NewEVM(vmctx, txContext, statedb, chainConfig, vmConf) - if isSystem { - balance := statedb.GetBalance(consensus.SystemAddress) - if balance.Cmp(common.Big0) > 0 { - statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) - statedb.AddBalance(vmctx.Coinbase, balance) - } - } statedb.SetTxContext(tx.Hash(), i) _, err = core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.GasLimit)) if writer != nil { @@ -962,19 +983,10 @@ func (api *API) TraceTransaction(ctx context.Context, hash common.Hash, config * } defer release() - parent, err := api.blockByNumberAndHash(ctx, rpc.BlockNumber(block.NumberU64()-1), block.ParentHash()) - if err != nil { - return nil, err - } - if !api.backend.ChainConfig().IsFeynman(block.Number(), block.Time()) { - // upgrade build-in system contract before trace if Feynman is not enabled - systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) - } else { - // upgrade build-in system contract before trace system tx if Feynman is enabled - if posa, ok := api.backend.Engine().(consensus.PoSA); ok { - if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { - systemcontracts.UpgradeBuildInSystemContract(api.backend.ChainConfig(), block.Number(), parent.Time(), block.Time(), statedb) - } + var isSystemTx bool + if posa, ok := api.backend.Engine().(consensus.PoSA); ok { + if isSystem, _ := posa.IsSystemTransaction(tx, block.Header()); isSystem { + isSystemTx = true } } @@ -984,7 +996,7 @@ func (api *API) TraceTransaction(ctx context.Context, hash common.Hash, config * TxIndex: int(index), TxHash: hash, } - return api.traceTx(ctx, msg, txctx, vmctx, statedb, config) + return api.traceTx(ctx, msg, txctx, vmctx, statedb, config, isSystemTx) } // TraceCall lets you trace a given eth_call. It collects the structured logs @@ -1052,13 +1064,13 @@ func (api *API) TraceCall(ctx context.Context, args ethapi.TransactionArgs, bloc if config != nil { traceConfig = &config.TraceConfig } - return api.traceTx(ctx, msg, new(Context), vmctx, statedb, traceConfig) + return api.traceTx(ctx, msg, new(Context), vmctx, statedb, traceConfig, false) } // traceTx configures a new tracer according to the provided configuration, and // executes the given message in the provided environment. The return value will // be tracer dependent. -func (api *API) traceTx(ctx context.Context, message *core.Message, txctx *Context, vmctx vm.BlockContext, statedb *state.StateDB, config *TraceConfig) (interface{}, error) { +func (api *API) traceTx(ctx context.Context, message *core.Message, txctx *Context, vmctx vm.BlockContext, statedb *state.StateDB, config *TraceConfig, isSystemTx bool) (interface{}, error) { var ( tracer Tracer err error @@ -1097,13 +1109,7 @@ func (api *API) traceTx(ctx context.Context, message *core.Message, txctx *Conte var intrinsicGas uint64 = 0 // Run the transaction with tracing enabled. - if posa, ok := api.backend.Engine().(consensus.PoSA); ok && message.From == vmctx.Coinbase && - posa.IsSystemContract(message.To) && message.GasPrice.Cmp(big.NewInt(0)) == 0 { - balance := statedb.GetBalance(consensus.SystemAddress) - if balance.Cmp(common.Big0) > 0 { - statedb.SetBalance(consensus.SystemAddress, big.NewInt(0)) - statedb.AddBalance(vmctx.Coinbase, balance) - } + if isSystemTx { intrinsicGas, _ = core.IntrinsicGas(message.Data, message.AccessList, false, true, true, false) }