From ee4b3cceeca1453c236623696e62c86bfb914208 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Tue, 19 Mar 2024 16:02:44 +0100 Subject: [PATCH 01/15] fix location check in slice to include tstorage --- vyper/builtins/functions.py | 6 +++--- vyper/evm/address_space.py | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index c1ac244b4b..14cd301275 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -367,7 +367,7 @@ def build_IR(self, expr, args, kwargs, context): # add 32 bytes to the buffer size bc word access might # be unaligned (see below) - if src.location == STORAGE: + if src.location.word_addressable: buflen += 32 # Get returntype string or bytes @@ -394,8 +394,8 @@ def build_IR(self, expr, args, kwargs, context): src_data = bytes_data_ptr(src) # general case. byte-for-byte copy - if src.location == STORAGE: - # because slice uses byte-addressing but storage + if src.location.word_addressable: + # because slice uses byte-addressing but storage/tstorage # is word-aligned, this algorithm starts at some number # of bytes before the data section starts, and might copy # an extra word. the pseudocode is: diff --git a/vyper/evm/address_space.py b/vyper/evm/address_space.py index fcbd4bcf63..08bef88e58 100644 --- a/vyper/evm/address_space.py +++ b/vyper/evm/address_space.py @@ -28,6 +28,10 @@ class AddrSpace: # TODO maybe make positional instead of defaulting to None store_op: Optional[str] = None + @property + def word_addressable(self) -> bool: + return self.word_scale == 1 + # alternative: # class Memory(AddrSpace): From 73a4a4b47521e54b39c17d8e7954fad8e17a318a Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Tue, 19 Mar 2024 16:58:42 +0100 Subject: [PATCH 02/15] fix location check in extract32 to include tstorage --- vyper/builtins/functions.py | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 14cd301275..9f67614943 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -36,7 +36,7 @@ from vyper.codegen.expr import Expr from vyper.codegen.ir_node import Encoding, scope_multi from vyper.codegen.keccak256_helper import keccak256_helper -from vyper.evm.address_space import MEMORY, STORAGE +from vyper.evm.address_space import MEMORY from vyper.exceptions import ( ArgumentException, CompilerPanic, @@ -828,19 +828,20 @@ class ECMul(_ECArith): _precompile = 0x7 -def _generic_element_getter(op): +def _generic_element_getter(loc): def f(index): + scale = loc.word_scale + # Indexing is done by words + # - add 'scale' to skip the length slot + # - for byte-addressable locations multiply by 32 + # - for word-addressable locations multiply by 1 which will be optimized out return IRnode.from_list( - [op, ["add", "_sub", ["add", 32, ["mul", 32, index]]]], typ=INT128_T + [loc.load_op, ["add", "_sub", ["add", scale, ["mul", scale, index]]]], typ=INT128_T ) return f -def _storage_element_getter(index): - return IRnode.from_list(["sload", ["add", "_sub", ["add", 1, index]]], typ=INT128_T) - - class Extract32(BuiltinFunctionT): _id = "extract32" _inputs = [("b", BytesT.any()), ("start", IntegerT.unsigneds())] @@ -875,18 +876,10 @@ def build_IR(self, expr, args, kwargs, context): sub, index = args ret_type = kwargs["output_type"] - # Get length and specific element - if sub.location == STORAGE: - lengetter = IRnode.from_list(["sload", "_sub"], typ=INT128_T) - elementgetter = _storage_element_getter - - else: - op = sub.location.load_op - lengetter = IRnode.from_list([op, "_sub"], typ=INT128_T) - elementgetter = _generic_element_getter(op) + lengetter = IRnode.from_list([sub.location.load_op, "_sub"], typ=INT128_T) + elementgetter = _generic_element_getter(sub.location) # TODO rewrite all this with cache_when_complex and bitshifts - # Special case: index known to be a multiple of 32 if isinstance(index.value, int) and not index.value % 32: o = IRnode.from_list( From f2447a742e02afe299d202131633c7897303ab35 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 21 Mar 2024 16:16:06 +0100 Subject: [PATCH 03/15] add tstorage tests for slice and extract32 --- .../builtins/codegen/test_extract32.py | 24 +++++++++++++------ .../functional/builtins/codegen/test_slice.py | 20 ++++++++++++---- tests/utils.py | 8 +++++++ 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/tests/functional/builtins/codegen/test_extract32.py b/tests/functional/builtins/codegen/test_extract32.py index a95b57b5ab..f3b4b2b1a4 100644 --- a/tests/functional/builtins/codegen/test_extract32.py +++ b/tests/functional/builtins/codegen/test_extract32.py @@ -1,6 +1,20 @@ -def test_extract32_extraction(tx_failed, get_contract_with_gas_estimation): - extract32_code = """ -y: Bytes[100] +import pytest + +from tests.utils import wrap_typ_with_storage_loc + +from vyper.evm.opcodes import version_check + + +@pytest.mark.parametrize('location', ["storage", "transient"]) +def test_extract32_extraction( + tx_failed, + get_contract_with_gas_estimation, + location +): + if location == "transient" and not version_check(begin="cancun"): + pytest.skip("Skipping test as storage_location is 'transient' and EVM version is pre-Cancun") + extract32_code = f""" +y: {wrap_typ_with_storage_loc("Bytes[100]", location)} @external def extrakt32(inp: Bytes[100], index: uint256) -> bytes32: return extract32(inp, index) @@ -43,8 +57,6 @@ def extrakt32_storage(index: uint256, inp: Bytes[100]) -> bytes32: with tx_failed(): c.extrakt32(S, i) - print("Passed bytes32 extraction test") - def test_extract32_code(tx_failed, get_contract_with_gas_estimation): extract32_code = """ @@ -84,5 +96,3 @@ def foq(inp: Bytes[32]) -> address: with tx_failed(): c.foq(b"crow" * 8) - - print("Passed extract32 test") diff --git a/tests/functional/builtins/codegen/test_slice.py b/tests/functional/builtins/codegen/test_slice.py index 9fc464ed35..6929b24cd7 100644 --- a/tests/functional/builtins/codegen/test_slice.py +++ b/tests/functional/builtins/codegen/test_slice.py @@ -2,9 +2,12 @@ import pytest from hypothesis import given, settings +from tests.utils import wrap_typ_with_storage_loc + from vyper.compiler import compile_code from vyper.compiler.settings import OptimizationLevel, Settings from vyper.exceptions import ArgumentException, TypeMismatch +from vyper.evm.opcodes import version_check _fun_bytes32_bounds = [(0, 32), (3, 29), (27, 5), (0, 5), (5, 3), (30, 2)] @@ -93,7 +96,7 @@ def _get_contract(): assert c.do_splice() == bytesdata[start : start + length] -@pytest.mark.parametrize("location", ("storage", "calldata", "memory", "literal", "code")) +@pytest.mark.parametrize("location", ["storage", "transient", "calldata", "memory", "literal", "code"]) @pytest.mark.parametrize("use_literal_start", (True, False)) @pytest.mark.parametrize("use_literal_length", (True, False)) @pytest.mark.parametrize("opt_level", list(OptimizationLevel)) @@ -112,10 +115,18 @@ def test_slice_bytes_fuzz( use_literal_length, length_bound, ): + if location == "transient" and not version_check(begin="cancun"): + pytest.skip("Skipping test as storage_location is 'transient' and EVM version is pre-Cancun") preamble = "" if location == "memory": spliced_code = f"foo: Bytes[{length_bound}] = inp" foo = "foo" + elif location == "transient" or location == "storage": + preamble = f""" +foo: {wrap_typ_with_storage_loc(f"Bytes[{length_bound}]", location)} + """ + spliced_code = "self.foo = inp" + foo = "self.foo" elif location == "storage": preamble = f""" foo: Bytes[{length_bound}] @@ -194,10 +205,11 @@ def _get_contract(): assert c.do_slice(bytesdata, start, length) == bytesdata[start:end], code -def test_slice_private(get_contract): +@pytest.mark.parametrize("location", ["storage"] if not version_check(begin="cancun") else ["storage", "transient"]) +def test_slice_private(get_contract, location): # test there are no buffer overruns in the slice function - code = """ -bytez: public(String[12]) + code = f""" +bytez: public({wrap_typ_with_storage_loc("String[12]", location)}) @internal def _slice(start: uint256, length: uint256): diff --git a/tests/utils.py b/tests/utils.py index 25dad818ca..1dff6354f3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -19,3 +19,11 @@ def parse_and_fold(source_code): ast = vy_ast.parse_to_ast(source_code) constant_fold(ast) return ast + + +def wrap_typ_with_storage_loc(typ, loc): + if loc == "storage": + return typ + elif loc == "transient": + return f"transient({typ})" + assert False, f"unreachable storage location {loc}" From 477312f13f87534ef5a71dc1a5ae17e26ad8fef0 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 21 Mar 2024 16:16:25 +0100 Subject: [PATCH 04/15] fix grammar to support tstorage --- vyper/ast/grammar.lark | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vyper/ast/grammar.lark b/vyper/ast/grammar.lark index 1c318a76a5..3feb4df92f 100644 --- a/vyper/ast/grammar.lark +++ b/vyper/ast/grammar.lark @@ -15,6 +15,7 @@ module: ( DOCSTRING | event_def | function_def | immutable_def + | transient_def | exports_decl | _NEWLINE )* @@ -47,9 +48,13 @@ constant_def: (constant_private | constant_with_getter) "=" expr immutable: "immutable" "(" type ")" immutable_def: NAME ":" immutable +// transient definitions +transient: "transient" "(" type ")" +transient_def: NAME ":" transient + variable: NAME ":" type // NOTE: Temporary until decorators used -variable_with_getter: NAME ":" "public" "(" (type | immutable) ")" +variable_with_getter: NAME ":" "public" "(" (type | immutable | transient) ")" variable_def: variable | variable_with_getter // A decorator "wraps" a method, modifying it's context. From b1b22237a192f257df93964fc2e42f426b337b33 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 21 Mar 2024 20:20:53 +0100 Subject: [PATCH 05/15] wip: refactor extract32 to use cache when complex --- vyper/builtins/functions.py | 59 +++++++++++++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 9f67614943..02922a90cf 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -836,7 +836,8 @@ def f(index): # - for byte-addressable locations multiply by 32 # - for word-addressable locations multiply by 1 which will be optimized out return IRnode.from_list( - [loc.load_op, ["add", "_sub", ["add", scale, ["mul", scale, index]]]], typ=INT128_T + [loc.load_op, ["add", "_sub", ["add", scale, ["mul", scale, index]]]] + #[loc.load_op, add_ofst("_sub", ["add", scale, ["mul", scale, index]])] ) return f @@ -876,12 +877,17 @@ def build_IR(self, expr, args, kwargs, context): sub, index = args ret_type = kwargs["output_type"] + + scale = sub.location.word_scale + load_op = sub.location.load_op + lengetter = IRnode.from_list([sub.location.load_op, "_sub"], typ=INT128_T) elementgetter = _generic_element_getter(sub.location) # TODO rewrite all this with cache_when_complex and bitshifts # Special case: index known to be a multiple of 32 if isinstance(index.value, int) and not index.value % 32: + """ o = IRnode.from_list( [ "with", @@ -894,8 +900,46 @@ def build_IR(self, expr, args, kwargs, context): typ=ret_type, annotation="extracting 32 bytes", ) + """ + with sub.cache_when_complex("_sub") as (b1, sub): + length = get_bytearray_length(sub) + idx = ["div", clamp2(0, index, ["sub", length, 32], signed=True), 32] + ret = [load_op, ["add", sub, ["add", scale, ["mul", scale, idx]]]] + o = IRnode.from_list(b1.resolve(ret), typ=ret_type, annotation="extracting 32 bytes") + return IRnode.from_list(clamp_basetype(o), typ=ret_type) + # General case - else: + with sub.cache_when_complex("_sub") as (b1, sub): + length = get_bytearray_length(sub) + with index.cache_when_complex("_index") as (b2, index): + idx = clamp2(0, index, ["sub", length, 32], signed=True) + mi32 = IRnode.from_list(["mod", idx, 32]) + di32 = IRnode.from_list(["div", idx, 32]) + + with mi32.cache_when_complex("_mi32") as ( + b3, mi32 + ), di32.cache_when_complex("_di32") as (b4, di32): + ret = [ + "if", + mi32, + [ + "add", + [ + "mul", + [load_op, ["add", sub, ["add", scale, ["mul", scale, di32]]]], + ["exp", 256, mi32]], + [ + "div", + [load_op, ["add", sub, (["add", scale, ["mul", scale, ["add", di32, 1]]])]], + ["exp", 256, ["sub", 32, mi32]], + ], + ], + [load_op, ["add", sub, ["add", scale, ["mul", scale, di32]]]], + ] + o = IRnode.from_list(b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret)))), typ=ret_type, annotation="extracting 32 bytes") + return IRnode.from_list(clamp_basetype(o), typ=ret_type) + + """ o = IRnode.from_list( [ "with", @@ -922,14 +966,17 @@ def build_IR(self, expr, args, kwargs, context): "_mi32", [ "add", - ["mul", elementgetter("_di32"), ["exp", 256, "_mi32"]], + [ + "mul", + [load_op, ["add", "_sub", ["add", scale, ["mul", scale, "_di32"]]]], + ["exp", 256, "_mi32"]], [ "div", - elementgetter(["add", "_di32", 1]), + [load_op, ["add", "_sub", ["add", scale, ["mul", scale, ["add", "_di32", 1]]]]], ["exp", 256, ["sub", 32, "_mi32"]], ], ], - elementgetter("_di32"), + [load_op, ["add", "_sub", ["add", scale, ["mul", scale, "_di32"]]]], ], ], ], @@ -939,7 +986,7 @@ def build_IR(self, expr, args, kwargs, context): typ=ret_type, annotation="extract32", ) - return IRnode.from_list(clamp_basetype(o), typ=ret_type) + """ class AsWeiValue(BuiltinFunctionT): From bb228e5943d293ce12e8f322834ad8546ed81086 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 21 Mar 2024 20:35:07 +0100 Subject: [PATCH 06/15] divide up the body of extract32 to chunks --- vyper/builtins/functions.py | 94 +++++-------------------------------- 1 file changed, 13 insertions(+), 81 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 02922a90cf..2dac618b13 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -877,30 +877,12 @@ def build_IR(self, expr, args, kwargs, context): sub, index = args ret_type = kwargs["output_type"] - scale = sub.location.word_scale load_op = sub.location.load_op - lengetter = IRnode.from_list([sub.location.load_op, "_sub"], typ=INT128_T) - elementgetter = _generic_element_getter(sub.location) - - # TODO rewrite all this with cache_when_complex and bitshifts + # TODO rewrite all this with bitshifts # Special case: index known to be a multiple of 32 if isinstance(index.value, int) and not index.value % 32: - """ - o = IRnode.from_list( - [ - "with", - "_sub", - sub, - elementgetter( - ["div", clamp2(0, index, ["sub", lengetter, 32], signed=True), 32] - ), - ], - typ=ret_type, - annotation="extracting 32 bytes", - ) - """ with sub.cache_when_complex("_sub") as (b1, sub): length = get_bytearray_length(sub) idx = ["div", clamp2(0, index, ["sub", length, 32], signed=True), 32] @@ -919,75 +901,25 @@ def build_IR(self, expr, args, kwargs, context): with mi32.cache_when_complex("_mi32") as ( b3, mi32 ), di32.cache_when_complex("_di32") as (b4, di32): + left_bytes = [ + "mul", + [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])], + ["exp", 256, mi32] + ] + right_bytes = [ + "div", + [load_op, add_ofst(sub, ["add", scale, ["mul", scale, ["add", di32, 1]]])], + ["exp", 256, ["sub", 32, mi32]], + ] ret = [ "if", mi32, - [ - "add", - [ - "mul", - [load_op, ["add", sub, ["add", scale, ["mul", scale, di32]]]], - ["exp", 256, mi32]], - [ - "div", - [load_op, ["add", sub, (["add", scale, ["mul", scale, ["add", di32, 1]]])]], - ["exp", 256, ["sub", 32, mi32]], - ], - ], - [load_op, ["add", sub, ["add", scale, ["mul", scale, di32]]]], + ["add", left_bytes, right_bytes], + [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])], ] o = IRnode.from_list(b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret)))), typ=ret_type, annotation="extracting 32 bytes") return IRnode.from_list(clamp_basetype(o), typ=ret_type) - """ - o = IRnode.from_list( - [ - "with", - "_sub", - sub, - [ - "with", - "_len", - lengetter, - [ - "with", - "_index", - clamp2(0, index, ["sub", "_len", 32], signed=True), - [ - "with", - "_mi32", - ["mod", "_index", 32], - [ - "with", - "_di32", - ["div", "_index", 32], - [ - "if", - "_mi32", - [ - "add", - [ - "mul", - [load_op, ["add", "_sub", ["add", scale, ["mul", scale, "_di32"]]]], - ["exp", 256, "_mi32"]], - [ - "div", - [load_op, ["add", "_sub", ["add", scale, ["mul", scale, ["add", "_di32", 1]]]]], - ["exp", 256, ["sub", 32, "_mi32"]], - ], - ], - [load_op, ["add", "_sub", ["add", scale, ["mul", scale, "_di32"]]]], - ], - ], - ], - ], - ], - ], - typ=ret_type, - annotation="extract32", - ) - """ - class AsWeiValue(BuiltinFunctionT): _id = "as_wei_value" From 87ca36a8cee9c3ef093f7106e3990b4adcf2a81c Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 21 Mar 2024 20:50:54 +0100 Subject: [PATCH 07/15] refactor extract32 using bitshifts --- vyper/builtins/functions.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 2dac618b13..ebcdbcdac8 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -901,16 +901,13 @@ def build_IR(self, expr, args, kwargs, context): with mi32.cache_when_complex("_mi32") as ( b3, mi32 ), di32.cache_when_complex("_di32") as (b4, di32): - left_bytes = [ - "mul", - [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])], - ["exp", 256, mi32] - ] - right_bytes = [ - "div", - [load_op, add_ofst(sub, ["add", scale, ["mul", scale, ["add", di32, 1]]])], - ["exp", 256, ["sub", 32, mi32]], - ] + + left_payload = [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])] + left_bytes = shl(["mul", 8, mi32], left_payload) + + right_payload = [load_op, add_ofst(sub, ["add", scale, ["mul", scale, ["add", di32, 1]]])] + right_bytes = shr(["mul", 8, ["sub", 32, mi32]], right_payload) + ret = [ "if", mi32, @@ -918,6 +915,7 @@ def build_IR(self, expr, args, kwargs, context): [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])], ] o = IRnode.from_list(b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret)))), typ=ret_type, annotation="extracting 32 bytes") + return IRnode.from_list(clamp_basetype(o), typ=ret_type) From b5af4d093bcb9a1ca160b8efce8c31d75cb63817 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 26 Mar 2024 20:07:26 -0400 Subject: [PATCH 08/15] fix lint --- .../builtins/codegen/test_extract32.py | 13 ++++------ .../functional/builtins/codegen/test_slice.py | 15 ++++++++---- tests/utils.py | 2 +- vyper/builtins/functions.py | 24 ++++++++++++------- 4 files changed, 32 insertions(+), 22 deletions(-) diff --git a/tests/functional/builtins/codegen/test_extract32.py b/tests/functional/builtins/codegen/test_extract32.py index f3b4b2b1a4..8d8c725d38 100644 --- a/tests/functional/builtins/codegen/test_extract32.py +++ b/tests/functional/builtins/codegen/test_extract32.py @@ -1,18 +1,15 @@ import pytest from tests.utils import wrap_typ_with_storage_loc - from vyper.evm.opcodes import version_check -@pytest.mark.parametrize('location', ["storage", "transient"]) -def test_extract32_extraction( - tx_failed, - get_contract_with_gas_estimation, - location -): +@pytest.mark.parametrize("location", ["storage", "transient"]) +def test_extract32_extraction(tx_failed, get_contract_with_gas_estimation, location): if location == "transient" and not version_check(begin="cancun"): - pytest.skip("Skipping test as storage_location is 'transient' and EVM version is pre-Cancun") + pytest.skip( + "Skipping test as storage_location is 'transient' and EVM version is pre-Cancun" + ) extract32_code = f""" y: {wrap_typ_with_storage_loc("Bytes[100]", location)} @external diff --git a/tests/functional/builtins/codegen/test_slice.py b/tests/functional/builtins/codegen/test_slice.py index 6929b24cd7..91f3694361 100644 --- a/tests/functional/builtins/codegen/test_slice.py +++ b/tests/functional/builtins/codegen/test_slice.py @@ -3,11 +3,10 @@ from hypothesis import given, settings from tests.utils import wrap_typ_with_storage_loc - from vyper.compiler import compile_code from vyper.compiler.settings import OptimizationLevel, Settings -from vyper.exceptions import ArgumentException, TypeMismatch from vyper.evm.opcodes import version_check +from vyper.exceptions import ArgumentException, TypeMismatch _fun_bytes32_bounds = [(0, 32), (3, 29), (27, 5), (0, 5), (5, 3), (30, 2)] @@ -96,7 +95,9 @@ def _get_contract(): assert c.do_splice() == bytesdata[start : start + length] -@pytest.mark.parametrize("location", ["storage", "transient", "calldata", "memory", "literal", "code"]) +@pytest.mark.parametrize( + "location", ["storage", "transient", "calldata", "memory", "literal", "code"] +) @pytest.mark.parametrize("use_literal_start", (True, False)) @pytest.mark.parametrize("use_literal_length", (True, False)) @pytest.mark.parametrize("opt_level", list(OptimizationLevel)) @@ -116,7 +117,9 @@ def test_slice_bytes_fuzz( length_bound, ): if location == "transient" and not version_check(begin="cancun"): - pytest.skip("Skipping test as storage_location is 'transient' and EVM version is pre-Cancun") + pytest.skip( + "Skipping test as storage_location is 'transient' and EVM version is pre-Cancun" + ) preamble = "" if location == "memory": spliced_code = f"foo: Bytes[{length_bound}] = inp" @@ -205,7 +208,9 @@ def _get_contract(): assert c.do_slice(bytesdata, start, length) == bytesdata[start:end], code -@pytest.mark.parametrize("location", ["storage"] if not version_check(begin="cancun") else ["storage", "transient"]) +@pytest.mark.parametrize( + "location", ["storage"] if not version_check(begin="cancun") else ["storage", "transient"] +) def test_slice_private(get_contract, location): # test there are no buffer overruns in the slice function code = f""" diff --git a/tests/utils.py b/tests/utils.py index 1dff6354f3..522bc23577 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -26,4 +26,4 @@ def wrap_typ_with_storage_loc(typ, loc): return typ elif loc == "transient": return f"transient({typ})" - assert False, f"unreachable storage location {loc}" + raise AssertionError(f"unreachable storage location {loc}") # pragma: nocover diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index ebcdbcdac8..a5435a9001 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -837,7 +837,7 @@ def f(index): # - for word-addressable locations multiply by 1 which will be optimized out return IRnode.from_list( [loc.load_op, ["add", "_sub", ["add", scale, ["mul", scale, index]]]] - #[loc.load_op, add_ofst("_sub", ["add", scale, ["mul", scale, index]])] + # [loc.load_op, add_ofst("_sub", ["add", scale, ["mul", scale, index]])] ) return f @@ -887,7 +887,9 @@ def build_IR(self, expr, args, kwargs, context): length = get_bytearray_length(sub) idx = ["div", clamp2(0, index, ["sub", length, 32], signed=True), 32] ret = [load_op, ["add", sub, ["add", scale, ["mul", scale, idx]]]] - o = IRnode.from_list(b1.resolve(ret), typ=ret_type, annotation="extracting 32 bytes") + o = IRnode.from_list( + b1.resolve(ret), typ=ret_type, annotation="extracting 32 bytes" + ) return IRnode.from_list(clamp_basetype(o), typ=ret_type) # General case @@ -898,14 +900,16 @@ def build_IR(self, expr, args, kwargs, context): mi32 = IRnode.from_list(["mod", idx, 32]) di32 = IRnode.from_list(["div", idx, 32]) - with mi32.cache_when_complex("_mi32") as ( - b3, mi32 - ), di32.cache_when_complex("_di32") as (b4, di32): - + with mi32.cache_when_complex("_mi32") as (b3, mi32), di32.cache_when_complex( + "_di32" + ) as (b4, di32): left_payload = [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])] left_bytes = shl(["mul", 8, mi32], left_payload) - right_payload = [load_op, add_ofst(sub, ["add", scale, ["mul", scale, ["add", di32, 1]]])] + right_payload = [ + load_op, + add_ofst(sub, ["add", scale, ["mul", scale, ["add", di32, 1]]]), + ] right_bytes = shr(["mul", 8, ["sub", 32, mi32]], right_payload) ret = [ @@ -914,7 +918,11 @@ def build_IR(self, expr, args, kwargs, context): ["add", left_bytes, right_bytes], [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])], ] - o = IRnode.from_list(b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret)))), typ=ret_type, annotation="extracting 32 bytes") + o = IRnode.from_list( + b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret)))), + typ=ret_type, + annotation="extracting 32 bytes", + ) return IRnode.from_list(clamp_basetype(o), typ=ret_type) From 48db56c8011af4411004834781172d9c6009fdcf Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 26 Mar 2024 20:32:01 -0400 Subject: [PATCH 09/15] simplify logic for extract32 --- vyper/builtins/functions.py | 85 +++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 814a7da322..efb8a30c04 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -8,6 +8,7 @@ from vyper.codegen.abi_encoder import abi_encode from vyper.codegen.context import Context, VariableRecord from vyper.codegen.core import ( + LOAD, STORE, IRnode, add_ofst, @@ -884,57 +885,47 @@ def infer_kwarg_types(self, node): @process_inputs def build_IR(self, expr, args, kwargs, context): - sub, index = args + bytez, index = args ret_type = kwargs["output_type"] - scale = sub.location.word_scale - load_op = sub.location.load_op - - # TODO rewrite all this with bitshifts - # Special case: index known to be a multiple of 32 - if isinstance(index.value, int) and not index.value % 32: - with sub.cache_when_complex("_sub") as (b1, sub): - length = get_bytearray_length(sub) - idx = ["div", clamp2(0, index, ["sub", length, 32], signed=True), 32] - ret = [load_op, ["add", sub, ["add", scale, ["mul", scale, idx]]]] - o = IRnode.from_list( - b1.resolve(ret), typ=ret_type, annotation="extracting 32 bytes" - ) - return IRnode.from_list(clamp_basetype(o), typ=ret_type) + def finalize(ret): + annotation = "extract32" + ret = IRnode.from_list(ret, typ=ret_type, annotation=annotation) + return clamp_basetype(ret) - # General case - with sub.cache_when_complex("_sub") as (b1, sub): - length = get_bytearray_length(sub) + with bytez.cache_when_complex("_sub") as (b1, bytez): + # merge + length = get_bytearray_length(bytez) + index = clamp("lt", index, ["sub", length, 32]) with index.cache_when_complex("_index") as (b2, index): - idx = clamp2(0, index, ["sub", length, 32], signed=True) - mi32 = IRnode.from_list(["mod", idx, 32]) - di32 = IRnode.from_list(["div", idx, 32]) - - with mi32.cache_when_complex("_mi32") as (b3, mi32), di32.cache_when_complex( - "_di32" - ) as (b4, di32): - left_payload = [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])] - left_bytes = shl(["mul", 8, mi32], left_payload) - - right_payload = [ - load_op, - add_ofst(sub, ["add", scale, ["mul", scale, ["add", di32, 1]]]), - ] - right_bytes = shr(["mul", 8, ["sub", 32, mi32]], right_payload) - - ret = [ - "if", - mi32, - ["add", left_bytes, right_bytes], - [load_op, add_ofst(sub, ["add", scale, ["mul", scale, di32]])], - ] - o = IRnode.from_list( - b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret)))), - typ=ret_type, - annotation="extracting 32 bytes", - ) - - return IRnode.from_list(clamp_basetype(o), typ=ret_type) + assert not index.typ.is_signed + + # "easy" case, byte- addressed locations: + if bytez.location.word_scale == 32: + word = LOAD(add_ofst(bytes_data_ptr(bytez), index)) + return finalize(b1.resolve(b2.resolve(word))) + + # storage and transient storage, word-addressed + assert bytez.location.word_scale == 1 + + slot = IRnode.from_list(["div", index, 32]) + # byte offset within the slot + byte_ofst = IRnode.from_list(["mod", index, 32]) + + with byte_ofst.cache_when_complex("byte_ofst") as ( + b3, + byte_ofst, + ), slot.cache_when_complex("slot") as (b4, slot): + # perform two loads and merge + w1 = LOAD(add_ofst(bytes_data_ptr(bytez), slot)) + w2 = LOAD(add_ofst(bytes_data_ptr(bytez), ["add", slot, 1])) + + left_bytes = shl(["mul", 8, byte_ofst], w1) + right_bytes = shr(["mul", 8, ["sub", 32, byte_ofst]], w2) + merged = ["or", left_bytes, right_bytes] + + ret = ["if", byte_ofst, merged, left_bytes] + return finalize(b1.resolve(b2.resolve(b3.resolve(b4.resolve(ret))))) class AsWeiValue(BuiltinFunctionT): From ebb84db3416edb85a5a37efeb57a560de1a00f70 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 26 Mar 2024 20:32:42 -0400 Subject: [PATCH 10/15] remove dead function --- vyper/builtins/functions.py | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index efb8a30c04..1e6192f025 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -839,21 +839,6 @@ class ECMul(_ECArith): _precompile = 0x7 -def _generic_element_getter(loc): - def f(index): - scale = loc.word_scale - # Indexing is done by words - # - add 'scale' to skip the length slot - # - for byte-addressable locations multiply by 32 - # - for word-addressable locations multiply by 1 which will be optimized out - return IRnode.from_list( - [loc.load_op, ["add", "_sub", ["add", scale, ["mul", scale, index]]]] - # [loc.load_op, add_ofst("_sub", ["add", scale, ["mul", scale, index]])] - ) - - return f - - class Extract32(BuiltinFunctionT): _id = "extract32" _inputs = [("b", BytesT.any()), ("start", IntegerT.unsigneds())] From 17aee50644e7005f37f790c5597b3146713bd2a4 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 4 Apr 2024 14:38:46 +0200 Subject: [PATCH 11/15] fix index clamping in extract32 --- vyper/builtins/functions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/builtins/functions.py b/vyper/builtins/functions.py index 1e6192f025..05d6dcb8b3 100644 --- a/vyper/builtins/functions.py +++ b/vyper/builtins/functions.py @@ -881,7 +881,7 @@ def finalize(ret): with bytez.cache_when_complex("_sub") as (b1, bytez): # merge length = get_bytearray_length(bytez) - index = clamp("lt", index, ["sub", length, 32]) + index = clamp2(0, index, ["sub", length, 32], signed=True) with index.cache_when_complex("_index") as (b2, index): assert not index.typ.is_signed From d22c5cd5df09672ae2a202372f7b55ca0bbd3c4a Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Thu, 4 Apr 2024 17:26:45 +0200 Subject: [PATCH 12/15] remove wrap_typ util and replace w if-else chain --- .../builtins/codegen/test_extract32.py | 7 ++++-- .../functional/builtins/codegen/test_slice.py | 23 +++++++++++++------ tests/utils.py | 8 ------- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/tests/functional/builtins/codegen/test_extract32.py b/tests/functional/builtins/codegen/test_extract32.py index 8d8c725d38..d5e9db0a6e 100644 --- a/tests/functional/builtins/codegen/test_extract32.py +++ b/tests/functional/builtins/codegen/test_extract32.py @@ -1,6 +1,5 @@ import pytest -from tests.utils import wrap_typ_with_storage_loc from vyper.evm.opcodes import version_check @@ -10,8 +9,12 @@ def test_extract32_extraction(tx_failed, get_contract_with_gas_estimation, locat pytest.skip( "Skipping test as storage_location is 'transient' and EVM version is pre-Cancun" ) + if location == "storage": + decl = "y: Bytes[100]" + else: + decl = "y: transient(Bytes[100])" extract32_code = f""" -y: {wrap_typ_with_storage_loc("Bytes[100]", location)} +{decl} @external def extrakt32(inp: Bytes[100], index: uint256) -> bytes32: return extract32(inp, index) diff --git a/tests/functional/builtins/codegen/test_slice.py b/tests/functional/builtins/codegen/test_slice.py index 9d5e233f8b..088f1a7627 100644 --- a/tests/functional/builtins/codegen/test_slice.py +++ b/tests/functional/builtins/codegen/test_slice.py @@ -2,7 +2,6 @@ import pytest from hypothesis import given, settings -from tests.utils import wrap_typ_with_storage_loc from vyper.compiler import compile_code from vyper.compiler.settings import OptimizationLevel, Settings from vyper.evm.opcodes import version_check @@ -124,15 +123,15 @@ def test_slice_bytes_fuzz( if location == "memory": spliced_code = f"foo: Bytes[{length_bound}] = inp" foo = "foo" - elif location == "transient" or location == "storage": + elif location == "storage": preamble = f""" -foo: {wrap_typ_with_storage_loc(f"Bytes[{length_bound}]", location)} +foo: Bytes[{length_bound}] """ spliced_code = "self.foo = inp" foo = "self.foo" - elif location == "storage": + elif location == "transient": preamble = f""" -foo: Bytes[{length_bound}] +foo: transient(Bytes[{length_bound}]) """ spliced_code = "self.foo = inp" foo = "self.foo" @@ -209,12 +208,22 @@ def _get_contract(): @pytest.mark.parametrize( - "location", ["storage"] if not version_check(begin="cancun") else ["storage", "transient"] + "location", ["storage", "transient"] ) def test_slice_private(get_contract, location): + if location == "transient" and not version_check(begin="cancun"): + pytest.skip( + "Skipping test as storage_location is 'transient' and EVM version is pre-Cancun" + ) + # test there are no buffer overruns in the slice function + if location == "storage": + decl = "bytez: public(String[12])" + else: + decl = "bytez: public(transient(String[12]))" + code = f""" -bytez: public({wrap_typ_with_storage_loc("String[12]", location)}) +{decl} @internal def _slice(start: uint256, length: uint256): diff --git a/tests/utils.py b/tests/utils.py index 522bc23577..25dad818ca 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -19,11 +19,3 @@ def parse_and_fold(source_code): ast = vy_ast.parse_to_ast(source_code) constant_fold(ast) return ast - - -def wrap_typ_with_storage_loc(typ, loc): - if loc == "storage": - return typ - elif loc == "transient": - return f"transient({typ})" - raise AssertionError(f"unreachable storage location {loc}") # pragma: nocover From 1a1567fc9d0622305a5834057f24182375115417 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 4 Apr 2024 12:12:48 -0400 Subject: [PATCH 13/15] fix lint --- tests/functional/builtins/codegen/test_slice.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/functional/builtins/codegen/test_slice.py b/tests/functional/builtins/codegen/test_slice.py index 088f1a7627..01421349bc 100644 --- a/tests/functional/builtins/codegen/test_slice.py +++ b/tests/functional/builtins/codegen/test_slice.py @@ -207,9 +207,7 @@ def _get_contract(): assert c.do_slice(bytesdata, start, length) == bytesdata[start:end], code -@pytest.mark.parametrize( - "location", ["storage", "transient"] -) +@pytest.mark.parametrize("location", ["storage", "transient"]) def test_slice_private(get_contract, location): if location == "transient" and not version_check(begin="cancun"): pytest.skip( From 277197085edaae6af9ab4b56bd8e1b73de7552de Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 4 Apr 2024 12:17:31 -0400 Subject: [PATCH 14/15] nit: style --- tests/functional/builtins/codegen/test_slice.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/builtins/codegen/test_slice.py b/tests/functional/builtins/codegen/test_slice.py index 01421349bc..03dc7cc56d 100644 --- a/tests/functional/builtins/codegen/test_slice.py +++ b/tests/functional/builtins/codegen/test_slice.py @@ -217,8 +217,10 @@ def test_slice_private(get_contract, location): # test there are no buffer overruns in the slice function if location == "storage": decl = "bytez: public(String[12])" - else: + elif location == "transient": decl = "bytez: public(transient(String[12]))" + else: + raise Exception("unreachable") code = f""" {decl} From af0e811699214f6148b9525abd720ba77b87bfee Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 4 Apr 2024 12:18:24 -0400 Subject: [PATCH 15/15] fix another nit --- tests/functional/builtins/codegen/test_extract32.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/functional/builtins/codegen/test_extract32.py b/tests/functional/builtins/codegen/test_extract32.py index d5e9db0a6e..96280ce862 100644 --- a/tests/functional/builtins/codegen/test_extract32.py +++ b/tests/functional/builtins/codegen/test_extract32.py @@ -11,8 +11,10 @@ def test_extract32_extraction(tx_failed, get_contract_with_gas_estimation, locat ) if location == "storage": decl = "y: Bytes[100]" - else: + elif location == "transient": decl = "y: transient(Bytes[100])" + else: + raise Exception("unreachable") extract32_code = f""" {decl} @external