Skip to content

Issue #2498 has been resolved. #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lanceherman wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Issue #2498 has been resolved. #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 103 additions & 23 deletions vyper/builtins/functions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,37 +191,61 @@ def build_IR(self, expr, args, kwargs, context):
class Convert(BuiltinFunctionT):
_id = "convert"

def _try_fold(self, node):
validate_call_args(node, 2)
value = node.args[0].get_folded_value()
target_type = node.args[1].get_folded_value()
if not isinstance(target_type, vy_ast.Name):
raise UnfoldableNode

# Get the target type
target_typedef = type_from_annotation(target_type)
if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)):
raise UnfoldableNode

# Convert the value based on target type
if isinstance(target_typedef, BoolT):
if isinstance(value, vy_ast.Int):
result = bool(value.value)
elif isinstance(value, vy_ast.Bool):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Bool.from_node(node, value=result)

elif isinstance(target_typedef, AddressT):
if isinstance(value, vy_ast.Hex):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)

elif isinstance(target_typedef, IntegerT):
if isinstance(value, (vy_ast.Int, vy_ast.Decimal)):
result = int(value.value)
else:
raise UnfoldableNode
return vy_ast.Int.from_node(node, value=result)

elif isinstance(target_typedef, BytesM_T):
if isinstance(value, vy_ast.Hex):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)

raise UnfoldableNode

Comment on lines +194 to +238

@coderabbitai coderabbitai Bot May 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Constant-folded convert() skips range / size validation – can emit ill-typed literals at compile time

The new folding branch happily constructs the target literal without any of the safety checks that the runtime implementation performs:

  • For IntegerT targets no ast_bounds (min / max) verification is done, so convert(2**300, int128) would now be folded into an out-of-range vy_ast.Int which later passes type-checking because its value has already been frozen.
  • For AddressT the hex literal length is not verified (should be exactly 20 bytes / 40 hex chars).
  • For BytesM_T the produced literal is not checked against target_typedef.length.
  • Silent narrowing of DecimalT → IntegerT truncates without a bounds check or a ZeroDivisionException- style guard used elsewhere.

This opens a correctness hole: an invalid literal is accepted during folding and only surfaces at execution, bypassing compile-time guarantees.

@@
-        elif isinstance(target_typedef, IntegerT):
-            if isinstance(value, (vy_ast.Int, vy_ast.Decimal)):
-                result = int(value.value)
-            else:
-                raise UnfoldableNode
-            return vy_ast.Int.from_node(node, value=result)
+        elif isinstance(target_typedef, IntegerT):
+            if not isinstance(value, (vy_ast.Int, vy_ast.Decimal)):
+                raise UnfoldableNode
+            result = int(value.value)
+            # validate range            ↓↓↓
+            lo, hi = target_typedef.ast_bounds
+            if result < lo or result > hi:
+                raise InvalidLiteral(
+                    f"value {result} out of range for {target_typedef}", node.args[0]
+                )
+            return vy_ast.Int.from_node(node, value=result)

Similar guards are needed for the AddressT and BytesM_T branches.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def _try_fold(self, node):
validate_call_args(node, 2)
value = node.args[0].get_folded_value()
target_type = node.args[1].get_folded_value()
if not isinstance(target_type, vy_ast.Name):
raise UnfoldableNode
# Get the target type
target_typedef = type_from_annotation(target_type)
if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)):
raise UnfoldableNode
# Convert the value based on target type
if isinstance(target_typedef, BoolT):
if isinstance(value, vy_ast.Int):
result = bool(value.value)
elif isinstance(value, vy_ast.Bool):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Bool.from_node(node, value=result)
elif isinstance(target_typedef, AddressT):
if isinstance(value, vy_ast.Hex):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)
elif isinstance(target_typedef, IntegerT):
if isinstance(value, (vy_ast.Int, vy_ast.Decimal)):
result = int(value.value)
else:
raise UnfoldableNode
return vy_ast.Int.from_node(node, value=result)
elif isinstance(target_typedef, BytesM_T):
if isinstance(value, vy_ast.Hex):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)
raise UnfoldableNode
def _try_fold(self, node):
validate_call_args(node, 2)
value = node.args[0].get_folded_value()
target_type = node.args[1].get_folded_value()
if not isinstance(target_type, vy_ast.Name):
raise UnfoldableNode
# Get the target type
target_typedef = type_from_annotation(target_type)
if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)):
raise UnfoldableNode
# Convert the value based on target type
if isinstance(target_typedef, BoolT):
if isinstance(value, vy_ast.Int):
result = bool(value.value)
elif isinstance(value, vy_ast.Bool):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Bool.from_node(node, value=result)
elif isinstance(target_typedef, AddressT):
if isinstance(value, vy_ast.Hex):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)
elif isinstance(target_typedef, IntegerT):
if not isinstance(value, (vy_ast.Int, vy_ast.Decimal)):
raise UnfoldableNode
result = int(value.value)
# validate range
lo, hi = target_typedef.ast_bounds
if result < lo or result > hi:
raise InvalidLiteral(
f"value {result} out of range for {target_typedef}", node.args[0]
)
return vy_ast.Int.from_node(node, value=result)
elif isinstance(target_typedef, BytesM_T):
if isinstance(value, vy_ast.Hex):
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)
raise UnfoldableNode

