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

Deployable bytecode attempts to access calldata #1399

Closed
Pet3ris opened this issue Apr 18, 2019 · 9 comments · Fixed by #1400
Closed

Deployable bytecode attempts to access calldata #1399

Pet3ris opened this issue Apr 18, 2019 · 9 comments · Fixed by #1400
Labels
bug Bug that shouldn't change language semantics when fixed. enhancement

Comments

@Pet3ris
Copy link

Pet3ris commented Apr 18, 2019

Hi There,

I'm trying to understand vyper constructor usage patterns. When compiling the example Vyper erc20 contract, the output (deployed) bytecode looks like this:

0x600035...000f3

The second opcode 0x35 is CALLDATALOAD. Given that in the EVM specification, contract creation calls assume calldata == [], how is calldata used in this case?

In Solidity, a comparable example would not include CALLDATALOAD in the executable part of deployed bytecode, reserving it instead for the runtime bytecode portion.

Let me know if I'm misunderstanding!

@jacqueswww
Copy link
Contributor

@Pet3ris the default -f bytecode includes the init code to deploy the contract. So if you have a __init__ function it would pass the information from calldata. I think that answers the question?

@Pet3ris
Copy link
Author

Pet3ris commented Apr 18, 2019

Hi @jacqueswww - this is precisely why I'm asking. According to the yellow paper, contract creation transactions come with empty calldata:

image

In section "Contract Creation", the specific environment set-up is listed here (note that I_d, the calldata string is empty).

image

The Solidity convention is to embed constructor arguments as part of the original bytecode that gets passed during contract creation (i in the yellow paper). See here.

Has Vyper adopted a different convention where the contract call assumes constructor arguments are passed during the contract creation phase?

@jacqueswww
Copy link
Contributor

jacqueswww commented Apr 18, 2019

@Pet3ris I don't think we use anything different, also you can check our tests also show we use standard web3 to deploy and test init.

given example program:

val: public(uint256)

@public
def __init__(a: uint256):
    self.val = a

Check the IR output (-if ir):

[seq,
  [mstore, 28, [calldataload, 0]],
  [mstore, 32, 1461501637330902918203684832716283019655932542976],
  [mstore, 64, 170141183460469231731687303715884105727],
  [mstore, 96, -170141183460469231731687303715884105728],
  [mstore, 128, 1701411834604692317316873037158841057270000000000],
  [mstore, 160, -1701411834604692317316873037158841057280000000000],
  # Line 3
  [codecopy, 320, ~codelen, 32],
  [assert, [iszero, callvalue]],
  # Line 5
  [sstore,
    '0' <self.val>,
    [mload, '320' <a>]],
  [return,
    0,
    [lll,
      [seq,
        [mstore, 28, [calldataload, 0]],
        [mstore, 32, 1461501637330902918203684832716283019655932542976],
        [mstore, 64, 170141183460469231731687303715884105727],
        [mstore, 96, -170141183460469231731687303715884105728],
        [mstore, 128, 1701411834604692317316873037158841057270000000000],
        [mstore, 160, -1701411834604692317316873037158841057280000000000],
        # Line 1
        [if,
          [eq, [mload, 0], '1013691446' <val()>],
          [seq,
            [assert, [iszero, callvalue]],
            # Line 3
            [mstore, 0, [sload, '0' <self.val>]],
            [return, 0, 32],
            # Line 1
            stop]],
        /* Default function */ [revert, 0, 0]],
      0]]]

Specifically:

[codecopy, 320, ~codelen, 32],

Seems to be in order ? ( I realise I answered incorrectly about calldata copying at first, sorry!).

@jacqueswww
Copy link
Contributor

jacqueswww commented Apr 18, 2019

Looking at the above -f asm and -f opcodes output ... I have no idea what the CALLDATALOAD is doing there and I believe we can remove it. [mstore, 28, [calldataload, 0]],

@jacqueswww jacqueswww added bug Bug that shouldn't change language semantics when fixed. enhancement labels Apr 18, 2019
@jacqueswww
Copy link
Contributor

jacqueswww commented Apr 18, 2019

@Pet3ris
Copy link
Author

Pet3ris commented Apr 18, 2019

@jacqueswww thanks a lot for the clarification and clearly wrong to assume you're violating the spec :)

@jacqueswww
Copy link
Contributor

jacqueswww commented Apr 18, 2019

@Pet3ris No problem! Great catch! So I just did a fix:

[seq,
  [mstore, 32, 1461501637330902918203684832716283019655932542976],
  [mstore, 64, 170141183460469231731687303715884105727],
  [mstore, 96, -170141183460469231731687303715884105728],
  [mstore, 128, 1701411834604692317316873037158841057270000000000],
  [mstore, 160, -1701411834604692317316873037158841057280000000000],
  # Line 3
  [codecopy, 320, ~codelen, 32],
  [assert, [iszero, callvalue]],
  # Line 5
  [sstore,
    '0' <self.val>,
    [mload, '320' <a>]],
  [return,
    0,
    [lll,
      [seq,
        [mstore, 28, [calldataload, 0]],
        [mstore, 32, 1461501637330902918203684832716283019655932542976],
        [mstore, 64, 170141183460469231731687303715884105727],
        [mstore, 96, -170141183460469231731687303715884105728],
        [mstore, 128, 1701411834604692317316873037158841057270000000000],
        [mstore, 160, -1701411834604692317316873037158841057280000000000],
        # Line 1
        [if,
          [eq, [mload, 0], '1013691446' <val()>],
          [seq,
            [assert, [iszero, callvalue]],
            # Line 3
            [mstore, 0, [sload, '0' <self.val>]],
            [return, 0, 32],
            # Line 1
            stop]],
        /* Default function */ [revert, 0, 0]],
      0]]]

Looks much better :) (calldata load for the function), nothing for the init.

@jacqueswww
Copy link
Contributor

Did a PR if you want to check it out :)

@Pet3ris
Copy link
Author

Pet3ris commented Apr 18, 2019

Nice!

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. enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants