Structs as args #1115

charles-cooper · 2018-12-03T23:25:33Z

- What I did

Partially implement #1019 - structs as arguments and return values.

WIP Checklist

Fix ABI output for function args
Fix units
Fix ABI output for nested structs/tuples
Tests

- How I did it

Add packing / unpacking code for structs

- How to verify it

See tests

- Description for the changelog

Ability to pass structs as args and return values. Structs which contain other structs or structs which contain bytearrays not supported.

vyper/parser/expr.py

fubuloubu · 2019-01-03T16:26:17Z

Is emitting a struct in an event possible?

charles-cooper · 2019-01-03T19:54:58Z

Not in this PR. I think that structs in events needs a bit more thought. Since each byte costs 8 gas, we probably should use the packed ABI encoding. I'm not sure what solidity produces / what web3 expects.

fubuloubu · 2019-01-03T21:17:51Z

Moved that convo to #1170

charles-cooper · 2019-01-03T22:34:53Z

Ok, based on the latest meeting, I rebased this PR and reduced the scope. For now, nested structs will not work with the ABI, and neither will struct arguments. Also, I opted to disallow passing structs with dynamic members for now, otherwise this PR will get too large. Should be ready for review (pending CI)!

fubuloubu · 2019-01-04T00:04:07Z

CI failed, but I also think this feature needs way more tests lol

fubuloubu · 2019-01-04T00:04:33Z

Also, can you add issues to track what was deemed out of scope?

charles-cooper · 2019-01-04T01:33:58Z

@fubuloubu I can add some typechecking tests. Any other specific tests you think should be in here?

fubuloubu · 2019-01-04T04:03:44Z

Anything with the ABI should be pretty throughly tested. That boundary a lot of things can go wrong haha.

fubuloubu · 2019-01-04T04:36:28Z

@jacqueswww when merged, can you cut as 0.1.0b7?

jacqueswww · 2019-01-04T13:07:19Z

@fubuloubu I want a few more bug fixes in first, but b7 will be soon. In the meantime just peg the commit hash in your requirements.

jacqueswww

LGTM, this is great addition to vyper! :) @charles-cooper I think the one test that this still needs is if one wants to use a struct in an extract contract interface?

fubuloubu · 2019-01-11T21:10:24Z

Testing this. It compiles, but I'm having difficulties getting the ABI to work with Web3py. What's the best way to do this?

File: https://github.com/GunClear/plasma-cash-vyper/blob/struct-args/contracts/RootChain.vy

jacqueswww · 2019-01-11T23:02:01Z

@fubuloubu I don't think web3py is ready for those types of nested tuples (yet).
You could probably patch web3py with the latest eth-abi and try interpreting in that way?

fubuloubu · 2019-01-12T01:27:55Z

v5 has eth-abi be right? That works for me.

v2 outputs the struct definitions right?

jacqueswww · 2019-01-13T15:14:43Z

I think I will just merge this for now - as I am scared of branches diverging too much. Will create a ticket for the outstanding test.

fubuloubu · 2019-01-15T04:35:20Z

Follow-up: ABI v2 doesn't seem to work with this :(

charles-cooper · 2019-01-15T05:49:15Z

@fubuloubu uh oh! Can you provide a reproducible case?

fubuloubu · 2019-01-15T15:41:45Z

dependencies:
pip install "py-evm>=0.2.0b39"
pip install "eth-tester>=0.1.0b36"
pip install "web3>=5.0.0a2"
pip install git+https://github.com/ethereum/vyper.git

>>> import vyper
>>> from web3 import Web3, EthereumTesterProvider

>>> w3 = Web3(EthereumTesterProvider())

>>> interface = vyper.compile_code("""
... struct MyStruct:
...     a: address
...     b: bytes32
... 
... @public
... def a(s: MyStruct) -> address:
...     return s.a
... 
... @public
... def b(s: MyStruct) -> bytes32:
...     return s.b
... """, output_formats=['abi', 'bytecode', 'bytecode_runtime'])

>>> txn_hash = w3.eth.contract(**interface).constructor().transact()
>>> address = w3.eth.waitForTransactionReceipt(txn_hash)['contractAddress']
>>> contract = w3.eth.contract(address, **interface)

