Escrows

contracts/Bounty.sol

@@ -35,15 +35,15 @@ contract Bounty is PullPayment, Destructible {
   }
 
   /**
-   * @dev Sends the contract funds to the researcher that proved the contract is broken.
+   * @dev Transfers the contract funds to the researcher that proved the contract is broken.
    * @param target contract
    */
   function claim(Target target) public {
     address researcher = researchers[target];
     require(researcher != address(0));
     // Check Target contract invariants
     require(!target.checkInvariant());
-    asyncSend(researcher, address(this).balance);
+    asyncTransfer(researcher, address(this).balance);
     claimed = true;
   }
 

contracts/crowdsale/distribution/RefundableCrowdsale.sol

@@ -10,7 +10,6 @@ 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 RefundEscrow as the crowdsale's escrow.
  */
 contract RefundableCrowdsale is FinalizableCrowdsale {
   using SafeMath for uint256;
@@ -19,7 +18,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
   uint256 public goal;
 
   // refund escrow used to hold funds while crowdsale is running
-  RefundEscrow public escrow;
+  RefundEscrow private escrow;
 
   /**
    * @dev Constructor, creates RefundEscrow.
@@ -38,7 +37,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
     require(isFinalized);
     require(!goalReached());
 
-    escrow.refund(msg.sender);
+    escrow.withdraw(msg.sender);
   }
 
   /**
@@ -55,7 +54,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
   function finalization() internal {
     if (goalReached()) {
       escrow.close();
-      escrow.withdraw();
+      escrow.beneficiaryWithdraw();
     } else {
       escrow.enableRefunds();
     }
@@ -67,7 +66,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
    * @dev Overrides Crowdsale fund forwarding, sending funds to escrow.
    */
   function _forwardFunds() internal {
-    escrow.invest.value(msg.value)(msg.sender);
+    escrow.deposit.value(msg.value)(msg.sender);
   }
 
 }

contracts/mocks/PullPaymentMock.sol

