skmgoldin / tcr

A generic token-curated registry
Apache License 2.0
278 stars 131 forks source link

Event emissions #50

Closed kangarang closed 6 years ago

kangarang commented 6 years ago

Description

Closes #46

Adds indexed to certain desirable event arguments. This enables UIs to filter and search for specific values. This PR also adds additional arguments to certain events to trigger UI changes more conveniently.

Up to three parameters can receive the attribute indexed which will cause the respective arguments to be searched for: It is possible to filter for specific values of indexed arguments in the user interface.

solidity events docs

Changes

kangarang commented 6 years ago

@eccheung4

eccheung4 commented 6 years ago

@kangarang looks good! just wondering if there should also be a _ApplicationExpired event in Registry.sol? Are we missing a case in updateStatus in Registry.sol like processProposal in Parameterizer.sol? } else if (now > prop.processBy) { An application always reverted if updateStatus isn't called before the processBy?

eccheung4 commented 6 years ago

Proposing we add back choice to event _VoteRevealed(uint pollID, uint numTokens, uint votesFor, uint votesAgainst); so it becomes event _VoteRevealed(uint pollID, uint numTokens, uint choice, uint votesFor, uint votesAgainst);

Without it, we have to do a bit more digging to figure out user's vote

skmgoldin commented 6 years ago

@eccheung4 the Registry doesn't have any processBy logic at all. Arguably it should, but it does not.

skmgoldin commented 6 years ago

"Loitering" attacks on the Parameterizer could be quite bad, but a loitering attack on the registry just means it contains one bad listing until someone challenges it.

kangarang commented 6 years ago

@eccheung4 for registry applications, it's not so much a big deal if the application doesn't get processed since the only entity that's affected by that scenario is the applicant. but a reparameterization proposal affects the entire network, so the processBy date ensures that reparameterizations are followed up promptly

kangarang commented 6 years ago

adding in choice right now

kangarang commented 6 years ago

let's plan on reviewing/pulling this sometime tomorrow? @skmgoldin

skmgoldin commented 6 years ago

Lets review it in code review tomorrow.