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 interface fixes #1842

Merged
merged 5 commits into from
Feb 10, 2020

Conversation

iamdefinitelyahuman
Copy link
Contributor

@iamdefinitelyahuman iamdefinitelyahuman commented Feb 3, 2020

What I did

Fix issues with bytes, string and fixed168x10 when importing an ABI as an interface. Fixes #1832

How I did it

  1. When determining if an interface has been implemented, ignore the length on bytes and string.
  2. Substitute fixed168x10 for decimal when converting ABI to AST.
  3. For bytes and string arrays, use a length of 1024*1024 in inputs, 1 in outputs. The compiler does not implicitly truncate values so this does not create an issue when a limit is exceeded.

How to verify it

Run the tests. I expanded the cases around ABIs as interfaces to check each type as an input and output.

Cute Animal Picture

image

@fubuloubu fubuloubu self-requested a review February 3, 2020 21:04
@iamdefinitelyahuman iamdefinitelyahuman force-pushed the json-interface-fixes branch 2 times, most recently from dbf189b to c1b191d Compare February 3, 2020 21:13
@michwill
Copy link

michwill commented Feb 6, 2020

Excellent, does it also fix the same issue with structs which contain uint256 arrays?

@michwill
Copy link

michwill commented Feb 6, 2020

E.g. this:

Z256: constant(uint256) = 0 

struct A:
    many: uint256[3]
    one: uint256

@private
@constant
def foo(_many: uint256[3], _one: uint256) -> A:
    a: A = A({many: [Z256, Z256, Z256], one: 0})
    a.many = _many
    a.one = _one
    return a

@public
def bar():
    m: uint256[3] = [Z256, Z256, Z256]
    m[0] = 42
    m[1] = 2020
    m[2] = 21000000
    out: A = foo(m, 33)
    assert out.many[0] == m[0]
    assert out.many[1] == m[1]
    assert out.many[2] == m[2]
    assert out.one == 33

@iamdefinitelyahuman
Copy link
Contributor Author

@michwill good call, I'll have a look at that as well

@michwill
Copy link

michwill commented Feb 6, 2020

As far as I can tell, this PR doesn't fix it for structs, and probably the solution would be suitable for this PR, too

Copy link
Member

@fubuloubu fubuloubu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, not really a big fan of this approach, but if there's no better way to solve the problem, the least we can do is add more notes to the code about it.

vyper/signatures/interface.py Show resolved Hide resolved
tests/parser/functions/test_interfaces.py Show resolved Hide resolved
vyper/signatures/interface.py Outdated Show resolved Hide resolved
vyper/signatures/interface.py Show resolved Hide resolved
vyper/signatures/interface.py Show resolved Hide resolved
vyper/signatures/interface.py Show resolved Hide resolved
@iamdefinitelyahuman
Copy link
Contributor Author

note to self, need to open another issue re: structs in json interfaces

@fubuloubu fubuloubu merged commit 0aa176f into vyperlang:master Feb 10, 2020
@iamdefinitelyahuman iamdefinitelyahuman deleted the json-interface-fixes branch February 11, 2020 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot generate AST from ABI containing string or bytes
3 participants