Skip to content

Commit

Permalink
Fixes to trace_call, trace_callMany, dual RPC daemon mode (erigontech…
Browse files Browse the repository at this point in the history
…#1491)

* Fixes to trace_call, trace_callMany, dual RPC daemon mode

* Fix compile error

* Fix compile error

* Compile error fix

* Compile error fix

* Fix typo

* Fix discrepancy when trace is empty

Co-authored-by: Alexey Sharp <alexeysharp@Alexeys-iMac.local>
  • Loading branch information
AlexeyAkhunov and Alexey Sharp authored Feb 12, 2021
1 parent 8777899 commit 7c81e91
Show file tree
Hide file tree
Showing 22 changed files with 82 additions and 49 deletions.
2 changes: 1 addition & 1 deletion accounts/abi/bind/backends/simulated.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ func (b *SimulatedBackend) callContract(_ context.Context, call ethereum.CallMsg
vmEnv := vm.NewEVM(evmContext, statedb, b.config, vm.Config{})
gasPool := new(core.GasPool).AddGas(math.MaxUint64)

return core.NewStateTransition(vmEnv, msg, gasPool).TransitionDb(true /* refunds */)
return core.NewStateTransition(vmEnv, msg, gasPool).TransitionDb(true /* refunds */, false /* gasBailout */)
}

// SendTransaction updates the pending block to include the given transaction.
Expand Down
2 changes: 1 addition & 1 deletion cmd/evm/internal/t8ntool/execution.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ func (pre *Prestate) Apply(vmConfig vm.Config, chainConfig *params.ChainConfig,
}
snapshot := ibs.Snapshot()
// (ret []byte, usedGas uint64, failed bool, err error)
msgResult, err := core.ApplyMessage(evm, msg, gaspool, true /* refunds */)
msgResult, err := core.ApplyMessage(evm, msg, gaspool, true /* refunds */, false /* gasBailout */)
if err != nil {
ibs.RevertToSnapshot(snapshot)
log.Info("rejected tx", "index", i, "hash", tx.Hash(), "from", msg.From(), "error", err)
Expand Down
9 changes: 7 additions & 2 deletions cmd/rpcdaemon/cli/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,16 @@ func OpenDB(cfg Flags) (ethdb.KV, ethdb.Backend, error) {
}
db = kv
}
} else if cfg.PrivateApiAddr != "" {
db, ethBackend, err = ethdb.NewRemote().Path(cfg.PrivateApiAddr).Open(cfg.TLSCertfile, cfg.TLSKeyFile, cfg.TLSCACert)
}
if cfg.PrivateApiAddr != "" {
var remoteDb ethdb.KV
remoteDb, ethBackend, err = ethdb.NewRemote().Path(cfg.PrivateApiAddr).Open(cfg.TLSCertfile, cfg.TLSKeyFile, cfg.TLSCACert)
if err != nil {
return nil, nil, fmt.Errorf("could not connect to remoteDb: %w", err)
}
if db == nil {
db = remoteDb
}
} else {
return nil, nil, fmt.Errorf("either remote db or lmdb must be specified")
}
Expand Down
26 changes: 18 additions & 8 deletions cmd/rpcdaemon/commands/trace_adhoc.go
Original file line number Diff line number Diff line change
Expand Up @@ -481,7 +481,7 @@ func (api *TraceAPIImpl) Call(ctx context.Context, args TraceCallParam, traceTyp
// this makes sure resources are cleaned up.
defer cancel()

traceResult := &TraceCallResult{}
traceResult := &TraceCallResult{Trace: []*ParityTrace{}}
var traceTypeTrace, traceTypeStateDiff, traceTypeVmTrace bool
for _, traceType := range traceTypes {
switch traceType {
Expand Down Expand Up @@ -517,7 +517,7 @@ func (api *TraceAPIImpl) Call(ctx context.Context, args TraceCallParam, traceTyp

gp := new(core.GasPool).AddGas(msg.Gas())
var execResult *core.ExecutionResult
execResult, err = core.ApplyMessage(evm, msg, gp, true /* refunds */)
execResult, err = core.ApplyMessage(evm, msg, gp, true /* refunds */, true /* gasBailout */)
if err != nil {
return nil, err
}
Expand All @@ -526,7 +526,7 @@ func (api *TraceAPIImpl) Call(ctx context.Context, args TraceCallParam, traceTyp
sdMap := make(map[common.Address]*StateDiffAccount)
traceResult.StateDiff = sdMap
sd := &StateDiff{sdMap: sdMap}
if err = ibs.CommitBlock(ctx, sd); err != nil {
if err = ibs.FinalizeTx(ctx, sd); err != nil {
return nil, err
}
// Create initial IntraBlockState, we will compare it with ibs (IntraBlockState after the transaction)
Expand Down Expand Up @@ -628,7 +628,7 @@ func (api *TraceAPIImpl) CallMany(ctx context.Context, calls json.RawMessage, bl
if tok != json.Delim(']') {
return nil, fmt.Errorf("expected end of [callparam, tracetypes]")
}
traceResult := &TraceCallResult{}
traceResult := &TraceCallResult{Trace: []*ParityTrace{}}
var traceTypeTrace, traceTypeStateDiff, traceTypeVmTrace bool
for _, traceType := range traceTypes {
switch traceType {
Expand Down Expand Up @@ -659,15 +659,18 @@ func (api *TraceAPIImpl) CallMany(ctx context.Context, calls json.RawMessage, bl

gp := new(core.GasPool).AddGas(msg.Gas())
var execResult *core.ExecutionResult
execResult, err = core.ApplyMessage(evm, msg, gp, true /* refunds */)
// Clone the state cache before applying the changes, clone is discarded
var cloneReader state.StateReader
if traceTypeStateDiff {
cloneCache := stateCache.Clone()
cloneReader = state.NewCachedReader(stateReader, cloneCache)
}
execResult, err = core.ApplyMessage(evm, msg, gp, true /* refunds */, true /* gasBailout */)
if err != nil {
return nil, fmt.Errorf("first run for txIndex %d error: %w", txIndex, err)
}
traceResult.Output = execResult.ReturnData
if traceTypeStateDiff {
// Clone the state cache before applying the changes, clone is discarded
cloneCache := stateCache.Clone()
cloneReader := state.NewCachedReader(stateReader, cloneCache)
initialIbs := state.New(cloneReader)
sdMap := make(map[common.Address]*StateDiffAccount)
traceResult.StateDiff = sdMap
Expand All @@ -679,6 +682,13 @@ func (api *TraceAPIImpl) CallMany(ctx context.Context, calls json.RawMessage, bl
return nil, err
}
sd.CompareStates(initialIbs, ibs)
} else {
if err = ibs.FinalizeTx(ctx, noop); err != nil {
return nil, err
}
if err = ibs.CommitBlock(ctx, cachedWriter); err != nil {
return nil, err
}
}

if traceTypeVmTrace {
Expand Down
3 changes: 2 additions & 1 deletion cmd/rpctest/rpctest/bench11.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (
// Transactions on which OpenEthereum reports incorrect traces
var wrongTxs = []string{
"0xe47180a05a7cc25c187b426fed5390365874add72a5681242ac4b288d4a6833a", // Block 7000000
"0x76ea0eae8561c10321b050bc28d00ae4e22f99dc303153cc7fb1334ec88ad41e", // Block 7000053
"0x76e720b0530aa72926319853f62c97c5907b26e2fc5c8ad5f51173f531d98d11", // Block 7000063
"0xc6f3cadc90aece146a7a90191d6668c7f152a2daf759ae97bde7df7f5d78ab3a", // Block 7000068
"0xd5a9b32b262202cda422dd5a2ccf8d7d56e9b3425ba7d350548e62a5bd26b481", // Block 8000011
"0x1953ad3591fa0f6f3f00dfa0f93a57e1dc7fa003e2192a18c64c71847cf64e0c", // Block 8000035
"0xfbd66bcbc4cb374946f350ca6835571b09f68c5f635ff9fc533c3fa2ac0d19cb", // Block 9000004
Expand Down
6 changes: 4 additions & 2 deletions core/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,10 @@ func CanTransfer(db vm.IntraBlockState, addr common.Address, amount *uint256.Int
}

// Transfer subtracts amount from sender and adds amount to recipient using the given Db
func Transfer(db vm.IntraBlockState, sender, recipient common.Address, amount *uint256.Int) {
db.SubBalance(sender, amount)
func Transfer(db vm.IntraBlockState, sender, recipient common.Address, amount *uint256.Int, bailout bool) {
if !bailout {
db.SubBalance(sender, amount)
}
db.AddBalance(recipient, amount)
}

Expand Down
2 changes: 1 addition & 1 deletion core/state_prefetcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,6 @@ func precacheTransaction(config *params.ChainConfig, bc ChainContext, author *co
cfg.SkipAnalysis = SkipAnalysis(config, header.Number.Uint64())
vm := vm.NewEVM(context, statedb, config, cfg)

_, err = ApplyMessage(vm, msg, gaspool, true /* refunds */)
_, err = ApplyMessage(vm, msg, gaspool, true /* refunds */, false /* gasBailout */)
return err
}
2 changes: 1 addition & 1 deletion core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo
}

// Apply the transaction to the current state (included in the env)
result, err := ApplyMessage(vmenv, msg, gp, true /* refunds */)
result, err := ApplyMessage(vmenv, msg, gp, true /* refunds */, false /* gasBailout */)
if err != nil {
return nil, err
}
Expand Down
33 changes: 22 additions & 11 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,11 @@ func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool) *StateTransition
// the gas used (which includes gas refunds) and an error if it failed. An error always
// indicates a core error meaning that the message would always fail for that particular
// state and would never be accepted within a block.
func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, refunds bool) (*ExecutionResult, error) {
return NewStateTransition(evm, msg, gp).TransitionDb(refunds)
// `refunds` is false when it is not required to apply gas refunds
// `gasBailout` is true when it is not required to fail transaction if the balance is not enough to pay gas.
// for trace_call to replicate OE/Pariry behaviour
func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, refunds bool, gasBailout bool) (*ExecutionResult, error) {
return NewStateTransition(evm, msg, gp).TransitionDb(refunds, gasBailout)
}

// to returns the recipient of the message.
Expand All @@ -176,24 +179,27 @@ func (st *StateTransition) to() common.Address {
return *st.msg.To()
}

func (st *StateTransition) buyGas() error {
func (st *StateTransition) buyGas(gasBailout bool) error {
mgval := new(big.Int).Mul(new(big.Int).SetUint64(st.msg.Gas()), st.gasPrice.ToBig())
gasCost, overflow := uint256.FromBig(mgval)
if overflow || st.state.GetBalance(st.msg.From()).Lt(gasCost) {
return ErrInsufficientFunds
if !gasBailout {
return ErrInsufficientFunds
}
} else {
st.state.SubBalance(st.msg.From(), gasCost)
}
if err := st.gp.SubGas(st.msg.Gas()); err != nil {
return err
}
st.gas += st.msg.Gas()

st.initialGas = st.msg.Gas()
st.state.SubBalance(st.msg.From(), gasCost)
return nil
}

// DESCRIBED: docs/programmers_guide/guide.md#nonce
func (st *StateTransition) preCheck() error {
func (st *StateTransition) preCheck(gasBailout bool) error {
// Make sure this transaction's nonce is correct.
if st.msg.CheckNonce() {
nonce := st.state.GetNonce(st.msg.From())
Expand All @@ -205,7 +211,7 @@ func (st *StateTransition) preCheck() error {
return ErrNonceTooLow
}
}
return st.buyGas()
return st.buyGas(gasBailout)
}

// TransitionDb will transition the state by applying the current message and
Expand All @@ -221,7 +227,7 @@ func (st *StateTransition) preCheck() error {
//
// However if any consensus issue encountered, return the error directly with
// nil evm execution result.
func (st *StateTransition) TransitionDb(refunds bool) (*ExecutionResult, error) {
func (st *StateTransition) TransitionDb(refunds bool, gasBailout bool) (*ExecutionResult, error) {
// First check this message satisfies all consensus rules before
// applying the message. The rules include these clauses
//
Expand All @@ -233,7 +239,7 @@ func (st *StateTransition) TransitionDb(refunds bool) (*ExecutionResult, error)
// 6. caller has enough balance to cover asset transfer for **topmost** call

// Check clauses 1-3, buy gas if everything is correct
if err := st.preCheck(); err != nil {
if err := st.preCheck(gasBailout); err != nil {
return nil, err
}
msg := st.msg
Expand All @@ -253,8 +259,13 @@ func (st *StateTransition) TransitionDb(refunds bool) (*ExecutionResult, error)
st.gas -= gas

// Check clause 6
var bailout bool
if !msg.Value().IsZero() && !st.evm.CanTransfer(st.state, msg.From(), msg.Value()) {
return nil, ErrInsufficientFundsForTransfer
if gasBailout {
bailout = true
} else {
return nil, ErrInsufficientFundsForTransfer
}
}
var (
ret []byte
Expand All @@ -269,7 +280,7 @@ func (st *StateTransition) TransitionDb(refunds bool) (*ExecutionResult, error)
} else {
// Increment the nonce for the next transaction
st.state.SetNonce(msg.From(), st.state.GetNonce(sender.Address())+1)
ret, st.gas, vmerr = st.evm.Call(sender, st.to(), st.data, st.gas, st.value)
ret, st.gas, vmerr = st.evm.Call(sender, st.to(), st.data, st.gas, st.value, bailout)
}
if refunds {
st.refundGas()
Expand Down
12 changes: 7 additions & 5 deletions core/vm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ type (
// CanTransferFunc is the signature of a transfer guard function
CanTransferFunc func(IntraBlockState, common.Address, *uint256.Int) bool
// TransferFunc is the signature of a transfer function
TransferFunc func(IntraBlockState, common.Address, common.Address, *uint256.Int)
TransferFunc func(IntraBlockState, common.Address, common.Address, *uint256.Int, bool)
// GetHashFunc returns the nth block hash in the blockchain
// and is used by the BLOCKHASH EVM op code.
GetHashFunc func(uint64) common.Hash
Expand Down Expand Up @@ -215,7 +215,7 @@ func (evm *EVM) Interpreter() Interpreter {
// parameters. It also handles any necessary value transfer required and takes
// the necessary steps to create accounts and reverses the state in case of an
// execution error or failed value transfer.
func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas uint64, value *uint256.Int) (ret []byte, leftOverGas uint64, err error) {
func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas uint64, value *uint256.Int, bailout bool) (ret []byte, leftOverGas uint64, err error) {
if evm.vmConfig.NoRecursion && evm.depth > 0 {
return nil, gas, nil
}
Expand All @@ -225,7 +225,9 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
}
// Fail if we're trying to transfer more than the available balance
if value.Sign() != 0 && !evm.Context.CanTransfer(evm.IntraBlockState, caller.Address(), value) {
return nil, gas, ErrInsufficientBalance
if !bailout {
return nil, gas, ErrInsufficientBalance
}
}
p, isPrecompile := evm.precompile(addr)

Expand All @@ -244,7 +246,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
}
evm.IntraBlockState.CreateAccount(addr, false)
}
evm.Transfer(evm.IntraBlockState, caller.Address(), to.Address(), value)
evm.Transfer(evm.IntraBlockState, caller.Address(), to.Address(), value, bailout)

// Capture the tracer start/end events in debug mode
if evm.vmConfig.Debug {
Expand Down Expand Up @@ -487,7 +489,7 @@ func (evm *EVM) create(caller ContractRef, codeAndHash *codeAndHash, gas uint64,
if evm.chainRules.IsEIP158 {
evm.IntraBlockState.SetNonce(address, 1)
}
evm.Transfer(evm.IntraBlockState, caller.Address(), address, value)
evm.Transfer(evm.IntraBlockState, caller.Address(), address, value, false /* bailout */)

// Initialise a new contract and set the code that is to be used by the EVM.
// The contract is a scoped environment for this execution context only.
Expand Down
2 changes: 1 addition & 1 deletion core/vm/evmc.go
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func (host *hostContext) Call(kind evmc.CallKind,
if static {
output, gasLeftU, err = host.env.StaticCall(host.contract, destination, input, gasU)
} else {
output, gasLeftU, err = host.env.Call(host.contract, destination, input, gasU, value)
output, gasLeftU, err = host.env.Call(host.contract, destination, input, gasU, value, false /* bailout */)
}
case evmc.DelegateCall:
output, gasLeftU, err = host.env.DelegateCall(host.contract, destination, input, gasU)
Expand Down
4 changes: 2 additions & 2 deletions core/vm/gas_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,11 @@ func TestEIP2200(t *testing.T) {

vmctx := Context{
CanTransfer: func(IntraBlockState, common.Address, *uint256.Int) bool { return true },
Transfer: func(IntraBlockState, common.Address, common.Address, *uint256.Int) {},
Transfer: func(IntraBlockState, common.Address, common.Address, *uint256.Int, bool) {},
}
vmenv := NewEVM(vmctx, state, params.AllEthashProtocolChanges, Config{ExtraEips: []int{2200}})

_, gas, err := vmenv.Call(AccountRef(common.Address{}), address, nil, tt.gaspool, new(uint256.Int))
_, gas, err := vmenv.Call(AccountRef(common.Address{}), address, nil, tt.gaspool, new(uint256.Int), false /* bailout */)
if !errors.Is(err, tt.failure) {
t.Errorf("test %d: failure mismatch: have %v, want %v", i, err, tt.failure)
}
Expand Down
2 changes: 1 addition & 1 deletion core/vm/instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, callContext *callCtx) ([]by
gas += params.CallStipend
}

ret, returnGas, err := interpreter.evm.Call(callContext.contract, toAddr, args, gas, &value)
ret, returnGas, err := interpreter.evm.Call(callContext.contract, toAddr, args, gas, &value, false /* bailout */)

if err != nil {
temp.Clear()
Expand Down
2 changes: 2 additions & 0 deletions core/vm/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ func Execute(code, input []byte, cfg *Config, blockNr uint64) ([]byte, *state.In
input,
cfg.GasLimit,
cfg.Value,
false, /* bailout */
)

return ret, cfg.State, err
Expand Down Expand Up @@ -202,6 +203,7 @@ func Call(address common.Address, input []byte, cfg *Config) ([]byte, uint64, er
input,
cfg.GasLimit,
cfg.Value,
false, /* bailout */
)

return ret, leftOverGas, err
Expand Down
4 changes: 2 additions & 2 deletions core/vm/runtime/runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -630,12 +630,12 @@ func benchmarkNonModifyingCode(gas uint64, code []byte, name string, b *testing.
//cfg.State.CreateAccount(cfg.Origin)
// set the receiver's (the executing contract) code for execution.
cfg.State.SetCode(destination, code)
vmenv.Call(sender, destination, nil, gas, cfg.Value) // nolint:errcheck
vmenv.Call(sender, destination, nil, gas, cfg.Value, false /* bailout */) // nolint:errcheck

b.Run(name, func(b *testing.B) {
b.ReportAllocs()
for i := 0; i < b.N; i++ {
vmenv.Call(sender, destination, nil, gas, cfg.Value) // nolint:errcheck
vmenv.Call(sender, destination, nil, gas, cfg.Value, false /* bailout */) // nolint:errcheck
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion eth/gasprice/gasprice.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func (gpo *Oracle) SuggestPrice(ctx context.Context) (*big.Int, error) {
// - The block is empty
// - All the transactions included are sent by the miner itself.
// In these cases, use the latest calculated price for samping.
if len(res.prices) == 0 {
if len(res.prices) == 0 && lastPrice != nil {
res.prices = []*big.Int{lastPrice}
}
// Besides, in order to collect enough data for sampling, if nothing
Expand Down
4 changes: 2 additions & 2 deletions eth/tracers/tracers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func TestPrestateTracerCreate2(t *testing.T) {
t.Fatalf("failed to prepare transaction for tracing: %v", err)
}
st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas()))
if _, err = st.TransitionDb(true /* refunds */); err != nil {
if _, err = st.TransitionDb(true /* refunds */, false /* gasBailout */); err != nil {
t.Fatalf("failed to execute transaction: %v", err)
}
// Retrieve the trace result and compare against the etalon
Expand Down Expand Up @@ -277,7 +277,7 @@ func TestCallTracer(t *testing.T) {
t.Fatalf("failed to prepare transaction for tracing: %v", err)
}
st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas()))
if _, err = st.TransitionDb(true /* refunds */); err != nil {
if _, err = st.TransitionDb(true /* refunds */, false /* gasBailout */); err != nil {
t.Fatalf("failed to execute transaction: %v", err)
}
// Retrieve the trace result and compare against the etalon
Expand Down
2 changes: 1 addition & 1 deletion internal/ethapi/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ func DoCall(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.Blo
// Setup the gas pool (also for unmetered requests)
// and apply the message.
gp := new(core.GasPool).AddGas(math.MaxUint64)
result, err := core.ApplyMessage(evm, msg, gp, true /* refunds */)
result, err := core.ApplyMessage(evm, msg, gp, true /* refunds */, false /* gasBailout */)
if err := vmError(); err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 7c81e91

Please sign in to comment.