From 4eb4d7114dc6e530304682171cfb9518005953b6 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 5 Sep 2018 12:01:27 -0300 Subject: [PATCH] Allow non-beneficiaries to trigger release of their funds (#1275) * add address argument to PullPayments#withdrawPayments * add argument to SplitPayment#claim * add address argument to PostDeliveryCrowdsale#withdrawTokens * add address argument to RefundableCrowdsale#claimRefund * rename SplitPayment#claim to SplitPayment#release --- .../distribution/PostDeliveryCrowdsale.sol | 9 ++++---- .../distribution/RefundableCrowdsale.sol | 5 +++-- contracts/payment/PullPayment.sol | 8 +++---- contracts/payment/SplitPayment.sol | 21 +++++++++---------- test/BreakInvariantBounty.test.js | 8 ++----- test/crowdsale/PostDeliveryCrowdsale.test.js | 8 +++---- test/crowdsale/RefundableCrowdsale.test.js | 8 +++---- test/examples/SampleCrowdsale.test.js | 2 +- test/payment/PullPayment.test.js | 2 +- test/payment/SplitPayment.test.js | 10 ++++----- 10 files changed, 39 insertions(+), 42 deletions(-) diff --git a/contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol b/contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol index b09d09709a5..5d3c75e817d 100644 --- a/contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol +++ b/contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol @@ -16,13 +16,14 @@ contract PostDeliveryCrowdsale is TimedCrowdsale { /** * @dev Withdraw tokens only after crowdsale ends. + * @param _beneficiary Whose tokens will be withdrawn. */ - function withdrawTokens() public { + function withdrawTokens(address _beneficiary) public { require(hasClosed()); - uint256 amount = balances[msg.sender]; + uint256 amount = balances[_beneficiary]; require(amount > 0); - balances[msg.sender] = 0; - _deliverTokens(msg.sender, amount); + balances[_beneficiary] = 0; + _deliverTokens(_beneficiary, amount); } /** diff --git a/contracts/crowdsale/distribution/RefundableCrowdsale.sol b/contracts/crowdsale/distribution/RefundableCrowdsale.sol index 05f95ca8584..b27d1e3d33d 100644 --- a/contracts/crowdsale/distribution/RefundableCrowdsale.sol +++ b/contracts/crowdsale/distribution/RefundableCrowdsale.sol @@ -32,12 +32,13 @@ contract RefundableCrowdsale is FinalizableCrowdsale { /** * @dev Investors can claim refunds here if crowdsale is unsuccessful + * @param _beneficiary Whose refund will be claimed. */ - function claimRefund() public { + function claimRefund(address _beneficiary) public { require(isFinalized); require(!goalReached()); - escrow_.withdraw(msg.sender); + escrow_.withdraw(_beneficiary); } /** diff --git a/contracts/payment/PullPayment.sol b/contracts/payment/PullPayment.sol index cd10424f315..4c6df8150e0 100644 --- a/contracts/payment/PullPayment.sol +++ b/contracts/payment/PullPayment.sol @@ -16,11 +16,11 @@ contract PullPayment { } /** - * @dev Withdraw accumulated balance, called by payee. + * @dev Withdraw accumulated balance. + * @param _payee Whose balance will be withdrawn. */ - function withdrawPayments() public { - address payee = msg.sender; - escrow.withdraw(payee); + function withdrawPayments(address _payee) public { + escrow.withdraw(_payee); } /** diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol index 04b01425683..576dba0fe7a 100644 --- a/contracts/payment/SplitPayment.sol +++ b/contracts/payment/SplitPayment.sol @@ -5,8 +5,8 @@ import "../math/SafeMath.sol"; /** * @title SplitPayment - * @dev Base contract that supports multiple payees claiming funds sent to this contract - * according to the proportion they own. + * @dev This contract can be used when payments need to be received by a group + * of people and split proportionately to some number of shares they own. */ contract SplitPayment { using SafeMath for uint256; @@ -36,27 +36,26 @@ contract SplitPayment { function () external payable {} /** - * @dev Claim your share of the balance. + * @dev Release one of the payee's proportional payment. + * @param _payee Whose payments will be released. */ - function claim() public { - address payee = msg.sender; - - require(shares[payee] > 0); + function release(address _payee) public { + require(shares[_payee] > 0); uint256 totalReceived = address(this).balance.add(totalReleased); uint256 payment = totalReceived.mul( - shares[payee]).div( + shares[_payee]).div( totalShares).sub( - released[payee] + released[_payee] ); require(payment != 0); assert(address(this).balance >= payment); - released[payee] = released[payee].add(payment); + released[_payee] = released[_payee].add(payment); totalReleased = totalReleased.add(payment); - payee.transfer(payment); + _payee.transfer(payment); } /** diff --git a/test/BreakInvariantBounty.test.js b/test/BreakInvariantBounty.test.js index 1f0e36aa9e8..94a5fa0f7c7 100644 --- a/test/BreakInvariantBounty.test.js +++ b/test/BreakInvariantBounty.test.js @@ -70,16 +70,12 @@ contract('BreakInvariantBounty', function ([_, owner, researcher, nonTarget]) { const researcherPrevBalance = await ethGetBalance(researcher); - const gas = await this.bounty.withdrawPayments.estimateGas({ from: researcher }); - const gasPrice = web3.toWei(1, 'gwei'); - const gasCost = (new web3.BigNumber(gas)).times(gasPrice); - - await this.bounty.withdrawPayments({ from: researcher, gasPrice: gasPrice }); + await this.bounty.withdrawPayments(researcher, { gasPrice: 0 }); const updatedBalance = await ethGetBalance(this.bounty.address); updatedBalance.should.be.bignumber.equal(0); const researcherCurrBalance = await ethGetBalance(researcher); - researcherCurrBalance.sub(researcherPrevBalance).should.be.bignumber.equal(reward.sub(gasCost)); + researcherCurrBalance.sub(researcherPrevBalance).should.be.bignumber.equal(reward); }); it('cannot claim reward from non-target', async function () { diff --git a/test/crowdsale/PostDeliveryCrowdsale.test.js b/test/crowdsale/PostDeliveryCrowdsale.test.js index ebc41111c3d..918c916c39f 100644 --- a/test/crowdsale/PostDeliveryCrowdsale.test.js +++ b/test/crowdsale/PostDeliveryCrowdsale.test.js @@ -51,7 +51,7 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { }); it('does not allow beneficiaries to withdraw tokens before crowdsale ends', async function () { - await expectThrow(this.crowdsale.withdrawTokens({ from: investor }), EVMRevert); + await expectThrow(this.crowdsale.withdrawTokens(investor), EVMRevert); }); context('after closing time', function () { @@ -60,13 +60,13 @@ contract('PostDeliveryCrowdsale', function ([_, investor, wallet, purchaser]) { }); it('allows beneficiaries to withdraw tokens', async function () { - await this.crowdsale.withdrawTokens({ from: investor }); + await this.crowdsale.withdrawTokens(investor); (await this.token.balanceOf(investor)).should.be.bignumber.equal(value); }); it('rejects multiple withdrawals', async function () { - await this.crowdsale.withdrawTokens({ from: investor }); - await expectThrow(this.crowdsale.withdrawTokens({ from: investor }), EVMRevert); + await this.crowdsale.withdrawTokens(investor); + await expectThrow(this.crowdsale.withdrawTokens(investor), EVMRevert); }); }); }); diff --git a/test/crowdsale/RefundableCrowdsale.test.js b/test/crowdsale/RefundableCrowdsale.test.js index 320697267aa..c031073353f 100644 --- a/test/crowdsale/RefundableCrowdsale.test.js +++ b/test/crowdsale/RefundableCrowdsale.test.js @@ -55,7 +55,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser context('before opening time', function () { it('denies refunds', async function () { - await expectThrow(this.crowdsale.claimRefund({ from: investor }), EVMRevert); + await expectThrow(this.crowdsale.claimRefund(investor), EVMRevert); }); }); @@ -65,7 +65,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser }); it('denies refunds', async function () { - await expectThrow(this.crowdsale.claimRefund({ from: investor }), EVMRevert); + await expectThrow(this.crowdsale.claimRefund(investor), EVMRevert); }); context('with unreached goal', function () { @@ -81,7 +81,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser it('refunds', async function () { const pre = await ethGetBalance(investor); - await this.crowdsale.claimRefund({ from: investor, gasPrice: 0 }); + await this.crowdsale.claimRefund(investor, { gasPrice: 0 }); const post = await ethGetBalance(investor); post.minus(pre).should.be.bignumber.equal(lessThanGoal); }); @@ -100,7 +100,7 @@ contract('RefundableCrowdsale', function ([_, owner, wallet, investor, purchaser }); it('denies refunds', async function () { - await expectThrow(this.crowdsale.claimRefund({ from: investor }), EVMRevert); + await expectThrow(this.crowdsale.claimRefund(investor), EVMRevert); }); it('forwards funds to wallet', async function () { diff --git a/test/examples/SampleCrowdsale.test.js b/test/examples/SampleCrowdsale.test.js index f7b88080fb3..aad435689d9 100644 --- a/test/examples/SampleCrowdsale.test.js +++ b/test/examples/SampleCrowdsale.test.js @@ -105,7 +105,7 @@ contract('SampleCrowdsale', function ([_, owner, wallet, investor]) { await increaseTimeTo(this.afterClosingTime); await this.crowdsale.finalize({ from: owner }); - await this.crowdsale.claimRefund({ from: investor, gasPrice: 0 }); + await this.crowdsale.claimRefund(investor, { gasPrice: 0 }); const balanceAfterRefund = await ethGetBalance(investor); balanceBeforeInvestment.should.be.bignumber.equal(balanceAfterRefund); diff --git a/test/payment/PullPayment.test.js b/test/payment/PullPayment.test.js index 16d2857ccf8..1eab3911f5d 100644 --- a/test/payment/PullPayment.test.js +++ b/test/payment/PullPayment.test.js @@ -42,7 +42,7 @@ contract('PullPayment', function ([_, payer, payee1, payee2]) { (await this.contract.payments(payee1)).should.be.bignumber.equal(amount); - await this.contract.withdrawPayments({ from: payee1 }); + await this.contract.withdrawPayments(payee1); (await this.contract.payments(payee1)).should.be.bignumber.equal(0); const balance = await ethGetBalance(payee1); diff --git a/test/payment/SplitPayment.test.js b/test/payment/SplitPayment.test.js index bb09ea73cac..c5a0bf617fa 100644 --- a/test/payment/SplitPayment.test.js +++ b/test/payment/SplitPayment.test.js @@ -61,12 +61,12 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, }); it('should throw if no funds to claim', async function () { - await expectThrow(this.contract.claim({ from: payee1 }), EVMRevert); + await expectThrow(this.contract.release(payee1), EVMRevert); }); it('should throw if non-payee want to claim', async function () { await ethSendTransaction({ from: payer1, to: this.contract.address, value: amount }); - await expectThrow(this.contract.claim({ from: nonpayee1 }), EVMRevert); + await expectThrow(this.contract.release(nonpayee1), EVMRevert); }); it('should distribute funds to payees', async function () { @@ -78,17 +78,17 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, // distribute to payees const initAmount1 = await ethGetBalance(payee1); - await this.contract.claim({ from: payee1 }); + await this.contract.release(payee1); const profit1 = (await ethGetBalance(payee1)).sub(initAmount1); profit1.sub(web3.toWei(0.20, 'ether')).abs().should.be.bignumber.lt(1e16); const initAmount2 = await ethGetBalance(payee2); - await this.contract.claim({ from: payee2 }); + await this.contract.release(payee2); const profit2 = (await ethGetBalance(payee2)).sub(initAmount2); profit2.sub(web3.toWei(0.10, 'ether')).abs().should.be.bignumber.lt(1e16); const initAmount3 = await ethGetBalance(payee3); - await this.contract.claim({ from: payee3 }); + await this.contract.release(payee3); const profit3 = (await ethGetBalance(payee3)).sub(initAmount3); profit3.sub(web3.toWei(0.70, 'ether')).abs().should.be.bignumber.lt(1e16);