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

FIP-0049 draft: Actor events #483

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Conversation

raulk
Copy link
Member

@raulk raulk commented Oct 13, 2022

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.

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,
Copy link
Contributor

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.

Copy link
Member Author

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

Copy link
Member

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.
Copy link
Member

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

Copy link
Member Author

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:

  1. To model CID links univocally (although having tag 42 officially recognised as CIDs already achieves this without DAG-CBOR constraints)
  2. To exclude other tags since there is no guarantee that future readers of events will understand them.
  3. 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
Copy link
Member

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?

Copy link
Member Author

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.
Copy link
Member

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.

Copy link
Member Author

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.

Copy link
Member Author

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 |
Copy link
Member

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>),
Copy link
Member

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**
Copy link
Member

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>
Copy link
Member

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.

Comment on lines +186 to +204
### 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.
Copy link
Member

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.
Copy link
Member

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.
Copy link
Member

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>
Copy link
Member

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

@kaitlin-beegle
Copy link
Collaborator

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.

@kaitlin-beegle kaitlin-beegle merged commit 30a22b2 into master Dec 5, 2022
@kaitlin-beegle kaitlin-beegle deleted the raulk/fip-nnnn-actor-events branch December 5, 2022 19:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Technical design: Logs and events
6 participants