Escrows (#1014)

* Added basic Escrow

* PullPayment now uses an Escrow, removing all trust from the contract

* Abstracted the Escrow tests to a behaviour

* Added ConditionalEscrow

* Added RefundableEscrow.

* RefundableCrowdsale now uses a RefundEscrow, removed RefundVault.

* Renaming after code review.

* Added log test helper.

* Now allowing empty deposits and withdrawals.

* Style fixes.

* Minor review comments.

* Add Deposited and Withdrawn events, removed Refunded

* The base Escrow is now Ownable, users of it (owners) must provide methods to access it.

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

@@ -3,31 +3,30 @@ 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.
  */
 contract RefundableCrowdsale is FinalizableCrowdsale {
   using SafeMath for uint256;
 
   // 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 private 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 +37,7 @@ contract RefundableCrowdsale is FinalizableCrowdsale {
     require(isFinalized);
     require(!goalReached());
 
-    vault.refund(msg.sender);
+    escrow.withdraw(msg.sender);
   }
 
   /**
@@ -50,23 +49,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.beneficiaryWithdraw();
     } 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.deposit.value(msg.value)(msg.sender);
   }
 
 }

contracts/mocks/ConditionalEscrowMock.sol

@@ -0,0 +1,18 @@
+pragma solidity ^0.4.24;
+
+
+import "../payment/ConditionalEscrow.sol";
+
+
+// mock class using ConditionalEscrow
+contract ConditionalEscrowMock is ConditionalEscrow {
+  mapping(address => bool) public allowed;
+
+  function setAllowed(address _payee, bool _allowed) public {
+    allowed[_payee] = _allowed;
+  }
+
+  function withdrawalAllowed(address _payee) public view returns (bool) {
+    return allowed[_payee];
+  }
+}

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/ConditionalEscrow.sol

@@ -0,0 +1,22 @@
+pragma solidity ^0.4.23;
+
+import "./Escrow.sol";
+
+
+/**
+ * @title ConditionalEscrow
+ * @dev Base abstract escrow to only allow withdrawal if a condition is met.
+ */
+contract ConditionalEscrow is Escrow {
+  /**
+  * @dev Returns whether an address is allowed to withdraw their funds. To be
+  * implemented by derived contracts.
+  * @param _payee The destination address of the funds.
+  */
+  function withdrawalAllowed(address _payee) public view returns (bool);
+
+  function withdraw(address _payee) public {
+    require(withdrawalAllowed(_payee));
+    super.withdraw(_payee);
+  }
+}

contracts/payment/Escrow.sol

@@ -0,0 +1,51 @@
+pragma solidity ^0.4.23;
+
+import "../math/SafeMath.sol";
+import "../ownership/Ownable.sol";
+
+
+/**
+ * @title Escrow
+ * @dev Base escrow contract, holds funds destinated to a payee until they
+ * withdraw them. The contract that uses the escrow as its payment method
+ * should be its owner, and provide public methods redirecting to the escrow's
+ * deposit and withdraw.
+ */
+contract Escrow is Ownable {
+  using SafeMath for uint256;
+
+  event Deposited(address indexed payee, uint256 weiAmount);
+  event Withdrawn(address indexed payee, uint256 weiAmount);
+
+  mapping(address => uint256) private deposits;
+
+  function depositsOf(address _payee) public view returns (uint256) {
+    return deposits[_payee];
+  }
+
+  /**
+  * @dev Stores the sent amount as credit to be withdrawn.
+  * @param _payee The destination address of the funds.
+  */
+  function deposit(address _payee) public onlyOwner payable {
+    uint256 amount = msg.value;
+    deposits[_payee] = deposits[_payee].add(amount);
+
+    emit Deposited(_payee, amount);
+  }
+
+  /**
+  * @dev Withdraw accumulated balance for a payee.
+  * @param _payee The address whose funds will be withdrawn and transferred to.
+  */
+  function withdraw(address _payee) public onlyOwner {
+    uint256 payment = deposits[_payee];
+    assert(address(this).balance >= payment);
+
+    deposits[_payee] = 0;
+
+    _payee.transfer(payment);
+
+    emit Withdrawn(_payee, payment);
+  }
+}

contracts/payment/PullPayment.sol

@@ -1,43 +1,42 @@
 pragma solidity ^0.4.24;
 
-
-import "../math/SafeMath.sol";
+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 {
-  using SafeMath for uint256;
+  Escrow private escrow;
 
-  mapping(address => uint256) public payments;
-  uint256 public totalPayments;
+  constructor() public {
+    escrow = new Escrow();
+  }
 
   /**
   * @dev Withdraw accumulated balance, called by payee.
   */
   function withdrawPayments() public {
     address payee = msg.sender;
-    uint256 payment = payments[payee];
-
-    require(payment != 0);
-    require(address(this).balance >= payment);
-
-    totalPayments = totalPayments.sub(payment);
-    payments[payee] = 0;
+    escrow.withdraw(payee);
+  }
 
-    payee.transfer(payment);
+  /**
+  * @dev Returns the credit owed to an address.
+  * @param _dest The creditor's address.
+  */
+  function payments(address _dest) public view returns (uint256) {
+    return escrow.depositsOf(_dest);
   }
 
   /**
   * @dev Called by the payer to store the sent amount as credit to be pulled.
-  * @param dest The destination address of the funds.
-  * @param amount The amount to transfer.
+  * @param _dest The destination address of the funds.
+  * @param _amount The amount to transfer.
   */
-  function asyncSend(address dest, uint256 amount) internal {
-    payments[dest] = payments[dest].add(amount);
-    totalPayments = totalPayments.add(amount);
+  function asyncTransfer(address _dest, uint256 _amount) internal {
+    escrow.deposit.value(_amount)(_dest);
   }
 }