feat: ethereum compatible actor event API #9623

iand · 2022-11-10T10:24:16Z

Related Issues

Part of #9473

Intended to be merged after #9624 and currently includes those commits - will rebase once #9624 is merged.

Replaces #9499 and #9517 (both are included as commits in this PR) and #9535

Proposed Changes

This PR implements the Ethereum compatible API for retrieving and subscribing to event logs produced by FVM actors. The naming and types are designed to be compatible with the Ethereum API (see for example eth_getfilterchanges). Terminology used in naming fields and types has been kept close to Ethereum but are mapped into Filecoin concepts.

Features:

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
New features have usage guidelines and / or documentation updates in
- Lotus Documentation
- Discussion Tutorials
Tests exist for new functionality or change in behavior
CI is green

vyzo

Can I ask for some better commit hyigene please?

Specifically, can you keep make gen and friends in separate commits?

Same for ffi and go.mod changes.
This make it easy to avoid certain conflicts during rebase by dropping some commits.

iand · 2022-11-10T11:17:26Z

Can I ask for some better commit hyigene please?

Specifically, can you keep make gen and friends in separate commits?

Same for ffi and go.mod changes. This make it easy to avoid certain conflicts during rebase by dropping some commits.

Will split those out once I am satisified that build is clean

raulk

First pass. This is looking pretty promising. Main issues revolve around:

DAG-CBOR encoding for event entry values.
Returning block data from block subscriptions performed via eth_subscribe?
Resolving supplied addresses into actor IDs for event matching.

Once we get the comments here sorted, we should move to implement:

eth_getLogs and the historical store. <= this one is critical as it blocks feature completeness.
An eventstore segregated from the chainstore (to be injected into the fvm).

raulk · 2022-11-11T20:32:03Z

api/api_full.go

@@ -26,7 +26,7 @@ import (
 	abinetwork "github.com/filecoin-project/go-state-types/network"

 	apitypes "github.com/filecoin-project/lotus/api/types"
-	"github.com/filecoin-project/lotus/chain/actors/builtin"
+	builtinactors "github.com/filecoin-project/lotus/chain/actors/builtin"


nit: Let's avoid making these changes unless an import conflict arises. The patch is large, so little thing that we can do to avoid extra noise helps!

This is to resolve a problem with make gen where we have two imports with the same package name

Ah I see. Yes, I've had that problem. I would've imagined it was solved in master already, though?

raulk · 2022-11-11T20:32:23Z

api/api_storage.go

@@ -322,7 +322,7 @@ type StorageMiner interface {

 	CheckProvable(ctx context.Context, pp abi.RegisteredPoStProof, sectors []storiface.SectorRef, expensive bool) (map[abi.SectorNumber]string, error) //perm:admin

-	ComputeProof(ctx context.Context, ssi []builtin.ExtendedSectorInfo, rand abi.PoStRandomness, poStEpoch abi.ChainEpoch, nv abinetwork.Version) ([]builtin.PoStProof, error) //perm:read
+	ComputeProof(ctx context.Context, ssi []builtinactors.ExtendedSectorInfo, rand abi.PoStRandomness, poStEpoch abi.ChainEpoch, nv abinetwork.Version) ([]builtinactors.PoStProof, error) //perm:read


nit: revert this file.

api/eth_types.go

raulk · 2022-11-13T22:52:53Z

node/impl/full/eth.go

+	for _, tsk := range tsks {
+		c, err := tsk.Cid()
+		if err != nil {
+			return nil, err
+		}
+		hash, err := api.EthHashFromCid(c)
+		if err != nil {
+			return nil, err
+		}
+
+		res.NewBlockHashes = append(res.NewBlockHashes, hash)
+	}


Are you sure we don't need to return the full block data here? See https://docs.infura.io/infura/networks/ethereum/json-rpc-methods/subscription-methods/eth_subscribe.

That's just for subscriptions which is handled in the ethSubscription.start method. The lines you are commenting on called by EthGetFilterChanges. See https://docs.infura.io/infura/networks/ethereum/json-rpc-methods/filter-methods/eth_getfilterchanges which says "For filters created with eth_newBlockFilter the return are block hashes (32 Bytes), e.g. ['0x3454645634534...']."

raulk · 2022-11-13T22:53:20Z

node/impl/full/eth.go

@@ -463,7 +496,7 @@ func (a *EthModule) EthProtocolVersion(ctx context.Context) (api.EthUint64, erro
 }

 func (a *EthModule) EthMaxPriorityFeePerGas(ctx context.Context) (api.EthBigInt, error) {
-	gasPremium, err := a.GasAPI.GasEstimateGasPremium(ctx, 0, builtin.SystemActorAddr, 10000, types.EmptyTSK)
+	gasPremium, err := a.GasAPI.GasEstimateGasPremium(ctx, 0, builtinactors.SystemActorAddr, 10000, types.EmptyTSK)


nit: revert

See earlier comment about gen conflict

raulk · 2022-11-13T22:54:39Z

node/config/types.go

@@ -601,3 +601,31 @@ type Wallet struct {
 type FeeConfig struct {
 	DefaultMaxFee types.FIL
 }
+
+type ActorEventConfig struct {


This should be nested inside an Eth API configuration struct.

As mentioned earlier, the filter implementation and associated config isn't necessarily ethereum specific. EnableRealTimeFilterAPI and EnableHistoricFilterAPI would also apply to any native filecoin apis that touch the same filter code

raulk · 2022-11-13T22:55:25Z

node/modules/actorevent.go

+
+var _ events.EventAPI = &EventAPI{}
+
+func EthEvent(cfg config.ActorEventConfig) func(helpers.MetricsCtx, fx.Lifecycle, *store.ChainStore, EventAPI, *messagepool.MessagePool) (*full.EthEvent, error) {


Should probably find another name for this component as it makes it sound like it's an Ethereum event, e.g. EthEventManager?

Renamed to EthEventAPI in follow on branch

raulk · 2022-11-13T22:58:28Z

chain/events/filter/event.go

+		evtArr, err := blockadt.AsArray(st, rct.EventsRoot)
+		if err != nil {
+			return nil, xerrors.Errorf("load events amt: %w", err)
+		}
+
+		ems[i].evs = make([]*types.Event, evtArr.Length())
+		var evt types.Event
+		_ = arr.ForEach(&evt, func(i int64) error {
+			cpy := evt
+			ems[i].evs[int(i)] = &cpy
+			return nil
+		})


So this is going to be the tricky part. The goal is to store events in a separate eventstore, so events won't be be available in the "actor store". That should probably be the thing we implement next.

Is that being done as part of the nv-18events branch?

raulk · 2022-11-13T23:06:48Z

chain/events/filter/event.go

+	}
+	for msgIdx, em := range ems {
+		for evIdx, ev := range em.Events() {
+			if !f.matchAddress(ev.Emitter) {


There was an error in the spec, which Alex pointed out. The Emitter will be an ActorID, not an Address. So we need to resolve the supplied address into an ActorID in the Eth API before we feed it further onwards.

raulk · 2022-11-14T11:48:00Z

As agreed with @iand, I'm merging this into the feat/nv18-events staging branch so I can unblock other refactors there. Follow-ups to this PR will be submitted as separate PRs against feat/nv18-events.

iand mentioned this pull request Nov 10, 2022

feat: ethereum compatible actor event API #9535

Closed

25 tasks

iand force-pushed the feat/nv18-ethevents branch from c2eabbf to c0fd878 Compare November 10, 2022 10:54

vyzo reviewed Nov 10, 2022

View reviewed changes

iand force-pushed the feat/nv18-ethevents branch from c0fd878 to 5621dd6 Compare November 10, 2022 11:14

iand force-pushed the feat/nv18-ethevents branch from 5621dd6 to acd5026 Compare November 10, 2022 11:36

iand added 2 commits November 10, 2022 15:02

Ethereum compatible actor event API

69210d0

make gen

7383ecb

iand force-pushed the feat/nv18-ethevents branch from 751eb2a to 7383ecb Compare November 10, 2022 15:05

raulk reviewed Nov 13, 2022

View reviewed changes

Fix eth types comments

91952f8

raulk changed the base branch from feat/nv18-fevm to feat/nv18-events November 14, 2022 11:10

Merge branch 'feat/nv18-events' into feat/nv18-ethevents

a915c70

fix Receipt#EventsRoot is now *cid.Cid.

efdbea5

raulk marked this pull request as ready for review November 14, 2022 11:55

raulk requested a review from a team as a code owner November 14, 2022 11:55

raulk merged commit 4eccf30 into filecoin-project:feat/nv18-events Nov 14, 2022

iand mentioned this pull request Nov 14, 2022

Ethereum actor events API fixes #9639

Merged

iand mentioned this pull request Dec 8, 2022

API: Ethereum compatible actor event methods backed by FVM #9473

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: ethereum compatible actor event API #9623

feat: ethereum compatible actor event API #9623

iand commented Nov 10, 2022 •

edited

Loading

vyzo left a comment

iand commented Nov 10, 2022

raulk left a comment •

edited

Loading

raulk Nov 11, 2022

iand Nov 14, 2022

raulk Nov 14, 2022

raulk Nov 11, 2022

raulk Nov 13, 2022

iand Nov 14, 2022

raulk Nov 13, 2022

iand Nov 14, 2022

raulk Nov 13, 2022

iand Nov 14, 2022

raulk Nov 13, 2022

iand Nov 14, 2022

raulk Nov 13, 2022

iand Nov 14, 2022

raulk Nov 13, 2022

raulk commented Nov 14, 2022


		var _ events.EventAPI = &EventAPI{}

		func EthEvent(cfg config.ActorEventConfig) func(helpers.MetricsCtx, fx.Lifecycle, store.ChainStore, EventAPI, messagepool.MessagePool) (*full.EthEvent, error) {

feat: ethereum compatible actor event API #9623

feat: ethereum compatible actor event API #9623

Conversation

iand commented Nov 10, 2022 • edited Loading

Related Issues

Proposed Changes

Checklist

vyzo left a comment

Choose a reason for hiding this comment

iand commented Nov 10, 2022

raulk left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raulk commented Nov 14, 2022

iand commented Nov 10, 2022 •

edited

Loading

raulk left a comment •

edited

Loading