core/vm: Move interpreter.ReadOnly check into the opcode implementati…

…ons (ethereum#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 <[email protected]> * core/vm: Remove panic added for testing Co-authored-by: Martin Holst Swende <[email protected]>
METADIUM · Dec 1, 2021 · 9393d1f · 9393d1f
1 parent 1988b47
commit 9393d1f
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 23 deletions.
diff --git 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
@@ -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
@@ -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,39 +935,34 @@ func newFrontierInstructionSet() JumpTable {
  minStack: minStack(2, 0),
  maxStack: maxStack(2, 0),
  memorySize: memoryLog,
- writes: true,
  },
  LOG1: {
  execute: makeLog(1),
  dynamicGas: makeGasLog(1),
  minStack: minStack(3, 0),
  maxStack: maxStack(3, 0),
  memorySize: memoryLog,
- writes: true,
  },
  LOG2: {
  execute: makeLog(2),
  dynamicGas: makeGasLog(2),
  minStack: minStack(4, 0),
  maxStack: maxStack(4, 0),
  memorySize: memoryLog,
- writes: true,
  },
  LOG3: {
  execute: makeLog(3),
  dynamicGas: makeGasLog(3),
  minStack: minStack(5, 0),
  maxStack: maxStack(5, 0),
  memorySize: memoryLog,
- writes: true,
  },
  LOG4: {
  execute: makeLog(4),
  dynamicGas: makeGasLog(4),
  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
@@ -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) {