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

"send" as a reserved keyword collides with the ERC777 specification #1068

Closed
lsaether opened this issue Nov 8, 2018 · 4 comments
Closed
Labels
bug Bug that shouldn't change language semantics when fixed.

Comments

@lsaether
Copy link

lsaether commented Nov 8, 2018

Version Information

  • vyper Version: 0.1.0b4
  • pyethereum Version: none
  • OS: 4.18.16-arch1-1-ARCH
  • Python Version (python --version): 3.7.1

What's your issue about?

While implementing the ethereum/EIPs#777 specification as a Vyper contract I was unable to due to the collision of the specification's send method with the reserved keyword send in Vyper used for sending ether. Trying to declare a send method on a contract throws the following exception:

vyper.exceptions.FunctionDeclarationException: Function name invalid: send

How can it be fixed?

I see three options:

  • Vyper changes the send keyword to something else.
  • ERC777 changes the send method to something else.
  • As @mcdee suggested in the ERC777 issue, Vyper could allow for a contract method send that is different from the built-in function send. These two would be disambiguated by the way in which they are invoked. The built-in function could retain the naked invocation send <address, amount> while a method declared on the contract would be invoked with self.send(). From the external point of view only the contract method would be accessible.

Cute Animal Picture

image

@ben-kaufman
Copy link
Contributor

Thanks for raising that up. Personally, I think ERC-777 should change that in order to keep things more clear, as send is used both in Vyper and Solidity and I think it would be best to reserve that.
I'm not sure about that but I haven't seen much usage of this currently in production so I think changing the proposed ERC should be a problem.

Any thought from others here?

@mcdee
Copy link

mcdee commented Nov 11, 2018

@ben-kaufman a quick scan of 4byte shows the following submitted signatures with the name send:

send()
send(address)
send(address[])
send(address,address,address[],uint256[])
send(address,address[],address,uint256[])
send(address,address,uint256)
send(address,address[],uint256[])
send(address,bytes)
send(address,string)
send(address,uint256)
send(address[],uint256[])
send(address,uint256,bytes)
send(address[],uint256[],uint256)
send(address,uint256,uint256,uint256)

There might be other databases out there that could provide details of the prevalence of these methods, but it's far from just a problem with ERC-777 and blocking keywords from being method names is going to be a general compatibility issue.

@ben-kaufman
Copy link
Contributor

@mcdee Yeah I agree it might be a problem with compatibility. Maybe as you said we can remove it from the reserve as the self. could be sufficient. I'm still not sure about that but personally both solutions sound to me better than the first option.

@jacqueswww
Copy link
Contributor

jacqueswww commented Nov 14, 2018

Removing send from ERC777 is the best option.

But whitelisting send as a function name is our best realistic option - and probably the on we will go with. There are no other keywords we can use for the send functionality, and programmers will naturally want to create methods named send.

@jacqueswww jacqueswww added the bug Bug that shouldn't change language semantics when fixed. label Nov 28, 2018
@jacqueswww jacqueswww added this to To do in Fix the Beta via automation Nov 28, 2018
Fix the Beta automation moved this from To do to Done Dec 12, 2018
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.
Projects
No open projects
Fix the Beta
  
Done
Development

No branches or pull requests

4 participants