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

feat: events: Lotus API to consume events (smart contract + built-in Actor events) #11540

Closed
wants to merge 76 commits into from

Conversation

aarshkshah1992
Copy link
Contributor

@aarshkshah1992 aarshkshah1992 commented Dec 19, 2023

TODO

  • Rebase on feat/nv22 after the FFI/ref-fvm/actor changes have been bubbled up to the feat/nv22 branch

Related Issues

Proposed Changes

This PR is for #11457. It implements a new API in Lotus for clients to consume both smart contract and the upcoming built-in Actor events in FIP-0083

Note that this is the first draft and it would be great to get community feedback on improvements we can make to the API here.

The existing ETH events API will remain as is and no changes have been made to it.

API Definition

type ActorEventAPI interface {
	GetActorEvents(ctx context.Context, filter *types.ActorEventFilter) ([]*types.ActorEvent, error)
	SubscribeActorEvents(ctx context.Context, filter *types.SubActorEventFilter) (<-chan *types.ActorEvent, error)
}

type ActorEventBlock struct {
	// what value codec does client want to match on ?
	Codec uint64 `json:"codec"`
	// data associated with the "event key"
	Value []byte `json:"value"`
}

type SubActorEventFilter struct {
	Filter  ActorEventFilter `json:"filter"`
	Prefill bool             `json:"prefill"`
}

type ActorEventFilter struct {
	// Matches events from one of these actors, or any actor if empty.
	// TODO: Should we also allow Eth addresses here?
	// For now, this MUST be a Filecoin address.
	Addresses []address.Address `json:"address"`

	// Matches events with the specified key/values, or all events if empty.
	// If the `Blocks` slice is empty, matches on the key only.
	Fields map[string][]ActorEventBlock `json:"fields"`

	// Epoch based filtering ?
	// Start epoch for the filter; -1 means no minimum
	MinEpoch abi.ChainEpoch `json:"minEpoch,omitempty"`

	// End epoch for the filter; -1 means no maximum
	MaxEpoch abi.ChainEpoch `json:"maxEpoch,omitempty"`
}

type ActorEvent struct {
	Entries     []EventEntry
	EmitterAddr address.Address // f4 address of emitter
	Reverted    bool
	Height      abi.ChainEpoch
	TipSetKey   cid.Cid // tipset that contained the message
	MsgCid      cid.Cid // cid of message that produced event
}

There are some changes/details we need to drive consensus on which I've left as review comments on the PR.

Additional Info

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • If the PR affects users (e.g., new feature, bug fix, system requirements change), update the CHANGELOG.md and add details to the UNRELEASED section.
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

rjan90 and others added 30 commits December 5, 2023 08:04
Bump version in master
Make gen
fix: lotus-provider: lotus-provider msg sending
fix: lotus-provider: Fix winning PoSt
fix: sql Scan cannot write to an object
Actually show miner-addrs in lotus-provider info-log
fix: lotus-provider: show addresses in log
fix: lotus-provider: Wait for the correct taskID
Fix log output format in wdPostTaskCmd
Also explicitly limit how many bytes we're willing to read in one go
such that we're capable of reading a worst-case tipset (like, really,
never going to happen worst-case). Previously, this wasn't an issue.
However, we've bumped the max number of messages from 8,192 to 150,000
and need to limit allocations somewhere else.
…sage-length

fix: exchange: allow up to 10k messages per block
It's not a const for the testground build, and needs to be an int 99%
of the time. So we might as well just cast here.
fix: lotus-provider: Fix log output format in wdPostTaskCmd
aarshkshah1992 and others added 2 commits February 7, 2024 15:08
Co-authored-by: Rod Vagg <rod@vagg.org>
@aarshkshah1992
Copy link
Contributor Author

itests will be green after we merge the FFI/ref-fvm deps changes for NV22 to this branch.

@Stebalien
Copy link
Member

itests will be green after we merge the FFI/ref-fvm deps changes for NV22 to this branch.

Merged.

@rvagg
Copy link
Member

rvagg commented Feb 8, 2024

I've merged feat/nv22 back in here, but it also needs a newer builtin-actors. Unfortunately, the v13.0.0-rc.1 already has the optional AggregateProofType change which requires the additional fixes in go-state-types and back here, so it has to be v13.0.0-dev.0; which can be installed with:

(cd build/actors/; ./pack.sh v13 v13.0.0-dev.0)

Then this works as is.

@rvagg rvagg force-pushed the feat/built-in-actor-events-api branch from 9a9c50b to 5f43f67 Compare February 8, 2024 05:01
@rvagg
Copy link
Member

rvagg commented Feb 8, 2024

@aarshkshah1992 latest commit from me is a cleanup and doc of the test. I've tried to structure it a bit better to be more easily understood since there's so many things going on in there.

I got to simplify a bunch of things while in there, including that height problem! Switching to using ChainGetMessagesInTipset and then following that up with a general StateSearchMsg (can't use the actual tipset for that call, has to be types.EmptyTSK, I guess because receipts come after the tipset..?). Then you end up with exactly the same events as the GetActorEvents produces and a simple require.Equal(t, expected, actual) works for comparing.

I think I also made the whole allocation->claim testing thing a bit clearer and actually tested the claim is what we expect. Which then verifies that data collection for the DDO flow is possible with this workflow.

@rvagg rvagg force-pushed the feat/built-in-actor-events-api branch 2 times, most recently from 2fa3ef7 to a450b1f Compare February 8, 2024 05:31
@rvagg rvagg force-pushed the feat/built-in-actor-events-api branch from a450b1f to e500393 Compare February 8, 2024 05:40
@rvagg
Copy link
Member

rvagg commented Feb 8, 2024

Some notes related to the differences to the Eth events API for this one, for anyone reviewing this who may think this is important:

  • FromEpoch instead of FromBlock, ToEpoch instead of ToBlock - we're still using hex to communicate the epoch but we take it as a string so we can also do "earliest" and "latest".
    • The hex is still something we could revisit, is there a good reason to use hex for this when it's just an epoch number? The returned ActorEvent object has a plain epoch integer in it, maybe we should be mirroring that and accepting a plain number (as a string) instead of a hex?
    • It's named height in ActorEvent returned by you use FromEpoch and ToEpoch, should we make the terms consistent?
  • FromEpoch and ToEpoch are string with json:omitempty, in the Eth API they are *string with json:omitempty. Does this matter? Empty string signals default and you don't have to mess with pointers to literals. For RPC it should be essentially the same I believe.
  • There's no direct equivalent to EthLog in here, although ActorEvent as some of the information; you're stuck with a raw EventEntry slice to decode yourself. We could do something like "topics" in EthLog and extract the $type up to the top level. But maybe that's something we can do later.

@rjan90
Copy link
Contributor

rjan90 commented Feb 9, 2024

Given that the TODO of Rebase on feat/nv22 after the FFI/ref-fvm/actor changes have been bubbled up to the feat/nv22 branch now is done, I think this PR now is ready for merge @aarshkshah1992 ?

@rvagg
Copy link
Member

rvagg commented Feb 9, 2024

I'm fine with this getting merged, probably needs to be squashed up because it's a bit of a mess.

I have some additional proposals for the API but they can be done separately.

@rvagg
Copy link
Member

rvagg commented Feb 21, 2024

#11618 has all of this and is condensed and is receiving updates. So moving discussion to there.

@rvagg rvagg closed this Feb 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 P1: Must be resolved
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

9 participants