Merge pull request #2138 from 0xProject/feat/3.0/update-apowner

Update AssetProxyOwner

contracts/multisig/contracts/src/AssetProxyOwner.sol

@@ -16,93 +16,205 @@
 
 */
 
-pragma solidity 0.4.24;
+pragma solidity ^0.5.9;
+pragma experimental ABIEncoderV2;
 
 import "./MultiSigWalletWithTimeLock.sol";
 import "@0x/contracts-utils/contracts/src/LibBytes.sol";
+import "@0x/contracts-utils/contracts/src/LibSafeMath.sol";
 
 
 contract AssetProxyOwner is
     MultiSigWalletWithTimeLock
 {
     using LibBytes for bytes;
+    using LibSafeMath for uint256;
 
-    event AssetProxyRegistration(address assetProxyContract, bool isRegistered);
+    struct TimeLock {
-
+        bool hasCustomTimeLock;
-    // Mapping of AssetProxy contract address =>
+        uint128 secondsTimeLocked;
-    // if this contract is allowed to call the AssetProxy's `removeAuthorizedAddressAtIndex` method without a time lock.
-    mapping (address => bool) public isAssetProxyRegistered;
-
-    bytes4 constant internal REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR = bytes4(keccak256("removeAuthorizedAddressAtIndex(address,uint256)"));
-
-    /// @dev Function will revert if the transaction does not call `removeAuthorizedAddressAtIndex`
-    ///      on an approved AssetProxy contract.
-    modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) {
-        Transaction storage txn = transactions[transactionId];
-        require(
-            isAssetProxyRegistered[txn.destination],
-            "UNREGISTERED_ASSET_PROXY"
-        );
-        require(
-            txn.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR,
-            "INVALID_FUNCTION_SELECTOR"
-        );
-        _;
     }
 
-    /// @dev Contract constructor sets initial owners, required number of confirmations,
+    event FunctionCallTimeLockRegistration(
-    ///      time lock, and list of AssetProxy addresses.
+        bytes4 functionSelector,
+        address destination,
+        bool hasCustomTimeLock,
+        uint128 newSecondsTimeLocked
+    );
+
+    // Function selector => destination => seconds timelocked
+    mapping (bytes4 => mapping (address => TimeLock)) public functionCallTimeLocks;
+
+    /// @dev Contract constructor sets initial owners, required number of confirmations, and default time lock
+    ///      It will also register unique timelocks for each passed in function selector / destination combo. 
+    /// @param _functionSelectors Array of function selectors for registered functions.
+    /// @param _destinations Array of destinations for registered function calls.
+    /// @param _functionCallTimeLockSeconds Array of seconds that each registered function call will be timelocked.
     /// @param _owners List of initial owners.
-    /// @param _assetProxyContracts Array of AssetProxy contract addresses.
     /// @param _required Number of required confirmations.
-    /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
+    /// @param _defaultSecondsTimeLocked Default duration in seconds needed after a transaction is confirmed to become executable.
     constructor (
+        bytes4[] memory _functionSelectors,
+        address[] memory _destinations,
+        uint128[] memory _functionCallTimeLockSeconds,
         address[] memory _owners,
-        address[] memory _assetProxyContracts,
         uint256 _required,
-        uint256 _secondsTimeLocked
+        uint256 _defaultSecondsTimeLocked
     )
         public
-        MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked)
+        MultiSigWalletWithTimeLock(
+            _owners,
+            _required,
+            _defaultSecondsTimeLocked
+        )
     {
-        for (uint256 i = 0; i < _assetProxyContracts.length; i++) {
+        uint256 length = _functionSelectors.length;
-            address assetProxy = _assetProxyContracts[i];
         require(
-                assetProxy != address(0),
+            length == _destinations.length && length == _functionCallTimeLockSeconds.length,
-                "INVALID_ASSET_PROXY"
+            "EQUAL_LENGTHS_REQUIRED"
+        );
+
+        // Register function timelocks
+        for (uint256 i = 0; i != length; i++) {
+            _registerFunctionCall(
+                true,  // all functions registered in constructor are assumed to have a custom timelock
+                _functionSelectors[i],
+                _destinations[i],
+                _functionCallTimeLockSeconds[i]
             );
-            isAssetProxyRegistered[assetProxy] = true;
         }
     }
 
-    /// @dev Registers or deregisters an AssetProxy to be able to execute
+    /// @dev Registers a custom timelock to a specific function selector / destination combo
-    ///      `removeAuthorizedAddressAtIndex` without a timelock.
+    /// @param hasCustomTimeLock True if timelock is custom.
-    /// @param assetProxyContract Address of AssetProxy contract.
+    /// @param functionSelector 4 byte selector of registered function.
-    /// @param isRegistered Status of approval for AssetProxy contract.
+    /// @param destination Address of destination where function will be called.
-    function registerAssetProxy(address assetProxyContract, bool isRegistered)
+    /// @param newSecondsTimeLocked Duration in seconds needed after a transaction is confirmed to become executable.
-        public
+    function registerFunctionCall(
+        bool hasCustomTimeLock,
+        bytes4 functionSelector,
+        address destination,
+        uint128 newSecondsTimeLocked
+    )
+        external
         onlyWallet
-        notNull(assetProxyContract)
     {
-        isAssetProxyRegistered[assetProxyContract] = isRegistered;
+        _registerFunctionCall(
-        emit AssetProxyRegistration(assetProxyContract, isRegistered);
+            hasCustomTimeLock,
+            functionSelector,
+            destination,
+            newSecondsTimeLocked
+        );
     }
 
-    /// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock.
+    /// @dev Allows anyone to execute a confirmed transaction.
+    ///      Transactions *must* encode the values with the signature "bytes[] data, address[] destinations, uint256[] values"
+    ///      The `destination` and `value` fields of the transaction in storage are ignored.
+    ///      All function calls must be successful or the entire call will revert.
     /// @param transactionId Transaction ID.
-    function executeRemoveAuthorizedAddressAtIndex(uint256 transactionId)
+    function executeTransaction(uint256 transactionId)
         public
         notExecuted(transactionId)
         fullyConfirmed(transactionId)
-        validRemoveAuthorizedAddressAtIndexTx(transactionId)
     {
-        Transaction storage txn = transactions[transactionId];
+        Transaction storage transaction = transactions[transactionId];
-        txn.executed = true;
+        transaction.executed = true;
-        if (_externalCall(txn.destination, txn.value, txn.data.length, txn.data)) {
+
+        // Decode batch transaction data from transaction.data
+        // `destination` and `value` fields of transaction are ignored
+        // Note that `destination` must be non-0, or the transaction cannot be submitted
+        // solhint-disable
+        (
+            bytes[] memory data,
+            address[] memory destinations,
+            uint256[] memory values
+        ) = abi.decode(
+            transaction.data,
+            (bytes[], address[], uint256[])
+        );
+        // solhint-enable
+
+        // Ensure lengths of array properties are equal
+        uint256 length = data.length;
+        require(
+            length == destinations.length && length == values.length,
+            "EQUAL_LENGTHS_REQUIRED"
+        );
+
+        uint256 transactionConfirmationTime = confirmationTimes[transactionId];
+        for (uint i = 0; i != length; i++) {
+            // Ensure that each function call is past its timelock
+            _assertValidFunctionCall(
+                transactionConfirmationTime,
+                data[i],
+                destinations[i]
+            );
+            // Call each function
+            // solhint-disable-next-line avoid-call-value
+            (bool didSucceed,) = destinations[i].call.value(values[i])(data[i]);
+            // Ensure that function call was successful
+            require(
+                didSucceed,
+                "FAILED_EXECUTION"
+            );
+        }
         emit Execution(transactionId);
+    }
+
+    /// @dev Registers a custom timelock to a specific function selector / destination combo
+    /// @param hasCustomTimeLock True if timelock is custom.
+    /// @param functionSelector 4 byte selector of registered function.
+    /// @param destination Address of destination where function will be called.
+    /// @param newSecondsTimeLocked Duration in seconds needed after a transaction is confirmed to become executable.
+    function _registerFunctionCall(
+        bool hasCustomTimeLock,
+        bytes4 functionSelector,
+        address destination,
+        uint128 newSecondsTimeLocked    
+    )
+        internal
+    {
+        // Clear the previous secondsTimeLocked if custom timelock not used
+        uint128 _secondsTimeLocked = hasCustomTimeLock ? newSecondsTimeLocked : 0;
+        TimeLock memory timeLock = TimeLock({
+            hasCustomTimeLock: hasCustomTimeLock,
+            secondsTimeLocked: _secondsTimeLocked
+        });
+        functionCallTimeLocks[functionSelector][destination] = timeLock;
+        emit FunctionCallTimeLockRegistration(
+            functionSelector,
+            destination,
+            hasCustomTimeLock,
+            _secondsTimeLocked
+        );
+    }
+
+    /// @dev Ensures that the function call has past its timelock.
+    /// @param transactionConfirmationTime Timestamp at which transaction was fully confirmed.
+    /// @param data Function calldata.
+    /// @param destination Address to call function on.
+    function _assertValidFunctionCall(
+        uint256 transactionConfirmationTime,
+        bytes memory data,
+        address destination
+    )
+        internal
+        view
+    {
+        bytes4 functionSelector = data.readBytes4(0);
+        TimeLock memory timeLock = functionCallTimeLocks[functionSelector][destination];
+        // solhint-disable not-rely-on-time
+        if (timeLock.hasCustomTimeLock) {
+            require(
+                block.timestamp >= transactionConfirmationTime.safeAdd(timeLock.secondsTimeLocked),
+                "CUSTOM_TIME_LOCK_INCOMPLETE"
+            );
         } else {
-            emit ExecutionFailure(transactionId);
+            require(
-            txn.executed = false;
+                block.timestamp >= transactionConfirmationTime.safeAdd(secondsTimeLocked),
-        }
+                "DEFAULT_TIME_LOCK_INCOMPLETE"
+            );
+        }
+        // solhint-enable not-rely-on-time
     }
 }

