daostack / infra

GNU General Public License v3.0
25 stars 22 forks source link

include the organization (avatar) address in voting machine events instead of hash #17

Closed dkent600 closed 5 years ago

dkent600 commented 5 years ago

When an application wants to filter any voting machine events by avatar, it has to compute the hash of the avatar address plus the address of the contract that generated the events (the latter of which takes some work to discover). Since there may be multiple contracts within a DAO all using the same voting machine, one may need to compute and filter by multiple hash values. I'm not sure that web3 supports filtering contract events by multiple values on the same event argument.

Would it be feasible to include the organization (avatar) address in voting machine events instead of the hash?

orenyodfat commented 5 years ago

It is a good point I think it can be done by keeping an internal mapping of the hash and the organization

On Wed, 3 Oct 2018 at 18:58 Doug Kent notifications@github.com wrote:

When an application wants to filter any voting machine events by avatar, it has to compute the hash of the avatar address plus the address of the contract that generated the events (the latter of which takes some work to discover). Since there may be multiple contracts within a DAO all using the same voting machine, one may need to compute and filter by multiple hash values. I'm not sure that web3 supports filtering contract events by multiple values on the same event argument.

Would it be feasible to include the organization (avatar) address in voting machine events instead of the hash?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/daostack/infra/issues/17, or mute the thread https://github.com/notifications/unsubscribe-auth/AFt3nlRK61Z6D7mMNbhW_IgGHeaouOktks5uhN6egaJpZM4XGWD3 .

orenyodfat commented 5 years ago

Seems like web3 supporting filtering contract events by multiple values on the same event argument. see https://web3js.readthedocs.io/en/1.0/web3-eth-contract.html#events . So for example filter a voting machine events on a specific organisation can be done with something like :

//calculate organisationIds orgSchemesAddressesArray.forEach(function (schemeAddress) { organizationIds[i++] =web3.utils.soliditySha3(item, orgAddress)); }) //filter past proposal events for an organisation votingMachine.getPastEvents('NewProposal', { filter: {_organization: organizationIds}, fromBlock: 0, toBlock: 'latest' }, function(error, events){ console.log(events); }) .then(function(events){ console.log(events) // same results as the optional callback above });

So ,I guess this issue can be solved on the client side (out side of the contract) which is preferable.

dkent600 commented 5 years ago

@orenyodfat

getPastEvents only exists in web3 version 1.0 which for arc.js comes after we will be supporting Infra. web3 1.0 is still in beta and because of that some will not want to use it, or they will, like us, not yet have upgraded to it.

In any case, I don't think Arc should be so hard to use, with or without web3 version 1.0.

Is storing the avatarId in the Proposal struct and using it when emitting events really not preferable? If we did this:

  1. it would be a simpler API that makes a lot more sense to the user -- an avatar address is exactly what we expect, not a hash that needs to be computed
  2. it would maintain the existing API that other Arc users, not just arc.js, are already using.
  3. your example code didn't show the work that is also needed to obtain the list of schemes in a DAO. So the task of fetching the events for an avatar is harder than your example code makes it seem.
  4. your example code appears to supply all of the DAOs scheme addresses, not just the ones that use voting machines. this would not seem to be the most efficient way to filter the events. Discovering the schemes that are using voting machines would be even more complicated.

Remember that not everyone is using arc.js.

Is this really not preferable?

orenyodfat commented 5 years ago

web3 0.2x also support that https://github.com/ethereum/wiki/wiki/JavaScript-API#contract-events

  1. might be simpler to the client and gas consuming for the contract - not sure this is the right way. client functionality is relative simple and free
  2. not sure about what is the api for ARC though yes...there will be a new API
  3. you can get it from RegisterScheme events
  4. do not make much difference
dkent600 commented 5 years ago

1.

web3 0.2x also support that https://github.com/ethereum/wiki/wiki/JavaScript-API#contract-events

I don't see the ability in 0.2.x to supply multiple values when filtering on an event argument.

client functionality is relative simple and free

I absolutely do not agree. BTW, there is another implication of this change: When one receives an emitted event one will not know from which avatar it was emitted without having computed all of the hashes for the given avatar. I think this alone is just not accceptable to expect of the client.

Is this not a simple change in Arc to make the voting machine event API work as always to vastly simplify how clients understand and use Arc (not to mention our own documentation of it)?

not sure about what is the api for ARC though yes...there will be a new API

Yes, a new API, meaning that instead of obtaining an _avatar, one obtains a hash of the avatar+contract.

you can get it from RegisterScheme events

Sure, and then you would want to determine whether each registered scheme is one that uses a voting machine.

4.

do not make much difference

Clients may not be comforted by that statement.