aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts
diff options
context:
space:
mode:
Diffstat (limited to 'packages/contracts')
-rw-r--r--packages/contracts/README.md46
-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
33 files changed, 449 insertions, 266 deletions
diff --git a/packages/contracts/README.md b/packages/contracts/README.md
index 9c829c753..2e6376f39 100644
--- a/packages/contracts/README.md
+++ b/packages/contracts/README.md
@@ -64,11 +64,51 @@ yarn lint
yarn test
```
-### Run Tests Against Geth
+#### Testing options
-Follow the instructions in the README for the devnet package to start the
-devnet.
+###### Revert stack traces
+
+If you want to see helpful stack traces (incl. line number, code snippet) for smart contract reverts, run the tests with:
+
+```
+yarn test:trace
+```
+
+**Note:** This currently slows down the test runs and is therefore not enabled by default.
+
+###### Backing Ethereum node
+
+By default, our tests run against an in-process [Ganache](https://github.com/trufflesuite/ganache-core) instance. In order to run the tests against [Geth](https://github.com/ethereum/go-ethereum), first follow the instructions in the README for the devnet package to start the devnet Geth node. Then run:
```bash
TEST_PROVIDER=geth yarn test
```
+
+###### Code coverage
+
+In order to see the Solidity code coverage output generated by `@0xproject/sol-cov`, run:
+
+```
+yarn test:coverage
+```
+
+###### Gas profiler
+
+In order to profile the gas costs for a specific smart contract call/transaction, you can run the tests in `profiler` mode.
+
+**Note:** Traces emitted by ganache have incorrect gas costs so we recommend using Geth for profiling.
+
+```
+TEST_PROVIDER=geth yarn test:profiler
+```
+
+You'll see a warning that you need to explicitly enable and disable the profiler before and after the block of code you want to profile.
+
+```typescript
+import { profiler } from './utils/profiler';
+profiler.start();
+// Some call to a smart contract
+profiler.stop();
+```
+
+Without explicitly starting and stopping the profiler, the profiler output will be too busy, and therefore unusable.
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 d07d82235..0457eb3ef 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -100,13 +100,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,
@@ -127,13 +126,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,
@@ -179,19 +177,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 expectRevertReasonOrAlwaysFailingTransactionAsync(
- erc20Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
- makerAddress,
- takerAddress,
- amount,
- {
- from: notAuthorized,
- },
- ),
+ erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+ from: notAuthorized,
+ }),
RevertReasons.SenderNotAuthorized,
);
});
@@ -202,10 +194,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);
@@ -234,10 +225,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);
@@ -251,9 +241,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);
});
});
@@ -262,7 +253,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);
@@ -270,7 +260,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(1);
await web3Wrapper.awaitTransactionSuccessAsync(
await erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -286,14 +276,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,
@@ -322,14 +311,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,
@@ -358,7 +346,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);
@@ -366,7 +353,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,
@@ -378,7 +365,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);
@@ -386,7 +372,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(0);
return expectRevertReasonOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -399,7 +385,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);
@@ -407,7 +392,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(500);
return expectRevertReasonOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -420,7 +405,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, {
@@ -431,15 +415,9 @@ describe('Asset Transfer Proxies', () => {
// Perform a transfer; expect this to fail.
const amount = new BigNumber(1);
return expectRevertReasonOrAlwaysFailingTransactionAsync(
- erc20Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
- makerAddress,
- takerAddress,
- amount,
- {
- from: exchangeAddress,
- },
- ),
+ erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+ from: exchangeAddress,
+ }),
RevertReasons.TransferFailed,
);
});
@@ -447,12 +425,11 @@ describe('Asset Transfer Proxies', () => {
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 expectRevertReasonOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -470,8 +447,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);
@@ -518,9 +495,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 3b8a369b4..c83041638 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -89,7 +89,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 cf73d274b..495f6d3c8 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -279,14 +279,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,
@@ -307,13 +306,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 expectRevertReasonOrAlwaysFailingTransactionAsync(
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 324cf319f..6f4feb8b7 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 8d4799109..44ed98c53 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -79,7 +79,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 3ea197395..a4ba54904 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);