contracts/multisig/contracts/src/MultiSigWallet.sol

@@ -1,5 +1,5 @@
 // solhint-disable
-pragma solidity ^0.4.15;
+pragma solidity ^0.5.9;
 
 
 /// @title Multisignature wallet - Allows multiple parties to agree on transactions before execution.
@@ -9,34 +9,34 @@ contract MultiSigWallet {
     /*
      *  Events
      */
-    event Confirmation(address indexed sender, uint indexed transactionId);
+    event Confirmation(address indexed sender, uint256 indexed transactionId);
-    event Revocation(address indexed sender, uint indexed transactionId);
+    event Revocation(address indexed sender, uint256 indexed transactionId);
-    event Submission(uint indexed transactionId);
+    event Submission(uint256 indexed transactionId);
-    event Execution(uint indexed transactionId);
+    event Execution(uint256 indexed transactionId);
-    event ExecutionFailure(uint indexed transactionId);
+    event ExecutionFailure(uint256 indexed transactionId);
-    event Deposit(address indexed sender, uint value);
+    event Deposit(address indexed sender, uint256 value);
     event OwnerAddition(address indexed owner);
     event OwnerRemoval(address indexed owner);
-    event RequirementChange(uint required);
+    event RequirementChange(uint256 required);
 
     /*
      *  Constants
      */
-    uint constant public MAX_OWNER_COUNT = 50;
+    uint256 constant public MAX_OWNER_COUNT = 50;
 
     /*
      *  Storage
      */
-    mapping (uint => Transaction) public transactions;
+    mapping (uint256 => Transaction) public transactions;
-    mapping (uint => mapping (address => bool)) public confirmations;
+    mapping (uint256 => mapping (address => bool)) public confirmations;
     mapping (address => bool) public isOwner;
     address[] public owners;
-    uint public required;
+    uint256 public required;
-    uint public transactionCount;
+    uint256 public transactionCount;
 
     struct Transaction {
         address destination;
-        uint value;
+        uint256 value;
         bytes data;
         bool executed;
     }
@@ -45,59 +45,88 @@ contract MultiSigWallet {
      *  Modifiers
      */
     modifier onlyWallet() {
-        require(msg.sender == address(this));
+        require(
+            msg.sender == address(this),
+            "ONLY_CALLABLE_BY_WALLET"
+        );
         _;
     }
 
     modifier ownerDoesNotExist(address owner) {
-        require(!isOwner[owner]);
+        require(
+            !isOwner[owner],
+            "OWNER_EXISTS"
+        );
         _;
     }
 
     modifier ownerExists(address owner) {
-        require(isOwner[owner]);
+        require(
+            isOwner[owner],
+            "OWNER_DOESNT_EXIST"
+        );
         _;
     }
 
-    modifier transactionExists(uint transactionId) {
+    modifier transactionExists(uint256 transactionId) {
-        require(transactions[transactionId].destination != 0);
+        require(
+            transactions[transactionId].destination != address(0),
+            "TX_DOESNT_EXIST"
+        );
         _;
     }
 
-    modifier confirmed(uint transactionId, address owner) {
+    modifier confirmed(uint256 transactionId, address owner) {
-        require(confirmations[transactionId][owner]);
+        require(
+            confirmations[transactionId][owner],
+            "TX_NOT_CONFIRMED"
+        );
         _;
     }
 
-    modifier notConfirmed(uint transactionId, address owner) {
+    modifier notConfirmed(uint256 transactionId, address owner) {
-        require(!confirmations[transactionId][owner]);
+        require(
+            !confirmations[transactionId][owner],
+            "TX_ALREADY_CONFIRMED"
+        );
         _;
     }
 
-    modifier notExecuted(uint transactionId) {
+    modifier notExecuted(uint256 transactionId) {
-        require(!transactions[transactionId].executed);
+        require(
+            !transactions[transactionId].executed,
+            "TX_ALREADY_EXECUTED"
+        );
         _;
     }
 
     modifier notNull(address _address) {
-        require(_address != 0);
+        require(
+            _address != address(0),
+            "NULL_ADDRESS"
+        );
         _;
     }
 
-    modifier validRequirement(uint ownerCount, uint _required) {
+    modifier validRequirement(uint256 ownerCount, uint256 _required) {
-        require(ownerCount <= MAX_OWNER_COUNT
+        require(
+            ownerCount <= MAX_OWNER_COUNT
             && _required <= ownerCount
             && _required != 0
-            && ownerCount != 0);
+            && ownerCount != 0,
+            "INVALID_REQUIREMENTS"
+        );
         _;
     }
 
     /// @dev Fallback function allows to deposit ether.
     function()
+        external
         payable
     {
-        if (msg.value > 0)
+        if (msg.value > 0) {
-            Deposit(msg.sender, msg.value);
+            emit Deposit(msg.sender, msg.value);
+        }
     }
 
     /*
@@ -106,12 +135,18 @@ contract MultiSigWallet {
     /// @dev Contract constructor sets initial owners and required number of confirmations.
     /// @param _owners List of initial owners.
     /// @param _required Number of required confirmations.
-    function MultiSigWallet(address[] _owners, uint _required)
+    constructor(
+        address[] memory _owners,
+        uint256 _required
+    )
         public
         validRequirement(_owners.length, _required)
     {
-        for (uint i=0; i<_owners.length; i++) {
+        for (uint256 i = 0; i < _owners.length; i++) {
-            require(!isOwner[_owners[i]] && _owners[i] != 0);
+            require(
+                !isOwner[_owners[i]] && _owners[i] != address(0),
+                "DUPLICATE_OR_NULL_OWNER"
+            );
             isOwner[_owners[i]] = true;
         }
         owners = _owners;
@@ -129,7 +164,7 @@ contract MultiSigWallet {
     {
         isOwner[owner] = true;
         owners.push(owner);
-        OwnerAddition(owner);
+        emit OwnerAddition(owner);
     }
 
     /// @dev Allows to remove an owner. Transaction has to be sent by wallet.
@@ -140,15 +175,17 @@ contract MultiSigWallet {
         ownerExists(owner)
     {
         isOwner[owner] = false;
-        for (uint i=0; i<owners.length - 1; i++)
+        for (uint256 i = 0; i < owners.length - 1; i++) {
             if (owners[i] == owner) {
                 owners[i] = owners[owners.length - 1];
                 break;
             }
+        }
         owners.length -= 1;
-        if (required > owners.length)
+        if (required > owners.length) {
             changeRequirement(owners.length);
-        OwnerRemoval(owner);
+        }
+        emit OwnerRemoval(owner);
     }
 
     /// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet.
@@ -160,26 +197,27 @@ contract MultiSigWallet {
         ownerExists(owner)
         ownerDoesNotExist(newOwner)
     {
-        for (uint i=0; i<owners.length; i++)
+        for (uint256 i = 0; i < owners.length; i++) {
             if (owners[i] == owner) {
                 owners[i] = newOwner;
                 break;
             }
+        }
         isOwner[owner] = false;
         isOwner[newOwner] = true;
-        OwnerRemoval(owner);
+        emit OwnerRemoval(owner);
-        OwnerAddition(newOwner);
+        emit OwnerAddition(newOwner);
     }
 
     /// @dev Allows to change the number of required confirmations. Transaction has to be sent by wallet.
     /// @param _required Number of required confirmations.
-    function changeRequirement(uint _required)
+    function changeRequirement(uint256 _required)
         public
         onlyWallet
         validRequirement(owners.length, _required)
     {
         required = _required;
-        RequirementChange(_required);
+        emit RequirementChange(_required);
     }
 
     /// @dev Allows an owner to submit and confirm a transaction.
@@ -187,9 +225,9 @@ contract MultiSigWallet {
     /// @param value Transaction ether value.
     /// @param data Transaction data payload.
     /// @return Returns transaction ID.
-    function submitTransaction(address destination, uint value, bytes data)
+    function submitTransaction(address destination, uint256 value, bytes memory data)
         public
-        returns (uint transactionId)
+        returns (uint256 transactionId)
     {
         transactionId = _addTransaction(destination, value, data);
         confirmTransaction(transactionId);
@@ -197,32 +235,32 @@ contract MultiSigWallet {
 
     /// @dev Allows an owner to confirm a transaction.
     /// @param transactionId Transaction ID.
-    function confirmTransaction(uint transactionId)
+    function confirmTransaction(uint256 transactionId)
         public
         ownerExists(msg.sender)
         transactionExists(transactionId)
         notConfirmed(transactionId, msg.sender)
     {
         confirmations[transactionId][msg.sender] = true;
-        Confirmation(msg.sender, transactionId);
+        emit Confirmation(msg.sender, transactionId);
         executeTransaction(transactionId);
     }
 
     /// @dev Allows an owner to revoke a confirmation for a transaction.
     /// @param transactionId Transaction ID.
-    function revokeConfirmation(uint transactionId)
+    function revokeConfirmation(uint256 transactionId)
         public
         ownerExists(msg.sender)
         confirmed(transactionId, msg.sender)
         notExecuted(transactionId)
     {
         confirmations[transactionId][msg.sender] = false;
-        Revocation(msg.sender, transactionId);
+        emit Revocation(msg.sender, transactionId);
     }
 
     /// @dev Allows anyone to execute a confirmed transaction.
     /// @param transactionId Transaction ID.
-    function executeTransaction(uint transactionId)
+    function executeTransaction(uint256 transactionId)
         public
         ownerExists(msg.sender)
         confirmed(transactionId, msg.sender)
@@ -231,10 +269,17 @@ contract MultiSigWallet {
         if (isConfirmed(transactionId)) {
             Transaction storage txn = transactions[transactionId];
             txn.executed = true;
-            if (_externalCall(txn.destination, txn.value, txn.data.length, txn.data))
+            if (
-                Execution(transactionId);
+                _externalCall(
-            else {
+                    txn.destination,
-                ExecutionFailure(transactionId);
+                    txn.value,
+                    txn.data.length,
+                    txn.data
+                )
+            ) {
+                emit Execution(transactionId);
+            } else {
+                emit ExecutionFailure(transactionId);
                 txn.executed = false;
             }
         }
@@ -242,7 +287,15 @@ contract MultiSigWallet {
 
     // call has been separated into its own function in order to take advantage
     // of the Solidity's code generator to produce a loop that copies tx.data into memory.
-    function _externalCall(address destination, uint value, uint dataLength, bytes data) internal returns (bool) {
+    function _externalCall(
+        address destination,
+        uint256 value,
+        uint256 dataLength,
+        bytes memory data
+    )
+        internal
+        returns (bool)
+    {
         bool result;
         assembly {
             let x := mload(0x40)   // "Allocate" memory for output (0x40 is where "free memory" pointer is stored by convention)
@@ -265,19 +318,21 @@ contract MultiSigWallet {
     /// @dev Returns the confirmation status of a transaction.
     /// @param transactionId Transaction ID.
     /// @return Confirmation status.
-    function isConfirmed(uint transactionId)
+    function isConfirmed(uint256 transactionId)
         public
-        constant
+        view
         returns (bool)
     {
-        uint count = 0;
+        uint256 count = 0;
-        for (uint i=0; i<owners.length; i++) {
+        for (uint256 i = 0; i < owners.length; i++) {
-            if (confirmations[transactionId][owners[i]])
+            if (confirmations[transactionId][owners[i]]) {
                 count += 1;
-            if (count == required)
+            }
+            if (count == required) {
                 return true;
             }
         }
+    }
 
     /*
      * Internal functions
@@ -287,10 +342,14 @@ contract MultiSigWallet {
     /// @param value Transaction ether value.
     /// @param data Transaction data payload.
     /// @return Returns transaction ID.
-    function _addTransaction(address destination, uint value, bytes data)
+    function _addTransaction(
+        address destination,
+        uint256 value,
+        bytes memory data
+    )
         internal
         notNull(destination)
-        returns (uint transactionId)
+        returns (uint256 transactionId)
     {
         transactionId = transactionCount;
         transactions[transactionId] = Transaction({
@@ -300,7 +359,7 @@ contract MultiSigWallet {
             executed: false
         });
         transactionCount += 1;
-        Submission(transactionId);
+        emit Submission(transactionId);
     }
 
     /*
@@ -309,15 +368,17 @@ contract MultiSigWallet {
     /// @dev Returns number of confirmations of a transaction.
     /// @param transactionId Transaction ID.
     /// @return Number of confirmations.
-    function getConfirmationCount(uint transactionId)
+    function getConfirmationCount(uint256 transactionId)
         public
-        constant
+        view
-        returns (uint count)
+        returns (uint256 count)
     {
-        for (uint i=0; i<owners.length; i++)
+        for (uint256 i = 0; i < owners.length; i++) {
-            if (confirmations[transactionId][owners[i]])
+            if (confirmations[transactionId][owners[i]]) {
                 count += 1;
             }
+        }
+    }
 
     /// @dev Returns total number of transactions after filers are applied.
     /// @param pending Include pending transactions.
@@ -325,21 +386,22 @@ contract MultiSigWallet {
     /// @return Total number of transactions after filters are applied.
     function getTransactionCount(bool pending, bool executed)
         public
-        constant
+        view
-        returns (uint count)
+        returns (uint256 count)
     {
-        for (uint i=0; i<transactionCount; i++)
+        for (uint256 i = 0; i < transactionCount; i++) {
-            if (   pending && !transactions[i].executed
+            if (pending && !transactions[i].executed || executed && transactions[i].executed) {
-                || executed && transactions[i].executed)
                 count += 1;
             }
+        }
+    }
 
     /// @dev Returns list of owners.
     /// @return List of owner addresses.
     function getOwners()
         public
-        constant
+        view
-        returns (address[])
+        returns (address[] memory)
     {
         return owners;
     }
@@ -347,23 +409,25 @@ contract MultiSigWallet {
     /// @dev Returns array with owner addresses, which confirmed transaction.
     /// @param transactionId Transaction ID.
     /// @return Returns array of owner addresses.
-    function getConfirmations(uint transactionId)
+    function getConfirmations(uint256 transactionId)
         public
-        constant
+        view
-        returns (address[] _confirmations)
+        returns (address[] memory _confirmations)
     {
         address[] memory confirmationsTemp = new address[](owners.length);
-        uint count = 0;
+        uint256 count = 0;
-        uint i;
+        uint256 i;
-        for (i=0; i<owners.length; i++)
+        for (i = 0; i < owners.length; i++) {
             if (confirmations[transactionId][owners[i]]) {
                 confirmationsTemp[count] = owners[i];
                 count += 1;
             }
+        }
         _confirmations = new address[](count);
-        for (i=0; i<count; i++)
+        for (i = 0; i < count; i++) {
             _confirmations[i] = confirmationsTemp[i];
         }
+    }
 
     /// @dev Returns list of transaction IDs in defined range.
     /// @param from Index start position of transaction array.
@@ -371,23 +435,28 @@ contract MultiSigWallet {
     /// @param pending Include pending transactions.
     /// @param executed Include executed transactions.
     /// @return Returns array of transaction IDs.
-    function getTransactionIds(uint from, uint to, bool pending, bool executed)
+    function getTransactionIds(
+        uint256 from,
+        uint256 to,
+        bool pending,
+        bool executed
+    )
         public
-        constant
+        view
-        returns (uint[] _transactionIds)
+        returns (uint256[] memory _transactionIds)
-    {
-        uint[] memory transactionIdsTemp = new uint[](transactionCount);
-        uint count = 0;
-        uint i;
-        for (i=0; i<transactionCount; i++)
-            if (   pending && !transactions[i].executed
-                || executed && transactions[i].executed)
     {
+        uint256[] memory transactionIdsTemp = new uint256[](transactionCount);
+        uint256 count = 0;
+        uint256 i;
+        for (i = 0; i < transactionCount; i++) {
+            if (pending && !transactions[i].executed || executed && transactions[i].executed) {
                 transactionIdsTemp[count] = i;
                 count += 1;
             }
-        _transactionIds = new uint[](to - from);
+        }
-        for (i=from; i<to; i++)
+        _transactionIds = new uint256[](to - from);
+        for (i = from; i < to; i++) {
             _transactionIds[i - from] = transactionIdsTemp[i];
         }
     }
+}

contracts/multisig/contracts/src/MultiSigWalletWithTimeLock.sol

@@ -16,7 +16,7 @@
 
 */
 
-pragma solidity 0.4.24;
+pragma solidity ^0.5.9;
 
 import "./MultiSigWallet.sol";
 
@@ -63,7 +63,7 @@ contract MultiSigWalletWithTimeLock is
     /// @param _required Number of required confirmations.
     /// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
     constructor (
-        address[] _owners,
+        address[] memory _owners,
         uint256 _required,
         uint256 _secondsTimeLocked
     )

contracts/multisig/contracts/test/ContractCallReceiver.sol

@@ -0,0 +1,51 @@
+/*
+
+  Copyright 2019 ZeroEx Intl.
+
+  Licensed under the Apache License, Version 2.0 (the "License");
+  you may not use this file except in compliance with the License.
+  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+
+*/
+
+pragma solidity ^0.5.9;
+
+import "@0x/contracts-utils/contracts/src/LibBytes.sol";
+
+
+contract ContractCallReceiver {
+
+    using LibBytes for bytes;
+
+    event ContractCall(
+        bytes4 functionSelector,
+        bytes data,
+        uint256 value
+    );
+
+    bytes4 constant internal ALWAYS_REVERT_SELECTOR = 0xF1F2F3F4;
+
+    function ()
+        external
+        payable
+    {
+        bytes4 selector = msg.data.readBytes4(0);
+        if (selector == ALWAYS_REVERT_SELECTOR) {
+            revert();
+        }
+
+        emit ContractCall(
+            selector,
+            msg.data,
+            msg.value
+        );
+    }
+}

contracts/multisig/contracts/test/TestAssetProxyOwner.sol

@@ -16,7 +16,8 @@
 
 */
 
-pragma solidity 0.4.24;
+pragma solidity ^0.5.9;
+pragma experimental ABIEncoderV2;
 
 import "../src/AssetProxyOwner.sol";
 
@@ -26,33 +27,52 @@ contract TestAssetProxyOwner is
     AssetProxyOwner
 {
     constructor (
+        bytes4[] memory _functionSelectors,
+        address[] memory _destinations,
+        uint128[] memory _functionCallTimeLockSeconds,
         address[] memory _owners,
-        address[] memory _assetProxyContracts,
         uint256 _required,
-        uint256 _secondsTimeLocked
+        uint256 _defaultSecondsTimeLocked
     )
         public
-        AssetProxyOwner(_owners, _assetProxyContracts, _required, _secondsTimeLocked)
+        AssetProxyOwner(
+            _functionSelectors,
+            _destinations,
+            _functionCallTimeLockSeconds,
+            _owners,
+            _required,
+            _defaultSecondsTimeLocked
+        )
     {}
     
-    function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id)
+    function registerFunctionCallBypassWallet(
-        public
+        bool hasCustomTimeLock,
-        view
+        bytes4 functionSelector,
-        validRemoveAuthorizedAddressAtIndexTx(id)
+        address destination,
-        returns (bool)
+        uint128 newSecondsTimeLocked
+    )
+        external
     {
-        // Do nothing. We expect reverts through the modifier
+        _registerFunctionCall(
-        return true;
+            hasCustomTimeLock,
+            functionSelector,
+            destination,
+            newSecondsTimeLocked
+        );
     }
 
-    /// @dev Compares first 4 bytes of byte array to `removeAuthorizedAddressAtIndex` function selector.
+    function assertValidFunctionCall(
-    /// @param data Transaction data.
+        uint256 transactionConfirmationTime,
-    /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`.
+        bytes calldata data,
-    function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data)
+        address destination
-        public
+    )
-        pure
+        external
-        returns (bool)
+        view
     {
-        return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
+        _assertValidFunctionCall(
+            transactionConfirmationTime,
+            data,
+            destination
+        );
     }
 }

contracts/multisig/package.json

@@ -34,7 +34,7 @@
         "lint-contracts": "solhint -c ../.solhint.json contracts/**/**/**/**/*.sol"
     },
     "config": {
-        "abis": "./generated-artifacts/@(AssetProxyOwner|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestRejectEther).json",
+        "abis": "./generated-artifacts/@(AssetProxyOwner|ContractCallReceiver|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestRejectEther).json",
         "abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
     },
     "repository": {
@@ -72,7 +72,7 @@
         "@0x/base-contract": "^5.3.1",
         "@0x/contracts-asset-proxy": "^2.2.5",
         "@0x/contracts-erc20": "^2.2.11",
-        "@0x/contracts-utils": "2.0.1",
+        "@0x/contracts-utils": "^3.2.1",
         "@0x/types": "^2.4.1",
         "@0x/typescript-typings": "^4.2.4",
         "@0x/utils": "^4.5.0",

contracts/multisig/src/artifacts.ts

@@ -6,6 +6,7 @@
 import { ContractArtifact } from 'ethereum-types';
 
 import * as AssetProxyOwner from '../generated-artifacts/AssetProxyOwner.json';
+import * as ContractCallReceiver from '../generated-artifacts/ContractCallReceiver.json';
 import * as MultiSigWallet from '../generated-artifacts/MultiSigWallet.json';
 import * as MultiSigWalletWithTimeLock from '../generated-artifacts/MultiSigWalletWithTimeLock.json';
 import * as TestAssetProxyOwner from '../generated-artifacts/TestAssetProxyOwner.json';
@@ -14,6 +15,7 @@ export const artifacts = {
     AssetProxyOwner: AssetProxyOwner as ContractArtifact,
     MultiSigWallet: MultiSigWallet as ContractArtifact,
     MultiSigWalletWithTimeLock: MultiSigWalletWithTimeLock as ContractArtifact,
+    ContractCallReceiver: ContractCallReceiver as ContractArtifact,
     TestAssetProxyOwner: TestAssetProxyOwner as ContractArtifact,
     TestRejectEther: TestRejectEther as ContractArtifact,
 };

contracts/multisig/src/wrappers.ts

@@ -4,6 +4,7 @@
  * -----------------------------------------------------------------------------
  */
 export * from '../generated-wrappers/asset_proxy_owner';
+export * from '../generated-wrappers/contract_call_receiver';
 export * from '../generated-wrappers/multi_sig_wallet';
 export * from '../generated-wrappers/multi_sig_wallet_with_time_lock';
 export * from '../generated-wrappers/test_asset_proxy_owner';

contracts/multisig/test/utils/asset_proxy_owner_wrapper.ts

@@ -1,72 +1,53 @@
-import { artifacts as proxyArtifacts } from '@0x/contracts-asset-proxy';
+import { constants, hexRandom, increaseTimeAndMineBlockAsync } from '@0x/contracts-test-utils';
-import { LogDecoder, Web3ProviderEngine } from '@0x/contracts-test-utils';
+import { AbiEncoder, BigNumber } from '@0x/utils';
-import { BigNumber } from '@0x/utils';
+import { LogWithDecodedArgs, TransactionReceiptWithDecodedLogs } from 'ethereum-types';
-import { Web3Wrapper } from '@0x/web3-wrapper';
+import * as _ from 'lodash';
-import { TransactionReceiptWithDecodedLogs } from 'ethereum-types';
 
-import { AssetProxyOwnerContract, TestAssetProxyOwnerContract } from '../../src';
+import { AssetProxyOwnerContract, AssetProxyOwnerSubmissionEventArgs, TestAssetProxyOwnerContract } from '../../src';
-import { artifacts } from '../../src/artifacts';
 
+// tslint:disable: no-unnecessary-type-assertion
 export class AssetProxyOwnerWrapper {
     private readonly _assetProxyOwner: AssetProxyOwnerContract | TestAssetProxyOwnerContract;
-    private readonly _web3Wrapper: Web3Wrapper;
+    constructor(assetproxyOwnerContract: AssetProxyOwnerContract | TestAssetProxyOwnerContract) {
-    private readonly _logDecoder: LogDecoder;
-    constructor(
-        assetproxyOwnerContract: AssetProxyOwnerContract | TestAssetProxyOwnerContract,
-        provider: Web3ProviderEngine,
-    ) {
         this._assetProxyOwner = assetproxyOwnerContract;
-        this._web3Wrapper = new Web3Wrapper(provider);
-        this._logDecoder = new LogDecoder(this._web3Wrapper, { ...artifacts, ...proxyArtifacts });
     }
     public async submitTransactionAsync(
-        destination: string,
+        data: string[],
-        data: string,
+        destinations: string[],
         from: string,
-        opts: { value?: BigNumber } = {},
+        opts: { values?: BigNumber[] } = {},
-    ): Promise<TransactionReceiptWithDecodedLogs> {
+    ): Promise<{ txReceipt: TransactionReceiptWithDecodedLogs; txId: BigNumber }> {
-        const value = opts.value === undefined ? new BigNumber(0) : opts.value;
+        const values = opts.values === undefined ? data.map(() => constants.ZERO_AMOUNT) : opts.values;
-        const txHash = await this._assetProxyOwner.submitTransaction.sendTransactionAsync(destination, value, data, {
+        const batchTransactionEncoder = AbiEncoder.create('(bytes[],address[],uint256[])');
-            from,
+        const batchTransactionData = batchTransactionEncoder.encode([data, destinations, values]);
-        });
+        const txReceipt = await this._assetProxyOwner.submitTransaction.awaitTransactionSuccessAsync(
-        const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
+            hexRandom(20), // submitTransaction will fail if this is a null address
-        return tx;
+            constants.ZERO_AMOUNT,
-    }
+            batchTransactionData,
-    public async confirmTransactionAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> {
+            { from },
-        const txHash = await this._assetProxyOwner.confirmTransaction.sendTransactionAsync(txId, { from });
-        const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
-        return tx;
-    }
-    public async revokeConfirmationAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> {
-        const txHash = await this._assetProxyOwner.revokeConfirmation.sendTransactionAsync(txId, { from });
-        const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
-        return tx;
-    }
-    public async executeTransactionAsync(
-        txId: BigNumber,
-        from: string,
-        opts: { gas?: number } = {},
-    ): Promise<TransactionReceiptWithDecodedLogs> {
-        const txHash = await this._assetProxyOwner.executeTransaction.sendTransactionAsync(txId, {
-            from,
-            gas: opts.gas,
-        });
-        const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
-        return tx;
-    }
-    public async executeRemoveAuthorizedAddressAtIndexAsync(
-        txId: BigNumber,
-        from: string,
-    ): Promise<TransactionReceiptWithDecodedLogs> {
-        // tslint:disable-next-line:no-unnecessary-type-assertion
-        const txHash = await (this
-            ._assetProxyOwner as TestAssetProxyOwnerContract).executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(
-            txId,
-            {
-                from,
-            },
         );
-        const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
+        const txId = (txReceipt.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>).args.transactionId;
-        return tx;
+        return { txReceipt, txId };
+    }
+    public async submitConfirmAndExecuteTransactionAsync(
+        data: string[],
+        destinations: string[],
+        signerAddresses: string[],
+        increaseTimeSeconds: number,
+        opts: { values?: BigNumber[]; executeFromAddress?: string; requiredSignatures?: number } = {},
+    ): Promise<{ executionTxReceipt: TransactionReceiptWithDecodedLogs; txId: BigNumber }> {
+        const submitResults = await this.submitTransactionAsync(data, destinations, signerAddresses[0], opts);
+        const requiredSignatures = opts.requiredSignatures === undefined ? 2 : opts.requiredSignatures;
+        for (const index of _.range(1, requiredSignatures)) {
+            await this._assetProxyOwner.confirmTransaction.awaitTransactionSuccessAsync(submitResults.txId, {
+                from: signerAddresses[index],
+            });
+        }
+        await increaseTimeAndMineBlockAsync(increaseTimeSeconds);
+        const executionTxReceipt = await this._assetProxyOwner.executeTransaction.awaitTransactionSuccessAsync(
+            submitResults.txId,
+            { from: opts.executeFromAddress === undefined ? signerAddresses[0] : opts.executeFromAddress },
+        );
+        return { executionTxReceipt, txId: submitResults.txId };
     }
 }

contracts/multisig/test/utils/index.ts

@@ -1,2 +1,2 @@
-export * from './asset_proxy_owner_wrapper';
 export * from './multi_sig_wrapper';
+export * from './asset_proxy_owner_wrapper';

contracts/multisig/tsconfig.json

@@ -4,6 +4,7 @@
     "include": ["./src/**/*", "./test/**/*", "./generated-wrappers/**/*"],
     "files": [
         "generated-artifacts/AssetProxyOwner.json",
+        "generated-artifacts/ContractCallReceiver.json",
         "generated-artifacts/MultiSigWallet.json",
         "generated-artifacts/MultiSigWalletWithTimeLock.json",
         "generated-artifacts/TestAssetProxyOwner.json",

contracts/test-utils/src/constants.ts

@@ -53,8 +53,8 @@ export const constants = {
     NUM_DUMMY_ERC1155_CONTRACTS_TO_DEPLOY: 2,
     NUM_ERC1155_FUNGIBLE_TOKENS_MINT: 4,
     NUM_ERC1155_NONFUNGIBLE_TOKENS_MINT: 4,
-    NULL_ADDRESS: '0x0000000000000000000000000000000000000000',
     NULL_BYTES4: '0x00000000',
+    NULL_ADDRESS: '0x0000000000000000000000000000000000000000',
     NULL_BYTES32: '0x0000000000000000000000000000000000000000000000000000000000000000',
     UNLIMITED_ALLOWANCE_IN_BASE_UNITS: MAX_UINT256,
     MAX_UINT256,

packages/types/src/index.ts

@@ -343,6 +343,12 @@ export enum RevertReason {
     TransfersSuccessful = 'TRANSFERS_SUCCESSFUL',
     // Staking
     InsufficientFunds = 'INSUFFICIENT_FUNDS',
+    // AssetProxyOwner
+    TxAlreadyExecuted = 'TX_ALREADY_EXECUTED',
+    DefaultTimeLockIncomplete = 'DEFAULT_TIME_LOCK_INCOMPLETE',
+    CustomTimeLockIncomplete = 'CUSTOM_TIME_LOCK_INCOMPLETE',
+    EqualLengthsRequired = 'EQUAL_LENGTHS_REQUIRED',
+    OnlyCallableByWallet = 'ONLY_CALLABLE_BY_WALLET',
 }
 
 export enum StatusCodes {

yarn.lock

@@ -643,12 +643,6 @@
     npmlog "^4.1.2"
     write-file-atomic "^2.3.0"
 
-"@0x/abi-gen-wrappers@^3.0.1":
-  version "3.0.3"
-  resolved "https://registry.yarnpkg.com/@0x/abi-gen-wrappers/-/abi-gen-wrappers-3.0.3.tgz#a30fdb3c520ce45fb327590281d000e086ff9609"
-  dependencies:
-    "@0x/base-contract" "^4.0.3"
-
 "@0x/asset-buyer@6.1.8":
   version "6.1.8"
   resolved "https://registry.yarnpkg.com/@0x/asset-buyer/-/asset-buyer-6.1.8.tgz#71f6abb366e89e62457c256644edb37e12113e94"
@@ -666,27 +660,6 @@
     ethereum-types "^2.1.3"
     lodash "^4.17.11"
 
-"@0x/base-contract@^4.0.1", "@0x/base-contract@^4.0.3":
-  version "4.0.3"
-  resolved "https://registry.yarnpkg.com/@0x/base-contract/-/base-contract-4.0.3.tgz#ea5e3640824ee096813350e55546d98455a57805"
-  dependencies:
-    "@0x/typescript-typings" "^4.0.0"
-    "@0x/utils" "^4.1.0"
-    "@0x/web3-wrapper" "^5.0.0"
-    ethereum-types "^2.0.0"
-    ethers "~4.0.4"
-    lodash "^4.17.11"
-
-"@0x/contract-addresses@^2.2.1":
-  version "2.3.3"
-  resolved "https://registry.yarnpkg.com/@0x/contract-addresses/-/contract-addresses-2.3.3.tgz#8cf009e7668c2fccca416177c85f3f6612c724c6"
-  dependencies:
-    lodash "^4.17.11"
-
-"@0x/contract-artifacts@^1.3.0":
-  version "1.5.1"
-  resolved "https://registry.npmjs.org/@0x/contract-artifacts/-/contract-artifacts-1.5.1.tgz#6fba56a1d3e2d5d897a75fcfa432e49e2ebb17a7"
-
 "@0x/contract-wrappers@^9.1.6", "@0x/contract-wrappers@^9.1.7":
   version "9.1.8"
   resolved "https://registry.yarnpkg.com/@0x/contract-wrappers/-/contract-wrappers-9.1.8.tgz#5923d35af3e4b442a57d02f74e02620b2d5b1356"
@@ -711,21 +684,6 @@
     lodash "^4.17.11"
     uuid "^3.3.2"
 
-"@0x/contracts-utils@2.0.1":
-  version "2.0.1"
-  resolved "https://registry.yarnpkg.com/@0x/contracts-utils/-/contracts-utils-2.0.1.tgz#32e298ab5e6edb045c37294063ff928b629db0a4"
-  dependencies:
-    "@0x/base-contract" "^4.0.1"
-    "@0x/order-utils" "^5.0.0"
-    "@0x/types" "^2.0.1"
-    "@0x/typescript-typings" "^4.0.0"
-    "@0x/utils" "^4.0.2"
-    "@0x/web3-wrapper" "^4.0.1"
-    bn.js "^4.11.8"
-    ethereum-types "^2.0.0"
-    ethereumjs-util "^5.1.1"
-    lodash "^4.17.5"
-
 "@0x/coordinator-server@^0.1.3":
   version "0.1.3"
   resolved "https://registry.yarnpkg.com/@0x/coordinator-server/-/coordinator-server-0.1.3.tgz#5fbb7c11bb641aa5386797769cab9a68a7d15b79"
@@ -755,28 +713,6 @@
     typeorm "0.2.7"
     websocket "^1.0.25"
 
-"@0x/order-utils@^5.0.0":
-  version "5.0.0"
-  resolved "https://registry.yarnpkg.com/@0x/order-utils/-/order-utils-5.0.0.tgz#7f43e0310ace31738895881501c8dda9c3a3aefa"
-  dependencies:
-    "@0x/abi-gen-wrappers" "^3.0.1"
-    "@0x/assert" "^2.0.1"
-    "@0x/base-contract" "^4.0.1"
-    "@0x/contract-addresses" "^2.2.1"
-    "@0x/contract-artifacts" "^1.3.0"
-    "@0x/json-schemas" "^3.0.1"
-    "@0x/types" "^2.0.1"
-    "@0x/typescript-typings" "^4.0.0"
-    "@0x/utils" "^4.0.2"
-    "@0x/web3-wrapper" "^4.0.1"
-    "@types/node" "*"
-    bn.js "^4.11.8"
-    ethereum-types "^2.0.0"
-    ethereumjs-abi "0.6.5"
-    ethereumjs-util "^5.1.1"
-    ethers "~4.0.4"
-    lodash "^4.17.11"
-
 "@0x/subproviders@^4.1.1":
   version "4.1.2"
   resolved "https://registry.yarnpkg.com/@0x/subproviders/-/subproviders-4.1.2.tgz#ab7bb0f482b11ccb4615fb5dd8ca85199cd0ae23"
@@ -806,32 +742,6 @@
   optionalDependencies:
     "@ledgerhq/hw-transport-node-hid" "^4.3.0"
 
-"@0x/web3-wrapper@^4.0.1":
-  version "4.0.2"
-  resolved "https://registry.yarnpkg.com/@0x/web3-wrapper/-/web3-wrapper-4.0.2.tgz#d4e0a4fa1217155e1aed4cd91086654fd99f2959"
-  dependencies:
-    "@0x/assert" "^2.0.2"
-    "@0x/json-schemas" "^3.0.2"
-    "@0x/typescript-typings" "^4.0.0"
-    "@0x/utils" "^4.0.3"
-    ethereum-types "^2.0.0"
-    ethereumjs-util "^5.1.1"
-    ethers "~4.0.4"
-    lodash "^4.17.11"
-
-"@0x/web3-wrapper@^5.0.0":
-  version "5.0.0"
-  resolved "https://registry.yarnpkg.com/@0x/web3-wrapper/-/web3-wrapper-5.0.0.tgz#a9b4baf0dca125181885b31c4dd5a3fbf6b87260"
-  dependencies:
-    "@0x/assert" "^2.0.3"
-    "@0x/json-schemas" "^3.0.3"
-    "@0x/typescript-typings" "^4.0.0"
-    "@0x/utils" "^4.1.0"
-    ethereum-types "^2.0.0"
-    ethereumjs-util "^5.1.1"
-    ethers "~4.0.4"
-    lodash "^4.17.11"
-
 "@0xproject/npm-cli-login@^0.0.11":
   version "0.0.11"
   resolved "https://registry.yarnpkg.com/@0xproject/npm-cli-login/-/npm-cli-login-0.0.11.tgz#3f1ec06112ce62aad300ff0575358f68aeecde2e"