Skip to content

Commit 62dc2ea

Browse files
authored
refactor: set EVM.readOnly and depth before running stateful precompile (#210)
## Why this should be merged Improved clarity. ## How this works Semantically equivalent refactor. These values aren't accessible / needed during execution of a stateful precompile, but are important when making outgoing calls so were originally only set in the `PrecompileEnvironment.Call()` implementation. This was confusing and led to a false-positive bug report. ## How this was tested Existing tests.
1 parent e7035f1 commit 62dc2ea

File tree

2 files changed

+28
-34
lines changed

2 files changed

+28
-34
lines changed

core/vm/contracts.libevm.go

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,12 @@ func (t CallType) isValid() bool {
101101
}
102102
}
103103

104+
// readOnly returns whether the CallType induces a read-only state if not
105+
// already in one.
106+
func (t CallType) readOnly() bool {
107+
return t == StaticCall
108+
}
109+
104110
// String returns a human-readable representation of the CallType.
105111
func (t CallType) String() string {
106112
if t.isValid() {
@@ -120,15 +126,28 @@ func (t CallType) OpCode() OpCode {
120126
// run runs the [PrecompiledContract], differentiating between stateful and
121127
// regular types, updating `args.gasRemaining` in the stateful case.
122128
func (args *evmCallArgs) run(p PrecompiledContract, input []byte) (ret []byte, err error) {
123-
switch p := p.(type) {
124-
default:
129+
sp, ok := p.(statefulPrecompile)
130+
if !ok {
125131
return p.Run(input)
126-
case statefulPrecompile:
127-
env := args.env()
128-
ret, err := p(env, input)
129-
args.gasRemaining = env.Gas()
130-
return ret, err
131132
}
133+
134+
env := args.env()
135+
// Depth and read-only setting are handled by [EVMInterpreter.Run],
136+
// which isn't used for precompiles, so we need to do it ourselves to
137+
// maintain the expected invariants.
138+
in := env.evm.interpreter
139+
140+
in.evm.depth++
141+
defer func() { in.evm.depth-- }()
142+
143+
if env.callType.readOnly() && !in.readOnly {
144+
in.readOnly = true
145+
defer func() { in.readOnly = false }()
146+
}
147+
148+
ret, err = sp(env, input)
149+
args.gasRemaining = env.Gas()
150+
return ret, err
132151
}
133152

134153
// PrecompiledStatefulContract is the stateful equivalent of a

core/vm/environment.libevm.go

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -63,19 +63,7 @@ func (e *environment) refundGas(add uint64) error {
6363
}
6464

6565
func (e *environment) ReadOnly() bool {
66-
// A switch statement provides clearer code coverage for difficult-to-test
67-
// cases.
68-
switch {
69-
case e.callType == StaticCall:
70-
// evm.interpreter.readOnly is only set to true via a call to
71-
// EVMInterpreter.Run() so, if a precompile is called directly with
72-
// StaticCall(), then readOnly might not be set yet.
73-
return true
74-
case e.evm.interpreter.readOnly:
75-
return true
76-
default:
77-
return false
78-
}
66+
return e.evm.interpreter.readOnly
7967
}
8068

8169
func (e *environment) Addresses() *libevm.AddressContext {
@@ -116,19 +104,6 @@ func (e *environment) Call(addr common.Address, input []byte, gas uint64, value
116104
}
117105

118106
func (e *environment) callContract(typ CallType, addr common.Address, input []byte, gas uint64, value *uint256.Int, opts ...CallOption) (retData []byte, retErr error) {
119-
// Depth and read-only setting are handled by [EVMInterpreter.Run], which
120-
// isn't used for precompiles, so we need to do it ourselves to maintain the
121-
// expected invariants.
122-
in := e.evm.interpreter
123-
124-
in.evm.depth++
125-
defer func() { in.evm.depth-- }()
126-
127-
if e.ReadOnly() && !in.readOnly { // i.e. the precompile was StaticCall()ed
128-
in.readOnly = true
129-
defer func() { in.readOnly = false }()
130-
}
131-
132107
var caller ContractRef = e.self
133108
if options.As[callConfig](opts...).unsafeCallerAddressProxying {
134109
// Note that, in addition to being unsafe, this breaks an EVM
@@ -141,7 +116,7 @@ func (e *environment) callContract(typ CallType, addr common.Address, input []by
141116
}
142117
}
143118

144-
if in.readOnly && value != nil && !value.IsZero() {
119+
if e.ReadOnly() && value != nil && !value.IsZero() {
145120
return nil, ErrWriteProtection
146121
}
147122
if !e.UseGas(gas) {

0 commit comments

Comments
 (0)