From 9be72d5b183d2113453ce229e9017250e337b68e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 15 Jun 2018 18:10:21 -0300 Subject: [PATCH] RefundableCrowdsale now uses a RefundEscrow, removed RefundVault. --- .../distribution/RefundableCrowdsale.sol | 25 ++--- .../distribution/utils/RefundVault.sol | 66 -------------- contracts/payment/Escrow.sol | 12 +-- contracts/payment/PullPayment.sol | 2 +- contracts/payment/RefundEscrow.sol | 91 +++++++++++++++++++ contracts/payment/RefundableEscrow.sol | 65 ------------- test/crowdsale/RefundVault.test.js | 59 ------------ test/payment/Escrow.behaviour.js | 10 +- ...bleEscrow.test.js => RefundEscrow.test.js} | 39 ++++---- 9 files changed, 138 insertions(+), 231 deletions(-) delete mode 100644 contracts/crowdsale/distribution/utils/RefundVault.sol create mode 100644 contracts/payment/RefundEscrow.sol delete mode 100644 contracts/payment/RefundableEscrow.sol delete mode 100644 test/crowdsale/RefundVault.test.js rename test/payment/{RefundableEscrow.test.js => RefundEscrow.test.js} (65%) diff --git a/contracts/crowdsale/distribution/RefundableCrowdsale.sol b/contracts/crowdsale/distribution/RefundableCrowdsale.sol index 19e5791ca16..3442c1b0291 100644 --- a/contracts/crowdsale/distribution/RefundableCrowdsale.sol +++ b/contracts/crowdsale/distribution/RefundableCrowdsale.sol @@ -3,14 +3,14 @@ pragma solidity ^0.4.24; import "../../math/SafeMath.sol"; import "./FinalizableCrowdsale.sol"; -import "./utils/RefundVault.sol"; +import "../../payment/RefundEscrow.sol"; /** * @title RefundableCrowdsale * @dev Extension of Crowdsale contract that adds a funding goal, and * the possibility of users getting a refund if goal is not met. - * Uses a RefundVault as the crowdsale's vault. + * Uses a RefundEscrow as the crowdsale's escrow. */ contract RefundableCrowdsale is FinalizableCrowdsale { using SafeMath for uint256; @@ -18,16 +18,16 @@ contract RefundableCrowdsale is FinalizableCrowdsale { // minimum amount of funds to be raised in weis uint256 public goal; - // refund vault used to hold funds while crowdsale is running - RefundVault public vault; + // refund escrow used to hold funds while crowdsale is running + RefundEscrow public escrow; /** - * @dev Constructor, creates RefundVault. + * @dev Constructor, creates RefundEscrow. * @param _goal Funding goal */ constructor(uint256 _goal) public { require(_goal > 0); - vault = new RefundVault(wallet); + escrow = new RefundEscrow(wallet); goal = _goal; } @@ -38,7 +38,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale { require(isFinalized); require(!goalReached()); - vault.refund(msg.sender); + escrow.refund(msg.sender); } /** @@ -50,23 +50,24 @@ contract RefundableCrowdsale is FinalizableCrowdsale { } /** - * @dev vault finalization task, called when owner calls finalize() + * @dev escrow finalization task, called when owner calls finalize() */ function finalization() internal { if (goalReached()) { - vault.close(); + escrow.close(); + escrow.withdraw(); } else { - vault.enableRefunds(); + escrow.enableRefunds(); } super.finalization(); } /** - * @dev Overrides Crowdsale fund forwarding, sending funds to vault. + * @dev Overrides Crowdsale fund forwarding, sending funds to escrow. */ function _forwardFunds() internal { - vault.deposit.value(msg.value)(msg.sender); + escrow.invest.value(msg.value)(msg.sender); } } diff --git a/contracts/crowdsale/distribution/utils/RefundVault.sol b/contracts/crowdsale/distribution/utils/RefundVault.sol deleted file mode 100644 index 38a120977ce..00000000000 --- a/contracts/crowdsale/distribution/utils/RefundVault.sol +++ /dev/null @@ -1,66 +0,0 @@ -pragma solidity ^0.4.24; - -import "../../../math/SafeMath.sol"; -import "../../../ownership/Ownable.sol"; - - -/** - * @title RefundVault - * @dev This contract is used for storing funds while a crowdsale - * is in progress. Supports refunding the money if crowdsale fails, - * and forwarding it if crowdsale is successful. - */ -contract RefundVault is Ownable { - using SafeMath for uint256; - - enum State { Active, Refunding, Closed } - - mapping (address => uint256) public deposited; - address public wallet; - State public state; - - event Closed(); - event RefundsEnabled(); - event Refunded(address indexed beneficiary, uint256 weiAmount); - - /** - * @param _wallet Vault address - */ - constructor(address _wallet) public { - require(_wallet != address(0)); - wallet = _wallet; - state = State.Active; - } - - /** - * @param investor Investor address - */ - function deposit(address investor) onlyOwner public payable { - require(state == State.Active); - deposited[investor] = deposited[investor].add(msg.value); - } - - function close() onlyOwner public { - require(state == State.Active); - state = State.Closed; - emit Closed(); - wallet.transfer(address(this).balance); - } - - function enableRefunds() onlyOwner public { - require(state == State.Active); - state = State.Refunding; - emit RefundsEnabled(); - } - - /** - * @param investor Investor address - */ - function refund(address investor) public { - require(state == State.Refunding); - uint256 depositedValue = deposited[investor]; - deposited[investor] = 0; - investor.transfer(depositedValue); - emit Refunded(investor, depositedValue); - } -} diff --git a/contracts/payment/Escrow.sol b/contracts/payment/Escrow.sol index 2b7f180e0ae..0bed368627f 100644 --- a/contracts/payment/Escrow.sol +++ b/contracts/payment/Escrow.sol @@ -11,10 +11,10 @@ import "../math/SafeMath.sol"; contract Escrow { using SafeMath for uint256; - mapping(address => uint256) private _deposits; + mapping(address => uint256) private deposits; - function deposits(address _payee) public view returns (uint256) { - return _deposits[_payee]; + function depositsOf(address _payee) public view returns (uint256) { + return deposits[_payee]; } /** @@ -25,7 +25,7 @@ contract Escrow { uint256 amount = msg.value; require(amount > 0); - _deposits[_payee] = _deposits[_payee].add(amount); + deposits[_payee] = deposits[_payee].add(amount); } /** @@ -34,12 +34,12 @@ contract Escrow { * @param _payee The address whose funds will be withdrawn and transferred to. */ function withdraw(address _payee) public { - uint256 payment = _deposits[_payee]; + uint256 payment = deposits[_payee]; require(payment != 0); require(address(this).balance >= payment); - _deposits[_payee] = 0; + deposits[_payee] = 0; _payee.transfer(payment); } diff --git a/contracts/payment/PullPayment.sol b/contracts/payment/PullPayment.sol index cc173da2521..f8bf3f4501f 100644 --- a/contracts/payment/PullPayment.sol +++ b/contracts/payment/PullPayment.sol @@ -28,7 +28,7 @@ contract PullPayment { & @param _dest The creditor's address. */ function payments(address _dest) public view returns (uint256) { - return escrow.deposits(_dest); + return escrow.depositsOf(_dest); } /** diff --git a/contracts/payment/RefundEscrow.sol b/contracts/payment/RefundEscrow.sol new file mode 100644 index 00000000000..90f8ee8374c --- /dev/null +++ b/contracts/payment/RefundEscrow.sol @@ -0,0 +1,91 @@ +pragma solidity ^0.4.23; + +import "./ConditionalEscrow.sol"; +import "../ownership/Ownable.sol"; + + +/** + * @title RefundEscrow + * @dev Escrow that holds investor funds for a unique benefitiary, and allows for + * either withdrawal by the benefiatiary, or refunds to the investors. + */ +contract RefundEscrow is ConditionalEscrow, Ownable { + enum State { Active, Refunding, Closed } + + event Closed(); + event RefundsEnabled(); + event Refunded(address indexed investor, uint256 weiAmount); + + State public state; + address public beneficiary; + + /** + * @dev Constructor. + * @param _beneficiary The beneficiary of the investments. + */ + constructor(address _beneficiary) public { + require(_beneficiary != address(0)); + beneficiary = _beneficiary; + state = State.Active; + } + + /** + * @dev Stores funds that may later be refunded. + * @param _investor The address funds will be sent to if a refund occurs. + */ + function invest(address _investor) payable public { + require(state == State.Active); + super.deposit(_investor); + } + + /** + * @dev Disable the base deposit function, use invest instead. + */ + function deposit(address _payee) payable public { + revert(); + } + + /** + * @dev Allows for the beneficiary to withdraw their funds, rejecting + * further investments. + */ + function close() onlyOwner public { + require(state == State.Active); + state = State.Closed; + emit Closed(); + } + + /** + * @dev Allows for refunds to take place, rejecting further investments. + */ + function enableRefunds() onlyOwner public { + require(state == State.Active); + state = State.Refunding; + emit RefundsEnabled(); + } + + /** + * @dev Withdraws the beneficiary's funds. + */ + function withdraw() public { + require(state == State.Closed); + beneficiary.transfer(address(this).balance); + } + + /** + * @dev Refunds an investor. + * @param _investor The address to refund. + */ + function refund(address _investor) public { + uint256 amount = depositsOf(_investor); + super.withdraw(_investor); + emit Refunded(_investor, amount); + } + + /** + * @dev Returns whether investors can withdraw their investments (be refunded). + */ + function withdrawalAllowed(address _payee) public view returns (bool) { + return state == State.Refunding; + } +} diff --git a/contracts/payment/RefundableEscrow.sol b/contracts/payment/RefundableEscrow.sol deleted file mode 100644 index 1031c1d4a6d..00000000000 --- a/contracts/payment/RefundableEscrow.sol +++ /dev/null @@ -1,65 +0,0 @@ -pragma solidity ^0.4.23; - -import "./ConditionalEscrow.sol"; -import "../ownership/Ownable.sol"; - - -/** - * @title RefundableEscrow - * @dev Escrow that holds investor funds for a unique benefitiary, and allows for - * either withdrawal by the benefiatiary, or refunds to the investors. - */ -contract RefundableEscrow is ConditionalEscrow, Ownable { - enum State { Active, Refunding, Closed } - - event Closed(); - event RefundsEnabled(); - - State public state; - address public beneficiary; - - constructor(address _beneficiary) public { - require(_beneficiary != address(0)); - beneficiary = _beneficiary; - state = State.Active; - } - - function invest() payable public { - require(state == State.Active); - super.deposit(msg.sender); - } - - // Disable the base deposit function, use invest instead. - function deposit(address _payee) payable public { - revert(); - } - - function close() onlyOwner public { - require(state == State.Active); - state = State.Closed; - emit Closed(); - } - - function enableRefunds() onlyOwner public { - require(state == State.Active); - state = State.Refunding; - emit RefundsEnabled(); - } - - function withdrawalAllowed(address _payee) public view returns (bool) { - return state == State.Refunding; - } - - function withdraw(address _payee) public { - if (_payee == beneficiary) { - beneficiaryWithdrawal(); - } else { - super.withdraw(_payee); - } - } - - function beneficiaryWithdrawal() internal { - require(state == State.Closed); - beneficiary.transfer(address(this).balance); - } -} diff --git a/test/crowdsale/RefundVault.test.js b/test/crowdsale/RefundVault.test.js deleted file mode 100644 index e3b88dfcede..00000000000 --- a/test/crowdsale/RefundVault.test.js +++ /dev/null @@ -1,59 +0,0 @@ -import ether from '../helpers/ether'; -import EVMRevert from '../helpers/EVMRevert'; - -const BigNumber = web3.BigNumber; - -require('chai') - .use(require('chai-as-promised')) - .use(require('chai-bignumber')(BigNumber)) - .should(); - -const RefundVault = artifacts.require('RefundVault'); - -contract('RefundVault', function ([_, owner, wallet, investor]) { - const value = ether(42); - - beforeEach(async function () { - this.vault = await RefundVault.new(wallet, { from: owner }); - }); - - it('should accept contributions', async function () { - await this.vault.deposit(investor, { value, from: owner }).should.be.fulfilled; - }); - - it('should not refund contribution during active state', async function () { - await this.vault.deposit(investor, { value, from: owner }); - await this.vault.refund(investor).should.be.rejectedWith(EVMRevert); - }); - - it('only owner can enter refund mode', async function () { - await this.vault.enableRefunds({ from: _ }).should.be.rejectedWith(EVMRevert); - await this.vault.enableRefunds({ from: owner }).should.be.fulfilled; - }); - - it('should refund contribution after entering refund mode', async function () { - await this.vault.deposit(investor, { value, from: owner }); - await this.vault.enableRefunds({ from: owner }); - - const pre = web3.eth.getBalance(investor); - await this.vault.refund(investor); - const post = web3.eth.getBalance(investor); - - post.minus(pre).should.be.bignumber.equal(value); - }); - - it('only owner can close', async function () { - await this.vault.close({ from: _ }).should.be.rejectedWith(EVMRevert); - await this.vault.close({ from: owner }).should.be.fulfilled; - }); - - it('should forward funds to wallet after closing', async function () { - await this.vault.deposit(investor, { value, from: owner }); - - const pre = web3.eth.getBalance(wallet); - await this.vault.close({ from: owner }); - const post = web3.eth.getBalance(wallet); - - post.minus(pre).should.be.bignumber.equal(value); - }); -}); diff --git a/test/payment/Escrow.behaviour.js b/test/payment/Escrow.behaviour.js index d1540b0d2de..79ccfeecb9d 100644 --- a/test/payment/Escrow.behaviour.js +++ b/test/payment/Escrow.behaviour.js @@ -14,7 +14,7 @@ export default function ([payer1, payer2, payee1, payee2]) { await this.contract.deposit(payee1, { from: payer1, value: amount }); const balance = await web3.eth.getBalance(this.contract.address); - const deposit = await this.contract.deposits(payee1); + const deposit = await this.contract.depositsOf(payee1); balance.should.be.bignumber.equal(amount); deposit.should.be.bignumber.equal(amount); @@ -29,7 +29,7 @@ export default function ([payer1, payer2, payee1, payee2]) { await this.contract.deposit(payee1, { from: payer2, value: amount * 2 }); const balance = await web3.eth.getBalance(this.contract.address); - const deposit = await this.contract.deposits(payee1); + const deposit = await this.contract.depositsOf(payee1); balance.should.be.bignumber.equal(amount * 3); deposit.should.be.bignumber.equal(amount * 3); @@ -40,8 +40,8 @@ export default function ([payer1, payer2, payee1, payee2]) { await this.contract.deposit(payee2, { from: payer1, value: amount * 2 }); const balance = await web3.eth.getBalance(this.contract.address); - const depositPayee1 = await this.contract.deposits(payee1); - const depositPayee2 = await this.contract.deposits(payee2); + const depositPayee1 = await this.contract.depositsOf(payee1); + const depositPayee2 = await this.contract.depositsOf(payee2); balance.should.be.bignumber.equal(amount * 3); depositPayee1.should.be.bignumber.equal(amount); @@ -55,7 +55,7 @@ export default function ([payer1, payer2, payee1, payee2]) { await this.contract.withdraw(payee1, { from: payer2 }); const escrowBalance = await web3.eth.getBalance(this.contract.address); - const finalDeposit = await this.contract.deposits(payee1); + const finalDeposit = await this.contract.depositsOf(payee1); const payeeFinalBalance = await web3.eth.getBalance(payee1); escrowBalance.should.be.bignumber.equal(0); diff --git a/test/payment/RefundableEscrow.test.js b/test/payment/RefundEscrow.test.js similarity index 65% rename from test/payment/RefundableEscrow.test.js rename to test/payment/RefundEscrow.test.js index 6b1bafd57c3..4c0dc4e59c8 100644 --- a/test/payment/RefundableEscrow.test.js +++ b/test/payment/RefundEscrow.test.js @@ -6,14 +6,14 @@ require('chai') .use(require('chai-bignumber')(BigNumber)) .should(); -const RefundableEscrow = artifacts.require('RefundableEscrow'); +const RefundEscrow = artifacts.require('RefundEscrow'); -contract('RefundableEscrow', function ([owner, beneficiary, investor1, investor2]) { +contract('RefundEscrow', function ([owner, beneficiary, investor1, investor2]) { const amount = web3.toWei(54.0, 'ether'); const investors = [investor1, investor2]; beforeEach(async function () { - this.contract = await RefundableEscrow.new(beneficiary); + this.contract = await RefundEscrow.new(beneficiary); }); it('disallows direct deposits', async function () { @@ -22,20 +22,20 @@ contract('RefundableEscrow', function ([owner, beneficiary, investor1, investor2 context('active state', function () { it('accepts investments', async function () { - await this.contract.invest({ from: investor1, value: amount }); + await this.contract.invest(investor1, { from: investor1, value: amount }); - const investment = await this.contract.deposits(investor1); + const investment = await this.contract.depositsOf(investor1); investment.should.be.bignumber.equal(amount); }); it('does not refund investors', async function () { - await this.contract.invest({ from: investor1, value: amount }); - await this.contract.withdraw(investor1).should.be.rejectedWith(EVMRevert); + await this.contract.invest(investor1, { from: investor1, value: amount }); + await this.contract.refund(investor1).should.be.rejectedWith(EVMRevert); }); it('does not allow beneficiary withdrawal', async function () { - await this.contract.invest({ from: investor1, value: amount }); - await this.contract.withdraw(beneficiary).should.be.rejectedWith(EVMRevert); + await this.contract.invest(investor1, { from: investor1, value: amount }); + await this.contract.withdraw().should.be.rejectedWith(EVMRevert); }); }); @@ -50,22 +50,22 @@ contract('RefundableEscrow', function ([owner, beneficiary, investor1, investor2 context('closed state', function () { beforeEach(async function () { - await Promise.all(investors.map(investor => this.contract.invest({ from: investor, value: amount }))); + await Promise.all(investors.map(investor => this.contract.invest(investor, { from: investor, value: amount }))); await this.contract.close({ from: owner }); }); it('rejects investments', async function () { - await this.contract.invest({ from: investor1, value: amount }).should.be.rejectedWith(EVMRevert); + await this.contract.invest(investor1, { from: investor1, value: amount }).should.be.rejectedWith(EVMRevert); }); it('does not refund investors', async function () { - await this.contract.withdraw(investor1).should.be.rejectedWith(EVMRevert); + await this.contract.refund(investor1).should.be.rejectedWith(EVMRevert); }); it('allows beneficiary withdrawal', async function () { const beneficiaryInitialBalance = await web3.eth.getBalance(beneficiary); - await this.contract.withdraw(beneficiary); + await this.contract.withdraw(); const beneficiaryFinalBalance = await web3.eth.getBalance(beneficiary); beneficiaryFinalBalance.sub(beneficiaryInitialBalance).should.be.bignumber.equal(amount * investors.length); @@ -83,27 +83,32 @@ contract('RefundableEscrow', function ([owner, beneficiary, investor1, investor2 context('refund state', function () { beforeEach(async function () { - await Promise.all(investors.map(investor => this.contract.invest({ from: investor, value: amount }))); + await Promise.all(investors.map(investor => this.contract.invest(investor, { from: investor, value: amount }))); await this.contract.enableRefunds({ from: owner }); }); it('rejects investments', async function () { - await this.contract.invest({ from: investor1, value: amount }).should.be.rejectedWith(EVMRevert); + await this.contract.invest(investor1, { from: investor1, value: amount }).should.be.rejectedWith(EVMRevert); }); it('refunds investors', async function () { for (let investor of [investor1, investor2]) { const investorInitialBalance = await web3.eth.getBalance(investor); - await this.contract.withdraw(investor); + const receipt = await this.contract.refund(investor); const investorFinalBalance = await web3.eth.getBalance(investor); investorFinalBalance.sub(investorInitialBalance).should.be.bignumber.equal(amount); + + receipt.logs.length.should.equal(1); + receipt.logs[0].event.should.equal('Refunded'); + receipt.logs[0].args.investor.should.equal(investor); + receipt.logs[0].args.weiAmount.should.be.bignumber.equal(amount); } }); it('does not allow beneficiary withdrawal', async function () { - await this.contract.withdraw(beneficiary).should.be.rejectedWith(EVMRevert); + await this.contract.withdraw().should.be.rejectedWith(EVMRevert); }); }); });