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

feat[lang]: add revert_on_failure kwarg for create builtins #3844

Merged

Conversation

DanielSchiavini
Copy link
Contributor

@DanielSchiavini DanielSchiavini commented Mar 7, 2024

Fixes #3744

What I did

  • Add revert_on_failure: bool = True to contract creation built-in functions

How I did it

  • Register the kwargs to the builtin methods
  • Add tests
  • Pass it through to the _create_ir method

How to verify it

  • Tests are included

Commit message

per title. add `revert_on_failure=` kwarg for create builtins to mirror
`raw_call()`. among other things, this makes it easier to calculate
create addresses.

Description for the changelog

Support kwarg revert_on_failure: bool = True for contract creation built-in functions

Cute Animal Picture

image

@DanielSchiavini DanielSchiavini marked this pull request as ready for review March 7, 2024 17:20
Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

might be nice to have specific tests for the revert_on_failure behavior

Copy link
Collaborator

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

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

We should also update the docs for these built-ins as part of that PR IMO.

Convention-wise, we should return the address 0x0 if revert_on_failure=False and the creation fails, agreed @charles-cooper?

@codecov-commenter
Copy link

codecov-commenter commented Mar 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.20%. Comparing base (c54d3b1) to head (44769c6).
Report is 1 commits behind head on master.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3844      +/-   ##
==========================================
- Coverage   90.02%   89.20%   -0.82%     
==========================================
  Files          95       95              
  Lines       14411    14411              
  Branches     3193     3193              
==========================================
- Hits        12973    12855     -118     
- Misses       1012     1110      +98     
- Partials      426      446      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@DanielSchiavini DanielSchiavini changed the title feat: revert_on_failure for contract creation feat[codegen]: revert_on_failure for contract creation Mar 25, 2024
@charles-cooper charles-cooper changed the title feat[codegen]: revert_on_failure for contract creation feat[lang]: add revert_on_failure kwarg for create builtins Apr 7, 2024
@charles-cooper charles-cooper enabled auto-merge (squash) April 7, 2024 23:18
Copy link
Collaborator

@pcaversaccio pcaversaccio left a comment

Choose a reason for hiding this comment

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

Can we please update the docs before merging.

@charles-cooper charles-cooper enabled auto-merge (squash) April 7, 2024 23:29
@charles-cooper charles-cooper merged commit 8fcbde2 into vyperlang:master Apr 7, 2024
149 checks passed
@DanielSchiavini DanielSchiavini deleted the 3744/revert_on_failure branch April 9, 2024 15:28
electriclilies pushed a commit to electriclilies/vyper that referenced this pull request Apr 27, 2024
…ang#3844)

per title. add `revert_on_failure=` kwarg for create builtins to mirror
`raw_call()`. among other things, this makes it easier to calculate
create addresses.

---------

Co-authored-by: Charles Cooper <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support kwarg revert_on_failure: bool = True for contract creation built-in functions
4 participants