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

Proposal: add emit keyword; make it the only way to emit events #2877

Closed
veox opened this issue Sep 8, 2017 · 30 comments
Closed

Proposal: add emit keyword; make it the only way to emit events #2877

veox opened this issue Sep 8, 2017 · 30 comments

Comments

@veox
Copy link
Contributor

veox commented Sep 8, 2017

EDIT: Initially proposed log; changed later to emit.

See issue #3252 for another related change.


The issue

The ERC20 token standard describes a Transfer event, as well as a transfer() method.

Their "invocation" syntax is not identical:

transfer(address to, uint value);
Transfer(address from, address to, uint256 _value);

... yet similar enough to cause confusion.

This highlights an issue for prospective Solidity programmers, where care must be taken to avoid accidentally "mapping" externally-callable functions to similarly-named events. (Sadly, I'm hard-pressed to pull actual examples of this resulting in security vulnerabilities out of my hat, but I do vaguely remember this having been an issue as recently as last year, around TheDAO hack.)

For this reason, ConsenSys even suggests prefixing event names with Log, "to prevent the risk of confusion between functions and events".

A solution

Propose introducing a new keyword, tentatively named log emit. So, instead of:

event Transfer(address from, address to, uint256 _value);
// ...
Transfer(from, to, value);

we would have:

event Transfer(address from, address to, uint256 _value);
// ...
emit Transfer(from, to, value);

This makes a (general) semantic differentiation between a function call and logging of an event. (It also renders Consensys' recommendation obsolete.)

@axic
Copy link
Member

axic commented Sep 8, 2017

I might be misunderstanding this, but events are marked event while functions as function in Solidity, so your example becomes:

function transfer(address to, uint value);
event Transfer(address from, address to, uint256 _value);

@axic
Copy link
Member

axic commented Sep 8, 2017

Oh you mean to have a log keyword when emitting an event instead of having it look like as a regular function call?

@veox
Copy link
Contributor Author

veox commented Sep 10, 2017

Oh you mean to have a log keyword when emitting an event

Yes. :) Should have said "emit events" instead of "raise events".

EDIT: Perhaps emit would even work better.

@chriseth
Copy link
Contributor

This sounds like a good proposal! I'm fine with either log or emit. Syntactically, this is similar to new C(...), so nothing alien to the language.

@elenadimitrova
Copy link
Collaborator

The (token) transfer method is additionally confusing itself with the ether transfer method in Solidity.

@axic
Copy link
Member

axic commented Sep 14, 2017

@elenadimitrova that is solved with #2683.

@axic
Copy link
Member

axic commented Sep 14, 2017

I'd go with emit instead of log if we keep this syntax.

@DIDHAV
Copy link

DIDHAV commented Sep 22, 2017

I would very much prefer 'log'. This will be more intuitive to client/web3 developers and is more clear that there is some data coming from the blockchain to be read ok the client.

@MicahZoltu
Copy link
Contributor

I have never liked Log, since it really isn't logging IMO but rather event emission to external viewers. Logging in something I would expect to show up in an and only file that can be turned off/tuned at runtime.

@federicobond
Copy link
Contributor

I like this change, would also open the door to treating the event as a first-class object. i.e:

var event = Transfer(to, value);
if (success)
  emit event;

An alternative that does not involve adding a new keyword is:

Transfer(to, value).log();

@veox veox changed the title Proposal: add log keyword; make it the only way to emit events Proposal: add emit keyword; make it the only way to emit events Dec 22, 2017
@veox
Copy link
Contributor Author

veox commented Dec 22, 2017

emit seems to me like a better option than log now.

Metaphorically, logging suggests putting an entry in a journal that can be later looked up by the logger - which is not the case. Emitting suggests sending a message out of the current context.

(An event emitted by a contract is logged in the transaction receipt.)

@devzl
Copy link

devzl commented Jan 26, 2018

It would be a really good thing if people started using this recommendation, especially in standards propositions.

@chriseth chriseth self-assigned this Feb 16, 2018
@fulldecent
Copy link
Contributor

ERC20 is pending again?

@veox
Copy link
Contributor Author

veox commented Feb 16, 2018

@fulldecent Nope! This discusses Solidity syntax, ERC20 (as a standard/template) not affected.


EDIT: I've edited the OP to not refer to ERC20 as "pending".

@fulldecent
Copy link
Contributor

Ok, I saw "The (currently pending) ERC20 token standard " in the original post. It was like a flashback.

@chriseth
Copy link
Contributor

chriseth commented Feb 16, 2018

I first intended to implement this similar to new X(), but since emit Event() never returns anything, it is probably better to put this at the statement level similar to return x;, it also simplifies parsing and makes it possible to do this even without having a keyword, I think - or can anyone think of a way where

NonKeyword NonKeyword `(` ( (expression `,`)* expression )? `)`

would be valid syntax at expression level?

NonKeyword NonKeyword; could be the declaration of a variable, but not with an opening parenthesis following. Not sure if we want to make the parser perform so much look-ahead, though.

@veox
Copy link
Contributor Author

veox commented Feb 16, 2018

Ah, emit not returning anything would indeed put it in a different bag than new (or copyof/shared). :/

@chriseth There's a hanging tick in the code block, making it harder to read. But I can't come up with syntax that would be valid in that case anyway.

@fulldecent
Copy link
Contributor

@devzl I am working on standards ERC-165 and ERC-721. Correctness and best practice in standards are important to me.

Could someone please tell me which version of Solidity will have this? If we are targeting that version that I can adopt this new language.

@veox
Copy link
Contributor Author

veox commented Feb 19, 2018

@fulldecent Looks like maybe 0.4.21?.. (See PR #3538.)

IMO if you specify use a strict pragma (e.g. current 0.4.19 0.4.20) in the examples then you don't need to worry about this.

@fulldecent
Copy link
Contributor

Thank you.

Just in general I think people should be able to skip to the examples and learn best practices that way.

Sort of like how I needed to decide between using /// or /** for natspec comments. We chose /// because the first example in solidity uses that.

@axic
Copy link
Member

axic commented Mar 30, 2018

Raised by @ProphetDaniel:

I believe the most appropriate name for this new word would be trigger rather than emit.

Why do you think trigger is better?

@ProphetDaniel
Copy link

ProphetDaniel commented Mar 30, 2018

Why do you think trigger is better?

@axic because if you ask famous search engines:

  • for "trigger event" you receive 449,000 occurrences
  • for "emit event" only 17,400 occurrences.

So this means trigger verb for event is more popular and so will be more easily understood by computer science students because of that.

Maybe the reason behind this popular conception is that an event is atomic. That means an event is so impulsive that its endurance tends to zero like triggering a gun shot.

@fulldecent
Copy link
Contributor

This is a good argument. Also "trigger log" is more prevalent than "emit log".

Source: https://trends.google.com/trends/explore?q=trigger%20event,emit%20event

@axic
Copy link
Member

axic commented Mar 30, 2018

There is one reasoning for emit: trigger suggests something definitely will happen when a log is "emitted", while emit only suggests it will be transmitted and free for anyone to be picked up.

The stress is on definitely, because there is no guarantee something/someone is listening for the events and that something will happen as a result.

This reasoning may be too thin to justify the keyword, but explains the reasoning behind it.

@ProphetDaniel
Copy link

emit just like light emission, suggests a signal is being emitted for a while.

trigger just like a firearm shot, suggests a signal will be triggered instantaneously with virtually no endurance.

As events are impulsive (endurance tends to zero) it looks clear to me trigger is more appropriate keyword here.

@ProphetDaniel
Copy link

There is one reasoning for emit: trigger suggests something definitely will happen when a log is "emitted"

Indeed, what definitely happens is:

  • The event is dispatched when the code trigger anEvent(), for example, is executed.

while emit only suggests it will be transmitted and free for anyone to be picked up.

As the underlying technology is a blockchain, the dispatched event is registered on the ledger. So what becomes available to be picked up is not the event, but the event record.

The stress is on definitely, because there is no guarantee something/someone is listening for the
events and that something will happen as a result. This reasoning may be too thin to justify the keyword, but explains the reasoning behind it.

An event could be lost by an intelligent agent which was sleeping at the time the event was triggered. But the agent can also look at the ledger when it wakes up to understand if while sleeping a relevant event happened.

@chriseth
Copy link
Contributor

@ProphetDaniel curiously, I have the opposite impression: trigger means that a mechanism is activated, the mechanism is integrated with what I am doing and I want to wait for the mechanism to finish doing its job (gun analogy: I want to wait for the bullet to exit the gun, although my direct interaction is only with the trigger). While with emit I have the impression that I do not care about what happens afterwards (light analogy: I send out the light, but I do not care where it goes).

Note that emitting light and triggering a gun both send out something, but when triggering a gun, I implicitly include the mechanism of the gun. Compare with emitting the bullet, which is also just the single action.

NB: I'm not a native speaker, so this might all be totally wrong.

@fulldecent
Copy link
Contributor

@ProphetDaniel

Emit a photon

@Thaina
Copy link

Thaina commented Sep 12, 2018

I don't know if this is the correct place but I would like to ask how the emit and subscribe cost gas? Is the number of subscriber will multiply in cost of emit?

@chriseth
Copy link
Contributor

Subscribers are off-chain actors, only emitting the event costs gas once. See the documentation for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests