diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol index 85c19bff9..c32ecba39 100644 --- a/contracts/payment/SplitPayment.sol +++ b/contracts/payment/SplitPayment.sol @@ -1,74 +1,27 @@ -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)); - _; - } - - /** - * @dev Modifier that throws if not allowed to update payees. - * Override from child contract with your own requirements for access control. - */ - modifier canUpdate() { - _; - } - - /** - * @dev Add a new payee to the contract. - * @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 { - require(_payee != address(0)); - require(_shares > 0); - require(!isPayee(_payee)); - require(maxPayees == 0 || payees.length.add(1) <= maxPayees); - - payees.push(Payee(_payee, _shares)); - payeeIndex[_payee] = payees.length; - 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 { + function SplitPayment(address[] _payees, uint256[] _shares) { 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]); @@ -76,73 +29,37 @@ contract SplitPayment is ReentrancyGuard { } /** - * @dev Return true if the payee is in the contract. - * @param _payee The address of the payee to check. + * @dev Add a new payee to the contract. + * @param _payee The address of the payee to add. + * @param _shares The number of shares owned by the payee. */ - function isPayee(address _payee) public constant returns (bool) { - return payeeIndex[_payee] > 0; + function addPayee(address _payee, uint256 _shares) internal { + require(_payee != address(0)); + require(_shares > 0); + require(shares[_payee] == 0); + + payees.push(_payee); + shares[_payee] = _shares; + totalShares = totalShares.add(_shares); } /** - * @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); } } diff --git a/contracts/payment/SplitPullPayment.sol b/contracts/payment/SplitPullPayment.sol deleted file mode 100644 index 0f0ef9ba6..000000000 --- a/contracts/payment/SplitPullPayment.sol +++ /dev/null @@ -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); - } -} diff --git a/test/SplitPayment.js b/test/SplitPayment.js index a12df2348..deed9d10d 100644 --- a/test/SplitPayment.js +++ b/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 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 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 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) }) }) diff --git a/test/SplitPullPayment.js b/test/SplitPullPayment.js deleted file mode 100644 index 775605339..000000000 --- a/test/SplitPullPayment.js +++ /dev/null @@ -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) - }) -}) diff --git a/test/helpers/SplitPaymentMock.sol b/test/helpers/SplitPaymentMock.sol index 80ce5a355..0f347a266 100644 --- a/test/helpers/SplitPaymentMock.sol +++ b/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 {} } diff --git a/test/helpers/SplitPullPaymentMock.sol b/test/helpers/SplitPullPaymentMock.sol deleted file mode 100644 index 559c80f6c..000000000 --- a/test/helpers/SplitPullPaymentMock.sol +++ /dev/null @@ -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 {} -}