feat[lang]!: make @external modifier optional in .vyi files

tests/functional/codegen/test_interfaces.py

@@ -695,3 +695,34 @@ def test_call(a: address, b: {type_str}) -> {type_str}:
     make_file("jsonabi.json", json.dumps(convert_v1_abi(abi)))
     c3 = get_contract(code, input_bundle=input_bundle)
     assert c3.test_call(c1.address, value) == value
+
+
+def test_interface_function_without_visibility(make_input_bundle, get_contract):
+    interface_code = """
+def foo() -> uint256:
+    ...
+
+@external
+def bar() -> uint256:
+    ...
+    """
+
+    code = """
+import a as FooInterface
+
+implements: FooInterface
+
+@external
+def foo() -> uint256:
+    return 1
+
+@external
+def bar() -> uint256:
+    return 1
+    """
+
+    input_bundle = make_input_bundle({"a.vyi": interface_code})
+
+    c = get_contract(code, input_bundle=input_bundle)
+
+    assert c.foo() == c.bar() == 1

tests/functional/syntax/test_interfaces.py

@@ -3,6 +3,7 @@
 from vyper import compiler
 from vyper.exceptions import (
     ArgumentException,
+    FunctionDeclarationException,
     InterfaceViolation,
     InvalidReference,
     InvalidType,
@@ -421,3 +422,92 @@ def test_builtins_not_found2(erc):
         compiler.compile_code(code)
     assert e.value._message == f"ethereum.ercs.{erc}"
     assert e.value._hint == f"try renaming `{erc}` to `I{erc}`"
+
+
+invalid_visibility_code = [
+    """
+import foo as Foo
+
+implements: Foo
+
+@external
+def foobar():
+    pass
+    """,
+    """
+import foo as Foo
+
+implements: Foo
+
+@internal
+def foobar():
+    pass
+    """,
+    """
+import foo as Foo
+
+implements: Foo
+
+def foobar():
+    pass
+    """,
+]
+
+
+@pytest.mark.parametrize("code", invalid_visibility_code)
+def test_internal_visibility_in_interface(make_input_bundle, code):
+    interface_code = """
+@internal
+def foobar():
+    ...
+"""
+
+    input_bundle = make_input_bundle({"foo.vyi": interface_code})
+
+    with pytest.raises(FunctionDeclarationException) as e:
+        compiler.compile_code(code, input_bundle=input_bundle)
+
+    assert e.value._message == "Interface functions can only be marked as `@external`"
+
+
+external_visibility_interface = [
+    """
+@external
+def foobar():
+    ...
+
+def bar():
+    ...
+    """,
+    """
+def foobar():
+    ...
+
+@external
+def bar():
+    ...
+    """,
+]
+
+
+@pytest.mark.parametrize("iface", external_visibility_interface)
+def test_internal_implemenatation_of_external_interface(make_input_bundle, iface):
+    input_bundle = make_input_bundle({"foo.vyi": iface})
+
+    code = """
+import foo as Foo
+
+implements: Foo
+
+@internal
+def foobar():
+    pass
+
+def bar():
+    pass
+    """
+
+    with pytest.raises(InterfaceViolation) as e:
+        compiler.compile_code(code, input_bundle=input_bundle)
+
+    assert e.value.message == "Contract does not implement all interface functions: bar(), foobar()"

vyper/semantics/analysis/module.py

@@ -193,7 +193,7 @@ def __init__(
         self._imported_modules: dict[PurePath, vy_ast.VyperNode] = {}
 
         # keep track of exported functions to prevent duplicate exports
-        self._exposed_functions: dict[ContractFunctionT, vy_ast.VyperNode] = {}
+        self._all_functions: dict[ContractFunctionT, vy_ast.VyperNode] = {}
 
         self._events: list[EventT] = []
 
@@ -414,7 +414,7 @@ def visit_ImplementsDecl(self, node):
             raise StructureException(msg, node.annotation, hint=hint)
 
         # grab exposed functions
-        funcs = self._exposed_functions
+        funcs = self._all_functions
         type_.validate_implements(node, funcs)
 
         node._metadata["interface_type"] = type_
@@ -608,10 +608,10 @@ def _self_t(self):
     def _add_exposed_function(self, func_t, node, relax=True):
         # call this before self._self_t.typ.add_member() for exception raising
         # priority
-        if not relax and (prev_decl := self._exposed_functions.get(func_t)) is not None:
+        if not relax and (prev_decl := self._all_functions.get(func_t)) is not None:
             raise StructureException("already exported!", node, prev_decl=prev_decl)
 
-        self._exposed_functions[func_t] = node
+        self._all_functions[func_t] = node
 
     def visit_VariableDecl(self, node):
         # postcondition of VariableDecl.validate

vyper/semantics/types/function.py

@@ -327,10 +327,17 @@ def from_vyi(cls, funcdef: vy_ast.FunctionDef) -> "ContractFunctionT":
         -------
         ContractFunctionT
         """
-        function_visibility, state_mutability, nonreentrant = _parse_decorators(funcdef)
+        function_visibility, state_mutability, nonreentrant = _parse_decorators(
+            funcdef, is_interface=True
+        )
+
+        assert not nonreentrant
+
+        # it's redundant to specify visibility in vyi - always should be external
+        if function_visibility is None:
+            function_visibility = FunctionVisibility.EXTERNAL
 
-        if nonreentrant:
-            raise FunctionDeclarationException("`@nonreentrant` not allowed in interfaces", funcdef)
+        assert function_visibility == FunctionVisibility.EXTERNAL
 
         if funcdef.name == "__init__":
             raise FunctionDeclarationException("Constructors cannot appear in interfaces", funcdef)
@@ -377,6 +384,10 @@ def from_FunctionDef(cls, funcdef: vy_ast.FunctionDef) -> "ContractFunctionT":
         """
         function_visibility, state_mutability, nonreentrant = _parse_decorators(funcdef)
 
+        # it's redundant to specify internal visibility - it's implied by not being external
+        if function_visibility is None:
+            function_visibility = FunctionVisibility.INTERNAL
+
         positional_args, keyword_args = _parse_args(funcdef)
 
         return_type = _parse_return_type(funcdef)
@@ -503,7 +514,10 @@ def implements(self, other: "ContractFunctionT") -> bool:
         if return_type and not return_type.compare_type(other_return_type):  # type: ignore
             return False
 
-        return self.mutability == other.mutability
+        if self.mutability != other.mutability:
+            return False
+
+        return self.visibility == other.visibility
 
     @cached_property
     def default_values(self) -> dict[str, vy_ast.VyperNode]:
@@ -695,8 +709,8 @@ def _parse_return_type(funcdef: vy_ast.FunctionDef) -> Optional[VyperType]:
 
 
 def _parse_decorators(
-    funcdef: vy_ast.FunctionDef,
-) -> tuple[FunctionVisibility, StateMutability, bool]:
+    funcdef: vy_ast.FunctionDef, is_interface: bool = False
+) -> tuple[FunctionVisibility | None, StateMutability, bool]:
     function_visibility = None
     state_mutability = None
     nonreentrant_node = None
@@ -714,6 +728,10 @@ def _parse_decorators(
         if decorator.get("id") == "nonreentrant":
             if nonreentrant_node is not None:
                 raise StructureException("nonreentrant decorator is already set", nonreentrant_node)
+            if is_interface:
+                raise FunctionDeclarationException(
+                    "`@nonreentrant` not allowed in interfaces", funcdef
+                )
 
             if funcdef.name == "__init__":
                 msg = "`@nonreentrant` decorator disallowed on `__init__`"
@@ -729,6 +747,12 @@ def _parse_decorators(
                         decorator,
                         hint="only one visibility decorator is allowed per function",
                     )
+
+                if is_interface and FunctionVisibility(decorator.id) != FunctionVisibility.EXTERNAL:
+                    raise FunctionDeclarationException(
+                        "Interface functions can only be marked as `@external`", decorator
+                    )
+
                 function_visibility = FunctionVisibility(decorator.id)
 
             elif StateMutability.is_valid_value(decorator.id):
@@ -751,9 +775,6 @@ def _parse_decorators(
         else:
             raise StructureException("Bad decorator syntax", decorator)
 
-    if function_visibility is None:
-        function_visibility = FunctionVisibility.INTERNAL
-
     if state_mutability is None:
         # default to nonpayable
         state_mutability = StateMutability.NONPAYABLE

vyper/semantics/types/module.py

@@ -107,7 +107,8 @@ def _ctor_modifiability_for_call(self, node: vy_ast.Call, modifiability: Modifia
     def validate_implements(
         self, node: vy_ast.ImplementsDecl, functions: dict[ContractFunctionT, vy_ast.VyperNode]
     ) -> None:
-        fns_by_name = {fn_t.name: fn_t for fn_t in functions.keys()}
+        # only external functions can implement interfaces
+        fns_by_name = {fn_t.name: fn_t for fn_t in functions.keys() if fn_t.is_external}
 
         unimplemented = []
 
@@ -117,6 +118,7 @@ def _is_function_implemented(fn_name, fn_type):
 
             to_compare = fns_by_name[fn_name]
             assert isinstance(to_compare, ContractFunctionT)
+            assert isinstance(fn_type, ContractFunctionT)
 
             return to_compare.implements(fn_type)