aboutsummaryrefslogtreecommitdiffstats
path: root/packages
diff options
context:
space:
mode:
authorAmir Bandeali <abandeali1@gmail.com>2018-06-26 07:53:15 +0800
committerGitHub <noreply@github.com>2018-06-26 07:53:15 +0800
commitcba92a01b6dee208b497817445b7ae4048e299c0 (patch)
treede513c4d4cf75e62488354eb912451c8fb7617ea /packages
parent9341afe764a18c2104e598b25699a30cc1438d8e (diff)
parenta89908540f8bca266780c59f3cb1521e7fb16b73 (diff)
downloaddexon-0x-contracts-cba92a01b6dee208b497817445b7ae4048e299c0.tar.gz
dexon-0x-contracts-cba92a01b6dee208b497817445b7ae4048e299c0.tar.zst
dexon-0x-contracts-cba92a01b6dee208b497817445b7ae4048e299c0.zip
Merge pull request #743 from 0xProject/feature/contracts/abi-encoded
ABI encode assetData fields
Diffstat (limited to 'packages')
-rw-r--r--packages/contracts/compiler.json1
-rw-r--r--packages/contracts/package.json2
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol4
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol4
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol14
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol93
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol94
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol36
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol3
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol3
-rw-r--r--packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol3
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol110
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol31
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol17
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol36
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol10
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol20
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol4
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol3
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol5
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol1
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol6
-rw-r--r--packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol3
-rw-r--r--packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol67
-rw-r--r--packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol5
-rw-r--r--packages/contracts/test/asset_proxy/decoder.ts3
-rw-r--r--packages/contracts/test/asset_proxy/proxies.ts74
-rw-r--r--packages/contracts/test/exchange/core.ts2
-rw-r--r--packages/contracts/test/exchange/dispatcher.ts9
-rw-r--r--packages/contracts/test/exchange/match_orders.ts2
-rw-r--r--packages/contracts/test/exchange/transactions.ts2
-rw-r--r--packages/contracts/test/exchange/wrapper.ts2
-rw-r--r--packages/order-utils/src/asset_proxy_utils.ts154
-rw-r--r--packages/order-utils/src/constants.ts6
-rw-r--r--packages/types/src/index.ts10
35 files changed, 478 insertions, 361 deletions
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index 6b7b69caa..e1ea401b3 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -27,6 +27,7 @@
"ERC721Proxy",
"Exchange",
"ExchangeWrapper",
+ "IAssetData",
"MixinAuthorizable",
"MultiSigWallet",
"MultiSigWalletWithTimeLock",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 8249b9e0c..95f2dbb4e 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -34,7 +34,7 @@
},
"config": {
"abis":
- "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+ "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|ERC20Proxy|ERC721Proxy|Exchange|ExchangeWrapper|IAssetData|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|TestAssetProxyOwner|TestAssetDataDecoders|TestAssetProxyDispatcher|TestLibBytes|TestLibs|TestSignatureValidator|TestValidator|TestWallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
},
"repository": {
"type": "git",
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
index 7ca823d1f..b74d2c231 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol
@@ -30,14 +30,14 @@ contract ERC20Proxy is
MixinERC20Transfer
{
// Id of this proxy.
- uint8 constant PROXY_ID = 1;
+ bytes4 constant PROXY_ID = bytes4(keccak256("ERC20Token(address)"));
/// @dev Gets the proxy id associated with the proxy address.
/// @return Proxy id.
function getProxyId()
external
view
- returns (uint8)
+ returns (bytes4)
{
return PROXY_ID;
}
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
index 7ff25aea3..f6293f97d 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol
@@ -30,14 +30,14 @@ contract ERC721Proxy is
MixinERC721Transfer
{
// Id of this proxy.
- uint8 constant PROXY_ID = 2;
+ bytes4 constant PROXY_ID = bytes4(keccak256("ERC721Token(address,uint256,bytes)"));
/// @dev Gets the proxy id associated with the proxy address.
/// @return Proxy id.
function getProxyId()
external
view
- returns (uint8)
+ returns (bytes4)
{
return PROXY_ID;
}
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
index 37c12f861..3b9584a44 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol
@@ -19,12 +19,10 @@
pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
-import "./libs/LibAssetProxyErrors.sol";
import "../../utils/Ownable/Ownable.sol";
import "./mixins/MAuthorizable.sol";
contract MixinAuthorizable is
- LibAssetProxyErrors,
Ownable,
MAuthorizable
{
@@ -33,7 +31,7 @@ contract MixinAuthorizable is
modifier onlyAuthorized {
require(
authorized[msg.sender],
- SENDER_NOT_AUTHORIZED
+ "SENDER_NOT_AUTHORIZED"
);
_;
}
@@ -49,7 +47,7 @@ contract MixinAuthorizable is
{
require(
!authorized[target],
- TARGET_ALREADY_AUTHORIZED
+ "TARGET_ALREADY_AUTHORIZED"
);
authorized[target] = true;
@@ -65,7 +63,7 @@ contract MixinAuthorizable is
{
require(
authorized[target],
- TARGET_NOT_AUTHORIZED
+ "TARGET_NOT_AUTHORIZED"
);
delete authorized[target];
@@ -91,15 +89,15 @@ contract MixinAuthorizable is
{
require(
authorized[target],
- TARGET_NOT_AUTHORIZED
+ "TARGET_NOT_AUTHORIZED"
);
require(
index < authorities.length,
- INDEX_OUT_OF_BOUNDS
+ "INDEX_OUT_OF_BOUNDS"
);
require(
authorities[index] == target,
- AUTHORIZED_ADDRESS_MISMATCH
+ "AUTHORIZED_ADDRESS_MISMATCH"
);
delete authorized[target];
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol
index 0e7f3fc89..a09db43bd 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol
@@ -21,11 +21,9 @@ pragma experimental ABIEncoderV2;
import "../../utils/LibBytes/LibBytes.sol";
import "../../tokens/ERC20Token/IERC20Token.sol";
-import "./libs/LibTransferErrors.sol";
-contract MixinERC20Transfer is
- LibTransferErrors
-{
+contract MixinERC20Transfer {
+
using LibBytes for bytes;
/// @dev Internal version of `transferFrom`.
@@ -42,41 +40,74 @@ contract MixinERC20Transfer is
internal
{
// Decode asset data.
- address token = assetData.readAddress(0);
-
+ address token = assetData.readAddress(16);
+
// Transfer tokens.
// We do a raw call so we can check the success separate
// from the return data.
- bool success = token.call(abi.encodeWithSelector(
- IERC20Token(token).transferFrom.selector,
- from,
- to,
- amount
- ));
- require(
- success,
- TRANSFER_FAILED
- );
+ // We construct calldata for the `token.transferFrom` ABI.
+ // The layout of this calldata is in the table below.
+ //
+ // | Area | Offset | Length | Contents |
+ // |----------|--------|---------|-------------------------------------|
+ // | Header | 0 | 4 | function selector |
+ // | Params | | 3 * 32 | function parameters: |
+ // | | 4 | | 1. from |
+ // | | 36 | | 2. to |
+ // | | 68 | | 3. amount |
- // Check return data.
- // If there is no return data, we assume the token incorrectly
- // does not return a bool. In this case we expect it to revert
- // on failure, which was handled above.
- // If the token does return data, we require that it is a single
- // value that evaluates to true.
+ bytes4 transferFromSelector = IERC20Token(token).transferFrom.selector;
+ bool success;
assembly {
- if returndatasize {
- success := 0
- if eq(returndatasize, 32) {
- // First 64 bytes of memory are reserved scratch space
- returndatacopy(0, 0, 32)
- success := mload(0)
- }
- }
+ /////// Setup State ///////
+ // `cdStart` is the start of the calldata for `token.transferFrom` (equal to free memory ptr).
+ let cdStart := mload(64)
+
+ /////// Setup Header Area ///////
+ // This area holds the 4-byte `transferFromSelector`.
+ // Any trailing data in transferFromSelector will be
+ // overwritten in the next `mstore` call.
+ mstore(cdStart, transferFromSelector)
+
+ /////// Setup Params Area ///////
+ // Each parameter is padded to 32-bytes.
+ // The entire Params Area is 96 bytes.
+ // A 20-byte mask is applied to addresses to
+ // zero-out the unused bytes.
+ mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff))
+ mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff))
+ mstore(add(cdStart, 68), amount)
+
+ /////// Call `token.transferFrom` using the calldata ///////
+ success := call(
+ gas, // forward all gas
+ token, // call address of token contract
+ 0, // don't send any ETH
+ cdStart, // pointer to start of input
+ 100, // length of input
+ cdStart, // write output over input
+ 32 // output size should be 32 bytes
+ )
+
+ /////// Check return data. ///////
+ // If there is no return data, we assume the token incorrectly
+ // does not return a bool. In this case we expect it to revert
+ // on failure, which was handled above.
+ // If the token does return data, we require that it is a single
+ // nonzero 32 bytes value.
+ // So the transfer succeeded if the call succeeded and either
+ // returned nothing, or returned a non-zero 32 byte value.
+ success := and(success, or(
+ iszero(returndatasize),
+ and(
+ eq(returndatasize, 32),
+ gt(mload(cdStart), 0)
+ )
+ ))
}
require(
success,
- TRANSFER_FAILED
+ "TRANSFER_FAILED"
);
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
index 944068bbb..c170917ea 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol
@@ -28,6 +28,8 @@ contract MixinERC721Transfer is
{
using LibBytes for bytes;
+ bytes4 constant SAFE_TRANSFER_FROM_SELECTOR = bytes4(keccak256("safeTransferFrom(address,address,uint256,bytes)"));
+
/// @dev Internal version of `transferFrom`.
/// @param assetData Encoded byte array.
/// @param from Address to transfer asset from.
@@ -54,11 +56,85 @@ contract MixinERC721Transfer is
bytes memory receiverData
) = decodeERC721AssetData(assetData);
- ERC721Token(token).safeTransferFrom(
- from,
- to,
- tokenId,
- receiverData
+ // We construct calldata for the `token.safeTransferFrom` ABI.
+ // The layout of this calldata is in the table below.
+ //
+ // | Area | Offset | Length | Contents |
+ // |----------|--------|---------|-------------------------------------|
+ // | Header | 0 | 4 | function selector |
+ // | Params | | 4 * 32 | function parameters: |
+ // | | 4 | | 1. from |
+ // | | 36 | | 2. to |
+ // | | 68 | | 3. tokenId |
+ // | | 100 | | 4. offset to receiverData (*) |
+ // | Data | | | receiverData: |
+ // | | 132 | 32 | receiverData Length |
+ // | | 164 | ** | receiverData Contents |
+
+ bytes4 safeTransferFromSelector = SAFE_TRANSFER_FROM_SELECTOR;
+ bool success;
+ assembly {
+ /////// Setup State ///////
+ // `cdStart` is the start of the calldata for
+ // `token.safeTransferFrom` (equal to free memory ptr).
+ let cdStart := mload(64)
+ // `dataAreaLength` is the total number of words
+ // needed to store `receiverData`
+ // As-per the ABI spec, this value is padded up to
+ // the nearest multiple of 32,
+ // and includes 32-bytes for length.
+ // It's calculated as folows:
+ // - Unpadded length in bytes = `mload(receiverData) + 32`
+ // - Add 31 to convert rounding down to rounding up.
+ // Combined with the previous and this is `63`.
+ // - Round down to nearest multiple of 32 by clearing
+ // bits 0x1F. This is done with `and` and a mask.
+ let dataAreaLength := and(add(mload(receiverData), 63), 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFE0)
+ // `cdEnd` is the end of the calldata for `token.safeTransferFrom`.
+ let cdEnd := add(cdStart, add(132, dataAreaLength))
+
+ /////// Setup Header Area ///////
+ // This area holds the 4-byte `transferFromSelector`.
+ // Any trailing data in transferFromSelector will be
+ // overwritten in the next `mstore` call.
+ mstore(cdStart, safeTransferFromSelector)
+
+ /////// Setup Params Area ///////
+ // Each parameter is padded to 32-bytes.
+ // The entire Params Area is 128 bytes.
+ // Notes:
+ // 1. A 20-byte mask is applied to addresses
+ // to zero-out the unused bytes.
+ // 2. The offset to `receiverData` is the length
+ // of the Params Area (128 bytes).
+ mstore(add(cdStart, 4), and(from, 0xffffffffffffffffffffffffffffffffffffffff))
+ mstore(add(cdStart, 36), and(to, 0xffffffffffffffffffffffffffffffffffffffff))
+ mstore(add(cdStart, 68), tokenId)
+ mstore(add(cdStart, 100), 128)
+
+ /////// Setup Data Area ///////
+ // This area holds `receiverData`.
+ let dataArea := add(cdStart, 132)
+ for {} lt(dataArea, cdEnd) {} {
+ mstore(dataArea, mload(receiverData))
+ dataArea := add(dataArea, 32)
+ receiverData := add(receiverData, 32)
+ }
+
+ /////// Call `token.safeTransferFrom` using the calldata ///////
+ success := call(
+ gas, // forward all gas
+ token, // call address of token contract
+ 0, // don't send any ETH
+ cdStart, // pointer to start of input
+ sub(cdEnd, cdStart), // length of input
+ cdStart, // write output over input
+ 0 // output size is 0 bytes
+ )
+ }
+ require(
+ success,
+ TRANSFER_FAILED
);
}
@@ -79,11 +155,9 @@ contract MixinERC721Transfer is
)
{
// Decode asset data.
- token = assetData.readAddress(0);
- tokenId = assetData.readUint256(20);
- if (assetData.length > 52) {
- receiverData = assetData.readBytesWithLength(52);
- }
+ token = assetData.readAddress(16);
+ tokenId = assetData.readUint256(36);
+ receiverData = assetData.readBytesWithLength(100);
return (
token,
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol
new file mode 100644
index 000000000..8c78ee8c4
--- /dev/null
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetData.sol
@@ -0,0 +1,36 @@
+/*
+
+ Copyright 2018 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.4.23;
+
+// @dev Interface of the asset proxy's assetData.
+// The asset proxies take an ABI encoded `bytes assetData` as argument.
+// This argument is ABI encoded as one of the methods of this interface.
+interface IAssetData {
+
+ function ERC20Token(
+ address tokenContract)
+ external pure;
+
+ function ERC721Token(
+ address tokenContract,
+ uint256 tokenId,
+ bytes receiverData)
+ external pure;
+
+}
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
index 22f43b12f..d12d23610 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/interfaces/IAssetProxy.sol
@@ -56,6 +56,5 @@ contract IAssetProxy is
function getProxyId()
external
view
- returns (uint8);
+ returns (bytes4);
}
-
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol
index dca4f400f..b0b20c044 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol
@@ -18,7 +18,10 @@
pragma solidity ^0.4.24;
+/// @dev This contract documents the revert reasons used in the AssetProxy contracts.
+/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons.
contract LibAssetProxyErrors {
+
/// Authorizable errors ///
string constant SENDER_NOT_AUTHORIZED = "SENDER_NOT_AUTHORIZED"; // Sender not authorized to call this method.
string constant TARGET_NOT_AUTHORIZED = "TARGET_NOT_AUTHORIZED"; // Target address not authorized to call this method.
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol
index ba784ab22..88187f196 100644
--- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibTransferErrors.sol
@@ -18,7 +18,10 @@
pragma solidity ^0.4.24;
+/// @dev This contract documents the revert reasons used in the `transferFrom` methods of different AssetProxy contracts.
+/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons.
contract LibTransferErrors {
+
/// Transfer errors ///
string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1.
string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
index b8d6c0722..3203322aa 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -19,17 +19,18 @@
pragma solidity ^0.4.24;
import "../../utils/Ownable/Ownable.sol";
-import "./libs/LibExchangeErrors.sol";
+import "../../utils/LibBytes/LibBytes.sol";
import "./mixins/MAssetProxyDispatcher.sol";
import "../AssetProxy/interfaces/IAssetProxy.sol";
contract MixinAssetProxyDispatcher is
Ownable,
- LibExchangeErrors,
MAssetProxyDispatcher
{
+ using LibBytes for bytes;
+
// Mapping from Asset Proxy Id's to their respective Asset Proxy
- mapping (uint8 => IAssetProxy) public assetProxies;
+ mapping (bytes4 => IAssetProxy) public assetProxies;
/// @dev Registers an asset proxy to an asset proxy id.
/// An id can only be assigned to a single proxy at a given time.
@@ -37,7 +38,7 @@ contract MixinAssetProxyDispatcher is
/// @param newAssetProxy Address of new asset proxy to register, or 0x0 to unset assetProxyId.
/// @param oldAssetProxy Existing asset proxy to overwrite, or 0x0 if assetProxyId is currently unused.
function registerAssetProxy(
- uint8 assetProxyId,
+ bytes4 assetProxyId,
address newAssetProxy,
address oldAssetProxy
)
@@ -48,17 +49,17 @@ contract MixinAssetProxyDispatcher is
address currentAssetProxy = assetProxies[assetProxyId];
require(
oldAssetProxy == currentAssetProxy,
- ASSET_PROXY_MISMATCH
+ "ASSET_PROXY_MISMATCH"
);
IAssetProxy assetProxy = IAssetProxy(newAssetProxy);
// Ensure that the id of newAssetProxy matches the passed in assetProxyId, unless it is being reset to 0.
if (newAssetProxy != address(0)) {
- uint8 newAssetProxyId = assetProxy.getProxyId();
+ bytes4 newAssetProxyId = assetProxy.getProxyId();
require(
newAssetProxyId == assetProxyId,
- ASSET_PROXY_ID_MISMATCH
+ "ASSET_PROXY_ID_MISMATCH"
);
}
@@ -74,7 +75,7 @@ contract MixinAssetProxyDispatcher is
/// @dev Gets an asset proxy.
/// @param assetProxyId Id of the asset proxy.
/// @return The asset proxy registered to assetProxyId. Returns 0x0 if no proxy is registered.
- function getAssetProxy(uint8 assetProxyId)
+ function getAssetProxy(bytes4 assetProxyId)
external
view
returns (address)
@@ -83,14 +84,12 @@ contract MixinAssetProxyDispatcher is
}
/// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws.
- /// @param assetData Byte array encoded for the respective asset proxy.
- /// @param assetProxyId Id of assetProxy to dispach to.
+ /// @param assetData Byte array encoded for the asset.
/// @param from Address to transfer token from.
/// @param to Address to transfer token to.
/// @param amount Amount of token to transfer.
function dispatchTransferFrom(
bytes memory assetData,
- uint8 assetProxyId,
address from,
address to,
uint256 amount
@@ -99,19 +98,94 @@ contract MixinAssetProxyDispatcher is
{
// Do nothing if no amount should be transferred.
if (amount > 0) {
+ // Ensure assetData length is valid
+ require(
+ assetData.length > 3,
+ "LENGTH_GREATER_THAN_3_REQUIRED"
+ );
+
// Lookup assetProxy
+ bytes4 assetProxyId;
+ assembly {
+ assetProxyId := and(mload(
+ add(assetData, 32)),
+ 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000
+ )
+ }
IAssetProxy assetProxy = assetProxies[assetProxyId];
+
// Ensure that assetProxy exists
require(
assetProxy != address(0),
- ASSET_PROXY_DOES_NOT_EXIST
+ "ASSET_PROXY_DOES_NOT_EXIST"
);
- // transferFrom will either succeed or throw.
- assetProxy.transferFrom(
- assetData,
- from,
- to,
- amount
+
+ // We construct calldata for the `assetProxy.transferFrom` ABI.
+ // The layout of this calldata is in the table below.
+ //
+ // | Area | Offset | Length | Contents |
+ // | -------- |--------|---------|-------------------------------------------- |
+ // | Header | 0 | 4 | function selector |
+ // | Params | | 4 * 32 | function parameters: |
+ // | | 4 | | 1. offset to assetData (*) |
+ // | | 36 | | 2. from |
+ // | | 68 | | 3. to |
+ // | | 100 | | 4. amount |
+ // | Data | | | assetData: |
+ // | | 132 | 32 | assetData Length |
+ // | | 164 | ** | assetData Contents |
+
+ bytes4 transferFromSelector = IAssetProxy(assetProxy).transferFrom.selector;
+ bool success;
+ assembly {
+ /////// Setup State ///////
+ // `cdStart` is the start of the calldata for `assetProxy.transferFrom` (equal to free memory ptr).
+ let cdStart := mload(64)
+ // `dataAreaLength` is the total number of words needed to store `assetData`
+ // As-per the ABI spec, this value is padded up to the nearest multiple of 32,
+ // and includes 32-bytes for length.
+ let dataAreaLength := and(add(mload(assetData), 63), 0xFFFFFFFFFFFE0)
+ // `cdEnd` is the end of the calldata for `assetProxy.transferFrom`.
+ let cdEnd := add(cdStart, add(132, dataAreaLength))
+
+
+ /////// Setup Header Area ///////
+ // This area holds the 4-byte `transferFromSelector`.
+ mstore(cdStart, transferFromSelector)
+
+ /////// Setup Params Area ///////
+ // Each parameter is padded to 32-bytes. The entire Params Area is 128 bytes.
+ // Notes:
+ // 1. The offset to `assetData` is the length of the Params Area (128 bytes).
+ // 2. A 20-byte mask is applied to addresses to zero-out the unused bytes.
+ mstore(add(cdStart, 4), 128)
+ mstore(add(cdStart, 36), and(from, 0xffffffffffffffffffffffffffffffffffffffff))
+ mstore(add(cdStart, 68), and(to, 0xffffffffffffffffffffffffffffffffffffffff))
+ mstore(add(cdStart, 100), amount)
+
+ /////// Setup Data Area ///////
+ // This area holds `assetData`.
+ let dataArea := add(cdStart, 132)
+ for {} lt(dataArea, cdEnd) {} {
+ mstore(dataArea, mload(assetData))
+ dataArea := add(dataArea, 32)
+ assetData := add(assetData, 32)
+ }
+
+ /////// Call `assetProxy.transferFrom` using the constructed calldata ///////
+ success := call(
+ gas, // forward all gas
+ assetProxy, // call address of asset proxy
+ 0, // don't send any ETH
+ cdStart, // pointer to start of input
+ sub(cdEnd, cdStart), // length of input
+ cdStart, // write output over input
+ 0 // output size is 0 bytes
+ )
+ }
+ require(
+ success,
+ "TRANSFER_FAILED"
);
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index b207b3e57..c0ed023ac 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -20,11 +20,9 @@ pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
import "./libs/LibConstants.sol";
-import "../../utils/LibBytes/LibBytes.sol";
import "./libs/LibFillResults.sol";
import "./libs/LibOrder.sol";
import "./libs/LibMath.sol";
-import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
@@ -35,14 +33,11 @@ contract MixinExchangeCore is
LibMath,
LibOrder,
LibFillResults,
- LibExchangeErrors,
MAssetProxyDispatcher,
MExchangeCore,
MSignatureValidator,
MTransactions
{
- using LibBytes for bytes;
-
// Mapping of orderHash => amount of takerAsset already bought by maker
mapping (bytes32 => uint256) public filled;
@@ -73,7 +68,7 @@ contract MixinExchangeCore is
// Ensure orderEpoch is monotonically increasing
require(
newOrderEpoch > oldOrderEpoch,
- INVALID_NEW_ORDER_EPOCH
+ "INVALID_NEW_ORDER_EPOCH"
);
// Update orderEpoch
@@ -285,20 +280,20 @@ contract MixinExchangeCore is
// An order can only be filled if its status is FILLABLE.
require(
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
- ORDER_UNFILLABLE
+ "ORDER_UNFILLABLE"
);
// Revert if fill amount is invalid
require(
takerAssetFillAmount != 0,
- INVALID_TAKER_AMOUNT
+ "INVALID_TAKER_AMOUNT"
);
// Validate sender is allowed to fill this order
if (order.senderAddress != address(0)) {
require(
order.senderAddress == msg.sender,
- INVALID_SENDER
+ "INVALID_SENDER"
);
}
@@ -306,7 +301,7 @@ contract MixinExchangeCore is
if (order.takerAddress != address(0)) {
require(
order.takerAddress == takerAddress,
- INVALID_TAKER
+ "INVALID_TAKER"
);
}
@@ -318,7 +313,7 @@ contract MixinExchangeCore is
order.makerAddress,
signature
),
- INVALID_ORDER_SIGNATURE
+ "INVALID_ORDER_SIGNATURE"
);
}
@@ -329,7 +324,7 @@ contract MixinExchangeCore is
order.takerAssetAmount,
order.makerAssetAmount
),
- ROUNDING_ERROR
+ "ROUNDING_ERROR"
);
}
@@ -347,14 +342,14 @@ contract MixinExchangeCore is
// An order can only be cancelled if its status is FILLABLE.
require(
orderInfo.orderStatus == uint8(OrderStatus.FILLABLE),
- ORDER_UNFILLABLE
+ "ORDER_UNFILLABLE"
);
// Validate sender is allowed to cancel this order
if (order.senderAddress != address(0)) {
require(
order.senderAddress == msg.sender,
- INVALID_SENDER
+ "INVALID_SENDER"
);
}
@@ -362,7 +357,7 @@ contract MixinExchangeCore is
address makerAddress = getCurrentContextAddress();
require(
order.makerAddress == makerAddress,
- INVALID_MAKER
+ "INVALID_MAKER"
);
}
@@ -412,33 +407,27 @@ contract MixinExchangeCore is
)
private
{
- uint8 makerAssetProxyId = uint8(order.makerAssetData.popLastByte());
- uint8 takerAssetProxyId = uint8(order.takerAssetData.popLastByte());
bytes memory zrxAssetData = ZRX_ASSET_DATA;
dispatchTransferFrom(
order.makerAssetData,
- makerAssetProxyId,
order.makerAddress,
takerAddress,
fillResults.makerAssetFilledAmount
);
dispatchTransferFrom(
order.takerAssetData,
- takerAssetProxyId,
takerAddress,
order.makerAddress,
fillResults.takerAssetFilledAmount
);
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
order.makerAddress,
order.feeRecipientAddress,
fillResults.makerFeePaid
);
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
takerAddress,
order.feeRecipientAddress,
fillResults.takerFeePaid
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
index e36fcc2c5..1a43eec79 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol
@@ -15,11 +15,9 @@ pragma solidity ^0.4.24;
pragma experimental ABIEncoderV2;
import "./libs/LibConstants.sol";
-import "../../utils/LibBytes/LibBytes.sol";
import "./libs/LibMath.sol";
import "./libs/LibOrder.sol";
import "./libs/LibFillResults.sol";
-import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
import "./mixins/MMatchOrders.sol";
import "./mixins/MTransactions.sol";
@@ -28,14 +26,11 @@ import "./mixins/MAssetProxyDispatcher.sol";
contract MixinMatchOrders is
LibConstants,
LibMath,
- LibExchangeErrors,
MAssetProxyDispatcher,
MExchangeCore,
MMatchOrders,
MTransactions
{
- using LibBytes for bytes;
-
/// @dev Match two complementary orders that have a profitable spread.
/// Each order is filled at their respective price point. However, the calculations are
/// carried out as though the orders are both being filled at the right order's price point.
@@ -144,7 +139,7 @@ contract MixinMatchOrders is
require(
safeMul(leftOrder.makerAssetAmount, rightOrder.makerAssetAmount) >=
safeMul(leftOrder.takerAssetAmount, rightOrder.takerAssetAmount),
- NEGATIVE_SPREAD_REQUIRED
+ "NEGATIVE_SPREAD_REQUIRED"
);
}
@@ -242,27 +237,22 @@ contract MixinMatchOrders is
)
private
{
- uint8 leftMakerAssetProxyId = uint8(leftOrder.makerAssetData.popLastByte());
- uint8 rightMakerAssetProxyId = uint8(rightOrder.makerAssetData.popLastByte());
bytes memory zrxAssetData = ZRX_ASSET_DATA;
// Order makers and taker
dispatchTransferFrom(
leftOrder.makerAssetData,
- leftMakerAssetProxyId,
leftOrder.makerAddress,
rightOrder.makerAddress,
matchedFillResults.right.takerAssetFilledAmount
);
dispatchTransferFrom(
rightOrder.makerAssetData,
- rightMakerAssetProxyId,
rightOrder.makerAddress,
leftOrder.makerAddress,
matchedFillResults.left.takerAssetFilledAmount
);
dispatchTransferFrom(
leftOrder.makerAssetData,
- leftMakerAssetProxyId,
leftOrder.makerAddress,
takerAddress,
matchedFillResults.leftMakerAssetSpreadAmount
@@ -271,14 +261,12 @@ contract MixinMatchOrders is
// Maker fees
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
leftOrder.makerAddress,
leftOrder.feeRecipientAddress,
matchedFillResults.left.makerFeePaid
);
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
rightOrder.makerAddress,
rightOrder.feeRecipientAddress,
matchedFillResults.right.makerFeePaid
@@ -288,7 +276,6 @@ contract MixinMatchOrders is
if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) {
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
takerAddress,
leftOrder.feeRecipientAddress,
safeAdd(
@@ -299,14 +286,12 @@ contract MixinMatchOrders is
} else {
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
takerAddress,
leftOrder.feeRecipientAddress,
matchedFillResults.left.takerFeePaid
);
dispatchTransferFrom(
zrxAssetData,
- ZRX_PROXY_ID,
takerAddress,
rightOrder.feeRecipientAddress,
matchedFillResults.right.takerFeePaid
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
index cbb55bfce..29172057a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol
@@ -19,23 +19,17 @@
pragma solidity ^0.4.24;
import "../../utils/LibBytes/LibBytes.sol";
-import "./libs/LibExchangeErrors.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
import "./interfaces/IWallet.sol";
import "./interfaces/IValidator.sol";
contract MixinSignatureValidator is
- LibExchangeErrors,
MSignatureValidator,
MTransactions
{
using LibBytes for bytes;
- // Personal message headers
- string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32";
- string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20";
-
// Mapping of hash => signer => signed
mapping (bytes32 => mapping (address => bool)) public preSigned;
@@ -59,7 +53,7 @@ contract MixinSignatureValidator is
signerAddress,
signature
),
- INVALID_SIGNATURE
+ "INVALID_SIGNATURE"
);
preSigned[hash][signerAddress] = true;
}
@@ -99,14 +93,14 @@ contract MixinSignatureValidator is
// TODO: Domain separation: make hash depend on role. (Taker sig should not be valid as maker sig, etc.)
require(
signature.length > 0,
- LENGTH_GREATER_THAN_0_REQUIRED
+ "LENGTH_GREATER_THAN_0_REQUIRED"
);
// Ensure signature is supported
uint8 signatureTypeRaw = uint8(signature.popLastByte());
require(
signatureTypeRaw < uint8(SignatureType.NSignatureTypes),
- SIGNATURE_UNSUPPORTED
+ "SIGNATURE_UNSUPPORTED"
);
// Pop last byte off of signature byte array.
@@ -124,7 +118,7 @@ contract MixinSignatureValidator is
// it an explicit option. This aids testing and analysis. It is
// also the initialization value for the enum type.
if (signatureType == SignatureType.Illegal) {
- revert(SIGNATURE_ILLEGAL);
+ revert("SIGNATURE_ILLEGAL");
// Always invalid signature.
// Like Illegal, this is always implicitly available and therefore
@@ -133,7 +127,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Invalid) {
require(
signature.length == 0,
- LENGTH_0_REQUIRED
+ "LENGTH_0_REQUIRED"
);
isValid = false;
return isValid;
@@ -142,7 +136,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.EIP712) {
require(
signature.length == 65,
- LENGTH_65_REQUIRED
+ "LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
@@ -155,13 +149,16 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.EthSign) {
require(
signature.length == 65,
- LENGTH_65_REQUIRED
+ "LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
- keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)),
+ keccak256(abi.encodePacked(
+ "\x19Ethereum Signed Message:\n32",
+ hash
+ )),
v,
r,
s
@@ -180,7 +177,7 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Caller) {
require(
signature.length == 0,
- LENGTH_0_REQUIRED
+ "LENGTH_0_REQUIRED"
);
isValid = signerAddress == msg.sender;
return isValid;
@@ -230,13 +227,16 @@ contract MixinSignatureValidator is
} else if (signatureType == SignatureType.Trezor) {
require(
signature.length == 65,
- LENGTH_65_REQUIRED
+ "LENGTH_65_REQUIRED"
);
v = uint8(signature[0]);
r = signature.readBytes32(1);
s = signature.readBytes32(33);
recovered = ecrecover(
- keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)),
+ keccak256(abi.encodePacked(
+ "\x19Ethereum Signed Message:\n\x20",
+ hash
+ )),
v,
r,
s
@@ -250,6 +250,6 @@ contract MixinSignatureValidator is
// that we currently support. In this case returning false
// may lead the caller to incorrectly believe that the
// signature was invalid.)
- revert(SIGNATURE_UNSUPPORTED);
+ revert("SIGNATURE_UNSUPPORTED");
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
index 20a4a12df..e0f450d0a 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol
@@ -20,12 +20,10 @@ pragma solidity ^0.4.24;
import "./libs/LibExchangeErrors.sol";
import "./mixins/MSignatureValidator.sol";
import "./mixins/MTransactions.sol";
-import "./libs/LibExchangeErrors.sol";
import "./libs/LibEIP712.sol";
contract MixinTransactions is
LibEIP712,
- LibExchangeErrors,
MSignatureValidator,
MTransactions
{
@@ -90,7 +88,7 @@ contract MixinTransactions is
// Prevent reentrancy
require(
currentContextAddress == address(0),
- REENTRANCY_ILLEGAL
+ "REENTRANCY_ILLEGAL"
);
bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(
@@ -102,7 +100,7 @@ contract MixinTransactions is
// Validate transaction has not been executed
require(
!transactions[transactionHash],
- INVALID_TX_HASH
+ "INVALID_TX_HASH"
);
// Transaction always valid if signer is sender of transaction
@@ -114,7 +112,7 @@ contract MixinTransactions is
signerAddress,
signature
),
- INVALID_TX_SIGNATURE
+ "INVALID_TX_SIGNATURE"
);
// Set the current transaction signer
@@ -125,7 +123,7 @@ contract MixinTransactions is
transactions[transactionHash] = true;
require(
address(this).delegatecall(data),
- FAILED_EXECUTION
+ "FAILED_EXECUTION"
);
// Reset current transaction signer
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
index 724f95518..00668ca43 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
@@ -22,13 +22,11 @@ pragma experimental ABIEncoderV2;
import "./libs/LibMath.sol";
import "./libs/LibOrder.sol";
import "./libs/LibFillResults.sol";
-import "./libs/LibExchangeErrors.sol";
import "./mixins/MExchangeCore.sol";
contract MixinWrapperFunctions is
LibMath,
LibFillResults,
- LibExchangeErrors,
MExchangeCore
{
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
@@ -50,7 +48,7 @@ contract MixinWrapperFunctions is
);
require(
fillResults.takerAssetFilledAmount == takerAssetFillAmount,
- COMPLETE_FILL_FAILED
+ "COMPLETE_FILL_FAILED"
);
return fillResults;
}
@@ -366,14 +364,6 @@ contract MixinWrapperFunctions is
signatures[i]
);
- // HACK: the proxyId is "popped" from the byte array before a fill is settled
- // by subtracting from the length of the array. Since the popped byte is
- // still in memory, we can "unpop" it by incrementing the length of the byte array.
- assembly {
- let len := mload(takerAssetData)
- mstore(takerAssetData, add(len, 1))
- }
-
// Update amounts filled and fees paid by maker and taker
addFillResults(totalFillResults, singleFillResults);
@@ -467,14 +457,6 @@ contract MixinWrapperFunctions is
signatures[i]
);
- // HACK: the proxyId is "popped" from the byte array before a fill is settled
- // by subtracting from the length of the array. Since the popped byte is
- // still in memory, we can "unpop" it by incrementing the length of the byte array.
- assembly {
- let len := mload(makerAssetData)
- mstore(makerAssetData, add(len, 1))
- }
-
// Update amounts filled and fees paid by maker and taker
addFillResults(totalFillResults, singleFillResults);
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol
index 2c331dc34..fa55dff00 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IAssetProxyDispatcher.sol
@@ -26,7 +26,7 @@ contract IAssetProxyDispatcher {
/// @param newAssetProxy Address of new asset proxy to register, or 0x0 to unset assetProxyId.
/// @param oldAssetProxy Existing asset proxy to overwrite, or 0x0 if assetProxyId is currently unused.
function registerAssetProxy(
- uint8 assetProxyId,
+ bytes4 assetProxyId,
address newAssetProxy,
address oldAssetProxy
)
@@ -35,7 +35,7 @@ contract IAssetProxyDispatcher {
/// @dev Gets an asset proxy.
/// @param assetProxyId Id of the asset proxy.
/// @return The asset proxy registered to assetProxyId. Returns 0x0 if no proxy is registered.
- function getAssetProxy(uint8 assetProxyId)
+ function getAssetProxy(bytes4 assetProxyId)
external
view
returns (address);
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol
index 4a9452448..488ca956c 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol
@@ -25,9 +25,6 @@ contract LibConstants {
// not constant to make testing easier.
bytes public ZRX_ASSET_DATA;
- // Proxy Id for ZRX token.
- uint8 constant ZRX_PROXY_ID = 1;
-
// @TODO: Remove when we deploy.
constructor (bytes memory zrxAssetData)
public
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
index a43f0f927..e37f41ada 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol
@@ -18,7 +18,10 @@
pragma solidity ^0.4.24;
+/// @dev This contract documents the revert reasons used in the Exchange contract.
+/// This contract is intended to serve as a reference, but is not actually used for efficiency reasons.
contract LibExchangeErrors {
+
/// Order validation errors ///
string constant ORDER_UNFILLABLE = "ORDER_UNFILLABLE"; // Order cannot be filled.
string constant INVALID_MAKER = "INVALID_MAKER"; // Invalid makerAddress.
@@ -56,9 +59,11 @@ contract LibExchangeErrors {
/// dispatchTransferFrom errors ///
string constant ASSET_PROXY_DOES_NOT_EXIST = "ASSET_PROXY_DOES_NOT_EXIST"; // No assetProxy registered at given id.
+ string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Asset transfer unsuccesful.
/// Length validation errors ///
string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0.
+ string constant LENGTH_GREATER_THAN_3_REQUIRED = "LENGTH_GREATER_THAN_3_REQUIRED"; // Byte array must have a length greater than 3.
string constant LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0.
string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65.
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
index 233547b9f..bfe2fd33f 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibMath.sol
@@ -23,7 +23,6 @@ import "../../../utils/SafeMath/SafeMath.sol";
contract LibMath is
SafeMath
{
- string constant ROUNDING_ERROR_ON_PARTIAL_AMOUNT = "A rounding error occurred when calculating partial transfer amounts.";
/// @dev Calculates partial value given a numerator and denominator.
/// @param numerator Numerator.
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
index 788f42c60..c2b506dcf 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
@@ -27,20 +27,18 @@ contract MAssetProxyDispatcher is
// Logs registration of new asset proxy
event AssetProxySet(
- uint8 id, // Id of new registered AssetProxy.
+ bytes4 id, // Id of new registered AssetProxy.
address newAssetProxy, // Address of new registered AssetProxy.
address oldAssetProxy // Address of AssetProxy that was overwritten at given id (or null address).
);
/// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws.
- /// @param assetData Byte array encoded for the respective asset proxy.
- /// @param assetProxyId Id of assetProxy to dispach to.
+ /// @param assetData Byte array encoded for the asset.
/// @param from Address to transfer token from.
/// @param to Address to transfer token to.
/// @param amount Amount of token to transfer.
function dispatchTransferFrom(
bytes memory assetData,
- uint8 assetProxyId,
address from,
address to,
uint256 amount
diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol
index d469a07f0..2ae69e0ef 100644
--- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol
+++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol
@@ -24,12 +24,11 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol";
contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher {
function publicDispatchTransferFrom(
bytes memory assetData,
- uint8 assetProxyId,
address from,
address to,
uint256 amount)
public
{
- dispatchTransferFrom(assetData, assetProxyId, from, to, amount);
+ dispatchTransferFrom(assetData, from, to, amount);
}
}
diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
index e4cbf318b..78b1ddf7c 100644
--- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol
@@ -19,17 +19,8 @@
pragma solidity ^0.4.24;
library LibBytes {
- using LibBytes for bytes;
- // Revert reasons
- string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED";
- string constant GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED";
- string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED";
- string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED";
- string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED";
- string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED";
- string constant FROM_LESS_THAN_TO_REQUIRED = "FROM_LESS_THAN_TO_REQUIRED";
- string constant TO_LESS_THAN_LENGTH_REQUIRED = "TO_LESS_THAN_LENGTH_REQUIRED";
+ using LibBytes for bytes;
/// @dev Gets the memory address for a byte array.
/// @param input Byte array to lookup.
@@ -176,8 +167,14 @@ library LibBytes {
pure
returns (bytes memory result)
{
- require(from <= to, FROM_LESS_THAN_TO_REQUIRED);
- require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED);
+ require(
+ from <= to,
+ "FROM_LESS_THAN_TO_REQUIRED"
+ );
+ require(
+ to < b.length,
+ "TO_LESS_THAN_LENGTH_REQUIRED"
+ );
// Create a new bytes structure and copy contents
result = new bytes(to - from);
@@ -199,8 +196,14 @@ library LibBytes {
pure
returns (bytes memory result)
{
- require(from <= to, FROM_LESS_THAN_TO_REQUIRED);
- require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED);
+ require(
+ from <= to,
+ "FROM_LESS_THAN_TO_REQUIRED"
+ );
+ require(
+ to < b.length,
+ "TO_LESS_THAN_LENGTH_REQUIRED"
+ );
// Create a new bytes structure around [from, to) in-place.
assembly {
@@ -220,7 +223,7 @@ library LibBytes {
{
require(
b.length > 0,
- GREATER_THAN_ZERO_LENGTH_REQUIRED
+ "GREATER_THAN_ZERO_LENGTH_REQUIRED"
);
// Store last byte.
@@ -244,7 +247,7 @@ library LibBytes {
{
require(
b.length >= 20,
- GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"
);
// Store last 20 bytes.
@@ -290,7 +293,7 @@ library LibBytes {
{
require(
b.length >= index + 20, // 20 is length of address
- GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"
);
// Add offset to index:
@@ -322,7 +325,7 @@ library LibBytes {
{
require(
b.length >= index + 20, // 20 is length of address
- GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"
);
// Add offset to index:
@@ -365,7 +368,7 @@ library LibBytes {
{
require(
b.length >= index + 32,
- GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"
);
// Arrays are prefixed by a 256 bit length parameter
@@ -392,7 +395,7 @@ library LibBytes {
{
require(
b.length >= index + 32,
- GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"
);
// Arrays are prefixed by a 256 bit length parameter
@@ -447,7 +450,7 @@ library LibBytes {
{
require(
b.length >= index + 4,
- GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED"
);
assembly {
result := mload(add(b, 32))
@@ -459,6 +462,8 @@ library LibBytes {
}
/// @dev Reads nested bytes from a specific position.
+ /// @dev NOTE: the returned value overlaps with the input value.
+ /// Both should be treated as immutable.
/// @param b Byte array containing nested bytes.
/// @param index Index of nested bytes.
/// @return result Nested bytes.
@@ -478,17 +483,13 @@ library LibBytes {
// length of nested bytes
require(
b.length >= index + nestedBytesLength,
- GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"
);
-
- // Allocate memory and copy value to result
- result = new bytes(nestedBytesLength);
- memCopy(
- result.contentAddress(),
- b.contentAddress() + index,
- nestedBytesLength
- );
-
+
+ // Return a pointer to the byte array as it exists inside `b`
+ assembly {
+ result := add(b, index)
+ }
return result;
}
@@ -508,7 +509,7 @@ library LibBytes {
// length of input
require(
b.length >= index + 32 /* 32 bytes to store length */ + input.length,
- GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"
);
// Copy <input> into <b>
@@ -533,7 +534,7 @@ library LibBytes {
// Dest length must be >= source length, or some bytes would not be copied.
require(
dest.length >= sourceLen,
- GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED
+ "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"
);
memCopy(
dest.contentAddress(),
diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
index 99b93d0d2..6f5761cc7 100644
--- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
+++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol
@@ -13,9 +13,6 @@ import "./IOwnable.sol";
contract Ownable is IOwnable {
address public owner;
- // Revert reasons
- string constant ONLY_CONTRACT_OWNER = "ONLY_CONTRACT_OWNER";
-
constructor ()
public
{
@@ -25,7 +22,7 @@ contract Ownable is IOwnable {
modifier onlyOwner() {
require(
msg.sender == owner,
- ONLY_CONTRACT_OWNER
+ "ONLY_CONTRACT_OWNER"
);
_;
}
diff --git a/packages/contracts/test/asset_proxy/decoder.ts b/packages/contracts/test/asset_proxy/decoder.ts
index 98d18aa38..902255caf 100644
--- a/packages/contracts/test/asset_proxy/decoder.ts
+++ b/packages/contracts/test/asset_proxy/decoder.ts
@@ -44,7 +44,6 @@ describe('TestAssetDataDecoders', () => {
it('should correctly decode ERC721 asset data', async () => {
const tokenId = generatePseudoRandomSalt();
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(testAddress, tokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
const expectedDecodedAssetData = assetProxyUtils.decodeERC721AssetData(encodedAssetData);
let decodedTokenAddress: string;
let decodedTokenId: BigNumber;
@@ -53,7 +52,7 @@ describe('TestAssetDataDecoders', () => {
decodedTokenAddress,
decodedTokenId,
decodedData,
- ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetDataWithoutProxyId);
+ ] = await testAssetProxyDecoder.publicDecodeERC721Data.callAsync(encodedAssetData);
expect(decodedTokenAddress).to.be.equal(expectedDecodedAssetData.tokenAddress);
expect(decodedTokenId).to.be.bignumber.equal(expectedDecodedAssetData.tokenId);
expect(decodedData).to.be.equal(expectedDecodedAssetData.receiverData);
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 2c27f7382..2313c0b0e 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -96,13 +96,12 @@ describe('Asset Transfer Proxies', () => {
it('should successfully transfer tokens', async () => {
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Perform a transfer from makerAddress to takerAddress
const erc20Balances = await erc20Wrapper.getBalancesAsync();
const amount = new BigNumber(10);
await web3Wrapper.awaitTransactionSuccessAsync(
await erc20Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -123,13 +122,12 @@ describe('Asset Transfer Proxies', () => {
it('should do nothing if transferring 0 amount of a token', async () => {
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Perform a transfer from makerAddress to takerAddress
const erc20Balances = await erc20Wrapper.getBalancesAsync();
const amount = new BigNumber(0);
await web3Wrapper.awaitTransactionSuccessAsync(
await erc20Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -174,19 +172,13 @@ describe('Asset Transfer Proxies', () => {
it('should throw if requesting address is not authorized', async () => {
// Construct ERC20 asset data
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
+
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(10);
return expectRevertOrAlwaysFailingTransactionAsync(
- erc20Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
- makerAddress,
- takerAddress,
- amount,
- {
- from: notAuthorized,
- },
- ),
+ erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+ from: notAuthorized,
+ }),
);
});
});
@@ -196,10 +188,9 @@ describe('Asset Transfer Proxies', () => {
const erc20Balances = await erc20Wrapper.getBalancesAsync();
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
const amount = new BigNumber(10);
const numTransfers = 2;
- const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId);
+ const assetData = _.times(numTransfers, () => encodedAssetData);
const fromAddresses = _.times(numTransfers, () => makerAddress);
const toAddresses = _.times(numTransfers, () => takerAddress);
const amounts = _.times(numTransfers, () => amount);
@@ -228,10 +219,9 @@ describe('Asset Transfer Proxies', () => {
it('should throw if not called by an authorized address', async () => {
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
const amount = new BigNumber(10);
const numTransfers = 2;
- const assetData = _.times(numTransfers, () => encodedAssetDataWithoutProxyId);
+ const assetData = _.times(numTransfers, () => encodedAssetData);
const fromAddresses = _.times(numTransfers, () => makerAddress);
const toAddresses = _.times(numTransfers, () => takerAddress);
const amounts = _.times(numTransfers, () => amount);
@@ -244,9 +234,10 @@ describe('Asset Transfer Proxies', () => {
});
});
- it('should have an id of 1', async () => {
+ it('should have an id of 0xf47261b0', async () => {
const proxyId = await erc20Proxy.getProxyId.callAsync();
- expect(proxyId).to.equal(1);
+ const expectedProxyId = '0xf47261b0';
+ expect(proxyId).to.equal(expectedProxyId);
});
});
@@ -255,7 +246,6 @@ describe('Asset Transfer Proxies', () => {
it('should successfully transfer tokens', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -263,7 +253,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(1);
await web3Wrapper.awaitTransactionSuccessAsync(
await erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -279,14 +269,13 @@ describe('Asset Transfer Proxies', () => {
it('should call onERC721Received when transferring to a smart contract without receiver data', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const txHash = await erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
erc721Receiver.address,
amount,
@@ -315,14 +304,13 @@ describe('Asset Transfer Proxies', () => {
erc721MakerTokenId,
receiverData,
);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const txHash = await erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
erc721Receiver.address,
amount,
@@ -351,7 +339,6 @@ describe('Asset Transfer Proxies', () => {
erc721MakerTokenId,
receiverData,
);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -359,7 +346,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(1);
return expectRevertOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
erc20Proxy.address, // the ERC20 proxy does not have an ERC721 receiver
amount,
@@ -371,7 +358,6 @@ describe('Asset Transfer Proxies', () => {
it('should throw if transferring 0 amount of a token', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -379,7 +365,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(0);
return expectRevertOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -391,7 +377,6 @@ describe('Asset Transfer Proxies', () => {
it('should throw if transferring > 1 amount of a token', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
@@ -399,7 +384,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(500);
return expectRevertOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -411,7 +396,6 @@ describe('Asset Transfer Proxies', () => {
it('should throw if allowances are too low', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Remove transfer approval for makerAddress.
await web3Wrapper.awaitTransactionSuccessAsync(
await erc721Token.setApprovalForAll.sendTransactionAsync(erc721Proxy.address, false, {
@@ -422,27 +406,20 @@ describe('Asset Transfer Proxies', () => {
// Perform a transfer; expect this to fail.
const amount = new BigNumber(1);
return expectRevertOrAlwaysFailingTransactionAsync(
- erc20Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
- makerAddress,
- takerAddress,
- amount,
- {
- from: notAuthorized,
- },
- ),
+ erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+ from: notAuthorized,
+ }),
);
});
it('should throw if requesting address is not authorized', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetProxyUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
return expectRevertOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -459,8 +436,8 @@ describe('Asset Transfer Proxies', () => {
const numTransfers = 2;
const assetData = [
- assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA).slice(0, -2),
- assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB).slice(0, -2),
+ assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdA),
+ assetProxyUtils.encodeERC721AssetData(erc721Token.address, makerTokenIdB),
];
const fromAddresses = _.times(numTransfers, () => makerAddress);
const toAddresses = _.times(numTransfers, () => takerAddress);
@@ -506,9 +483,10 @@ describe('Asset Transfer Proxies', () => {
});
});
- it('should have an id of 2', async () => {
+ it('should have an id of 0x08e937fa', async () => {
const proxyId = await erc721Proxy.getProxyId.callAsync();
- expect(proxyId).to.equal(2);
+ const expectedProxyId = '0x08e937fa';
+ expect(proxyId).to.equal(expectedProxyId);
});
});
});
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index ff652d3aa..5b2d4590f 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -86,7 +86,7 @@ describe('Exchange core', () => {
artifacts.Exchange,
provider,
txDefaults,
- zrxToken.address,
+ assetProxyUtils.encodeERC20AssetData(zrxToken.address),
);
exchangeWrapper = new ExchangeWrapper(exchange, provider);
await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts
index abbfd7ac7..f96f15f16 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -276,14 +276,13 @@ describe('AssetProxyDispatcher', () => {
);
// Construct metadata for ERC20 proxy
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
+
// Perform a transfer from makerAddress to takerAddress
const erc20Balances = await erc20Wrapper.getBalancesAsync();
const amount = new BigNumber(10);
await web3Wrapper.awaitTransactionSuccessAsync(
await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
- AssetProxyId.ERC20,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -304,13 +303,11 @@ describe('AssetProxyDispatcher', () => {
it('should throw if dispatching to unregistered proxy', async () => {
// Construct metadata for ERC20 proxy
const encodedAssetData = assetProxyUtils.encodeERC20AssetData(zrxToken.address);
- const encodedAssetDataWithoutProxyId = encodedAssetData.slice(0, -2);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(10);
return expectRevertOrAlwaysFailingTransactionAsync(
assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
- AssetProxyId.ERC20,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index b8dca04fd..18a46187f 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -96,7 +96,7 @@ describe('matchOrders', () => {
artifacts.Exchange,
provider,
txDefaults,
- zrxToken.address,
+ assetProxyUtils.encodeERC20AssetData(zrxToken.address),
);
exchangeWrapper = new ExchangeWrapper(exchange, provider);
await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/contracts/test/exchange/transactions.ts b/packages/contracts/test/exchange/transactions.ts
index 9a625880c..43b9f46b2 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -73,7 +73,7 @@ describe('Exchange transactions', () => {
artifacts.Exchange,
provider,
txDefaults,
- zrxToken.address,
+ assetProxyUtils.encodeERC20AssetData(zrxToken.address),
);
exchangeWrapper = new ExchangeWrapper(exchange, provider);
await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index 703f644b8..a39a81057 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -80,7 +80,7 @@ describe('Exchange wrappers', () => {
artifacts.Exchange,
provider,
txDefaults,
- zrxToken.address,
+ assetProxyUtils.encodeERC20AssetData(zrxToken.address),
);
exchangeWrapper = new ExchangeWrapper(exchange, provider);
await exchangeWrapper.registerAssetProxyAsync(AssetProxyId.ERC20, erc20Proxy.address, owner);
diff --git a/packages/order-utils/src/asset_proxy_utils.ts b/packages/order-utils/src/asset_proxy_utils.ts
index 915ee5032..8140ad89d 100644
--- a/packages/order-utils/src/asset_proxy_utils.ts
+++ b/packages/order-utils/src/asset_proxy_utils.ts
@@ -1,32 +1,43 @@
import { AssetProxyId, ERC20AssetData, ERC721AssetData } from '@0xproject/types';
-import { BigNumber, NULL_BYTES } from '@0xproject/utils';
+import { BigNumber } from '@0xproject/utils';
import BN = require('bn.js');
import ethUtil = require('ethereumjs-util');
-import * as _ from 'lodash';
-const ERC20_ASSET_DATA_BYTE_LENGTH = 21;
-const ERC721_ASSET_DATA_MINIMUM_BYTE_LENGTH = 53;
-const ASSET_DATA_ADDRESS_OFFSET = 0;
-const ERC721_ASSET_DATA_TOKEN_ID_OFFSET = 20;
-const ERC721_ASSET_DATA_RECEIVER_DATA_LENGTH_OFFSET = 52;
-const ERC721_ASSET_DATA_RECEIVER_DATA_OFFSET = 84;
+import { constants } from './constants';
+
+// TODO: Push upstream to DefinitelyTyped
+interface EthAbi {
+ simpleEncode(signature: string, ...args: any[]): Buffer;
+ rawDecode(signature: string[], data: Buffer): any[];
+}
+// tslint:disable:no-var-requires
+const ethAbi = require('ethereumjs-abi') as EthAbi;
export const assetProxyUtils = {
encodeAssetProxyId(assetProxyId: AssetProxyId): Buffer {
return ethUtil.toBuffer(assetProxyId);
},
decodeAssetProxyId(encodedAssetProxyId: Buffer): AssetProxyId {
- return ethUtil.bufferToInt(encodedAssetProxyId);
+ const hexString = ethUtil.bufferToHex(encodedAssetProxyId);
+ if (hexString === AssetProxyId.ERC20) {
+ return AssetProxyId.ERC20;
+ }
+ if (hexString === AssetProxyId.ERC721) {
+ return AssetProxyId.ERC721;
+ }
+ throw new Error(`Invalid ProxyId: ${hexString}`);
},
encodeAddress(address: string): Buffer {
if (!ethUtil.isValidAddress(address)) {
throw new Error(`Invalid Address: ${address}`);
}
const encodedAddress = ethUtil.toBuffer(address);
- return encodedAddress;
+ const padded = ethUtil.setLengthLeft(encodedAddress, constants.WORD_LENGTH);
+ return padded;
},
decodeAddress(encodedAddress: Buffer): string {
- const address = ethUtil.bufferToHex(encodedAddress);
+ const unpadded = ethUtil.setLengthLeft(encodedAddress, constants.ADDRESS_LENGTH);
+ const address = ethUtil.bufferToHex(unpadded);
if (!ethUtil.isValidAddress(address)) {
throw new Error(`Invalid Address: ${address}`);
}
@@ -37,33 +48,27 @@ export const assetProxyUtils = {
const formattedValue = new BN(value.toString(base));
const encodedValue = ethUtil.toBuffer(formattedValue);
// tslint:disable-next-line:custom-no-magic-numbers
- const paddedValue = ethUtil.setLengthLeft(encodedValue, 32);
+ const paddedValue = ethUtil.setLengthLeft(encodedValue, constants.WORD_LENGTH);
return paddedValue;
},
decodeUint256(encodedValue: Buffer): BigNumber {
const formattedValue = ethUtil.bufferToHex(encodedValue);
- const value = new BigNumber(formattedValue, 16);
+ const value = new BigNumber(formattedValue, constants.BASE_16);
return value;
},
encodeERC20AssetData(tokenAddress: string): string {
- const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC20);
- const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress);
- const encodedAssetData = Buffer.concat([encodedAddress, encodedAssetProxyId]);
- const encodedAssetDataHex = ethUtil.bufferToHex(encodedAssetData);
- return encodedAssetDataHex;
+ return ethUtil.bufferToHex(ethAbi.simpleEncode('ERC20Token(address)', tokenAddress));
},
- decodeERC20AssetData(proxyData: string): ERC20AssetData {
- const encodedAssetData = ethUtil.toBuffer(proxyData);
- if (encodedAssetData.byteLength !== ERC20_ASSET_DATA_BYTE_LENGTH) {
+ decodeERC20AssetData(assetData: string): ERC20AssetData {
+ const data = ethUtil.toBuffer(assetData);
+ if (data.byteLength < constants.ERC20_ASSET_DATA_BYTE_LENGTH) {
throw new Error(
- `Could not decode ERC20 Proxy Data. Expected length of encoded data to be 21. Got ${
- encodedAssetData.byteLength
- }`,
+ `Could not decode ERC20 Proxy Data. Expected length of encoded data to be at least ${
+ constants.ERC20_ASSET_DATA_BYTE_LENGTH
+ }. Got ${data.byteLength}`,
);
}
- const assetProxyIdOffset = encodedAssetData.byteLength - 1;
- const encodedAssetProxyId = encodedAssetData.slice(assetProxyIdOffset);
- const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
+ const assetProxyId = ethUtil.bufferToHex(data.slice(0, constants.SELECTOR_LENGTH));
if (assetProxyId !== AssetProxyId.ERC20) {
throw new Error(
`Could not decode ERC20 Proxy Data. Expected Asset Proxy Id to be ERC20 (${
@@ -71,98 +76,61 @@ export const assetProxyUtils = {
}), but got ${assetProxyId}`,
);
}
- const encodedTokenAddress = encodedAssetData.slice(ASSET_DATA_ADDRESS_OFFSET, assetProxyIdOffset);
- const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress);
- const erc20AssetData = {
+ const [tokenAddress] = ethAbi.rawDecode(['address'], data.slice(constants.SELECTOR_LENGTH));
+ return {
assetProxyId,
- tokenAddress,
+ tokenAddress: ethUtil.addHexPrefix(tokenAddress),
};
- return erc20AssetData;
},
encodeERC721AssetData(tokenAddress: string, tokenId: BigNumber, receiverData?: string): string {
- const encodedAssetProxyId = assetProxyUtils.encodeAssetProxyId(AssetProxyId.ERC721);
- const encodedAddress = assetProxyUtils.encodeAddress(tokenAddress);
- const encodedTokenId = assetProxyUtils.encodeUint256(tokenId);
- let encodedAssetData = Buffer.concat([encodedAddress, encodedTokenId]);
- if (!_.isUndefined(receiverData)) {
- const encodedReceiverData = ethUtil.toBuffer(receiverData);
- const receiverDataLength = new BigNumber(encodedReceiverData.byteLength);
- const encodedReceiverDataLength = assetProxyUtils.encodeUint256(receiverDataLength);
- encodedAssetData = Buffer.concat([encodedAssetData, encodedReceiverDataLength, encodedReceiverData]);
- }
- encodedAssetData = Buffer.concat([encodedAssetData, encodedAssetProxyId]);
- const encodedAssetDataHex = ethUtil.bufferToHex(encodedAssetData);
- return encodedAssetDataHex;
+ // TODO: Pass `tokendId` as a BigNumber.
+ return ethUtil.bufferToHex(
+ ethAbi.simpleEncode(
+ 'ERC721Token(address,uint256,bytes)',
+ tokenAddress,
+ `0x${tokenId.toString(constants.BASE_16)}`,
+ ethUtil.toBuffer(receiverData || '0x'),
+ ),
+ );
},
decodeERC721AssetData(assetData: string): ERC721AssetData {
- const encodedAssetData = ethUtil.toBuffer(assetData);
- if (encodedAssetData.byteLength < ERC721_ASSET_DATA_MINIMUM_BYTE_LENGTH) {
+ const data = ethUtil.toBuffer(assetData);
+ if (data.byteLength < constants.ERC721_ASSET_DATA_MINIMUM_BYTE_LENGTH) {
throw new Error(
- `Could not decode ERC20 Proxy Data. Expected length of encoded data to be at least 53. Got ${
- encodedAssetData.byteLength
- }`,
+ `Could not decode ERC721 Asset Data. Expected length of encoded data to be at least ${
+ constants.ERC721_ASSET_DATA_MINIMUM_BYTE_LENGTH
+ }. Got ${data.byteLength}`,
);
}
-
- const encodedTokenAddress = encodedAssetData.slice(
- ASSET_DATA_ADDRESS_OFFSET,
- ERC721_ASSET_DATA_TOKEN_ID_OFFSET,
- );
- const proxyIdOffset = encodedAssetData.byteLength - 1;
- const encodedAssetProxyId = encodedAssetData.slice(proxyIdOffset);
- const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
+ const assetProxyId = ethUtil.bufferToHex(data.slice(0, constants.SELECTOR_LENGTH));
if (assetProxyId !== AssetProxyId.ERC721) {
throw new Error(
- `Could not decode ERC721 Proxy Data. Expected Asset Proxy Id to be ERC721 (${
+ `Could not decode ERC721 Asset Data. Expected Asset Proxy Id to be ERC721 (${
AssetProxyId.ERC721
}), but got ${assetProxyId}`,
);
}
- const tokenAddress = assetProxyUtils.decodeAddress(encodedTokenAddress);
- const encodedTokenId = encodedAssetData.slice(
- ERC721_ASSET_DATA_TOKEN_ID_OFFSET,
- ERC721_ASSET_DATA_RECEIVER_DATA_LENGTH_OFFSET,
+ const [tokenAddress, tokenId, receiverData] = ethAbi.rawDecode(
+ ['address', 'uint256', 'bytes'],
+ data.slice(constants.SELECTOR_LENGTH),
);
- const tokenId = assetProxyUtils.decodeUint256(encodedTokenId);
- let receiverData = NULL_BYTES;
- const lengthUpToReceiverDataLength = ERC721_ASSET_DATA_RECEIVER_DATA_LENGTH_OFFSET + 1;
- if (encodedAssetData.byteLength > lengthUpToReceiverDataLength) {
- const encodedReceiverDataLength = encodedAssetData.slice(
- ERC721_ASSET_DATA_RECEIVER_DATA_LENGTH_OFFSET,
- ERC721_ASSET_DATA_RECEIVER_DATA_OFFSET,
- );
- const receiverDataLength = assetProxyUtils.decodeUint256(encodedReceiverDataLength);
- const lengthUpToReceiverData = ERC721_ASSET_DATA_RECEIVER_DATA_OFFSET + 1;
- const expectedReceiverDataLength = new BigNumber(encodedAssetData.byteLength - lengthUpToReceiverData);
- if (!receiverDataLength.equals(expectedReceiverDataLength)) {
- throw new Error(
- `Data length (${receiverDataLength}) does not match actual length of data (${expectedReceiverDataLength})`,
- );
- }
- const encodedReceiverData = encodedAssetData.slice(
- ERC721_ASSET_DATA_RECEIVER_DATA_OFFSET,
- receiverDataLength.add(ERC721_ASSET_DATA_RECEIVER_DATA_OFFSET).toNumber(),
- );
- receiverData = ethUtil.bufferToHex(encodedReceiverData);
- }
- const erc721AssetData: ERC721AssetData = {
+ return {
assetProxyId,
- tokenAddress,
- tokenId,
- receiverData,
+ tokenAddress: ethUtil.addHexPrefix(tokenAddress),
+ tokenId: new BigNumber(tokenId.toString()),
+ receiverData: ethUtil.bufferToHex(receiverData),
};
- return erc721AssetData;
},
decodeAssetDataId(assetData: string): AssetProxyId {
const encodedAssetData = ethUtil.toBuffer(assetData);
- if (encodedAssetData.byteLength < 1) {
+ if (encodedAssetData.byteLength < constants.SELECTOR_LENGTH) {
throw new Error(
- `Could not decode Proxy Data. Expected length of encoded data to be at least 1. Got ${
+ `Could not decode Proxy Data. Expected length of encoded data to be at least 4. Got ${
encodedAssetData.byteLength
}`,
);
}
- const encodedAssetProxyId = encodedAssetData.slice(-1);
+ const encodedAssetProxyId = encodedAssetData.slice(0, constants.SELECTOR_LENGTH);
const assetProxyId = assetProxyUtils.decodeAssetProxyId(encodedAssetProxyId);
return assetProxyId;
},
diff --git a/packages/order-utils/src/constants.ts b/packages/order-utils/src/constants.ts
index ed5bd8101..383a657b8 100644
--- a/packages/order-utils/src/constants.ts
+++ b/packages/order-utils/src/constants.ts
@@ -5,4 +5,10 @@ export const constants = {
// tslint:disable-next-line:custom-no-magic-numbers
UNLIMITED_ALLOWANCE_IN_BASE_UNITS: new BigNumber(2).pow(256).minus(1),
TESTRPC_NETWORK_ID: 50,
+ ADDRESS_LENGTH: 20,
+ WORD_LENGTH: 32,
+ ERC20_ASSET_DATA_BYTE_LENGTH: 36,
+ ERC721_ASSET_DATA_MINIMUM_BYTE_LENGTH: 53,
+ SELECTOR_LENGTH: 4,
+ BASE_16: 16,
};
diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts
index e5a0f41a9..88abdc1e7 100644
--- a/packages/types/src/index.ts
+++ b/packages/types/src/index.ts
@@ -152,18 +152,18 @@ export interface ECSignature {
}
export enum AssetProxyId {
- INVALID,
- ERC20,
- ERC721,
+ INVALID = '0x00000000',
+ ERC20 = '0xf47261b0',
+ ERC721 = '0x08e937fa',
}
export interface ERC20AssetData {
- assetProxyId: AssetProxyId;
+ assetProxyId: string;
tokenAddress: string;
}
export interface ERC721AssetData {
- assetProxyId: AssetProxyId;
+ assetProxyId: string;
tokenAddress: string;
tokenId: BigNumber;
receiverData: string;