From 701f7b1e83aa53164528837e89986be1fa976dfe Mon Sep 17 00:00:00 2001 From: Manuel Araoz Date: Tue, 22 Nov 2016 20:12:51 -0800 Subject: [PATCH] fix multisig problems --- contracts/DayLimit.sol | 2 +- contracts/Multisig.sol | 2 +- contracts/MultisigWallet.sol | 93 +++++++++++++++++------------------- 3 files changed, 46 insertions(+), 51 deletions(-) diff --git a/contracts/DayLimit.sol b/contracts/DayLimit.sol index bd8c3b1cf..d4f1cb521 100644 --- a/contracts/DayLimit.sol +++ b/contracts/DayLimit.sol @@ -53,7 +53,7 @@ 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 onlyOwner returns (bool) { // reset the spend limit if we're on a different day to last time. if (today() > lastDay) { spentToday = 0; diff --git a/contracts/Multisig.sol b/contracts/Multisig.sol index 66dfa3e91..561bfe0b4 100644 --- a/contracts/Multisig.sol +++ b/contracts/Multisig.sol @@ -3,7 +3,7 @@ pragma solidity ^0.4.4; /* * Multisig - * interface contract for multisig proxy contracts; see below for docs. + * Interface contract for multisig proxy contracts; see below for docs. */ contract Multisig { // EVENTS diff --git a/contracts/MultisigWallet.sol b/contracts/MultisigWallet.sol index 0045727df..1eb24c091 100644 --- a/contracts/MultisigWallet.sol +++ b/contracts/MultisigWallet.sol @@ -1,34 +1,18 @@ pragma solidity ^0.4.4; -// interface contract for multisig proxy contracts; see below for docs. -contract multisig { - // EVENTS - - // logged events: - // Funds has arrived into the wallet (record how much). - event Deposit(address _from, uint value); - // Single transaction going out of the wallet (record who signed for it, how much, and to whom it's going). - event SingleTransact(address owner, uint value, address to, bytes data); - // Multi-sig transaction going out of the wallet (record who signed for it last, the operation hash, how much, and to whom it's going). - event MultiTransact(address owner, bytes32 operation, uint value, address to, bytes data); - // Confirmation still needed for a transaction. - event ConfirmationNeeded(bytes32 operation, address initiator, uint value, address to, bytes data); - - - // FUNCTIONS - - // TODO: document - function changeOwner(address _from, address _to) external; - function execute(address _to, uint _value, bytes _data) external returns (bytes32); - function confirm(bytes32 _h) returns (bool); -} - -// usage: -// bytes32 h = Wallet(w).from(oneOwner).execute(to, value, data); -// Wallet(w).from(anotherOwner).confirm(h); -contract Wallet is multisig, Shareable, daylimit { - +import "./Multisig.sol"; +import "./Shareable.sol"; +import "./DayLimit.sol"; + + +/* + * MultisigWallet + * usage: + * bytes32 h = Wallet(w).from(oneOwner).execute(to, value, data); + * Wallet(w).from(anotherOwner).confirm(h); + */ +contract MultisigWallet is Multisig, Shareable, DayLimit { // TYPES // Transaction structure to remember details of transaction lest it need be saved for a later call. @@ -38,13 +22,17 @@ contract Wallet is multisig, Shareable, daylimit { bytes data; } - // METHODS - // constructor - just pass on the owner array to the multiowned and + // CONSTRUCTOR + + // just pass on the owner array to the multiowned and // the limit to daylimit - function Wallet(address[] _owners, uint _required, uint _daylimit) - multiowned(_owners, _required) daylimit(_daylimit) { - } + function MultisigWallet(address[] _owners, uint _required, uint _daylimit) + Shareable(_owners, _required) + DayLimit(_daylimit) { } + + + // METHODS // kills the contract sending everything to `_to`. function kill(address _to) onlymanyowners(sha3(msg.data)) external { @@ -52,7 +40,7 @@ contract Wallet is multisig, Shareable, daylimit { } // gets called when no other function matches - function() { + function() payable { // just being sent some cash? if (msg.value > 0) Deposit(msg.sender, msg.value); @@ -62,46 +50,53 @@ contract Wallet is multisig, Shareable, daylimit { // If not, goes into multisig process. We provide a hash on return to allow the sender to provide // shortcuts for the other confirmations (allowing them to avoid replicating the _to, _value // and _data arguments). They still get the option of using them if they want, anyways. - function execute(address _to, uint _value, bytes _data) external onlyowner returns (bytes32 _r) { + function execute(address _to, uint _value, bytes _data) external onlyOwner returns (bytes32 _r) { // first, take the opportunity to check that we're under the daily limit. if (underLimit(_value)) { SingleTransact(msg.sender, _value, _to, _data); // yes - just execute the call. - _to.call.value(_value)(_data); + if (!_to.call.value(_value)(_data)) { + throw; + } return 0; } // determine our operation hash. _r = sha3(msg.data, block.number); - if (!confirm(_r) && m_txs[_r].to == 0) { - m_txs[_r].to = _to; - m_txs[_r].value = _value; - m_txs[_r].data = _data; + if (!confirm(_r) && txs[_r].to == 0) { + txs[_r].to = _to; + txs[_r].value = _value; + txs[_r].data = _data; ConfirmationNeeded(_r, msg.sender, _value, _to, _data); } } - // confirm a transaction through just the hash. we use the previous transactions map, m_txs, in order + // confirm a transaction through just the hash. we use the previous transactions map, txs, in order // to determine the body of the transaction from the hash provided. function confirm(bytes32 _h) onlymanyowners(_h) returns (bool) { - if (m_txs[_h].to != 0) { - m_txs[_h].to.call.value(m_txs[_h].value)(m_txs[_h].data); - MultiTransact(msg.sender, _h, m_txs[_h].value, m_txs[_h].to, m_txs[_h].data); - delete m_txs[_h]; + if (txs[_h].to != 0) { + if (!txs[_h].to.call.value(txs[_h].value)(txs[_h].data)) { + throw; + } + MultiTransact(msg.sender, _h, txs[_h].value, txs[_h].to, txs[_h].data); + delete txs[_h]; return true; } } + // INTERNAL METHODS function clearPending() internal { - uint length = m_pendingIndex.length; - for (uint i = 0; i < length; ++i) - delete m_txs[m_pendingIndex[i]]; + uint length = pendingsIndex.length; + for (uint i = 0; i < length; ++i) { + delete txs[pendingsIndex[i]]; + } super.clearPending(); } + // FIELDS // pending transactions we have at present. - mapping (bytes32 => Transaction) m_txs; + mapping (bytes32 => Transaction) txs; }