Skip to content

Commit

Permalink
Allow non-beneficiaries to trigger release of their funds (#1275)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
frangio committed Sep 5, 2018
1 parent 2aa5dd2 commit 4eb4d71
Show file tree
Hide file tree
Showing 10 changed files with 39 additions and 42 deletions.
9 changes: 5 additions & 4 deletions contracts/crowdsale/distribution/PostDeliveryCrowdsale.sol
Expand Up @@ -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);
}

/**
Expand Down
5 changes: 3 additions & 2 deletions contracts/crowdsale/distribution/RefundableCrowdsale.sol
Expand Up @@ -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);
}

/**
Expand Down
8 changes: 4 additions & 4 deletions contracts/payment/PullPayment.sol
Expand Up @@ -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);
}

/**
Expand Down
21 changes: 10 additions & 11 deletions contracts/payment/SplitPayment.sol
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
8 changes: 2 additions & 6 deletions test/BreakInvariantBounty.test.js
Expand Up @@ -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 () {
Expand Down
8 changes: 4 additions & 4 deletions test/crowdsale/PostDeliveryCrowdsale.test.js
Expand Up @@ -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 () {
Expand All @@ -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);
});
});
});
Expand Down
8 changes: 4 additions & 4 deletions test/crowdsale/RefundableCrowdsale.test.js
Expand Up @@ -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);
});
});

Expand All @@ -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 () {
Expand All @@ -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);
});
Expand All @@ -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 () {
Expand Down
2 changes: 1 addition & 1 deletion test/examples/SampleCrowdsale.test.js
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/payment/PullPayment.test.js
Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions test/payment/SplitPayment.test.js
Expand Up @@ -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 () {
Expand All @@ -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);

Expand Down

0 comments on commit 4eb4d71

Please sign in to comment.