Move readFirst4 to LibBytes

3 files changed, 42 insertions, 20 deletions

diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol
index 1061d0b9a..1b74dfc44 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol
@@ -19,8 +19,10 @@
 pragma solidity ^0.4.10;
 
 import "../../multisig/MultiSigWalletWithTimeLock.sol";
+import "../../utils/LibBytes/LibBytes.sol";
 
 contract AssetProxyOwner is
+    LibBytes,
     MultiSigWalletWithTimeLock
 {
 
@@ -31,6 +33,8 @@ contract AssetProxyOwner is
 
     bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)"));
 
+    /// @dev Function will revert if the transaction does not call `removeAuthorizedAddress`
+    ///      on an approved AssetProxy contract.
     modifier validRemoveAuthorizedAddressTx(uint256 transactionId) {
         Transaction storage tx = transactions[transactionId];
         require(isAssetProxyRegistered[tx.destination]);
@@ -41,20 +45,22 @@ contract AssetProxyOwner is
     /// @dev Contract constructor sets initial owners, required number of confirmations,
     ///      time lock, and list of AssetProxy addresses.
     /// @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 _assetProxyContracts Array of AssetProxy contract addresses.
     function AssetProxyOwner(
         address[] memory _owners,
+        address[] memory _assetProxyContracts,
         uint256 _required,
-        uint256 _secondsTimeLocked,
-        address[] memory _assetProxyContracts)
+        uint256 _secondsTimeLocked
+    )
         public
         MultiSigWalletWithTimeLock(_owners, _required, _secondsTimeLocked)
     {
         for (uint256 i = 0; i < _assetProxyContracts.length; i++) {
-            require(_assetProxyContracts[i] != address(0));
-            isAssetProxyRegistered[_assetProxyContracts[i]] = true;
+            address assetProxy = _assetProxyContracts[i];
+            require(assetProxy != address(0));
+            isAssetProxyRegistered[assetProxy] = true;
         }
     }
 
@@ -101,19 +107,4 @@ contract AssetProxyOwner is
         require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes);
         return true;
     }
-
-    /// @dev Reads the first 4 bytes from a byte array of arbitrary length.
-    /// @param data Byte array to read first 4 bytes from.
-    /// @return First 4 bytes of data.
-    function readFirst4(bytes memory data)
-        public
-        pure
-        returns (bytes4 result)
-    {
-        require(data.length >= 4);
-        assembly {
-            result := mload(add(data, 32))
-        }
-        return result;
-    }
 }
diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
index ac4602933..5a6801262 100644
--- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
+++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol
@@ -130,4 +130,16 @@ contract TestLibBytes is
         writeUint256(b, index, input);
         return b;
     }
+
+    /// @dev Reads the first 4 bytes from a byte array of arbitrary length.
+    /// @param b Byte array to read first 4 bytes from.
+    /// @return First 4 bytes of data.
+    function publicReadFirst4(bytes memory b)
+        public
+        pure
+        returns (bytes4 result)
+    {
+        result = readFirst4(b);
+        return result;
+    }
 }
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index 2c5d9e756..975565773 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -21,6 +21,7 @@ pragma solidity ^0.4.24;
 contract LibBytes {
 
     // Revert reasons
+    string constant GTE_4_LENGTH_REQUIRED = "Length must be greater than or equal to 4.";
     string constant GTE_20_LENGTH_REQUIRED = "Length must be greater than or equal to 20.";
     string constant GTE_32_LENGTH_REQUIRED = "Length must be greater than or equal to 32.";
 
@@ -203,4 +204,22 @@ contract LibBytes {
     {
         writeBytes32(b, index, bytes32(input));
     }
+
+    /// @dev Reads the first 4 bytes from a byte array of arbitrary length.
+    /// @param b Byte array to read first 4 bytes from.
+    /// @return First 4 bytes of data.
+    function readFirst4(bytes memory b)
+        internal
+        pure
+        returns (bytes4 result)
+    {
+        require(
+            b.length >= 4,
+            GTE_4_LENGTH_REQUIRED
+        );
+        assembly {
+            result := mload(add(b, 32))
+        }
+        return result;
+    }
 }