From 8336785a9b122df236cb4516f4689892afde2f81 Mon Sep 17 00:00:00 2001 From: Ariel Barmat Date: Tue, 29 Aug 2017 03:36:47 -0300 Subject: [PATCH 01/34] Add a SplitPayment contract to distribute funds among multiple beneficiaries --- contracts/payment/SplitPayment.sol | 148 +++++++++++++++++++++++++++++ test/SplitPayment.js | 102 ++++++++++++++++++++ test/helpers/SplitPaymentMock.sol | 9 ++ 3 files changed, 259 insertions(+) create mode 100644 contracts/payment/SplitPayment.sol create mode 100644 test/SplitPayment.js create mode 100644 test/helpers/SplitPaymentMock.sol diff --git a/contracts/payment/SplitPayment.sol b/contracts/payment/SplitPayment.sol new file mode 100644 index 000000000..85c19bff9 --- /dev/null +++ b/contracts/payment/SplitPayment.sol @@ -0,0 +1,148 @@ +pragma solidity ^0.4.15; + +import '../ReentrancyGuard.sol'; +import '../math/SafeMath.sol'; + +/** + * @title SplitPayment + * @dev Base contract supporting the distribution of funds send to this contract to multiple payees. + */ +contract SplitPayment is ReentrancyGuard { + using SafeMath for uint256; + + struct Payee { + address addr; + uint256 shares; + } + + uint256 public totalShares = 0; + uint256 public maxPayees = 0; + + mapping(address => uint256) payeeIndex; + Payee[] 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 { + 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. + */ + function getPayeeCount() public constant returns (uint256) { + return payees.length; + } + + /** + * @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)); + + return getPayeeAtIndex(payeeIndex[_payee] - 1); + } + + /** + * @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); + + return (payees[_idx].addr, payees[_idx].shares); + } + + /** + * @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); + } + } +} diff --git a/test/SplitPayment.js b/test/SplitPayment.js new file mode 100644 index 000000000..a12df2348 --- /dev/null +++ b/test/SplitPayment.js @@ -0,0 +1,102 @@ +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 SplitPaymentMock = artifacts.require('./helpers/SplitPaymentMock.sol') + +contract('SplitPayment', 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 SplitPaymentMock.new() + await this.contract.addPayeeMany(this.payees, this.shares) + }) + + it('should accept payments', async function () { + await web3.eth.sendTransaction({ from: owner, to: this.contract.address, value: amount }) + + const balance = web3.eth.getBalance(this.contract.address) + 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 return if address is not payee', async function () { + const isPayee = await this.contract.isPayee.call(nonpayee1) + isPayee.should.equal(false) + }) + + 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 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 distribute funds to payees', async function () { + await web3.eth.sendTransaction({from: payer1, to: this.contract.address, value: amount}) + + const initBalance = web3.eth.getBalance(this.contract.address) + initBalance.should.be.bignumber.equal(amount) + + 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) + + const profit1 = web3.eth.getBalance(payee1) - initAmount1 + const profit2 = web3.eth.getBalance(payee2) - initAmount2 + const profit3 = web3.eth.getBalance(payee3) - initAmount3 + + 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); + }) + + 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) + }) +}) diff --git a/test/helpers/SplitPaymentMock.sol b/test/helpers/SplitPaymentMock.sol new file mode 100644 index 000000000..80ce5a355 --- /dev/null +++ b/test/helpers/SplitPaymentMock.sol @@ -0,0 +1,9 @@ +pragma solidity ^0.4.15; + +import '../../contracts/payment/SplitPayment.sol'; + +// mock class using SplitPayment +contract SplitPaymentMock is SplitPayment { + function SplitPaymentMock() SplitPayment(0) payable { } + function () payable {} +} From 69e83e508627ab23cfa40e926375cca7353cab6a Mon Sep 17 00:00:00 2001 From: Ariel Barmat Date: Tue, 29 Aug 2017 03:40:41 -0300 Subject: [PATCH 02/34] Add a SplitPullPayment contract that combines distribution of funds and delayed withdrawal from each party --- contracts/payment/SplitPullPayment.sol | 28 +++++++++++++++++ test/SplitPullPayment.js | 42 ++++++++++++++++++++++++++ test/helpers/SplitPullPaymentMock.sol | 9 ++++++ 3 files changed, 79 insertions(+) create mode 100644 contracts/payment/SplitPullPayment.sol create mode 100644 test/SplitPullPayment.js create mode 100644 test/helpers/SplitPullPaymentMock.sol diff --git a/contracts/payment/SplitPullPayment.sol b/contracts/payment/SplitPullPayment.sol new file mode 100644 index 000000000..0f0ef9ba6 --- /dev/null +++ b/contracts/payment/SplitPullPayment.sol @@ -0,0 +1,28 @@ +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/SplitPullPayment.js b/test/SplitPullPayment.js new file mode 100644 index 000000000..775605339 --- /dev/null +++ b/test/SplitPullPayment.js @@ -0,0 +1,42 @@ +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/SplitPullPaymentMock.sol b/test/helpers/SplitPullPaymentMock.sol new file mode 100644 index 000000000..559c80f6c --- /dev/null +++ b/test/helpers/SplitPullPaymentMock.sol @@ -0,0 +1,9 @@ +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 {} +} From dc1017c92983656071f530edcebe4635586c34fe Mon Sep 17 00:00:00 2001 From: Ariel Barmat Date: Sat, 9 Sep 2017 11:58:23 -0300 Subject: [PATCH 03/34] Simplify implementation using similar interface as PullPayment contract --- contracts/payment/SplitPayment.sol | 145 ++++++------------------- contracts/payment/SplitPullPayment.sol | 28 ----- test/SplitPayment.js | 86 ++++++--------- test/SplitPullPayment.js | 42 ------- test/helpers/SplitPaymentMock.sol | 5 +- test/helpers/SplitPullPaymentMock.sol | 9 -- 6 files changed, 65 insertions(+), 250 deletions(-) delete mode 100644 contracts/payment/SplitPullPayment.sol delete mode 100644 test/SplitPullPayment.js delete mode 100644 test/helpers/SplitPullPaymentMock.sol 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 {} -} From 27f8609ac949fb3a0b24b8194e6ff3eb2dcd0f67 Mon Sep 17 00:00:00 2001 From: nedodn Date: Wed, 27 Sep 2017 14:07:08 -0500 Subject: [PATCH 04/34] Update TokenTimelock.sol: Issue #464 Removed deprecated function claim() as per Issue #464. --- contracts/token/TokenTimelock.sol | 9 --------- 1 file changed, 9 deletions(-) diff --git a/contracts/token/TokenTimelock.sol b/contracts/token/TokenTimelock.sol index 1618e3fe7..f4255fbbf 100644 --- a/contracts/token/TokenTimelock.sol +++ b/contracts/token/TokenTimelock.sol @@ -28,15 +28,6 @@ contract TokenTimelock { releaseTime = _releaseTime; } - /** - * @notice Transfers tokens held by timelock to beneficiary. - * Deprecated: please use TokenTimelock#release instead. - */ - function claim() public { - require(msg.sender == beneficiary); - release(); - } - /** * @notice Transfers tokens held by timelock to beneficiary. */ From 9b0e89c4bfb11711df5b8205ebfdaab6561d6331 Mon Sep 17 00:00:00 2001 From: Eric Thomas Date: Thu, 28 Sep 2017 17:18:47 -0600 Subject: [PATCH 05/34] Add `npm init` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If you don't do this, you'll receive this error: ``` ❯ zeppelin npm install zeppelin-solidity npm WARN saveError ENOENT: no such file or directory, open '/Users/et/package.json' npm WARN enoent ENOENT: no such file or directory, open '/Users/et/package.json' npm WARN et No description npm WARN et No repository field. npm WARN et No README data npm WARN et No license field. + zeppelin-solidity@1.3.0 updated 1 package in 0.677s ``` --- docs/source/getting-started.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/getting-started.rst b/docs/source/getting-started.rst index 6f84f870a..0381c2cf7 100644 --- a/docs/source/getting-started.rst +++ b/docs/source/getting-started.rst @@ -9,6 +9,7 @@ Zeppelin integrates with `Truffle `_, an To install the Zeppelin library, run:: + npm init # follow instructions npm i zeppelin-solidity After that, you'll get all the library's contracts in the contracts/zeppelin folder. You can use the contracts in the library like so:: From a9055f5ce2dd81276c6b0adbec584e05260ffefc Mon Sep 17 00:00:00 2001 From: Rudy Godoy Date: Fri, 29 Sep 2017 14:55:18 -0500 Subject: [PATCH 06/34] Fixes typos --- docs/source/ecrecovery.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/ecrecovery.rst b/docs/source/ecrecovery.rst index dea50f8a8..ac6668686 100644 --- a/docs/source/ecrecovery.rst +++ b/docs/source/ecrecovery.rst @@ -1,7 +1,7 @@ -ECReovery +ECRecovery ============================================= -Returns the signer of the the hash using the signature divided in v, r, and s values. +Returns the signer of the hash using the signature divided in v, r, and s values. recover(bytes32 hash, bytes sig) internal returns (address) """"""""""""""""""""""""""""""""""""""""""""""""" From 2ad88b59f6d2658c49bb4cda3fec056a4cb5ebe5 Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Sun, 1 Oct 2017 22:27:42 -0400 Subject: [PATCH 07/34] Small indenting fix for bounty docs --- docs/source/bounty.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/bounty.rst b/docs/source/bounty.rst index d82707eca..6ad0fbf96 100644 --- a/docs/source/bounty.rst +++ b/docs/source/bounty.rst @@ -7,8 +7,8 @@ To create a bounty for your contract, inherit from the base `Bounty` contract an import "./YourContract.sol"; contract YourBounty is Bounty { - function deployContract() internal returns(address) { - return new YourContract() + function deployContract() internal returns(address) { + return new YourContract() } } From 1333f45cdcfc4d187a41117a594440c2d017a3ee Mon Sep 17 00:00:00 2001 From: Brett Sun Date: Sun, 1 Oct 2017 22:34:58 -0400 Subject: [PATCH 08/34] Add missing code blocks to bounty docs --- docs/source/bounty.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/source/bounty.rst b/docs/source/bounty.rst index 6ad0fbf96..89074c6c9 100644 --- a/docs/source/bounty.rst +++ b/docs/source/bounty.rst @@ -14,7 +14,7 @@ To create a bounty for your contract, inherit from the base `Bounty` contract an Next, implement invariant logic into your smart contract. -Your main contract should inherit from the Target class and implement the checkInvariant method. This is a function that should check everything your contract assumes to be true all the time. If this function returns false, it means your contract was broken in some way and is in an inconsistent state. This is what security researchers will try to acomplish when trying to get the bounty. +Your main contract should inherit from the `Target` class and implement the ```checkInvariant()``` method. This is a function that should check everything your contract assumes to be true all the time. If this function returns false, it means your contract was broken in some way and is in an inconsistent state. This is what security researchers will try to acomplish when trying to get the bounty. At contracts/YourContract.sol:: @@ -35,7 +35,7 @@ At ```migrations/2_deploy_contracts.js```:: deployer.deploy(YourBounty); }; -Next, add a reward to the bounty contract +Next, add a reward to the bounty contract. After deploying the contract, send reward funds into the bounty contract. From 790833e5d4481ca43deb75019b5cc4917bbbcea2 Mon Sep 17 00:00:00 2001 From: Dmitry Dudin Date: Sun, 8 Oct 2017 04:26:21 +0300 Subject: [PATCH 09/34] Update SimpleToken.sol --- contracts/examples/SimpleToken.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/examples/SimpleToken.sol b/contracts/examples/SimpleToken.sol index b1ea2bdcb..93a469bf1 100644 --- a/contracts/examples/SimpleToken.sol +++ b/contracts/examples/SimpleToken.sol @@ -14,7 +14,7 @@ contract SimpleToken is StandardToken { string public constant name = "SimpleToken"; string public constant symbol = "SIM"; - uint256 public constant decimals = 18; + uint8 public constant decimals = 18; uint256 public constant INITIAL_SUPPLY = 10000; From 01b92d1d56175129bab9ec793db980f527f9851c Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 9 Oct 2017 19:01:04 -0300 Subject: [PATCH 10/34] fix a problem when revoke is called twice --- contracts/token/TokenVesting.sol | 12 +++++++++--- test/TokenVesting.js | 21 +++++++++++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index 3cf576f5b..6d43edff4 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -27,6 +27,7 @@ contract TokenVesting is Ownable { bool revocable; mapping (address => uint256) released; + mapping (address => bool) revoked; /** * @dev Creates a vesting contract that vests its balance of any ERC20 token to the @@ -65,17 +66,22 @@ contract TokenVesting is Ownable { } /** - * @notice Allows the owner to revoke the vesting. Tokens already vested remain in the contract. + * @notice Allows the owner to revoke the vesting. Tokens already vested + * remain in the contract, the rest are returned to the owner. * @param token ERC20 token which is being vested */ function revoke(ERC20Basic token) onlyOwner { require(revocable); + require(!revoked[token]); uint256 balance = token.balanceOf(this); uint256 vested = vestedAmount(token); + uint256 vesting = balance - vested; - token.transfer(owner, balance - vested); + revoked[token] = true; + + token.transfer(owner, vesting); Revoked(); } @@ -87,7 +93,7 @@ contract TokenVesting is Ownable { function vestedAmount(ERC20Basic token) constant returns (uint256) { if (now < cliff) { return 0; - } else if (now >= start + duration) { + } else if (now >= start + duration || revoked[token]) { return token.balanceOf(this); } else { uint256 currentBalance = token.balanceOf(this); diff --git a/test/TokenVesting.js b/test/TokenVesting.js index dee1001a7..c391c6577 100644 --- a/test/TokenVesting.js +++ b/test/TokenVesting.js @@ -80,8 +80,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should return the non-vested tokens when revoked by owner', async function () { - await increaseTimeTo(this.start + this.cliff + duration.weeks(1)); - await this.vesting.release(this.token.address); + await increaseTimeTo(this.start + this.cliff + duration.weeks(12)); const vested = await this.vesting.vestedAmount(this.token.address); const balance = await this.token.balanceOf(this.vesting.address); @@ -93,15 +92,25 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { }); it('should keep the vested tokens when revoked by owner', async function () { - await increaseTimeTo(this.start + this.cliff + duration.weeks(1)); - await this.vesting.release(this.token.address); + await increaseTimeTo(this.start + this.cliff + duration.weeks(12)); + + const vestedPre = await this.vesting.vestedAmount(this.token.address); + + await this.vesting.revoke(this.token.address, { from: owner }); + + const vestedPost = await this.vesting.vestedAmount(this.token.address); + + vestedPre.should.bignumber.equal(vestedPost); + }); + + it('should fail to be revoked a second time', async function () { + await increaseTimeTo(this.start + this.cliff + duration.weeks(12)); const vested = await this.vesting.vestedAmount(this.token.address); await this.vesting.revoke(this.token.address, { from: owner }); - const balance = await this.token.balanceOf(this.vesting.address); - balance.should.bignumber.equal(vested); + await this.vesting.revoke(this.token.address, { from: owner }).should.be.rejectedWith(EVMThrow); }); }); From 7d08c4da7fd94898143479ed31dee10d941388a7 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 9 Oct 2017 19:02:57 -0300 Subject: [PATCH 11/34] make TokenVesting variables public --- contracts/token/TokenVesting.sol | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index 6d43edff4..2612fd42a 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -18,16 +18,16 @@ contract TokenVesting is Ownable { event Revoked(); // beneficiary of tokens after they are released - address beneficiary; + address public beneficiary; - uint256 cliff; - uint256 start; - uint256 duration; + uint256 public cliff; + uint256 public start; + uint256 public duration; - bool revocable; + bool public revocable; - mapping (address => uint256) released; - mapping (address => bool) revoked; + mapping (address => uint256) public released; + mapping (address => bool) public revoked; /** * @dev Creates a vesting contract that vests its balance of any ERC20 token to the From a184013d1e928f39e0bbf88cc98019c62f833a8e Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Mon, 9 Oct 2017 19:08:32 -0300 Subject: [PATCH 12/34] explicitly mark functions public --- contracts/token/TokenVesting.sol | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index 2612fd42a..c8e8c9214 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -53,7 +53,7 @@ contract TokenVesting is Ownable { * @notice Transfers vested tokens to beneficiary. * @param token ERC20 token which is being vested */ - function release(ERC20Basic token) { + function release(ERC20Basic token) public { uint256 vested = vestedAmount(token); require(vested > 0); @@ -70,7 +70,7 @@ contract TokenVesting is Ownable { * remain in the contract, the rest are returned to the owner. * @param token ERC20 token which is being vested */ - function revoke(ERC20Basic token) onlyOwner { + function revoke(ERC20Basic token) public onlyOwner { require(revocable); require(!revoked[token]); @@ -90,7 +90,7 @@ contract TokenVesting is Ownable { * @dev Calculates the amount that has already vested but hasn't been released yet. * @param token ERC20 token which is being vested */ - function vestedAmount(ERC20Basic token) constant returns (uint256) { + function vestedAmount(ERC20Basic token) public constant returns (uint256) { if (now < cliff) { return 0; } else if (now >= start + duration || revoked[token]) { From d5e0714faf4b139aac319bab6ad3a10cd0918640 Mon Sep 17 00:00:00 2001 From: Francisco Giordano Date: Wed, 11 Oct 2017 16:31:31 -0300 Subject: [PATCH 13/34] use safeTransfer --- contracts/token/TokenVesting.sol | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index c8e8c9214..2d8eabb6a 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -1,6 +1,7 @@ pragma solidity ^0.4.11; import './ERC20Basic.sol'; +import './SafeERC20.sol'; import '../ownership/Ownable.sol'; import '../math/Math.sol'; import '../math/SafeMath.sol'; @@ -13,6 +14,7 @@ import '../math/SafeMath.sol'; */ contract TokenVesting is Ownable { using SafeMath for uint256; + using SafeERC20 for ERC20Basic; event Released(uint256 amount); event Revoked(); @@ -58,7 +60,7 @@ contract TokenVesting is Ownable { require(vested > 0); - token.transfer(beneficiary, vested); + token.safeTransfer(beneficiary, vested); released[token] = released[token].add(vested); @@ -81,7 +83,7 @@ contract TokenVesting is Ownable { revoked[token] = true; - token.transfer(owner, vesting); + token.safeTransfer(owner, vesting); Revoked(); } From 74636b73335403956020bc866094f1a8a3d277d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Triay?= Date: Thu, 12 Oct 2017 00:34:03 -0300 Subject: [PATCH 14/34] [TokenVesting] Use SafeMath --- contracts/token/TokenVesting.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index 2d8eabb6a..a85f02dfe 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -47,7 +47,7 @@ contract TokenVesting is Ownable { beneficiary = _beneficiary; revocable = _revocable; duration = _duration; - cliff = _start + _cliff; + cliff = _start.add(_cliff); start = _start; } From fff8e040b7099d54123949596bba316d1ec398a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Triay?= Date: Fri, 13 Oct 2017 15:32:44 -0300 Subject: [PATCH 15/34] [TokenVesting] vestedAmount returns the historical vested amount --- contracts/token/TokenVesting.sol | 37 ++++++++++++++++++-------------- test/TokenVesting.js | 3 +-- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index a85f02dfe..c201a4770 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -57,14 +57,15 @@ contract TokenVesting is Ownable { */ function release(ERC20Basic token) public { uint256 vested = vestedAmount(token); + uint256 unreleased = releasableAmount(token); - require(vested > 0); + require(unreleased > 0); - token.safeTransfer(beneficiary, vested); + token.safeTransfer(beneficiary, unreleased); - released[token] = released[token].add(vested); + released[token] = released[token].add(unreleased); - Released(vested); + Released(unreleased); } /** @@ -78,12 +79,12 @@ contract TokenVesting is Ownable { uint256 balance = token.balanceOf(this); - uint256 vested = vestedAmount(token); - uint256 vesting = balance - vested; + uint256 unreleased = releasableAmount(token); + uint256 refund = balance.sub(unreleased); revoked[token] = true; - token.safeTransfer(owner, vesting); + token.safeTransfer(owner, refund); Revoked(); } @@ -92,20 +93,24 @@ contract TokenVesting is Ownable { * @dev Calculates the amount that has already vested but hasn't been released yet. * @param token ERC20 token which is being vested */ + function releasableAmount(ERC20Basic token) public constant returns (uint256) { + return vestedAmount(token).sub(released[token]); + } + + /** + * @dev Calculates the amount that has already vested. + * @param token ERC20 token which is being vested + */ function vestedAmount(ERC20Basic token) public constant returns (uint256) { + uint256 currentBalance = token.balanceOf(this); + uint256 totalBalance = currentBalance.add(released[token]); + if (now < cliff) { return 0; } else if (now >= start + duration || revoked[token]) { - return token.balanceOf(this); + return totalBalance; } else { - uint256 currentBalance = token.balanceOf(this); - uint256 totalBalance = currentBalance.add(released[token]); - - uint256 vested = totalBalance.mul(now - start).div(duration); - uint256 unreleased = vested.sub(released[token]); - - // currentBalance can be 0 in case of vesting being revoked earlier. - return Math.min256(currentBalance, unreleased); + return totalBalance.mul(now - start).div(duration); } } } diff --git a/test/TokenVesting.js b/test/TokenVesting.js index c391c6577..6a4d86206 100644 --- a/test/TokenVesting.js +++ b/test/TokenVesting.js @@ -83,12 +83,11 @@ contract('TokenVesting', function ([_, owner, beneficiary]) { await increaseTimeTo(this.start + this.cliff + duration.weeks(12)); const vested = await this.vesting.vestedAmount(this.token.address); - const balance = await this.token.balanceOf(this.vesting.address); await this.vesting.revoke(this.token.address, { from: owner }); const ownerBalance = await this.token.balanceOf(owner); - ownerBalance.should.bignumber.equal(balance.sub(vested)); + ownerBalance.should.bignumber.equal(amount.sub(vested)); }); it('should keep the vested tokens when revoked by owner', async function () { From aa431dfb20893f79c23af720a58b3b9c995dc359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Triay?= Date: Fri, 13 Oct 2017 16:13:54 -0300 Subject: [PATCH 16/34] [TokenVesting] Increase released amount before transfer --- contracts/token/TokenVesting.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index c201a4770..2861672d8 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -61,10 +61,10 @@ contract TokenVesting is Ownable { require(unreleased > 0); - token.safeTransfer(beneficiary, unreleased); - released[token] = released[token].add(unreleased); + token.safeTransfer(beneficiary, unreleased); + Released(unreleased); } From eb9f88bafcf00555d1ba045360e3256cdb8f58ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Triay?= Date: Fri, 13 Oct 2017 16:20:24 -0300 Subject: [PATCH 17/34] [TokenVesting] Remove unused variable --- contracts/token/TokenVesting.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index 2861672d8..a4ca4b088 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -56,7 +56,6 @@ contract TokenVesting is Ownable { * @param token ERC20 token which is being vested */ function release(ERC20Basic token) public { - uint256 vested = vestedAmount(token); uint256 unreleased = releasableAmount(token); require(unreleased > 0); From b60e434e0b300db45ccc03109b3298867e7f45fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mart=C3=ADn=20Triay?= Date: Mon, 16 Oct 2017 03:48:32 -0300 Subject: [PATCH 18/34] [TokenVesting] Add missing safemath ops --- contracts/token/TokenVesting.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index a4ca4b088..da6d4a53f 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -106,10 +106,10 @@ contract TokenVesting is Ownable { if (now < cliff) { return 0; - } else if (now >= start + duration || revoked[token]) { + } else if (now >= start.add(duration) || revoked[token]) { return totalBalance; } else { - return totalBalance.mul(now - start).div(duration); + return totalBalance.mul(now.sub(start)).div(duration); } } } From dfb9ebd647aadaaa0a02b8a9f47b37287b24361e Mon Sep 17 00:00:00 2001 From: Norton Wang Date: Mon, 16 Oct 2017 17:50:25 -0400 Subject: [PATCH 19/34] Update ecrecovery.rst Fix typo --- docs/source/ecrecovery.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/source/ecrecovery.rst b/docs/source/ecrecovery.rst index dea50f8a8..76db6da8e 100644 --- a/docs/source/ecrecovery.rst +++ b/docs/source/ecrecovery.rst @@ -1,4 +1,4 @@ -ECReovery +ECRecovery ============================================= Returns the signer of the the hash using the signature divided in v, r, and s values. From 2d83c557b1c89b4486ffe68bf71b2def7f624e61 Mon Sep 17 00:00:00 2001 From: Mikhail Melnik Date: Tue, 17 Oct 2017 16:24:16 +0300 Subject: [PATCH 20/34] Finish minting should me called only once Multiple calls to `finishMinting` will emit multiple `MintFinished` events which may be surprising if one rely on `MintFinished` event. --- contracts/token/MintableToken.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/MintableToken.sol b/contracts/token/MintableToken.sol index 234def478..4f3e3c505 100644 --- a/contracts/token/MintableToken.sol +++ b/contracts/token/MintableToken.sol @@ -43,7 +43,7 @@ contract MintableToken is StandardToken, Ownable { * @dev Function to stop minting new tokens. * @return True if the operation was successful. */ - function finishMinting() onlyOwner public returns (bool) { + function finishMinting() onlyOwner canMint public returns (bool) { mintingFinished = true; MintFinished(); return true; From ca1babe1f7b3609611e3880c7973ac728071a715 Mon Sep 17 00:00:00 2001 From: "Dora E. Mondrian" <33023863+doraemondrian@users.noreply.github.com> Date: Mon, 23 Oct 2017 03:06:51 -0700 Subject: [PATCH 21/34] Use address type --- contracts/crowdsale/Crowdsale.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/crowdsale/Crowdsale.sol b/contracts/crowdsale/Crowdsale.sol index 32eb1b4dd..26ed8df85 100644 --- a/contracts/crowdsale/Crowdsale.sol +++ b/contracts/crowdsale/Crowdsale.sol @@ -44,7 +44,7 @@ contract Crowdsale { require(_startTime >= now); require(_endTime >= _startTime); require(_rate > 0); - require(_wallet != 0x0); + require(_wallet != address(0)); token = createTokenContract(); startTime = _startTime; @@ -67,7 +67,7 @@ contract Crowdsale { // low level token purchase function function buyTokens(address beneficiary) public payable { - require(beneficiary != 0x0); + require(beneficiary != address(0)); require(validPurchase()); uint256 weiAmount = msg.value; From e4427befbbac5aedc02b1df0306b4533a41d625e Mon Sep 17 00:00:00 2001 From: "Dora E. Mondrian" <33023863+doraemondrian@users.noreply.github.com> Date: Mon, 23 Oct 2017 03:08:30 -0700 Subject: [PATCH 22/34] Use address type --- contracts/token/MintableToken.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/MintableToken.sol b/contracts/token/MintableToken.sol index 234def478..600d94db5 100644 --- a/contracts/token/MintableToken.sol +++ b/contracts/token/MintableToken.sol @@ -35,7 +35,7 @@ contract MintableToken is StandardToken, Ownable { totalSupply = totalSupply.add(_amount); balances[_to] = balances[_to].add(_amount); Mint(_to, _amount); - Transfer(0x0, _to, _amount); + Transfer(address(0), _to, _amount); return true; } From d7b67eca52aa952db6916bf6a48c6669ff148942 Mon Sep 17 00:00:00 2001 From: "Dora E. Mondrian" <33023863+doraemondrian@users.noreply.github.com> Date: Mon, 23 Oct 2017 03:10:00 -0700 Subject: [PATCH 23/34] Use address type --- contracts/token/TokenVesting.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index a4ca4b088..d1911f818 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -41,7 +41,7 @@ contract TokenVesting is Ownable { * @param _revocable whether the vesting is revocable or not */ function TokenVesting(address _beneficiary, uint256 _start, uint256 _cliff, uint256 _duration, bool _revocable) { - require(_beneficiary != 0x0); + require(_beneficiary != address(0)); require(_cliff <= _duration); beneficiary = _beneficiary; From 09b1926c41e4acff3d46ad8a49cfdb9940ba37fe Mon Sep 17 00:00:00 2001 From: "Dora E. Mondrian" <33023863+doraemondrian@users.noreply.github.com> Date: Mon, 23 Oct 2017 03:10:40 -0700 Subject: [PATCH 24/34] Use address type --- contracts/ownership/Claimable.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/ownership/Claimable.sol b/contracts/ownership/Claimable.sol index 321b9aa68..299f2c8aa 100644 --- a/contracts/ownership/Claimable.sol +++ b/contracts/ownership/Claimable.sol @@ -34,6 +34,6 @@ contract Claimable is Ownable { function claimOwnership() onlyPendingOwner public { OwnershipTransferred(owner, pendingOwner); owner = pendingOwner; - pendingOwner = 0x0; + pendingOwner = address(0); } } From ac4a19dd7d59047f2c0601cf12f9abaaf221012b Mon Sep 17 00:00:00 2001 From: "Dora E. Mondrian" <33023863+doraemondrian@users.noreply.github.com> Date: Mon, 23 Oct 2017 03:11:14 -0700 Subject: [PATCH 25/34] Use address type --- contracts/ownership/DelayedClaimable.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/ownership/DelayedClaimable.sol b/contracts/ownership/DelayedClaimable.sol index 329cd5252..0384b436b 100644 --- a/contracts/ownership/DelayedClaimable.sol +++ b/contracts/ownership/DelayedClaimable.sol @@ -35,7 +35,7 @@ contract DelayedClaimable is Claimable { require((block.number <= end) && (block.number >= start)); OwnershipTransferred(owner, pendingOwner); owner = pendingOwner; - pendingOwner = 0x0; + pendingOwner = address(0); end = 0; } From 5ad07e1892a08e8485d0ba6752eeb67fbd4182b2 Mon Sep 17 00:00:00 2001 From: Chen Yi-Cyuan Date: Mon, 30 Oct 2017 09:58:52 +0800 Subject: [PATCH 26/34] improve mum performance and reduce gas cost --- contracts/math/SafeMath.sol | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contracts/math/SafeMath.sol b/contracts/math/SafeMath.sol index 47b7db51b..05ad6ee17 100644 --- a/contracts/math/SafeMath.sol +++ b/contracts/math/SafeMath.sol @@ -7,8 +7,11 @@ pragma solidity ^0.4.11; */ library SafeMath { function mul(uint256 a, uint256 b) internal constant returns (uint256) { + if (a == 0) { + return 0; + } uint256 c = a * b; - assert(a == 0 || c / a == b); + assert(c / a == b); return c; } From e74652415f854faae343c8157b5d6b7f89a6737a Mon Sep 17 00:00:00 2001 From: thesved <2893181+thesved@users.noreply.github.com> Date: Mon, 30 Oct 2017 11:07:26 +0100 Subject: [PATCH 27/34] Math.sol is not used, not needed Small thing, but the Math.sol is not really needed, since it is not used. --- contracts/token/TokenVesting.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/token/TokenVesting.sol b/contracts/token/TokenVesting.sol index 7d6c87ea0..7fb33b896 100644 --- a/contracts/token/TokenVesting.sol +++ b/contracts/token/TokenVesting.sol @@ -3,7 +3,6 @@ pragma solidity ^0.4.11; import './ERC20Basic.sol'; import './SafeERC20.sol'; import '../ownership/Ownable.sol'; -import '../math/Math.sol'; import '../math/SafeMath.sol'; /** From be692e59ebf2a03e9dabdc60d3903be1cdbe2387 Mon Sep 17 00:00:00 2001 From: Andy Chen Date: Tue, 31 Oct 2017 00:13:27 -0700 Subject: [PATCH 28/34] Update RefundVault.sol --- contracts/crowdsale/RefundVault.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/crowdsale/RefundVault.sol b/contracts/crowdsale/RefundVault.sol index ea74269de..fb721d17a 100644 --- a/contracts/crowdsale/RefundVault.sol +++ b/contracts/crowdsale/RefundVault.sol @@ -23,7 +23,7 @@ contract RefundVault is Ownable { event Refunded(address indexed beneficiary, uint256 weiAmount); function RefundVault(address _wallet) { - require(_wallet != 0x0); + require(_wallet != address(0)); wallet = _wallet; state = State.Active; } From 2413f83504208261c6b6bd7811e8cc0ed3f53b61 Mon Sep 17 00:00:00 2001 From: Alejandro Santander Date: Fri, 10 Nov 2017 11:41:17 -0300 Subject: [PATCH 29/34] Include 'revert' in expectThrow() helper --- package-lock.json | 20 ++++++++++---------- test/helpers/expectThrow.js | 3 ++- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/package-lock.json b/package-lock.json index 03db7b09d..52be47677 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "zeppelin-solidity", - "version": "1.2.0", + "version": "1.3.0", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -5084,15 +5084,6 @@ "xtend": "4.0.1" } }, - "string_decoder": { - "version": "1.0.3", - "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", - "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", - "dev": true, - "requires": { - "safe-buffer": "5.1.1" - } - }, "string-width": { "version": "1.0.2", "resolved": "https://registry.npmjs.org/string-width/-/string-width-1.0.2.tgz", @@ -5115,6 +5106,15 @@ "function-bind": "1.1.0" } }, + "string_decoder": { + "version": "1.0.3", + "resolved": "https://registry.npmjs.org/string_decoder/-/string_decoder-1.0.3.tgz", + "integrity": "sha512-4AH6Z5fzNNBcH+6XDMfA/BTt87skxqJlO0lAh3Dker5zThcAxG6mKz+iGu308UKoPPQ8Dcqx/4JhujzltRa+hQ==", + "dev": true, + "requires": { + "safe-buffer": "5.1.1" + } + }, "stringstream": { "version": "0.0.5", "resolved": "https://registry.npmjs.org/stringstream/-/stringstream-0.0.5.tgz", diff --git a/test/helpers/expectThrow.js b/test/helpers/expectThrow.js index 6aeab6064..c6641e66f 100644 --- a/test/helpers/expectThrow.js +++ b/test/helpers/expectThrow.js @@ -10,8 +10,9 @@ export default async promise => { // we distinguish this from an actual out of gas event? (The // testrpc log actually show an 'invalid jump' event.) const outOfGas = error.message.search('out of gas') >= 0; + const revert = error.message.search('revert') >= 0; assert( - invalidOpcode || outOfGas, + invalidOpcode || outOfGas || revert, "Expected throw, got '" + error + "' instead", ); return; From 070226d624f037643b601159fdb97f0b962140e2 Mon Sep 17 00:00:00 2001 From: Santiago Palladino Date: Fri, 10 Nov 2017 13:18:20 -0300 Subject: [PATCH 30/34] Add Slack notifications --- .travis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index e4af9c60f..13c33fbca 100644 --- a/.travis.yml +++ b/.travis.yml @@ -15,3 +15,9 @@ matrix: - env: SOLIDITY_COVERAGE=true script: - yarn test +notifications: + slack: + rooms: + - secure: uEhwUkuwJp5pBNh+VTEytPHz3FDKsnPrKO+8MTAKv5hKi4PCRoVhLv6pklr82HUpL6pvSvJbUPA0HVebOXA+MMSxdny/BHZTh2mtw5Y78l2Ad0svDTWuV2Lus2pmhYigRhT0Wo00/SRX9+pxm0kg4EIFJSTS+uR9G76x0l9NljpEGXrqxlDxjxoHBgk8Ciru2LHaLzX/utE3jlABts4Sb1F3wc2BwFkjd6BDCRTGAPhVJwwFk41ZfnmLVbgSNUyk46Cb38oG5oXHb0FI3d3jV/k1OUhRyFfmA2fLXRk0wavibW8TG1gGJJWZ7xTCKzw/Cvup6mpehSAeQef8eekMdjpWEhF9hYRq1BvOs0384UU8NQ0O+BtdXU+X3Nyr84TMJN/iIfgN7gYX7AsvXH3jC0JfNUcIkWlJvyXdE6l2GV1hMmhL09GFEBbSpuSXRIWlOXTcYBlp5NbvE8xO8PUW+T6N5RG2XXjv1g8wCpr6Wwk1+LmRkX5trv8MFBZ2pM8p4H5da5++Ov8egLonNGK2jbx6aBLBX3tPf+g70LZEkiQ4eBfZw8VIgXIvKreisicppNuCD27gNmSEPNt0NkwiEBcTCJ9GSVAO0CU2g4ggvHDX2A+RW5XPET9bGkBXKLfFyV7Qe+MSQjXkCnW3bIRh7Wo1V31XiUiYOLuZPIiH3EQ= + on_success: change + on_failure: always From c4ca7f03e33a0085719db805fa6739c8ce6e9ce9 Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Fri, 10 Nov 2017 13:29:58 -0800 Subject: [PATCH 31/34] improve timer test helper --- test/helpers/timer.js | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/test/helpers/timer.js b/test/helpers/timer.js index 2a0b7c252..a947b0849 100644 --- a/test/helpers/timer.js +++ b/test/helpers/timer.js @@ -1,15 +1,29 @@ +'use strict'; + // timer for tests specific to testrpc -module.exports = s => { +// s is the amount of seconds to advance +// if account is provided, will send a transaction from that account to force testrpc to mine the block +module.exports = (s) => { return new Promise((resolve, reject) => { web3.currentProvider.sendAsync({ - jsonrpc: '2.0', + jsonrpc: '2.0', method: 'evm_increaseTime', - params: [s], // 60 seaconds, may need to be hex, I forget - id: new Date().getTime() // Id of the request; anything works, really + params: [s], + id: new Date().getTime() }, function(err) { - if (err) return reject(err); - resolve(); + if (err) { + return reject(err); + } + web3.currentProvider.sendAsync({ + jsonrpc: '2.0', + method: 'evm_mine', + id: new Date().getTime() + }, (err, result) => { + if (err) { + return reject(err); + } + resolve(result); + }); }); - //setTimeout(() => resolve(), s * 1000 + 600) // 600ms breathing room for testrpc to sync }); }; From 7dd0ee621214cab1dbfe252368646d4e9bd1be82 Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Sun, 12 Nov 2017 15:10:46 -0500 Subject: [PATCH 32/34] remove timer --- test/DayLimit.js | 6 ++++-- test/helpers/timer.js | 29 ----------------------------- 2 files changed, 4 insertions(+), 31 deletions(-) delete mode 100644 test/helpers/timer.js diff --git a/test/DayLimit.js b/test/DayLimit.js index 66befb00e..9d96383c5 100644 --- a/test/DayLimit.js +++ b/test/DayLimit.js @@ -1,6 +1,7 @@ 'use strict'; const assertJump = require('./helpers/assertJump'); -const timer = require('./helpers/timer'); +import latestTime from './helpers/latestTime' +import {increaseTimeTo, duration} from './helpers/increaseTime' var DayLimitMock = artifacts.require('./helpers/DayLimitMock.sol'); @@ -11,6 +12,7 @@ contract('DayLimit', function(accounts) { let initLimit = 10; beforeEach( async function() { + this.startTime = latestTime(); dayLimit = await DayLimitMock.new(initLimit); }); @@ -99,7 +101,7 @@ contract('DayLimit', function(accounts) { spentToday = await dayLimit.spentToday(); assert.equal(spentToday, 8); - await timer(day); + await increaseTimeTo(this.startTime + duration.days(1)); await dayLimit.attemptSpend(3); spentToday = await dayLimit.spentToday(); diff --git a/test/helpers/timer.js b/test/helpers/timer.js deleted file mode 100644 index a947b0849..000000000 --- a/test/helpers/timer.js +++ /dev/null @@ -1,29 +0,0 @@ -'use strict'; - -// timer for tests specific to testrpc -// s is the amount of seconds to advance -// if account is provided, will send a transaction from that account to force testrpc to mine the block -module.exports = (s) => { - return new Promise((resolve, reject) => { - web3.currentProvider.sendAsync({ - jsonrpc: '2.0', - method: 'evm_increaseTime', - params: [s], - id: new Date().getTime() - }, function(err) { - if (err) { - return reject(err); - } - web3.currentProvider.sendAsync({ - jsonrpc: '2.0', - method: 'evm_mine', - id: new Date().getTime() - }, (err, result) => { - if (err) { - return reject(err); - } - resolve(result); - }); - }); - }); -}; From 365c875ced771e1f7de85b609965e003a822bc96 Mon Sep 17 00:00:00 2001 From: Facundo Spagnuolo Date: Wed, 20 Sep 2017 10:54:26 -0300 Subject: [PATCH 33/34] Create detailed ERC20 interface --- contracts/token/DetailedERC20.sol | 15 +++++++++++++ test/DetailedERC20.js | 35 ++++++++++++++++++++++++++++++ test/helpers/DetailedERC20Mock.sol | 8 +++++++ 3 files changed, 58 insertions(+) create mode 100644 contracts/token/DetailedERC20.sol create mode 100644 test/DetailedERC20.js create mode 100644 test/helpers/DetailedERC20Mock.sol diff --git a/contracts/token/DetailedERC20.sol b/contracts/token/DetailedERC20.sol new file mode 100644 index 000000000..c61cde2e9 --- /dev/null +++ b/contracts/token/DetailedERC20.sol @@ -0,0 +1,15 @@ +pragma solidity ^0.4.11; + +import './ERC20.sol'; + +contract DetailedERC20 is ERC20 { + string public name; + string public symbol; + uint8 public decimals; + + function DetailedERC20(string _name, string _symbol, uint8 _decimals) { + name = _name; + symbol = _symbol; + decimals = _decimals; + } +} diff --git a/test/DetailedERC20.js b/test/DetailedERC20.js new file mode 100644 index 000000000..ce8f43920 --- /dev/null +++ b/test/DetailedERC20.js @@ -0,0 +1,35 @@ +const BigNumber = web3.BigNumber; + +require('chai') + .use(require('chai-as-promised')) + .use(require('chai-bignumber')(BigNumber)) + .should(); + +const DetailedERC20Mock = artifacts.require('./helpers/DetailedERC20Mock.sol'); + +contract('DetailedERC20', accounts => { + let detailedERC20 = null; + + const _name = "My Detailed ERC20"; + const _symbol = "MDT"; + const _decimals = 18; + + beforeEach(async function() { + detailedERC20 = await DetailedERC20Mock.new(_name, _symbol, _decimals); + }); + + it('has a name', async function () { + const name = await detailedERC20.name(); + name.should.be.equal(_name); + }); + + it('has a symbol', async function () { + const symbol = await detailedERC20.symbol(); + symbol.should.be.equal(_symbol); + }); + + it('has an amount of decimals', async function () { + const decimals = await detailedERC20.decimals(); + decimals.should.be.bignumber.equal(_decimals) + }); +}); diff --git a/test/helpers/DetailedERC20Mock.sol b/test/helpers/DetailedERC20Mock.sol new file mode 100644 index 000000000..5396ee4ef --- /dev/null +++ b/test/helpers/DetailedERC20Mock.sol @@ -0,0 +1,8 @@ +pragma solidity ^0.4.11; + +import '../../contracts/token/StandardToken.sol'; +import '../../contracts/token/DetailedERC20.sol'; + +contract DetailedERC20Mock is StandardToken, DetailedERC20 { + function DetailedERC20Mock(string _name, string _symbol, uint8 _decimals) DetailedERC20(_name, _symbol, _decimals) {} +} From 666a3a73e07dcd080af66a0092dd457ffb787589 Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Tue, 14 Nov 2017 12:50:43 -0500 Subject: [PATCH 34/34] remove unused 'day' test variable --- test/DayLimit.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/DayLimit.js b/test/DayLimit.js index 9d96383c5..a71ab6a05 100644 --- a/test/DayLimit.js +++ b/test/DayLimit.js @@ -6,7 +6,6 @@ import {increaseTimeTo, duration} from './helpers/increaseTime' var DayLimitMock = artifacts.require('./helpers/DayLimitMock.sol'); contract('DayLimit', function(accounts) { - const day = 60 * 60 * 24; let dayLimit; let initLimit = 10;