Skip to content
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

_abi_encode builtin evaluates argument multiple times the argument is a complex type #2459

Closed
Tracked by #2471
charles-cooper opened this issue Sep 17, 2021 · 1 comment
Assignees
Labels
bug Bug that shouldn't change language semantics when fixed.

Comments

@charles-cooper
Copy link
Member

Version Information

  • vyper Version (output of vyper --version): v0.2.16

What's your issue about?

interface Foo:
    def get_counter() -> (uint256, String[6]): payable

@external
def foo() -> Bytes[128]:
    return _abi_encode(Foo(msg.sender).get_counter(), method_id=method_id("foo()"))

In the generated code the following sequence appears twice
[assert, [call, gas, caller, 0, [seq, [mstore, 320, 3932606317], 348], 4, 416, 128]]

How can it be fixed?

commit 8f6ca4d contains the fix:

diff --git a/vyper/old_codegen/abi.py b/vyper/old_codegen/abi.py
index 04b39797..16ae1a27 100644
--- a/vyper/old_codegen/abi.py
+++ b/vyper/old_codegen/abi.py
@@ -414,10 +414,18 @@ def abi_encode(dst, lll_node, pos=None, bufsz=None, returns_len=False):
         return LLLnode.from_list(lll_ret, pos=pos, annotation=f"abi_encode {lll_node.typ}")
 
     lll_ret = ["seq"]
+
+    # contains some computation, we need to only do it once.
+    is_complex_lll = lll_node.value in ("seq", "seq_unchecked")
+    if is_complex_lll:
+        to_encode = LLLnode.from_list("to_encode", typ=lll_node.typ, location=lll_node.location, encoding=lll_node.encoding)
+    else:
+        to_encode = lll_node
+
     dyn_ofst = "dyn_ofst"  # current offset in the dynamic section
     dst_begin = "dst"  # pointer to beginning of buffer
     dst_loc = "dst_loc"  # pointer to write location in static section
-    os = o_list(lll_node, pos=pos)
+    os = o_list(to_encode, pos=pos)
 
     for i, o in enumerate(os):
         abi_t = abi_type_of(o.typ)
@@ -477,6 +485,9 @@ def abi_encode(dst, lll_node, pos=None, bufsz=None, returns_len=False):
 
     lll_ret = ["with", dst_begin, dst, ["with", dst_loc, dst_begin, lll_ret]]
 
+    if is_complex_lll:
+        lll_ret = ["with", to_encode, lll_node, lll_ret]
+
     return LLLnode.from_list(lll_ret, pos=pos, annotation=f"abi_encode {lll_node.typ}")
 
@charles-cooper charles-cooper self-assigned this Sep 17, 2021
@charles-cooper charles-cooper added the bug Bug that shouldn't change language semantics when fixed. label Sep 17, 2021
@charles-cooper
Copy link
Member Author

fixed in #2447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug that shouldn't change language semantics when fixed.
Projects
None yet
Development

No branches or pull requests

1 participant