From 6dad0f8dd9169c2d2fe3f8b1916f0a97533ca792 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 20 May 2024 20:19:07 -0400 Subject: [PATCH 01/15] feat[lang]: add linearization check for initializers add a check that each init function is called after its dependencies misc/refactor: - additional rewrite of a few lines without changing semantics --- .../syntax/modules/test_initializers.py | 48 ++++++++++++++++++- vyper/semantics/analysis/module.py | 16 +++++-- 2 files changed, 59 insertions(+), 5 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 29d611d54a..0c1cabd093 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -197,10 +197,10 @@ def foo(): @deploy def __init__(): - lib2.__init__() # demonstrate we can call lib1.__init__ through lib2.lib1 # (not sure this should be allowed, really. lib2.lib1.__init__() + lib2.__init__() """ input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2}) @@ -238,6 +238,52 @@ def __init__(): assert compile_code(main, input_bundle=input_bundle) is not None +def test_initialize_wrong_order(make_input_bundle): + lib1 = """ +counter: uint256 + +@deploy +def __init__(): + pass + """ + lib2 = """ +import lib1 + +uses: lib1 + +counter: uint256 + +@deploy +def __init__(): + pass + +@internal +def foo(): + lib1.counter += 1 + """ + main = """ +import lib1 +import lib2 + +initializes: lib2[lib1 := lib1] +initializes: lib1 + +@deploy +def __init__(): + lib2.__init__() + lib1.__init__() + """ + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2}) + + with pytest.raises(InitializerException) as e: + assert compile_code(main, input_bundle=input_bundle) is not None + + expected = "Tried to initialize `lib2`, but it depends on `lib1`, " + expected += "which has not been initialized yet." + assert e.value._message == expected + assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." + + def test_imported_as_different_names(make_input_bundle): lib1 = """ counter: uint256 diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 63eafdbaf4..6bd442a82e 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -331,10 +331,7 @@ def validate_initialized_modules(self): # don't call `__init__()` for modules which don't have # `__init__()` function for m in should_initialize.copy(): - for f in m.functions.values(): - if f.is_constructor: - break - else: + if not any(f.is_constructor for f in m.functions.values()): del should_initialize[m] init_calls = [] @@ -375,6 +372,17 @@ def validate_initialized_modules(self): hint += "as a top-level statement to your contract" raise InitializerException(msg, call_node.func, hint=hint) + initializer_info = should_initialize[initialized_module.module_t] + for dep_info in initializer_info.dependencies: # type: ModuleInfo + dep_t = dep_info.module_t + if dep_t not in seen_initializers and dep_t.init_function is not None: + msg = f"Tried to initialize `{initialized_module.alias}`, " + msg += f"but it depends on `{dep_info.alias}`, which has not " + msg += "been initialized yet." + hint = f"call `{dep_info.alias}.__init__()` before " + hint += f"`{initialized_module.alias}.__init__()`." + raise InitializerException(msg, call_node.func, hint=hint) + del should_initialize[initialized_module.module_t] seen_initializers[initialized_module.module_t] = call_node.func From 132b4876bd045bead1d97c46da54cd35948e757f Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 07:05:00 -0400 Subject: [PATCH 02/15] use a better check --- vyper/semantics/analysis/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 6bd442a82e..edc0442c6c 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -331,7 +331,7 @@ def validate_initialized_modules(self): # don't call `__init__()` for modules which don't have # `__init__()` function for m in should_initialize.copy(): - if not any(f.is_constructor for f in m.functions.values()): + if m.init_function is None: del should_initialize[m] init_calls = [] From 349a39bed4d2913e692bcec75ee1ef6aa2fa851e Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 07:06:56 -0400 Subject: [PATCH 03/15] update another is_constructor loop --- vyper/semantics/analysis/module.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index edc0442c6c..2dba89d8ba 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -335,10 +335,8 @@ def validate_initialized_modules(self): del should_initialize[m] init_calls = [] - for f in self.ast.get_children(vy_ast.FunctionDef): - if f._metadata["func_type"].is_constructor: - init_calls = f.get_descendants(vy_ast.Call) - break + if module_t.init_function is not None: + init_calls = module_t.init_function.ast_def.get_descendants(vy_ast.Call) seen_initializers = {} for call_node in init_calls: From 0a69f6e58df6b4390a3a1aeb59d3ff404738d6ce Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 08:28:40 -0400 Subject: [PATCH 04/15] add test for nested case --- .../syntax/modules/test_initializers.py | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 0c1cabd093..f8adf886c6 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -283,6 +283,57 @@ def __init__(): assert e.value._message == expected assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." +def test_initializer_order_nested(make_input_bundle): + lib1 = """ +a: public(uint256) + +@deploy +@payable +def __init__(x: uint256): + self.a = x + """ + lib2 = """ +import lib1 + +uses: lib1 + +a: uint256 + +@deploy +def __init__(): + # not initialized when called + self.a = lib1.a + """ + lib3 = """ +import lib1 + +initializes: lib1 + +a: uint256 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + lib1.__init__(0) + """ + main = """ +import lib1 +import lib2 +import lib3 + +initializes: lib2[lib1 := lib1] +initializes: lib3 + +@deploy +def __init__(): + lib3.__init__(0) + lib2.__init__() + """ + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) + + assert compile_code(main, input_bundle=input_bundle) is not None + def test_imported_as_different_names(make_input_bundle): lib1 = """ From 47d05a4a81ca4d8a01025c60272fba08849c1df3 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 08:24:57 -0400 Subject: [PATCH 05/15] fix recursion for init functions --- .../syntax/modules/test_initializers.py | 1 + vyper/semantics/analysis/module.py | 22 +++++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index f8adf886c6..9f1f7808b5 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -283,6 +283,7 @@ def __init__(): assert e.value._message == expected assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." + def test_initializer_order_nested(make_input_bundle): lib1 = """ a: public(uint256) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index e57a7db5c1..80ab507099 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -325,9 +325,11 @@ def validate_used_modules(self): err_list.raise_if_not_empty() def validate_initialized_modules(self): - # check all `initializes:` modules have `__init__()` called exactly once + # check all `initializes:` modules have `__init__()` called exactly once, + # and check they are called in dependency order module_t = self.ast._metadata["type"] should_initialize = {t.module_info.module_t: t for t in module_t.initialized_modules} + # don't call `__init__()` for modules which don't have # `__init__()` function for m in should_initialize.copy(): @@ -338,7 +340,12 @@ def validate_initialized_modules(self): if module_t.init_function is not None: init_calls = module_t.init_function.ast_def.get_descendants(vy_ast.Call) + # map of seen __init__() function calls seen_initializers = {} + + # modules which were already initialized in another module + already_initialized = OrderedSet() + for call_node in init_calls: expr_info = call_node.func._expr_info if expr_info is None: @@ -373,7 +380,11 @@ def validate_initialized_modules(self): initializer_info = should_initialize[initialized_module.module_t] for dep_info in initializer_info.dependencies: # type: ModuleInfo dep_t = dep_info.module_t - if dep_t not in seen_initializers and dep_t.init_function is not None: + + if dep_t in already_initialized or dep_t.init_function is None: + continue + + if dep_t not in seen_initializers: msg = f"Tried to initialize `{initialized_module.alias}`, " msg += f"but it depends on `{dep_info.alias}`, which has not " msg += "been initialized yet." @@ -384,6 +395,13 @@ def validate_initialized_modules(self): del should_initialize[initialized_module.module_t] seen_initializers[initialized_module.module_t] = call_node.func + # add the modules that the module initialized, these are already + # checked in the recursion. + initialized = initialized_module.module_t.initialized_modules + already_initialized.update( + s.module_info.module_t for s in initialized.initialized_modules + ) + if len(should_initialize) > 0: err_list = ExceptionList() for s in should_initialize.values(): From ac2364b721e7759d40f208882e098709b4aef419 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 09:47:06 -0400 Subject: [PATCH 06/15] small polish --- vyper/semantics/analysis/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 80ab507099..8e71f5d15f 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -353,7 +353,7 @@ def validate_initialized_modules(self): # refactor so that range() is properly tagged. continue - call_t = call_node.func._expr_info.typ + call_t = expr_info.typ if not isinstance(call_t, ContractFunctionT): continue From dca48e82b4fc207eba36de1e05dd28efb08d3a71 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 09:53:14 -0400 Subject: [PATCH 07/15] fix typo --- vyper/semantics/analysis/module.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 8e71f5d15f..74b29730b2 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -397,7 +397,7 @@ def validate_initialized_modules(self): # add the modules that the module initialized, these are already # checked in the recursion. - initialized = initialized_module.module_t.initialized_modules + initialized = initialized_module.module_t already_initialized.update( s.module_info.module_t for s in initialized.initialized_modules ) From 496bfdaaa6b48e6548289c55613c6030f2c57403 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 10:23:25 -0400 Subject: [PATCH 08/15] add another test --- .../syntax/modules/test_initializers.py | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 9f1f7808b5..0f759187f9 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -318,7 +318,9 @@ def __init__(x: uint256): self.a = x lib1.__init__(0) """ - main = """ + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) + + main1 = """ import lib1 import lib2 import lib3 @@ -331,9 +333,29 @@ def __init__(): lib3.__init__(0) lib2.__init__() """ - input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) + assert compile_code(main1, input_bundle=input_bundle) is not None - assert compile_code(main, input_bundle=input_bundle) is not None + main2 = """ +import lib1 +import lib2 +import lib3 + +initializes: lib2[lib1 := lib1] +initializes: lib3 + +@deploy +def __init__(): + lib2.__init__() # opposite order! + lib3.__init__(0) + """ + with pytest.raises(InitializerException) as e: + compile_code(main2, input_bundle=input_bundle) + + expected = "Tried to initialize `lib2`, but it depends on `lib1`, which " + expected += "has not been initialized yet." + assert e.value._message == expected + + assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." def test_imported_as_different_names(make_input_bundle): From 2c0e558cc4d611532aaa112d09d7cb6425e66fbc Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 13:05:06 -0400 Subject: [PATCH 09/15] formatting --- vyper/semantics/analysis/module.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 74b29730b2..00a5bab361 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -386,8 +386,8 @@ def validate_initialized_modules(self): if dep_t not in seen_initializers: msg = f"Tried to initialize `{initialized_module.alias}`, " - msg += f"but it depends on `{dep_info.alias}`, which has not " - msg += "been initialized yet." + msg += f"but it depends on `{dep_info.alias}`, which has " + msg += "not been initialized yet." hint = f"call `{dep_info.alias}.__init__()` before " hint += f"`{initialized_module.alias}.__init__()`." raise InitializerException(msg, call_node.func, hint=hint) From 9a365e2fb9124729a98311631e0bcff35e1e95f3 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 13:15:54 -0400 Subject: [PATCH 10/15] add recursion test --- .../syntax/modules/test_initializers.py | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 0f759187f9..0df8531c42 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -357,6 +357,76 @@ def __init__(): assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." +def test_initializer_nested_order2(make_input_bundle): + lib1 = """ +import lib4 + +a: public(uint256) + +initializes: lib4 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + lib4.__init__(x) + """ + + lib2 = """ +import lib1 +import lib4 + +uses: lib1 +uses: lib4 + +a: uint256 + +@deploy +def __init__(): + # not initialized when called + self.a = lib1.a + lib4.a + """ + + lib3 = """ +import lib1 + +initializes: lib1 + +a: uint256 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + lib1.__init__(0) + """ + lib4 = """ +a: uint256 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + """ + main = """ +import lib1 +import lib2 +import lib3 +import lib4 + +initializes: lib2[lib1 := lib1, lib4 := lib4] +initializes: lib3 + +@deploy +def __init__(): + lib3.__init__(0) + lib2.__init__() + """ + + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3, "lib4.vy": lib4}) + + assert compile_code(main, input_bundle=input_bundle) is not None + def test_imported_as_different_names(make_input_bundle): lib1 = """ From cab34c501fdbcbe2019829b0a49173915528ea44 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Tue, 21 May 2024 13:19:45 -0400 Subject: [PATCH 11/15] add recursion --- .../functional/syntax/modules/test_initializers.py | 5 ++++- vyper/semantics/analysis/module.py | 4 +--- vyper/semantics/types/module.py | 14 ++++++++++++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 0df8531c42..2d6cf4f143 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -357,6 +357,7 @@ def __init__(): assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." + def test_initializer_nested_order2(make_input_bundle): lib1 = """ import lib4 @@ -423,7 +424,9 @@ def __init__(): lib2.__init__() """ - input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3, "lib4.vy": lib4}) + input_bundle = make_input_bundle( + {"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3, "lib4.vy": lib4} + ) assert compile_code(main, input_bundle=input_bundle) is not None diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 00a5bab361..e4cf48a1bd 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -398,9 +398,7 @@ def validate_initialized_modules(self): # add the modules that the module initialized, these are already # checked in the recursion. initialized = initialized_module.module_t - already_initialized.update( - s.module_info.module_t for s in initialized.initialized_modules - ) + already_initialized.update(initialized.initialized_modules_recursed) if len(should_initialize) > 0: err_list = ExceptionList() diff --git a/vyper/semantics/types/module.py b/vyper/semantics/types/module.py index b3e3f2ef2b..9fc9dc4a65 100644 --- a/vyper/semantics/types/module.py +++ b/vyper/semantics/types/module.py @@ -476,15 +476,25 @@ def used_modules(self): ret.append(used_module) return ret - @property + @cached_property def initialized_modules(self): - # modules which are initialized to + # modules which are initialized ret = [] for node in self.initializes_decls: info = node._metadata["initializes_info"] ret.append(info) return ret + @cached_property + def initialized_modules_recursed(self) -> OrderedSet["ModuleT"]: + # modules which are initialized, taking into account recursion + ret: OrderedSet = OrderedSet() + for info in self.initialized_modules: + module_t = info.module_info.module_t + ret |= module_t.initialized_modules_recursed + ret.add(module_t) + return ret + @cached_property def exposed_functions(self): # return external functions that are exposed in the runtime From cea79cfac74b0306fe4ef6225972e9af0797a27a Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 23 May 2024 16:35:06 -0400 Subject: [PATCH 12/15] disallow touching state of `uses` modules in `__init__()` functions --- .../syntax/modules/test_initializers.py | 135 +++--------------- vyper/semantics/analysis/local.py | 41 ++++-- vyper/semantics/analysis/module.py | 4 +- 3 files changed, 51 insertions(+), 129 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 2d6cf4f143..9144ac9999 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -286,149 +286,58 @@ def __init__(): def test_initializer_order_nested(make_input_bundle): lib1 = """ -a: public(uint256) - -@deploy -@payable -def __init__(x: uint256): - self.a = x - """ - lib2 = """ -import lib1 - -uses: lib1 - -a: uint256 - -@deploy -def __init__(): - # not initialized when called - self.a = lib1.a - """ - lib3 = """ -import lib1 - -initializes: lib1 - -a: uint256 - -@deploy -@payable -def __init__(x: uint256): - self.a = x - lib1.__init__(0) - """ - input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) - - main1 = """ -import lib1 import lib2 import lib3 -initializes: lib2[lib1 := lib1] -initializes: lib3 +uses: lib3 +initializes: lib2[lib3 := lib3] @deploy def __init__(): - lib3.__init__(0) lib2.__init__() + lib3.counter += 1 """ - assert compile_code(main1, input_bundle=input_bundle) is not None - - main2 = """ -import lib1 -import lib2 -import lib3 - -initializes: lib2[lib1 := lib1] -initializes: lib3 - -@deploy -def __init__(): - lib2.__init__() # opposite order! - lib3.__init__(0) - """ - with pytest.raises(InitializerException) as e: - compile_code(main2, input_bundle=input_bundle) - - expected = "Tried to initialize `lib2`, but it depends on `lib1`, which " - expected += "has not been initialized yet." - assert e.value._message == expected - - assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." - - -def test_initializer_nested_order2(make_input_bundle): - lib1 = """ -import lib4 - -a: public(uint256) - -initializes: lib4 - -@deploy -@payable -def __init__(x: uint256): - self.a = x - lib4.__init__(x) - """ - lib2 = """ -import lib1 -import lib4 +import lib3 -uses: lib1 -uses: lib4 +uses: lib3 -a: uint256 +counter: uint256 @deploy def __init__(): - # not initialized when called - self.a = lib1.a + lib4.a - """ + self.counter = 1 +@external +def foo() ->uint256: + return lib3.counter + """ lib3 = """ -import lib1 - -initializes: lib1 - -a: uint256 +counter: uint256 @deploy -@payable -def __init__(x: uint256): - self.a = x - lib1.__init__(0) +def __init__(): + self.counter = 1 """ - lib4 = """ -a: uint256 + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) -@deploy -@payable -def __init__(x: uint256): - self.a = x - """ main = """ import lib1 -import lib2 import lib3 -import lib4 -initializes: lib2[lib1 := lib1, lib4 := lib4] +initializes: lib1[lib3 := lib3] initializes: lib3 @deploy def __init__(): - lib3.__init__(0) - lib2.__init__() + lib3.__init__() + lib1.__init__() """ + with pytest.raises(ImmutableViolation) as e: + compile_code(main, input_bundle=input_bundle) - input_bundle = make_input_bundle( - {"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3, "lib4.vy": lib4} - ) - - assert compile_code(main, input_bundle=input_bundle) is not None + assert e.value._message == "Cannot access `lib3` state from `__init__()` function!" + assert e.value._hint == "add `initializes: lib3` as a top-level statement to your contract" def test_imported_as_different_names(make_input_bundle): diff --git a/vyper/semantics/analysis/local.py b/vyper/semantics/analysis/local.py index 26c6a4ef9f..8c6391e682 100644 --- a/vyper/semantics/analysis/local.py +++ b/vyper/semantics/analysis/local.py @@ -253,7 +253,7 @@ def _get_module_chain(node: vy_ast.ExprNode) -> list[ModuleInfo]: return ret -def check_module_uses(node: vy_ast.ExprNode) -> Optional[ModuleInfo]: +def check_module_uses(node: vy_ast.ExprNode, is_deploy: bool = False) -> Optional[ModuleInfo]: """ validate module usage, and that if we use lib1.lib2., that lib1 at least `uses` lib2. @@ -266,19 +266,32 @@ def check_module_uses(node: vy_ast.ExprNode) -> Optional[ModuleInfo]: if len(module_infos) == 0: return None + level = ModuleOwnership.USES + if is_deploy: + level = ModuleOwnership.INITIALIZES + for module_info in module_infos: - if module_info.ownership < ModuleOwnership.USES: - msg = f"Cannot access `{module_info.alias}` state!\n note that" - # CMC 2024-04-12 add UX note about nonreentrant. might be nice - # in the future to be more specific about exactly which state is - # used, although that requires threading a bit more context into - # this function. - msg += " use of the `@nonreentrant` decorator is also considered" - msg += " state access" - - hint = f"add `uses: {module_info.alias}` or " - hint += f"`initializes: {module_info.alias}` as " - hint += "a top-level statement to your contract" + if module_info.ownership < level: + # cannot touch state of `used` modules in ctor + if is_deploy: + msg = f"Cannot access `{module_info.alias}` state from " + msg += "`__init__()` function!" + + hint = f"add `initializes: {module_info.alias}` as " + hint += "a top-level statement to your contract" + else: + msg = f"Cannot access `{module_info.alias}` state!\n note that" + # CMC 2024-04-12 add UX note about nonreentrant. might be nice + # in the future to be more specific about exactly which state is + # used, although that requires threading a bit more context into + # this function. + msg += " use of the `@nonreentrant` decorator is also" + msg += " considered state access" + + hint = f"add `uses: {module_info.alias}` or " + hint += f"`initializes: {module_info.alias}` as " + hint += "a top-level statement to your contract" + raise ImmutableViolation(msg, hint=hint) # the leftmost- referenced module @@ -452,7 +465,7 @@ def _handle_modification(self, target: vy_ast.ExprNode): info._writes.add(var_access) def _handle_module_access(self, target: vy_ast.ExprNode): - root_module_info = check_module_uses(target) + root_module_info = check_module_uses(target, self.func.is_deploy) if root_module_info is not None: # log the access diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index e4cf48a1bd..48d6eca315 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -325,8 +325,8 @@ def validate_used_modules(self): err_list.raise_if_not_empty() def validate_initialized_modules(self): - # check all `initializes:` modules have `__init__()` called exactly once, - # and check they are called in dependency order + # check all `initializes:` modules have `__init__()` called exactly + # once, and check they are called in dependency order module_t = self.ast._metadata["type"] should_initialize = {t.module_info.module_t: t for t in module_t.initialized_modules} From 0605e29432538426eb721ce3eb082eafced1f154 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 2 Jun 2024 09:34:17 -0400 Subject: [PATCH 13/15] Revert "disallow touching state of `uses` modules in `__init__()` functions" This reverts commit cea79cfac74b0306fe4ef6225972e9af0797a27a. --- .../syntax/modules/test_initializers.py | 135 +++++++++++++++--- vyper/semantics/analysis/local.py | 41 ++---- vyper/semantics/analysis/module.py | 4 +- 3 files changed, 129 insertions(+), 51 deletions(-) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index 9144ac9999..2d6cf4f143 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -286,58 +286,149 @@ def __init__(): def test_initializer_order_nested(make_input_bundle): lib1 = """ +a: public(uint256) + +@deploy +@payable +def __init__(x: uint256): + self.a = x + """ + lib2 = """ +import lib1 + +uses: lib1 + +a: uint256 + +@deploy +def __init__(): + # not initialized when called + self.a = lib1.a + """ + lib3 = """ +import lib1 + +initializes: lib1 + +a: uint256 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + lib1.__init__(0) + """ + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) + + main1 = """ +import lib1 import lib2 import lib3 -uses: lib3 -initializes: lib2[lib3 := lib3] +initializes: lib2[lib1 := lib1] +initializes: lib3 @deploy def __init__(): + lib3.__init__(0) lib2.__init__() - lib3.counter += 1 """ - lib2 = """ -import lib3 + assert compile_code(main1, input_bundle=input_bundle) is not None -uses: lib3 + main2 = """ +import lib1 +import lib2 +import lib3 -counter: uint256 +initializes: lib2[lib1 := lib1] +initializes: lib3 @deploy def __init__(): - self.counter = 1 + lib2.__init__() # opposite order! + lib3.__init__(0) + """ + with pytest.raises(InitializerException) as e: + compile_code(main2, input_bundle=input_bundle) -@external -def foo() ->uint256: - return lib3.counter + expected = "Tried to initialize `lib2`, but it depends on `lib1`, which " + expected += "has not been initialized yet." + assert e.value._message == expected + + assert e.value._hint == "call `lib1.__init__()` before `lib2.__init__()`." + + +def test_initializer_nested_order2(make_input_bundle): + lib1 = """ +import lib4 + +a: public(uint256) + +initializes: lib4 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + lib4.__init__(x) """ - lib3 = """ -counter: uint256 + + lib2 = """ +import lib1 +import lib4 + +uses: lib1 +uses: lib4 + +a: uint256 @deploy def __init__(): - self.counter = 1 + # not initialized when called + self.a = lib1.a + lib4.a """ - input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) + lib3 = """ +import lib1 + +initializes: lib1 + +a: uint256 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + lib1.__init__(0) + """ + lib4 = """ +a: uint256 + +@deploy +@payable +def __init__(x: uint256): + self.a = x + """ main = """ import lib1 +import lib2 import lib3 +import lib4 -initializes: lib1[lib3 := lib3] +initializes: lib2[lib1 := lib1, lib4 := lib4] initializes: lib3 @deploy def __init__(): - lib3.__init__() - lib1.__init__() + lib3.__init__(0) + lib2.__init__() """ - with pytest.raises(ImmutableViolation) as e: - compile_code(main, input_bundle=input_bundle) - assert e.value._message == "Cannot access `lib3` state from `__init__()` function!" - assert e.value._hint == "add `initializes: lib3` as a top-level statement to your contract" + input_bundle = make_input_bundle( + {"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3, "lib4.vy": lib4} + ) + + assert compile_code(main, input_bundle=input_bundle) is not None def test_imported_as_different_names(make_input_bundle): diff --git a/vyper/semantics/analysis/local.py b/vyper/semantics/analysis/local.py index 8c6391e682..26c6a4ef9f 100644 --- a/vyper/semantics/analysis/local.py +++ b/vyper/semantics/analysis/local.py @@ -253,7 +253,7 @@ def _get_module_chain(node: vy_ast.ExprNode) -> list[ModuleInfo]: return ret -def check_module_uses(node: vy_ast.ExprNode, is_deploy: bool = False) -> Optional[ModuleInfo]: +def check_module_uses(node: vy_ast.ExprNode) -> Optional[ModuleInfo]: """ validate module usage, and that if we use lib1.lib2., that lib1 at least `uses` lib2. @@ -266,32 +266,19 @@ def check_module_uses(node: vy_ast.ExprNode, is_deploy: bool = False) -> Optiona if len(module_infos) == 0: return None - level = ModuleOwnership.USES - if is_deploy: - level = ModuleOwnership.INITIALIZES - for module_info in module_infos: - if module_info.ownership < level: - # cannot touch state of `used` modules in ctor - if is_deploy: - msg = f"Cannot access `{module_info.alias}` state from " - msg += "`__init__()` function!" - - hint = f"add `initializes: {module_info.alias}` as " - hint += "a top-level statement to your contract" - else: - msg = f"Cannot access `{module_info.alias}` state!\n note that" - # CMC 2024-04-12 add UX note about nonreentrant. might be nice - # in the future to be more specific about exactly which state is - # used, although that requires threading a bit more context into - # this function. - msg += " use of the `@nonreentrant` decorator is also" - msg += " considered state access" - - hint = f"add `uses: {module_info.alias}` or " - hint += f"`initializes: {module_info.alias}` as " - hint += "a top-level statement to your contract" - + if module_info.ownership < ModuleOwnership.USES: + msg = f"Cannot access `{module_info.alias}` state!\n note that" + # CMC 2024-04-12 add UX note about nonreentrant. might be nice + # in the future to be more specific about exactly which state is + # used, although that requires threading a bit more context into + # this function. + msg += " use of the `@nonreentrant` decorator is also considered" + msg += " state access" + + hint = f"add `uses: {module_info.alias}` or " + hint += f"`initializes: {module_info.alias}` as " + hint += "a top-level statement to your contract" raise ImmutableViolation(msg, hint=hint) # the leftmost- referenced module @@ -465,7 +452,7 @@ def _handle_modification(self, target: vy_ast.ExprNode): info._writes.add(var_access) def _handle_module_access(self, target: vy_ast.ExprNode): - root_module_info = check_module_uses(target, self.func.is_deploy) + root_module_info = check_module_uses(target) if root_module_info is not None: # log the access diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 48d6eca315..e4cf48a1bd 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -325,8 +325,8 @@ def validate_used_modules(self): err_list.raise_if_not_empty() def validate_initialized_modules(self): - # check all `initializes:` modules have `__init__()` called exactly - # once, and check they are called in dependency order + # check all `initializes:` modules have `__init__()` called exactly once, + # and check they are called in dependency order module_t = self.ast._metadata["type"] should_initialize = {t.module_info.module_t: t for t in module_t.initialized_modules} From 9740de32409716dca2e3679b534204b0bae583e0 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 2 Jun 2024 10:07:20 -0400 Subject: [PATCH 14/15] fix the algorithm --- vyper/semantics/analysis/global_.py | 26 +++++++++++++++++++++++++- vyper/semantics/analysis/module.py | 23 ----------------------- vyper/semantics/types/module.py | 10 ---------- 3 files changed, 25 insertions(+), 34 deletions(-) diff --git a/vyper/semantics/analysis/global_.py b/vyper/semantics/analysis/global_.py index 23b45a1114..395b6eaabc 100644 --- a/vyper/semantics/analysis/global_.py +++ b/vyper/semantics/analysis/global_.py @@ -3,6 +3,7 @@ from vyper.exceptions import ExceptionList, InitializerException from vyper.semantics.analysis.base import InitializesInfo, UsesInfo from vyper.semantics.types.module import ModuleT +from vyper.utils import OrderedSet def validate_compilation_target(module_t: ModuleT): @@ -54,6 +55,30 @@ def _validate_global_initializes_constraint(module_t: ModuleT): all_used_modules = _collect_used_modules_r(module_t) all_initialized_modules = _collect_initialized_modules_r(module_t) + hint = None + + init_calls = [] + if module_t.init_function is not None: + init_calls = list(module_t.init_function.reachable_internal_functions) + seen: OrderedSet = OrderedSet() + + for init_t in init_calls: + seen.add(init_t) + init_m = init_t.decl_node.module_node._metadata["type"] + init_info = all_initialized_modules[init_m] + for dep in init_info.dependencies: + m = dep.module_t + if m.init_function is None: + continue + if m.init_function not in seen: + # TODO: recover source info + msg = f"Tried to initialize `{init_info.module_info.alias}`, " + msg += f"but it depends on `{dep.alias}`, which has not been " + msg += "initialized yet." + hint = f"call `{dep.alias}.__init__()` before " + hint += f"`{init_info.module_info.alias}.__init__()`." + raise InitializerException(msg, hint=hint) + err_list = ExceptionList() for u, uses in all_used_modules.items(): @@ -61,7 +86,6 @@ def _validate_global_initializes_constraint(module_t: ModuleT): msg = f"module `{u}` is used but never initialized!" # construct a hint if the module is in scope - hint = None found_module = module_t.find_module_info(u) if found_module is not None: # TODO: do something about these constants diff --git a/vyper/semantics/analysis/module.py b/vyper/semantics/analysis/module.py index 4b35f3f6ea..7628579254 100644 --- a/vyper/semantics/analysis/module.py +++ b/vyper/semantics/analysis/module.py @@ -339,9 +339,6 @@ def validate_initialized_modules(self): # map of seen __init__() function calls seen_initializers = {} - # modules which were already initialized in another module - already_initialized = OrderedSet() - for call_node in init_calls: expr_info = call_node.func._expr_info if expr_info is None: @@ -373,29 +370,9 @@ def validate_initialized_modules(self): hint += "as a top-level statement to your contract" raise InitializerException(msg, call_node.func, hint=hint) - initializer_info = should_initialize[initialized_module.module_t] - for dep_info in initializer_info.dependencies: # type: ModuleInfo - dep_t = dep_info.module_t - - if dep_t in already_initialized or dep_t.init_function is None: - continue - - if dep_t not in seen_initializers: - msg = f"Tried to initialize `{initialized_module.alias}`, " - msg += f"but it depends on `{dep_info.alias}`, which has " - msg += "not been initialized yet." - hint = f"call `{dep_info.alias}.__init__()` before " - hint += f"`{initialized_module.alias}.__init__()`." - raise InitializerException(msg, call_node.func, hint=hint) - del should_initialize[initialized_module.module_t] seen_initializers[initialized_module.module_t] = call_node.func - # add the modules that the module initialized, these are already - # checked in the recursion. - initialized = initialized_module.module_t - already_initialized.update(initialized.initialized_modules_recursed) - if len(should_initialize) > 0: err_list = ExceptionList() for s in should_initialize.values(): diff --git a/vyper/semantics/types/module.py b/vyper/semantics/types/module.py index 09c578d8ef..b2d91bdcc4 100644 --- a/vyper/semantics/types/module.py +++ b/vyper/semantics/types/module.py @@ -486,16 +486,6 @@ def initialized_modules(self): ret.append(info) return ret - @cached_property - def initialized_modules_recursed(self) -> OrderedSet["ModuleT"]: - # modules which are initialized, taking into account recursion - ret: OrderedSet = OrderedSet() - for info in self.initialized_modules: - module_t = info.module_info.module_t - ret |= module_t.initialized_modules_recursed - ret.add(module_t) - return ret - @cached_property def exposed_functions(self): # return external functions that are exposed in the runtime From e46e5350b520458481011181aeb3b018487f35c1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sun, 2 Jun 2024 10:16:29 -0400 Subject: [PATCH 15/15] add another test --------- Co-authored-by: trocher --- .../syntax/modules/test_initializers.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tests/functional/syntax/modules/test_initializers.py b/tests/functional/syntax/modules/test_initializers.py index ec15b0e1fe..a809685989 100644 --- a/tests/functional/syntax/modules/test_initializers.py +++ b/tests/functional/syntax/modules/test_initializers.py @@ -114,6 +114,59 @@ def __init__(): assert compile_code(main, input_bundle=input_bundle) is not None +# test multiple uses in different nodes of the import tree +def test_distant_use_initialize(make_input_bundle): + lib3 = """ +counter: uint256 + +@deploy +def __init__(): + self.counter = 1 + """ + lib2 = """ +import lib3 + +uses: lib3 + +counter: uint256 + +@deploy +def __init__(): + self.counter = 1 + +@external +def foo() ->uint256: + return lib3.counter + """ + lib1 = """ +import lib2 +import lib3 + +uses: lib3 +initializes: lib2[lib3 := lib3] + +@deploy +def __init__(): + lib2.__init__() + lib3.counter += 1 + """ + main = """ +import lib1 +import lib3 + +initializes: lib1[lib3 := lib3] +initializes: lib3 + +@deploy +def __init__(): + lib3.__init__() + lib1.__init__() + """ + input_bundle = make_input_bundle({"lib1.vy": lib1, "lib2.vy": lib2, "lib3.vy": lib3}) + + assert compile_code(main, input_bundle=input_bundle) is not None + + def test_initialize_multi_line_uses(make_input_bundle): lib1 = """ counter: uint256