Skip to content

Commit fc807df

Browse files
grandizzygrandizzy
authored
fix(cheatcodes): reset interpreter gas to the value of gas spent (#8481)
* fix(cheatcodes): do not record gas changes if exec frame not started * Fix * Fmt * Use gas.spent(), enable repro test
1 parent 1bae886 commit fc807df

File tree

3 files changed

+342
-13
lines changed

3 files changed

+342
-13
lines changed

‎crates/cheatcodes/src/inspector.rs‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,25 +1333,29 @@ impl Cheatcodes {
13331333
fn meter_gas(&mut self, interpreter: &mut Interpreter) {
13341334
match &self.gas_metering {
13351335
None => {}
1336-
// need to store gas metering
1336+
// Need to store gas metering.
13371337
Some(None) => self.gas_metering = Some(Some(interpreter.gas)),
13381338
Some(Some(gas)) => {
13391339
match interpreter.current_opcode() {
13401340
opcode::CREATE | opcode::CREATE2 => {
1341-
// set we're about to enter CREATE frame to meter its gas on first opcode
1342-
// inside it
1341+
// Set we're about to enter CREATE frame to meter its gas on first opcode
1342+
// inside it.
13431343
self.gas_metering_create = Some(None)
13441344
}
13451345
opcode::STOP | opcode::RETURN | opcode::SELFDESTRUCT | opcode::REVERT => {
1346-
// If we are ending current execution frame, we want to just fully reset gas
1347-
// otherwise weird things with returning gas from a call happen
1348-
// ref: https://github.com/bluealloy/revm/blob/2cb991091d32330cfe085320891737186947ce5a/crates/revm/src/evm_impl.rs#L190
1349-
//
1350-
// It would be nice if we had access to the interpreter in `call_end`, as we
1351-
// could just do this there instead.
13521346
match &self.gas_metering_create {
13531347
None | Some(None) => {
1354-
interpreter.gas = Gas::new(0);
1348+
// If we are ending current execution frame, we want to reset
1349+
// interpreter gas to the value of gas spent during frame, so only
1350+
// the consumed gas is erased.
1351+
// ref: https://github.com/bluealloy/revm/blob/2cb991091d32330cfe085320891737186947ce5a/crates/revm/src/evm_impl.rs#L190
1352+
//
1353+
// It would be nice if we had access to the interpreter in
1354+
// `call_end`, as we could just do this there instead.
1355+
interpreter.gas = Gas::new(interpreter.gas.spent());
1356+
1357+
// Make sure CREATE gas metering is resetted.
1358+
self.gas_metering_create = None
13551359
}
13561360
Some(Some(gas)) => {
13571361
// If this was CREATE frame, set correct gas limit. This is needed
@@ -1367,18 +1371,18 @@ impl Cheatcodes {
13671371
// post-create erase ref: https://github.com/bluealloy/revm/blob/2cb991091d32330cfe085320891737186947ce5a/crates/revm/src/instructions/host.rs#L279
13681372
interpreter.gas = Gas::new(gas.limit());
13691373

1370-
// reset CREATE gas metering because we're about to exit its frame
1374+
// Reset CREATE gas metering because we're about to exit its frame.
13711375
self.gas_metering_create = None
13721376
}
13731377
}
13741378
}
13751379
_ => {
1376-
// if just starting with CREATE opcodes, record its inner frame gas
1380+
// If just starting with CREATE opcodes, record its inner frame gas.
13771381
if self.gas_metering_create == Some(None) {
13781382
self.gas_metering_create = Some(Some(interpreter.gas))
13791383
}
13801384

1381-
// dont monitor gas changes, keep it constant
1385+
// Don't monitor gas changes, keep it constant.
13821386
interpreter.gas = *gas;
13831387
}
13841388
}

‎crates/forge/tests/it/repros.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,3 +361,6 @@ test_repro!(8287);
361361

362362
// https://github.com/foundry-rs/foundry/issues/8168
363363
test_repro!(8168);
364+
365+
// https://github.com/foundry-rs/foundry/issues/8383
366+
test_repro!(8383);

‎testdata/default/repros/Issue8383.t.sol‎

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
// SPDX-License-Identifier: MIT OR Apache-2.0
2+
pragma solidity 0.8.18;
3+
4+
import "ds-test/test.sol";
5+
import "cheats/Vm.sol";
6+
7+
// https://github.com/foundry-rs/foundry/issues/8383
8+
contract Issue8383Test is DSTest {
9+
Vm constant vm = Vm(HEVM_ADDRESS);
10+
11+
address internal _verifier;
12+
13+
mapping(bytes32 => bool) internal _vectorTested;
14+
mapping(bytes32 => bool) internal _vectorResult;
15+
16+
function setUp() public {
17+
_verifier = address(new P256Verifier());
18+
}
19+
20+
function _verifyViaVerifier(bytes32 hash, uint256 r, uint256 s, uint256 x, uint256 y) internal returns (bool) {
21+
return _verifyViaVerifier(hash, bytes32(r), bytes32(s), bytes32(x), bytes32(y));
22+
}
23+
24+
function _verifyViaVerifier(bytes32 hash, bytes32 r, bytes32 s, bytes32 x, bytes32 y) internal returns (bool) {
25+
bytes memory payload = abi.encode(hash, r, s, x, y);
26+
if (uint256(y) & 0xff == 0) {
27+
bytes memory truncatedPayload = abi.encodePacked(hash, r, s, x, bytes31(y));
28+
_verifierCall(truncatedPayload);
29+
}
30+
if (uint256(keccak256(abi.encode(payload, "1"))) & 0x1f == 0) {
31+
uint256 r = uint256(keccak256(abi.encode(payload, "2")));
32+
payload = abi.encodePacked(payload, new bytes(r & 0xff));
33+
}
34+
bytes32 payloadHash = keccak256(payload);
35+
if (_vectorTested[payloadHash]) return _vectorResult[payloadHash];
36+
_vectorTested[payloadHash] = true;
37+
return (_vectorResult[payloadHash] = _verifierCall(payload));
38+
}
39+
40+
function _verifierCall(bytes memory payload) internal returns (bool) {
41+
(bool success, bytes memory result) = _verifier.call(payload);
42+
return abi.decode(result, (bool));
43+
}
44+
45+
function testP256VerifyOutOfBounds() public {
46+
vm.pauseGasMetering();
47+
uint256 p = 0xFFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF;
48+
_verifyViaVerifier(bytes32(0), 1, 1, 1, 1);
49+
_verifyViaVerifier(bytes32(0), 1, 1, 0, 1);
50+
_verifyViaVerifier(bytes32(0), 1, 1, 1, 0);
51+
_verifyViaVerifier(bytes32(0), 1, 1, 1, p);
52+
_verifyViaVerifier(bytes32(0), 1, 1, p, 1);
53+
_verifyViaVerifier(bytes32(0), 1, 1, p - 1, 1);
54+
vm.resumeGasMetering();
55+
}
56+
}
57+
58+
contract P256Verifier {
59+
uint256 private constant GX = 0x6B17D1F2E12C4247F8BCE6E563A440F277037D812DEB33A0F4A13945D898C296;
60+
uint256 private constant GY = 0x4FE342E2FE1A7F9B8EE7EB4A7C0F9E162BCE33576B315ECECBB6406837BF51F5;
61+
uint256 private constant P = 0xFFFFFFFF00000001000000000000000000000000FFFFFFFFFFFFFFFFFFFFFFFF; // `A = P - 3`.
62+
uint256 private constant N = 0xFFFFFFFF00000000FFFFFFFFFFFFFFFFBCE6FAADA7179E84F3B9CAC2FC632551;
63+
uint256 private constant B = 0x5AC635D8AA3A93E7B3EBBD55769886BC651D06B0CC53B0F63BCE3C3E27D2604B;
64+
65+
fallback() external payable {
66+
assembly {
67+
// For this implementation, we will use the memory without caring about
68+
// the free memory pointer or zero pointer.
69+
// The slots `0x00`, `0x20`, `0x40`, `0x60`, will not be accessed for the `Points[16]` array,
70+
// and can be used for storing other variables.
71+
72+
mstore(0x40, P) // Set `0x40` to `P`.
73+
74+
function jAdd(x1, y1, z1, x2, y2, z2) -> x3, y3, z3 {
75+
if iszero(z1) {
76+
x3 := x2
77+
y3 := y2
78+
z3 := z2
79+
leave
80+
}
81+
if iszero(z2) {
82+
x3 := x1
83+
y3 := y1
84+
z3 := z1
85+
leave
86+
}
87+
let p := mload(0x40)
88+
let zz1 := mulmod(z1, z1, p)
89+
let zz2 := mulmod(z2, z2, p)
90+
let u1 := mulmod(x1, zz2, p)
91+
let u2 := mulmod(x2, zz1, p)
92+
let s1 := mulmod(y1, mulmod(zz2, z2, p), p)
93+
let s2 := mulmod(y2, mulmod(zz1, z1, p), p)
94+
let h := addmod(u2, sub(p, u1), p)
95+
let hh := mulmod(h, h, p)
96+
let hhh := mulmod(h, hh, p)
97+
let r := addmod(s2, sub(p, s1), p)
98+
x3 := addmod(addmod(mulmod(r, r, p), sub(p, hhh), p), sub(p, mulmod(2, mulmod(u1, hh, p), p)), p)
99+
y3 := addmod(mulmod(r, addmod(mulmod(u1, hh, p), sub(p, x3), p), p), sub(p, mulmod(s1, hhh, p)), p)
100+
z3 := mulmod(h, mulmod(z1, z2, p), p)
101+
}
102+
103+
function setJPoint(i, x, y, z) {
104+
// We will multiply by `0x80` (i.e. `shl(7, i)`) instead
105+
// since the memory expansion costs are cheaper than doing `mul(0x60, i)`.
106+
// Also help combine the lookup expression for `u1` and `u2` in `jMultShamir`.
107+
i := shl(7, i)
108+
mstore(i, x)
109+
mstore(add(i, returndatasize()), y)
110+
mstore(add(i, 0x40), z)
111+
}
112+
113+
function setJPointDouble(i, j) {
114+
j := shl(7, j)
115+
let x := mload(j)
116+
let y := mload(add(j, returndatasize()))
117+
let z := mload(add(j, 0x40))
118+
let p := mload(0x40)
119+
let yy := mulmod(y, y, p)
120+
let zz := mulmod(z, z, p)
121+
let s := mulmod(4, mulmod(x, yy, p), p)
122+
let m := addmod(mulmod(3, mulmod(x, x, p), p), mulmod(mload(returndatasize()), mulmod(zz, zz, p), p), p)
123+
let x2 := addmod(mulmod(m, m, p), sub(p, mulmod(2, s, p)), p)
124+
let y2 := addmod(mulmod(m, addmod(s, sub(p, x2), p), p), sub(p, mulmod(8, mulmod(yy, yy, p), p)), p)
125+
let z2 := mulmod(2, mulmod(y, z, p), p)
126+
setJPoint(i, x2, y2, z2)
127+
}
128+
129+
function setJPointAdd(i, j, k) {
130+
j := shl(7, j)
131+
k := shl(7, k)
132+
let x, y, z :=
133+
jAdd(
134+
mload(j),
135+
mload(add(j, returndatasize())),
136+
mload(add(j, 0x40)),
137+
mload(k),
138+
mload(add(k, returndatasize())),
139+
mload(add(k, 0x40))
140+
)
141+
setJPoint(i, x, y, z)
142+
}
143+
144+
let r := calldataload(0x20)
145+
let n := N
146+
147+
{
148+
let s := calldataload(0x40)
149+
if lt(shr(1, n), s) { s := sub(n, s) }
150+
151+
// Perform `modExp(s, N - 2, N)`.
152+
// After which, we can abuse `returndatasize()` to get `0x20`.
153+
mstore(0x800, 0x20)
154+
mstore(0x820, 0x20)
155+
mstore(0x840, 0x20)
156+
mstore(0x860, s)
157+
mstore(0x880, sub(n, 2))
158+
mstore(0x8a0, n)
159+
160+
let p := mload(0x40)
161+
mstore(0x20, xor(3, p)) // Set `0x20` to `A`.
162+
let Qx := calldataload(0x60)
163+
let Qy := calldataload(0x80)
164+
165+
if iszero(
166+
and( // The arguments of `and` are evaluated last to first.
167+
and(
168+
and(gt(calldatasize(), 0x9f), and(lt(iszero(r), lt(r, n)), lt(iszero(s), lt(s, n)))),
169+
eq(
170+
mulmod(Qy, Qy, p),
171+
addmod(mulmod(addmod(mulmod(Qx, Qx, p), mload(returndatasize()), p), Qx, p), B, p)
172+
)
173+
),
174+
and(
175+
// We need to check that the `returndatasize` is indeed 32,
176+
// so that we can return false if the chain does not have the modexp precompile.
177+
eq(returndatasize(), 0x20),
178+
staticcall(gas(), 0x05, 0x800, 0xc0, returndatasize(), 0x20)
179+
)
180+
)
181+
) {
182+
// POC Note:
183+
// Changing this to `return(0x80, 0x20)` fixes it.
184+
// Alternatively, adding `if mload(0x8c0) { invalid() }` just before the return also fixes it.
185+
return(0x8c0, 0x20)
186+
}
187+
188+
setJPoint(0x01, Qx, Qy, 1)
189+
setJPoint(0x04, GX, GY, 1)
190+
setJPointDouble(0x02, 0x01)
191+
setJPointDouble(0x08, 0x04)
192+
setJPointAdd(0x03, 0x01, 0x02)
193+
setJPointAdd(0x05, 0x01, 0x04)
194+
setJPointAdd(0x06, 0x02, 0x04)
195+
setJPointAdd(0x07, 0x03, 0x04)
196+
setJPointAdd(0x09, 0x01, 0x08)
197+
setJPointAdd(0x0a, 0x02, 0x08)
198+
setJPointAdd(0x0b, 0x03, 0x08)
199+
setJPointAdd(0x0c, 0x04, 0x08)
200+
setJPointAdd(0x0d, 0x01, 0x0c)
201+
setJPointAdd(0x0e, 0x02, 0x0c)
202+
setJPointAdd(0x0f, 0x03, 0x0c)
203+
}
204+
205+
let i := 0
206+
let u1 := mulmod(calldataload(0x00), mload(0x00), n)
207+
let u2 := mulmod(r, mload(0x00), n)
208+
let y := 0
209+
let z := 0
210+
let x := 0
211+
let p := mload(0x40)
212+
for {} 1 {} {
213+
if z {
214+
let yy := mulmod(y, y, p)
215+
let zz := mulmod(z, z, p)
216+
let s := mulmod(4, mulmod(x, yy, p), p)
217+
let m :=
218+
addmod(mulmod(3, mulmod(x, x, p), p), mulmod(mload(returndatasize()), mulmod(zz, zz, p), p), p)
219+
let x2 := addmod(mulmod(m, m, p), sub(p, mulmod(2, s, p)), p)
220+
let y2 := addmod(mulmod(m, addmod(s, sub(p, x2), p), p), sub(p, mulmod(8, mulmod(yy, yy, p), p)), p)
221+
let z2 := mulmod(2, mulmod(y, z, p), p)
222+
yy := mulmod(y2, y2, p)
223+
zz := mulmod(z2, z2, p)
224+
s := mulmod(4, mulmod(x2, yy, p), p)
225+
m :=
226+
addmod(mulmod(3, mulmod(x2, x2, p), p), mulmod(mload(returndatasize()), mulmod(zz, zz, p), p), p)
227+
x := addmod(mulmod(m, m, p), sub(p, mulmod(2, s, p)), p)
228+
z := mulmod(2, mulmod(y2, z2, p), p)
229+
y := addmod(mulmod(m, addmod(s, sub(p, x), p), p), sub(p, mulmod(8, mulmod(yy, yy, p), p)), p)
230+
}
231+
for { let o := or(and(shr(245, shl(i, u1)), 0x600), and(shr(247, shl(i, u2)), 0x180)) } o {} {
232+
let z2 := mload(add(o, 0x40))
233+
if iszero(z2) { break }
234+
if iszero(z) {
235+
x := mload(o)
236+
y := mload(add(o, returndatasize()))
237+
z := z2
238+
break
239+
}
240+
let zz1 := mulmod(z, z, p)
241+
let zz2 := mulmod(z2, z2, p)
242+
let u1_ := mulmod(x, zz2, p)
243+
let s1 := mulmod(y, mulmod(zz2, z2, p), p)
244+
let h := addmod(mulmod(mload(o), zz1, p), sub(p, u1_), p)
245+
let hh := mulmod(h, h, p)
246+
let hhh := mulmod(h, hh, p)
247+
let r_ := addmod(mulmod(mload(add(o, returndatasize())), mulmod(zz1, z, p), p), sub(p, s1), p)
248+
x := addmod(addmod(mulmod(r_, r_, p), sub(p, hhh), p), sub(p, mulmod(2, mulmod(u1_, hh, p), p)), p)
249+
y := addmod(mulmod(r_, addmod(mulmod(u1_, hh, p), sub(p, x), p), p), sub(p, mulmod(s1, hhh, p)), p)
250+
z := mulmod(h, mulmod(z, z2, p), p)
251+
break
252+
}
253+
// Just unroll twice. Fully unrolling will only save around 1% to 2% gas, but make the
254+
// bytecode very bloated, which may incur more runtime costs after Verkle.
255+
// See: https://notes.ethereum.org/%40vbuterin/verkle_tree_eip
256+
// It's very unlikely that Verkle will come before the P256 precompile. But who knows?
257+
if z {
258+
let yy := mulmod(y, y, p)
259+
let zz := mulmod(z, z, p)
260+
let s := mulmod(4, mulmod(x, yy, p), p)
261+
let m :=
262+
addmod(mulmod(3, mulmod(x, x, p), p), mulmod(mload(returndatasize()), mulmod(zz, zz, p), p), p)
263+
let x2 := addmod(mulmod(m, m, p), sub(p, mulmod(2, s, p)), p)
264+
let y2 := addmod(mulmod(m, addmod(s, sub(p, x2), p), p), sub(p, mulmod(8, mulmod(yy, yy, p), p)), p)
265+
let z2 := mulmod(2, mulmod(y, z, p), p)
266+
yy := mulmod(y2, y2, p)
267+
zz := mulmod(z2, z2, p)
268+
s := mulmod(4, mulmod(x2, yy, p), p)
269+
m :=
270+
addmod(mulmod(3, mulmod(x2, x2, p), p), mulmod(mload(returndatasize()), mulmod(zz, zz, p), p), p)
271+
x := addmod(mulmod(m, m, p), sub(p, mulmod(2, s, p)), p)
272+
z := mulmod(2, mulmod(y2, z2, p), p)
273+
y := addmod(mulmod(m, addmod(s, sub(p, x), p), p), sub(p, mulmod(8, mulmod(yy, yy, p), p)), p)
274+
}
275+
for { let o := or(and(shr(243, shl(i, u1)), 0x600), and(shr(245, shl(i, u2)), 0x180)) } o {} {
276+
let z2 := mload(add(o, 0x40))
277+
if iszero(z2) { break }
278+
if iszero(z) {
279+
x := mload(o)
280+
y := mload(add(o, returndatasize()))
281+
z := z2
282+
break
283+
}
284+
let zz1 := mulmod(z, z, p)
285+
let zz2 := mulmod(z2, z2, p)
286+
let u1_ := mulmod(x, zz2, p)
287+
let s1 := mulmod(y, mulmod(zz2, z2, p), p)
288+
let h := addmod(mulmod(mload(o), zz1, p), sub(p, u1_), p)
289+
let hh := mulmod(h, h, p)
290+
let hhh := mulmod(h, hh, p)
291+
let r_ := addmod(mulmod(mload(add(o, returndatasize())), mulmod(zz1, z, p), p), sub(p, s1), p)
292+
x := addmod(addmod(mulmod(r_, r_, p), sub(p, hhh), p), sub(p, mulmod(2, mulmod(u1_, hh, p), p)), p)
293+
y := addmod(mulmod(r_, addmod(mulmod(u1_, hh, p), sub(p, x), p), p), sub(p, mulmod(s1, hhh, p)), p)
294+
z := mulmod(h, mulmod(z, z2, p), p)
295+
break
296+
}
297+
i := add(i, 4)
298+
if eq(i, 256) { break }
299+
}
300+
301+
if iszero(z) {
302+
mstore(returndatasize(), iszero(r))
303+
return(returndatasize(), 0x20)
304+
}
305+
306+
// Perform `modExp(z, P - 2, P)`.
307+
// `0x800`, `0x820, `0x840` are still set to `0x20`.
308+
mstore(0x860, z)
309+
mstore(0x880, sub(p, 2))
310+
mstore(0x8a0, p)
311+
312+
mstore(
313+
returndatasize(),
314+
and( // The arguments of `and` are evaluated last to first.
315+
eq(mod(mulmod(x, mulmod(mload(returndatasize()), mload(returndatasize()), p), p), n), r),
316+
staticcall(gas(), 0x05, 0x800, 0xc0, returndatasize(), returndatasize())
317+
)
318+
)
319+
return(returndatasize(), returndatasize())
320+
}
321+
}
322+
}

0 commit comments

Comments
 (0)