@@ -9,9 +9,9 @@ contract PullPaymentMock is PullPayment {
 
   constructor() public payable { }
 
-  // test helper function to call asyncSend
-  function callSend(address dest, uint256 amount) public {
-    asyncSend(dest, amount);
+  // test helper function to call asyncTransfer
+  function callTransfer(address dest, uint256 amount) public {
+    asyncTransfer(dest, amount);
   }
 
 }

contracts/payment/Escrow.sol

@@ -23,8 +23,6 @@ contract Escrow {
   */
   function deposit(address _payee) payable public {
     uint256 amount = msg.value;
-    require(amount > 0);
-
     deposits[_payee] = deposits[_payee].add(amount);
   }
 
@@ -35,9 +33,7 @@ contract Escrow {
   */
   function withdraw(address _payee) public {
     uint256 payment = deposits[_payee];
-
-    require(payment != 0);
-    require(address(this).balance >= payment);
+    assert(address(this).balance >= payment);
 
     deposits[_payee] = 0;
 

contracts/payment/PullPayment.sol

@@ -6,10 +6,10 @@ import "./Escrow.sol";
 /**
  * @title PullPayment
  * @dev Base contract supporting async send for pull payments. Inherit from this
- * contract and use asyncSend instead of send or transfer.
+ * contract and use asyncTransfer instead of send or transfer.
  */
 contract PullPayment {
-  Escrow public escrow;
+  Escrow private escrow;
 
   constructor() public {
     escrow = new Escrow();
@@ -25,7 +25,7 @@ contract PullPayment {
 
   /**
   * @dev Returns the credit owed to an address.
-  & @param _dest The creditor's address.
+  * @param _dest The creditor's address.
   */
   function payments(address _dest) public view returns (uint256) {
     return escrow.depositsOf(_dest);
@@ -36,7 +36,7 @@ contract PullPayment {
   * @param _dest The destination address of the funds.
   * @param _amount The amount to transfer.
   */
-  function asyncSend(address _dest, uint256 _amount) internal {
+  function asyncTransfer(address _dest, uint256 _amount) internal {
     escrow.deposit.value(_amount)(_dest);
   }
 }

contracts/payment/RefundEscrow.sol

@@ -6,22 +6,23 @@ 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.
+ * @dev Escrow that holds funds for a beneficiary, deposited from multiple parties.
+ * The contract owner may close the deposit period, and allow for either withdrawal
+ * by the beneficiary, or refunds to the depositors.
  */
 contract RefundEscrow is ConditionalEscrow, Ownable {
   enum State { Active, Refunding, Closed }
 
   event Closed();
   event RefundsEnabled();
-  event Refunded(address indexed investor, uint256 weiAmount);
+  event Refunded(address indexed refundee, uint256 weiAmount);
 
   State public state;
   address public beneficiary;
 
   /**
    * @dev Constructor.
-   * @param _beneficiary The beneficiary of the investments.
+   * @param _beneficiary The beneficiary of the deposits.
    */
   constructor(address _beneficiary) public {
     require(_beneficiary != address(0));
@@ -31,23 +32,16 @@ contract RefundEscrow is ConditionalEscrow, Ownable {
 
   /**
    * @dev Stores funds that may later be refunded.
-   * @param _investor The address funds will be sent to if a refund occurs.
+   * @param _refundee The address funds will be sent to if a refund occurs.
    */
-  function invest(address _investor) payable public {
+  function deposit(address _refundee) 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();
+    super.deposit(_refundee);
   }
 
   /**
    * @dev Allows for the beneficiary to withdraw their funds, rejecting
-   * further investments.
+   * further deposits.
    */
   function close() onlyOwner public {
     require(state == State.Active);
@@ -56,7 +50,7 @@ contract RefundEscrow is ConditionalEscrow, Ownable {
   }
 
   /**
-   * @dev Allows for refunds to take place, rejecting further investments.
+   * @dev Allows for refunds to take place, rejecting further deposits.
    */
   function enableRefunds() onlyOwner public {
     require(state == State.Active);
@@ -67,23 +61,23 @@ contract RefundEscrow is ConditionalEscrow, Ownable {
   /**
    * @dev Withdraws the beneficiary's funds.
    */
-  function withdraw() public {
+  function beneficiaryWithdraw() public {
     require(state == State.Closed);
     beneficiary.transfer(address(this).balance);
   }
 
   /**
-   * @dev Refunds an investor.
-   * @param _investor The address to refund.
+   * @dev Refunds a refundee.
+   * @param _refundee The address to refund.
    */
-  function refund(address _investor) public {
-    uint256 amount = depositsOf(_investor);
-    super.withdraw(_investor);
-    emit Refunded(_investor, amount);
+  function withdraw(address _refundee) public {
+    uint256 amount = depositsOf(_refundee);
+    super.withdraw(_refundee);
+    emit Refunded(_refundee, amount);
   }
 
   /**
-   * @dev Returns whether investors can withdraw their investments (be refunded).
+   * @dev Returns whether refundees can withdraw their deposits (be refunded).
    */
   function withdrawalAllowed(address _payee) public view returns (bool) {
     return state == State.Refunding;

test/payment/ConditionalEscrow.test.js

@@ -1,4 +1,4 @@
-import shouldBehaveLikeEscrow from './Escrow.behaviour';
+import { shouldBehaveLikeEscrow } from './Escrow.behaviour';
 import EVMRevert from '../helpers/EVMRevert';
 
 const BigNumber = web3.BigNumber;
@@ -11,12 +11,12 @@ const ConditionalEscrowMock = artifacts.require('ConditionalEscrowMock');
 
 contract('ConditionalEscrow', function (accounts) {
   beforeEach(async function () {
-    this.contract = await ConditionalEscrowMock.new();
+    this.escrow = await ConditionalEscrowMock.new();
   });
 
   context('when withdrawal is allowed', function () {
     beforeEach(async function () {
-      await Promise.all(accounts.map(payee => this.contract.setAllowed(payee, true)));
+      await Promise.all(accounts.map(payee => this.escrow.setAllowed(payee, true)));
     });
 
     shouldBehaveLikeEscrow(accounts);
@@ -30,13 +30,13 @@ contract('ConditionalEscrow', function (accounts) {
     const withdrawer = accounts[2];
 
     beforeEach(async function () {
-      await this.contract.setAllowed(payee, false);
+      await this.escrow.setAllowed(payee, false);
     });
 
     it('reverts on withdrawals', async function () {
-      await this.contract.deposit(payee, { from: payer, value: amount });
+      await this.escrow.deposit(payee, { from: payer, value: amount });
 
-      await this.contract.withdraw(payee, { from: withdrawer }).should.be.rejectedWith(EVMRevert);
+      await this.escrow.withdraw(payee, { from: withdrawer }).should.be.rejectedWith(EVMRevert);
     });
   });
 });

test/payment/Escrow.behaviour.js

@@ -1,47 +1,45 @@
-import EVMRevert from '../helpers/EVMRevert';
-
 const BigNumber = web3.BigNumber;
 
 require('chai')
   .use(require('chai-bignumber')(BigNumber))
   .should();
 
-export default function ([payer1, payer2, payee1, payee2]) {
+export function shouldBehaveLikeEscrow ([payer1, payer2, payee1, payee2]) {
   const amount = web3.toWei(42.0, 'ether');
 
   describe('as an escrow', function () {
     it('can accept a single deposit', async function () {
-      await this.contract.deposit(payee1, { from: payer1, value: amount });
+      await this.escrow.deposit(payee1, { from: payer1, value: amount });
 
-      const balance = await web3.eth.getBalance(this.contract.address);
-      const deposit = await this.contract.depositsOf(payee1);
+      const balance = await web3.eth.getBalance(this.escrow.address);
+      const deposit = await this.escrow.depositsOf(payee1);
 
       balance.should.be.bignumber.equal(amount);
       deposit.should.be.bignumber.equal(amount);
     });
 
-    it('reverts on empty deposits', async function () {
-      await this.contract.deposit(payee1, { from: payer1, value: 0 }).should.be.rejectedWith(EVMRevert);
+    it('can accept an empty deposit', async function () {
+      await this.escrow.deposit(payee1, { from: payer1, value: 0 });
     });
 
     it('can add multiple deposits on a single account', async function () {
-      await this.contract.deposit(payee1, { from: payer1, value: amount });
-      await this.contract.deposit(payee1, { from: payer2, value: amount * 2 });
+      await this.escrow.deposit(payee1, { from: payer1, value: amount });
+      await this.escrow.deposit(payee1, { from: payer2, value: amount * 2 });
 
-      const balance = await web3.eth.getBalance(this.contract.address);
-      const deposit = await this.contract.depositsOf(payee1);
+      const balance = await web3.eth.getBalance(this.escrow.address);
+      const deposit = await this.escrow.depositsOf(payee1);
 
       balance.should.be.bignumber.equal(amount * 3);
       deposit.should.be.bignumber.equal(amount * 3);
     });
 
     it('can track deposits to multiple accounts', async function () {
-      await this.contract.deposit(payee1, { from: payer1, value: amount });
-      await this.contract.deposit(payee2, { from: payer1, value: amount * 2 });
+      await this.escrow.deposit(payee1, { from: payer1, value: amount });
+      await this.escrow.deposit(payee2, { from: payer1, value: amount * 2 });
 
-      const balance = await web3.eth.getBalance(this.contract.address);
-      const depositPayee1 = await this.contract.depositsOf(payee1);
-      const depositPayee2 = await this.contract.depositsOf(payee2);
+      const balance = await web3.eth.getBalance(this.escrow.address);
+      const depositPayee1 = await this.escrow.depositsOf(payee1);
+      const depositPayee2 = await this.escrow.depositsOf(payee2);
 
       balance.should.be.bignumber.equal(amount * 3);
       depositPayee1.should.be.bignumber.equal(amount);
@@ -51,20 +49,20 @@ export default function ([payer1, payer2, payee1, payee2]) {
     it('can withdraw payments from any account', async function () {
       const payeeInitialBalance = await web3.eth.getBalance(payee1);
 
-      await this.contract.deposit(payee1, { from: payer1, value: amount });
-      await this.contract.withdraw(payee1, { from: payer2 });
+      await this.escrow.deposit(payee1, { from: payer1, value: amount });
+      await this.escrow.withdraw(payee1, { from: payer2 });
 
-      const escrowBalance = await web3.eth.getBalance(this.contract.address);
-      const finalDeposit = await this.contract.depositsOf(payee1);
+      const escrowBalance = await web3.eth.getBalance(this.escrow.address);
+      const finalDeposit = await this.escrow.depositsOf(payee1);
       const payeeFinalBalance = await web3.eth.getBalance(payee1);
 
       escrowBalance.should.be.bignumber.equal(0);
       finalDeposit.should.be.bignumber.equal(0);
       payeeFinalBalance.sub(payeeInitialBalance).should.be.bignumber.equal(amount);
     });
 
-    it('reverts on empty withdrawals', async function () {
-      await this.contract.withdraw(payee1, { from: payer1 }).should.be.rejectedWith(EVMRevert);
+    it('can do an empty withdrawal', async function () {
+      await this.escrow.withdraw(payee1, { from: payer1 });
     });
   });
 };

test/payment/Escrow.test.js

@@ -1,10 +1,10 @@
-import shouldBehaveLikeEscrow from './Escrow.behaviour';
+import { shouldBehaveLikeEscrow } from './Escrow.behaviour';
 
 const Escrow = artifacts.require('Escrow');
 
 contract('Escrow', function (accounts) {
   beforeEach(async function () {
-    this.contract = await Escrow.new();
+    this.escrow = await Escrow.new();
   });
 
   shouldBehaveLikeEscrow(accounts);