diff --git a/README.md b/README.md index dbdb39f8c..bf8353e83 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,8 @@ With Zeppelin, you can build distributed applications, protocols and organizatio - using common contract security patterns (See [Onward with Ethereum Smart Contract Security](https://medium.com/bitcorps-blog/onward-with-ethereum-smart-contract-security-97a827e47702#.y3kvdetbz)) - in the [Solidity language](http://solidity.readthedocs.io/en/develop/). +> NOTE: New to smart contract development? Check our [introductory guide](https://medium.com/zeppelin-blog/the-hitchhikers-guide-to-smart-contracts-in-ethereum-848f08001f05#.cox40d2ut). + ## Getting Started Zeppelin integrates with [Truffle](https://github.com/ConsenSys/truffle), an Ethereum development environment. Please install Truffle and initialize your project with `truffle init`. diff --git a/contracts/DayLimit.sol b/contracts/DayLimit.sol index d4f1cb521..41f9e4994 100644 --- a/contracts/DayLimit.sol +++ b/contracts/DayLimit.sol @@ -11,7 +11,7 @@ import './Shareable.sol'; * on a particular resource per calendar day. is multiowned to allow the limit to be altered. resource that method * uses is specified in the modifier. */ -contract DayLimit is Shareable { +contract DayLimit { // FIELDS uint public dailyLimit; @@ -38,13 +38,13 @@ contract DayLimit is Shareable { // METHODS - // (re)sets the daily limit. needs many of the owners to confirm. doesn't alter the amount already spent today. - function setDailyLimit(uint _newLimit) onlymanyowners(sha3(msg.data)) external { + // (re)sets the daily limit. doesn't alter the amount already spent today. + function _setDailyLimit(uint _newLimit) internal { dailyLimit = _newLimit; } - // resets the amount already spent today. needs many of the owners to confirm - function resetSpentToday() onlymanyowners(sha3(msg.data)) external { + // resets the amount already spent today. + function _resetSpentToday() internal { spentToday = 0; } @@ -53,14 +53,14 @@ contract DayLimit is Shareable { // checks to see if there is at least `_value` left from the daily limit today. if there is, subtracts it and // returns true. otherwise just returns false. - function underLimit(uint _value) internal onlyOwner returns (bool) { + function underLimit(uint _value) internal returns (bool) { // reset the spend limit if we're on a different day to last time. if (today() > lastDay) { spentToday = 0; lastDay = today(); } // check to see if there's enough left - if so, subtract and return true. - // overflow protection // dailyLimit check + // overflow protection // dailyLimit check if (spentToday + _value >= spentToday && spentToday + _value <= dailyLimit) { spentToday += _value; return true; diff --git a/contracts/MultisigWallet.sol b/contracts/MultisigWallet.sol index 1eb24c091..867de28ff 100644 --- a/contracts/MultisigWallet.sol +++ b/contracts/MultisigWallet.sol @@ -83,6 +83,14 @@ contract MultisigWallet is Multisig, Shareable, DayLimit { } } + function setDailyLimit(uint _newLimit) onlymanyowners(sha3(msg.data)) external { + _setDailyLimit(_newLimit); + } + + function resetSpentToday() onlymanyowners(sha3(msg.data)) external { + _resetSpentToday(); + } + // INTERNAL METHODS diff --git a/contracts/Shareable.sol b/contracts/Shareable.sol index b73f31eee..d416f80be 100644 --- a/contracts/Shareable.sol +++ b/contracts/Shareable.sol @@ -4,7 +4,7 @@ pragma solidity ^0.4.4; /* * Shareable * - * Based on https://github.com/ethereum/dapp-bin/blob/master/wallet/wallet.sol + * Based on https://github.com/ethereum/dapp-bin/blob/master/wallet/wallet.sol * * inheritable "property" contract that enables methods to be protected by requiring the acquiescence of either a single, or, crucially, each of a number of, designated owners. * @@ -98,7 +98,7 @@ contract Shareable { return address(owners[ownerIndex + 1]); } - function isOwner(address _addr) returns (bool) { + function isOwner(address _addr) constant returns (bool) { return ownerIndex[uint(_addr)] > 0; } @@ -162,4 +162,3 @@ contract Shareable { } } - diff --git a/contracts/examples/BadPushPayments.sol b/contracts/examples/BadPushPayments.sol index 2d85ffeec..7bc11c665 100644 --- a/contracts/examples/BadPushPayments.sol +++ b/contracts/examples/BadPushPayments.sol @@ -7,7 +7,7 @@ contract BadPushPayments { address highestBidder; uint highestBid; - function bid() { + function bid() payable { if (msg.value < highestBid) throw; if (highestBidder != 0) { diff --git a/contracts/examples/GoodPullPayments.sol b/contracts/examples/GoodPullPayments.sol index 1469eb57f..fc0cd986b 100644 --- a/contracts/examples/GoodPullPayments.sol +++ b/contracts/examples/GoodPullPayments.sol @@ -6,7 +6,7 @@ contract GoodPullPayments { uint highestBid; mapping(address => uint) refunds; - function bid() external { + function bid() external payable { if (msg.value < highestBid) throw; if (highestBidder != 0) { diff --git a/contracts/examples/PullPaymentBid.sol b/contracts/examples/PullPaymentBid.sol index ad4b612b5..8ba51422f 100644 --- a/contracts/examples/PullPaymentBid.sol +++ b/contracts/examples/PullPaymentBid.sol @@ -8,7 +8,7 @@ contract PullPaymentBid is PullPayment { address public highestBidder; uint public highestBid; - function bid() external { + function bid() external payable { if (msg.value <= highestBid) throw; if (highestBidder != 0) { diff --git a/contracts/examples/StoppableBid.sol b/contracts/examples/StoppableBid.sol index 61b9d43aa..53e1aebdc 100644 --- a/contracts/examples/StoppableBid.sol +++ b/contracts/examples/StoppableBid.sol @@ -9,7 +9,7 @@ contract StoppableBid is Stoppable, PullPayment { address public highestBidder; uint public highestBid; - function bid() external stopInEmergency { + function bid() external payable stopInEmergency { if (msg.value <= highestBid) throw; if (highestBidder != 0) { diff --git a/contracts/test-helpers/DayLimitMock.sol b/contracts/test-helpers/DayLimitMock.sol new file mode 100644 index 000000000..0ba5ca23f --- /dev/null +++ b/contracts/test-helpers/DayLimitMock.sol @@ -0,0 +1,23 @@ +pragma solidity ^0.4.4; +import "../DayLimit.sol"; + +contract DayLimitMock is DayLimit { + uint public totalSpending; + + function DayLimitMock(uint _value) DayLimit(_value) { + totalSpending = 0; + } + + function attemptSpend(uint _value) external limitedDaily(_value) { + totalSpending += _value; + } + + function setDailyLimit(uint _newLimit) external { + _setDailyLimit(_newLimit); + } + + function resetSpentToday() external { + _resetSpentToday(); + } + +} diff --git a/contracts/test-helpers/MultisigWalletMock.sol b/contracts/test-helpers/MultisigWalletMock.sol new file mode 100644 index 000000000..19adc558d --- /dev/null +++ b/contracts/test-helpers/MultisigWalletMock.sol @@ -0,0 +1,12 @@ +pragma solidity ^0.4.4; +import "../MultisigWallet.sol"; + +contract MultisigWalletMock is MultisigWallet { + uint public totalSpending; + + function MultisigWalletMock(address[] _owners, uint _required, uint _daylimit) + MultisigWallet(_owners, _required, _daylimit) payable { } + + function changeOwner(address _from, address _to) external { } + +} diff --git a/contracts/test-helpers/ShareableMock.sol b/contracts/test-helpers/ShareableMock.sol new file mode 100644 index 000000000..ebf06546e --- /dev/null +++ b/contracts/test-helpers/ShareableMock.sol @@ -0,0 +1,16 @@ +pragma solidity ^0.4.4; +import "../Shareable.sol"; + +contract ShareableMock is Shareable { + + uint public count = 0; + + function ShareableMock(address[] _owners, uint _required) Shareable(_owners, _required) { + + } + + function increaseCount(bytes32 action) onlymanyowners(action) { + count = count + 1; + } + +} diff --git a/test/DayLimit.js b/test/DayLimit.js new file mode 100644 index 000000000..559001f5d --- /dev/null +++ b/test/DayLimit.js @@ -0,0 +1,72 @@ +contract('DayLimit', function(accounts) { + + it('should construct with the passed daily limit', async function() { + let initLimit = 10; + let dayLimit = await DayLimitMock.new(initLimit); + let dailyLimit = await dayLimit.dailyLimit(); + assert.equal(initLimit, dailyLimit); + }); + + it('should be able to spend if daily limit is not reached', async function() { + let limit = 10; + let dayLimit = await DayLimitMock.new(limit); + + await dayLimit.attemptSpend(8); + let spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + + await dayLimit.attemptSpend(2); + spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 10); + }); + + it('should prevent spending if daily limit is reached', async function() { + let limit = 10; + let dayLimit = await DayLimitMock.new(limit); + + await dayLimit.attemptSpend(8); + let spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + + await dayLimit.attemptSpend(3); + spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + }); + + it('should allow spending if daily limit is reached and then set higher', async function() { + let limit = 10; + let dayLimit = await DayLimitMock.new(limit); + + await dayLimit.attemptSpend(8); + let spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + + await dayLimit.attemptSpend(3); + spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + + await dayLimit.setDailyLimit(15); + await dayLimit.attemptSpend(3); + spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 11); + }); + + it('should allow spending if daily limit is reached and then amount spent is reset', async function() { + let limit = 10; + let dayLimit = await DayLimitMock.new(limit); + + await dayLimit.attemptSpend(8); + let spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + + await dayLimit.attemptSpend(3); + spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 8); + + await dayLimit.resetSpentToday(15); + await dayLimit.attemptSpend(3); + spentToday = await dayLimit.spentToday(); + assert.equal(spentToday, 3); + }); + +}); diff --git a/test/MultisigWallet.js b/test/MultisigWallet.js new file mode 100644 index 000000000..32c1ec781 --- /dev/null +++ b/test/MultisigWallet.js @@ -0,0 +1,163 @@ +contract('MultisigWallet', function(accounts) { + //from https://gist.github.com/xavierlepretre/88682e871f4ad07be4534ae560692ee6 + web3.eth.getTransactionReceiptMined = function (txnHash, interval) { + var transactionReceiptAsync; + interval = interval ? interval : 500; + transactionReceiptAsync = function(txnHash, resolve, reject) { + try { + var receipt = web3.eth.getTransactionReceipt(txnHash); + if (receipt == null) { + setTimeout(function () { + transactionReceiptAsync(txnHash, resolve, reject); + }, interval); + } else { + resolve(receipt); + } + } catch(e) { + reject(e); + } + }; + + if (Array.isArray(txnHash)) { + var promises = []; + txnHash.forEach(function (oneTxHash) { + promises.push(web3.eth.getTransactionReceiptMined(oneTxHash, interval)); + }); + return Promise.all(promises); + } else { + return new Promise(function (resolve, reject) { + transactionReceiptAsync(txnHash, resolve, reject); + }); + } +}; + + + it('should send balance to passed address upon death', async function() { + //Give account[0] 20 ether + web3.eth.sendTransaction({from: web3.eth.coinbase, to: accounts[0], value: web3.toWei('20','ether')}, function(err, result) { + if(err) + console.log("ERROR:" + err); + }); + + let dailyLimit = 10; + let ownersRequired = 2; + + //Create MultisigWallet contract with 10 ether + let wallet = await MultisigWalletMock.new(accounts, ownersRequired, dailyLimit, {value: web3.toWei('10', 'ether')}); + + //Get balances of owner and wallet after wallet creation. + let ownerBalance = web3.eth.getBalance(accounts[0]); + let walletBalance = web3.eth.getBalance(wallet.address); + let hash = 1234; + + //Call kill function from two different owner accounts, satisfying owners required + await wallet.kill(accounts[0], {data: hash}); + let txnHash = await wallet.kill(accounts[0], {from: accounts[1], data: hash}); + + let receiptMined = await web3.eth.getTransactionReceiptMined(txnHash); + + //Get balances of owner and wallet after kill function is complete, compare with previous values + let newOwnerBalance = web3.eth.getBalance(accounts[0]); + let newWalletBalance = web3.eth.getBalance(wallet.address); + + assert.isTrue(newOwnerBalance > ownerBalance); + assert.isTrue(newWalletBalance < walletBalance); + }); + + it('should execute transaction if below daily limit', async function() { + //Give account[0] 20 ether + web3.eth.sendTransaction({from: web3.eth.coinbase, to: accounts[0], value: web3.toWei('20','ether')}, function(err, result) { + if(err) + console.log("ERROR:" + err); + }); + + let dailyLimit = 10; + let ownersRequired = 2; + + //Create MultisigWallet contract with 10 ether + let wallet = await MultisigWalletMock.new(accounts, ownersRequired, dailyLimit, {value: web3.toWei('10', 'ether')}); + + let accountBalance = web3.eth.getBalance(accounts[2]); + let hash = 1234; + + //Owner account0 commands wallet to send 9 wei to account2 + let txnHash = await wallet.execute(accounts[2], 9, hash); + let receiptMined = await web3.eth.getTransactionReceiptMined(txnHash); + + //Balance of account2 should have increased + let newAccountBalance = web3.eth.getBalance(accounts[2]); + assert.isTrue(newAccountBalance > accountBalance); + }); + + it('should prevent execution of transaction if above daily limit', async function() { + //Give account[0] 20 ether + web3.eth.sendTransaction({from: web3.eth.coinbase, to: accounts[0], value: web3.toWei('20','ether')}, function(err, result) { + if(err) + console.log("ERROR:" + err); + }); + + let dailyLimit = 10; + let ownersRequired = 2; + + //Create MultisigWallet contract with 10 ether + let wallet = await MultisigWalletMock.new(accounts, ownersRequired, dailyLimit, {value: web3.toWei('10', 'ether')}); + + let accountBalance = web3.eth.getBalance(accounts[2]); + let hash = 1234; + + //Owner account0 commands wallet to send 9 wei to account2 + let txnHash = await wallet.execute(accounts[2], 9, hash); + let receiptMined = await web3.eth.getTransactionReceiptMined(txnHash); + + //Balance of account2 should have increased + let newAccountBalance = web3.eth.getBalance(accounts[2]); + assert.isTrue(newAccountBalance > accountBalance); + + accountBalance = newAccountBalance; + hash = 4567; + + //Owner account0 commands wallet to send 2 more wei to account2, going over the daily limit of 10 + txnHash = await wallet.execute(accounts[2], 2, hash); + receiptMined = await web3.eth.getTransactionReceiptMined(txnHash); + + //Balance of account2 should not change + newAccountBalance = web3.eth.getBalance(accounts[2]); + assert.equal(newAccountBalance.toString(), accountBalance.toString()); + }); + + it('should execute transaction if above daily limit and enough owners approve', async function() { + //Give account[0] 20 ether + web3.eth.sendTransaction({from: web3.eth.coinbase, to: accounts[0], value: web3.toWei('20','ether')}, function(err, result) { + if(err) + console.log("ERROR:" + err); + }); + + let dailyLimit = 10; + let ownersRequired = 2; + + //Create MultisigWallet contract with 10 ether + let wallet = await MultisigWalletMock.new(accounts, ownersRequired, dailyLimit, {value: web3.toWei('10', 'ether')}); + + let accountBalance = web3.eth.getBalance(accounts[2]); + let hash = 1234; + + //Owner account0 commands wallet to send 11 wei to account2 + let txnHash = await wallet.execute(accounts[2], 11, hash); + let receiptMined = await web3.eth.getTransactionReceiptMined(txnHash); + + //Balance of account2 should not change + let newAccountBalance = web3.eth.getBalance(accounts[2]); + assert.equal(newAccountBalance.toString(), accountBalance.toString()); + + accountBalance = newAccountBalance; + + //Owner account1 commands wallet to send 11 wei to account2 + txnHash = await wallet.execute(accounts[2], 2, hash); + receiptMined = await web3.eth.getTransactionReceiptMined(txnHash); + + //Balance of account2 should change + newAccountBalance = web3.eth.getBalance(accounts[2]); + assert.isTrue(newAccountBalance > accountBalance); + }); + +}); diff --git a/test/Shareable.js b/test/Shareable.js new file mode 100644 index 000000000..fba268268 --- /dev/null +++ b/test/Shareable.js @@ -0,0 +1,101 @@ +contract('Shareable', function(accounts) { + + it('should construct with correct owners and number of sigs required', async function() { + let requiredSigs = 2; + let owners = accounts.slice(1,4); + let shareable = await ShareableMock.new(owners, requiredSigs); + + let required = await shareable.required(); + assert.equal(required, requiredSigs); + let owner = await shareable.getOwner(0); + assert.equal(owner, accounts[0]); + + for(let i = 0; i < accounts.length; i++) { + let owner = await shareable.getOwner(i); + let isowner = await shareable.isOwner(accounts[i]); + if(i <= owners.length) { + assert.equal(accounts[i], owner); + assert.isTrue(isowner); + } else { + assert.notEqual(accounts[i], owner); + assert.isFalse(isowner); + } + } + }); + + it('should only perform multisig function with enough sigs', async function() { + let requiredSigs = 3; + let owners = accounts.slice(1,4); + let shareable = await ShareableMock.new(owners, requiredSigs); + let hash = 1234; + + let initCount = await shareable.count(); + initCount = initCount.toString(); + + for(let i = 0; i < requiredSigs; i++) { + await shareable.increaseCount(hash, {from: accounts[i]}); + let count = await shareable.count(); + if(i == requiredSigs - 1) { + assert.equal(Number(initCount)+1, count.toString()); + } else { + assert.equal(initCount, count.toString()); + } + } + }); + + it('should require approval from different owners', async function() { + let requiredSigs = 2; + let owners = accounts.slice(1,4); + let shareable = await ShareableMock.new(owners, requiredSigs); + let hash = 1234; + + let initCount = await shareable.count(); + initCount = initCount.toString(); + + //Count shouldn't increase when the same owner calls repeatedly + for(let i = 0; i < 2; i++) { + await shareable.increaseCount(hash); + let count = await shareable.count(); + assert.equal(initCount, count.toString()); + } + }); + + it('should reset sig count after operation is approved', async function() { + let requiredSigs = 3; + let owners = accounts.slice(1,4); + let shareable = await ShareableMock.new(owners, requiredSigs); + let hash = 1234; + + let initCount = await shareable.count(); + + for(let i = 0; i < requiredSigs * 3; i++) { + await shareable.increaseCount(hash, {from: accounts[i % 4]}); + let count = await shareable.count(); + if((i%(requiredSigs)) == requiredSigs - 1) { + initCount = Number(initCount)+1; + assert.equal(initCount, count); + } else { + assert.equal(initCount.toString(), count); + } + } + }); + + it('should not perform multisig function after an owner revokes', async function() { + let requiredSigs = 3; + let owners = accounts.slice(1,4); + let shareable = await ShareableMock.new(owners, requiredSigs); + let hash = 1234; + + let initCount = await shareable.count(); + + for(let i = 0; i < requiredSigs; i++) { + if(i == 1) { + await shareable.revoke(hash, {from: accounts[i-1]}); + } + await shareable.increaseCount(hash, {from: accounts[i]}); + let count = await shareable.count(); + assert.equal(initCount.toString(), count); + } + }); + +});