Consider using fallback {} instead of function() {} (S)

[axic]

Not sure which is better:

• omitting the parentheses makes sense because it can have no arguments
• omitting the parentheses doesn't make sense because it is inconsistent with the rest of the language (and could be confused with modifier areas)

[federicobond]

I like that it makes it easier for beginners to look it up when they find such function in the code. This has confused more than one beginner. I prefer not omitting the parentheses because we are still talking about a function.

[decanus]

What about adding a modifier instead, similar to payable.

function foo() fallback {}

[shrugs]

my 2 cents: fallback () {}. It's also worth noting that you'll probably want something like

fallback () payable {
}

especially because

fallback payable {
}

looks like a weird anonymous constructor function with two modifiers in front of it.

[axic]

@decanus Fallback is different to payable/constructor as there can only be a single fallback function. And given its risky use may warrant its on syntactical element.

[VoR0220]

The current setup has always made sense to me in the sense of the EVM because it's the "empty" function signature. AKA the default in a switch statement.

[federicobond]

Right, but it's one of those things that only make sense after you learn about it, and it's not easy to find the right answers.

[VoR0220]

Good point.

[chriseth]

Actually I think we should even split the semantics of the fallback function into two:

One function that is called only if there is value and zero data (the "receive ether trigger function") and one function that is called as a "wildcard", if no other function matches (this should be considered a low-level feature).

[redsquirrel]

I like the idea of splitting it up.

…

On Mon, Dec 4, 2017 at 12:08 PM chriseth ***@***.***> wrote: Actually I think we should even split the semantics of the fallback function into two: One function that is called only if there is value and zero data (the "receive ether trigger function") and one function that is called as a "wildcard", if no other function matches (this should be considered a low-level feature). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#3198 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAJ0LkyhhXChwpUE5xo6y9p5Iecyc2lks5s89KhgaJpZM4QcinE> .

[federicobond]

I have been thinking about that too. Utility contracts, for example, may have a single fallback function that decodes parameters and stores or forwards them somewhere else.

[VoR0220]

I like this idea. It adds the needed "default" variable to the current switch case concept wrt to how functions in a contract currently execute.

[chriseth]

@axic what do you think about having a wildcard function and a "receive ether" function?

[VoR0220]

@chriseth what is a "wildcard function" in this instance?

[chriseth]

A wildcard function is one that is executed if a function selector is given, but it does not match any defined functions. The "receive ether" function is run if no calldata is given.

[VoR0220]

Ahhhh...so it's basically a default logic statement as opposed to a default receive statement. You're splitting the fallback into two portions...which tbf is a good design decision imo for safety, but how to do this gracefully in the language is a different matter. What would the syntax look like?

[axic]

(I actually thought I've answered this issue.)

@axic what do you think about having a wildcard function and a "receive ether" function?

Sounds good, but not sure about the naming and if the only point of the non-wildcard function is to receive ether, then it would have payable set implicitly or would it still require it to be written explicitly?

I see a couple of different options:

• 1. transfer for empty calldata and non-zero value
• 2. empty for empty calldata and no matter the value
• 3. wildcard for non-empty calldata and fallback for empty calldata

Though I think we cannot really use the words fallback and wildcard together as it seems confusing.

Option 1):

contract C {
  function f() {}

  wildcard() { }
  wildcard() payable { }

  // allow receiving incoming transfers
  transfer() payable { } // payable must be explicitly declared here
  transfer() { } // invalid syntax

  // default is not accepting incoming transfers if `transfer()` is not declared

Option 2):

contract C {
  function f() {}

  wildcard() { }
  wildcard() payable { }

  // allow receiving incoming transfers
  empty() payable { }

  // explicitly disallow incoming transfers (this the default)
  empty() { }

Option 3):

