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

Implement new with CREATE2 and function call options #8177

Merged
merged 7 commits into from
Jan 23, 2020
Merged

Conversation

Marenz
Copy link
Contributor

@Marenz Marenz commented Jan 22, 2020

Missing:

  • Proper tests for the generated CREATE2 code
  • CodeGen for calling functions using the new fn{value:123, gas:43}() syntax or an "not yet implemented" error
  • tests for the type checks on the options (i.e. salt has to be bytes32, etc)
  • tests and implementation in type checker that options cannot be given twice (neither in same options set (f{value: 0, value: 0}), nor by applying the options set twice (f{value: 0}{value: 0}))
  • tests and implementation that salt can only be used starting from the EVM version where create2 was introduced (@Marenz)
  • ast json import (@erak)
  • documentation (@chriseth)
  • test for options with overloaded functions (@chriseth) Overload resolution for .value()-modified functions #526 (comment)

Problems:

We want to parse

callableexpression{value:123, gas:53}(params);

but this also matches

try this.ext() { r = s; } catch (bytes memory) { r = s; }

i was able to work around this, checkout the last commit (parser hack).
However this hack still has a weakness. It can't parse

try this.fn(){value:3, salt:2}(){value:5}() {  } catch (bytes memory) {  }

that is, if fn returns a function and we want to call that function with call options again, it will error (but only in the try/catch case)

I can maybe hack-fix that as well, but it will be even uglier.

I am open to alternative solutions.

@chriseth chriseth force-pushed the new2-2136 branch 2 times, most recently from 40b8286 to 9014960 Compare January 22, 2020 23:27
arguments.

In particular, the counter ("nonce") is not used. This allows for more flexibility
in creating contracts and in particular, you are able to deriven the address of the
Copy link
Contributor Author

Choose a reason for hiding this comment

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

deriven -> derive

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you are repeating in particular here very near each other

@chriseth chriseth marked this pull request as ready for review January 23, 2020 14:16
@chriseth chriseth changed the title Implement new with CREATE2 Implement new with CREATE2 and function call options Jan 23, 2020
@chriseth chriseth force-pushed the new2-2136 branch 2 times, most recently from ca9f0ff to 92d808e Compare January 23, 2020 18:30
chriseth
chriseth previously approved these changes Jan 23, 2020
@chriseth chriseth force-pushed the new2-2136 branch 3 times, most recently from 1c783dc to 844c924 Compare January 23, 2020 20:16
@chriseth chriseth merged commit 45caaf5 into develop Jan 23, 2020
@chriseth chriseth deleted the new2-2136 branch January 23, 2020 22:19
@GriffGreen
Copy link

Amazing work guys!

@dmihal
Copy link

dmihal commented Apr 28, 2020

Are there any plans to add support for calculating contract addresses?

@axic
Copy link
Member

axic commented Apr 28, 2020

There was a discussion #2136 (comment), but I think we forgot to follow up on it.

@chriseth
Copy link
Contributor

Created #8798 for this.

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.

6 participants