From 2ef7f09a7cacc52007ddfd69c816534cdbcec8b3 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 7 Nov 2024 02:51:54 -0500 Subject: [PATCH 01/15] fix[codegen]: assert staticcall for identity and ecrecover precompiles in 93a957947af1088addc, the assert for memory copying calls to the identity precompile was optimized out; the reasoning being that if the identity precompile fails due to OOG, the caller contract would also likely fail with OOG. however, due to the 63/64ths rule, there are cases where the identity precompile could fail with OOG, but the caller contract has enough gas to successfully return out of the call context. (note that even prior to 93a957947af1088addc, some calls to the identity precompile did not check the success flag. cf. commit cf03d27be6a74c0c). this only affects pre-cancun compilation targets, since post-cancun the `mcopy` instruction is chosen. this commit fixes the identity precompile call for pre-cancun targets. a similar optimization was also applied in the past to omit the check when calling the ecrecover precompile. a check for the ecrecover call is applied in this commit. while this is a performance regression, ecrecover is generally not a hotspot; as for the identity precompile, it is only used by the compiler for memory copies pre-cancun, so the performance regression there is not too relevant (as of nov 2024). --- vyper/builtins/functions.py | 2 +- vyper/codegen/core.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 62539872bc..55d5443a8f 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -781,7 +781,7 @@ def build_IR(self, expr, args, kwargs, context): ["mstore", add_ofst(input_buf, 32), args[1]], ["mstore", add_ofst(input_buf, 64), args[2]], ["mstore", add_ofst(input_buf, 96), args[3]], - ["staticcall", "gas", 1, input_buf, 128, output_buf, 32], + ["assert", ["staticcall", "gas", 1, input_buf, 128, output_buf, 32]], ["mload", output_buf], ], typ=AddressT(), diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index 0ad7fa79c6..aaf6f35047 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -326,7 +326,7 @@ def copy_bytes(dst, src, length, length_bound): copy_op = ["mcopy", dst, src, length] gas_bound = _mcopy_gas_bound(length_bound) else: - copy_op = ["staticcall", "gas", 4, src, length, dst, length] + copy_op = ["assert", ["staticcall", "gas", 4, src, length, dst, length]] gas_bound = _identity_gas_bound(length_bound) elif src.location == CALLDATA: copy_op = ["calldatacopy", dst, src, length] From 71824c7b0834877ba5aad5de22090c5b6f698a21 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 2 Jan 2025 10:56:57 -0500 Subject: [PATCH 02/15] add a test --- .../builtins/codegen/test_ecrecover.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index 8db51fdd07..d42181eebe 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -86,3 +86,26 @@ def test_ecrecover() -> bool: """ c = get_contract(code) assert c.test_ecrecover() is True + + +def test_ecrecover_oog_handling(get_contract, tx_failed): + code = """ +@external +@view +def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: + return ecrecover(hash, v, r, s) + """ + c = get_contract(code) + + h = b"\x35" * 32 + local_account = Account.from_key(b"\x46" * 32) + sig = local_account.signHash(h) + v, r, s = sig.v, sig.r, sig.s + + assert c.do_ecrecover(h, v, r, s) == local_account.address + + with tx_failed(): + # provide enough spare gas for the top-level call to not oog but + # not enough for ecrecover to succeed + spare_change = 500 + c.do_ecrecover(h, v, r, s, gas=(21000 + spare_change + 3000)) From c3f08d71bb4a1adaaf4c094aca33d218b6a3385f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 2 Jan 2025 12:00:06 -0500 Subject: [PATCH 03/15] add more tests, use gas_used to specify gas exactly --- .../builtins/codegen/test_ecrecover.py | 7 ++-- .../codegen/types/test_dynamic_array.py | 38 +++++++++++++++++ tests/functional/codegen/types/test_lists.py | 41 +++++++++++++++++++ tests/functional/codegen/types/test_string.py | 38 +++++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index d42181eebe..45dee8665b 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -88,7 +88,8 @@ def test_ecrecover() -> bool: assert c.test_ecrecover() is True -def test_ecrecover_oog_handling(get_contract, tx_failed): +def test_ecrecover_oog_handling(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 code = """ @external @view @@ -103,9 +104,9 @@ def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: v, r, s = sig.v, sig.r, sig.s assert c.do_ecrecover(h, v, r, s) == local_account.address + gas_used = env.last_result.gas_used with tx_failed(): # provide enough spare gas for the top-level call to not oog but # not enough for ecrecover to succeed - spare_change = 500 - c.do_ecrecover(h, v, r, s, gas=(21000 + spare_change + 3000)) + c.do_ecrecover(h, v, r, s, gas=gas_used - 1) diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 2f647ac38c..22b86e8da5 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -3,6 +3,7 @@ import pytest +from tests.evm_backends.base_env import EvmError from tests.utils import decimal_to_int from vyper.compiler import compile_code from vyper.exceptions import ( @@ -1901,3 +1902,40 @@ def foo(): c = get_contract(code) with tx_failed(): c.foo() + + +def test_dynarray_copy_oog(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 + code = """ + +@external +def foo(a: DynArray[uint256, 4000]) -> uint256: + b: DynArray[uint256, 4000] = a + return b[0] + """ + c = get_contract(code) + dynarray = [2] * 4000 + assert c.foo(dynarray) == 2 + gas_used = env.last_result.gas_used + with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + c.foo(dynarray, gas=gas_used - 1) + + +def test_dynarray_copy_oog2(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 + code = """ +@external +@view +def foo(x: String[1000000], y: String[1000000]) -> DynArray[String[1000000], 2]: + z: DynArray[String[1000000], 2] = [x, y] + # Some code + return z + """ + + c = get_contract(code) + calldata0 = "a" * 10 + calldata1 = "b" * 1000000 + assert c.foo(calldata0, calldata1) == [calldata0, calldata1] + gas_used = env.last_result.gas_used + with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + c.foo(calldata0, calldata1, gas=gas_used - 1) diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index 953a9a9f9f..eba589bc01 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -2,6 +2,7 @@ import pytest +from tests.evm_backends.base_env import EvmError from tests.utils import decimal_to_int from vyper.exceptions import ArrayIndexException, OverflowException, TypeMismatch @@ -848,3 +849,43 @@ def foo() -> {return_type}: return MY_CONSTANT[0][0] """ assert_compile_failed(lambda: get_contract(code), TypeMismatch) + + +def test_array_copy_oog(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 + code = """ +@internal +def bar(x: uint256[3000]) -> uint256[3000]: + a: uint256[3000] = x + return a + +@external +def foo(x: uint256[3000]) -> uint256: + s: uint256[3000] = self.bar(x) + return s[0] + """ + + c = get_contract(code) + array = [2] * 3000 + assert c.foo(array) == array[0] + gas_used = env.last_result.gas_used + with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + c.foo(array, gas=gas_used - 1) + + +def test_array_copy_oog2(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 + code = """ +@external +def foo(x: uint256[2500]) -> uint256: + s: uint256[2500] = x + t: uint256[2500] = s + return t[0] + """ + + c = get_contract(code) + array = [2] * 2500 + assert c.foo(array) == array[0] + gas_used = env.last_result.gas_used + with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + c.foo(array, gas=gas_used - 1) diff --git a/tests/functional/codegen/types/test_string.py b/tests/functional/codegen/types/test_string.py index 1c186eeb6e..5493ffddca 100644 --- a/tests/functional/codegen/types/test_string.py +++ b/tests/functional/codegen/types/test_string.py @@ -1,5 +1,7 @@ import pytest +from tests.evm_backends.base_env import EvmError + def test_string_return(get_contract): code = """ @@ -359,3 +361,39 @@ def compare_var_storage_not_equal_false() -> bool: assert c.compare_var_storage_equal_false() is False assert c.compare_var_storage_not_equal_true() is True assert c.compare_var_storage_not_equal_false() is False + + +def test_string_copy_oog(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 + code = """ +@external +@view +def foo(x: String[1000000]) -> String[1000000]: + return x + """ + c = get_contract(code) + calldata = "a" * 1000000 + assert c.foo(calldata) == calldata + gas_used = env.last_result.gas_used - 1 + # note: EvmError catches both reverts and exceptional halts (oog). + # this is dependent on the target evm version. + with tx_failed(EvmError): + c.foo(calldata, gas=gas_used - 1) + + +def test_string_copy_oog2(env, get_contract, tx_failed): + # GHSA-vgf2-gvx8-xwc3 + code = """ +@external +@view +def foo(x: String[1000000]) -> uint256: + return len(x) + """ + c = get_contract(code) + calldata = "a" * 1000000 + assert c.foo(calldata) == len(calldata) + gas_used = env.last_result.gas_used - 1 + # note: EvmError catches both reverts and exceptional halts (oog). + # this is dependent on the target evm version. + with tx_failed(EvmError): + c.foo(calldata, gas=gas_used - 1) From 4eb8332638345b7e104d32383fe3cf1aeb0f71d0 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 14 Jan 2025 12:25:15 -0500 Subject: [PATCH 04/15] handle failures when --optimize is set to none --- tests/functional/builtins/codegen/test_ecrecover.py | 3 ++- tests/functional/codegen/types/test_lists.py | 11 +++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index 45dee8665b..d50ea91bcc 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -1,6 +1,7 @@ from eth_account import Account from eth_account._utils.signing import to_bytes32 +from tests.evm_backends.base_env import EvmError from tests.utils import ZERO_ADDRESS @@ -106,7 +107,7 @@ def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: assert c.do_ecrecover(h, v, r, s) == local_account.address gas_used = env.last_result.gas_used - with tx_failed(): + with tx_failed(EvmError): # provide enough spare gas for the top-level call to not oog but # not enough for ecrecover to succeed c.do_ecrecover(h, v, r, s, gas=gas_used - 1) diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index eba589bc01..d54ff36b4f 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -4,6 +4,7 @@ from tests.evm_backends.base_env import EvmError from tests.utils import decimal_to_int +from vyper.compiler.settings import OptimizationLevel from vyper.exceptions import ArrayIndexException, OverflowException, TypeMismatch @@ -851,7 +852,7 @@ def foo() -> {return_type}: assert_compile_failed(lambda: get_contract(code), TypeMismatch) -def test_array_copy_oog(env, get_contract, tx_failed): +def test_array_copy_oog(env, get_contract, tx_failed, optimize, request): # GHSA-vgf2-gvx8-xwc3 code = """ @internal @@ -864,6 +865,9 @@ def foo(x: uint256[3000]) -> uint256: s: uint256[3000] = self.bar(x) return s[0] """ + if optimize == OptimizationLevel.NONE: + # fails in get_contract due to code too large + request.node.add_marker(pytest.mark.xfail(strict=True)) c = get_contract(code) array = [2] * 3000 @@ -873,7 +877,7 @@ def foo(x: uint256[3000]) -> uint256: c.foo(array, gas=gas_used - 1) -def test_array_copy_oog2(env, get_contract, tx_failed): +def test_array_copy_oog2(env, get_contract, tx_failed, optimize, request): # GHSA-vgf2-gvx8-xwc3 code = """ @external @@ -882,6 +886,9 @@ def foo(x: uint256[2500]) -> uint256: t: uint256[2500] = s return t[0] """ + if optimize == OptimizationLevel.NONE: + # fails in get_contract due to code too large + request.node.add_marker(pytest.mark.xfail(strict=True)) c = get_contract(code) array = [2] * 2500 From 0ccac1fdbd9619448b0d506945866ab92abf6594 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 14 Jan 2025 12:54:28 -0500 Subject: [PATCH 05/15] refine xfail condition --- tests/functional/codegen/types/test_lists.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index d54ff36b4f..9dedbc6360 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -852,7 +852,7 @@ def foo() -> {return_type}: assert_compile_failed(lambda: get_contract(code), TypeMismatch) -def test_array_copy_oog(env, get_contract, tx_failed, optimize, request): +def test_array_copy_oog(env, get_contract, tx_failed, optimize, experimental_codegen, request): # GHSA-vgf2-gvx8-xwc3 code = """ @internal @@ -865,7 +865,7 @@ def foo(x: uint256[3000]) -> uint256: s: uint256[3000] = self.bar(x) return s[0] """ - if optimize == OptimizationLevel.NONE: + if optimize == OptimizationLevel.NONE and not experimental_codegen: # fails in get_contract due to code too large request.node.add_marker(pytest.mark.xfail(strict=True)) @@ -877,7 +877,7 @@ def foo(x: uint256[3000]) -> uint256: c.foo(array, gas=gas_used - 1) -def test_array_copy_oog2(env, get_contract, tx_failed, optimize, request): +def test_array_copy_oog2(env, get_contract, tx_failed, optimize, experimental_codegen, request): # GHSA-vgf2-gvx8-xwc3 code = """ @external @@ -886,7 +886,7 @@ def foo(x: uint256[2500]) -> uint256: t: uint256[2500] = s return t[0] """ - if optimize == OptimizationLevel.NONE: + if optimize == OptimizationLevel.NONE and not experimental_codegen: # fails in get_contract due to code too large request.node.add_marker(pytest.mark.xfail(strict=True)) From b501329c8d7e968b40127bc3d5bbb8e3ac389ac8 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 15 Jan 2025 11:00:16 -0500 Subject: [PATCH 06/15] add comments to tests --- tests/functional/codegen/types/test_dynamic_array.py | 2 ++ tests/functional/codegen/types/test_lists.py | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 22b86e8da5..303d0920f9 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -1918,6 +1918,7 @@ def foo(a: DynArray[uint256, 4000]) -> uint256: assert c.foo(dynarray) == 2 gas_used = env.last_result.gas_used with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + # should fail pre-cancun due to identity precompile returning 0 c.foo(dynarray, gas=gas_used - 1) @@ -1938,4 +1939,5 @@ def foo(x: String[1000000], y: String[1000000]) -> DynArray[String[1000000], 2]: assert c.foo(calldata0, calldata1) == [calldata0, calldata1] gas_used = env.last_result.gas_used with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + # should fail pre-cancun due to identity precompile returning 0 c.foo(calldata0, calldata1, gas=gas_used - 1) diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index 9dedbc6360..678cee219e 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -874,6 +874,8 @@ def foo(x: uint256[3000]) -> uint256: assert c.foo(array) == array[0] gas_used = env.last_result.gas_used with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + # depends on EVM version. pre-cancun, will revert due to checking + # success flag from identity precompile. c.foo(array, gas=gas_used - 1) @@ -895,4 +897,6 @@ def foo(x: uint256[2500]) -> uint256: assert c.foo(array) == array[0] gas_used = env.last_result.gas_used with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + # depends on EVM version. pre-cancun, will revert due to checking + # success flag from identity precompile. c.foo(array, gas=gas_used - 1) From a882c9240092c73d44e5ab2382b5381d0c8e1555 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 15 Jan 2025 11:50:37 -0500 Subject: [PATCH 07/15] refine test failure reasons gas_used - 1 will always revert. we want to supply enough gas that the inner call will return 0 but the outer call will not oog. --- .../builtins/codegen/test_ecrecover.py | 6 ++-- .../codegen/types/test_dynamic_array.py | 28 ++++++++++++---- tests/functional/codegen/types/test_lists.py | 22 ++++++++++--- tests/functional/codegen/types/test_string.py | 32 +++++++++++++------ 4 files changed, 65 insertions(+), 23 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index d50ea91bcc..9adece00fc 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -105,9 +105,9 @@ def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: v, r, s = sig.v, sig.r, sig.s assert c.do_ecrecover(h, v, r, s) == local_account.address - gas_used = env.last_result.gas_used - with tx_failed(EvmError): + gas_used = env.last_result.gas_used + with tx_failed(): # provide enough spare gas for the top-level call to not oog but # not enough for ecrecover to succeed - c.do_ecrecover(h, v, r, s, gas=gas_used - 1) + c.do_ecrecover(h, v, r, s, gas=gas_used) diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 303d0920f9..948349043e 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -1,4 +1,6 @@ import itertools +from vyper.evm.opcodes import version_check +import contextlib from typing import Any, Callable import pytest @@ -1916,10 +1918,17 @@ def foo(a: DynArray[uint256, 4000]) -> uint256: c = get_contract(code) dynarray = [2] * 4000 assert c.foo(dynarray) == 2 + gas_used = env.last_result.gas_used - with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog - # should fail pre-cancun due to identity precompile returning 0 - c.foo(dynarray, gas=gas_used - 1) + if version_check(begin="cancun"): + ctx = contextlib.nullcontext + else: + ctx = tx_failed + + with ctx(): + # depends on EVM version. pre-cancun, will revert due to checking + # success flag from identity precompile. + c.foo(dynarray, gas=gas_used) def test_dynarray_copy_oog2(env, get_contract, tx_failed): @@ -1937,7 +1946,14 @@ def foo(x: String[1000000], y: String[1000000]) -> DynArray[String[1000000], 2]: calldata0 = "a" * 10 calldata1 = "b" * 1000000 assert c.foo(calldata0, calldata1) == [calldata0, calldata1] + gas_used = env.last_result.gas_used - with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog - # should fail pre-cancun due to identity precompile returning 0 - c.foo(calldata0, calldata1, gas=gas_used - 1) + if version_check(begin="cancun"): + ctx = contextlib.nullcontext + else: + ctx = tx_failed + + with ctx(): + # depends on EVM version. pre-cancun, will revert due to checking + # success flag from identity precompile. + c.foo(calldata0, calldata1, gas=gas_used) diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index 678cee219e..7d92cd9f22 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -1,4 +1,6 @@ import itertools +import contextlib +from vyper.evm.opcodes import version_check import pytest @@ -872,11 +874,17 @@ def foo(x: uint256[3000]) -> uint256: c = get_contract(code) array = [2] * 3000 assert c.foo(array) == array[0] + + # get the minimum gas for the contract complete execution gas_used = env.last_result.gas_used - with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + if version_check(begin= "cancun"): + ctx = contextlib.nullcontext + else: + ctx = tx_failed + with ctx(): # depends on EVM version. pre-cancun, will revert due to checking # success flag from identity precompile. - c.foo(array, gas=gas_used - 1) + c.foo(array, gas=gas_used) def test_array_copy_oog2(env, get_contract, tx_failed, optimize, experimental_codegen, request): @@ -895,8 +903,14 @@ def foo(x: uint256[2500]) -> uint256: c = get_contract(code) array = [2] * 2500 assert c.foo(array) == array[0] + + # get the minimum gas for the contract complete execution gas_used = env.last_result.gas_used - with tx_failed(EvmError): # catch reverts *and* exceptional halt from oog + if version_check(begin= "cancun"): + ctx = contextlib.nullcontext + else: + ctx = tx_failed + with ctx(): # depends on EVM version. pre-cancun, will revert due to checking # success flag from identity precompile. - c.foo(array, gas=gas_used - 1) + c.foo(array, gas=gas_used) diff --git a/tests/functional/codegen/types/test_string.py b/tests/functional/codegen/types/test_string.py index 5493ffddca..2589e55bc9 100644 --- a/tests/functional/codegen/types/test_string.py +++ b/tests/functional/codegen/types/test_string.py @@ -374,11 +374,17 @@ def foo(x: String[1000000]) -> String[1000000]: c = get_contract(code) calldata = "a" * 1000000 assert c.foo(calldata) == calldata - gas_used = env.last_result.gas_used - 1 - # note: EvmError catches both reverts and exceptional halts (oog). - # this is dependent on the target evm version. - with tx_failed(EvmError): - c.foo(calldata, gas=gas_used - 1) + + gas_used = env.last_result.gas_used + if version_check(begin="cancun"): + ctx = contextlib.nullcontext + else: + ctx = tx_failed + + with ctx(): + # depends on EVM version. pre-cancun, will revert due to checking + # success flag from identity precompile. + c.foo(calldata, gas=gas_used) def test_string_copy_oog2(env, get_contract, tx_failed): @@ -392,8 +398,14 @@ def foo(x: String[1000000]) -> uint256: c = get_contract(code) calldata = "a" * 1000000 assert c.foo(calldata) == len(calldata) - gas_used = env.last_result.gas_used - 1 - # note: EvmError catches both reverts and exceptional halts (oog). - # this is dependent on the target evm version. - with tx_failed(EvmError): - c.foo(calldata, gas=gas_used - 1) + + gas_used = env.last_result.gas_used + if version_check(begin="cancun"): + ctx = contextlib.nullcontext + else: + ctx = tx_failed + + with ctx(): + # depends on EVM version. pre-cancun, will revert due to checking + # success flag from identity precompile. + c.foo(calldata, gas=gas_used) From 298aa070195d4b81b2a4feeb7fceff1c2ad35859 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 Jan 2025 10:20:32 -0500 Subject: [PATCH 08/15] fix lint --- tests/functional/builtins/codegen/test_ecrecover.py | 1 - tests/functional/codegen/types/test_dynamic_array.py | 5 ++--- tests/functional/codegen/types/test_lists.py | 9 ++++----- tests/functional/codegen/types/test_string.py | 4 +++- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index 9adece00fc..7b08747347 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -1,7 +1,6 @@ from eth_account import Account from eth_account._utils.signing import to_bytes32 -from tests.evm_backends.base_env import EvmError from tests.utils import ZERO_ADDRESS diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index 948349043e..a532912044 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -1,13 +1,12 @@ -import itertools -from vyper.evm.opcodes import version_check import contextlib +import itertools from typing import Any, Callable import pytest -from tests.evm_backends.base_env import EvmError from tests.utils import decimal_to_int from vyper.compiler import compile_code +from vyper.evm.opcodes import version_check from vyper.exceptions import ( ArgumentException, ArrayIndexException, diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index 7d92cd9f22..f86ea9e733 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -1,12 +1,11 @@ -import itertools import contextlib -from vyper.evm.opcodes import version_check +import itertools import pytest -from tests.evm_backends.base_env import EvmError from tests.utils import decimal_to_int from vyper.compiler.settings import OptimizationLevel +from vyper.evm.opcodes import version_check from vyper.exceptions import ArrayIndexException, OverflowException, TypeMismatch @@ -877,7 +876,7 @@ def foo(x: uint256[3000]) -> uint256: # get the minimum gas for the contract complete execution gas_used = env.last_result.gas_used - if version_check(begin= "cancun"): + if version_check(begin="cancun"): ctx = contextlib.nullcontext else: ctx = tx_failed @@ -906,7 +905,7 @@ def foo(x: uint256[2500]) -> uint256: # get the minimum gas for the contract complete execution gas_used = env.last_result.gas_used - if version_check(begin= "cancun"): + if version_check(begin="cancun"): ctx = contextlib.nullcontext else: ctx = tx_failed diff --git a/tests/functional/codegen/types/test_string.py b/tests/functional/codegen/types/test_string.py index 2589e55bc9..c018fbd7ed 100644 --- a/tests/functional/codegen/types/test_string.py +++ b/tests/functional/codegen/types/test_string.py @@ -1,6 +1,8 @@ +import contextlib + import pytest -from tests.evm_backends.base_env import EvmError +from vyper.evm.opcodes import version_check def test_string_return(get_contract): From 08c286aeb73a7a9317b59b05bddf7feb2a46000e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 Jan 2025 10:28:02 -0500 Subject: [PATCH 09/15] fix poc --- tests/functional/codegen/types/test_string.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/codegen/types/test_string.py b/tests/functional/codegen/types/test_string.py index c018fbd7ed..47f74df93a 100644 --- a/tests/functional/codegen/types/test_string.py +++ b/tests/functional/codegen/types/test_string.py @@ -395,7 +395,8 @@ def test_string_copy_oog2(env, get_contract, tx_failed): @external @view def foo(x: String[1000000]) -> uint256: - return len(x) + y: String[1000000] = x + return len(y) """ c = get_contract(code) calldata = "a" * 1000000 From e1847bd05a8eebb6209d45a1001239d192ed7e0f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 Jan 2025 09:10:20 -0500 Subject: [PATCH 10/15] fix test case when optimization level is none --- .../builtins/codegen/test_ecrecover.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index 7b08747347..a068788a36 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -1,7 +1,10 @@ +import contextlib + from eth_account import Account from eth_account._utils.signing import to_bytes32 from tests.utils import ZERO_ADDRESS +from vyper.compiler.settings import OptimizationLevel def test_ecrecover_test(get_contract): @@ -88,7 +91,7 @@ def test_ecrecover() -> bool: assert c.test_ecrecover() is True -def test_ecrecover_oog_handling(env, get_contract, tx_failed): +def test_ecrecover_oog_handling(env, get_contract, tx_failed, optimize): # GHSA-vgf2-gvx8-xwc3 code = """ @external @@ -106,7 +109,18 @@ def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: assert c.do_ecrecover(h, v, r, s) == local_account.address gas_used = env.last_result.gas_used - with tx_failed(): + + if optimize == OptimizationLevel.NONE: + # if optimizations are off, enough gas is used by the contract + # that the gas provided to ecrecover (63/64ths rule) is enough + # for it to succeed + ctx = contextlib.nullcontext + else: + # in other cases, the gas forwarded is small enough for ecrecover + # to fail with oog, which we handle by reverting. + ctx = tx_failed + + with ctx(): # provide enough spare gas for the top-level call to not oog but # not enough for ecrecover to succeed c.do_ecrecover(h, v, r, s, gas=gas_used) From 7360419c9c12585b0b7d30f5ac6220563432f2dd Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 Jan 2025 09:42:23 -0500 Subject: [PATCH 11/15] fix another test case --- tests/functional/builtins/codegen/test_ecrecover.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index a068788a36..85d2756ffd 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -91,7 +91,7 @@ def test_ecrecover() -> bool: assert c.test_ecrecover() is True -def test_ecrecover_oog_handling(env, get_contract, tx_failed, optimize): +def test_ecrecover_oog_handling(env, get_contract, tx_failed, optimize, experimental_codegen): # GHSA-vgf2-gvx8-xwc3 code = """ @external @@ -110,7 +110,7 @@ def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: gas_used = env.last_result.gas_used - if optimize == OptimizationLevel.NONE: + if optimize == OptimizationLevel.NONE and not experimental_codegen: # if optimizations are off, enough gas is used by the contract # that the gas provided to ecrecover (63/64ths rule) is enough # for it to succeed From 4b999e60e36cd2cd78abc6b73579dd2679a4b984 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 Jan 2025 13:22:42 -0500 Subject: [PATCH 12/15] add IR checks --- .../builtins/codegen/test_ecrecover.py | 4 +++- .../codegen/types/test_dynamic_array.py | 5 ++++- tests/functional/codegen/types/test_lists.py | 6 +++++- tests/functional/codegen/types/test_string.py | 5 +++++ tests/utils.py | 16 ++++++++++++++++ 5 files changed, 33 insertions(+), 3 deletions(-) diff --git a/tests/functional/builtins/codegen/test_ecrecover.py b/tests/functional/builtins/codegen/test_ecrecover.py index 85d2756ffd..47a225068d 100644 --- a/tests/functional/builtins/codegen/test_ecrecover.py +++ b/tests/functional/builtins/codegen/test_ecrecover.py @@ -3,7 +3,7 @@ from eth_account import Account from eth_account._utils.signing import to_bytes32 -from tests.utils import ZERO_ADDRESS +from tests.utils import ZERO_ADDRESS, check_precompile_asserts from vyper.compiler.settings import OptimizationLevel @@ -99,6 +99,8 @@ def test_ecrecover_oog_handling(env, get_contract, tx_failed, optimize, experime def do_ecrecover(hash: bytes32, v: uint256, r:uint256, s:uint256) -> address: return ecrecover(hash, v, r, s) """ + check_precompile_asserts(code) + c = get_contract(code) h = b"\x35" * 32 diff --git a/tests/functional/codegen/types/test_dynamic_array.py b/tests/functional/codegen/types/test_dynamic_array.py index a532912044..3289b6a9dd 100644 --- a/tests/functional/codegen/types/test_dynamic_array.py +++ b/tests/functional/codegen/types/test_dynamic_array.py @@ -4,7 +4,7 @@ import pytest -from tests.utils import decimal_to_int +from tests.utils import check_precompile_asserts, decimal_to_int from vyper.compiler import compile_code from vyper.evm.opcodes import version_check from vyper.exceptions import ( @@ -1914,6 +1914,8 @@ def foo(a: DynArray[uint256, 4000]) -> uint256: b: DynArray[uint256, 4000] = a return b[0] """ + check_precompile_asserts(code) + c = get_contract(code) dynarray = [2] * 4000 assert c.foo(dynarray) == 2 @@ -1940,6 +1942,7 @@ def foo(x: String[1000000], y: String[1000000]) -> DynArray[String[1000000], 2]: # Some code return z """ + check_precompile_asserts(code) c = get_contract(code) calldata0 = "a" * 10 diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index f86ea9e733..8d0d8291e9 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -3,7 +3,7 @@ import pytest -from tests.utils import decimal_to_int +from tests.utils import check_precompile_asserts, decimal_to_int from vyper.compiler.settings import OptimizationLevel from vyper.evm.opcodes import version_check from vyper.exceptions import ArrayIndexException, OverflowException, TypeMismatch @@ -866,6 +866,8 @@ def foo(x: uint256[3000]) -> uint256: s: uint256[3000] = self.bar(x) return s[0] """ + check_precompile_asserts(code) + if optimize == OptimizationLevel.NONE and not experimental_codegen: # fails in get_contract due to code too large request.node.add_marker(pytest.mark.xfail(strict=True)) @@ -895,6 +897,8 @@ def foo(x: uint256[2500]) -> uint256: t: uint256[2500] = s return t[0] """ + check_precompile_asserts(code) + if optimize == OptimizationLevel.NONE and not experimental_codegen: # fails in get_contract due to code too large request.node.add_marker(pytest.mark.xfail(strict=True)) diff --git a/tests/functional/codegen/types/test_string.py b/tests/functional/codegen/types/test_string.py index 47f74df93a..b4e6919ea7 100644 --- a/tests/functional/codegen/types/test_string.py +++ b/tests/functional/codegen/types/test_string.py @@ -2,6 +2,7 @@ import pytest +from tests.utils import check_precompile_asserts from vyper.evm.opcodes import version_check @@ -373,6 +374,8 @@ def test_string_copy_oog(env, get_contract, tx_failed): def foo(x: String[1000000]) -> String[1000000]: return x """ + check_precompile_asserts(code) + c = get_contract(code) calldata = "a" * 1000000 assert c.foo(calldata) == calldata @@ -398,6 +401,8 @@ def foo(x: String[1000000]) -> uint256: y: String[1000000] = x return len(y) """ + check_precompile_asserts(code) + c = get_contract(code) calldata = "a" * 1000000 assert c.foo(calldata) == len(calldata) diff --git a/tests/utils.py b/tests/utils.py index 8548c4f47a..04a6b11311 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -3,6 +3,7 @@ import os from vyper import ast as vy_ast +from vyper.compiler.phases import CompilerData from vyper.semantics.analysis.constant_folding import constant_fold from vyper.utils import DECIMAL_EPSILON, round_towards_zero @@ -28,3 +29,18 @@ def parse_and_fold(source_code): def decimal_to_int(*args): s = decimal.Decimal(*args) return round_towards_zero(s / DECIMAL_EPSILON) + + +def check_precompile_asserts(source_code): + # check deploy IR (which contains runtime IR) + ir_node = CompilerData(source_code).ir_nodes + + def _check(ir_node, parent=None): + if ir_node.value == "staticcall": + precompile_addr = ir_node.args[1] + if isinstance(precompile_addr.value, int) and precompile_addr.value < 10: + assert parent is not None and parent.value == "assert" + for arg in ir_node.args: + _check(arg, ir_node) + + _check(ir_node) From a56f8af7ceee22c8750f6985387ff6ec9429a844 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 19 Jan 2025 10:27:04 -0500 Subject: [PATCH 13/15] track specific failures instead of general xfail --- tests/functional/codegen/types/test_lists.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/functional/codegen/types/test_lists.py b/tests/functional/codegen/types/test_lists.py index 8d0d8291e9..26cd16ed32 100644 --- a/tests/functional/codegen/types/test_lists.py +++ b/tests/functional/codegen/types/test_lists.py @@ -3,6 +3,7 @@ import pytest +from tests.evm_backends.base_env import EvmError from tests.utils import check_precompile_asserts, decimal_to_int from vyper.compiler.settings import OptimizationLevel from vyper.evm.opcodes import version_check @@ -869,8 +870,10 @@ def foo(x: uint256[3000]) -> uint256: check_precompile_asserts(code) if optimize == OptimizationLevel.NONE and not experimental_codegen: - # fails in get_contract due to code too large - request.node.add_marker(pytest.mark.xfail(strict=True)) + # fails in bytecode generation due to jumpdests too large + with pytest.raises(AssertionError): + get_contract(code) + return c = get_contract(code) array = [2] * 3000 @@ -900,8 +903,10 @@ def foo(x: uint256[2500]) -> uint256: check_precompile_asserts(code) if optimize == OptimizationLevel.NONE and not experimental_codegen: - # fails in get_contract due to code too large - request.node.add_marker(pytest.mark.xfail(strict=True)) + # fails in creating contract due to code too large + with tx_failed(EvmError): + get_contract(code) + return c = get_contract(code) array = [2] * 2500 From 99f9b2f992730c32b3e04197510853ece0899dfb Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 19 Jan 2025 13:50:54 -0500 Subject: [PATCH 14/15] add a comment --- tests/utils.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/utils.py b/tests/utils.py index 04a6b11311..70e943ff37 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -32,7 +32,10 @@ def decimal_to_int(*args): def check_precompile_asserts(source_code): - # check deploy IR (which contains runtime IR) + # common sanity check for some tests, that calls to precompiles + # are correctly wrapped in an assert. + + # check the deploy IR (which contains runtime IR) ir_node = CompilerData(source_code).ir_nodes def _check(ir_node, parent=None): From cdf0e6437a674bdcb4ea92a87b8716d64d3b96eb Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 19 Jan 2025 13:53:41 -0500 Subject: [PATCH 15/15] recurse on both deploy and runtime IR --- tests/utils.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index 70e943ff37..b9dc443c0d 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -35,8 +35,9 @@ def check_precompile_asserts(source_code): # common sanity check for some tests, that calls to precompiles # are correctly wrapped in an assert. - # check the deploy IR (which contains runtime IR) - ir_node = CompilerData(source_code).ir_nodes + compiler_data = CompilerData(source_code) + deploy_ir = compiler_data.ir_nodes + runtime_ir = compiler_data.ir_runtime def _check(ir_node, parent=None): if ir_node.value == "staticcall": @@ -46,4 +47,6 @@ def _check(ir_node, parent=None): for arg in ir_node.args: _check(arg, ir_node) - _check(ir_node) + _check(deploy_ir) + # technically runtime_ir is contained in deploy_ir, but check it anyways. + _check(runtime_ir)