Update AssetProxyOwner to 0.4.24

3 files changed, 21 insertions, 70 deletions

diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
index 7da3d87e2..bb02d8489 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
@@ -16,15 +16,18 @@
 
 */
 
-pragma solidity 0.4.10;
+pragma solidity 0.4.24;
 
 import "../../multisig/MultiSigWalletWithTimeLock.sol";
+import "../../utils/LibBytes/LibBytes.sol";
 
 
 contract AssetProxyOwner is
     MultiSigWalletWithTimeLock
 {
 
+    using LibBytes for bytes;
+
     event AssetProxyRegistration(address assetProxyContract, bool isRegistered);
 
     // Mapping of AssetProxy contract address =>
@@ -37,8 +40,14 @@ contract AssetProxyOwner is
     ///      on an approved AssetProxy contract.
     modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) {
         Transaction storage tx = transactions[transactionId];
-        require(isAssetProxyRegistered[tx.destination]);
-        require(readBytes4(tx.data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR);
+        require(
+            isAssetProxyRegistered[tx.destination],
+            "UNREGISTERED_ASSET_PROXY"
+        );
+        require(
+            tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR,
+            "INVALID_FUNCTION_SELECTOR"
+        );
         _;
     }
 
@@ -48,7 +57,7 @@ contract AssetProxyOwner is
     /// @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.
-    function AssetProxyOwner(
+    constructor (
         address[] memory _owners,
         address[] memory _assetProxyContracts,
         uint256 _required,
@@ -74,7 +83,7 @@ contract AssetProxyOwner is
         notNull(assetProxyContract)
     {
         isAssetProxyRegistered[assetProxyContract] = isRegistered;
-        AssetProxyRegistration(assetProxyContract, isRegistered);
+        emit AssetProxyRegistration(assetProxyContract, isRegistered);
     }
 
     /// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock.
@@ -89,36 +98,10 @@ contract AssetProxyOwner is
         tx.executed = true;
         // solhint-disable-next-line avoid-call-value
         if (tx.destination.call.value(tx.value)(tx.data))
-            Execution(transactionId);
+            emit Execution(transactionId);
         else {
-            ExecutionFailure(transactionId);
+            emit ExecutionFailure(transactionId);
             tx.executed = false;
         }
     }
-
-    /// @dev Reads an unpadded bytes4 value from a position in a byte array.
-    /// @param b Byte array containing a bytes4 value.
-    /// @param index Index in byte array of bytes4 value.
-    /// @return bytes4 value from byte array.
-    function readBytes4(
-        bytes memory b,
-        uint256 index
-    )
-        internal
-        returns (bytes4 result)
-    {
-        require(b.length >= index + 4);
-
-        // Arrays are prefixed by a 32 byte length field
-        index += 32;
-
-        // Read the bytes4 from array memory
-        assembly {
-            result := mload(add(b, index))
-            // Solidity does not require us to clean the trailing bytes.
-            // We do it anyway
-            result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000)
-        }
-        return result;
-    }
 }
diff --git a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
index 75e782d43..38ec42a72 100644
--- a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
+++ b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
@@ -16,7 +16,7 @@
 
 */
 
-pragma solidity 0.4.10;
+pragma solidity 0.4.24;
 
 import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol";
 
@@ -26,7 +26,7 @@ contract TestAssetProxyOwner is
     AssetProxyOwner
 {
 
-    function TestAssetProxyOwner(
+    constructor (
         address[] memory _owners,
         address[] memory _assetProxyContracts,
         uint256 _required,
@@ -38,6 +38,7 @@ contract TestAssetProxyOwner is
     
     function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id)
         public
+        view
         validRemoveAuthorizedAddressAtIndexTx(id)
         returns (bool)
     {
@@ -50,23 +51,9 @@ contract TestAssetProxyOwner is
     /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`.
     function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data)
         public
+        pure
         returns (bool)
     {
-        return readBytes4(data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
-    }
-
-    /// @dev Reads an unpadded bytes4 value from a position in a byte array.
-    /// @param b Byte array containing a bytes4 value.
-    /// @param index Index in byte array of bytes4 value.
-    /// @return bytes4 value from byte array.
-    function publicReadBytes4(
-        bytes memory b,
-        uint256 index
-    )
-        public
-        returns (bytes4 result)
-    {
-        result = readBytes4(b, index);
-        return result;
+        return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
     }
 }
diff --git a/packages/contracts/test/multisig/asset_proxy_owner.ts b/packages/contracts/test/multisig/asset_proxy_owner.ts
index 9515941ff..2f626daef 100644
--- a/packages/contracts/test/multisig/asset_proxy_owner.ts
+++ b/packages/contracts/test/multisig/asset_proxy_owner.ts
@@ -148,25 +148,6 @@ describe('AssetProxyOwner', () => {
         });
     });
 
-    describe('readBytes4', () => {
-        it('should revert if byte array has a length < 4', async () => {
-            const byteArrayLessThan4Bytes = '0x010101';
-            return expectContractCallFailedWithoutReasonAsync(
-                testAssetProxyOwner.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)),
-            );
-        });
-        it('should return the first 4 bytes of a byte array of arbitrary length', async () => {
-            const byteArrayLongerThan32Bytes =
-                '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
-            const first4Bytes = await testAssetProxyOwner.publicReadBytes4.callAsync(
-                byteArrayLongerThan32Bytes,
-                new BigNumber(0),
-            );
-            const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10);
-            expect(first4Bytes).to.equal(expectedFirst4Bytes);
-        });
-    });
-
     describe('registerAssetProxy', () => {
         it('should throw if not called by multisig', async () => {
             const isRegistered = true;