-
Notifications
You must be signed in to change notification settings - Fork 164
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
FIP-0049 draft: Actor events #483
Conversation
While this may have been tolerable for a limited number of actors, it will no | ||
longer scale for a world of user-defined actors. | ||
|
||
A future FIP could enhance built-in actors to emit events when power changes, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 This will solve a series of long standing problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree. One of these problems is introspection by tools like Lily, accounting tools, and more. The lack of events forces these tools to rely on statediffing, which isn't a cheap process. There's a lot of overhead we can remove by emitting events from builtin-actors, and migrating observability tools to consume those. cc @frrist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to reducing (and nearly eliminating) the need to perform state-diffing, these events will serve as a definitive source of truth for activities occurring on-chain, and expose information that is currently (near-)impossible to collect, such as sector termination fees, sector expirations, etc.
We'd be happy to provide feedback/participate in writing such a FIP when the times comes.
|
||
### New chain types | ||
|
||
We introduce a DAG-CBOR encoded `Event` type to represent an actor event. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DAG-CBOR or just CBOR? It doesn't have any links. Though I don't think we should prevent actors from emitting CIDs as values - just don't expect them to resolve anywhere.
Do you mean to include other DAG-CBOR restrictions like "maps can only be keyed by strings" and exclusion of all other CBOR tags? Etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention with mandating DAG-CBOR (and having FVM validate the input) is to make events eventually traversable to their constituents parts for the benefit of future light clients and similar scenarios.
I chose to mandate DAG-CBOR because it's worth:
- To model CID links univocally (although having tag 42 officially recognised as CIDs already achieves this without DAG-CBOR constraints)
- To exclude other tags since there is no guarantee that future readers of events will understand them.
- To guarantee minimal encodings to prevent potential attack vectors.
|--------|-------------|---------| | ||
| `0x01` | `b00000001` | indexed | | ||
|
||
Flag `0x80` is currently only used as a client hint. In the future, this flag |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand "currently" here. If you're referring to EVM, can you expand on that here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, currently is the wrong word here. I will clarify.
- special type indicators (e.g. "this is an address") | ||
|
||
> TODO: make flags a varint for extensibility? Can also retype later, if we | ||
> overflow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a 64-bit number in CBOR anyway. You may as well just use u64 now if we'll also constrain which bits can be set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is that? CBOR integer encoding is variable length, and DAG-CBOR requires shortest encoding possible for integers, so we definitely use the variability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coming back to this. CBOR integers are variable length encoded, and DAG-CBOR mandates minimal encoding for integers. However, at the programming level we represent integers as u64 anyway, so we can use u64 here and benefit from minimal varint encoding while we stay below decimal 24, which in bitflags would cover 4 bits (we currently only use 2).
|
||
| hex | binary | meaning | | ||
|--------|-------------|---------| | ||
| `0x01` | `b00000001` | indexed | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarify that the other flags must not be set, i.e. render a block invalid. And that the syscall will fail if any specified.
emitter: 1260, | ||
entries: [ // type, sender, receiver indexed | ||
(0x01, "type", "transfer"), | ||
(0x01, "sender", <id address>), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: use ActorID instead of ID Address.
> - Define maximum lengths for entries, key size, value size? | ||
> - Do we want to introduce standard keys now, e.g. type? | ||
|
||
**Event examples** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: suggest moving long examples down to the design rational section, so the spec is more concise.
exit_code: ExitCode, | ||
return_data: RawBytes, | ||
gas_used: i64, | ||
events: Cid, // new field; root of AMT<Event> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: specify AMT branching factor.
### Client handling | ||
|
||
Honouring indexing hints is optional but highly encouraged at this stage. | ||
Clients may offer new JSON-RPC operations to subscribe to and query events. | ||
|
||
Clients may prefer to store and track event data in a dedicated store, | ||
segregated from the chain store, potentially backed by a different database | ||
engine more suited to the expected write, query, and indexing patterns for this | ||
data. | ||
|
||
Note that there's a timing difference between the moment the client receives the | ||
events AMT root CID (after message execution), and the moment that the event | ||
data (IPLD blocks) is effectively made available in the client's blockstore (at | ||
Machine flush time). Consequently, events cannot be viewed/logged/inspected | ||
until the Machine is flushed. | ||
|
||
FVM implementations may accept a separate events store, and offer an "early | ||
events flush" Machine option to instruct the FVM to flush events on every | ||
message execution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this section is part of the spec, but implementation advice.
We are adding a new field to the `MessageReceipt` type. While this type is not | ||
serialized over the wire nor gossiped, it is returned from JSON-RPC calls. | ||
Hence, users of Filecoin clients may notice a new field, which could in turn | ||
break some of their code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's worth noting that this is a consensus-breaking change, and clients must upgrade to the new behaviour in order to sync the chain.
|
||
FVM implementations may accept a separate events store, and offer an "early | ||
events flush" Machine option to instruct the FVM to flush events on every | ||
message execution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the vein of implementation advice, and perhaps this is what is meant by the "early events flush" operation: Could FVM implementations choose to expose events in their execution response? (related code).
@@ -0,0 +1,331 @@ | |||
--- | |||
fip: <to be assigned> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will be FIP-0049
FYI @raulk - going to merge. Looks like nits and relevant feedback have been incorporated as issues, many of which have been resolved. Want to merge the draft in before additional refinements come in the following days/weeks. |
This FIP adds the ability for actors to emit externally observable events during execution. It is proposed for nv18, and is necessary for the Filecoin EVM runtime. It relates to other FIPs that will be submitted in the next weeks.
Events are fire-and-forget, and can be thought of as a side effect from execution. The payloads of events are self-describing objects signalling that some important circumstance, action or transition has ocurred.
Actor events enable external agents to observe the activity on chain, and may eventually become the source of internal message triggers.
This FIP introduces a minimal design for actor events, including their schema, a new syscall, a new field in the
MessageReceipt
structure, and mechanics to commit these execution side effects on the chain. By minimal we mean that the protocol stays largely unopinionated and unprescriptive around higher level concerns like event indexing, traversal, and proofs (e.g. inclusion proofs for light clients), and other future higher-level features.Closes filecoin-project/ref-fvm#728.