Ethereum actor events API fixes #9639
Conversation
|
Fixing a bug in tests, don't merge yet |
|
Fixed the test bug, this is ready for review |
chain/events/filter/event.go
Outdated
| idAddr, err := address.NewIDAddress(uint64(ev.Emitter)) | ||
| if err != nil { | ||
| return xerrors.Errorf("convert emitter to id address: %w", err) | ||
| } | ||
| addr, err = ra.LookupRobustAddress(ctx, idAddr, te.rctTs) | ||
| if err != nil { | ||
| return xerrors.Errorf("lookup robust address: %w", err) | ||
| } | ||
|
|
||
| // if robust address is not f4 then we won't match against it so bail early | ||
| if addr.Protocol() != address.Delegated { | ||
| continue | ||
| } | ||
|
|
||
| addressLookups[ev.Emitter] = addr |
There was a problem hiding this comment.
Hm, the logic in this component should stay agnostic of addressing assumptions. After all, this is a general facility to stream matching events. How about we move the matching logic to the filter, making the user supply a func(emitter actor.ID, want address.Address) (bool, error) at origin? I'm afraid that only the upstream component will know how to perform a match that's semantically valid for its use case.
node/modules/actorevent.go
Outdated
| @@ -51,6 +52,7 @@ func EthEvent(cfg config.ActorEventConfig) func(helpers.MetricsCtx, fx.Lifecycle | |||
| if cfg.EnableRealTimeFilterAPI { | |||
| ee.EventFilterManager = &filter.EventFilterManager{ | |||
| ChainStore: cs, | |||
| RobustAddresser: sm, | |||
There was a problem hiding this comment.
I would change this to a predicate as per above; that way we keep the f4 assumption away from the deeper event subscription facilities.
|
@raulk PTAL |
| @@ -50,7 +53,25 @@ func EthEvent(cfg config.ActorEventConfig) func(helpers.MetricsCtx, fx.Lifecycle | |||
|
|
|||
| if cfg.EnableRealTimeFilterAPI { | |||
| ee.EventFilterManager = &filter.EventFilterManager{ | |||
| ChainStore: cs, | |||
| ChainStore: cs, | |||
| AddressResolver: func(ctx context.Context, emitter abi.ActorID, ts *types.TipSet) (address.Address, bool) { | |||
There was a problem hiding this comment.
Let's keep a TODO somewhere that we expect to focus optimization efforts here.
| } | ||
|
|
||
| // if robust address is not f4 then we won't match against it so bail early | ||
| if addr.Protocol() != address.Delegated { |
There was a problem hiding this comment.
Also need to check that the namespace of the delegated address is the EAM.
Some fixes from original event api PR (#9623) that was merged earlier.