AztecProtocol / aztec-packages

Apache License 2.0
199 stars 224 forks source link

Asses whether to allow collisions of event selectors #2707

Open benesjan opened 1 year ago

benesjan commented 1 year ago

Here is a chatgpt summary of a discussion I had with @spalladino on slack:

The discussion revolves around the length of selectors for events and functions in a PR. Currently, both function and event selectors are 4 bytes long. However, in the EVM's case, event selectors are 32 bytes long. This is because log data is cheaper than calldata in the EVM. In this context, event selectors go on L1 as part of the log preimage, making logs more costly. The question raised is whether there would be any issues with using a 4 byte selector.

Palla suggests that the main reason for 32-byte selectors might be to avoid clashes, especially when processing events in transactions. Knowing exactly which event you're dealing with simplifies the process. Allowing conflicts would require checking the emitter address for each log, which could be complicated if you don't have external knowledge of the addresses.

Jan Benes doesn't see the issue of conflicts as a significant problem because all logs contain the contract address in their preimage, and the kernel verifies that the address corresponds to the emitter.

Palla presents a scenario where you're processing a random transaction and encounter an event with a selector that could be A or B. Checking the contract address that emitted it (0x12345) wouldn't be enough to determine whether the event was A or B, unless you have external knowledge of what's in 0x12345.

Palla shares a personal experience where the selector for ERC20 and ERC721 transfers was the same, causing issues when deserializing them. This required manual handling when building something that processes token transfers.

Finally, Palla is unsure whether avoiding these conflicts is worth the extra L1 gas cost.

TODO: Once it's know how much EIP-4844 blob space will cost asses whether not having event topic collisions is worth the L1 calldata cost.

dbanks12 commented 1 year ago

Should this ticket replace this other one? https://github.com/AztecProtocol/aztec-packages/issues/1050

benesjan commented 1 year ago

@dbanks12 I am specifically talking about event selectors so no.

dbanks12 commented 1 year ago

Ah got it, was just combing through old tickets. Makes sense!