Simplify implementation using similar interface as PullPayment contract

contracts/payment/SplitPayment.sol

@@ -1,48 +1,31 @@
-pragma solidity ^0.4.15;
+pragma solidity ^0.4.11;
 
-import '../ReentrancyGuard.sol';
 import '../math/SafeMath.sol';
 
 /**
  * @title SplitPayment
- * @dev Base contract supporting the distribution of funds send to this contract to multiple payees.
+ * @dev Base contract that supports multiple payees claiming funds sent to this contract 
+ * according to the proportion they own.
  */
-contract SplitPayment is ReentrancyGuard {
+contract SplitPayment {
   using SafeMath for uint256;
 
-  struct Payee {
-    address addr;
-    uint256 shares;
-  }
-
   uint256 public totalShares = 0;
-  uint256 public maxPayees = 0;
+  uint256 public totalReleased = 0;
 
-  mapping(address => uint256) payeeIndex;
-  Payee[] payees;
+  mapping(address => uint256) public shares;
+  mapping(address => uint256) public released;
+  address[] public payees;
 
   /**
    * @dev Constructor
-   * @param _maxPayees Total number of payees allowed. Zero for no limit.
-   */
-  function SplitPayment(uint256 _maxPayees) {
-    maxPayees = _maxPayees;
-  }
-
-  /**
-   * @dev Modifier that throws if you want to distribute funds and you are not a payee.
    */
-  modifier canDistribute() {
-    require(isPayee(msg.sender));
-    _;
-  }
+  function SplitPayment(address[] _payees, uint256[] _shares) {
+    require(_payees.length == _shares.length);
 
-  /**
-   * @dev Modifier that throws if not allowed to update payees.
-   * Override from child contract with your own requirements for access control. 
-   */
-  modifier canUpdate() {
-    _;
+    for (uint256 i = 0; i < _payees.length; i++) {
+      addPayee(_payees[i], _shares[i]);
+    }
   }
 
   /**
@@ -50,99 +33,33 @@ contract SplitPayment is ReentrancyGuard {
    * @param _payee The address of the payee to add.
    * @param _shares The number of shares owned by the payee.
    */
-  function addPayee(address _payee, uint256 _shares) public canUpdate {
+  function addPayee(address _payee, uint256 _shares) internal {
     require(_payee != address(0));
     require(_shares > 0);
-    require(!isPayee(_payee));
-    require(maxPayees == 0 || payees.length.add(1) <= maxPayees);
+    require(shares[_payee] == 0);
 
-    payees.push(Payee(_payee, _shares));
-    payeeIndex[_payee] = payees.length;
+    payees.push(_payee);
+    shares[_payee] = _shares;
     totalShares = totalShares.add(_shares);
   }
 
   /**
-   * @dev Add multiple payees to the contract.
-   * @param _payees An array of addresses of payees to add.
-   * @param _shares An array of the shares corresponding to each payee in the _payees array.
-   */
-  function addPayeeMany(address[] _payees, uint256[] _shares) public canUpdate {
-    require(_payees.length == _shares.length);
-    require(maxPayees == 0 || payees.length.add(_payees.length) <= maxPayees);
-
-    for (uint256 i = 0; i < _payees.length; i++) {
-      addPayee(_payees[i], _shares[i]);
-    }
-  }
-
-  /**
-   * @dev Return true if the payee is in the contract.
-   * @param _payee The address of the payee to check.
-   */
-  function isPayee(address _payee) public constant returns (bool) {
-    return payeeIndex[_payee] > 0;
-  }
-
-  /**
-   * @dev Return the number of payees in the contract.
+   * @dev Claim your share of the balance.
    */
-  function getPayeeCount() public constant returns (uint256) {
-    return payees.length;
-  }
+  function claim() public {
+    address payee = msg.sender;
 
-  /**
-   * @dev Return the address of the payee and its shares.
-   * Throws if the payee is not in the contract.
-   * @param _payee The address of the payee to get.
-   */
-  function getPayee(address _payee) public constant returns (address, uint256) {
-    require(isPayee(_payee));
+    require(shares[payee] > 0);
 
-    return getPayeeAtIndex(payeeIndex[_payee] - 1);
-  }
+    uint256 totalReceived = this.balance.add(totalReleased);
+    uint256 payment = totalReceived.mul(shares[payee]).div(totalShares).sub(released[payee]);
 
-  /**
-   * @dev Return the address of the payee and its shares by index. 
-   * Allows iterating through the payee list from a client by knowing the payee count.
-   * @param _idx The index of the payee in the internal list.
-   */
-  function getPayeeAtIndex(uint256 _idx) public constant returns (address, uint256) {
-    require(_idx < payees.length);
+    require(payment != 0);
+    require(this.balance >= payment);
 
-    return (payees[_idx].addr, payees[_idx].shares);
-  }
+    released[payee] = released[payee].add(payment);
+    totalReleased = totalReleased.add(payment);
 
-  /**
-   * @dev Perform the payment to a payee. 
-   * This can be overriden to provide different transfer mechanisms.
-   * @param _payee The address of the payee to be paid.
-   * @param _amount The amount for the payment.
-   */
-  function pay(address _payee, uint256 _amount) internal {
-    _payee.transfer(_amount);
-  }
-
-  /**
-   * @dev Return the total amount of funds available for distribution. 
-   */
-  function toDistribute() internal returns (uint256) {
-    return this.balance;
-  }
-
-  /**
-   * @dev Send payments to the registered payees according to their shares and the total 
-   * amount of funds to distribute. 
-   */
-  function distributeFunds() public canDistribute nonReentrant {
-    uint256 amountDistribute = toDistribute();
-    assert(amountDistribute > 0);
-
-    Payee memory payee;
-    for (uint256 i = 0; i < payees.length; i++) {
-      payee = payees[i];
-
-      uint256 amount = amountDistribute.mul(payee.shares).div(totalShares);
-      pay(payee.addr, amount);
-    }
+    payee.transfer(payment);
   }
 }

contracts/payment/SplitPullPayment.sol

@@ -1,28 +0,0 @@
-pragma solidity ^0.4.15;
-
-import './PullPayment.sol';
-import './SplitPayment.sol';
-
-/**
- * @title SplitPullPayment
- * @dev Contract supporting the distribution of funds combined with withdrawals through PullPayment.
- */
-contract SplitPullPayment is SplitPayment, PullPayment {
-  /**
-   * @dev Return the total amount of funds available for distribution. 
-   * @dev Override from SplitPayment to take into account credited funds for pull payments.
-   */
-  function toDistribute() internal returns (uint256) {
-    return this.balance.sub(totalPayments);
-  }
-
-  /**
-   * @dev Perform the payment to a payee. 
-   * @dev Override from SplitPayment to do an asyncSend for later withdrawal.
-   * @param _payee The address of the payee to be paid.
-   * @param _amount The amount for the payment.
-   */
-  function pay(address _payee, uint256 _amount) internal {
-    asyncSend(_payee, _amount);
-  }
-}

test/SplitPayment.js

@@ -15,8 +15,7 @@ contract('SplitPayment', function ([owner, payee1, payee2, payee3, nonpayee1, pa
     this.payees = [payee1, payee2, payee3]
     this.shares = [20, 10, 70]
 
-    this.contract = await SplitPaymentMock.new()
-    await this.contract.addPayeeMany(this.payees, this.shares)
+    this.contract = await SplitPaymentMock.new(this.payees, this.shares)
   })
 
   it('should accept payments', async function () {
@@ -26,77 +25,54 @@ contract('SplitPayment', function ([owner, payee1, payee2, payee3, nonpayee1, pa
     balance.should.be.bignumber.equal(amount)
   })
 
-  it('should return if address is payee', async function () {
-    const isPayee = await this.contract.isPayee.call(payee1)
-    isPayee.should.equal(true)
+  it('should store shares if address is payee', async function () {
+    const shares = await this.contract.shares.call(payee1)
+    shares.should.be.bignumber.not.equal(0)
   })
 
-  it('should return if address is not payee', async function () {
-    const isPayee = await this.contract.isPayee.call(nonpayee1)
-    isPayee.should.equal(false)
+  it('should not store shares if address is not payee', async function () {
+    const shares = await this.contract.shares.call(nonpayee1)
+    shares.should.be.bignumber.equal(0)
   })
 
-  it('should return the correct payee by address', async function () {
-    const payeeIdx = 0
-    const [payee, shares] = await this.contract.getPayee.call(this.payees[payeeIdx])
-    payee.should.be.equal(payee1)
-    shares.should.be.bignumber.equal(this.shares[payeeIdx])
+  it('should throw if no funds to claim', async function () {
+    await this.contract.claim({from: payee1}).should.be.rejectedWith(EVMThrow)
   })
 
-  it('should return the correct payee by index', async function () {
-    const payeeIdx = 1
-    const [payee, shares] = await this.contract.getPayeeAtIndex.call(payeeIdx)
-    payee.should.be.equal(payee2)
-    shares.should.be.bignumber.equal(this.shares[payeeIdx])
-  })
-
-  it('should throw if payees and shares array have different sizes', async function () {
-    const payees = [payee1, payee2, payee3]
-    const shares = [50, 50]
-    await this.contract.addPayeeMany(payees, shares).should.be.rejectedWith(EVMThrow)
-  })
-
-  it('should throw if try to add same payee multiple times', async function () {
-    const payees = [payee1, payee1]
-    const shares = [50, 50]
-    await this.contract.addPayeeMany(payees, shares).should.be.rejectedWith(EVMThrow)
-  })
-
-  it('should throw if try to add payee with zero shares', async function () {
-    await this.contract.addPayee(nonpayee1, 0).should.be.rejectedWith(EVMThrow)
-  })
-
-  it('should throw if no funds to distribute', async function () {
-    await this.contract.distributeFunds({from: payee1}).should.be.rejectedWith(EVMThrow)
+  it('should throw if non-payee want to claim', async function () {
+    await web3.eth.sendTransaction({from: payer1, to: this.contract.address, value: amount})
+    await this.contract.claim({from: nonpayee1}).should.be.rejectedWith(EVMThrow)
   })
-
+  
   it('should distribute funds to payees', async function () {
     await web3.eth.sendTransaction({from: payer1, to: this.contract.address, value: amount})
 
+    // receive funds
     const initBalance = web3.eth.getBalance(this.contract.address)
     initBalance.should.be.bignumber.equal(amount)
 
+    // distribute to payees
     const initAmount1 = web3.eth.getBalance(payee1)
-    const initAmount2 = web3.eth.getBalance(payee2)
-    const initAmount3 = web3.eth.getBalance(payee3)
-
-    await this.contract.distributeFunds({from: payee1})
-
-    // Contract should have zero balance after distribution
-    const afterBalance = web3.eth.getBalance(this.contract.address)
-    afterBalance.should.be.bignumber.equal(0)
-
+    await this.contract.claim({from: payee1})
     const profit1 = web3.eth.getBalance(payee1) - initAmount1
+    assert(Math.abs(profit1 - web3.toWei(0.20, 'ether')) < 1e16)
+
+    const initAmount2 = web3.eth.getBalance(payee2)
+    await this.contract.claim({from: payee2})
     const profit2 = web3.eth.getBalance(payee2) - initAmount2
+    assert(Math.abs(profit2 - web3.toWei(0.10, 'ether')) < 1e16)
+
+    const initAmount3 = web3.eth.getBalance(payee3)
+    await this.contract.claim({from: payee3})
     const profit3 = web3.eth.getBalance(payee3) - initAmount3
+    assert(Math.abs(profit3 - web3.toWei(0.70, 'ether')) < 1e16)
 
-    assert(Math.abs(profit1 - web3.toWei(0.20, 'ether')) < 1e16);
-    assert(Math.abs(profit2 - web3.toWei(0.10, 'ether')) < 1e16);
-    assert(Math.abs(profit3 - web3.toWei(0.70, 'ether')) < 1e16);
-  })
+    // end balance should be zero
+    const endBalance = web3.eth.getBalance(this.contract.address)
+    endBalance.should.be.bignumber.equal(0)
 
-  it('should throw if non-payee want to distribute funds', async function () {
-    await web3.eth.sendTransaction({from: payer1, to: this.contract.address, value: amount})
-    await this.contract.distributeFunds({from: nonpayee1}).should.be.rejectedWith(EVMThrow)
+    // check correct funds released accounting
+    const totalReleased = await this.contract.totalReleased.call()
+    totalReleased.should.be.bignumber.equal(initBalance)
   })
 })

test/SplitPullPayment.js

@@ -1,42 +0,0 @@
-const BigNumber = web3.BigNumber
-
-const should = require('chai')
-  .use(require('chai-as-promised'))
-  .use(require('chai-bignumber')(BigNumber))
-  .should()
-
-const EVMThrow = require('./helpers/EVMThrow.js')
-const SplitPullPaymentMock = artifacts.require('./helpers/SplitPullPaymentMock.sol')
-
-contract('SplitPullPayment', function ([owner, payee1, payee2, payee3, nonpayee1, payer1]) {
-  const amount = web3.toWei(1.0, 'ether')
-
-  beforeEach(async function () {
-    this.payees = [payee1, payee2, payee3]
-    this.shares = [20, 10, 70]
-
-    this.contract = await SplitPullPaymentMock.new()
-    await this.contract.addPayeeMany(this.payees, this.shares)
-  })
-
-  it('should distribute funds to payees', async function () {
-    await web3.eth.sendTransaction({from: payer1, to: this.contract.address, value: amount})
-
-    await this.contract.distributeFunds({from: payee1})
-
-    const amount1 = await this.contract.payments.call(payee1)
-    amount1.should.be.bignumber.equal(web3.toWei(0.20, 'ether'))
-
-    const amount2 = await this.contract.payments.call(payee2)
-    amount2.should.be.bignumber.equal(web3.toWei(0.10, 'ether'))
-
-    const amount3 = await this.contract.payments.call(payee3)
-    amount3.should.be.bignumber.equal(web3.toWei(0.70, 'ether'))
-
-    const balance = web3.eth.getBalance(this.contract.address)
-    balance.should.be.bignumber.equal(amount)
-
-    const totalPayments = await this.contract.totalPayments.call()
-    balance.should.be.bignumber.equal(amount)
-  })
-})

test/helpers/SplitPaymentMock.sol

@@ -1,9 +1,10 @@
-pragma solidity ^0.4.15;
+pragma solidity ^0.4.11;
 
 import '../../contracts/payment/SplitPayment.sol';
 
 // mock class using SplitPayment
 contract SplitPaymentMock is SplitPayment {
-  function SplitPaymentMock() SplitPayment(0) payable { }
+  function SplitPaymentMock(address[] _payees, uint256[] _shares) 
+    SplitPayment(_payees, _shares) payable {}
   function () payable {}
 }

test/helpers/SplitPullPaymentMock.sol

@@ -1,9 +0,0 @@
-pragma solidity ^0.4.15;
-
-import '../../contracts/payment/SplitPullPayment.sol';
-
-// mock class using SplitPullPaymentMock
-contract SplitPullPaymentMock is SplitPullPayment {
-  function SplitPullPaymentMock() SplitPayment(0) payable { }
-  function () payable {}
-}