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

byte array argument packing for method doesn't comply with ABI #743

Closed
denis-bogdanas opened this issue Apr 2, 2018 · 2 comments
Closed
Labels
bug Bug that shouldn't change language semantics when fixed.

Comments

@denis-bogdanas
Copy link

According to ABI specification dynamic part of byte arrays should be encoded like this:
enc(X) = enc(k) pad_right(X)

Right now they are properly encoded for events, but not for methods. For methods byte arrays are not padded to full chunks of 32 bytes, but are encoded like this:

enc(X) = enc(k) X

Curiously python ethereum API is compatible with this encoding, so this test passes:

def test_selfcall_code_zz(get_contract_with_gas_estimation):
    selfcall_code_6 = """
@public
def hardtest(arg1: bytes <= 64, arg2: bytes <= 64) -> bytes <= 128:
    return concat(arg1, arg2)
    """

    c = get_contract_with_gas_estimation(selfcall_code_6)
    assert c.hardtest(b"hello" * 5, b"hello" * 10) == b"hello" * 15

Looks like an issue in both ethereum API and Vyper, that will likely cause problems in production. Also harmonizing encoding format will allow using the same algorithm for both event and method args.

@jacqueswww
Copy link
Contributor

jacqueswww commented Apr 2, 2018

Very strange that it actually works! +1 for using pad_right.
And +1 for refactoring the packing of arguments code.

@jacqueswww jacqueswww added the bug Bug that shouldn't change language semantics when fixed. label Apr 3, 2018
jacqueswww added a commit to jacqueswww/vyper that referenced this issue Dec 19, 2018
@jacqueswww
Copy link
Contributor

Closing, this works now 😄 I also added a test for it: #1158

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

2 participants