From 2085feb54797085f8aef2b93a36589e0eb8a433f Mon Sep 17 00:00:00 2001 From: Ben Chan Date: Sun, 10 Jul 2016 18:01:38 -0700 Subject: [PATCH] Add executeConfirm support to perform multi-sig in 1 transaction --- wallet/wallet.sol | 139 ++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 123 insertions(+), 16 deletions(-) diff --git a/wallet/wallet.sol b/wallet/wallet.sol index c522a9b..fd6f414 100644 --- a/wallet/wallet.sol +++ b/wallet/wallet.sol @@ -138,33 +138,100 @@ contract multiowned { uint ownerIndexBit = 2**ownerIndex; return !(pending.ownersDone & ownerIndexBit == 0); } - - // INTERNAL METHODS + // Gets the next available sequence ID for signing when using confirmAndCheckUsingECRecover + function getNextSequenceId() constant returns (uint) { + uint highestSequenceId = 0; + for (uint i = 0; i < c_maxSequenceIdWindowSize; i++) { + if (m_sequenceIdsUsed[i] > highestSequenceId) { + highestSequenceId = m_sequenceIdsUsed[i]; + } + } + return highestSequenceId + 1; + } + + // INTERNAL METHODS + // Called within the onlymanyowners modifier. + // Records a confirmation by msg.sender and returns true if the operation has the required number of confirmations function confirmAndCheck(bytes32 _operation) internal returns (bool) { - // determine what index the present sender is: - uint ownerIndex = m_ownerIndex[uint(msg.sender)]; - // make sure they're an owner - if (ownerIndex == 0) return; + return confirmAndCheckOperationForOwner(_operation, msg.sender); + } + + // This operation will look for 2 confirmations + // The first confirmation will be verified using ecrecover + // The second confirmation will be verified using msg.sender + function confirmWithSenderAndECRecover(bytes32 _operation, uint _sequenceId, bytes _signature) internal returns (bool) { + // We expect confirmAndCheckUsingECRecover to run and return false, but mark the pending operation as having 1 confirm + return confirmAndCheckUsingECRecover(_operation, _sequenceId, _signature) || confirmAndCheck(_operation); + } + + // Gets an owner using ecrecover, records their confirmation and + // returns true if the operation has the required number of confirmations + function confirmAndCheckUsingECRecover(bytes32 _operation, uint _sequenceId, bytes _signature) internal returns (bool) { + // Verify that the sequence id has not been used before + // Create mapping of the sequence ids being used + uint lowestValueIndex = 0; + for (uint i = 0; i < c_maxSequenceIdWindowSize; i++) { + if (m_sequenceIdsUsed[i] == _sequenceId) { + // This sequence ID has been used before. Disallow! + throw; + } + if (m_sequenceIdsUsed[i] < m_sequenceIdsUsed[lowestValueIndex]) { + lowestValueIndex = i; + } + } + if (_sequenceId < m_sequenceIdsUsed[lowestValueIndex]) { + // The sequence ID being used is lower than the lowest value in the window + // so we cannot accept it as it may have been used before + throw; + } + m_sequenceIdsUsed[lowestValueIndex] = _sequenceId; + + // We need to unpack the signature, which is given as an array of 65 bytes (from eth.sign) + bytes32 r; + bytes32 s; + uint8 v; + + if (_signature.length != 65) + throw; + + assembly { + r := mload(add(_signature, 32)) + s := mload(add(_signature, 64)) + v := and(mload(add(_signature, 65)), 255) + } + + var ownerAddress = ecrecover(_operation, v, r, s); + return confirmAndCheckOperationForOwner(_operation, ownerAddress); + } + + // Records confirmations for an operation by the given owner and + // returns true if the operation has the required number of confirmations + function confirmAndCheckOperationForOwner(bytes32 _operation, address _owner) private returns (bool) { + // Determine what index the present sender is + uint ownerIndex = m_ownerIndex[uint(_owner)]; + // Make sure they're an owner + if (ownerIndex == 0) return false; var pending = m_pending[_operation]; - // if we're not yet working on this operation, switch over and reset the confirmation status. + // If we're not yet working on this operation, add it if (pending.yetNeeded == 0) { - // reset count of confirmations needed. + // Reset count of confirmations needed. pending.yetNeeded = m_required; - // reset which owners have confirmed (none) - set our bitmap to 0. + // Reset which owners have confirmed (none) - set our bitmap to 0. pending.ownersDone = 0; pending.index = m_pendingIndex.length++; m_pendingIndex[pending.index] = _operation; } - // determine the bit to set for this owner. + + // Determine the bit to set for this owner on the pending state for the operation uint ownerIndexBit = 2**ownerIndex; - // make sure we (the message sender) haven't confirmed this operation previously. + // Make sure the owner has not confirmed this operation previously. if (pending.ownersDone & ownerIndexBit == 0) { - Confirmation(msg.sender, _operation); - // ok - check if count is enough to go ahead. + Confirmation(_owner, _operation); + // Check if this confirmation puts us at the required number of needed confirmations. if (pending.yetNeeded <= 1) { - // enough confirmations: reset and run interior. + // Enough confirmations: mark operation as passed and return true to continue execution delete m_pendingIndex[m_pending[_operation].index]; delete m_pending[_operation]; return true; @@ -176,6 +243,8 @@ contract multiowned { pending.ownersDone |= ownerIndexBit; } } + + return false; } function reorganizeOwners() private { @@ -216,6 +285,15 @@ contract multiowned { // the ongoing operations. mapping(bytes32 => PendingState) m_pending; bytes32[] m_pendingIndex; + + // When we use ecrecover to verify signatures (in addition to msg.sender), an array window of sequence ids is used. + // This prevents from replay attacks by the first signer. + // + // Sequence IDs may not be repeated and should start from 1 onwards. Stores the last 10 largest sequence ids in a window + // New sequence ids being added must replace the smallest of those numbers and must be larger than the smallest value stored. + // This allows some degree of flexibility for submission of multiple transactions in a block. + uint constant c_maxSequenceIdWindowSize = 10; + uint[10] m_sequenceIdsUsed; } // inheritable "property" contract that enables methods to be protected by placing a linear limit (specifiable) @@ -363,9 +441,38 @@ contract Wallet is multisig, multiowned, daylimit { return true; } } - + + // Execute and confirm a transaction with 2 signatures - one using the msg.sender and another using ecrecover + // The signature is a signed form (using eth.sign) of tightly packed to, value, data, expiretime and sequenceId + // Sequence IDs are numbers starting from 1. They used to prevent replay attacks and may not be repeated. + function executeAndConfirm(address _to, uint _value, bytes _data, uint _expireTime, uint _sequenceId, bytes _signature) + external onlyowner + returns (bytes32) + { + if (_expireTime < block.timestamp) { + throw; + } + + // The unique hash is the combination of all arguments except the signature + var operationHash = sha3(_to, _value, _data, _expireTime, _sequenceId); + + // Confirm the operation + if (confirmWithSenderAndECRecover(operationHash, _sequenceId, _signature)) { + if (!(_to.call.value(_value)(_data))) { + throw; + } + MultiTransact(msg.sender, operationHash, _value, _to, _data); + return 0; + } + + m_txs[operationHash].to = _to; + m_txs[operationHash].value = _value; + m_txs[operationHash].data = _data; + ConfirmationNeeded(operationHash, msg.sender, _value, _to, _data); + return operationHash; + } + // INTERNAL METHODS - function clearPending() internal { uint length = m_pendingIndex.length; for (uint i = 0; i < length; ++i)