diff options
Diffstat (limited to 'packages/contracts/src')
5 files changed, 26 insertions, 57 deletions
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol index 785f3aba9..258443bca 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol @@ -18,7 +18,6 @@ pragma solidity 0.4.24; -import "../../utils/LibBytes/LibBytes.sol"; import "./MixinAuthorizable.sol"; diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol index caf05b0dd..65b664b8b 100644 --- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol @@ -18,7 +18,6 @@ pragma solidity 0.4.24; -import "../../utils/LibBytes/LibBytes.sol"; import "./MixinAuthorizable.sol"; 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..4d00e92d3 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, @@ -59,7 +68,10 @@ contract AssetProxyOwner is { for (uint256 i = 0; i < _assetProxyContracts.length; i++) { address assetProxy = _assetProxyContracts[i]; - require(assetProxy != address(0)); + require( + assetProxy != address(0), + "INVALID_ASSET_PROXY" + ); isAssetProxyRegistered[assetProxy] = true; } } @@ -74,7 +86,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 +101,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/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol index 80475e6e3..e90b62f19 100644 --- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,7 +19,6 @@ pragma solidity 0.4.24; import "../../utils/Ownable/Ownable.sol"; -import "../../utils/LibBytes/LibBytes.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; @@ -28,7 +27,6 @@ contract MixinAssetProxyDispatcher is Ownable, MAssetProxyDispatcher { - using LibBytes for bytes; // Mapping from Asset Proxy Id's to their respective Asset Proxy mapping (bytes4 => IAssetProxy) public assetProxies; @@ -90,7 +88,7 @@ contract MixinAssetProxyDispatcher is "LENGTH_GREATER_THAN_3_REQUIRED" ); - // Lookup assetProxy + // Lookup assetProxy. We do not use `LibBytes.readBytes4` for gas efficiency reasons. bytes4 assetProxyId; assembly { assetProxyId := and(mload( 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; } } |