From 961e4a4990821ee4a1294e3149d31b4048b9d600 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Apr 2023 16:34:36 -0700 Subject: [PATCH 1/8] fix: call graph stability the use of `set` (which does not guarantee order of its elements) instead of `dict` (which guarantees insertion order) led to instability across runs of the compiler between different versions of python. this commit patches the problem by using a derivation of `dict` to track the call graph instead of `set`. --- vyper/semantics/types/function.py | 4 +++- vyper/utils.py | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 99a1957fd7..308fc785fc 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -3,7 +3,9 @@ from collections import OrderedDict from typing import Any, Dict, List, Optional, Set, Tuple + from vyper import ast as vy_ast +from vyper.utils import OrderedSet from vyper.ast.validation import validate_call_args from vyper.exceptions import ( ArgumentException, @@ -89,7 +91,7 @@ def __init__( self.nonreentrant = nonreentrant # a list of internal functions this function calls - self.called_functions: Set["ContractFunctionT"] = set() + self.called_functions = OrderedSet() # special kwargs that are allowed in call site self.call_site_kwargs = { diff --git a/vyper/utils.py b/vyper/utils.py index 37a3f13b3d..b7afe88705 100644 --- a/vyper/utils.py +++ b/vyper/utils.py @@ -11,6 +11,11 @@ from vyper.exceptions import DecimalOverrideException, InvalidLiteral +class OrderedSet(dict): + def add(self, item): + self[item] = None + + class DecimalContextOverride(decimal.Context): def __setattr__(self, name, value): if name == "prec": From 0b5e44d923e68ca0bb167cc90ebb6d19c1469e99 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 26 Apr 2023 16:43:06 -0700 Subject: [PATCH 2/8] fix lint --- vyper/semantics/types/function.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/vyper/semantics/types/function.py b/vyper/semantics/types/function.py index 308fc785fc..75fa3a1214 100644 --- a/vyper/semantics/types/function.py +++ b/vyper/semantics/types/function.py @@ -1,11 +1,9 @@ import re import warnings from collections import OrderedDict -from typing import Any, Dict, List, Optional, Set, Tuple - +from typing import Any, Dict, List, Optional, Tuple from vyper import ast as vy_ast -from vyper.utils import OrderedSet from vyper.ast.validation import validate_call_args from vyper.exceptions import ( ArgumentException, @@ -30,7 +28,7 @@ from vyper.semantics.types.shortcuts import UINT256_T from vyper.semantics.types.subscriptable import TupleT from vyper.semantics.types.utils import type_from_abi, type_from_annotation -from vyper.utils import keccak256 +from vyper.utils import OrderedSet, keccak256 class ContractFunctionT(VyperType): From 6b8b2d76f7edc7bd3c127ab0f8a445cdc80c8df2 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Fri, 28 Apr 2023 20:00:38 -0700 Subject: [PATCH 3/8] add tests for call graph stability --- tests/parser/test_call_graph_stability.py | 54 +++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 tests/parser/test_call_graph_stability.py diff --git a/tests/parser/test_call_graph_stability.py b/tests/parser/test_call_graph_stability.py new file mode 100644 index 0000000000..9fb26014a2 --- /dev/null +++ b/tests/parser/test_call_graph_stability.py @@ -0,0 +1,54 @@ +from hypothesis import given, settings +from hypothesis.strategies import text , lists +import random +import string +import pytest +from vyper.compiler.phases import CompilerData +import vyper.ast as vy_ast + +# random names for functions +@settings(max_examples=20, deadline=1000) +@given(lists(text(alphabet=string.ascii_lowercase, min_size=1), unique=True,min_size=1, max_size=10)) +@pytest.mark.fuzzing +def test_call_graph_stability_fuzz(func_names): + def generate_func_def(func_name, i): + return f""" +@internal +def {func_name}() -> uint256: + return {i} + """ + func_defs = "\n".join(generate_func_def(s, i) for i, s in enumerate(func_names)) + + + for i in range(10): + fs = func_names.copy() + random.shuffle(fs) + + self_calls = "\n".join(f" self.{f}()" for f in func_names) + code = f""" +{func_defs} + +@external +def foo(): +{self_calls} + """ + t = CompilerData(code) + + # check the .called_functions data structure on foo() directly + foo = t.vyper_module_folded.get_children(vy_ast.FunctionDef, filters={"name": "foo"})[0] + foo_t = foo._metadata["type"] + assert [f.name for f in foo_t.called_functions] == func_names + + # now for sanity, ensure the order that the function definitions appear + # in the IR is the same as the order of the calls + sigs = t.function_signatures + del sigs["foo"] + ir = t.ir_runtime + ir_funcs = [] + # search for function labels + for d in ir.args: # currently: (seq ... (seq (label foo ...)) ...) + if d.value == "seq" and d.args[0].value == "label": + r = d.args[0].args[0].value + if isinstance(r, str) and r.startswith("internal"): + ir_funcs.append(r) + assert ir_funcs == [f.internal_function_label for f in sigs.values()] From c8657410206c75ff5219ed49fdfc6e4131a88762 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 29 Apr 2023 13:30:17 -0700 Subject: [PATCH 4/8] fix lint --- tests/parser/test_call_graph_stability.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/parser/test_call_graph_stability.py b/tests/parser/test_call_graph_stability.py index 9fb26014a2..94b491b182 100644 --- a/tests/parser/test_call_graph_stability.py +++ b/tests/parser/test_call_graph_stability.py @@ -1,14 +1,19 @@ -from hypothesis import given, settings -from hypothesis.strategies import text , lists import random import string + import pytest -from vyper.compiler.phases import CompilerData +from hypothesis import given, settings +from hypothesis.strategies import lists, text + import vyper.ast as vy_ast +from vyper.compiler.phases import CompilerData + # random names for functions @settings(max_examples=20, deadline=1000) -@given(lists(text(alphabet=string.ascii_lowercase, min_size=1), unique=True,min_size=1, max_size=10)) +@given( + lists(text(alphabet=string.ascii_lowercase, min_size=1), unique=True, min_size=1, max_size=10) +) @pytest.mark.fuzzing def test_call_graph_stability_fuzz(func_names): def generate_func_def(func_name, i): @@ -17,10 +22,10 @@ def generate_func_def(func_name, i): def {func_name}() -> uint256: return {i} """ + func_defs = "\n".join(generate_func_def(s, i) for i, s in enumerate(func_names)) - - for i in range(10): + for _ in range(10): fs = func_names.copy() random.shuffle(fs) From dd04dfe83ee8929bd357790c30d352d088a3ab64 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Sat, 29 Apr 2023 15:19:12 -0700 Subject: [PATCH 5/8] fuzz mutability as well --- tests/parser/test_call_graph_stability.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/parser/test_call_graph_stability.py b/tests/parser/test_call_graph_stability.py index 94b491b182..526b9365cc 100644 --- a/tests/parser/test_call_graph_stability.py +++ b/tests/parser/test_call_graph_stability.py @@ -17,8 +17,10 @@ @pytest.mark.fuzzing def test_call_graph_stability_fuzz(func_names): def generate_func_def(func_name, i): + mutability = random.choice(["@pure", "@view", "@nonpayable", "@payable"]) return f""" @internal +{mutability} def {func_name}() -> uint256: return {i} """ From 89b353d6fa67dd57857f2203e072f6a2082518d5 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 1 May 2023 09:26:33 -0700 Subject: [PATCH 6/8] fix deadline --- tests/parser/test_call_graph_stability.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/parser/test_call_graph_stability.py b/tests/parser/test_call_graph_stability.py index 526b9365cc..cd8b5ae5f7 100644 --- a/tests/parser/test_call_graph_stability.py +++ b/tests/parser/test_call_graph_stability.py @@ -10,7 +10,7 @@ # random names for functions -@settings(max_examples=20, deadline=1000) +@settings(max_examples=20, deadline=None) @given( lists(text(alphabet=string.ascii_lowercase, min_size=1), unique=True, min_size=1, max_size=10) ) From fea5b2ba6558bd6d36750cfc2c61d73ed6749a4d Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Wed, 3 May 2023 10:32:16 -0700 Subject: [PATCH 7/8] use sampled_from --- tests/parser/test_call_graph_stability.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/tests/parser/test_call_graph_stability.py b/tests/parser/test_call_graph_stability.py index cd8b5ae5f7..6785169ba3 100644 --- a/tests/parser/test_call_graph_stability.py +++ b/tests/parser/test_call_graph_stability.py @@ -1,9 +1,9 @@ import random import string +import hypothesis.strategies as st import pytest from hypothesis import given, settings -from hypothesis.strategies import lists, text import vyper.ast as vy_ast from vyper.compiler.phases import CompilerData @@ -12,12 +12,19 @@ # random names for functions @settings(max_examples=20, deadline=None) @given( - lists(text(alphabet=string.ascii_lowercase, min_size=1), unique=True, min_size=1, max_size=10) + st.lists( + st.tuples( + st.sampled_from(["@pure", "@view", "@nonpayable", "@payable"]), + st.text(alphabet=string.ascii_lowercase, min_size=1), + ), + unique_by=lambda x: x[1], # unique on function name + min_size=1, + max_size=10, + ) ) @pytest.mark.fuzzing -def test_call_graph_stability_fuzz(func_names): - def generate_func_def(func_name, i): - mutability = random.choice(["@pure", "@view", "@nonpayable", "@payable"]) +def test_call_graph_stability_fuzz(funcs): + def generate_func_def(mutability, func_name, i): return f""" @internal {mutability} @@ -25,11 +32,11 @@ def {func_name}() -> uint256: return {i} """ - func_defs = "\n".join(generate_func_def(s, i) for i, s in enumerate(func_names)) + func_defs = "\n".join(generate_func_def(m, s, i) for i, (m, s) in enumerate(funcs)) for _ in range(10): - fs = func_names.copy() - random.shuffle(fs) + func_names = [f for (_, f) in funcs] + random.shuffle(func_names) self_calls = "\n".join(f" self.{f}()" for f in func_names) code = f""" From df005d22c77e33160b159549f2d532f5f2883ed1 Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Thu, 4 May 2023 11:50:46 -0700 Subject: [PATCH 8/8] add comment to the set class --- vyper/utils.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/vyper/utils.py b/vyper/utils.py index b7afe88705..2440117d0c 100644 --- a/vyper/utils.py +++ b/vyper/utils.py @@ -12,6 +12,14 @@ class OrderedSet(dict): + """ + a minimal "ordered set" class. this is needed in some places + because, while dict guarantees you can recover insertion order + vanilla sets do not. + no attempt is made to fully implement the set API, will add + functionality as needed. + """ + def add(self, item): self[item] = None