def fetch_call_return(self, node):
_, target_typedef = self.infer_arg_types(node)

# note: more type conversion validation happens in convert.py
return target_typedef.typedef

# TODO: push this down into convert.py for more consistency
def infer_arg_types(self, node, expected_return_typ=None):
validate_call_args(node, 2)

self._validate_arg_types(node)
value_type = get_possible_types_from_node(node.args[0]).pop()
target_type = type_from_annotation(node.args[1])
value_types = get_possible_types_from_node(node.args[0])

# For `convert` of integer literals, we need to match type inference rules in
# convert.py codegen routines.
# TODO: This can probably be removed once constant folding for `convert` is implemented
if len(value_types) > 1 and all(isinstance(v, IntegerT) for v in value_types):
# Get the smallest (and unsigned if available) type for non-integer target types
# (note this is different from the ordering returned by `get_possible_types_from_node`)
if not isinstance(target_type, IntegerT):
value_types = sorted(value_types, key=lambda v: (v.is_signed, v.bits), reverse=True)
else:
# filter out the target type from list of possible types
value_types = [i for i in value_types if not target_type.compare_type(i)]

value_type = value_types.pop()

# block conversions between same type
if target_type.compare_type(value_type):
raise InvalidType(f"Value and target type are both '{target_type}'", node)

return [value_type, TYPE_T(target_type)]
Comment on lines 245 to 249

@coderabbitai coderabbitai Bot May 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Non-deterministic type selection in infer_arg_types

get_possible_types_from_node(...).pop() takes an arbitrary element from a set.
Two identical compilations can therefore pick different source types when multiple are possible, leading to flaky error messages or, worse, silently different IR.

Use a deterministic selection strategy, e.g. sort by priority:

-        value_type = get_possible_types_from_node(node.args[0]).pop()
+        possible = sorted(
+            get_possible_types_from_node(node.args[0]),
+            key=lambda t: (t.typ, getattr(t, "bits", 0))  # or any stable rule
+        )
+        value_type = possible[0]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def infer_arg_types(self, node, expected_return_typ=None):
validate_call_args(node, 2)
self._validate_arg_types(node)
value_type = get_possible_types_from_node(node.args[0]).pop()
target_type = type_from_annotation(node.args[1])
value_types = get_possible_types_from_node(node.args[0])
# For `convert` of integer literals, we need to match type inference rules in
# convert.py codegen routines.
# TODO: This can probably be removed once constant folding for `convert` is implemented
if len(value_types) > 1 and all(isinstance(v, IntegerT) for v in value_types):
# Get the smallest (and unsigned if available) type for non-integer target types
# (note this is different from the ordering returned by `get_possible_types_from_node`)
if not isinstance(target_type, IntegerT):
value_types = sorted(value_types, key=lambda v: (v.is_signed, v.bits), reverse=True)
else:
# filter out the target type from list of possible types
value_types = [i for i in value_types if not target_type.compare_type(i)]
value_type = value_types.pop()
# block conversions between same type
if target_type.compare_type(value_type):
raise InvalidType(f"Value and target type are both '{target_type}'", node)
return [value_type, TYPE_T(target_type)]
def infer_arg_types(self, node, expected_return_typ=None):
self._validate_arg_types(node)
- value_type = get_possible_types_from_node(node.args[0]).pop()
+ possible = sorted(
+ get_possible_types_from_node(node.args[0]),
+ key=lambda t: (t.typ, getattr(t, "bits", 0)) # stable ordering
+ )
+ value_type = possible[0]
target_type = type_from_annotation(node.args[1])
return [value_type, TYPE_T(target_type)]


