Skip to content

Commit 5c8b399

Browse files
orenyodfatorenyodfat
authored
Joinandquit fix2 (#779)
* joinandquit fix * spelling * opt * test coverage
1 parent 4ea1138 commit 5c8b399

File tree

3 files changed

+39
-31
lines changed

3 files changed

+39
-31
lines changed

‎contracts/schemes/JoinAndQuit.sol‎

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ contract JoinAndQuit is
2020
using SafeERC20 for IERC20;
2121
using StringUtil for string;
2222

23+
enum MemeberState { None, Candidate, Accepted, Rejected, ReputationRedeemed }
24+
2325
event JoinInProposal(
2426
address indexed _avatar,
2527
bytes32 indexed _proposalId,
@@ -55,12 +57,10 @@ contract JoinAndQuit is
5557
struct Proposal {
5658
address proposedMember;
5759
uint256 funding;
58-
bool executed;
59-
bool accepted;
6060
}
6161

6262
struct MemberFund {
63-
bool candidate;
63+
MemeberState state;
6464
bool rageQuit;
6565
uint256 funding;
6666
}
@@ -127,13 +127,12 @@ contract JoinAndQuit is
127127
returns(bool) {
128128
Proposal memory proposal = proposals[_proposalId];
129129
require(proposal.proposedMember != address(0), "not a valid proposal");
130-
require(proposal.executed == false, "proposal already been executed");
131-
proposals[_proposalId].executed = true;
130+
require(fundings[proposal.proposedMember].state == MemeberState.Candidate, "proposal already been executed");
132131

133132
bool success;
134133
// Check if vote was successful:
135134
if ((_decision == 1) && (avatar.nativeReputation().balanceOf(proposal.proposedMember) == 0)) {
136-
proposals[_proposalId].accepted = true;
135+
fundings[proposal.proposedMember].state = MemeberState.Accepted;
137136
fundings[proposal.proposedMember].funding = proposal.funding;
138137
totalDonation = totalDonation.add(proposal.funding);
139138
if (fundingToken == IERC20(0)) {
@@ -146,15 +145,16 @@ contract JoinAndQuit is
146145
//this should be called/check after the transfer to the avatar.
147146
setFundingGoalReachedFlag();
148147
} else {
148+
fundings[proposal.proposedMember].state = MemeberState.Rejected;
149149
if (fundingToken == IERC20(0)) {
150150
// solhint-disable-next-line
151151
(success, ) = proposal.proposedMember.call{value:proposal.funding}("");
152-
require(success, "sendEther to avatar failed");
152+
require(success, "sendEther back to candidate failed");
153153
} else {
154154
fundingToken.safeTransfer(proposal.proposedMember, proposal.funding);
155155
}
156156
}
157-
fundings[proposal.proposedMember].candidate = false;
157+
158158
emit ProposalExecuted(address(avatar), _proposalId, _decision);
159159
return true;
160160
}
@@ -174,22 +174,21 @@ contract JoinAndQuit is
174174
returns(bytes32)
175175
{
176176
address proposer = msg.sender;
177-
require(!fundings[proposer].candidate, "already a candidate");
177+
require(fundings[proposer].state != MemeberState.Candidate, "already a candidate");
178+
require(fundings[proposer].state != MemeberState.Accepted, "accepted and not redeemed yet");
178179
require(avatar.nativeReputation().balanceOf(proposer) == 0, "already a member");
179180
require(_feeAmount >= minFeeToJoin, "_feeAmount should be >= then the minFeeToJoin");
180-
fundings[proposer].candidate = true;
181+
fundings[proposer].state = MemeberState.Candidate;
181182
if (fundingToken == IERC20(0)) {
182-
require(_feeAmount == msg.value, "ETH received shoul match the _feeAmount");
183+
require(_feeAmount == msg.value, "ETH received should match the _feeAmount");
183184
} else {
184185
fundingToken.safeTransferFrom(proposer, address(this), _feeAmount);
185186
}
186187
bytes32 proposalId = votingMachine.propose(2, voteParamsHash, proposer, address(avatar));
187188

188189
Proposal memory proposal = Proposal({
189-
executed: false,
190190
proposedMember: proposer,
191-
funding : _feeAmount,
192-
accepted: false
191+
funding : _feeAmount
193192
});
194193
proposals[proposalId] = proposal;
195194

@@ -211,22 +210,22 @@ contract JoinAndQuit is
211210
* @return reputation the redeemed reputation.
212211
*/
213212
function redeemReputation(bytes32 _proposalId) public returns(uint256 reputation) {
214-
Proposal memory _proposal = proposals[_proposalId];
215-
Proposal storage proposal = proposals[_proposalId];
213+
Proposal memory proposal = proposals[_proposalId];
216214
require(proposal.proposedMember != address(0), "no member to redeem");
217215
require(!fundings[proposal.proposedMember].rageQuit, "member already rageQuit");
218-
require(proposal.accepted == true, " proposal not accepted");
216+
require(fundings[proposal.proposedMember].state == MemeberState.Accepted, "member not accepeted");
219217
//set proposal proposedMember to zero to prevent reentrancy attack.
220-
proposal.proposedMember = address(0);
218+
proposals[_proposalId].proposedMember = address(0);
219+
fundings[proposal.proposedMember].state = MemeberState.ReputationRedeemed;
221220
if (memberReputation == 0) {
222-
reputation = _proposal.funding;
221+
reputation = proposal.funding;
223222
} else {
224223
reputation = memberReputation;
225224
}
226225
require(
227226
Controller(
228-
avatar.owner()).mintReputation(reputation, _proposal.proposedMember), "failed to mint reputation");
229-
emit RedeemReputation(address(avatar), _proposalId, _proposal.proposedMember, reputation);
227+
avatar.owner()).mintReputation(reputation, proposal.proposedMember), "failed to mint reputation");
228+
emit RedeemReputation(address(avatar), _proposalId, proposal.proposedMember, reputation);
230229
}
231230

232231
/**
@@ -264,11 +263,11 @@ contract JoinAndQuit is
264263
refundAmount = userDonation.mul(fundingToken.balanceOf(address(avatar))).div(totalDonation);
265264
}
266265
totalDonation = totalDonation.sub(userDonation);
267-
sendToBeneficiary(refundAmount, msg.sender);
268266
uint256 msgSenderReputation = avatar.nativeReputation().balanceOf(msg.sender);
269267
require(
270268
Controller(
271269
avatar.owner()).burnReputation(msgSenderReputation, msg.sender));
270+
sendToBeneficiary(refundAmount, msg.sender);
272271
emit RageQuit(address(avatar), msg.sender, refundAmount);
273272
}
274273

‎contracts/schemes/ReputationTokenTrade.sol‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
pragma solidity ^0.6.10;
1+
pragma solidity ^0.6.12;
22
// SPDX-License-Identifier: GPL-3.0
33

44
import "../votingMachines/VotingMachineCallbacks.sol";

‎test/joinandquit.js‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,8 @@ contract('JoinAndQuit', accounts => {
274274
//Vote with reputation to trigger execution
275275
var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1);
276276
await testSetup.joinAndQuitParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]});
277-
var proposal = await testSetup.joinAndQuit.proposals(proposalId);
278-
assert.equal(proposal.executed,true);
277+
var funding = await testSetup.joinAndQuit.fundings(accounts[3]);
278+
assert.equal(funding.state,2);
279279
assert.equal(await testSetup.standardTokenMock.balanceOf(testSetup.org.avatar.address),testSetup.minFeeToJoin);
280280
assert.equal(await testSetup.standardTokenMock.balanceOf(testSetup.joinAndQuit.address),0);
281281
assert.equal((await testSetup.joinAndQuit.fundings(accounts[3])).funding,testSetup.minFeeToJoin);
@@ -291,8 +291,8 @@ contract('JoinAndQuit', accounts => {
291291
//Vote with reputation to trigger execution
292292
var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1);
293293
await testSetup.joinAndQuitParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]});
294-
var proposal = await testSetup.joinAndQuit.proposals(proposalId);
295-
assert.equal(proposal.executed,true);
294+
var funding = await testSetup.joinAndQuit.fundings(accounts[3]);
295+
assert.equal(funding.state,2);
296296
assert.equal(await avatarBalance(testSetup),testSetup.minFeeToJoin);
297297
assert.equal(await web3.eth.getBalance(testSetup.joinAndQuit.address),0);
298298
assert.equal((await testSetup.joinAndQuit.fundings(accounts[3])).funding,testSetup.minFeeToJoin);
@@ -309,8 +309,8 @@ contract('JoinAndQuit', accounts => {
309309
//Vote with reputation to trigger execution
310310
var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1);
311311
await testSetup.joinAndQuitParams.votingMachine.absoluteVote.vote(proposalId,2,0,helpers.NULL_ADDRESS,{from:accounts[2]});
312-
var proposal = await testSetup.joinAndQuit.proposals(proposalId);
313-
assert.equal(proposal.executed,true);
312+
var funding = await testSetup.joinAndQuit.fundings(accounts[3]);
313+
assert.equal(funding.state,3);
314314
assert.equal(await testSetup.standardTokenMock.balanceOf(testSetup.org.avatar.address),0);
315315
assert.equal(await testSetup.standardTokenMock.balanceOf(testSetup.joinAndQuit.address),0);
316316
assert.equal((await testSetup.joinAndQuit.fundings(accounts[3])).funding,0);
@@ -329,8 +329,8 @@ contract('JoinAndQuit', accounts => {
329329
var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1);
330330
var balanceBefore = await web3.eth.getBalance(accounts[3]);
331331
await testSetup.joinAndQuitParams.votingMachine.absoluteVote.vote(proposalId,2,0,helpers.NULL_ADDRESS,{from:accounts[2]});
332-
var proposal = await testSetup.joinAndQuit.proposals(proposalId);
333-
assert.equal(proposal.executed,true);
332+
var funding = await testSetup.joinAndQuit.fundings(accounts[3]);
333+
assert.equal(funding.state,3);
334334
assert.equal(await avatarBalance(testSetup),0);
335335
assert.equal(await web3.eth.getBalance(testSetup.joinAndQuit.address),0);
336336
assert.equal((await testSetup.joinAndQuit.fundings(accounts[3])).funding,0);
@@ -352,6 +352,15 @@ contract('JoinAndQuit', accounts => {
352352
//Vote with reputation to trigger execution
353353
var proposalId = await helpers.getValueFromLogs(tx, '_proposalId',1);
354354
await testSetup.joinAndQuitParams.votingMachine.absoluteVote.vote(proposalId,1,0,helpers.NULL_ADDRESS,{from:accounts[2]});
355+
try {
356+
await testSetup.joinAndQuit.proposeToJoin(
357+
"description-hash",
358+
testSetup.minFeeToJoin,
359+
{from:accounts[3]});
360+
assert(false, 'accepted candidate which not redeemed yet cannt be proposed again');
361+
} catch (ex) {
362+
helpers.assertVMException(ex);
363+
}
355364
tx = await testSetup.joinAndQuit.redeemReputation(proposalId);
356365
assert.equal(tx.logs[0].event, "RedeemReputation");
357366
assert.equal(tx.logs[0].args._amount, testSetup.memberReputation);

0 commit comments

Comments
 (0)