contract C {
  function f() {}

  wildcard() { }
  wildcard() payable { }

  // allow receiving incoming transfers
  fallback() payable { }

  // explicitly disallow incoming transfers (this the default)
  fallback() { }

[leonardoalt]

I like the idea of having two separate functions:

• receive() for empty calldata and non-zero value, with parentheses and implicit payable
• fallback() for non-empty calldata and any value, with parentheses

[chriseth]

I would prefer receive over transfer, although I would say this is still not the best wording.

Also, fallback should either

• have an argument of bytes (the calldata) and return bytes (the return data)
• or be forced to be followed by an assembly block

and it should somehow be made clear that this is a very low-level function that should usually not be implemented.

[ekpyron]

Splitting into two functions is a good idea.
Regarding syntax: I would use parentheses, as in falllback() {} (it seems everybody agreed on that).
Not sure about what's the best names, yet, though.

[GNSPS]

Using the parentheses would make this a breaking change, right?

Asking because I remember I had some contracts for testing (not on mainnet) that had a function called fallback() and those would no longer behave the same way (I'm guessing that no function ID would make it into the beginning switch case).

[axic]

function fallback() and fallback() would be parsed differently, so function fallback() should still be valid.

[GNSPS]

Ooops, completely missed that! 😄

[axic]

Also relevant: #2630.

[chriseth]

It might make sense to not allow the wildcard function together with any other function, but that probably conflicts with upgradable contracts.

[chriseth]

We might also have the wildcard directly drop into inline assembly to show that it is a low-level feature.

[ekpyron]

Crazy proposal: we could keep everything to fallback and distinguish the purpose based on the signature instead:

• fallback() {}: called for empty calldata without value - cannot be payable
• fallback(uint256 value) payable {}: called for empty calldata with value - has to be payable
• fallback(bytes4 sig) {}: called for non-matched signature without value - cannot be payable
• fallback(bytes4 sig, uint256 value) payable {}: called for non-matched signature with value - has to be payable

This would prevent us from flooding the language with more and more keyboards and is still entirely un-ambiguous and explicit...

[axic]

That sounds like a reasonably crazy idea. Not sure I like that I like it 😉

[leonardoalt]

I like it too.
But why not omit uint256 value and just use msg.value? Then payable would be the only difference

[ekpyron]

And then we can skip sig and just use msg.sig and close the issue, since that's what we already have :-D.

[leonardoalt]

Not really, since the functions wouldn't be split then

[ekpyron]

I wasn't being serious :-). A potential problem in skipping value is that we usually don't allow the same function to both have a "payable" and a "non-payable" version - but fallbacks could just be an exception there, so maybe not a problem...

[ekpyron]

The problem is that to be entirely un-ambiguous we need to require the value argument to always be > 0 (or to always have msg.value > 0 in the payable case, if we skip the argument). And this problem might be enough to tear the whole idea down, I'm not sure :-).

[chriseth]

I would prefer to make it more visible that a contract is doing something weird. A contract should not just react on all signatures. If it does, it should be a proxy and have no other functions and people should really know what they are doing. In contrast, a contract that has some functions and can also receive ether is something "regular" and the function that is called when the contract is sent ether should be recognizable from its name (even with limited prior knowledge about the EVM).

[ekpyron]

The presence of a fallback(bytes4 sig) version (and/or the payable variant) could just not be allowed, if the contract has any other functions (except other fallbacks maybe)...

[ekpyron]

But yes, I tend to agree that it's probably better to introduce an easily recognizable name for the "receive ether" function - if we can come up with a good one, of course.

[chriseth]

Proposal by @axic: Call the function receiveEther() payable { ... } to make it more explicit. After all, the community should move away from plain Ether anyway, so it is good to make it explicit.

Only called with zero data and non-zero value.

Fallback function (fallback() [payable] [returns (bytes memory)] { ... }) reverts by default. It is called if no other function matches.

If there is a payable fallback function but no receiveEther function, issue a warning.

[chriseth]

There can only be at most one fallback function. Also of course at most one receiveEther function.

[loredanacirstea]

How about fallback() payable calldata {}?
Probably another name than calldata, as this one already has its use, but I used it for clarifying intent.
No calldata -> empty msg.data.

[chriseth]

@loredanacirstea I would really like to avoid the name fallback for the situation of receiving Ether (without data) - what does it have to do with "falling back"? It would be a smaller change, but I don't see a justification apart from the historical one. Please correct me if I misunderstood you.

[loredanacirstea]

Ah, I'm not set on the name. It can be called something else. I was suggesting to only use 1 function with two optional modifiers: payable & calldata ("calldata" or another name).

[leonardoalt]

If there is a payable fallback function but no receiveEther function, issue a warning.

@chriseth @axic I guess this is the incentive to actually implement the receiveEther function? Otherwise people would just keep things the way they are and it'd be the same.

[ekpyron]

Alternative to receiveEther: we could just call it ether() payable - that's already a keyword and it should be clear that the ether is received...

For the other case I suggested to add arguments to fallback, e.g. fallback(bytes4 sig, bytes calldata data) to make it clear that it is only called with at least a signature - but we had multiple objections against that: (1) it's not entirely clear if data starts at calldata offset 0 or 4 and (2) we would probably need to generally revert for at least one, but less than 4 bytes of calldata and that might be too restrictive.

[ekpyron]

I'll start implementing this as fallback() and ether() - switching to different names in the process should be trivial.
Another open question: should it be required to annotate any or both of them as external and should it be required to mark ether() as payable?

[axic]

To clarify, do we now only allow two cases?

1. calldata length > 0 && ether == 0 -> "fallback"
2. calldata length == 0 && ether != 0 -> "ether"

[ekpyron]

@axic That's what viper does, but I think we don't want to force ether==0 for the "fallback" case, but instead allow "fallback" to be "payable", i.e the basic distinction is

1. calldata length > 0 --> "fallback"
2. calldata length == 0 --> "ether"

And case 1 is further divided as:
1a. ether > 0 and "fallback" was declared payable ==> fine and executes "fallback"
1b. ether > 0 and "fallback" was not declared payable ==> revert
1c. ether = 0 ==> always fine and executes "fallback"

I'm currently still changing the parser, but there's still some details to iron out - e.g. what happens for "calldata length = 0" and "ether = 0"?

[ekpyron]

Also still open: should fallback be allowed to (optionally) return bytes memory? That was suggested above, but never definitively decided. I vote for it.
EDIT: we might only do it as second step, though - allowing it later won't be breaking.

[chriseth]

I think empty data should always execute the receive function.

[chriseth]

I'm voting for optionally returning bytes memory.

[ekpyron]

@chriseth now suggested to change the semantics slightly - if there is no receive-ether-function, then fallback will also be called for msg.data.length == 0, but we issue a warning suggesting to implement a receive-ether function.

[ekpyron]

Done in #7385