def build_IR(self, expr, context):
Expand Down Expand Up @@ -290,6 +314,39 @@ class Slice(BuiltinFunctionT):
("length", UINT256_T),
]

def _try_fold(self, node):
validate_call_args(node, 3)
value = node.args[0].get_folded_value()
start = node.args[1].get_folded_value()
length = node.args[2].get_folded_value()

if not isinstance(start, vy_ast.Int) or not isinstance(length, vy_ast.Int):
raise UnfoldableNode

start_val = start.value
length_val = length.value

if length_val < 1:
raise ArgumentException("Length cannot be less than 1", node.args[2])

if isinstance(value, vy_ast.Bytes):
if start_val + length_val > len(value.value):
raise ArgumentException("slice out of bounds", node)
result = value.value[start_val:start_val + length_val]
return vy_ast.Bytes.from_node(node, value=result)
elif isinstance(value, vy_ast.Str):
if start_val + length_val > len(value.value):
raise ArgumentException("slice out of bounds", node)
result = value.value[start_val:start_val + length_val]
return vy_ast.Str.from_node(node, value=result)
elif isinstance(value, vy_ast.Hex):
if start_val + length_val > len(value.bytes_value):
raise ArgumentException("slice out of bounds", node)
result = value.bytes_value[start_val:start_val + length_val]
return vy_ast.Bytes.from_node(node, value=result)
else:
raise UnfoldableNode

def fetch_call_return(self, node):
arg_type, _, _ = self.infer_arg_types(node)

Expand Down Expand Up @@ -833,6 +890,29 @@ class Extract32(BuiltinFunctionT):
_inputs = [("b", BytesT.any()), ("start", IntegerT.unsigneds())]
_kwargs = {"output_type": KwargSettings(TYPE_T.any(), BYTES32_T)}

def _try_fold(self, node):
validate_call_args(node, 2)
value = node.args[0].get_folded_value()
start = node.args[1].get_folded_value()

if not isinstance(start, vy_ast.Int):
raise UnfoldableNode

start_val = start.value

if isinstance(value, vy_ast.Bytes):
if start_val + 32 > len(value.value):
raise ArgumentException("extract32 out of bounds", node)
result = value.value[start_val:start_val + 32]
return vy_ast.Bytes.from_node(node, value=result)
elif isinstance(value, vy_ast.Hex):
if start_val + 32 > len(value.bytes_value):
raise ArgumentException("extract32 out of bounds", node)
result = value.bytes_value[start_val:start_val + 32]
return vy_ast.Bytes.from_node(node, value=result)
else:
raise UnfoldableNode

Comment on lines +893 to +915

@coderabbitai coderabbitai Bot May 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

extract32() folding ignores output_type kwarg and returns wrong AST node

Issues:

  1. The kw-argument output_type (default bytes32) is not examined, so
    extract32(b, 0, output_type=int128) still folds to a Bytes node.
  2. Even for the default case the function emits a dynamic Bytes
    literal, not a fixed-length BytesM_T(32) or vy_ast.Hex, breaking
    the return-type contract.
  3. No negative-index / bounds checking for start < 0.
@@
-            result = value.value[start_val:start_val + 32]
-            return vy_ast.Bytes.from_node(node, value=result)
+            result = value.value[start_val : start_val + 32]
+
+            out_typedef = self.infer_kwarg_types(node)["output_type"].typedef
+
+            if isinstance(out_typedef, BytesM_T):
+                return vy_ast.Hex.from_node(node, value=result.hex())
+            elif isinstance(out_typedef, IntegerT):
+                return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big"))
+            elif isinstance(out_typedef, AddressT):
+                return vy_ast.Hex.from_node(node, value="0x" + result.hex())
+            else:
+                raise CompilerPanic("Unhandled output_type in extract32 folding")

Failing to honour output_type can propagate a mismatched type deep into
IR generation and surface as opaque post-optimisation crashes.

Committable suggestion skipped: line range outside the PR's diff.

def fetch_call_return(self, node):
self._validate_arg_types(node)
return_type = self.infer_kwarg_types(node)["output_type"].typedef
Expand Down