From 2d298066be112aa95043c2c9ad7800172d1a1b09 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Mon, 13 May 2024 08:32:54 +0200 Subject: [PATCH 01/14] fix hardcoded storage references --- vyper/codegen/core.py | 4 +-- vyper/codegen/stmt.py | 6 ++-- vyper/semantics/analysis/module.py | 38 +++++++++++++------------- vyper/semantics/types/function.py | 9 ++++-- vyper/semantics/types/subscriptable.py | 2 +- 5 files changed, 32 insertions(+), 27 deletions(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index f834f1da55..ae19b3aa93 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -150,7 +150,7 @@ def make_byte_array_copier(dst, src): return STORE(dst, 0) with src.cache_when_complex("src") as (b1, src): - has_storage = STORAGE in (src.location, dst.location) + has_storage = any(loc.word_addressable for loc in (src.location, dst.location)) is_memory_copy = dst.location == src.location == MEMORY batch_uses_identity = is_memory_copy and not version_check(begin="cancun") if src.typ.maxlen <= 32 and (has_storage or batch_uses_identity): @@ -934,7 +934,7 @@ def _complex_make_setter(left, right): assert left.encoding == Encoding.VYPER len_ = left.typ.memory_bytes_required - has_storage = STORAGE in (left.location, right.location) + has_storage = any(loc.word_addressable for loc in (left.location, right.location)) if has_storage: if _opt_codesize(): # assuming PUSH2, a single sstore(dst (sload src)) is 8 bytes, diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py index 1018a6ec43..75a528f0b1 100644 --- a/vyper/codegen/stmt.py +++ b/vyper/codegen/stmt.py @@ -17,7 +17,7 @@ ) from vyper.codegen.expr import Expr from vyper.codegen.return_ import make_return_stmt -from vyper.evm.address_space import MEMORY, STORAGE +from vyper.evm.address_space import MEMORY from vyper.exceptions import CodegenPanic, StructureException, TypeCheckFailure, tag_exceptions from vyper.semantics.types import DArrayT from vyper.semantics.types.shortcuts import UINT256_T @@ -315,12 +315,12 @@ def _get_target(self, target): if isinstance(target, vy_ast.Tuple): target = Expr(target, self.context).ir_node for node in target.args: - if (node.location == STORAGE and self.context.is_constant()) or not node.mutable: + if (node.location.word_addressable and self.context.is_constant()) or not node.mutable: raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target target = Expr.parse_pointer_expr(target, self.context) - if (target.location == STORAGE and self.context.is_constant()) or not target.mutable: + if (target.location.word_addressable and self.context.is_constant()) or not target.mutable: raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 06469ccef2..ab056e6eed 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -621,10 +621,28 @@ def visit_VariableDecl(self, node): assert isinstance(node.target, vy_ast.Name) name = node.target.id + data_loc = ( + DataLocation.CODE + if node.is_immutable + else DataLocation.UNSET + if node.is_constant + else DataLocation.TRANSIENT + if node.is_transient + else DataLocation.STORAGE + ) + + modifiability = ( + Modifiability.RUNTIME_CONSTANT + if node.is_immutable + else Modifiability.CONSTANT + if node.is_constant + else Modifiability.MODIFIABLE + ) + if node.is_public: # generate function type and add to metadata # we need this when building the public getter - func_t = ContractFunctionT.getter_from_VariableDecl(node) + func_t = ContractFunctionT.getter_from_VariableDecl(node, data_loc) node._metadata["getter_type"] = func_t self._add_exposed_function(func_t, node) @@ -648,24 +666,6 @@ def visit_VariableDecl(self, node): ) raise ImmutableViolation(message, node) - data_loc = ( - DataLocation.CODE - if node.is_immutable - else DataLocation.UNSET - if node.is_constant - else DataLocation.TRANSIENT - if node.is_transient - else DataLocation.STORAGE - ) - - modifiability = ( - Modifiability.RUNTIME_CONSTANT - if node.is_immutable - else Modifiability.CONSTANT - if node.is_constant - else Modifiability.MODIFIABLE - ) - type_ = type_from_annotation(node.annotation, data_loc) if node.is_transient and not version_check(begin="cancun"): diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 7eab0958a6..3778b36dc7 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -443,7 +443,7 @@ def set_reentrancy_key_position(self, position: VarOffset) -> None: self.reentrancy_key_position = position @classmethod - def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl) -> "ContractFunctionT": + def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl, data_loc: DataLocation) -> "ContractFunctionT": """ Generate a `ContractFunctionT` object from an `VariableDecl` node. @@ -453,6 +453,7 @@ def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl) -> "ContractFunctio --------- node : VariableDecl Vyper ast node to generate the function definition from. + data_loc : DataLocation Returns ------- @@ -460,7 +461,11 @@ def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl) -> "ContractFunctio """ if not node.is_public: raise CompilerPanic("getter generated for non-public function") - type_ = type_from_annotation(node.annotation, DataLocation.STORAGE) + + assert data_loc not in (DataLocation.MEMORY, DataLocation.CALLDATA) + + type_ = type_from_annotation(node.annotation, data_loc) + arguments, return_type = type_.getter_signature args = [] for i, item in enumerate(arguments): diff --git a/vyper/semantics/types/subscriptable.py b/vyper/semantics/types/subscriptable.py index 5144952be8..b15adb6c52 100644 --- a/vyper/semantics/types/subscriptable.py +++ b/vyper/semantics/types/subscriptable.py @@ -46,7 +46,7 @@ class HashMapT(_SubscriptableT): _equality_attrs = ("key_type", "value_type") - # disallow everything but storage + # disallow everything but storage or transient _invalid_locations = ( DataLocation.UNSET, DataLocation.CALLDATA, From 086eca79ffab3b9f00848f3eedc9c7741e8c7612 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Mon, 13 May 2024 08:33:22 +0200 Subject: [PATCH 02/14] lint --- vyper/codegen/stmt.py | 4 +++- vyper/semantics/types/function.py | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py index 75a528f0b1..3ae75a3e48 100644 --- a/vyper/codegen/stmt.py +++ b/vyper/codegen/stmt.py @@ -315,7 +315,9 @@ def _get_target(self, target): if isinstance(target, vy_ast.Tuple): target = Expr(target, self.context).ir_node for node in target.args: - if (node.location.word_addressable and self.context.is_constant()) or not node.mutable: + if ( + node.location.word_addressable and self.context.is_constant() + ) or not node.mutable: raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 3778b36dc7..62bd84bd0e 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -443,7 +443,9 @@ def set_reentrancy_key_position(self, position: VarOffset) -> None: self.reentrancy_key_position = position @classmethod - def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl, data_loc: DataLocation) -> "ContractFunctionT": + def getter_from_VariableDecl( + cls, node: vy_ast.VariableDecl, data_loc: DataLocation + ) -> "ContractFunctionT": """ Generate a `ContractFunctionT` object from an `VariableDecl` node. From 93cca323e48c824c4aa8e87c085ef936352c2f89 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Tue, 14 May 2024 08:50:02 +0200 Subject: [PATCH 03/14] add util to extract location from decl --- vyper/ast/nodes.pyi | 1 + vyper/semantics/analysis/module.py | 32 +++++++++++------------------- vyper/semantics/analysis/utils.py | 21 ++++++++++++++++++++ vyper/semantics/types/function.py | 8 ++++---- 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/vyper/ast/nodes.pyi b/vyper/ast/nodes.pyi index d67c496188..1c7aaf55ee 100644 --- a/vyper/ast/nodes.pyi +++ b/vyper/ast/nodes.pyi @@ -234,6 +234,7 @@ class VariableDecl(VyperNode): is_constant: bool = ... is_public: bool = ... is_immutable: bool = ... + is_transient: bool = ... _expanded_getter: FunctionDef = ... class AugAssign(VyperNode): diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index ab056e6eed..f49e60d588 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -51,8 +51,8 @@ check_modifiability, get_exact_type_from_node, get_expr_info, + location_from_decl, ) -from vyper.semantics.data_locations import DataLocation from vyper.semantics.namespace import Namespace, get_namespace, override_global_namespace from vyper.semantics.types import EventT, FlagT, InterfaceT, StructT from vyper.semantics.types.function import ContractFunctionT @@ -621,28 +621,10 @@ def visit_VariableDecl(self, node): assert isinstance(node.target, vy_ast.Name) name = node.target.id - data_loc = ( - DataLocation.CODE - if node.is_immutable - else DataLocation.UNSET - if node.is_constant - else DataLocation.TRANSIENT - if node.is_transient - else DataLocation.STORAGE - ) - - modifiability = ( - Modifiability.RUNTIME_CONSTANT - if node.is_immutable - else Modifiability.CONSTANT - if node.is_constant - else Modifiability.MODIFIABLE - ) - if node.is_public: # generate function type and add to metadata # we need this when building the public getter - func_t = ContractFunctionT.getter_from_VariableDecl(node, data_loc) + func_t = ContractFunctionT.getter_from_VariableDecl(node) node._metadata["getter_type"] = func_t self._add_exposed_function(func_t, node) @@ -666,6 +648,16 @@ def visit_VariableDecl(self, node): ) raise ImmutableViolation(message, node) + data_loc = location_from_decl(node) + + modifiability = ( + Modifiability.RUNTIME_CONSTANT + if node.is_immutable + else Modifiability.CONSTANT + if node.is_constant + else Modifiability.MODIFIABLE + ) + type_ = type_from_annotation(node.annotation, data_loc) if node.is_transient and not version_check(begin="cancun"): diff --git a/vyper/semantics/analysis/utils.py b/vyper/semantics/analysis/utils.py index be323b1d13..b2726ed98f 100644 --- a/vyper/semantics/analysis/utils.py +++ b/vyper/semantics/analysis/utils.py @@ -19,6 +19,7 @@ from vyper.semantics import types from vyper.semantics.analysis.base import ExprInfo, Modifiability, ModuleInfo, VarAccess, VarInfo from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions +from vyper.semantics.data_locations import DataLocation from vyper.semantics.namespace import get_namespace from vyper.semantics.types.base import TYPE_T, VyperType from vyper.semantics.types.bytestrings import BytesT, StringT @@ -681,3 +682,23 @@ def check_modifiability(node: vy_ast.ExprNode, modifiability: Modifiability) -> info = get_expr_info(node) return info.modifiability <= modifiability + + +def location_from_decl(node: vy_ast.VariableDecl) -> DataLocation: + """ + Extract the data location from a variable declaration node. + """ + + assert isinstance(node, vy_ast.VariableDecl) + + data_loc = ( + DataLocation.CODE + if node.is_immutable + else DataLocation.UNSET + if node.is_constant + else DataLocation.TRANSIENT + if node.is_transient + else DataLocation.STORAGE + ) + + return data_loc diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 62bd84bd0e..02d3dac489 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -27,6 +27,7 @@ from vyper.semantics.analysis.utils import ( check_modifiability, get_exact_type_from_node, + location_from_decl, uses_state, validate_expected_type, ) @@ -443,9 +444,7 @@ def set_reentrancy_key_position(self, position: VarOffset) -> None: self.reentrancy_key_position = position @classmethod - def getter_from_VariableDecl( - cls, node: vy_ast.VariableDecl, data_loc: DataLocation - ) -> "ContractFunctionT": + def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl) -> "ContractFunctionT": """ Generate a `ContractFunctionT` object from an `VariableDecl` node. @@ -455,7 +454,6 @@ def getter_from_VariableDecl( --------- node : VariableDecl Vyper ast node to generate the function definition from. - data_loc : DataLocation Returns ------- @@ -464,6 +462,8 @@ def getter_from_VariableDecl( if not node.is_public: raise CompilerPanic("getter generated for non-public function") + data_loc = location_from_decl(node) + assert data_loc not in (DataLocation.MEMORY, DataLocation.CALLDATA) type_ = type_from_annotation(node.annotation, data_loc) From fde138caac7cecde24980ea54fa0f68f0d97cb10 Mon Sep 17 00:00:00 2001 From: cyberthirst Date: Tue, 14 May 2024 09:12:00 +0200 Subject: [PATCH 04/14] update location validation for maps --- vyper/semantics/types/subscriptable.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vyper/semantics/types/subscriptable.py b/vyper/semantics/types/subscriptable.py index b15adb6c52..c392ff48b1 100644 --- a/vyper/semantics/types/subscriptable.py +++ b/vyper/semantics/types/subscriptable.py @@ -84,10 +84,11 @@ def from_annotation(cls, node: vy_ast.Subscript) -> "HashMapT": ) k_ast, v_ast = node.slice.elements - key_type = type_from_annotation(k_ast, DataLocation.STORAGE) + key_type = type_from_annotation(k_ast) if not key_type._as_hashmap_key: raise InvalidType("can only use primitive types as HashMap key!", k_ast) + # TODO: thread through actual location - might also be TRANSIENT value_type = type_from_annotation(v_ast, DataLocation.STORAGE) return cls(key_type, value_type) From 9ee1b21b5738e7c358249dbe8ed5cb6b8d0edeb8 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 May 2024 08:36:57 -0400 Subject: [PATCH 05/14] add a comment --- vyper/codegen/core.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index ae19b3aa93..718e50ce39 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -934,8 +934,9 @@ def _complex_make_setter(left, right): assert left.encoding == Encoding.VYPER len_ = left.typ.memory_bytes_required - has_storage = any(loc.word_addressable for loc in (left.location, right.location)) - if has_storage: + # locations with no dedicated copy opcode + # (i.e. storage and transient storage) + if any(loc.word_addressable for loc in (left.location, right.location)): if _opt_codesize(): # assuming PUSH2, a single sstore(dst (sload src)) is 8 bytes, # sstore(add (dst ofst), (sload (add (src ofst)))) is 16 bytes, From 4b5fc574d51e89110ab981ba7fbc7b4704960271 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 May 2024 13:13:24 -0400 Subject: [PATCH 06/14] update a variable name --- vyper/codegen/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index 718e50ce39..e9231241bb 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -150,10 +150,10 @@ def make_byte_array_copier(dst, src): return STORE(dst, 0) with src.cache_when_complex("src") as (b1, src): - has_storage = any(loc.word_addressable for loc in (src.location, dst.location)) + no_copy_opcode = any(loc.word_addressable for loc in (src.location, dst.location)) is_memory_copy = dst.location == src.location == MEMORY batch_uses_identity = is_memory_copy and not version_check(begin="cancun") - if src.typ.maxlen <= 32 and (has_storage or batch_uses_identity): + if src.typ.maxlen <= 32 and (no_copy_opcode or batch_uses_identity): # it's cheaper to run two load/stores instead of copy_bytes ret = ["seq"] From 7baaf6dee042ef00dbd02ef6122a7ac2fa58df65 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 May 2024 13:21:26 -0400 Subject: [PATCH 07/14] add `has_copy_opcode` property --- vyper/codegen/core.py | 4 ++-- vyper/evm/address_space.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index e9231241bb..1852774b2d 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -150,7 +150,7 @@ def make_byte_array_copier(dst, src): return STORE(dst, 0) with src.cache_when_complex("src") as (b1, src): - no_copy_opcode = any(loc.word_addressable for loc in (src.location, dst.location)) + no_copy_opcode = any(not loc.has_copy_opcode for loc in (src.location, dst.location)) is_memory_copy = dst.location == src.location == MEMORY batch_uses_identity = is_memory_copy and not version_check(begin="cancun") if src.typ.maxlen <= 32 and (no_copy_opcode or batch_uses_identity): @@ -936,7 +936,7 @@ def _complex_make_setter(left, right): # locations with no dedicated copy opcode # (i.e. storage and transient storage) - if any(loc.word_addressable for loc in (left.location, right.location)): + if any(not loc.has_copy_opcode for loc in (left.location, right.location)): if _opt_codesize(): # assuming PUSH2, a single sstore(dst (sload src)) is 8 bytes, # sstore(add (dst ofst), (sload (add (src ofst)))) is 16 bytes, diff --git a/vyper/evm/address_space.py b/vyper/evm/address_space.py index 08bef88e58..375821b739 100644 --- a/vyper/evm/address_space.py +++ b/vyper/evm/address_space.py @@ -27,6 +27,7 @@ class AddrSpace: load_op: str # TODO maybe make positional instead of defaulting to None store_op: Optional[str] = None + has_copy_opcode: bool = True # has a dedicated opcode to copy to memory @property def word_addressable(self) -> bool: @@ -43,8 +44,8 @@ def word_addressable(self) -> bool: # MEMORY = Memory() MEMORY = AddrSpace("memory", 32, "mload", "mstore") -STORAGE = AddrSpace("storage", 1, "sload", "sstore") -TRANSIENT = AddrSpace("transient", 1, "tload", "tstore") +STORAGE = AddrSpace("storage", 1, "sload", "sstore", has_copy_opcode=False) +TRANSIENT = AddrSpace("transient", 1, "tload", "tstore", has_copy_opcode=False) CALLDATA = AddrSpace("calldata", 32, "calldataload") # immutables address space: "immutables" section of memory # which is read-write in deploy code but then gets turned into From 5d6449322285f71461d456c44eeeab2d8ba8013d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 May 2024 13:42:30 -0400 Subject: [PATCH 08/14] clean up --- vyper/codegen/core.py | 6 ++++++ vyper/codegen/stmt.py | 11 +++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index 1852774b2d..d4a5181270 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -136,6 +136,12 @@ def address_space_to_data_location(s: AddrSpace) -> DataLocation: raise CompilerPanic("unreachable!") # pragma: nocover +def writeable(context, ir_node): + if context.is_constant() and ir_node.location in (STORAGE, TRANSIENT): + return False + return ir_node.mutable + + # Copy byte array word-for-word (including layout) # TODO make this a private function def make_byte_array_copier(dst, src): diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py index cb4dac2d64..7af81236cb 100644 --- a/vyper/codegen/stmt.py +++ b/vyper/codegen/stmt.py @@ -14,6 +14,7 @@ get_type_for_exact_size, make_setter, wrap_value_for_external_return, + writeable, ) from vyper.codegen.expr import Expr from vyper.codegen.return_ import make_return_stmt @@ -317,15 +318,13 @@ def _get_target(self, target): if isinstance(target, vy_ast.Tuple): target = Expr(target, self.context).ir_node - for node in target.args: - if ( - node.location.word_addressable and self.context.is_constant() - ) or not node.mutable: - raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") + items = target.args + if any(not writeable(self.context, item) for item in items): + raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target target = Expr.parse_pointer_expr(target, self.context) - if (target.location.word_addressable and self.context.is_constant()) or not target.mutable: + if not writeable(self.context, target): raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target From 5e3eef498f1a93ac7cd1bcbc67f33fe27a0f1341 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 16 May 2024 17:55:48 -0400 Subject: [PATCH 09/14] rework some logic for cancun --- vyper/codegen/core.py | 27 ++++++++++++++++++--------- vyper/evm/address_space.py | 16 ++++++++++------ 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index d4a5181270..3185b5c9b6 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -137,6 +137,8 @@ def address_space_to_data_location(s: AddrSpace) -> DataLocation: def writeable(context, ir_node): + assert ir_node.is_pointer # sanity check + if context.is_constant() and ir_node.location in (STORAGE, TRANSIENT): return False return ir_node.mutable @@ -156,12 +158,9 @@ def make_byte_array_copier(dst, src): return STORE(dst, 0) with src.cache_when_complex("src") as (b1, src): - no_copy_opcode = any(not loc.has_copy_opcode for loc in (src.location, dst.location)) - is_memory_copy = dst.location == src.location == MEMORY - batch_uses_identity = is_memory_copy and not version_check(begin="cancun") - if src.typ.maxlen <= 32 and (no_copy_opcode or batch_uses_identity): + if src.typ.maxlen <= 32 and not copy_opcode_available(dst, src): + # if there is no batch copy opcode available, # it's cheaper to run two load/stores instead of copy_bytes - ret = ["seq"] # store length word len_ = get_bytearray_length(src) @@ -919,6 +918,15 @@ def make_setter(left, right): return _complex_make_setter(left, right) +# locations with no dedicated copy opcode +# (i.e. storage and transient storage) +def copy_opcode_available(left, right): + if left.location == MEMORY and right.location == MEMORY: + return version_check(begin="cancun") + + return left.location == MEMORY and right.location.has_copy_opcode + + def _complex_make_setter(left, right): if right.value == "~empty" and left.location == MEMORY: # optimized memzero @@ -940,9 +948,10 @@ def _complex_make_setter(left, right): assert left.encoding == Encoding.VYPER len_ = left.typ.memory_bytes_required - # locations with no dedicated copy opcode - # (i.e. storage and transient storage) - if any(not loc.has_copy_opcode for loc in (left.location, right.location)): + # special logic for identity precompile (pre-cancun) in the else branch + mem2mem = left.location == right.location == MEMORY + + if not copy_opcode_available(left, right) and not mem2mem: if _opt_codesize(): # assuming PUSH2, a single sstore(dst (sload src)) is 8 bytes, # sstore(add (dst ofst), (sload (add (src ofst)))) is 16 bytes, @@ -989,7 +998,7 @@ def _complex_make_setter(left, right): base_unroll_cost + (nth_word_cost * (n_words - 1)) >= identity_base_cost ) - # calldata to memory, code to memory, cancun, or codesize - + # calldata to memory, code to memory, cancun, or opt-codesize - # batch copy is always better. else: should_batch_copy = True diff --git a/vyper/evm/address_space.py b/vyper/evm/address_space.py index 375821b739..19735c3981 100644 --- a/vyper/evm/address_space.py +++ b/vyper/evm/address_space.py @@ -27,12 +27,16 @@ class AddrSpace: load_op: str # TODO maybe make positional instead of defaulting to None store_op: Optional[str] = None - has_copy_opcode: bool = True # has a dedicated opcode to copy to memory + copy_op: Optional[str] = None @property def word_addressable(self) -> bool: return self.word_scale == 1 + @property + def has_copy_opcode(self): + return self.copy_op is not None + # alternative: # class Memory(AddrSpace): @@ -43,13 +47,13 @@ def word_addressable(self) -> bool: # # MEMORY = Memory() -MEMORY = AddrSpace("memory", 32, "mload", "mstore") -STORAGE = AddrSpace("storage", 1, "sload", "sstore", has_copy_opcode=False) -TRANSIENT = AddrSpace("transient", 1, "tload", "tstore", has_copy_opcode=False) -CALLDATA = AddrSpace("calldata", 32, "calldataload") +MEMORY = AddrSpace("memory", 32, "mload", "mstore", "mcopy") +STORAGE = AddrSpace("storage", 1, "sload", "sstore") +TRANSIENT = AddrSpace("transient", 1, "tload", "tstore") +CALLDATA = AddrSpace("calldata", 32, "calldataload", None, "calldatacopy") # immutables address space: "immutables" section of memory # which is read-write in deploy code but then gets turned into # the "data" section of the runtime code IMMUTABLES = AddrSpace("immutables", 32, "iload", "istore") # data addrspace: "data" section of runtime code, read-only. -DATA = AddrSpace("data", 32, "dload") +DATA = AddrSpace("data", 32, "dload", None, "dloadbytes") From 6dc7d75a29b62ea0843d6329c895c02a2fe280f0 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 May 2024 12:13:36 -0400 Subject: [PATCH 10/14] rework getter_from_VariableDecl flow --- vyper/semantics/analysis/module.py | 30 +++++++++++++++++++----------- vyper/semantics/analysis/utils.py | 21 --------------------- vyper/semantics/types/function.py | 8 ++------ 3 files changed, 21 insertions(+), 38 deletions(-) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 3121592932..dcaf27d661 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -51,8 +51,8 @@ check_modifiability, get_exact_type_from_node, get_expr_info, - location_from_decl, ) +from vyper.semantics.data_locations import DataLocation from vyper.semantics.namespace import Namespace, get_namespace, override_global_namespace from vyper.semantics.types import EventT, FlagT, InterfaceT, StructT from vyper.semantics.types.function import ContractFunctionT @@ -621,13 +621,6 @@ def visit_VariableDecl(self, node): assert isinstance(node.target, vy_ast.Name) name = node.target.id - if node.is_public: - # generate function type and add to metadata - # we need this when building the public getter - func_t = ContractFunctionT.getter_from_VariableDecl(node) - node._metadata["getter_type"] = func_t - self._add_exposed_function(func_t, node) - # TODO: move this check to local analysis if node.is_immutable: # mutability is checked automatically preventing assignment @@ -648,7 +641,15 @@ def visit_VariableDecl(self, node): ) raise ImmutableViolation(message, node) - data_loc = location_from_decl(node) + location = ( + DataLocation.CODE + if node.is_immutable + else DataLocation.UNSET + if node.is_constant + else DataLocation.TRANSIENT + if node.is_transient + else DataLocation.STORAGE + ) modifiability = ( Modifiability.RUNTIME_CONSTANT @@ -658,7 +659,7 @@ def visit_VariableDecl(self, node): else Modifiability.MODIFIABLE ) - type_ = type_from_annotation(node.annotation, data_loc) + type_ = type_from_annotation(node.annotation, location) if node.is_transient and not version_check(begin="cancun"): raise EvmVersionException("`transient` is not available pre-cancun", node.annotation) @@ -666,13 +667,20 @@ def visit_VariableDecl(self, node): var_info = VarInfo( type_, decl_node=node, - location=data_loc, + location=location, modifiability=modifiability, is_public=node.is_public, ) node.target._metadata["varinfo"] = var_info # TODO maybe put this in the global namespace node._metadata["type"] = type_ + if node.is_public: + # generate function type and add to metadata + # we need this when building the public getter + func_t = ContractFunctionT.getter_from_VariableDecl(node) + node._metadata["getter_type"] = func_t + self._add_exposed_function(func_t, node) + def _finalize(): # add the variable name to `self` namespace if the variable is either # 1. a public constant or immutable; or diff --git a/vyper/semantics/analysis/utils.py b/vyper/semantics/analysis/utils.py index b2726ed98f..be323b1d13 100644 --- a/vyper/semantics/analysis/utils.py +++ b/vyper/semantics/analysis/utils.py @@ -19,7 +19,6 @@ from vyper.semantics import types from vyper.semantics.analysis.base import ExprInfo, Modifiability, ModuleInfo, VarAccess, VarInfo from vyper.semantics.analysis.levenshtein_utils import get_levenshtein_error_suggestions -from vyper.semantics.data_locations import DataLocation from vyper.semantics.namespace import get_namespace from vyper.semantics.types.base import TYPE_T, VyperType from vyper.semantics.types.bytestrings import BytesT, StringT @@ -682,23 +681,3 @@ def check_modifiability(node: vy_ast.ExprNode, modifiability: Modifiability) -> info = get_expr_info(node) return info.modifiability <= modifiability - - -def location_from_decl(node: vy_ast.VariableDecl) -> DataLocation: - """ - Extract the data location from a variable declaration node. - """ - - assert isinstance(node, vy_ast.VariableDecl) - - data_loc = ( - DataLocation.CODE - if node.is_immutable - else DataLocation.UNSET - if node.is_constant - else DataLocation.TRANSIENT - if node.is_transient - else DataLocation.STORAGE - ) - - return data_loc diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 02d3dac489..783288d03f 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -27,7 +27,6 @@ from vyper.semantics.analysis.utils import ( check_modifiability, get_exact_type_from_node, - location_from_decl, uses_state, validate_expected_type, ) @@ -462,11 +461,8 @@ def getter_from_VariableDecl(cls, node: vy_ast.VariableDecl) -> "ContractFunctio if not node.is_public: raise CompilerPanic("getter generated for non-public function") - data_loc = location_from_decl(node) - - assert data_loc not in (DataLocation.MEMORY, DataLocation.CALLDATA) - - type_ = type_from_annotation(node.annotation, data_loc) + # calculated by caller (ModuleAnalyzer.visit_VariableDecl) + type_ = node.target._metadata["varinfo"].typ arguments, return_type = type_.getter_signature args = [] From e41696a3be508a59c209874865e8994a60ac90c3 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 May 2024 12:28:10 -0400 Subject: [PATCH 11/14] add another helper function --- vyper/codegen/core.py | 3 ++- vyper/evm/address_space.py | 4 ++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/vyper/codegen/core.py b/vyper/codegen/core.py index 3185b5c9b6..661f349dca 100644 --- a/vyper/codegen/core.py +++ b/vyper/codegen/core.py @@ -8,6 +8,7 @@ STORAGE, TRANSIENT, AddrSpace, + legal_in_staticcall, ) from vyper.evm.opcodes import version_check from vyper.exceptions import CompilerPanic, TypeCheckFailure, TypeMismatch @@ -139,7 +140,7 @@ def address_space_to_data_location(s: AddrSpace) -> DataLocation: def writeable(context, ir_node): assert ir_node.is_pointer # sanity check - if context.is_constant() and ir_node.location in (STORAGE, TRANSIENT): + if context.is_constant() and not legal_in_staticcall(ir_node.location): return False return ir_node.mutable diff --git a/vyper/evm/address_space.py b/vyper/evm/address_space.py index 19735c3981..e362d44fcc 100644 --- a/vyper/evm/address_space.py +++ b/vyper/evm/address_space.py @@ -57,3 +57,7 @@ def has_copy_opcode(self): IMMUTABLES = AddrSpace("immutables", 32, "iload", "istore") # data addrspace: "data" section of runtime code, read-only. DATA = AddrSpace("data", 32, "dload", None, "dloadbytes") + + +def legal_in_staticcall(location: AddrSpace): + return location not in (STORAGE, TRANSIENT) From 56544ce170aede1fb2714364a0f829db744ee853 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 May 2024 12:32:46 -0400 Subject: [PATCH 12/14] add some pragma: nocover --- vyper/codegen/stmt.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py index 7af81236cb..562a9d85d7 100644 --- a/vyper/codegen/stmt.py +++ b/vyper/codegen/stmt.py @@ -313,18 +313,18 @@ def parse_Return(self): def _get_target(self, target): _dbg_expr = target - if isinstance(target, vy_ast.Name) and target.id in self.context.forvars: + if isinstance(target, vy_ast.Name) and target.id in self.context.forvars: # pragma: nocover raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") if isinstance(target, vy_ast.Tuple): target = Expr(target, self.context).ir_node items = target.args - if any(not writeable(self.context, item) for item in items): + if any(not writeable(self.context, item) for item in items): # pragma: nocover raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target target = Expr.parse_pointer_expr(target, self.context) - if not writeable(self.context, target): + if not writeable(self.context, target): # pragma: nocover raise TypeCheckFailure(f"Failed constancy check\n{_dbg_expr}") return target From f4405b51503bc60188fd7bd4a49cb48924e2a642 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 17 May 2024 13:13:30 -0400 Subject: [PATCH 13/14] add pre-cancun test coverage for opt-codesize and opt-none --- .github/workflows/test.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8edb0c096a..8d491e2530 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -91,6 +91,12 @@ jobs: - evm-version: paris - evm-version: shanghai + # test pre-cancun with opt-codesize and opt-none + - evm-version: shanghai + opt-mode: none + - evm-version: shanghai + opt-mode: codesize + # test py-evm - evm-backend: py-evm evm-version: shanghai From daa84449e66e7055a6bbdd78a2b6b58382c16830 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 18 May 2024 16:38:47 -0400 Subject: [PATCH 14/14] update docstring --- vyper/evm/address_space.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vyper/evm/address_space.py b/vyper/evm/address_space.py index e362d44fcc..c0fdecf6b9 100644 --- a/vyper/evm/address_space.py +++ b/vyper/evm/address_space.py @@ -20,6 +20,8 @@ class AddrSpace: load_op: the opcode for loading a word from this address space store_op: the opcode for storing a word to this address space (an address space is read-only if store_op is None) + copy_op: the opcode for batch-copying from this address space + to memory """ name: str