Add executeConfirm support to perform multi-sig in 1 transaction

Author: bencxr

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() returns (uint) {
+        uint highestSequenceId = 0;
+        for (var 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 (var 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)