Skip to content

Issues 2498 2 #2

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 2 commits into
base: master
Choose a base branch
from
Open

Issues 2498 2 #2

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f4c123
Issue #2498 has been resolved.
lanceherman May 8, 2025
6cd3a7e
This is a second commit of Issue #2498
lanceherman May 8, 2025
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
181 changes: 155 additions & 26 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,74 @@ 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

target_typedef = type_from_annotation(target_type)
if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)):
raise UnfoldableNode

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):
if len(value.value) != 42: # 0x + 40 hex chars
raise InvalidLiteral("Address must be exactly 20 bytes", node.args[0])
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)
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):
if len(value.bytes_value) != target_typedef.length:
raise InvalidLiteral(
f"Expected {target_typedef.length} bytes, got {len(value.bytes_value)}",
node.args[0]
)
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)

raise UnfoldableNode
Comment on lines +194 to +246

@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

Potentially unsafe conversions & missing edge-case validation in _try_fold

  1. DecimalT ➞ IntegerT truncates silently via int(value.value).
    Fractions are discarded instead of being rejected or rounded – this may yield unexpected compile-time values.

  2. Hex ➞ AddressT only checks len(value.value)==42, but does not:
    • validate the literal actually represents 20 bytes (bytes.fromhex / .bytes_value would be safer)
    • reject literals without the "0x" prefix (e.g. "deadbeef…") – the length check alone would pass.

  3. Converting to BytesM_T assumes the slice must be exactly target_typedef.length. Allowing shorter literals to left-pad or throwing a dedicated error would be clearer.

  4. No constant-folding path exists for:
    • Int ➞ Address (very common in assembly-style patterns)
    • Bytes literals to BytesM_T (only Hex handled).

Consider tightening validation and clarifying behaviour. Example for the DecimalT issue:

-            result = int(value.value)
+            if isinstance(value, vy_ast.Decimal) and value.value % 1 != 0:
+                raise InvalidLiteral("Cannot truncate decimal when converting to integer", node.args[0])
+            result = int(value.value)
📝 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
target_typedef = type_from_annotation(target_type)
if not isinstance(target_typedef, (BoolT, AddressT, IntegerT, BytesM_T)):
raise UnfoldableNode
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):
if len(value.value) != 42: # 0x + 40 hex chars
raise InvalidLiteral("Address must be exactly 20 bytes", node.args[0])
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)
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):
if len(value.bytes_value) != target_typedef.length:
raise InvalidLiteral(
f"Expected {target_typedef.length} bytes, got {len(value.bytes_value)}",
node.args[0]
)
result = value.value
else:
raise UnfoldableNode
return vy_ast.Hex.from_node(node, value=result)
raise UnfoldableNode
elif isinstance(target_typedef, IntegerT):
if not isinstance(value, (vy_ast.Int, vy_ast.Decimal)):
raise UnfoldableNode
+ if isinstance(value, vy_ast.Decimal) and value.value % 1 != 0:
+ raise InvalidLiteral("Cannot truncate decimal when converting to integer", node.args[0])
result = int(value.value)
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)


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)
possible = sorted(
get_possible_types_from_node(node.args[0]),
key=lambda t: (t.typ, getattr(t, "bits", 0))
)
value_type = possible[0]
Comment on lines +255 to +260

@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

sorted() key may raise at runtime

possible = sorted(..., key=lambda t: (t.typ, getattr(t, "bits", 0)))

If t.typ is an Enum (or any non-orderable object) Python 3 will raise TypeError. This sneaks through unit tests if all first elements are equal but explodes on heterogeneous sets.

Safer alternative:

- key=lambda t: (t.typ, getattr(t, "bits", 0))
+ key=lambda t: (str(t.typ), getattr(t, "bits", 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
self._validate_arg_types(node)
possible = sorted(
get_possible_types_from_node(node.args[0]),
key=lambda t: (t.typ, getattr(t, "bits", 0))
)
value_type = possible[0]
self._validate_arg_types(node)
possible = sorted(
get_possible_types_from_node(node.args[0]),
key=lambda t: (str(t.typ), getattr(t, "bits", 0))
)
value_type = possible[0]

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 build_IR(self, expr, context):
Expand Down Expand Up @@ -290,6 +327,50 @@ 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 start_val < 0:
raise ArgumentException("Start index cannot be negative", node.args[1])

if isinstance(value, vy_ast.Bytes):
if start_val + length_val > len(value.value):
raise ArgumentException(
f"slice out of bounds: start={start_val}, length={length_val}, max_length={len(value.value)}",
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(
f"slice out of bounds: start={start_val}, length={length_val}, max_length={len(value.value)}",
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(
f"slice out of bounds: start={start_val}, length={length_val}, max_length={len(value.bytes_value)}",
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 @@ -336,8 +417,11 @@ def fetch_call_return(self, node):

def infer_arg_types(self, node, expected_return_typ=None):
self._validate_arg_types(node)
# return a concrete type for `b`
b_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))
)
b_type = possible[0]
return [b_type, self._inputs[1][1], self._inputs[2][1]]

@process_inputs
Expand Down Expand Up @@ -833,14 +917,59 @@ 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 start_val < 0:
raise ArgumentException("Start index cannot be negative", node.args[1])

output_type = self.infer_kwarg_types(node)["output_type"].typedef

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

if isinstance(output_type, BytesM_T):
return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
elif isinstance(output_type, IntegerT):
return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big"))
elif isinstance(output_type, AddressT):
return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
else:
raise UnfoldableNode

Comment on lines +920 to +960

@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 may violate the requested output size

The function always extracts 32 bytes, yet:

  • output_type may be BytesM_T(n) with n ≠ 32
  • AddressT expects 20 bytes but receives 32

This creates silently-wrong literals and type mismatch down the road.

Minimal fix:

@@
-        if isinstance(output_type, BytesM_T):
-            return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
+        if isinstance(output_type, BytesM_T):
+            expected = output_type.length
+            if expected != 32:
+                result = result[:expected]
+            return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
         elif isinstance(output_type, IntegerT):
@@
-        elif isinstance(output_type, AddressT):
-            return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
+        elif isinstance(output_type, AddressT):
+            # right-align as per ABI: take the last 20 bytes
+            return vy_ast.Hex.from_node(node, value=f"0x{result[-20:].hex()}")

Consider additionally rejecting BytesM_T with length > 32 at semantic level.

📝 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()
start = node.args[1].get_folded_value()
if not isinstance(start, vy_ast.Int):
raise UnfoldableNode
start_val = start.value
if start_val < 0:
raise ArgumentException("Start index cannot be negative", node.args[1])
output_type = self.infer_kwarg_types(node)["output_type"].typedef
if isinstance(value, vy_ast.Bytes):
if start_val + 32 > len(value.value):
raise ArgumentException(
f"extract32 out of bounds: start={start_val}, max_length={len(value.value)}",
node
)
result = value.value[start_val:start_val + 32]
elif isinstance(value, vy_ast.Hex):
if start_val + 32 > len(value.bytes_value):
raise ArgumentException(
f"extract32 out of bounds: start={start_val}, max_length={len(value.bytes_value)}",
node
)
result = value.bytes_value[start_val:start_val + 32]
else:
raise UnfoldableNode
if isinstance(output_type, BytesM_T):
return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
elif isinstance(output_type, IntegerT):
return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big"))
elif isinstance(output_type, AddressT):
return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
else:
raise UnfoldableNode
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 start_val < 0:
raise ArgumentException("Start index cannot be negative", node.args[1])
output_type = self.infer_kwarg_types(node)["output_type"].typedef
if isinstance(value, vy_ast.Bytes):
if start_val + 32 > len(value.value):
raise ArgumentException(
f"extract32 out of bounds: start={start_val}, max_length={len(value.value)}",
node
)
result = value.value[start_val:start_val + 32]
elif isinstance(value, vy_ast.Hex):
if start_val + 32 > len(value.bytes_value):
raise ArgumentException(
f"extract32 out of bounds: start={start_val}, max_length={len(value.bytes_value)}",
node
)
result = value.bytes_value[start_val:start_val + 32]
else:
raise UnfoldableNode
if isinstance(output_type, BytesM_T):
expected = output_type.length
if expected != 32:
result = result[:expected]
return vy_ast.Hex.from_node(node, value=f"0x{result.hex()}")
elif isinstance(output_type, IntegerT):
return vy_ast.Int.from_node(node, value=int.from_bytes(result, "big"))
elif isinstance(output_type, AddressT):
# right-align as per ABI: take the last 20 bytes
return vy_ast.Hex.from_node(node, value=f"0x{result[-20:].hex()}")
else:
raise UnfoldableNode

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

def infer_arg_types(self, node, expected_return_typ=None):
self._validate_arg_types(node)
input_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))
)
input_type = possible[0]
return [input_type, UINT256_T]

def infer_kwarg_types(self, node):
Expand Down