refactor[codegen]: refactor venom builtins for uniformity and cleanliness

tests/functional/builtins/codegen/test_create_functions.py

@@ -1145,10 +1145,13 @@ def deploy_from_calldata(s: Bytes[1024], arg: uint256, salt: bytes32) -> address
     assert env.get_code(res) == runtime
 
 
-# evaluation of the value kwarg changes the value of the salt kwarg
-# value kwarg comes after the salt kwarg in the source code
-@pytest.mark.xfail(raises=AssertionError, reason="salt kwarg is evaluated after value kwarg")
-def test_raw_create_order_of_eval_of_kwargs(get_contract, env, create2_address_of, keccak):
+# value is evaluated after salt because that is the source order.
+def test_raw_create_order_of_eval_of_kwargs(
+    get_contract, env, create2_address_of, keccak, experimental_codegen
+):
+    if not experimental_codegen:
+        pytest.xfail("legacy codegen does not preserve this source order")
+
     to_deploy_code = """
 foo: public(uint256)
 
@@ -1162,8 +1165,6 @@ def __init__(arg: uint256):
     initcode = bytes.fromhex(out["bytecode"].removeprefix("0x"))
     runtime = bytes.fromhex(out["bytecode_runtime"].removeprefix("0x"))
 
-    # the implementation of `raw_create` firstly caches
-    # `value` and then `salt`, here the source order is `salt` then `value`
     deployer_code = """
 c: Bytes[1024]
 salt: bytes32
