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

Reputation Voting #644

Merged
merged 61 commits into from
Sep 4, 2020
Merged

Reputation Voting #644

merged 61 commits into from
Sep 4, 2020

Conversation

kronosapiens
Copy link
Member

@kronosapiens kronosapiens commented Jun 13, 2019

Closes #80
Closes #81
Closes #82
Closes #83
Closes #84
Closes #85
Closes #86
Closes #87
Closes #88
Closes #89

Depends on #757, #760, #771, #845

Implementation references:

https://blog.colony.io/towards-better-ethereum-voting-protocols-7e54cb5a0119/
https://blog.colony.io/token-weighted-voting-implementation-part-1-72f836b5423b/
https://blog.colony.io/token-weighted-voting-implementation-part-2-13e490fe1b8a/

Initially I started working on both reputation and token weighted voting, but token voting ended up requiring some more problem-solving w.r.t. managing vote secrets and communicating with the token locking contract, so in the spirit of hitting OKRs it's going to wait until QX.

Features

  • Creating new binary polls
  • Reputation voting rate & reveal
  • Defining the actions votes will determine 💪
  • Poll lifecycle management
  • Tests w/ 100% code coverage for new contracts 🎉
  • Vote power thresholds
  • Staking / spam prevention
  • Stake redistribution
  • Events
  • Handle voter compensation (& any unpaid surplus)
  • Escalation flow
  • Arbitrary targets

