From 9393d1fb5d178667e4d2750f7f540a3db5985818 Mon Sep 17 00:00:00 2001 From: Alex Beregszaszi Date: Wed, 1 Dec 2021 09:21:21 +0000 Subject: [PATCH] core/vm: Move interpreter.ReadOnly check into the opcode implementations (#23970) * core/vm: Move interpreter.ReadOnly check into the opcode implementations Also remove the same check from the interpreter inner loop. * core/vm: Remove obsolete operation.writes flag * core/vm: Capture fault states in logger Co-authored-by: Martin Holst Swende * core/vm: Remove panic added for testing Co-authored-by: Martin Holst Swende --- core/vm/instructions.go | 18 ++++++++++++++++++ core/vm/interpreter.go | 11 ----------- core/vm/jump_table.go | 11 ----------- eth/tracers/logger/logger_json.go | 5 ++++- 4 files changed, 22 insertions(+), 23 deletions(-) diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 350baecff8..f9e1186edf 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -514,6 +514,9 @@ func opSload(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]by } func opSstore(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { + if interpreter.readOnly { + return nil, ErrWriteProtection + } loc := scope.Stack.pop() val := scope.Stack.pop() interpreter.evm.StateDB.SetState(scope.Contract.Address(), @@ -561,6 +564,9 @@ func opGas(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte } func opCreate(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { + if interpreter.readOnly { + return nil, ErrWriteProtection + } var ( value = scope.Stack.pop() offset, size = scope.Stack.pop(), scope.Stack.pop() @@ -604,6 +610,9 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]b } func opCreate2(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { + if interpreter.readOnly { + return nil, ErrWriteProtection + } var ( endowment = scope.Stack.pop() offset, size = scope.Stack.pop(), scope.Stack.pop() @@ -653,6 +662,9 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt // Get the arguments from the memory. args := scope.Memory.GetPtr(int64(inOffset.Uint64()), int64(inSize.Uint64())) + if interpreter.readOnly && !value.IsZero() { + return nil, ErrWriteProtection + } var bigVal = big0 //TODO: use uint256.Int instead of converting with toBig() // By using big0 here, we save an alloc for the most common case (non-ether-transferring contract calls), @@ -794,6 +806,9 @@ func opStop(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt } func opSuicide(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { + if interpreter.readOnly { + return nil, ErrWriteProtection + } beneficiary := scope.Stack.pop() balance := interpreter.evm.StateDB.GetBalance(scope.Contract.Address()) interpreter.evm.StateDB.AddBalance(beneficiary.Bytes20(), balance) @@ -810,6 +825,9 @@ func opSuicide(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([] // make log instruction function func makeLog(size int) executionFunc { return func(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) { + if interpreter.readOnly { + return nil, ErrWriteProtection + } topics := make([]common.Hash, size) stack := scope.Stack mStart, mSize := stack.pop(), stack.pop() diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 4afbd86594..99257d57a8 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -202,17 +202,6 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) ( } else if sLen > operation.maxStack { return nil, &ErrStackOverflow{stackLen: sLen, limit: operation.maxStack} } - // If the operation is valid, enforce write restrictions - if in.readOnly && in.evm.chainRules.IsByzantium { - // If the interpreter is operating in readonly mode, make sure no - // state-modifying operation is performed. The 3rd stack item - // for a call operation is the value. Transferring value from one - // account to the others means the state is modified and should also - // return with an error. - if operation.writes || (op == CALL && stack.Back(2).Sign() != 0) { - return nil, ErrWriteProtection - } - } // Static portion of gas cost = operation.constantGas // For tracing if !contract.UseGas(operation.constantGas) { diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index bcc15beb23..d3427f4323 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -40,8 +40,6 @@ type operation struct { // memorySize returns the memory size required for the operation memorySize memorySizeFunc - - writes bool // determines whether this a state modifying operation } var ( @@ -123,7 +121,6 @@ func newConstantinopleInstructionSet() JumpTable { minStack: minStack(4, 1), maxStack: maxStack(4, 1), memorySize: memoryCreate2, - writes: true, } return instructionSet } @@ -511,7 +508,6 @@ func newFrontierInstructionSet() JumpTable { dynamicGas: gasSStore, minStack: minStack(2, 0), maxStack: maxStack(2, 0), - writes: true, }, JUMP: { execute: opJump, @@ -939,7 +935,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(2, 0), maxStack: maxStack(2, 0), memorySize: memoryLog, - writes: true, }, LOG1: { execute: makeLog(1), @@ -947,7 +942,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(3, 0), maxStack: maxStack(3, 0), memorySize: memoryLog, - writes: true, }, LOG2: { execute: makeLog(2), @@ -955,7 +949,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(4, 0), maxStack: maxStack(4, 0), memorySize: memoryLog, - writes: true, }, LOG3: { execute: makeLog(3), @@ -963,7 +956,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(5, 0), maxStack: maxStack(5, 0), memorySize: memoryLog, - writes: true, }, LOG4: { execute: makeLog(4), @@ -971,7 +963,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(6, 0), maxStack: maxStack(6, 0), memorySize: memoryLog, - writes: true, }, CREATE: { execute: opCreate, @@ -980,7 +971,6 @@ func newFrontierInstructionSet() JumpTable { minStack: minStack(3, 1), maxStack: maxStack(3, 1), memorySize: memoryCreate, - writes: true, }, CALL: { execute: opCall, @@ -1010,7 +1000,6 @@ func newFrontierInstructionSet() JumpTable { dynamicGas: gasSelfdestruct, minStack: minStack(1, 0), maxStack: maxStack(1, 0), - writes: true, }, } } diff --git a/eth/tracers/logger/logger_json.go b/eth/tracers/logger/logger_json.go index 14d90c2dba..4a7abacba2 100644 --- a/eth/tracers/logger/logger_json.go +++ b/eth/tracers/logger/logger_json.go @@ -47,7 +47,10 @@ func (l *JSONLogger) CaptureStart(env *vm.EVM, from, to common.Address, create b l.env = env } -func (l *JSONLogger) CaptureFault(uint64, vm.OpCode, uint64, uint64, *vm.ScopeContext, int, error) {} +func (l *JSONLogger) CaptureFault(pc uint64, op vm.OpCode, gas uint64, cost uint64, scope *vm.ScopeContext, depth int, err error) { + // TODO: Add rData to this interface as well + l.CaptureState(pc, op, gas, cost, scope, nil, depth, err) +} // CaptureState outputs state information on the logger. func (l *JSONLogger) CaptureState(pc uint64, op vm.OpCode, gas, cost uint64, scope *vm.ScopeContext, rData []byte, depth int, err error) {