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: deterministic proxies with CREATE2 and EIP1167 #2460

Merged
merged 9 commits into from
Sep 18, 2021

Conversation

skellet0r
Copy link
Contributor

@skellet0r skellet0r commented Sep 18, 2021

What I did

Extended the create_forwarder_to built-in by adding functionality to deploy minimal proxies using the CREATE2 opcode.

The following is now possible:

@external
def foo(_target: address, _salt: bytes32) -> address:
    return create_forwarder_to(_target, salt=_salt)

The deployment address of the proxy can be determined with the following:

keccak256(0xff ++ deployingAddr ++ salt ++ keccak256(bytecode))[12:]

where bytecode is equal to:

pre = "0x602D3D8160093D39F3363d3d373d3d3d363d73"
post = "0x5af43d82803e903d91602b57fd5bf3"
bytecode = pre ++ target ++ post

How I did it

Modified the built-in to accept and handle new keyword salt.

How to verify it

Check the new tests added. A set of tests verifying syntax, and a test verifying runtime execution.

Description for the changelog

Added support for deterministic deployment of minimal proxies using CREATE2

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov-commenter
Copy link

codecov-commenter commented Sep 18, 2021

Codecov Report

Merging #2460 (fd4b30c) into master (b418e9f) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2460      +/-   ##
==========================================
- Coverage   84.67%   84.66%   -0.02%     
==========================================
  Files          94       94              
  Lines        9438     9446       +8     
  Branches     2206     2208       +2     
==========================================
+ Hits         7992     7997       +5     
- Misses        937      939       +2     
- Partials      509      510       +1     
Impacted Files Coverage Δ
vyper/builtin_functions/functions.py 88.59% <100.00%> (-0.25%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b418e9f...fd4b30c. Read the comment docs.

@charles-cooper
Copy link
Member

Nice work and neat PR. Is there a corresponding issue that this closes?

Also I think this can be done without the is_deterministic flag. You can just look for the presence of the salt kwarg and use that to decide whether to use CREATE2.

@skellet0r
Copy link
Contributor Author

skellet0r commented Sep 18, 2021

True, I started off without the is_deterministic flag but didn't want to exclude EMPTY_BYTES32 as the salt value. Although, how often will that ever be used as a salt ? 🤷‍♀️

And hmm, technically it's related to issue #649, but since this is embedded in create_forwarder_to, not sure it'd be eligible to close that issue.

Edit: ah actually I could just annotate that it's deterministic during the annotation phase ...

@charles-cooper
Copy link
Member

I think you can just check in the AST whether the keyword is set at all. Not sure if you can do it in the implementation of CreateForwarderTo itself

@charles-cooper
Copy link
Member

Yeah I think in build_LLL you can just check expr.keywords

@skellet0r
Copy link
Contributor Author

Closes #649

stolen from charles, booleans affecting control flow, better to name them `should_*`
@charles-cooper
Copy link
Member

🔥 🔥

@charles-cooper charles-cooper merged commit fecf04c into vyperlang:master Sep 18, 2021
@skellet0r skellet0r deleted the feat/create2 branch September 18, 2021 23:38
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.

sick!

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.

None yet

4 participants