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

Feature: fetch events by block #298

Open
dkent600 opened this issue Jul 27, 2018 · 21 comments
Open

Feature: fetch events by block #298

dkent600 opened this issue Jul 27, 2018 · 21 comments
Assignees

Comments

@dkent600
Copy link
Contributor

dkent600 commented Jul 27, 2018

There is a request to get/watch for Arc-related events as each block is generated, returning the found set of events in the order in which they were fired in each block.

Should look similar to the current EventFetcher or EntityFetcher interfaces.

@dkent600
Copy link
Contributor Author

dkent600 commented Jul 27, 2018

@tibetsprague Can you please comment on more specific details of this request? For example:

  • ability to specify an event filter
  • would there be a default set of events?
  • exactly which events would be desired? What about events that aren't technically fired by Arc contracts, like token events that come from zeppelin?
  • anything else?

@dkent600
Copy link
Contributor Author

@tibetsprague
Copy link
Contributor

Let’s have a bigger discussion about this next week. Tuesday?

@orenyodfat
Copy link
Contributor

orenyodfat commented Jul 28, 2018 via email

@orenyodfat
Copy link
Contributor

fetch events by block can be done by web3 .
are you referring to a more sophisticate wrapper which could aggregate multiple event sources to one output?

@dkent600
Copy link
Contributor Author

dkent600 commented Aug 9, 2018

@orenyodfat

fetch events by block can be done by web3 .

That functionality does not allow one to go back in time, only to watch going forward

are you referring to a more sophisticate wrapper which could aggregate multiple event sources to one output?

yes

@dkent600
Copy link
Contributor Author

dkent600 commented Aug 9, 2018

@orenyodfat @tibetsprague @dev-matan-tsuberi

I want to put together an API that, to the extent feasible, looks like the existing EventFetcher system in Arc.js, which itself looks a lot like truffle's and web3's event filtering API. All-around consistency lowers the learning curve for both Arc.js and application developers, decreases the likelihood of bugs and increases the chances for code reuse.

Refer here to information about the Arc.js event API: https://daostack.github.io/arc.js/Events/

So the code might look something like as follows.

First get the EventFetcherFactory, specifying the contract events we want to aggregate, and that we want to filter the GenesisProtocol.Redeem event by _proposalId:

const aggregateEventService = new AggregateEventService();

const fetcherFactory = aggregateEventService.createEventAggregatorFetcher(
[
  {
    contract: WrapperService.wrappers.GenesisProtocol,
    eventName: "Redeem",
    eventArgsFilter: { _proposalId: "0x..."  }
  },
  {
    contract: WrapperService.wrappers.ContributionReward,
    eventName: "Redeem"
  }
]);

Now get an EventFetcher, specifying blocks between someBlockNum and latest, filtering all events by _avatar.

const fetcher = fetcherFactory({ _avatar: "0x..."}, { fromBlock: someBlockNum });

Now get the events that fall within the filter we specified, the easy way that doesn't
require that we supply a callback, though we could if we wanted:

const events = await fetcher.get();

/**
  * `aggregatedEvent` contains the transaction receipt and a Map of event log 
  * entries keyed by what you passed to
  * `aggregateEventService.createEventAggregatorFetcher` above
  */
