feat[lang]!: ban nonreentrant on internal functions

tests/functional/codegen/modules/test_nonreentrant.py

@@ -34,45 +34,3 @@ def __default__():
     assert c.foo() == 5
     with tx_failed():
         c.re_enter()
-
-
-def test_internal_nonreentrant(make_input_bundle, get_contract, tx_failed):
-    lib1 = """
-interface Foo:
-    def foo() -> uint256: nonpayable
-
-implements: Foo
-
-@external
-def foo() -> uint256:
-    return self._safe_fn()
-
-@internal
-@nonreentrant
-def _safe_fn() -> uint256:
-    return 10
-    """
-    main = """
-import lib1
-
-initializes: lib1
-
-exports: lib1.foo
-
-@external
-@nonreentrant
-def re_enter():
-    extcall lib1.Foo(self).foo()  # should always throw
-
-@external
-def __default__():
-    # sanity: make sure we don't revert due to bad selector
-    pass
-    """
-
-    input_bundle = make_input_bundle({"lib1.vy": lib1})
-
-    c = get_contract(main, input_bundle=input_bundle)
-    assert c.foo() == 10
-    with tx_failed():
-        c.re_enter()

tests/functional/syntax/modules/test_initializers.py

@@ -1306,50 +1306,23 @@ def nonreentrant_library_bundle(make_input_bundle):
     # test simple case
     lib1 = """
 # lib1.vy
-@internal
-@nonreentrant
-def bar():
-    pass
-
-# lib1.vy
 @external
 @nonreentrant
 def ext_bar():
     pass
     """
-    # test case with recursion
+    # test case with exports
     lib2 = """
-@internal
-def bar():
-    self.baz()
-
-@external
-def ext_bar():
-    self.baz()
-
-@nonreentrant
-@internal
-def baz():
-    return
-    """
-    # test case with nested recursion
-    lib3 = """
 import lib1
 uses: lib1
 
-@internal
-def bar():
-    lib1.bar()
-
-@external
-def ext_bar():
-    lib1.bar()
+exports: lib1.ext_bar
     """
 
-    return make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3})
+    return make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2})
 
 
-@pytest.mark.parametrize("lib", ("lib1", "lib2", "lib3"))
+@pytest.mark.parametrize("lib", ("lib1", "lib2"))
 def test_nonreentrant_exports(nonreentrant_library_bundle, lib):
     main = f"""
 import {lib}
@@ -1368,24 +1341,6 @@ def foo():
     assert e.value.annotations[0].lineno == 4
 
 
-@pytest.mark.parametrize("lib", ("lib1", "lib2", "lib3"))
-def test_internal_nonreentrant_import(nonreentrant_library_bundle, lib):
-    main = f"""
-import {lib}
-
-@external
-def foo():
-    {lib}.bar()  # line 6
-    """
-    with pytest.raises(ImmutableViolation) as e:
-        compile_code(main, input_bundle=nonreentrant_library_bundle)
-    assert e.value._message == f"Cannot access `{lib}` state!" + NONREENTRANT_NOTE
-
-    hint = f"add `uses: {lib}` or `initializes: {lib}` as a top-level statement to your contract"
-    assert e.value._hint == hint
-    assert e.value.annotations[0].lineno == 6
-
-
 def test_global_initialize_missed_import_hint(make_input_bundle, chdir_tmp_path):
     lib1 = """
 import lib2

tests/functional/syntax/signatures/test_invalid_function_decorators.py

@@ -42,6 +42,25 @@ def __init__():
     """,
         FunctionDeclarationException,
     ),
+    (
+        # nonreentrant on an internal function
+        """
+@nonreentrant
+def foo():
+    pass
+    """,
+        FunctionDeclarationException,
+    ),
+    (
+        # nonreentrant on an internal function
+        """
+@internal
+@nonreentrant
+def foo():
+    pass
+    """,
+        FunctionDeclarationException,
+    ),
 ]
 
 

tests/unit/cli/storage_layout/test_storage_layout.py

@@ -18,9 +18,6 @@ def public_foo1():
 def public_foo2():
     pass
 
-
-@internal
-@nonreentrant
 def _bar():
     pass
 

tests/unit/cli/storage_layout/test_storage_layout_overrides.py

@@ -68,7 +68,6 @@ def public_foo2():
 
 
 @internal
-@nonreentrant
 def _bar():
     pass
 

vyper/codegen/function_definitions/common.py

@@ -150,6 +150,8 @@ def get_nonreentrant_lock(func_t):
     if not func_t.nonreentrant:
         return ["pass"], ["pass"]
 
+    assert func_t.is_external
+
     nkey = func_t.reentrancy_key_position.position
 
     LOAD, STORE = "sload", "sstore"

vyper/semantics/types/function.py

@@ -435,9 +435,10 @@ def from_FunctionDef(cls, funcdef: vy_ast.FunctionDef) -> "ContractFunctionT":
                 raise FunctionDeclarationException(
                     "Constructor may not use default arguments", funcdef.args.defaults[0]
                 )
-            if decorators.nonreentrant:
-                msg = "`@nonreentrant` decorator disallowed on `__init__`"
-                raise FunctionDeclarationException(msg, decorators.nonreentrant_node)
+
+        if function_visibility != FunctionVisibility.EXTERNAL and decorators.nonreentrant:
+            msg = f"`@nonreentrant` decorator disallowed on {function_visibility} functions!"
+            raise FunctionDeclarationException(msg, decorators.nonreentrant_node)
 
         return cls(
             funcdef.name,