feat: generate contracts for Dojo events #11
Conversation
remybar
force-pushed
the
feat-contract_for_events
branch
2 times, most recently
from
a62baad to
42d3c47
Compare
remybar
force-pushed
the
feat-contract_for_events
branch
from
42d3c47 to
03ff137
Compare
| event_selector: felt252, | ||
| keys: Span<felt252>, | ||
| values: Span<felt252>, | ||
| historical: bool |
There was a problem hiding this comment.
I'm wondering if the historical should come from the calldata or something we defined when registering the event. @Larkooo could it make sense for the user to toggle this?
There was a problem hiding this comment.
At the moment, the historical flag is provided through the dojo::event attribute. That means, it is readable from the event contract (with historical()). An option would be to call the event contract in the emit() function to get the value of the historical flag. So, no need to provide it when registering the event nor when calling the emit() function.
What do you think ?
There was a problem hiding this comment.
Since we will add permissions, passing the boolean is effectively the cheapest way to do it, instead of reading additional data from the storage.
So let's keep it like this for the first iteration I think.
| values: Span<felt252>, | ||
| historical: bool | ||
| ) { | ||
| EventEmitter::emit( |
There was a problem hiding this comment.
Is there a real safety issue to emit an unexpected event from a "malicious" contract ?
I would say that, as the address of the system which emitted the event is stored in the event data, it's easy to filter out malicious events.
On the other hand, it can be heavy for a project to handle permissions for all its events.
There was a problem hiding this comment.
We checked with @Larkooo and it seems we need permissions. The point I feel hard it's the cost.. since permissions checks is expensive. But without permissions, everybody could overwrite some values... which could lead to unexpected client behavior.
remybar
force-pushed
the
feat-contract_for_events
branch
from
4a2df9b to
0ce9233
Compare
|
@remybar just added the Also added the permissions, let's sync on that when you have a chance. Thank you again for the great work on that! |
At the moment, a Dojo event is tagged with the attributes
dojo::eventanddojo::model. To avoid that, this PR implements contract management for Dojo events directly with thedojo::event.dojo::modelis no more useful for Dojo events.