-
-
Notifications
You must be signed in to change notification settings - Fork 836
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.
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
fix[lang]: stateless modules should not be initialized #4347
base: master
Are you sure you want to change the base?
Changes from 14 commits
1b1ac45
3a9e29d
5527244
3c695da
5000efc
e3d7582
c92e1ad
cf91784
1ab6433
28c9be7
0c5f645
47efc8e
5fe6c84
fececb7
4cc01bb
ba19783
b668f41
6c7a30f
948d047
3dbbeb6
1f4a2d6
be5aa0b
7b2f38b
bb4944f
b41bc28
2bcba89
7f2f452
f64109f
bf7307c
a075979
b67ab3f
ef0c49d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
def test_export_nonreentrant(make_input_bundle, get_contract, tx_failed): | ||
lib1 = """ | ||
phony: uint32 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this feels off, see my comment at |
||
|
||
interface Foo: | ||
def foo() -> uint256: nonpayable | ||
|
||
|
@@ -38,6 +40,8 @@ def __default__(): | |
|
||
def test_internal_nonreentrant(make_input_bundle, get_contract, tx_failed): | ||
lib1 = """ | ||
phony: uint32 | ||
|
||
interface Foo: | ||
def foo() -> uint256: nonpayable | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -218,6 +218,8 @@ def bar(x: {type}): | |
|
||
def test_exports_abi(make_input_bundle): | ||
lib1 = """ | ||
phony: uint32 | ||
|
||
@external | ||
def foo(): | ||
pass | ||
|
@@ -330,6 +332,8 @@ def __init__(): | |
def test_event_export_from_init(make_input_bundle): | ||
# test that events get exported when used in init functions | ||
lib1 = """ | ||
phony: uint32 | ||
|
||
event MyEvent: | ||
pass | ||
|
||
|
@@ -361,6 +365,8 @@ def __init__(): | |
def test_event_export_from_function_export(make_input_bundle): | ||
# test events used in exported functions are exported | ||
lib1 = """ | ||
phony: uint32 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe we shouldn't add phony here -- the code change is telling us to remove |
||
|
||
event MyEvent: | ||
pass | ||
|
||
|
@@ -396,6 +402,8 @@ def foo(): | |
def test_event_export_unused_function(make_input_bundle): | ||
# test events in unused functions are not exported | ||
lib1 = """ | ||
phony: uint32 | ||
|
||
event MyEvent: | ||
pass | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -444,19 +444,19 @@ def find_module_info(self, needle: "ModuleT") -> Optional["ModuleInfo"]: | |
return s | ||
return None | ||
|
||
@property | ||
@cached_property | ||
def variable_decls(self): | ||
return self._module.get_children(vy_ast.VariableDecl) | ||
|
||
@property | ||
@cached_property | ||
def uses_decls(self): | ||
return self._module.get_children(vy_ast.UsesDecl) | ||
|
||
@property | ||
@cached_property | ||
def initializes_decls(self): | ||
return self._module.get_children(vy_ast.InitializesDecl) | ||
|
||
@property | ||
@cached_property | ||
def exports_decls(self): | ||
return self._module.get_children(vy_ast.ExportsDecl) | ||
|
||
|
@@ -469,7 +469,7 @@ def used_modules(self): | |
ret.append(used_module) | ||
return ret | ||
|
||
@property | ||
@cached_property | ||
def initialized_modules(self): | ||
# modules which are initialized to | ||
ret = [] | ||
|
@@ -559,3 +559,19 @@ def immutable_section_bytes(self): | |
@cached_property | ||
def interface(self): | ||
return InterfaceT.from_ModuleT(self) | ||
|
||
@cached_property | ||
def is_stateless(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this definition omits to further support the statement above, consider following: #main.vy
import lib1
#initializes: lib1
exports: lib1.foo #lib1.vy
@external
@nonreentrant
def foo() -> uint256:
return 5 which yields:
|
||
""" | ||
Determine whether ModuleT is stateless by examining its top-level | ||
declarations. A module has state if it contains storage variables, | ||
transient variables, or immutables, or if it includes a "uses" or | ||
"initializes" declaration. | ||
""" | ||
if len(self.initializes_decls) > 0 or len(self.uses_decls) > 0: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we really check for |
||
return False | ||
if any(not v.is_constant for v in self.variable_decls): | ||
return False | ||
if self.init_function is not None: | ||
charles-cooper marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return False | ||
return True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's please add a comment here that we're testing the initialization and how it relates to state