Design choices:

  • Actions are defined as bytes arrays and invoked using the call opcode pointed at the colony. An optional target allows for transactions to be executed against any contract, not just the colony.

  • Two classes of poll -- rootPoll and domainPoll. Root polls accept arbitrary bytes32 which can be executed by the extensions -- the simpler and more powerful implementation, with the limitation what all polls go to the entirety of the colony. Domain polls, on the other hand, can be held at, you guessed it, any domain in the colony, and can execute authDomain-ed functions only. Domain polls infer the "domain of action" by parsing the bytes32 action directly -- leveraging our convenient convention that the first two arguments of any domain-level permissioned function is (permissionDomainId, childSkillIndex), which means we can uniquely infer the domain from the action. With the domain of action, we can check that the domain of vote is in the inheritance hierarchy.

  • Escalation is implemented very simply as the requirement that new votes over the same variable have more "vote power" (total reputation) voting on it. Since (domain) reputation always aggregates up the tree, this has the nice property of allowing "escalation" to be either a re-vote in the original domain with higher turnout, or a vote in a parent domain, which by definition has more reputation in total. This requirement is restricted to votes on the setExpenditureState function, which is the general-purpose arbitration state change function (see Arbitration State Changes #760).

  • Staking occurs using the new stake-management system (see Add support for stake management #757).

  • One thing I am quite happy about is how the rate/reveal flow eliminates the need for a hasVoted mapping. A user can update their secret as many times as they want during the voting period, but since the secret can only be revealed once, there's no way to double-vote.

  • Stakers can stake on both sides if they choose. It is unlikely they will, of course, but the point is that it is not explicitly restricted.

  • Winning stakers get their entire stake back, plus a proportion of the loser's stake. Losing stakers pay the voter fees, and then get a proportion of their stake back. The proportion is a function of how close the vote was (e.g. winning with 60% of the vote gets you 20% of the losing side's stake, 120 - 100 = 20).

  • Any unspent voter compensation is returned back to stakers proportional to their contribution.

@kronosapiens kronosapiens self-assigned this Jun 13, 2019
@kronosapiens kronosapiens added this to the Sprint 28 milestone Jun 13, 2019
@kronosapiens kronosapiens force-pushed the feat/voting branch 4 times, most recently from 0f57fca to 7871a3a Compare June 13, 2019 12:25
@elenadimitrova elenadimitrova removed this from the Sprint 28 milestone Jun 14, 2019
@kronosapiens kronosapiens force-pushed the feat/voting branch 13 times, most recently from c254d5b to 5dad7af Compare June 21, 2019 08:42
@kronosapiens kronosapiens changed the title Token and Reputation Voting Reputation Voting Jun 21, 2019
@kronosapiens kronosapiens force-pushed the feat/voting branch 3 times, most recently from a223bbf to eedc7ad Compare June 21, 2019 08:56
@area
Copy link
Member

area commented Sep 4, 2020

So I've added a commit to update the expected stateRoots in the smoke test. But why?! This pull request doesn't modify the migrations, and it doesn't modify the storage layout of any contract, let alone all five of the ones tested in the smoke test!

Of course, the above statement, which is where I was yesterday, while well-intentioned is incorrect. This PR does, in fact, modify a migration!

Go ahead dear reader, have another look, I'll wait...

It adds a function to IColony.sol, which has to be wired up - which is automatically handled by the migrations without having to actually alter the files themselves. The migration that's changed is the one setting up the Colony Version Resolver, which is after the one that sets up the colonyNetwork, which is why the colonyNetwork address remains the same, which was throwing me for a loop.

When that migration changes, the deploying address has done an extra transaction, which means that the addresses of both the colony token and the metaColony token are different as they are deployed after that migration, which is why the storage of the colony/metacolony contracts changes. TokenLocking is also deployed afterwards to a different address for the same reason, which is why the colonyNetwork storage changes as it stores that address. The storage of TokenLocking itself is also different, because the TokenLocking resolver is deployed after the Colony Version Resolver and is therefore at a different address. Finally, he same is true for the ReputationMiningCycle resolver, which is stored on the mining cycle contracts, leading to a statehash change there.

Frankly, I'm disappointed this took me that long to figure out, but no use grousing (any more) about it. The last mystery to solve is why it passes locally for Daniel, but did not pass locally for me or on CI. Given that the change here revolves around the ABI, my best guess here is that Daniel has done something like this on a clean checkout:

git checkout develop
yarn run provision:token:contracts
yarn run test:contract:smoke
_<tests pass>_
git checkout feat/voting
yarn run test:contract:smoke
_<tests pass>_

But the contracts have not been rebuilt, and only the old ABI has been wired up. That's fine for the smoke tests, which don't use the new getCapabilityRoles, but means the old values pass. On CI, where the contracts are built every time, and the new ABI will be wired up, the old values are no longer correct for the reasons explained above. A test that did use that function (i.e. a test creating a Domain Motion) would presumably fail in the above state, if this speculation is correct...

docs/_Interface_IColony.md Show resolved Hide resolved
test/extensions/voting-rep.js Outdated Show resolved Hide resolved
contracts/colony/IColony.sol Outdated Show resolved Hide resolved
contracts/extensions/VotingReputation.sol Outdated Show resolved Hide resolved
test/extensions/voting-rep.js Outdated Show resolved Hide resolved
@kronosapiens kronosapiens merged commit cfaf3d9 into develop Sep 4, 2020
@kronosapiens kronosapiens deleted the feat/voting branch September 4, 2020 23:10
area added a commit that referenced this pull request Sep 10, 2020
This branch has added extra functions to IColony, which as discussed
in #644 means these variables need updating.
kronosapiens pushed a commit that referenced this pull request Sep 15, 2020
This branch has added extra functions to IColony, which as discussed
in #644 means these variables need updating.
kronosapiens pushed a commit that referenced this pull request Sep 16, 2020
This branch has added extra functions to IColony, which as discussed
in #644 means these variables need updating.
kronosapiens pushed a commit that referenced this pull request Sep 17, 2020
This branch has added extra functions to IColony, which as discussed
in #644 means these variables need updating.
area added a commit that referenced this pull request Sep 21, 2020
This branch has added extra functions to IColony, which as discussed
in #644 means these variables need updating.
kronosapiens pushed a commit that referenced this pull request Jan 21, 2021
This branch has added extra functions to IColony, which as discussed
in #644 means these variables need updating.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment