aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts
diff options
context:
space:
mode:
authorFabio Berger <me@fabioberger.com>2018-06-26 14:13:59 +0800
committerFabio Berger <me@fabioberger.com>2018-06-26 14:13:59 +0800
commit9219f9d8ae2166c7c7b1d0a4ede78890094a3831 (patch)
tree85f1d4cb4b8a4dbcc8491c93cdbfafbf6cf00edc /packages/contracts
parentfb03003b3a9e7beb1054a50bad4320e60ca52e04 (diff)
parentcba92a01b6dee208b497817445b7ae4048e299c0 (diff)
downloaddexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.tar
dexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.tar.gz
dexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.tar.bz2
dexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.tar.lz
dexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.tar.xz
dexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.tar.zst
dexon-sol-tools-9219f9d8ae2166c7c7b1d0a4ede78890094a3831.zip
Merge branch 'v2-prototype' into feature/combinatorial-testing
* v2-prototype: (21 commits) Don't use variables for revert reasons Add comments to dispatchTransferFrom Document IAssetData Compute bytes4 proxyid constants Return inplace bytes in LibBytes.readBytesWithLength Add Greg's documentation to MixinErc721Transfer Add Greg's documentation to MixinErc20Transfer Gas golf the transfer success logic Optimize like in PR #767 Run prettier and fix linting errors Fix tests Update tests Fix AssetProxyUtils Update tests Fix ERC721Proxy TokenId Update AssetProxyUtils Add IAssetData WIP ABI encoded asset data Remove proxyId argument from dispatchTransferFrom ... # Conflicts: # packages/contracts/test/asset_proxy/proxies.ts
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.ts100
-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, 460 insertions, 281 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 f99f03875..7960e46d2 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -1,6 +1,6 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { assetProxyUtils, generatePseudoRandomSalt } from '@0xproject/order-utils';
-import { RevertReasons } from '@0xproject/types';
+import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
@@ -62,10 +62,7 @@ describe('Asset Transfer Proxies', () => {
erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner);
erc721Wrapper = new ERC721Wrapper(provider, usedAddresses, owner);
- [zrxToken] = await erc20Wrapper.deployDummyTokensAsync(
- constants.NUM_DUMMY_ERC20_TO_DEPLOY,
- constants.DUMMY_TOKEN_DECIMALS,
- );
+ [zrxToken] = await erc20Wrapper.deployDummyTokensAsync();
erc20Proxy = await erc20Wrapper.deployProxyAsync();
await erc20Wrapper.setBalancesAndAllowancesAsync();
await web3Wrapper.awaitTransactionSuccessAsync(
@@ -103,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,
@@ -130,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,
@@ -175,27 +170,21 @@ describe('Asset Transfer Proxies', () => {
transferAmount,
{ from: exchangeAddress },
),
- RevertReasons.TransferFailed,
+ RevertReason.TransferFailed,
);
});
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,
- },
- ),
- RevertReasons.SenderNotAuthorized,
+ erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+ from: notAuthorized,
+ }),
+ RevertReason.SenderNotAuthorized,
);
});
});
@@ -205,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);
@@ -237,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);
@@ -249,14 +236,15 @@ describe('Asset Transfer Proxies', () => {
erc20Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, {
from: notAuthorized,
}),
- RevertReasons.SenderNotAuthorized,
+ RevertReason.SenderNotAuthorized,
);
});
});
- 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);
});
});
@@ -265,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);
@@ -273,7 +260,7 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(1);
await web3Wrapper.awaitTransactionSuccessAsync(
await erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
@@ -289,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,
@@ -325,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,
@@ -361,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);
@@ -369,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,
@@ -381,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);
@@ -389,20 +372,19 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(0);
return expectRevertReasonOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
{ from: exchangeAddress },
),
- RevertReasons.InvalidAmount,
+ RevertReason.InvalidAmount,
);
});
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);
@@ -410,20 +392,19 @@ describe('Asset Transfer Proxies', () => {
const amount = new BigNumber(500);
return expectRevertReasonOrAlwaysFailingTransactionAsync(
erc721Proxy.transferFrom.sendTransactionAsync(
- encodedAssetDataWithoutProxyId,
+ encodedAssetData,
makerAddress,
takerAddress,
amount,
{ from: exchangeAddress },
),
- RevertReasons.InvalidAmount,
+ RevertReason.InvalidAmount,
);
});
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, {
@@ -434,34 +415,27 @@ 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,
- },
- ),
- RevertReasons.TransferFailed,
+ erc20Proxy.transferFrom.sendTransactionAsync(encodedAssetData, makerAddress, takerAddress, amount, {
+ from: exchangeAddress,
+ }),
+ RevertReason.TransferFailed,
);
});
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,
{ from: notAuthorized },
),
- RevertReasons.SenderNotAuthorized,
+ RevertReason.SenderNotAuthorized,
);
});
});
@@ -473,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);
@@ -491,8 +465,7 @@ describe('Asset Transfer Proxies', () => {
txHash,
constants.AWAIT_TRANSACTION_MINED_MS,
);
- const numApproveEvents = 2;
- expect(res.logs.length).to.equal(numTransfers + numApproveEvents);
+ expect(res.logs.length).to.equal(numTransfers);
const newOwnerMakerAssetA = await erc721Token.ownerOf.callAsync(makerTokenIdA);
const newOwnerMakerAssetB = await erc721Token.ownerOf.callAsync(makerTokenIdB);
@@ -517,14 +490,15 @@ describe('Asset Transfer Proxies', () => {
erc721Proxy.batchTransferFrom.sendTransactionAsync(assetData, fromAddresses, toAddresses, amounts, {
from: notAuthorized,
}),
- RevertReasons.SenderNotAuthorized,
+ RevertReason.SenderNotAuthorized,
);
});
});
- 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 63cbc155a..52bddf996 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -88,7 +88,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 59d80c6d6..41ea88d4e 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -282,14 +282,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,
@@ -310,13 +309,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 044108bfb..7c4ad165e 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -99,7 +99,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 6f820e673..cfabe0435 100644
--- a/packages/contracts/test/exchange/transactions.ts
+++ b/packages/contracts/test/exchange/transactions.ts
@@ -82,7 +82,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 504b78f5c..7e3203291 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -83,7 +83,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);