SplitPayment improvements (#1417)

* Renamed file, added events and set _addPayee to private

* unwanted file

* adjusted test

* adjusted test

* PaymentReceived event added

* Added event testing.

* Fix static tests

contracts/payment/SplitPayment.sol → contracts/payment/PaymentSplitter.sol

@@ -3,13 +3,17 @@ pragma solidity ^0.4.24;
 import "../math/SafeMath.sol";
 
 /**
- * @title SplitPayment
+ * @title PaymentSplitter
  * @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 {
+contract PaymentSplitter {
   using SafeMath for uint256;
 
+  event PayeeAdded(address account, uint256 shares);
+  event PaymentReleased(address to, uint256 amount);
+  event PaymentReceived(address from, uint256 amount);
+
   uint256 private _totalShares;
   uint256 private _totalReleased;
 
@@ -32,7 +36,9 @@ contract SplitPayment {
   /**
    * @dev payable fallback
    */
-  function () external payable {}
+  function () external payable {
+    emit PaymentReceived(msg.sender, msg.value);
+  }
 
   /**
    * @return the total shares of the contract.
@@ -89,6 +95,7 @@ contract SplitPayment {
     _totalReleased = _totalReleased.add(payment);
 
     account.transfer(payment);
+    emit PaymentReleased(account, payment);
   }
 
   /**
@@ -96,7 +103,7 @@ contract SplitPayment {
    * @param account The address of the payee to add.
    * @param shares_ The number of shares owned by the payee.
    */
-  function _addPayee(address account, uint256 shares_) internal {
+  function _addPayee(address account, uint256 shares_) private {
     require(account != address(0));
     require(shares_ > 0);
     require(_shares[account] == 0);
@@ -104,5 +111,6 @@ contract SplitPayment {
     _payees.push(account);
     _shares[account] = shares_;
     _totalShares = _totalShares.add(shares_);
+    emit PayeeAdded(account, shares_);
   }
 }

test/payment/SplitPayment.test.js → test/payment/PaymentSplitter.test.js

@@ -1,4 +1,5 @@
 const { ethGetBalance } = require('../helpers/web3');
+const expectEvent = require('../helpers/expectEvent');
 const { sendEther } = require('./../helpers/sendTransaction');
 const { ether } = require('../helpers/ether');
 const { ZERO_ADDRESS } = require('./../helpers/constants');
@@ -10,33 +11,33 @@ require('chai')
   .should();
 
 const shouldFail = require('../helpers/shouldFail');
-const SplitPayment = artifacts.require('SplitPayment');
+const PaymentSplitter = artifacts.require('PaymentSplitter');
 
-contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1, payer1]) {
+contract('PaymentSplitter', function ([_, owner, payee1, payee2, payee3, nonpayee1, payer1]) {
   const amount = ether(1.0);
 
   it('rejects an empty set of payees', async function () {
-    await shouldFail.reverting(SplitPayment.new([], []));
+    await shouldFail.reverting(PaymentSplitter.new([], []));
   });
 
   it('rejects more payees than shares', async function () {
-    await shouldFail.reverting(SplitPayment.new([payee1, payee2, payee3], [20, 30]));
+    await shouldFail.reverting(PaymentSplitter.new([payee1, payee2, payee3], [20, 30]));
   });
 
   it('rejects more shares than payees', async function () {
-    await shouldFail.reverting(SplitPayment.new([payee1, payee2], [20, 30, 40]));
+    await shouldFail.reverting(PaymentSplitter.new([payee1, payee2], [20, 30, 40]));
   });
 
   it('rejects null payees', async function () {
-    await shouldFail.reverting(SplitPayment.new([payee1, ZERO_ADDRESS], [20, 30]));
+    await shouldFail.reverting(PaymentSplitter.new([payee1, ZERO_ADDRESS], [20, 30]));
   });
 
   it('rejects zero-valued shares', async function () {
-    await shouldFail.reverting(SplitPayment.new([payee1, payee2], [20, 0]));
+    await shouldFail.reverting(PaymentSplitter.new([payee1, payee2], [20, 0]));
   });
 
   it('rejects repeated payees', async function () {
-    await shouldFail.reverting(SplitPayment.new([payee1, payee1], [20, 30]));
+    await shouldFail.reverting(PaymentSplitter.new([payee1, payee1], [20, 30]));
   });
 
   context('once deployed', function () {
@@ -44,7 +45,7 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1,
       this.payees = [payee1, payee2, payee3];
       this.shares = [20, 10, 70];
 
-      this.contract = await SplitPayment.new(this.payees, this.shares);
+      this.contract = await PaymentSplitter.new(this.payees, this.shares);
     });
 
     it('should have total shares', async function () {
@@ -90,19 +91,22 @@ contract('SplitPayment', function ([_, owner, payee1, payee2, payee3, nonpayee1,
 
       // distribute to payees
       const initAmount1 = await ethGetBalance(payee1);
-      await this.contract.release(payee1);
+      const { logs: logs1 } = 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);
+      expectEvent.inLogs(logs1, 'PaymentReleased', { to: payee1, amount: profit1 });
 
       const initAmount2 = await ethGetBalance(payee2);
-      await this.contract.release(payee2);
+      const { logs: logs2 } = 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);
+      expectEvent.inLogs(logs2, 'PaymentReleased', { to: payee2, amount: profit2 });
 
       const initAmount3 = await ethGetBalance(payee3);
-      await this.contract.release(payee3);
+      const { logs: logs3 } = 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);
+      expectEvent.inLogs(logs3, 'PaymentReleased', { to: payee3, amount: profit3 });
 
       // end balance should be zero
       (await ethGetBalance(this.contract.address)).should.be.bignumber.equal(0);