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

`create_with_code_of()` name is somewhat misleading #1177

Closed
GNSPS opened this issue Jan 4, 2019 · 6 comments
Closed

`create_with_code_of()` name is somewhat misleading #1177

GNSPS opened this issue Jan 4, 2019 · 6 comments

Comments

@GNSPS
Copy link

@GNSPS GNSPS commented Jan 4, 2019

What's your issue about?

I was not familiar with the inner workings of the Vyper compiler before diving deep into the Uniswap codebase.
While inspecting their contracts I always assumed that create_with_code_of() would CODECOPY the whole code of the target contract but, once I dug into Vyper and much to my big surprise, it creates a forwarder (which makes a lot of sense reusability-wise).

How can it be fixed?

Like I was fooled I believe that others might be and I would propose a more expressive name for this (given its name is already long), something along the lines of: create_forwarder_to or create_delegated_copy_of

Cute Animal Picture

@fubuloubu

This comment has been minimized.

Copy link
Member

@fubuloubu fubuloubu commented Jan 4, 2019

That is a great point! We should discuss at our meeting on Monday

@jacqueswww

This comment has been minimized.

Copy link
Collaborator

@jacqueswww jacqueswww commented Jan 6, 2019

Agreed, I think we should think of something clearer. create_delegated_copy_of and create_forwarder are good candidates.

@jacqueswww

This comment has been minimized.

Copy link
Collaborator

@jacqueswww jacqueswww commented Jan 7, 2019

Approved not decided on the right name yet, other suggested names:
create_proxy
create_instance_of

@jacqueswww

This comment has been minimized.

Copy link
Collaborator

@jacqueswww jacqueswww commented Feb 20, 2019

@charles-cooper @fubuloubu I am keen on getting this fixed. I think the term forwarder best describes the behaviour, as proxies is quite a broad IT term. Can I go ahead with create_forwarder_to ?

@jacqueswww jacqueswww added this to To do in Fix the Beta via automation Feb 20, 2019
@fubuloubu

This comment has been minimized.

Copy link
Member

@fubuloubu fubuloubu commented Feb 20, 2019

SGTM

@jacqueswww

This comment has been minimized.

Copy link
Collaborator

@jacqueswww jacqueswww commented Feb 21, 2019

Cool, I will pick this one up next - is simple enough hehe ;)

@jacqueswww jacqueswww added the Approved label Mar 4, 2019
Fix the Beta automation moved this from To do to Done Mar 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Fix the Beta
  
Done
3 participants
You can’t perform that action at this time.