>>> contract.functions.a(contract.address, b'\x00' * 32).call()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/contract.py", line 1003, in __call__
    clone._set_function_info()
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/contract.py", line 1012, in _set_function_info
    self.kwargs
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/contracts.py", line 128, in find_matching_fn_abi
    raise ValidationError(message)
web3.exceptions.ValidationError:
Could not identify the intended function with name `a`, positional argument(s) of type `(<class 'str'>, <class 'bytes'>)` and keyword argument(s) of type `{}`.
Found 1 function(s) with the name `a`: ['a((address,bytes32))']
Function invocation failed due to improper number of arguments.

>>> contract.functions.a((contract.address, b'\x00' * 32)).call()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/contract.py", line 1003, in __call__
    clone._set_function_info()
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/contract.py", line 1012, in _set_function_info
    self.kwargs
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/contracts.py", line 97, in find_matching_fn_abi
    function_candidates = pipe(abi, *filters)
  File "cytoolz/functoolz.pyx", line 589, in cytoolz.functoolz.pipe
  File "cytoolz/functoolz.pyx", line 565, in cytoolz.functoolz.c_pipe
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/abi.py", line 203, in filter_by_encodability
    in contract_abi
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/abi.py", line 204, in <listcomp>
    if check_if_arguments_can_be_encoded(function_abi, args, kwargs)
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/abi.py", line 221, in check_if_arguments_can_be_encoded
    for _type, arg in zip(types, arguments)
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/abi.py", line 221, in <genexpr>
    for _type, arg in zip(types, arguments)
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/abi.py", line 166, in is_encodable
    base, sub, arrlist = process_type(_type)
  File "/home/bryant/.pyenv/versions/test/lib/python3.6/site-packages/web3/_utils/abi.py", line 136, in process_type
    type_str_repr,
ValueError: Cannot process type '(address,bytes32)': tuple types not supported

charles-cooper · 2019-01-15T18:29:17Z

@fubuloubu that looks like a bug. Could you please create an issue?

fubuloubu · 2019-01-15T18:53:47Z

A think it's a bug against web3py v5 if it doesn't implement the full abi V2 spec yet.

How did you decide what the ABI output should be in moving forward with this PR? Basically follow Solidity?

charles-cooper · 2019-01-15T19:09:40Z

@fubuloubu Oh that's right. I even said so in my comments above (and apparently promptly forgot!)

To answer your question, I wanted to follow Solidity but tuples are not supported by eth-abi yet. The signature should be ABI-encoded as

...
    "name": "b",
    "inputs": [
      {
        "type": "tuple",
        "components": [
          { "type": "address", "name": "s_a"},
          {"type": "bytes32", "name" "s_b"}],
        "name": "s"
      }
    ],
...

and something similar for outputs. But that doesn't work, so for now I kept the input/output encoding the same as before (tuple inputs are encoded as (address,bytes32) and outputs are encoded as an array "outputs": [{ "type": "address", "name": "s_a"},{"type": "bytes32", "name" "s_b"}]) so as to keep tests passing.

fubuloubu · 2019-01-15T19:35:39Z

I really wished the ABI had entries for structs, so we can just reference structs in different places instead of this tuple nonsense lol

jacqueswww reviewed Dec 4, 2018

View reviewed changes

vyper/parser/expr.py Show resolved Hide resolved

jacqueswww reviewed Dec 4, 2018

View reviewed changes

vyper/parser/expr.py Outdated Show resolved Hide resolved

jacqueswww reviewed Dec 4, 2018

View reviewed changes

vyper/parser/expr.py Outdated Show resolved Hide resolved

jacqueswww reviewed Dec 4, 2018

View reviewed changes

vyper/parser/expr.py Outdated Show resolved Hide resolved

charles-cooper force-pushed the cmc-struct-args branch from 6664b38 to 881db2b Compare December 5, 2018 19:35

charles-cooper and others added 8 commits January 3, 2019 12:12

Implement structs as args and return values

5df695c

Add basic struct return test.

ba30675

Patch struct ABI output.

