From f03e5c6bd12c88fffbad324fd7493d3acedea0aa Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Fri, 1 Jun 2018 11:34:45 -0700 Subject: Style audit proxies --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 8 +++- .../current/protocol/AssetProxy/ERC721Proxy.sol | 33 ++++++++++++----- .../TestAssetDataDecoders.sol | 43 +++++++++++++++++----- 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index 11383adaf..54cbeb963 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -79,7 +79,10 @@ contract ERC20Proxy is return PROXY_ID; } - /// @dev Decodes ERC20 Asset Proxy data + /// @dev Decodes ERC20 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC20 proxy id. + /// @return token ERC20 token address. function decodeERC20AssetData(bytes memory assetData) internal pure @@ -88,10 +91,13 @@ contract ERC20Proxy is address token ) { + // Validate encoded data length require( assetData.length == 21, INVALID_ASSET_DATA_LENGTH ); + + // Decode data proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 9dac02d87..58c23b03b 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -53,7 +53,7 @@ contract ERC721Proxy is uint8 proxyId, address token, uint256 tokenId, - bytes memory data + bytes memory receiverData ) = decodeERC721AssetData(assetData); // Data must be intended for this proxy. @@ -76,11 +76,10 @@ contract ERC721Proxy is INVALID_AMOUNT ); - // Transfer token. - // Save gas by calling safeTransferFrom only when there is data present. - // Either succeeds or throws. - if(data.length > 0) { - ERC721Token(token).safeTransferFrom(from, to, tokenId, data); + // Transfer token. Saves gas by calling safeTransferFrom only + // when there is receiverData present. Either succeeds or throws. + if(receiverData.length > 0) { + ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } else { ERC721Token(token).transferFrom(from, to, tokenId); } @@ -96,7 +95,13 @@ contract ERC721Proxy is return PROXY_ID; } - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC721 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC721 proxy id. + /// @return token ERC721 token address. + /// @return tokenId ERC721 token id. + /// @return receiverData Additional data with no specific format, which + /// is passed to the receiving contract's onERC721Received. function decodeERC721AssetData(bytes memory assetData) internal pure @@ -104,20 +109,28 @@ contract ERC721Proxy is uint8 proxyId, address token, uint256 tokenId, - bytes memory data + bytes memory receiverData ) { + // Validate encoded data length require( assetData.length >= 53, INVALID_ASSET_DATA_LENGTH ); + + // Decode asset data proxyId = uint8(assetData[0]); token = readAddress(assetData, 1); tokenId = readUint256(assetData, 21); if (assetData.length > 53) { - data = readBytes(assetData, 53); + receiverData = readBytes(assetData, 53); } - return (proxyId, token, tokenId, data); + return ( + proxyId, + token, + tokenId, + receiverData + ); } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 45787d88b..2c6a8fdb0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -27,26 +27,51 @@ contract TestAssetDataDecoders is ERC721Proxy { - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC20 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC20 proxy id. + /// @return token ERC20 token address. function publicDecodeERC20Data(bytes memory assetData) public pure - returns (uint8, address) + returns ( + uint8 proxyId, + address token + ) { - return ERC20Proxy.decodeERC20AssetData(assetData); + (proxyId, token) = decodeERC20AssetData(assetData); + return (proxyId, token); } - /// @dev Decodes ERC721 Asset Proxy data + /// @dev Decodes ERC721 Asset data. + /// @param assetData Encoded byte array. + /// @return proxyId Intended ERC721 proxy id. + /// @return token ERC721 token address. + /// @return tokenId ERC721 token id. + /// @return receiverData Additional data with no specific format, which + /// is passed to the receiving contract's onERC721Received. function publicDecodeERC721Data(bytes memory assetData) public pure returns ( - uint8, - address, - uint256, - bytes memory + uint8 proxyId, + address token, + uint256 tokenId, + bytes memory receiverData ) { - return ERC721Proxy.decodeERC721AssetData(assetData); + ( + proxyId, + token, + tokenId, + receiverData + ) = decodeERC721AssetData(assetData); + + return ( + proxyId, + token, + tokenId, + receiverData + ); } } -- cgit