Allow Event Definitions at File Level

[mudgen]

If events can be defined at the file level then free functions can emit events.

Suggestion by @haltman-at : If all events that are possibly emitted by a contract are added to its ABI, then they should also be added to the AST to be able to find out which events exactly are emitted (by AST ID).

[chriseth]

Related: #9765

I think that there should be a mechanism to access all events emitted from a contract, not just the ones declared there.

Any ideas how this could be handled? Should we include all events (and in the future also errors) that are emitted in the already existing abi field or should we introduce a new field (maybe 'effectiveABI' or 'indirectABI')?

@yann300 @alcuadrado @haltman-at

[alcuadrado]

Any ideas how this could be handled? Should we include all events (and in the future also errors) that are emitted in the already existing abi field or should we introduce a new field (maybe 'effectiveABI' or 'indirectABI')?

Wouldn't this always be incomplete? As which contracts are called can change at runtime.

[chriseth]

@alcuadrado the idea was to include all event calls that are part of the bytecode. Excluding delegatecall, these are all events that can be emitted from that address.

[alcuadrado]

I think I misunderstood you.

Let's say that you have:

contract A {
  event EA();

  function f() public {
    emit EA();
  }
}

contract B {
  event EB();
  event EC();
  A a;

  constructor(A _a) {
    a = _a;
  }

  function f() public {
    emit EB();
    a.f();
  }
}

The events for A would be EA. But what about the B ones?

1. Include all of its events EB, EC. -- This is what is currently happening.
2. Include all its events and A, because it's called from B. ie: EA, EB, EC`. -- This is what I understood.
3. Include the events from B that are effectively called: EB -- Is this what you meant?

[chriseth]

Yes, I meant 3, or maybe 1 + 3, so all that are defined in the contract plus those that are called. In your example, there is no difference, but take a look here:

library L {
  event LA();
  event LB();
  event LC();

  function f() internal {
    emit LB();
  }
}

contract B {
  event BA();
  function f() public {
    emit BA();
    emit L.LA();
    L.f();
  }
}

In that example, the events for B should be BA, LA and LB (the latter two maybe called L.LA and L.LB).

[haltman-at]

Hm, this is interesting. Right now Truffle Decoder already assumes that any contract can emit events from any library (due to delegatecalls). Roughly speaking -- first it checks the contract's ABI, and for each event scans the corresponding AST for more info (if possible -- if it can't find it then it just uses the info from the ABI). Then it does the same for all libraries in the project.

How to handle free events, if those become allowed, is an interesting question. "Effective ABI" is kind of hacky but it might be the most compatible thing. I think adding library/free functions to the ABI field proper would be... confusing. It certainly would cause problems with Truffle Decoder as it is -- I mean, I can alter Truffle Decoder, obviously, but it's not obvious to me what the proper way would be.

Like, let's say we're trying to decode a library event emitted by a particular contract; imagine this event has been added to the contract's ABI. Truffle Decoder (as it currently exists) will see the ABI entry, say "Oh, this event was defined in this contract", check the AST for it, fail to find it, and say "Darn, I guess we just have to decode in ABI-only mode". When in fact, it could have found the AST information had it checked elsewhere. And had the event not been in the ABI, it would have eventually done so (checking library ABIs, then scanning the appropriate ASTs for more info). So putting the event in the ABI means we get less info out.

Now, OK, like I said, that's just how Truffle Decoder currently works, not necessarily how it has to work... but still, you see the trouble, right? It adds a lot more "anything can come from anywhere", if an event given in a particular contract's ABI doesn't have to be defined in that contract or one of its ancestors!

So yeah, maybe effectiveABI -- or maybe better yet, extraABI, which would contain only the extra stuff? And you could combine it with the ordinary ABI to get the full "effective ABI".

I dunno. If you're doing free events, I guess you have to do something like this. But if you don't do free events, then it probably makes more sense to leave things as-is at the moment...

[chriseth]

Thanks for shedding some light on how truffle decoder works with event!

Maybe one thing to consider before we adding something like 'extraABI`: I think it would be far better to find a solution that maybe needs some implementation work in truffle (as well as solidity) but "just works" in a backwards-compatible matter. We could add another field to each ABI entry that is not defined in the contract, for example. I think having to scan two different items in the ABI that might even theoretically conflict with each other is far inferior to scanning a single list and maybe doing some extra work identifying the item in the AST.

[alcuadrado]

Thanks for your answer, @chriseth!

Now I get the difference. This is mostly about libraries and delegate calls, right?

This is tangentially related, but I think it will be useful to understand the whole picture:

A very common problem that people have when testing smart contracts is that lots of times when sending a TX to a smart contract, you also want to decoded another contract's events, because of internal transactions/messages.

Currently, this is not trivial with most javascript libraries, but it could be if their design acknowledges this usecase, and doesn't do events decoding on a per-contract basis. I think this would mean trying to decode all the events in the project though.

[chriseth]

@alcuadrado it's not really about delegatecalls, because with delegatecall, we actually don't really know what code will be executed. Concerning decoding events at other addresses: What should be happening is that the tools should take a look at the metadata hash at the event's address, then download the ABI (including the events) from ipfs and doing the decoding.

What do you mean by "trying to decode all the events in the project" - the event usually has a signature and you can just select the one that matches.

[haltman-at]

Well, you can't just select the one that matches by signature, due to the fact that signatures don't take into account which parameters are indexed, so say you have two libraries that declare events with the same signature but with different parameters indexed, which poses a bit of a problem...

[chriseth]

I agree that this is a problem, but it is unrelated to this matter, isn't it?

[haltman-at]

Hm, yes, I suppose that is mostly unrelated.

[haltman-at]

Here's an idea for how to handle this in a way that's more possible for tools like Truffle Decoder to handle. So, let's say that abi is changed to include any events that can be emitted, as discussed above. (Where here I'm assuming we're ignoring delegate calls, and just listing ones that can be emitted by this particular bytecode without a delegatecall, because that's what's discussed above.)

Maybe this would be fine if additional corresponding information were added to the AST? Like right now the AST for a contract has linearizedBaseContracts; maybe it could also have internalizedLibraries or something, listing all libraries it has inlined code from?

That doesn't cover the case of file-level events, but I think those are maybe less of a problem than the library problem?

Alternatively, maybe one could even put directly in the AST a list of AST IDs of events the contract can emit! (Where again by "can emit" I mean the same thing as above, i.e., these would correspond to the ones listed in the ABI (not necessarily in the same order).) That doesn't really seem like something that should go in the AST, I'll admit, but it would certainly do the job...

And then these same ideas can be applied to the error case as well, obviously.

[chriseth]

Adding the referenced events / errors to the AST sounds like a very good idea!

[github-actions]

Hi everyone! This issue has been closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

[github-actions]

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

[github-actions]

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.