events.forEach((aggregatedEvent) => { 

  // enumerate keys and values together for demo purposes
  aggregatedEvent.events.entries.forEach((keyValuePair) {

    const eventInfo = keyValuePair[0]; 
    /**
      * eventInfo is what you passed into `aggregateEventService.createEventAggregatorFetcher`
      */
    console.log(`Contract: ${eventInfo.contract.name}, event: ${eventInfo.eventName}`); 
    // output: "Contract: GenesisProtocol, event: Redeem"

    /**
      * get the event args for GenesisProtocol.Redeem, just like they would look from web3
      */
    const args = keyValuePair[1]; 
    console.log(`event args: ${args}`); 
  }
});

See this code in Redeemer for why the Map can be useful: https://github.com/daostack/arc.js/blob/b050e67650d53a87d4778e04d7766099446f8d3a/lib/wrappers/redeemer.ts#L104

Events on different contracts may have the same names.

Start up a watch using the same EventFetcher as in the get example above, this time
passing a calback and a value of 8 for the optional block depth:

fetcher.watch((aggregatedEvent, 8) => {
  ... same code as in the loop above
});

and later:

fetcher.stopWatching();

Subscribe to a PubSub event. This will setup a watch, again using the same EventFetcher as above:

const subscription = await fetcher.subscribe(
  "anyEventNameYouWant",
  (topic: string, aggregatedEvent) => {
  ... same code as in the loops above
});

and later:

subscription.unsubscribe();

@orenyodfat
Copy link
Contributor

@dkent600 re "That functionality does not allow one to go back in time, only to watch going forward" you can get past events also using web3

@orenyodfat
Copy link
Contributor

re the api :
How about make it simpler api ?
something like :
arc.js.subscribesToEvents(contractsAddressesList,EventsNameList,fromBlock,toBlock,callback)
The callback will be called back once there is an event in the blocks range return the the event with all its info.
If there are more than one events from any of the contractsAddressesList and EventsNameList in the same block the callback will be called back with the lists of the events.

@dkent600
Copy link
Contributor Author

you can get past events also using web3

Yes, that is what the current implementation is doing

@dkent600
Copy link
Contributor Author

@orenyodfat

How about make it simpler api ?

Because of what I described above:

All-around consistency lowers the learning curve for both Arc.js and application developers, decreases the likelihood of bugs and increases the chances for code reuse.

I'll add a further reason: At any point we may want to add dependency injection to Arc.js. Dependency injection vastly improves the story of module dependency management (which is already presenting challenges in terms of circular dependencies), and with writing clean, well-structured automated test code. Dependency injection works best with code that is well modularized, which includes the use of interfaces, factory classes and services.

In general I have endeavored to structure the code in Arc.js in such as way as to readily lend itself to adopting the dependency injection pattern, including the pattern for handling events that is represented in the proposal given in this ticket.

The proposed API is not so complicated, and is identical to other APIs in Arc.js and so will be familiar to developers, with little learning curve.

@orenyodfat
Copy link
Contributor

orenyodfat commented Aug 13, 2018

Thanks @dkent600 .
which function the client need to call to get these events ? It suppose to be simple

@dkent600
Copy link
Contributor Author

dkent600 commented Aug 13, 2018

which function the client need to call to get these events ?

See the example code given above

It suppose to be simple

It works the same here as with individual contract events. It is possible to be too simple, and you lose some things. Please review the reasoning I gave for this design.

@dkent600 dkent600 self-assigned this Aug 13, 2018
@tibetsprague
Copy link
Contributor

@dkent600 I think we want filtering per event, and dont need a top level meta filter that would be applied to all events.

I dont really understand the need for both watch and subscribe, why would you want to watch for only one event, I guess if you are using the same fetcher in multiple places but only want to watch for one event in some part of the app?

does aggregatedEvent represent a single transaction and all the events in it? then I think it should be called something like transactionEvents

@dkent600
Copy link
Contributor Author

dkent600 commented Aug 21, 2018

@tibetsprague

I think we want filtering per event, and dont need a top level meta filter that would be applied to all events.

I'm guessing you are referring to AggregateEventService.createEventAggregatorFetcher. Note that AggregateEventService.createEventAggregatorFetcher, Web3EventService.createEventFetcherFactory and Web3EventService.createEntityFetcherFactory all differ in the arguments they take. But the API below them, FetcherFactory and EventFetcher are identical.

This design makes sense, is symmetrical and consistent, with all the benefits that flow therefrom. It provides maximum use-case flexibility and coding efficiency to applications and to automated test code, developer familiarity and confidence with a shallow learning curve, and easier code maintenance.

I dont really understand the need for both watch and subscribe, why would you want to watch for only one event

I dont understand the question. Why not watch and subscribe? Not clear what you mean by "only one event"? The use cases are identical to every other event in Arc.js.

I guess if you are using the same fetcher in multiple places but only want to watch for one event in some part of the app?

Again I am not clear what you mean by "only one event". In any case, using a fetcher in multiple places is definitely a use case, exactly like with all of the other events in Arc.js.

does aggregatedEvent represent a single transaction and all the events in it? then I think it should be called something like transactionEvents

aggregatedEvent represents a single transaction and all the requested events in it, aggregated into a single event. It is totally up to the user as to what they want to call that variable, but we could change the name in the docs and the typings.

@tibetsprague
Copy link
Contributor

You know I'm realizing that we may need to adjust this API to focus on giving aggregated events per block, not per transaction. Or at least make that an option. The thing I really need is to know that I have received all the events in a block so I can track what blocks have so far been read and fully "processed"

@tibetsprague
Copy link
Contributor

I do think changing the name of the aggregatedEvent in the docs would help explain what it is better

@dkent600
Copy link
Contributor Author

@tibetsprague Per our phone call, I will go ahead with:

  1. filtering on event arg values on an individual contract event basis (and perhaps also on event values-in-common)
  2. support for requiredDepth

@tibetsprague
Copy link
Contributor

Sounds good. AND the rest I’m on board with

@orenyodfat
Copy link
Contributor

@dkent600 could you please share how the api will looks like (its name and signature) and how it could be used ?

@dkent600
Copy link
Contributor Author

@orenyodfat

could you please share how the api will looks like (its name and signature) and how it could be used ?

I updated this comment to give examples of filtering and specifying a required depth.

Filtering is specified on individual events via aggregateEventService.createEventAggregatorFetcher and globally via the FetcherFactory function (fetcherFactory({ _avatar: "0x..."}, { fromBlock: someBlockNum }))

Required depth is an optional last argument to the Fetcher get, watch and subscribe functions (fetcher.watch((aggregatedEvent, 8)).

This all continues to maintain consistency with the event fetching API elsewhere in Arc.js.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants