From a07499796af0a00bf12283f5591f934843a1fbc2 Mon Sep 17 00:00:00 2001 From: cardmaniac992 <44122792+cardmaniac992@users.noreply.github.com> Date: Thu, 18 Oct 2018 16:26:54 +0200 Subject: [PATCH] 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 --- .../{SplitPayment.sol => PaymentSplitter.sol} | 16 ++++++++--- ...ayment.test.js => PaymentSplitter.test.js} | 28 +++++++++++-------- 2 files changed, 28 insertions(+), 16 deletions(-) rename contracts/payment/{SplitPayment.sol => PaymentSplitter.sol} (84%) rename test/payment/{SplitPayment.test.js => PaymentSplitter.test.js} (73%) diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/PaymentSplitter.sol similarity index 84% rename from contracts/payment/SplitPayment.sol rename to contracts/payment/PaymentSplitter.sol index 960772182..0a1b470ef 100644 --- a/contracts/payment/SplitPayment.sol +++ b/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_); } } diff --git a/test/payment/SplitPayment.test.js b/test/payment/PaymentSplitter.test.js similarity index 73% rename from test/payment/SplitPayment.test.js rename to test/payment/PaymentSplitter.test.js index 6a10a5131..60e5654dc 100644 --- a/test/payment/SplitPayment.test.js +++ b/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);