Skip to content

Commit

Permalink
ref: claimReward
Browse files Browse the repository at this point in the history 
[dependency](Consensys/PLCRVoting#51)

the salt bae ux working group has found that adding some state to polls allows users to claimReward w/o
having to provide salt

* refactor tests
* rm incorrect salt test
  • Loading branch information
@kangarang @skmgoldin
kangarang authored and skmgoldin committed Aug 30, 2018
1 parent c1e8ea2 commit 915d884
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 340 deletions.
31 changes: 13 additions & 18 deletions contracts/Parameterizer.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -246,23 +246,23 @@ contract Parameterizer {
/**
@notice Claim the tokens owed for the msg.sender in the provided challenge
@param _challengeID the challenge ID to claim tokens for
@param _salt the salt used to vote in the challenge being withdrawn for
*/
function claimReward(uint _challengeID, uint _salt) public {
function claimReward(uint _challengeID) public {
Challenge storage challenge = challenges[_challengeID];
// ensure voter has not already claimed tokens and challenge results have been processed
require(challenges[_challengeID].tokenClaims[msg.sender] == false);
require(challenges[_challengeID].resolved == true);
require(challenge.tokenClaims[msg.sender] == false);
require(challenge.resolved == true);

uint voterTokens = voting.getNumPassingTokens(msg.sender, _challengeID, _salt);
uint reward = voterReward(msg.sender, _challengeID, _salt);
uint voterTokens = voting.getNumPassingTokens(msg.sender, _challengeID);
uint reward = voterReward(msg.sender, _challengeID);

// subtract voter's information to preserve the participation ratios of other voters
// compared to the remaining pool of rewards
challenges[_challengeID].winningTokens -= voterTokens;
challenges[_challengeID].rewardPool -= reward;
challenge.winningTokens -= voterTokens;
challenge.rewardPool -= reward;

// ensures a voter cannot claim tokens again
challenges[_challengeID].tokenClaims[msg.sender] = true;
challenge.tokenClaims[msg.sender] = true;

emit _RewardClaimed(_challengeID, reward, msg.sender);
require(token.transfer(msg.sender, reward));
Expand All @@ -272,15 +272,11 @@ contract Parameterizer {
@dev Called by a voter to claim their rewards for each completed vote.
Someone must call updateStatus() before this can be called.
@param _challengeIDs The PLCR pollIDs of the challenges rewards are being claimed for
@param _salts The salts of a voter's commit hashes in the given polls
*/
function claimRewards(uint[] _challengeIDs, uint[] _salts) public {
// make sure the array lengths are the same
require(_challengeIDs.length == _salts.length);

function claimRewards(uint[] _challengeIDs) public {
// loop through arrays, claiming each individual vote reward
for (uint i = 0; i < _challengeIDs.length; i++) {
claimReward(_challengeIDs[i], _salts[i]);
claimReward(_challengeIDs[i]);
}
}

Expand All @@ -292,14 +288,13 @@ contract Parameterizer {
@dev Calculates the provided voter's token reward for the given poll.
@param _voter The address of the voter whose reward balance is to be returned
@param _challengeID The ID of the challenge the voter's reward is being calculated for
@param _salt The salt of the voter's commit hash in the given poll
@return The uint indicating the voter's reward
*/
function voterReward(address _voter, uint _challengeID, uint _salt)
function voterReward(address _voter, uint _challengeID)
public view returns (uint) {
uint winningTokens = challenges[_challengeID].winningTokens;
uint rewardPool = challenges[_challengeID].rewardPool;
uint voterTokens = voting.getNumPassingTokens(_voter, _challengeID, _salt);
uint voterTokens = voting.getNumPassingTokens(_voter, _challengeID);
return (voterTokens * rewardPool) / winningTokens;
}

Expand Down
39 changes: 6 additions & 33 deletions contracts/Registry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,36 +276,14 @@ contract Registry {
@dev Called by a voter to claim their reward for each completed vote. Someone
must call updateStatus() before this can be called.
@param _challengeID The PLCR pollID of the challenge a reward is being claimed for
@param _salt The salt of a voter's commit hash in the given poll
*/
function claimReward(uint _challengeID, uint _salt) public {
// Ensures the voter has not already claimed tokens and challenge results have been processed
require(challenges[_challengeID].tokenClaims[msg.sender] == false);
require(challenges[_challengeID].resolved == true);

uint voterTokens = voting.getNumPassingTokens(msg.sender, _challengeID, _salt);
uint reward = voterReward(msg.sender, _challengeID, _salt);

// Subtracts the voter's information to preserve the participation ratios
// of other voters compared to the remaining pool of rewards
challenges[_challengeID].totalTokens -= voterTokens;
challenges[_challengeID].rewardPool -= reward;

// Ensures a voter cannot claim tokens again
challenges[_challengeID].tokenClaims[msg.sender] = true;

require(token.transfer(msg.sender, reward));

emit _RewardClaimed(_challengeID, reward, msg.sender);
}

function claimRewardSaltBaeStyle(uint _challengeID) public {
function claimReward(uint _challengeID) public {
Challenge storage challenge = challenges[_challengeID];
// Ensures the voter has not already claimed tokens and challenge results have been processed
require(challenge.tokenClaims[msg.sender] == false);
require(challenge.resolved == true);

uint voterTokens = voting.getNumPassingTokensSaltBaeStyle(msg.sender, _challengeID);
uint voterTokens = voting.getNumPassingTokens(msg.sender, _challengeID);
uint reward = (voterTokens * challenge.rewardPool) / challenge.totalTokens;

// Subtracts the voter's information to preserve the participation ratios
Expand All @@ -325,15 +303,11 @@ contract Registry {
@dev Called by a voter to claim their rewards for each completed vote. Someone
must call updateStatus() before this can be called.
@param _challengeIDs The PLCR pollIDs of the challenges rewards are being claimed for
@param _salts The salts of a voter's commit hashes in the given polls
*/
function claimRewards(uint[] _challengeIDs, uint[] _salts) public {
// make sure the array lengths are the same
require(_challengeIDs.length == _salts.length);

function claimRewards(uint[] _challengeIDs) public {
// loop through arrays, claiming each individual vote reward
for (uint i = 0; i < _challengeIDs.length; i++) {
claimReward(_challengeIDs[i], _salts[i]);
claimReward(_challengeIDs[i]);
}
}

Expand All @@ -345,14 +319,13 @@ contract Registry {
@dev Calculates the provided voter's token reward for the given poll.
@param _voter The address of the voter whose reward balance is to be returned
@param _challengeID The pollID of the challenge a reward balance is being queried for
@param _salt The salt of the voter's commit hash in the given poll
@return The uint indicating the voter's reward
*/
function voterReward(address _voter, uint _challengeID, uint _salt)
function voterReward(address _voter, uint _challengeID)
public view returns (uint) {
uint totalTokens = challenges[_challengeID].totalTokens;
uint rewardPool = challenges[_challengeID].rewardPool;
uint voterTokens = voting.getNumPassingTokens(_voter, _challengeID, _salt);
uint voterTokens = voting.getNumPassingTokens(_voter, _challengeID);
return (voterTokens * rewardPool) / totalTokens;
}

Expand Down
18 changes: 9 additions & 9 deletions test/parameterizer/claimReward.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ contract('Parameterizer', (accounts) => {

await parameterizer.processProposal(propID);

await utils.as(voterAlice, parameterizer.claimReward, challengeID, '420');
await utils.as(voterAlice, parameterizer.claimReward, challengeID);
await utils.as(voterAlice, voting.withdrawVotingRights, '10');

const voterAliceFinalBalance = await token.balanceOf.call(voterAlice);
Expand Down Expand Up @@ -90,16 +90,16 @@ contract('Parameterizer', (accounts) => {

const voterAliceReward = await parameterizer.voterReward.call(
voterAlice,
challengeID, '420',
challengeID,
);
await utils.as(voterAlice, parameterizer.claimReward, challengeID, '420');
await utils.as(voterAlice, parameterizer.claimReward, challengeID);
await utils.as(voterAlice, voting.withdrawVotingRights, '10');

const voterBobReward = await parameterizer.voterReward.call(
voterBob,
challengeID, '420',
challengeID,
);
await utils.as(voterBob, parameterizer.claimReward, challengeID, '420');
await utils.as(voterBob, parameterizer.claimReward, challengeID);
await utils.as(voterBob, voting.withdrawVotingRights, '20');

// TODO: do better than approximately.
Expand Down Expand Up @@ -133,7 +133,7 @@ contract('Parameterizer', (accounts) => {
await utils.increaseTime(paramConfig.pRevealStageLength + 1);

try {
await utils.as(voterAlice, parameterizer.claimReward, challengeID, '420');
await utils.as(voterAlice, parameterizer.claimReward, challengeID);
assert(false, 'should not have been able to claimReward for unresolved challenge');
} catch (err) {
assert(utils.isEVMException(err), err.toString());
Expand Down Expand Up @@ -174,10 +174,10 @@ contract('Parameterizer', (accounts) => {

await parameterizer.processProposal(propID);

await utils.as(voterAlice, parameterizer.claimReward, challengeID, '420');
await utils.as(voterAlice, parameterizer.claimReward, challengeID);

try {
await utils.as(voterAlice, parameterizer.claimReward, challengeID, '420');
await utils.as(voterAlice, parameterizer.claimReward, challengeID);
} catch (err) {
assert(utils.isEVMException(err), err.toString());
return;
Expand Down Expand Up @@ -215,7 +215,7 @@ contract('Parameterizer', (accounts) => {
assert.strictEqual(resolved, true, 'Challenge has not been resolved');

try {
await utils.as(voterBob, parameterizer.claimReward, challengeID, '420');
await utils.as(voterBob, parameterizer.claimReward, challengeID);
} catch (err) {
assert(utils.isEVMException(err), err.toString());
return;
Expand Down
14 changes: 6 additions & 8 deletions test/parameterizer/claimRewards.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,11 @@ contract('Parameterizer', (accounts) => {

// array args
const challengeIDs = [challengeID];
const salts = ['420'];

const aliceVoterReward = await parameterizer.voterReward.call(voterAlice, challengeID, '420');
const aliceVoterReward = await parameterizer.voterReward.call(voterAlice, challengeID);

// multi claimRewards, arrays as inputs
await utils.as(voterAlice, parameterizer.claimRewards, challengeIDs, salts);
await utils.as(voterAlice, parameterizer.claimRewards, challengeIDs);
await utils.as(voterAlice, voting.withdrawVotingRights, '10');

// state assertion
Expand Down Expand Up @@ -111,14 +110,13 @@ contract('Parameterizer', (accounts) => {

// array args
const challengeIDs = [challengeID1, challengeID2, challengeID3];
const salts = ['420', '420', '420'];

const aliceVoterReward1 = await parameterizer.voterReward.call(voterAlice, challengeID1, '420');
const aliceVoterReward2 = await parameterizer.voterReward.call(voterAlice, challengeID2, '420');
const aliceVoterReward3 = await parameterizer.voterReward.call(voterAlice, challengeID3, '420');
const aliceVoterReward1 = await parameterizer.voterReward.call(voterAlice, challengeID1);
const aliceVoterReward2 = await parameterizer.voterReward.call(voterAlice, challengeID2);
const aliceVoterReward3 = await parameterizer.voterReward.call(voterAlice, challengeID3);

// multi claimRewards, arrays as inputs
await utils.as(voterAlice, parameterizer.claimRewards, challengeIDs, salts);
await utils.as(voterAlice, parameterizer.claimRewards, challengeIDs);
await utils.as(voterAlice, voting.withdrawVotingRights, '30');

// state assertion
Expand Down
2 changes: 1 addition & 1 deletion test/parameterizer/tokenClaims.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ contract('Parameterizer', (accounts) => {

// Process the proposal and claim a reward
await parameterizer.processProposal(propID);
await utils.as(alice, parameterizer.claimReward, challengeID, '420');
await utils.as(alice, parameterizer.claimReward, challengeID);

const result = await parameterizer.tokenClaims.call(challengeID, alice);
assert.strictEqual(
Expand Down
4 changes: 2 additions & 2 deletions test/parameterizer/voterReward.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ contract('Parameterizer', (accounts) => {

// Grab the challenge struct after the proposal has been processed
const challenge = await parameterizer.challenges.call(challengeID);
const voterTokens = await voting.getNumPassingTokens(voterAlice, challengeID, '420'); // 10
const voterTokens = await voting.getNumPassingTokens(voterAlice, challengeID); // 10
const rewardPool = challenge[0]; // 250,000
const totalTokens = challenge[4]; // 10

const expectedVoterReward = (voterTokens.mul(rewardPool)).div(totalTokens); // 250,000
const voterReward = await parameterizer.voterReward(voterAlice, challengeID, '420');
const voterReward = await parameterizer.voterReward(voterAlice, challengeID);

assert.strictEqual(
expectedVoterReward.toString(10), voterReward.toString(10),
Expand Down
53 changes: 6 additions & 47 deletions test/registry/claimReward.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ contract('Registry', (accounts) => {
await utils.as(applicant, registry.updateStatus, listing);

// Alice claims reward
const aliceVoterReward = await registry.voterReward(voterAlice, pollID, '420');
await utils.as(voterAlice, registry.claimReward, pollID, '420');
const aliceVoterReward = await registry.voterReward(voterAlice, pollID);
await utils.as(voterAlice, registry.claimReward, pollID);

// Alice withdraws her voting rights
await utils.as(voterAlice, voting.withdrawVotingRights, '500');
Expand All @@ -71,54 +71,13 @@ contract('Registry', (accounts) => {

try {
const nonPollID = '666';
await utils.as(voterAlice, registry.claimReward, nonPollID, '420');
await utils.as(voterAlice, registry.claimReward, nonPollID);
assert(false, 'should not have been able to claimReward for non-existant challengeID');
} catch (err) {
assert(utils.isEVMException(err), err.toString());
}
});

it('should revert if provided salt is incorrect', async () => {
const listing = utils.getListingHash('sugar.net');

const applicantStartingBalance = await token.balanceOf.call(applicant);
const aliceStartBal = await token.balanceOf.call(voterAlice);
await utils.addToWhitelist(listing, minDeposit, applicant, registry);

const pollID = await utils.challengeAndGetPollID(listing, challenger, registry);

// Alice is so committed
await utils.commitVote(pollID, '0', 500, '420', voterAlice, voting);
await utils.increaseTime(paramConfig.commitStageLength + 1);

// Alice is so revealing
await utils.as(voterAlice, voting.revealVote, pollID, '0', '420');
await utils.increaseTime(paramConfig.revealStageLength + 1);

const applicantFinalBalance = await token.balanceOf.call(applicant);
const aliceFinalBalance = await token.balanceOf.call(voterAlice);
const expectedBalance = applicantStartingBalance.sub(minDeposit);

assert.strictEqual(
applicantFinalBalance.toString(10), expectedBalance.toString(10),
'applicants final balance should be what they started with minus the minDeposit',
);
assert.strictEqual(
aliceFinalBalance.toString(10), (aliceStartBal.sub(bigTen(500))).toString(10),
'alices final balance should be exactly the same as her starting balance',
);

// Update status
await utils.as(applicant, registry.updateStatus, listing);

try {
await utils.as(voterAlice, registry.claimReward, pollID, '421');
assert(false, 'should not have been able to claimReward with the wrong salt');
} catch (err) {
assert(utils.isEVMException(err), err.toString());
}
});

it('should not transfer tokens if msg.sender has already claimed tokens for a challenge', async () => {
const listing = utils.getListingHash('sugar.net');

Expand All @@ -142,10 +101,10 @@ contract('Registry', (accounts) => {
await utils.as(applicant, registry.updateStatus, listing);

// Claim reward
await utils.as(voterAlice, registry.claimReward, pollID, '420');
await utils.as(voterAlice, registry.claimReward, pollID);

try {
await utils.as(voterAlice, registry.claimReward, pollID, '420');
await utils.as(voterAlice, registry.claimReward, pollID);
assert(false, 'should not have been able to call claimReward twice');
} catch (err) {
assert(utils.isEVMException(err), err.toString());
Expand Down Expand Up @@ -187,7 +146,7 @@ contract('Registry', (accounts) => {
await utils.increaseTime(paramConfig.revealStageLength + 1);

try {
await utils.as(voterAlice, registry.claimReward, pollID, '420');
await utils.as(voterAlice, registry.claimReward, pollID);
assert(false, 'should not have been able to claimReward for unresolved challenge');
} catch (err) {
assert(utils.isEVMException(err), err.toString());
Expand Down
Loading

0 comments on commit 915d884

Please sign in to comment.