@@ -1194,11 +1195,10 @@ def deploy_from_calldata(s: Bytes[1024], arg: uint256, salt: bytes32, value_: ui
 
 
 # test vararg and kwarg order of evaluation
-# test fails because `value` gets evaluated
-# before the 1st vararg which doesn't follow
-# source code order
-@pytest.mark.xfail(raises=AssertionError)
-def test_raw_create_eval_order(get_contract):
+def test_raw_create_eval_order(get_contract, experimental_codegen):
+    if not experimental_codegen:
+        pytest.xfail("legacy codegen does not preserve this source order")
+
     code = """
 a: public(uint256)
 

tests/functional/builtins/codegen/test_raw_call.py

@@ -529,6 +529,53 @@ def foo() -> String[32]:
     assert c.foo() == "goo"
 
 
+def test_raw_call_runtime_kwargs_source_order(get_contract, experimental_codegen):
+    if not experimental_codegen:
+        pytest.xfail("legacy codegen does not preserve this source order")
+
+    code = """
+canary: public(uint256)
+
+@internal
+def set_gas(v: uint256) -> uint256:
+    self.canary = v
+    return 50000
+
+@internal
+def set_value(v: uint256) -> uint256:
+    self.canary = v
+    return 0
+
+@external
+def value_then_gas() -> uint256:
+    success: bool = raw_call(
+        0x0000000000000000000000000000000000000004,
+        b"",
+        max_outsize=0,
+        value=self.set_value(1),
+        gas=self.set_gas(2),
+        revert_on_failure=False,
+    )
+    return self.canary
+
+@external
+def gas_then_value() -> uint256:
+    success: bool = raw_call(
+        0x0000000000000000000000000000000000000004,
+        b"",
+        max_outsize=0,
+        gas=self.set_gas(1),
+        value=self.set_value(2),
+        revert_on_failure=False,
+    )
+    return self.canary
+    """
+    c = get_contract(code)
+
+    assert c.value_then_gas() == 2
+    assert c.gas_then_value() == 2
+
+
 def test_raw_call_clean_mem_kwargs_value(get_contract, env):
     # test msize uses clean memory and does not get overwritten by
     # any raw_call() kwargs

tests/unit/compiler/venom/test_builtin_kwargs.py

@@ -1,26 +1,82 @@
 import pytest
 
 from vyper import ast as vy_ast
-from vyper.codegen_venom.builtins.abi import _get_bool_kwarg as get_abi_bool_kwarg
-from vyper.codegen_venom.builtins.misc import _get_bool_kwarg as get_misc_bool_kwarg
+from vyper.codegen_venom.builtins._kwargs import (
+    get_bool_kwarg,
+    get_kwarg_ast_constants,
+    get_literal_kwarg,
+    kwarg_is_provided,
+    validate_kwargs,
+)
 from vyper.exceptions import CompilerPanic
 
 
 def _call_node(source):
     return vy_ast.parse_to_ast(source).body[0].value
 
 
-@pytest.mark.parametrize("get_bool_kwarg", [get_abi_bool_kwarg, get_misc_bool_kwarg])
-def test_bool_kwarg_uses_reduced_value(get_bool_kwarg):
+def test_kwarg_ast_constants_return_folded_nodes():
     call_node = _call_node("foo(flag=FLAG)")
     call_node.keywords[0].value._set_folded_value(vy_ast.NameConstant(value=False))
 
-    assert get_bool_kwarg(call_node, "flag", True) is False
+    constants = get_kwarg_ast_constants(call_node, ("flag",))
 
+    assert constants["flag"].value is False
 
-@pytest.mark.parametrize("get_bool_kwarg", [get_abi_bool_kwarg, get_misc_bool_kwarg])
-def test_bool_kwarg_rejects_unreduced_value(get_bool_kwarg):
+
+def test_kwarg_ast_constants_reject_unfolded_values():
+    call_node = _call_node("foo(flag=FLAG)")
+
+    with pytest.raises(CompilerPanic, match="unfoldable constant kwarg: flag"):
+        get_kwarg_ast_constants(call_node, ("flag",))
+
+
+def test_kwarg_helpers_reject_unexpected_kwargs():
+    call_node = _call_node("foo(flag=False)")
+
+    with pytest.raises(CompilerPanic, match="unexpected kwarg: flag"):
+        validate_kwargs(call_node, ("other",))
+
+
+def test_kwarg_helpers_reject_duplicate_kwargs():
+    call_node = _call_node("foo(flag=False, flag=True)")
+
+    with pytest.raises(CompilerPanic, match="duplicate kwarg: flag"):
+        validate_kwargs(call_node, ("flag",))
+
+
+def test_kwarg_is_provided():
+    call_node = _call_node("foo(flag=False)")
+
+    assert kwarg_is_provided(call_node, "flag") is True
+    assert kwarg_is_provided(call_node, "other") is False
+
+
+def test_bool_kwarg_uses_reduced_value():
     call_node = _call_node("foo(flag=FLAG)")
+    call_node.keywords[0].value._set_folded_value(vy_ast.NameConstant(value=False))
+    constants = get_kwarg_ast_constants(call_node, ("flag",))
+
+    assert get_bool_kwarg(constants, "flag", True) is False
+
+
+def test_bool_kwarg_rejects_unreduced_value():
+    call_node = _call_node("foo(flag=FLAG)")
+
+    with pytest.raises(CompilerPanic, match="unfoldable constant kwarg: flag"):
+        get_kwarg_ast_constants(call_node, ("flag",))
+
+
+def test_literal_kwarg_uses_reduced_value():
+    call_node = _call_node("foo(revert_on_failure=REVERT)")
+    call_node.keywords[0].value._set_folded_value(vy_ast.NameConstant(value=False))
+    constants = get_kwarg_ast_constants(call_node, ("revert_on_failure",))
+
+    assert get_literal_kwarg(constants, "revert_on_failure", True) is False
+
+
+def test_literal_kwarg_rejects_unreduced_value():
+    call_node = _call_node("foo(revert_on_failure=REVERT)")
 
-    with pytest.raises(CompilerPanic, match="unfoldable boolean kwarg: flag"):
-        get_bool_kwarg(call_node, "flag", True)
+    with pytest.raises(CompilerPanic, match="unfoldable constant kwarg: revert_on_failure"):
+        get_kwarg_ast_constants(call_node, ("revert_on_failure",))

vyper/codegen_venom/builtins/_kwargs.py

@@ -0,0 +1,100 @@
+from __future__ import annotations
+
+from collections.abc import Iterable
+from typing import Any
+
+from vyper import ast as vy_ast
+from vyper.exceptions import CompilerPanic
+
+
+_UNSET = object()
+
+
+def validate_kwargs(node: vy_ast.Call, allowed_kwarg_names: Iterable[str]) -> None:
+    seen = set()
+    for kw in node.keywords:
+        if kw.arg in seen:  # pragma: nocover
+            raise CompilerPanic(f"duplicate kwarg: {kw.arg}", kw)
+        seen.add(kw.arg)
+
+    allowed_kwarg_names = set(allowed_kwarg_names)
+    for kw in node.keywords:
+        if kw.arg not in allowed_kwarg_names:  # pragma: nocover
+            raise CompilerPanic(f"unexpected kwarg: {kw.arg}", kw)
+
+
+def kwarg_is_provided(node: vy_ast.Call, kwarg_name: str) -> bool:
+    return any(kw.arg == kwarg_name for kw in node.keywords)
+
+
+def get_kwarg_ast_constants(
+    node: vy_ast.Call,
+    kwarg_names: Iterable[str],
+    error_prefix: str = "unfoldable constant kwarg",
+) -> dict[str, vy_ast.Constant]:
+    kwarg_names = set(kwarg_names)
+    ret = {}
+    for kw in node.keywords:
+        if kw.arg not in kwarg_names:
+            continue
+
+        kw_node = kw.value.reduced()
+        if not isinstance(kw_node, vy_ast.Constant):  # pragma: nocover
+            raise CompilerPanic(f"{error_prefix}: {kw.arg}", kw_node)
+        ret[kw.arg] = kw_node
+
+    return ret
+
+
+def get_kwarg_values(
+    node: vy_ast.Call,
+    ctx,
+    kwarg_names: Iterable[str],
+):
+    from vyper.codegen_venom.expr import Expr
+
+    kwarg_names = set(kwarg_names)
+    ret = {}
+    for kw in node.keywords:
+        if kw.arg in kwarg_names:
+            ret[kw.arg] = Expr(kw.value.reduced(), ctx).lower_value()
+    return ret
+
+
+def _literal_value(node: vy_ast.VyperNode) -> Any:
+    if isinstance(node, vy_ast.Int):
+        return node.value
+    if isinstance(node, vy_ast.NameConstant):
+        return node.value
+    return _UNSET
+
+
+def get_bool_kwarg(
+    kwarg_constants: dict[str, vy_ast.Constant],
+    kwarg_name: str,
+    default: bool,
+) -> bool:
+    kw_node = kwarg_constants.get(kwarg_name)
+    if kw_node is None:
+        return default
+    if isinstance(kw_node, vy_ast.NameConstant):
+        return kw_node.value
+    if isinstance(kw_node, vy_ast.Int):
+        return bool(kw_node.value)
+    raise CompilerPanic(f"unfoldable boolean kwarg: {kwarg_name}", kw_node)
+
+
+def get_literal_kwarg(
+    kwarg_constants: dict[str, vy_ast.Constant],
+    kwarg_name: str,
+    default,
+):
+    kw_node = kwarg_constants.get(kwarg_name)
+    if kw_node is None:
+        return default
+
+    value = _literal_value(kw_node)
+    if value is not _UNSET:
+        return value
+
+    raise CompilerPanic(f"unfoldable literal kwarg: {kwarg_name}", kw_node)

vyper/codegen_venom/builtins/abi.py

@@ -14,8 +14,12 @@
 from vyper.codegen.core import calculate_type_for_external_return
 from vyper.codegen_venom.abi import abi_decode_to_buf, abi_encode_to_buf
 from vyper.codegen_venom.buffer import Buffer, Ptr
+from vyper.codegen_venom.builtins._kwargs import (
+    get_bool_kwarg,
+    get_kwarg_ast_constants,
+    validate_kwargs,
+)
 from vyper.codegen_venom.value import VyperValue
-from vyper.exceptions import CompilerPanic
 from vyper.semantics.data_locations import DataLocation
 from vyper.semantics.types import BytesT, TupleT
 from vyper.utils import fourbytes_to_int
@@ -25,34 +29,17 @@
     from vyper.codegen_venom.context import VenomCodegenContext
 
 
-def _get_kwarg_value(node: vy_ast.Call, kwarg_name: str, default=None):
-    """Extract a keyword argument value from a Call node."""
-    for kw in node.keywords:
-        if kw.arg == kwarg_name:
-            return kw.value
-    return default
-
-
-def _get_bool_kwarg(node: vy_ast.Call, kwarg_name: str, default: bool) -> bool:
-    """Extract a boolean keyword argument (must be literal)."""
-    kw_node = _get_kwarg_value(node, kwarg_name)
-    if kw_node is None:
-        return default
-    kw_node = kw_node.reduced()
-    # The value should be a NameConstant (True/False)
-    if isinstance(kw_node, vy_ast.NameConstant):
-        return kw_node.value
-    # Could also be an Int with constant value
-    if isinstance(kw_node, vy_ast.Int):
-        return bool(kw_node.value)
-    raise CompilerPanic(f"unfoldable boolean kwarg: {kwarg_name}", kw_node)
+_ABI_ENCODE_KWARGS = ("ensure_tuple", "method_id")
+_ABI_DECODE_KWARGS = ("unwrap_tuple",)
 
 
 def _parse_method_id(method_id_node: vy_ast.VyperNode) -> Optional[int]:
     """Parse method_id kwarg to integer."""
     if method_id_node is None:
         return None
 
+    method_id_node = method_id_node.reduced()
+
     # Handle bytes literal: method_id=0xaabbccdd
     if isinstance(method_id_node, vy_ast.Hex):
         hex_val = method_id_node.value
@@ -69,17 +56,6 @@ def _parse_method_id(method_id_node: vy_ast.VyperNode) -> Optional[int]:
     if isinstance(method_id_node, vy_ast.Int):
         return method_id_node.value
 
-    # If it has a folded value (constant expression)
-    if hasattr(method_id_node, "_metadata") and "folded_value" in method_id_node._metadata:
-        folded = method_id_node._metadata["folded_value"]
-        if isinstance(folded, vy_ast.Bytes):
-            return fourbytes_to_int(folded.value)
-        if isinstance(folded, vy_ast.Hex):
-            hex_val = folded.value
-            if isinstance(hex_val, str):
-                hex_str = hex_val[2:] if hex_val.startswith("0x") else hex_val
-                return fourbytes_to_int(bytes.fromhex(hex_str))
-
     return None
 
 
@@ -120,8 +96,10 @@ def lower_abi_encode(node: vy_ast.Call, ctx: VenomCodegenContext) -> VyperValue:
     b = ctx.builder
 
     # Parse kwargs
-    ensure_tuple = _get_bool_kwarg(node, "ensure_tuple", default=True)
-    method_id_node = _get_kwarg_value(node, "method_id")
+    validate_kwargs(node, _ABI_ENCODE_KWARGS)
+    kwarg_constants = get_kwarg_ast_constants(node, _ABI_ENCODE_KWARGS)
+    ensure_tuple = get_bool_kwarg(kwarg_constants, "ensure_tuple", default=True)
+    method_id_node = kwarg_constants.get("method_id")
     method_id = _parse_method_id(method_id_node)
 
     # Evaluate all args - primitives get values, complex types get pointers
@@ -201,7 +179,9 @@ def lower_abi_decode(node: vy_ast.Call, ctx: VenomCodegenContext) -> VyperValue:
     # Parse args
     data_node = node.args[0]
     output_type_node = node.args[1]
-    unwrap_tuple = _get_bool_kwarg(node, "unwrap_tuple", default=True)
+    validate_kwargs(node, _ABI_DECODE_KWARGS)
+    kwarg_constants = get_kwarg_ast_constants(node, _ABI_DECODE_KWARGS)
+    unwrap_tuple = get_bool_kwarg(kwarg_constants, "unwrap_tuple", default=True)
 
     # Get output type from type annotation
     output_typ = output_type_node._metadata["type"].typedef
@@ -239,7 +219,11 @@ def lower_abi_decode(node: vy_ast.Call, ctx: VenomCodegenContext) -> VyperValue:
 
     # Decode with bounds checking
     hi = b.add(data_ptr, data_len)
-    buf = Buffer(_ptr=data_ptr, size=wrapped_typ.memory_bytes_required, annotation="abi_decode_src")
+    buf = Buffer(
+        _ptr=data_ptr,
+        size=wrapped_typ.memory_bytes_required,
+        annotation="abi_decode_src",
+    )
     ptr = Ptr(operand=data_ptr, location=DataLocation.MEMORY, buf=buf)
     src = VyperValue.from_ptr(ptr, wrapped_typ)
     abi_decode_to_buf(ctx, output_val.operand, src, hi=hi)