d61ee45

Add struct return codegen.

c6a574e

Move codegen code into parser_utils

ce1f364

Fix unit typecheck for struct literals

aa0c555

Simplify struct literal code

cdb738d

Fix struct detection for return

0cf8aee

charles-cooper added 4 commits January 3, 2019 13:35

Add private struct return test

2f70566

Fix return tuple type

6e7ac96

Fix return struct for private calls

4208ce9

Add struct packing for args

d1c78a3

charles-cooper force-pushed the cmc-struct-args branch from c250d1e to d1c78a3 Compare January 3, 2019 22:30

Fix lint

57506e6

Remove test to fail return struct

0cf6d01

Fix a typechecker case

2c66b62

charles-cooper mentioned this pull request Jan 4, 2019

VIP: Structs as arguments #1019

Closed

charles-cooper changed the title ~~WIP: Structs as args~~ Structs as args Jan 4, 2019

charles-cooper mentioned this pull request Jan 4, 2019

Fix tuple return #1175

Merged

jacqueswww approved these changes Jan 10, 2019

View reviewed changes

jacqueswww merged commit e5c95b2 into vyperlang:master Jan 13, 2019

charles-cooper deleted the cmc-struct-args branch September 24, 2019 11:48

fubuloubu mentioned this pull request Oct 24, 2019

Support Nested Structs #1648

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Structs as args #1115

Structs as args #1115

charles-cooper commented Dec 3, 2018 •

edited

Loading

fubuloubu commented Jan 3, 2019

charles-cooper commented Jan 3, 2019

fubuloubu commented Jan 3, 2019

charles-cooper commented Jan 3, 2019

fubuloubu commented Jan 4, 2019

fubuloubu commented Jan 4, 2019

charles-cooper commented Jan 4, 2019

fubuloubu commented Jan 4, 2019

fubuloubu commented Jan 4, 2019

jacqueswww commented Jan 4, 2019

jacqueswww left a comment •

edited

Loading

fubuloubu commented Jan 11, 2019 •

edited

Loading

jacqueswww commented Jan 11, 2019 •

edited

Loading

fubuloubu commented Jan 12, 2019

jacqueswww commented Jan 13, 2019

fubuloubu commented Jan 15, 2019

charles-cooper commented Jan 15, 2019

fubuloubu commented Jan 15, 2019 •

edited

Loading

charles-cooper commented Jan 15, 2019

fubuloubu commented Jan 15, 2019

charles-cooper commented Jan 15, 2019

fubuloubu commented Jan 15, 2019

Structs as args #1115

Structs as args #1115

Conversation

charles-cooper commented Dec 3, 2018 • edited Loading

- What I did

WIP Checklist

- How I did it

- How to verify it

- Description for the changelog

fubuloubu commented Jan 3, 2019

charles-cooper commented Jan 3, 2019

fubuloubu commented Jan 3, 2019

charles-cooper commented Jan 3, 2019

fubuloubu commented Jan 4, 2019

fubuloubu commented Jan 4, 2019

charles-cooper commented Jan 4, 2019

fubuloubu commented Jan 4, 2019

fubuloubu commented Jan 4, 2019

jacqueswww commented Jan 4, 2019

jacqueswww left a comment • edited Loading

Choose a reason for hiding this comment

fubuloubu commented Jan 11, 2019 • edited Loading

jacqueswww commented Jan 11, 2019 • edited Loading

fubuloubu commented Jan 12, 2019

jacqueswww commented Jan 13, 2019

fubuloubu commented Jan 15, 2019

charles-cooper commented Jan 15, 2019

fubuloubu commented Jan 15, 2019 • edited Loading

charles-cooper commented Jan 15, 2019

fubuloubu commented Jan 15, 2019

charles-cooper commented Jan 15, 2019

fubuloubu commented Jan 15, 2019

charles-cooper commented Dec 3, 2018 •

edited

Loading

jacqueswww left a comment •

edited

Loading

fubuloubu commented Jan 11, 2019 •

edited

Loading

jacqueswww commented Jan 11, 2019 •

edited

Loading

fubuloubu commented Jan 15, 2019 •

edited

Loading