aboutsummaryrefslogtreecommitdiffstats
path: root/packages/contracts
diff options
context:
space:
mode:
authorFabio Berger <me@fabioberger.com>2018-09-28 19:08:12 +0800
committerFabio Berger <me@fabioberger.com>2018-09-28 19:08:12 +0800
commitba7de7204d29d4004c347190be7a3b8c84951b82 (patch)
tree9dddbd1ded45484a6cb968cdf799bf5ce991477b /packages/contracts
parentf3ad64aa1c2930affbfd074316b5f407580b7523 (diff)
parenta737cfa004ee1dc18be935f61fb9c289ed5623fd (diff)
downloaddexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.tar
dexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.tar.gz
dexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.tar.bz2
dexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.tar.lz
dexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.tar.xz
dexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.tar.zst
dexon-sol-tools-ba7de7204d29d4004c347190be7a3b8c84951b82.zip
merge development
Diffstat (limited to 'packages/contracts')
-rw-r--r--packages/contracts/README.md33
-rw-r--r--packages/contracts/compiler.json3
-rw-r--r--packages/contracts/package.json39
-rw-r--r--packages/contracts/src/2.0.0/extensions/Forwarder/Forwarder.sol1
-rw-r--r--packages/contracts/src/2.0.0/extensions/Forwarder/MixinAssets.sol1
-rw-r--r--packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol3
-rw-r--r--packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol11
-rw-r--r--packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol1
-rw-r--r--packages/contracts/src/2.0.0/extensions/Forwarder/mixins/MAssets.sol1
-rw-r--r--packages/contracts/src/2.0.0/extensions/OrderValidator/OrderValidator.sol4
-rw-r--r--packages/contracts/src/2.0.0/multisig/MultiSigWallet.sol90
-rw-r--r--packages/contracts/src/2.0.0/multisig/MultiSigWalletWithTimeLock.sol99
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol68
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol3
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/MixinAuthorizable.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAssetProxy.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAuthorizable.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxy/mixins/MAuthorizable.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol61
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/Exchange.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol5
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol57
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol16
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol4
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol10
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol6
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/libs/LibFillResults.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol94
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MAssetProxyDispatcher.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol5
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MMatchOrders.sol1
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MTransactions.sol22
-rw-r--r--packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol5
-rw-r--r--packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol69
-rw-r--r--packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol1
-rw-r--r--packages/contracts/src/2.0.0/test/DummyERC721Receiver/DummyERC721Receiver.sol1
-rw-r--r--packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol8
-rw-r--r--packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol24
-rw-r--r--packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol36
-rw-r--r--packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol1
-rw-r--r--packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol1
-rw-r--r--packages/contracts/src/2.0.0/tokens/ERC20Token/ERC20Token.sol1
-rw-r--r--packages/contracts/src/2.0.0/tokens/ERC20Token/MintableERC20Token.sol1
-rw-r--r--packages/contracts/src/2.0.0/tokens/ERC20Token/UnlimitedAllowanceERC20Token.sol1
-rw-r--r--packages/contracts/src/2.0.0/tokens/ERC721Token/MintableERC721Token.sol1
-rw-r--r--packages/contracts/src/2.0.0/tokens/ZRXToken/ZRXToken.sol6
-rw-r--r--packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol13
-rw-r--r--packages/contracts/src/2.0.0/utils/Ownable/IOwnable.sol7
-rw-r--r--packages/contracts/src/2.0.0/utils/Ownable/Ownable.sol11
-rw-r--r--packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol1
-rw-r--r--packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol1
-rw-r--r--packages/contracts/test/asset_proxy/authorizable.ts4
-rw-r--r--packages/contracts/test/asset_proxy/proxies.ts237
-rw-r--r--packages/contracts/test/exchange/core.ts39
-rw-r--r--packages/contracts/test/exchange/dispatcher.ts54
-rw-r--r--packages/contracts/test/exchange/internal.ts201
-rw-r--r--packages/contracts/test/exchange/match_orders.ts314
-rw-r--r--packages/contracts/test/exchange/signature_validator.ts14
-rw-r--r--packages/contracts/test/extensions/forwarder.ts33
-rw-r--r--packages/contracts/test/libraries/lib_bytes.ts95
-rw-r--r--packages/contracts/test/multisig/asset_proxy_owner.ts100
-rw-r--r--packages/contracts/test/multisig/multi_sig_with_time_lock.ts189
-rw-r--r--packages/contracts/test/tokens/unlimited_allowance_token.ts8
-rw-r--r--packages/contracts/test/utils/artifacts.ts2
-rw-r--r--packages/contracts/test/utils/assertions.ts17
-rw-r--r--packages/contracts/test/utils/block_timestamp.ts7
-rw-r--r--packages/contracts/test/utils/constants.ts1
-rw-r--r--packages/contracts/test/utils/multi_sig_wrapper.ts15
-rw-r--r--packages/contracts/test/utils/web3_wrapper.ts4
-rw-r--r--packages/contracts/tsconfig.json6
72 files changed, 1683 insertions, 493 deletions
diff --git a/packages/contracts/README.md b/packages/contracts/README.md
index 2e6376f39..33cbdae26 100644
--- a/packages/contracts/README.md
+++ b/packages/contracts/README.md
@@ -1,14 +1,35 @@
## Contracts
-Smart contracts that implement the 0x protocol.
+Smart contracts that implement the 0x protocol. Addresses of the deployed contracts can be found [here](https://0xproject.com/wiki#Deployed-Addresses).
## Usage
-* [Docs](https://0xproject.com/docs/contracts)
-* [Overview of 0x protocol architecture](https://0xproject.com/wiki#Architecture)
-* [0x smart contract interactions](https://0xproject.com/wiki#Contract-Interactions)
-* [Deployed smart contract addresses](https://0xproject.com/wiki#Deployed-Addresses)
-* [0x protocol message format](https://0xproject.com/wiki#Message-Format)
+### 2.0.0
+
+Contracts that make up and interact with version 2.0.0 of the protocol can be found in the `src/2.0.0` directory. The contents of this directory are broken down into the following subdirectories:
+
+* protocol
+ * This directory contains the contracts that make up version 2.0.0. A full specification can be found [here](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md).
+* extensions
+ * This directory contains contracts that interact with the 2.0.0 contracts and will be used in production, such as the [Forwarder](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/forwarder-specification.md) contract.
+* examples
+ * This directory contains example implementations of contracts that interact with the protocol but are _not_ intended for use in production. Examples include [filter](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#filter-contracts) contracts, a [Wallet](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#wallet) contract, and a [Validator](https://github.com/0xProject/0x-protocol-specification/blob/master/v2/v2-specification.md#validator) contract, among others.
+* tokens
+ * This directory contains implementations of different tokens and token standards, including [wETH](https://weth.io/), ZRX, [ERC20](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-20.md), and [ERC721](https://github.com/ethereum/EIPs/blob/master/EIPS/eip-721.md).
+* multisig
+ * This directory contains the [Gnosis MultiSigWallet](https://github.com/gnosis/MultiSigWallet) and a custom extension that adds a timelock to transactions within the MultiSigWallet.
+* utils
+ * This directory contains libraries and utils that are shared across all of the other directories.
+* test
+ * This directory contains mocks and other contracts that are used solely for testing contracts within the other directories.
+
+### 1.0.0
+
+Contracts that make up version 1.0.0 of the protocol can be found in `src/1.0.0`. These contracts are considered deprecated and will have limited support going forward.
+
+## Bug bounty
+
+A bug bounty for the 2.0.0 contracts is ongoing! Instructions can be found [here](https://0xproject.com/wiki#Bug-Bounty).
## Contributing
diff --git a/packages/contracts/compiler.json b/packages/contracts/compiler.json
index f66114e87..559b7cd1f 100644
--- a/packages/contracts/compiler.json
+++ b/packages/contracts/compiler.json
@@ -1,5 +1,5 @@
{
- "artifactsDir": "../migrations/artifacts/2.0.0",
+ "artifactsDir": "../migrations/artifacts/development",
"contractsDir": "src/",
"compilerSettings": {
"optimizer": {
@@ -23,6 +23,7 @@
"DummyERC20Token",
"DummyERC721Receiver",
"DummyERC721Token",
+ "DummyMultipleReturnERC20Token",
"DummyNoReturnERC20Token",
"ERC20Proxy",
"ERC20Token",
diff --git a/packages/contracts/package.json b/packages/contracts/package.json
index 5d2f290ac..5cc239ae7 100644
--- a/packages/contracts/package.json
+++ b/packages/contracts/package.json
@@ -1,7 +1,7 @@
{
"private": true,
"name": "contracts",
- "version": "2.1.42",
+ "version": "2.1.47",
"engines": {
"node": ">=6.12"
},
@@ -11,10 +11,9 @@
"test": "test"
},
"scripts": {
- "watch_without_deps": "yarn pre_build && tsc -w",
- "build": "yarn pre_build && tsc",
+ "build": "yarn pre_build && tsc -b",
"pre_build": "run-s compile copy_artifacts generate_contract_wrappers",
- "copy_artifacts": "copyfiles -u 4 '../migrations/artifacts/2.0.0/**/*' ./lib/artifacts;",
+ "copy_artifacts": "copyfiles -u 4 '../migrations/artifacts/development/**/*' ./lib/artifacts;",
"test": "yarn run_mocha",
"rebuild_and_test": "run-s build test",
"test:coverage": "SOLIDITY_COVERAGE=true run-s build run_mocha coverage:report:text coverage:report:lcov",
@@ -33,7 +32,7 @@
"lint-contracts": "solhint src/2.0.0/**/**/**/**/*.sol"
},
"config": {
- "abis": "../migrations/artifacts/2.0.0/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
+ "abis": "../migrations/artifacts/development/@(AssetProxyOwner|DummyERC20Token|DummyERC721Receiver|DummyERC721Token|DummyMultipleReturnERC20Token|DummyNoReturnERC20Token|ERC20Proxy|ERC721Proxy|Forwarder|Exchange|ExchangeWrapper|IAssetData|IAssetProxy|InvalidERC721Receiver|MixinAuthorizable|MultiSigWallet|MultiSigWalletWithTimeLock|OrderValidator|ReentrantERC20Token|TestAssetProxyOwner|TestAssetProxyDispatcher|TestConstants|TestExchangeInternals|TestLibBytes|TestLibs|TestSignatureValidator|TestStaticCallReceiver|Validator|Wallet|TokenRegistry|Whitelist|WETH9|ZRXToken).json"
},
"repository": {
"type": "git",
@@ -46,16 +45,16 @@
},
"homepage": "https://github.com/0xProject/0x-monorepo/packages/contracts/README.md",
"devDependencies": {
- "@0xproject/abi-gen": "^1.0.7",
- "@0xproject/dev-utils": "^1.0.6",
- "@0xproject/sol-compiler": "^1.1.1",
- "@0xproject/sol-cov": "^2.1.1",
- "@0xproject/subproviders": "^2.0.1",
- "@0xproject/tslint-config": "^1.0.6",
+ "@0xproject/abi-gen": "^1.0.11",
+ "@0xproject/dev-utils": "^1.0.10",
+ "@0xproject/sol-compiler": "^1.1.5",
+ "@0xproject/sol-cov": "^2.1.5",
+ "@0xproject/subproviders": "^2.0.5",
+ "@0xproject/tslint-config": "^1.0.7",
"@types/bn.js": "^4.11.0",
"@types/ethereumjs-abi": "^0.6.0",
"@types/lodash": "4.14.104",
- "@types/node": "^8.0.53",
+ "@types/node": "*",
"@types/yargs": "^10.0.0",
"chai": "^4.0.1",
"chai-as-promised": "^7.1.0",
@@ -73,18 +72,18 @@
"yargs": "^10.0.3"
},
"dependencies": {
- "@0xproject/base-contract": "^2.0.1",
- "@0xproject/order-utils": "^1.0.1-rc.6",
- "@0xproject/types": "^1.0.1-rc.6",
- "@0xproject/typescript-typings": "^1.0.5",
- "@0xproject/utils": "^1.0.7",
- "@0xproject/web3-wrapper": "^2.0.1",
+ "@0xproject/base-contract": "^2.0.5",
+ "@0xproject/order-utils": "^1.0.5",
+ "@0xproject/types": "^1.1.1",
+ "@0xproject/typescript-typings": "^2.0.2",
+ "@0xproject/utils": "^1.0.11",
+ "@0xproject/web3-wrapper": "^3.0.1",
"@types/js-combinatorics": "^0.5.29",
"bn.js": "^4.11.8",
- "ethereum-types": "^1.0.5",
+ "ethereum-types": "^1.0.8",
"ethereumjs-abi": "0.6.5",
"ethereumjs-util": "^5.1.1",
- "ethers": "3.0.22",
+ "ethers": "4.0.0-beta.14",
"js-combinatorics": "^0.5.3",
"lodash": "^4.17.5"
}
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/Forwarder.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/Forwarder.sol
index 6b17bb29b..94dec40ed 100644
--- a/packages/contracts/src/2.0.0/extensions/Forwarder/Forwarder.sol
+++ b/packages/contracts/src/2.0.0/extensions/Forwarder/Forwarder.sol
@@ -34,7 +34,6 @@ contract Forwarder is
MixinExchangeWrapper,
MixinForwarderCore
{
-
constructor (
address _exchange,
bytes memory _zrxAssetData,
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinAssets.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinAssets.sol
index d6a38aa6e..43efb5ff3 100644
--- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinAssets.sol
+++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinAssets.sol
@@ -31,7 +31,6 @@ contract MixinAssets is
LibConstants,
MAssets
{
-
using LibBytes for bytes;
bytes4 constant internal ERC20_TRANSFER_SELECTOR = bytes4(keccak256("transfer(address,uint256)"));
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol
index 9e816716c..fea9a53c2 100644
--- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol
+++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinExchangeWrapper.sol
@@ -34,7 +34,6 @@ contract MixinExchangeWrapper is
LibConstants,
MExchangeWrapper
{
-
/// @dev Fills the input order.
/// Returns false if the transaction would otherwise revert.
/// @param order Order struct containing order specifications.
@@ -61,7 +60,7 @@ contract MixinExchangeWrapper is
// Call `fillOrder` and handle any exceptions gracefully
assembly {
let success := call(
- gas, // forward all gas, TODO: look into gas consumption of assert/throw
+ gas, // forward all gas
exchange, // call address of Exchange contract
0, // transfer 0 wei
add(fillOrderCalldata, 32), // pointer to start of input (skip array length in first 32 bytes)
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol
index 14f191879..54487f726 100644
--- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol
+++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinForwarderCore.sol
@@ -39,7 +39,6 @@ contract MixinForwarderCore is
MExchangeWrapper,
IForwarderCore
{
-
using LibBytes for bytes;
/// @dev Constructor approves ERC20 proxy to transfer ZRX and WETH on this contract's behalf.
@@ -47,10 +46,12 @@ contract MixinForwarderCore is
public
{
address proxyAddress = EXCHANGE.getAssetProxy(ERC20_DATA_ID);
- if (proxyAddress != address(0)) {
- ETHER_TOKEN.approve(proxyAddress, MAX_UINT);
- ZRX_TOKEN.approve(proxyAddress, MAX_UINT);
- }
+ require(
+ proxyAddress != address(0),
+ "UNREGISTERED_ASSET_PROXY"
+ );
+ ETHER_TOKEN.approve(proxyAddress, MAX_UINT);
+ ZRX_TOKEN.approve(proxyAddress, MAX_UINT);
}
/// @dev Purchases as much of orders' makerAssets as possible by selling up to 95% of transaction's ETH value.
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol
index 5863b522d..d2814a49b 100644
--- a/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol
+++ b/packages/contracts/src/2.0.0/extensions/Forwarder/MixinWeth.sol
@@ -28,7 +28,6 @@ contract MixinWeth is
LibConstants,
MWeth
{
-
/// @dev Default payabale function, this allows us to withdraw WETH
function ()
public
diff --git a/packages/contracts/src/2.0.0/extensions/Forwarder/mixins/MAssets.sol b/packages/contracts/src/2.0.0/extensions/Forwarder/mixins/MAssets.sol
index 83636432a..9e7f80d97 100644
--- a/packages/contracts/src/2.0.0/extensions/Forwarder/mixins/MAssets.sol
+++ b/packages/contracts/src/2.0.0/extensions/Forwarder/mixins/MAssets.sol
@@ -24,7 +24,6 @@ import "../interfaces/IAssets.sol";
contract MAssets is
IAssets
{
-
/// @dev Transfers given amount of asset to sender.
/// @param assetData Byte array encoded for the respective asset proxy.
/// @param amount Amount of asset to transfer to sender.
diff --git a/packages/contracts/src/2.0.0/extensions/OrderValidator/OrderValidator.sol b/packages/contracts/src/2.0.0/extensions/OrderValidator/OrderValidator.sol
index a18345245..8bfde3847 100644
--- a/packages/contracts/src/2.0.0/extensions/OrderValidator/OrderValidator.sol
+++ b/packages/contracts/src/2.0.0/extensions/OrderValidator/OrderValidator.sol
@@ -28,11 +28,11 @@ import "../../utils/LibBytes/LibBytes.sol";
contract OrderValidator {
+ using LibBytes for bytes;
+
bytes4 constant internal ERC20_DATA_ID = bytes4(keccak256("ERC20Token(address)"));
bytes4 constant internal ERC721_DATA_ID = bytes4(keccak256("ERC721Token(address,uint256)"));
- using LibBytes for bytes;
-
struct TraderInfo {
uint256 makerBalance; // Maker's balance of makerAsset
uint256 makerAllowance; // Maker's allowance to corresponding AssetProxy
diff --git a/packages/contracts/src/2.0.0/multisig/MultiSigWallet.sol b/packages/contracts/src/2.0.0/multisig/MultiSigWallet.sol
index eb54fe047..516e7391c 100644
--- a/packages/contracts/src/2.0.0/multisig/MultiSigWallet.sol
+++ b/packages/contracts/src/2.0.0/multisig/MultiSigWallet.sol
@@ -1,13 +1,14 @@
// solhint-disable
-pragma solidity ^0.4.10;
+pragma solidity ^0.4.15;
/// @title Multisignature wallet - Allows multiple parties to agree on transactions before execution.
/// @author Stefan George - <stefan.george@consensys.net>
contract MultiSigWallet {
- uint constant public MAX_OWNER_COUNT = 50;
-
+ /*
+ * Events
+ */
event Confirmation(address indexed sender, uint indexed transactionId);
event Revocation(address indexed sender, uint indexed transactionId);
event Submission(uint indexed transactionId);
@@ -18,6 +19,14 @@ contract MultiSigWallet {
event OwnerRemoval(address indexed owner);
event RequirementChange(uint required);
+ /*
+ * Constants
+ */
+ uint constant public MAX_OWNER_COUNT = 50;
+
+ /*
+ * Storage
+ */
mapping (uint => Transaction) public transactions;
mapping (uint => mapping (address => bool)) public confirmations;
mapping (address => bool) public isOwner;
@@ -32,60 +41,54 @@ contract MultiSigWallet {
bool executed;
}
+ /*
+ * Modifiers
+ */
modifier onlyWallet() {
- if (msg.sender != address(this))
- throw;
+ require(msg.sender == address(this));
_;
}
modifier ownerDoesNotExist(address owner) {
- if (isOwner[owner])
- throw;
+ require(!isOwner[owner]);
_;
}
modifier ownerExists(address owner) {
- if (!isOwner[owner])
- throw;
+ require(isOwner[owner]);
_;
}
modifier transactionExists(uint transactionId) {
- if (transactions[transactionId].destination == 0)
- throw;
+ require(transactions[transactionId].destination != 0);
_;
}
modifier confirmed(uint transactionId, address owner) {
- if (!confirmations[transactionId][owner])
- throw;
+ require(confirmations[transactionId][owner]);
_;
}
modifier notConfirmed(uint transactionId, address owner) {
- if (confirmations[transactionId][owner])
- throw;
+ require(!confirmations[transactionId][owner]);
_;
}
modifier notExecuted(uint transactionId) {
- if (transactions[transactionId].executed)
- throw;
+ require(!transactions[transactionId].executed);
_;
}
modifier notNull(address _address) {
- if (_address == 0)
- throw;
+ require(_address != 0);
_;
}
modifier validRequirement(uint ownerCount, uint _required) {
- if ( ownerCount > MAX_OWNER_COUNT
- || _required > ownerCount
- || _required == 0
- || ownerCount == 0)
- throw;
+ require(ownerCount <= MAX_OWNER_COUNT
+ && _required <= ownerCount
+ && _required != 0
+ && ownerCount != 0);
_;
}
@@ -108,8 +111,7 @@ contract MultiSigWallet {
validRequirement(_owners.length, _required)
{
for (uint i=0; i<_owners.length; i++) {
- if (isOwner[_owners[i]] || _owners[i] == 0)
- throw;
+ require(!isOwner[_owners[i]] && _owners[i] != 0);
isOwner[_owners[i]] = true;
}
owners = _owners;
@@ -151,7 +153,7 @@ contract MultiSigWallet {
/// @dev Allows to replace an owner with a new owner. Transaction has to be sent by wallet.
/// @param owner Address of owner to be replaced.
- /// @param owner Address of new owner.
+ /// @param newOwner Address of new owner.
function replaceOwner(address owner, address newOwner)
public
onlyWallet
@@ -222,20 +224,44 @@ contract MultiSigWallet {
/// @param transactionId Transaction ID.
function executeTransaction(uint transactionId)
public
+ ownerExists(msg.sender)
+ confirmed(transactionId, msg.sender)
notExecuted(transactionId)
{
if (isConfirmed(transactionId)) {
- Transaction tx = transactions[transactionId];
- tx.executed = true;
- if (tx.destination.call.value(tx.value)(tx.data))
+ Transaction storage txn = transactions[transactionId];
+ txn.executed = true;
+ if (external_call(txn.destination, txn.value, txn.data.length, txn.data))
Execution(transactionId);
else {
ExecutionFailure(transactionId);
- tx.executed = false;
+ txn.executed = false;
}
}
}
+ // call has been separated into its own function in order to take advantage
+ // of the Solidity's code generator to produce a loop that copies tx.data into memory.
+ function external_call(address destination, uint value, uint dataLength, bytes data) internal returns (bool) {
+ bool result;
+ assembly {
+ let x := mload(0x40) // "Allocate" memory for output (0x40 is where "free memory" pointer is stored by convention)
+ let d := add(data, 32) // First 32 bytes are the padded length of data, so exclude that
+ result := call(
+ sub(gas, 34710), // 34710 is the value that solidity is currently emitting
+ // It includes callGas (700) + callVeryLow (3, to pay for SUB) + callValueTransferGas (9000) +
+ // callNewAccountGas (25000, in case the destination address does not exist and needs creating)
+ destination,
+ value,
+ d,
+ dataLength, // Size of the input (in bytes) - this is what fixes the padding problem
+ x,
+ 0 // Output is ignored, therefore the output size is zero
+ )
+ }
+ return result;
+ }
+
/// @dev Returns the confirmation status of a transaction.
/// @param transactionId Transaction ID.
/// @return Confirmation status.
@@ -364,4 +390,4 @@ contract MultiSigWallet {
for (i=from; i<to; i++)
_transactionIds[i - from] = transactionIdsTemp[i];
}
-}
+} \ No newline at end of file
diff --git a/packages/contracts/src/2.0.0/multisig/MultiSigWalletWithTimeLock.sol b/packages/contracts/src/2.0.0/multisig/MultiSigWalletWithTimeLock.sol
index 8c5e6e1e6..9513d3b30 100644
--- a/packages/contracts/src/2.0.0/multisig/MultiSigWalletWithTimeLock.sol
+++ b/packages/contracts/src/2.0.0/multisig/MultiSigWalletWithTimeLock.sol
@@ -16,47 +16,57 @@
*/
-// solhint-disable
-pragma solidity ^0.4.10;
+pragma solidity 0.4.24;
import "./MultiSigWallet.sol";
/// @title Multisignature wallet with time lock- Allows multiple parties to execute a transaction after a time lock has passed.
/// @author Amir Bandeali - <amir@0xProject.com>
-contract MultiSigWalletWithTimeLock is MultiSigWallet {
-
- event ConfirmationTimeSet(uint indexed transactionId, uint confirmationTime);
- event TimeLockChange(uint secondsTimeLocked);
-
- uint public secondsTimeLocked;
-
- mapping (uint => uint) public confirmationTimes;
-
- modifier notFullyConfirmed(uint transactionId) {
- require(!isConfirmed(transactionId));
+// solhint-disable not-rely-on-time
+contract MultiSigWalletWithTimeLock is
+ MultiSigWallet
+{
+ event ConfirmationTimeSet(uint256 indexed transactionId, uint256 confirmationTime);
+ event TimeLockChange(uint256 secondsTimeLocked);
+
+ uint256 public secondsTimeLocked;
+
+ mapping (uint256 => uint256) public confirmationTimes;
+
+ modifier notFullyConfirmed(uint256 transactionId) {
+ require(
+ !isConfirmed(transactionId),
+ "TX_FULLY_CONFIRMED"
+ );
_;
}
- modifier fullyConfirmed(uint transactionId) {
- require(isConfirmed(transactionId));
+ modifier fullyConfirmed(uint256 transactionId) {
+ require(
+ isConfirmed(transactionId),
+ "TX_NOT_FULLY_CONFIRMED"
+ );
_;
}
- modifier pastTimeLock(uint transactionId) {
- require(block.timestamp >= confirmationTimes[transactionId] + secondsTimeLocked);
+ modifier pastTimeLock(uint256 transactionId) {
+ require(
+ block.timestamp >= confirmationTimes[transactionId] + secondsTimeLocked,
+ "TIME_LOCK_INCOMPLETE"
+ );
_;
}
- /*
- * Public functions
- */
-
/// @dev Contract constructor sets initial owners, required number of confirmations, and time lock.
/// @param _owners List of initial owners.
/// @param _required Number of required confirmations.
/// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
- function MultiSigWalletWithTimeLock(address[] _owners, uint _required, uint _secondsTimeLocked)
+ constructor (
+ address[] _owners,
+ uint256 _required,
+ uint256 _secondsTimeLocked
+ )
public
MultiSigWallet(_owners, _required)
{
@@ -65,17 +75,17 @@ contract MultiSigWalletWithTimeLock is MultiSigWallet {
/// @dev Changes the duration of the time lock for transactions.
/// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
- function changeTimeLock(uint _secondsTimeLocked)
+ function changeTimeLock(uint256 _secondsTimeLocked)
public
onlyWallet
{
secondsTimeLocked = _secondsTimeLocked;
- TimeLockChange(_secondsTimeLocked);
+ emit TimeLockChange(_secondsTimeLocked);
}
/// @dev Allows an owner to confirm a transaction.
/// @param transactionId Transaction ID.
- function confirmTransaction(uint transactionId)
+ function confirmTransaction(uint256 transactionId)
public
ownerExists(msg.sender)
transactionExists(transactionId)
@@ -83,52 +93,35 @@ contract MultiSigWalletWithTimeLock is MultiSigWallet {
notFullyConfirmed(transactionId)
{
confirmations[transactionId][msg.sender] = true;
- Confirmation(msg.sender, transactionId);
+ emit Confirmation(msg.sender, transactionId);
if (isConfirmed(transactionId)) {
setConfirmationTime(transactionId, block.timestamp);
}
}
- /// @dev Allows an owner to revoke a confirmation for a transaction.
- /// @param transactionId Transaction ID.
- function revokeConfirmation(uint transactionId)
- public
- ownerExists(msg.sender)
- confirmed(transactionId, msg.sender)
- notExecuted(transactionId)
- notFullyConfirmed(transactionId)
- {
- confirmations[transactionId][msg.sender] = false;
- Revocation(msg.sender, transactionId);
- }
-
/// @dev Allows anyone to execute a confirmed transaction.
/// @param transactionId Transaction ID.
- function executeTransaction(uint transactionId)
+ function executeTransaction(uint256 transactionId)
public
notExecuted(transactionId)
fullyConfirmed(transactionId)
pastTimeLock(transactionId)
{
- Transaction storage tx = transactions[transactionId];
- tx.executed = true;
- if (tx.destination.call.value(tx.value)(tx.data))
- Execution(transactionId);
- else {
- ExecutionFailure(transactionId);
- tx.executed = false;
+ Transaction storage txn = transactions[transactionId];
+ txn.executed = true;
+ if (external_call(txn.destination, txn.value, txn.data.length, txn.data)) {
+ emit Execution(transactionId);
+ } else {
+ emit ExecutionFailure(transactionId);
+ txn.executed = false;
}
}
- /*
- * Internal functions
- */
-
/// @dev Sets the time of when a submission first passed.
- function setConfirmationTime(uint transactionId, uint confirmationTime)
+ function setConfirmationTime(uint256 transactionId, uint256 confirmationTime)
internal
{
confirmationTimes[transactionId] = confirmationTime;
- ConfirmationTimeSet(transactionId, confirmationTime);
+ emit ConfirmationTimeSet(transactionId, confirmationTime);
}
}
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol
index 004c3892d..258443bca 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC20Proxy.sol
@@ -18,7 +18,6 @@
pragma solidity 0.4.24;
-import "../../utils/LibBytes/LibBytes.sol";
import "./MixinAuthorizable.sol";
@@ -59,15 +58,64 @@ contract ERC20Proxy is
mstore(96, 0)
revert(0, 100)
}
-
- /////// Token contract address ///////
- // The token address is found as follows:
- // * It is stored at offset 4 in `assetData` contents.
- // * This is stored at offset 32 from `assetData`.
- // * The offset to `assetData` from Params is stored at offset
- // 4 in calldata.
- // * The offset of Params in calldata is 4.
- // So we read location 4 and add 32 + 4 + 4 to it.
+
+ // `transferFrom`.
+ // The function is marked `external`, so no abi decodeding is done for
+ // us. Instead, we expect the `calldata` memory to contain the
+ // following:
+ //
+ // | 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 |
+ //
+ // (*): offset is computed from start of function parameters, so offset
+ // by an additional 4 bytes in the calldata.
+ //
+ // (**): see table below to compute length of assetData Contents
+ //
+ // WARNING: The ABIv2 specification allows additional padding between
+ // the Params and Data section. This will result in a larger
+ // offset to assetData.
+
+ // Asset data itself is encoded as follows:
+ //
+ // | Area | Offset | Length | Contents |
+ // |----------|--------|---------|-------------------------------------|
+ // | Header | 0 | 4 | function selector |
+ // | Params | | 1 * 32 | function parameters: |
+ // | | 4 | 12 + 20 | 1. token address |
+
+ // 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 |
+
+ /////// Read token address from calldata ///////
+ // * The token address is stored in `assetData`.
+ //
+ // * The "offset to assetData" is stored at offset 4 in the calldata (table 1).
+ // [assetDataOffsetFromParams = calldataload(4)]
+ //
+ // * Notes that the "offset to assetData" is relative to the "Params" area of calldata;
+ // add 4 bytes to account for the length of the "Header" area (table 1).
+ // [assetDataOffsetFromHeader = assetDataOffsetFromParams + 4]
+ //
+ // * The "token address" is offset 32+4=36 bytes into "assetData" (tables 1 & 2).
+ // [tokenOffset = assetDataOffsetFromHeader + 36 = calldataload(4) + 4 + 36]
let token := calldataload(add(calldataload(4), 40))
/////// Setup Header Area ///////
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol
index 9d0bc0f74..65b664b8b 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/ERC721Proxy.sol
@@ -18,7 +18,6 @@
pragma solidity 0.4.24;
-import "../../utils/LibBytes/LibBytes.sol";
import "./MixinAuthorizable.sol";
@@ -80,6 +79,8 @@ contract ERC721Proxy is
// (*): offset is computed from start of function parameters, so offset
// by an additional 4 bytes in the calldata.
//
+ // (**): see table below to compute length of assetData Contents
+ //
// WARNING: The ABIv2 specification allows additional padding between
// the Params and Data section. This will result in a larger
// offset to assetData.
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/MixinAuthorizable.sol
index ff4660a31..fe9bbf848 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/MixinAuthorizable.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/MixinAuthorizable.sol
@@ -26,7 +26,6 @@ contract MixinAuthorizable is
Ownable,
MAuthorizable
{
-
/// @dev Only authorized addresses can invoke functions with this modifier.
modifier onlyAuthorized {
require(
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAssetProxy.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAssetProxy.sol
index 3651dd694..b25d2d75a 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAssetProxy.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAssetProxy.sol
@@ -24,7 +24,6 @@ import "./IAuthorizable.sol";
contract IAssetProxy is
IAuthorizable
{
-
/// @dev Transfers assets. Either succeeds or throws.
/// @param assetData Byte array encoded for the respective asset proxy.
/// @param from Address to transfer asset from.
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAuthorizable.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAuthorizable.sol
index 8fac43a47..ba1d4aa77 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAuthorizable.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/interfaces/IAuthorizable.sol
@@ -24,7 +24,6 @@ import "../../../utils/Ownable/IOwnable.sol";
contract IAuthorizable is
IOwnable
{
-
/// @dev Authorizes an address.
/// @param target Address to authorize.
function addAuthorizedAddress(address target)
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxy/mixins/MAuthorizable.sol b/packages/contracts/src/2.0.0/protocol/AssetProxy/mixins/MAuthorizable.sol
index 8afc8c8d8..d63fb7f6d 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxy/mixins/MAuthorizable.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxy/mixins/MAuthorizable.sol
@@ -24,7 +24,6 @@ import "../interfaces/IAuthorizable.sol";
contract MAuthorizable is
IAuthorizable
{
-
// Event logged when a new address is authorized.
event AuthorizedAddressAdded(
address indexed target,
diff --git a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
index 8b7333646..edb788fab 100644
--- a/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
+++ b/packages/contracts/src/2.0.0/protocol/AssetProxyOwner/AssetProxyOwner.sol
@@ -16,14 +16,16 @@
*/
-pragma solidity 0.4.10;
+pragma solidity 0.4.24;
import "../../multisig/MultiSigWalletWithTimeLock.sol";
+import "../../utils/LibBytes/LibBytes.sol";
contract AssetProxyOwner is
MultiSigWalletWithTimeLock
{
+ using LibBytes for bytes;
event AssetProxyRegistration(address assetProxyContract, bool isRegistered);
@@ -36,9 +38,15 @@ contract AssetProxyOwner is
/// @dev Function will revert if the transaction does not call `removeAuthorizedAddressAtIndex`
/// on an approved AssetProxy contract.
modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) {
- Transaction storage tx = transactions[transactionId];
- require(isAssetProxyRegistered[tx.destination]);
- require(readBytes4(tx.data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR);
+ Transaction storage txn = transactions[transactionId];
+ require(
+ isAssetProxyRegistered[txn.destination],
+ "UNREGISTERED_ASSET_PROXY"
+ );
+ require(
+ txn.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR,
+ "INVALID_FUNCTION_SELECTOR"
+ );
_;
}
@@ -48,7 +56,7 @@ contract AssetProxyOwner is
/// @param _assetProxyContracts Array of AssetProxy contract addresses.
/// @param _required Number of required confirmations.
/// @param _secondsTimeLocked Duration needed after a transaction is confirmed and before it becomes executable, in seconds.
- function AssetProxyOwner(
+ constructor (
address[] memory _owners,
address[] memory _assetProxyContracts,
uint256 _required,
@@ -59,7 +67,10 @@ contract AssetProxyOwner is
{
for (uint256 i = 0; i < _assetProxyContracts.length; i++) {
address assetProxy = _assetProxyContracts[i];
- require(assetProxy != address(0));
+ require(
+ assetProxy != address(0),
+ "INVALID_ASSET_PROXY"
+ );
isAssetProxyRegistered[assetProxy] = true;
}
}
@@ -74,7 +85,7 @@ contract AssetProxyOwner is
notNull(assetProxyContract)
{
isAssetProxyRegistered[assetProxyContract] = isRegistered;
- AssetProxyRegistration(assetProxyContract, isRegistered);
+ emit AssetProxyRegistration(assetProxyContract, isRegistered);
}
/// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock.
@@ -85,35 +96,13 @@ contract AssetProxyOwner is
fullyConfirmed(transactionId)
validRemoveAuthorizedAddressAtIndexTx(transactionId)
{
- Transaction storage tx = transactions[transactionId];
- tx.executed = true;
- // solhint-disable-next-line avoid-call-value
- if (tx.destination.call.value(tx.value)(tx.data))
- Execution(transactionId);
- else {
- ExecutionFailure(transactionId);
- tx.executed = false;
+ Transaction storage txn = transactions[transactionId];
+ txn.executed = true;
+ if (external_call(txn.destination, txn.value, txn.data.length, txn.data)) {
+ emit Execution(transactionId);
+ } else {
+ emit ExecutionFailure(transactionId);
+ txn.executed = false;
}
}
-
- /// @dev Reads an unpadded bytes4 value from a position in a byte array.
- /// @param b Byte array containing a bytes4 value.
- /// @param index Index in byte array of bytes4 value.
- /// @return bytes4 value from byte array.
- function readBytes4(
- bytes memory b,
- uint256 index
- )
- internal
- returns (bytes4 result)
- {
- require(b.length >= index + 4);
- assembly {
- result := mload(add(b, 32))
- // Solidity does not require us to clean the trailing bytes.
- // We do it anyway
- result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000)
- }
- return result;
- }
}
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/Exchange.sol b/packages/contracts/src/2.0.0/protocol/Exchange/Exchange.sol
index 7507d3da1..ead36009f 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/Exchange.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/Exchange.sol
@@ -37,7 +37,6 @@ contract Exchange is
MixinAssetProxyDispatcher,
MixinWrapperFunctions
{
-
string constant public VERSION = "2.0.1-alpha";
// Mixins are instantiated in the order they are inherited
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
index 80475e6e3..87b09b6b3 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinAssetProxyDispatcher.sol
@@ -19,7 +19,6 @@
pragma solidity 0.4.24;
import "../../utils/Ownable/Ownable.sol";
-import "../../utils/LibBytes/LibBytes.sol";
import "./mixins/MAssetProxyDispatcher.sol";
import "../AssetProxy/interfaces/IAssetProxy.sol";
@@ -28,8 +27,6 @@ contract MixinAssetProxyDispatcher is
Ownable,
MAssetProxyDispatcher
{
- using LibBytes for bytes;
-
// Mapping from Asset Proxy Id's to their respective Asset Proxy
mapping (bytes4 => IAssetProxy) public assetProxies;
@@ -90,7 +87,7 @@ contract MixinAssetProxyDispatcher is
"LENGTH_GREATER_THAN_3_REQUIRED"
);
- // Lookup assetProxy
+ // Lookup assetProxy. We do not use `LibBytes.readBytes4` for gas efficiency reasons.
bytes4 assetProxyId;
assembly {
assetProxyId := and(mload(
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
index be163ec97..736dcd0b1 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinExchangeCore.sol
@@ -75,7 +75,11 @@ contract MixinExchangeCore is
// Update orderEpoch
orderEpoch[makerAddress][senderAddress] = newOrderEpoch;
- emit CancelUpTo(makerAddress, senderAddress, newOrderEpoch);
+ emit CancelUpTo(
+ makerAddress,
+ senderAddress,
+ newOrderEpoch
+ );
}
/// @dev Fills the input order.
@@ -107,14 +111,7 @@ contract MixinExchangeCore is
public
nonReentrant
{
- // Fetch current order status
- OrderInfo memory orderInfo = getOrderInfo(order);
-
- // Validate context
- assertValidCancel(order, orderInfo);
-
- // Perform cancel
- updateCancelledState(order, orderInfo.orderHash);
+ cancelOrderInternal(order);
}
/// @dev Gets information about an order: status, hash, and amount filled.
@@ -231,11 +228,31 @@ contract MixinExchangeCore is
);
// Settle order
- settleOrder(order, takerAddress, fillResults);
+ settleOrder(
+ order,
+ takerAddress,
+ fillResults
+ );
return fillResults;
}
+ /// @dev After calling, the order can not be filled anymore.
+ /// Throws if order is invalid or sender does not have permission to cancel.
+ /// @param order Order to cancel. Order must be OrderStatus.FILLABLE.
+ function cancelOrderInternal(Order memory order)
+ internal
+ {
+ // Fetch current order status
+ OrderInfo memory orderInfo = getOrderInfo(order);
+
+ // Validate context
+ assertValidCancel(order, orderInfo);
+
+ // Perform cancel
+ updateCancelledState(order, orderInfo.orderHash);
+ }
+
/// @dev Updates state with results of a fill order.
/// @param order that was filled.
/// @param takerAddress Address of taker who filled the order.
@@ -404,16 +421,6 @@ contract MixinExchangeCore is
safeMul(order.makerAssetAmount, takerAssetFilledAmount),
"INVALID_FILL_PRICE"
);
-
- // Validate fill order rounding
- require(
- !isRoundingErrorFloor(
- takerAssetFilledAmount,
- order.takerAssetAmount,
- order.makerAssetAmount
- ),
- "ROUNDING_ERROR"
- );
}
/// @dev Validates context for cancelOrder. Succeeds or throws.
@@ -463,17 +470,17 @@ contract MixinExchangeCore is
{
// Compute proportional transfer amounts
fillResults.takerAssetFilledAmount = takerAssetFilledAmount;
- fillResults.makerAssetFilledAmount = getPartialAmountFloor(
+ fillResults.makerAssetFilledAmount = safeGetPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
order.makerAssetAmount
);
- fillResults.makerFeePaid = getPartialAmountFloor(
- takerAssetFilledAmount,
- order.takerAssetAmount,
+ fillResults.makerFeePaid = safeGetPartialAmountFloor(
+ fillResults.makerAssetFilledAmount,
+ order.makerAssetAmount,
order.makerFee
);
- fillResults.takerFeePaid = getPartialAmountFloor(
+ fillResults.takerFeePaid = safeGetPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
order.takerFee
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
index 075a610b5..b4f6bdb26 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinMatchOrders.sol
@@ -177,13 +177,13 @@ contract MixinMatchOrders is
{
// Derive maker asset amounts for left & right orders, given store taker assert amounts
uint256 leftTakerAssetAmountRemaining = safeSub(leftOrder.takerAssetAmount, leftOrderTakerAssetFilledAmount);
- uint256 leftMakerAssetAmountRemaining = getPartialAmountFloor(
+ uint256 leftMakerAssetAmountRemaining = safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
leftTakerAssetAmountRemaining
);
uint256 rightTakerAssetAmountRemaining = safeSub(rightOrder.takerAssetAmount, rightOrderTakerAssetFilledAmount);
- uint256 rightMakerAssetAmountRemaining = getPartialAmountFloor(
+ uint256 rightMakerAssetAmountRemaining = safeGetPartialAmountFloor(
rightOrder.makerAssetAmount,
rightOrder.takerAssetAmount,
rightTakerAssetAmountRemaining
@@ -205,7 +205,7 @@ contract MixinMatchOrders is
matchedFillResults.left.takerAssetFilledAmount = matchedFillResults.right.makerAssetFilledAmount;
// Round down to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
- matchedFillResults.left.makerAssetFilledAmount = getPartialAmountFloor(
+ matchedFillResults.left.makerAssetFilledAmount = safeGetPartialAmountFloor(
leftOrder.makerAssetAmount,
leftOrder.takerAssetAmount,
matchedFillResults.left.takerAssetFilledAmount
@@ -217,7 +217,7 @@ contract MixinMatchOrders is
matchedFillResults.right.makerAssetFilledAmount = matchedFillResults.left.takerAssetFilledAmount;
// Round up to ensure the maker's exchange rate does not exceed the price specified by the order.
// We favor the maker when the exchange rate must be rounded.
- matchedFillResults.right.takerAssetFilledAmount = getPartialAmountCeil(
+ matchedFillResults.right.takerAssetFilledAmount = safeGetPartialAmountCeil(
rightOrder.takerAssetAmount,
rightOrder.makerAssetAmount,
matchedFillResults.right.makerAssetFilledAmount
@@ -231,24 +231,24 @@ contract MixinMatchOrders is
);
// Compute fees for left order
- matchedFillResults.left.makerFeePaid = getPartialAmountFloor(
+ matchedFillResults.left.makerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.left.makerAssetFilledAmount,
leftOrder.makerAssetAmount,
leftOrder.makerFee
);
- matchedFillResults.left.takerFeePaid = getPartialAmountFloor(
+ matchedFillResults.left.takerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.left.takerAssetFilledAmount,
leftOrder.takerAssetAmount,
leftOrder.takerFee
);
// Compute fees for right order
- matchedFillResults.right.makerFeePaid = getPartialAmountFloor(
+ matchedFillResults.right.makerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.right.makerAssetFilledAmount,
rightOrder.makerAssetAmount,
rightOrder.makerFee
);
- matchedFillResults.right.takerFeePaid = getPartialAmountFloor(
+ matchedFillResults.right.takerFeePaid = safeGetPartialAmountFloor(
matchedFillResults.right.takerAssetFilledAmount,
rightOrder.takerAssetAmount,
rightOrder.takerFee
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol
index 4eb6a2fa6..176e28351 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinSignatureValidator.sol
@@ -251,7 +251,7 @@ contract MixinSignatureValidator is
walletAddress, // address of Wallet contract
cdStart, // pointer to start of input
mload(calldata), // length of input
- cdStart, // write input over output
+ cdStart, // write output over input
32 // output size is 32 bytes
)
@@ -301,7 +301,7 @@ contract MixinSignatureValidator is
validatorAddress, // address of Validator contract
cdStart, // pointer to start of input
mload(calldata), // length of input
- cdStart, // write input over output
+ cdStart, // write output over input
32 // output size is 32 bytes
)
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
index 4a59b6c0f..3a76ca202 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinTransactions.sol
@@ -28,7 +28,6 @@ contract MixinTransactions is
MSignatureValidator,
MTransactions
{
-
// Mapping of transaction hash => executed
// This prevents transactions from being executed more than once.
mapping (bytes32 => bool) public transactions;
@@ -36,15 +35,6 @@ contract MixinTransactions is
// Address of current transaction signer
address public currentContextAddress;
- // Hash for the EIP712 ZeroEx Transaction Schema
- bytes32 constant internal EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked(
- "ZeroExTransaction(",
- "uint256 salt,",
- "address signerAddress,",
- "bytes data",
- ")"
- ));
-
/// @dev Executes an exchange method call in the context of signer.
/// @param salt Arbitrary number to ensure uniqueness of transaction hash.
/// @param signerAddress Address of transaction signer.
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
index a5459a21e..cddff0e5f 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/MixinWrapperFunctions.sol
@@ -36,7 +36,6 @@ contract MixinWrapperFunctions is
MExchangeCore,
MWrapperFunctions
{
-
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
/// @param order Order struct containing order specifications.
/// @param takerAssetFillAmount Desired amount of takerAsset to sell.
@@ -82,7 +81,7 @@ contract MixinWrapperFunctions is
// Delegate to `fillOrder` and handle any exceptions gracefully
assembly {
let success := delegatecall(
- gas, // forward all gas, TODO: look into gas consumption of assert/throw
+ gas, // forward all gas
address, // call address of this contract
add(fillOrderCalldata, 32), // pointer to start of input (skip array length in first 32 bytes)
mload(fillOrderCalldata), // length of input
@@ -377,10 +376,11 @@ contract MixinWrapperFunctions is
/// @param orders Array of order specifications.
function batchCancelOrders(LibOrder.Order[] memory orders)
public
+ nonReentrant
{
uint256 ordersLength = orders.length;
for (uint256 i = 0; i != ordersLength; i++) {
- cancelOrder(orders[i]);
+ cancelOrderInternal(orders[i]);
}
}
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol
index b02f7632e..203edc1fd 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibEIP712.sol
@@ -20,6 +20,7 @@ pragma solidity 0.4.24;
contract LibEIP712 {
+
// EIP191 header for EIP712 prefix
string constant internal EIP191_HEADER = "\x19\x01";
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibFillResults.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibFillResults.sol
index 1b4181d94..659ae9a69 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibFillResults.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibFillResults.sol
@@ -24,7 +24,6 @@ import "../../../utils/SafeMath/SafeMath.sol";
contract LibFillResults is
SafeMath
{
-
struct FillResults {
uint256 makerAssetFilledAmount; // Total amount of makerAsset(s) filled.
uint256 takerAssetFilledAmount; // Total amount of takerAsset(s) filled.
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol
index 0e0fba5d2..c0b85ea10 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibMath.sol
@@ -24,6 +24,83 @@ import "../../../utils/SafeMath/SafeMath.sol";
contract LibMath is
SafeMath
{
+ /// @dev Calculates partial value given a numerator and denominator rounded down.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target rounded down.
+ function safeGetPartialAmountFloor(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ internal
+ pure
+ returns (uint256 partialAmount)
+ {
+ require(
+ denominator > 0,
+ "DIVISION_BY_ZERO"
+ );
+
+ require(
+ !isRoundingErrorFloor(
+ numerator,
+ denominator,
+ target
+ ),
+ "ROUNDING_ERROR"
+ );
+
+ partialAmount = safeDiv(
+ safeMul(numerator, target),
+ denominator
+ );
+ return partialAmount;
+ }
+
+ /// @dev Calculates partial value given a numerator and denominator rounded down.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target rounded up.
+ function safeGetPartialAmountCeil(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ internal
+ pure
+ returns (uint256 partialAmount)
+ {
+ require(
+ denominator > 0,
+ "DIVISION_BY_ZERO"
+ );
+
+ require(
+ !isRoundingErrorCeil(
+ numerator,
+ denominator,
+ target
+ ),
+ "ROUNDING_ERROR"
+ );
+
+ // safeDiv computes `floor(a / b)`. We use the identity (a, b integer):
+ // ceil(a / b) = floor((a + b - 1) / b)
+ // To implement `ceil(a / b)` using safeDiv.
+ partialAmount = safeDiv(
+ safeAdd(
+ safeMul(numerator, target),
+ safeSub(denominator, 1)
+ ),
+ denominator
+ );
+ return partialAmount;
+ }
/// @dev Calculates partial value given a numerator and denominator rounded down.
/// @param numerator Numerator.
@@ -43,7 +120,7 @@ contract LibMath is
denominator > 0,
"DIVISION_BY_ZERO"
);
-
+
partialAmount = safeDiv(
safeMul(numerator, target),
denominator
@@ -69,7 +146,7 @@ contract LibMath is
denominator > 0,
"DIVISION_BY_ZERO"
);
-
+
// safeDiv computes `floor(a / b)`. We use the identity (a, b integer):
// ceil(a / b) = floor((a + b - 1) / b)
// To implement `ceil(a / b)` using safeDiv.
@@ -128,7 +205,11 @@ contract LibMath is
// 1000 * remainder < numerator * target
// so we have a rounding error iff:
// 1000 * remainder >= numerator * target
- uint256 remainder = mulmod(target, numerator, denominator);
+ uint256 remainder = mulmod(
+ target,
+ numerator,
+ denominator
+ );
isError = safeMul(1000, remainder) >= safeMul(numerator, target);
return isError;
}
@@ -160,8 +241,11 @@ contract LibMath is
return false;
}
// Compute remainder as before
- uint256 remainder = mulmod(target, numerator, denominator);
- // TODO: safeMod
+ uint256 remainder = mulmod(
+ target,
+ numerator,
+ denominator
+ );
remainder = safeSub(denominator, remainder) % denominator;
isError = safeMul(1000, remainder) >= safeMul(numerator, target);
return isError;
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol
index 68f4f5f1b..0fe7c2161 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/libs/LibOrder.sol
@@ -24,7 +24,6 @@ import "./LibEIP712.sol";
contract LibOrder is
LibEIP712
{
-
// Hash for the EIP712 Order Schema
bytes32 constant internal EIP712_ORDER_SCHEMA_HASH = keccak256(abi.encodePacked(
"Order(",
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
index c6904300a..0ddfca270 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MAssetProxyDispatcher.sol
@@ -24,7 +24,6 @@ import "../interfaces/IAssetProxyDispatcher.sol";
contract MAssetProxyDispatcher is
IAssetProxyDispatcher
{
-
// Logs registration of new asset proxy
event AssetProxyRegistered(
bytes4 id, // Id of new registered AssetProxy.
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol
index d85913e0f..742499568 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MExchangeCore.sol
@@ -72,6 +72,11 @@ contract MExchangeCore is
internal
returns (LibFillResults.FillResults memory fillResults);
+ /// @dev After calling, the order can not be filled anymore.
+ /// @param order Order struct containing order specifications.
+ function cancelOrderInternal(LibOrder.Order memory order)
+ internal;
+
/// @dev Updates state with results of a fill order.
/// @param order that was filled.
/// @param takerAddress Address of taker who filled the order.
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MMatchOrders.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MMatchOrders.sol
index a31ec1585..96fa34bc0 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MMatchOrders.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MMatchOrders.sol
@@ -26,7 +26,6 @@ import "../interfaces/IMatchOrders.sol";
contract MMatchOrders is
IMatchOrders
{
-
/// @dev Validates context for matchOrders. Succeeds or throws.
/// @param leftOrder First order to match.
/// @param rightOrder Second order to match.
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MTransactions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MTransactions.sol
index f2b5e4b16..4f61a4945 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MTransactions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MTransactions.sol
@@ -23,6 +23,28 @@ import "../interfaces/ITransactions.sol";
contract MTransactions is
ITransactions
{
+ // Hash for the EIP712 ZeroEx Transaction Schema
+ bytes32 constant internal EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked(
+ "ZeroExTransaction(",
+ "uint256 salt,",
+ "address signerAddress,",
+ "bytes data",
+ ")"
+ ));
+
+ /// @dev Calculates EIP712 hash of the Transaction.
+ /// @param salt Arbitrary number to ensure uniqueness of transaction hash.
+ /// @param signerAddress Address of transaction signer.
+ /// @param data AbiV2 encoded calldata.
+ /// @return EIP712 hash of the Transaction.
+ function hashZeroExTransaction(
+ uint256 salt,
+ address signerAddress,
+ bytes memory data
+ )
+ internal
+ pure
+ returns (bytes32 result);
/// @dev The current function will be called in the context of this address (either 0x transaction signer or `msg.sender`).
/// If calling a fill function, this address will represent the taker.
diff --git a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol
index e04d4a429..4adfbde01 100644
--- a/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol
+++ b/packages/contracts/src/2.0.0/protocol/Exchange/mixins/MWrapperFunctions.sol
@@ -24,8 +24,9 @@ import "../libs/LibFillResults.sol";
import "../interfaces/IWrapperFunctions.sol";
-contract MWrapperFunctions {
-
+contract MWrapperFunctions is
+ IWrapperFunctions
+{
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
/// @param order LibOrder.Order struct containing order specifications.
/// @param takerAssetFillAmount Desired amount of takerAsset to sell.
diff --git a/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol
new file mode 100644
index 000000000..733d4437e
--- /dev/null
+++ b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyMultipleReturnERC20Token.sol
@@ -0,0 +1,69 @@
+/*
+
+ 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.24;
+
+import "./DummyERC20Token.sol";
+
+
+// solhint-disable no-empty-blocks
+contract DummyMultipleReturnERC20Token is
+ DummyERC20Token
+{
+ constructor (
+ string _name,
+ string _symbol,
+ uint256 _decimals,
+ uint256 _totalSupply
+ )
+ public
+ DummyERC20Token(
+ _name,
+ _symbol,
+ _decimals,
+ _totalSupply
+ )
+ {}
+
+ /// @dev send `value` token to `to` from `from` on the condition it is approved by `from`
+ /// @param _from The address of the sender
+ /// @param _to The address of the recipient
+ /// @param _value The amount of token to be transferred
+ function transferFrom(
+ address _from,
+ address _to,
+ uint256 _value
+ )
+ external
+ returns (bool)
+ {
+ emit Transfer(
+ _from,
+ _to,
+ _value
+ );
+
+ // HACK: This contract will not compile if we remove `returns (bool)`, so we manually return 64 bytes (equiavalent to true, true)
+ assembly {
+ mstore(0, 1)
+ mstore(32, 1)
+ return(0, 64)
+ }
+ }
+}
+
diff --git a/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol
index 79156d3dd..e16825a16 100644
--- a/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol
+++ b/packages/contracts/src/2.0.0/test/DummyERC20Token/DummyNoReturnERC20Token.sol
@@ -25,7 +25,6 @@ import "./DummyERC20Token.sol";
contract DummyNoReturnERC20Token is
DummyERC20Token
{
-
constructor (
string _name,
string _symbol,
diff --git a/packages/contracts/src/2.0.0/test/DummyERC721Receiver/DummyERC721Receiver.sol b/packages/contracts/src/2.0.0/test/DummyERC721Receiver/DummyERC721Receiver.sol
index ac95e47bd..6c8371559 100644
--- a/packages/contracts/src/2.0.0/test/DummyERC721Receiver/DummyERC721Receiver.sol
+++ b/packages/contracts/src/2.0.0/test/DummyERC721Receiver/DummyERC721Receiver.sol
@@ -24,7 +24,6 @@ import "../../tokens/ERC721Token/IERC721Receiver.sol";
contract DummyERC721Receiver is
IERC721Receiver
{
-
// Function selector for ERC721Receiver.onERC721Received
// 0x150b7a02
bytes4 constant internal ERC721_RECEIVED = bytes4(keccak256("onERC721Received(address,address,uint256,bytes)"));
diff --git a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
index 8bfdd2e66..99dd47a78 100644
--- a/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
+++ b/packages/contracts/src/2.0.0/test/ReentrantERC20Token/ReentrantERC20Token.sol
@@ -25,10 +25,10 @@ import "../../protocol/Exchange/interfaces/IExchange.sol";
import "../../protocol/Exchange/libs/LibOrder.sol";
+// solhint-disable no-unused-vars
contract ReentrantERC20Token is
ERC20Token
{
-
using LibBytes for bytes;
// solhint-disable-next-line var-name-mixedcase
@@ -50,6 +50,7 @@ contract ReentrantERC20Token is
MARKET_SELL_ORDERS,
MATCH_ORDERS,
CANCEL_ORDER,
+ BATCH_CANCEL_ORDERS,
CANCEL_ORDERS_UP_TO,
SET_SIGNATURE_VALIDATOR_APPROVAL
}
@@ -149,6 +150,11 @@ contract ReentrantERC20Token is
EXCHANGE.cancelOrder.selector,
order
);
+ } else if (currentFunctionId == uint8(ExchangeFunction.BATCH_CANCEL_ORDERS)) {
+ calldata = abi.encodeWithSelector(
+ EXCHANGE.batchCancelOrders.selector,
+ orders
+ );
} else if (currentFunctionId == uint8(ExchangeFunction.CANCEL_ORDERS_UP_TO)) {
calldata = abi.encodeWithSelector(
EXCHANGE.cancelOrdersUpTo.selector,
diff --git a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
index 75e782d43..52c66cb56 100644
--- a/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
+++ b/packages/contracts/src/2.0.0/test/TestAssetProxyOwner/TestAssetProxyOwner.sol
@@ -16,7 +16,7 @@
*/
-pragma solidity 0.4.10;
+pragma solidity 0.4.24;
import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol";
@@ -25,8 +25,7 @@ import "../../protocol/AssetProxyOwner/AssetProxyOwner.sol";
contract TestAssetProxyOwner is
AssetProxyOwner
{
-
- function TestAssetProxyOwner(
+ constructor (
address[] memory _owners,
address[] memory _assetProxyContracts,
uint256 _required,
@@ -38,6 +37,7 @@ contract TestAssetProxyOwner is
function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id)
public
+ view
validRemoveAuthorizedAddressAtIndexTx(id)
returns (bool)
{
@@ -50,23 +50,9 @@ contract TestAssetProxyOwner is
/// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`.
function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data)
public
+ pure
returns (bool)
{
- return readBytes4(data, 0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
- }
-
- /// @dev Reads an unpadded bytes4 value from a position in a byte array.
- /// @param b Byte array containing a bytes4 value.
- /// @param index Index in byte array of bytes4 value.
- /// @return bytes4 value from byte array.
- function publicReadBytes4(
- bytes memory b,
- uint256 index
- )
- public
- returns (bytes4 result)
- {
- result = readBytes4(b, index);
- return result;
+ return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR;
}
}
diff --git a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
index da9313e02..27187f8f8 100644
--- a/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
+++ b/packages/contracts/src/2.0.0/test/TestExchangeInternals/TestExchangeInternals.sol
@@ -63,6 +63,42 @@ contract TestExchangeInternals is
}
/// @dev Calculates partial value given a numerator and denominator.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target.
+ function publicSafeGetPartialAmountFloor(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ public
+ pure
+ returns (uint256 partialAmount)
+ {
+ return safeGetPartialAmountFloor(numerator, denominator, target);
+ }
+
+ /// @dev Calculates partial value given a numerator and denominator.
+ /// Reverts if rounding error is >= 0.1%
+ /// @param numerator Numerator.
+ /// @param denominator Denominator.
+ /// @param target Value to calculate partial of.
+ /// @return Partial value of target.
+ function publicSafeGetPartialAmountCeil(
+ uint256 numerator,
+ uint256 denominator,
+ uint256 target
+ )
+ public
+ pure
+ returns (uint256 partialAmount)
+ {
+ return safeGetPartialAmountCeil(numerator, denominator, target);
+ }
+
+ /// @dev Calculates partial value given a numerator and denominator.
/// @param numerator Numerator.
/// @param denominator Denominator.
/// @param target Value to calculate partial of.
diff --git a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol
index c8c58545f..a10f981fc 100644
--- a/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol
+++ b/packages/contracts/src/2.0.0/test/TestLibs/TestLibs.sol
@@ -31,7 +31,6 @@ contract TestLibs is
LibFillResults,
LibAbiEncoder
{
-
function publicAbiEncodeFillOrder(
Order memory order,
uint256 takerAssetFillAmount,
diff --git a/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol b/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol
index e1a610469..ea3e2de59 100644
--- a/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol
+++ b/packages/contracts/src/2.0.0/test/TestSignatureValidator/TestSignatureValidator.sol
@@ -26,7 +26,6 @@ contract TestSignatureValidator is
MixinSignatureValidator,
MixinTransactions
{
-
function publicIsValidSignature(
bytes32 hash,
address signer,
diff --git a/packages/contracts/src/2.0.0/tokens/ERC20Token/ERC20Token.sol b/packages/contracts/src/2.0.0/tokens/ERC20Token/ERC20Token.sol
index 5ef5ee7ce..725d304df 100644
--- a/packages/contracts/src/2.0.0/tokens/ERC20Token/ERC20Token.sol
+++ b/packages/contracts/src/2.0.0/tokens/ERC20Token/ERC20Token.sol
@@ -24,7 +24,6 @@ import "./IERC20Token.sol";
contract ERC20Token is
IERC20Token
{
-
mapping (address => uint256) internal balances;
mapping (address => mapping (address => uint256)) internal allowed;
diff --git a/packages/contracts/src/2.0.0/tokens/ERC20Token/MintableERC20Token.sol b/packages/contracts/src/2.0.0/tokens/ERC20Token/MintableERC20Token.sol
index cd1c7b4bb..9dc924422 100644
--- a/packages/contracts/src/2.0.0/tokens/ERC20Token/MintableERC20Token.sol
+++ b/packages/contracts/src/2.0.0/tokens/ERC20Token/MintableERC20Token.sol
@@ -26,7 +26,6 @@ contract MintableERC20Token is
SafeMath,
UnlimitedAllowanceERC20Token
{
-
/// @dev Mints new tokens
/// @param _to Address of the beneficiary that will own the minted token
/// @param _value Amount of tokens to mint
diff --git a/packages/contracts/src/2.0.0/tokens/ERC20Token/UnlimitedAllowanceERC20Token.sol b/packages/contracts/src/2.0.0/tokens/ERC20Token/UnlimitedAllowanceERC20Token.sol
index e6f7c063e..2e5bd4348 100644
--- a/packages/contracts/src/2.0.0/tokens/ERC20Token/UnlimitedAllowanceERC20Token.sol
+++ b/packages/contracts/src/2.0.0/tokens/ERC20Token/UnlimitedAllowanceERC20Token.sol
@@ -24,7 +24,6 @@ import "../ERC20Token/ERC20Token.sol";
contract UnlimitedAllowanceERC20Token is
ERC20Token
{
-
uint256 constant internal MAX_UINT = 2**256 - 1;
/// @dev ERC20 transferFrom, modified such that an allowance of MAX_UINT represents an unlimited allowance. See https://github.com/ethereum/EIPs/issues/717
diff --git a/packages/contracts/src/2.0.0/tokens/ERC721Token/MintableERC721Token.sol b/packages/contracts/src/2.0.0/tokens/ERC721Token/MintableERC721Token.sol
index 85d192779..bc5cd2cc2 100644
--- a/packages/contracts/src/2.0.0/tokens/ERC721Token/MintableERC721Token.sol
+++ b/packages/contracts/src/2.0.0/tokens/ERC721Token/MintableERC721Token.sol
@@ -24,7 +24,6 @@ import "./ERC721Token.sol";
contract MintableERC721Token is
ERC721Token
{
-
/// @dev Function to mint a new token
/// Reverts if the given token ID already exists
/// @param _to Address of the beneficiary that will own the minted token
diff --git a/packages/contracts/src/2.0.0/tokens/ZRXToken/ZRXToken.sol b/packages/contracts/src/2.0.0/tokens/ZRXToken/ZRXToken.sol
index 28c0b2fb3..f4855759c 100644
--- a/packages/contracts/src/2.0.0/tokens/ZRXToken/ZRXToken.sol
+++ b/packages/contracts/src/2.0.0/tokens/ZRXToken/ZRXToken.sol
@@ -22,11 +22,13 @@ pragma solidity 0.4.11;
import { UnlimitedAllowanceToken_v1 as UnlimitedAllowanceToken } from "../../../1.0.0/UnlimitedAllowanceToken/UnlimitedAllowanceToken_v1.sol";
-contract ZRXToken is UnlimitedAllowanceToken {
+contract ZRXToken is
+ UnlimitedAllowanceToken
+{
// solhint-disable const-name-snakecase
uint8 constant public decimals = 18;
- uint public totalSupply = 10**27; // 1 billion tokens, 18 decimal places
+ uint256 public totalSupply = 10**27; // 1 billion tokens, 18 decimal places
string constant public name = "0x Protocol Token";
string constant public symbol = "ZRX";
// solhint-enableconst-name-snakecase
diff --git a/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol b/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol
index 504e950a8..369f588ad 100644
--- a/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol
+++ b/packages/contracts/src/2.0.0/utils/LibBytes/LibBytes.sol
@@ -188,7 +188,8 @@ library LibBytes {
memCopy(
result.contentAddress(),
b.contentAddress() + from,
- result.length);
+ result.length
+ );
return result;
}
@@ -433,7 +434,8 @@ library LibBytes {
pure
returns (uint256 result)
{
- return uint256(readBytes32(b, index));
+ result = uint256(readBytes32(b, index));
+ return result;
}
/// @dev Writes a uint256 into a specific position in a byte array.
@@ -467,8 +469,13 @@ library LibBytes {
b.length >= index + 4,
"GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED"
);
+
+ // Arrays are prefixed by a 32 byte length field
+ index += 32;
+
+ // Read the bytes4 from array memory
assembly {
- result := mload(add(b, 32))
+ result := mload(add(b, index))
// Solidity does not require us to clean the trailing bytes.
// We do it anyway
result := and(result, 0xFFFFFFFF00000000000000000000000000000000000000000000000000000000)
diff --git a/packages/contracts/src/2.0.0/utils/Ownable/IOwnable.sol b/packages/contracts/src/2.0.0/utils/Ownable/IOwnable.sol
index 116b8dc89..5deb13497 100644
--- a/packages/contracts/src/2.0.0/utils/Ownable/IOwnable.sol
+++ b/packages/contracts/src/2.0.0/utils/Ownable/IOwnable.sol
@@ -1,13 +1,8 @@
pragma solidity 0.4.24;
-/*
- * Ownable
- *
- * Base contract with an owner.
- * Provides onlyOwner modifier, which prevents function from running if it is called by anyone other than the owner.
- */
contract IOwnable {
+
function transferOwnership(address newOwner)
public;
}
diff --git a/packages/contracts/src/2.0.0/utils/Ownable/Ownable.sol b/packages/contracts/src/2.0.0/utils/Ownable/Ownable.sol
index aca65aad2..0c830be68 100644
--- a/packages/contracts/src/2.0.0/utils/Ownable/Ownable.sol
+++ b/packages/contracts/src/2.0.0/utils/Ownable/Ownable.sol
@@ -1,16 +1,11 @@
pragma solidity 0.4.24;
-/*
- * Ownable
- *
- * Base contract with an owner.
- * Provides onlyOwner modifier, which prevents function from running if it is called by anyone other than the owner.
- */
-
import "./IOwnable.sol";
-contract Ownable is IOwnable {
+contract Ownable is
+ IOwnable
+{
address public owner;
constructor ()
diff --git a/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol b/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol
index 1dee512d4..9f98a7a16 100644
--- a/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol
+++ b/packages/contracts/src/2.0.0/utils/ReentrancyGuard/ReentrancyGuard.sol
@@ -15,6 +15,7 @@
limitations under the License.
*/
+
pragma solidity 0.4.24;
diff --git a/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol b/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol
index 63a2a085f..2855edb9d 100644
--- a/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol
+++ b/packages/contracts/src/2.0.0/utils/SafeMath/SafeMath.sol
@@ -2,6 +2,7 @@ pragma solidity 0.4.24;
contract SafeMath {
+
function safeMul(uint256 a, uint256 b)
internal
pure
diff --git a/packages/contracts/test/asset_proxy/authorizable.ts b/packages/contracts/test/asset_proxy/authorizable.ts
index e99c6cee3..0c0da46b3 100644
--- a/packages/contracts/test/asset_proxy/authorizable.ts
+++ b/packages/contracts/test/asset_proxy/authorizable.ts
@@ -2,6 +2,7 @@ import { BlockchainLifecycle } from '@0xproject/dev-utils';
import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
+import * as _ from 'lodash';
import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mixin_authorizable';
import { artifacts } from '../utils/artifacts';
@@ -28,8 +29,7 @@ describe('Authorizable', () => {
});
before(async () => {
const accounts = await web3Wrapper.getAvailableAddressesAsync();
- owner = address = accounts[0];
- notOwner = accounts[1];
+ [owner, address, notOwner] = _.slice(accounts, 0, 3);
authorizable = await MixinAuthorizableContract.deployFrom0xArtifactAsync(
artifacts.MixinAuthorizable,
provider,
diff --git a/packages/contracts/test/asset_proxy/proxies.ts b/packages/contracts/test/asset_proxy/proxies.ts
index 6031e554d..4d70031d1 100644
--- a/packages/contracts/test/asset_proxy/proxies.ts
+++ b/packages/contracts/test/asset_proxy/proxies.ts
@@ -8,6 +8,8 @@ import * as _ from 'lodash';
import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
import { DummyERC721ReceiverContract } from '../../generated_contract_wrappers/dummy_erc721_receiver';
import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token';
+import { DummyMultipleReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_multiple_return_erc20_token';
+import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token';
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { IAssetProxyContract } from '../../generated_contract_wrappers/i_asset_proxy';
@@ -42,6 +44,8 @@ describe('Asset Transfer Proxies', () => {
let erc721Receiver: DummyERC721ReceiverContract;
let erc20Proxy: ERC20ProxyContract;
let erc721Proxy: ERC721ProxyContract;
+ let noReturnErc20Token: DummyNoReturnERC20TokenContract;
+ let multipleReturnErc20Token: DummyMultipleReturnERC20TokenContract;
let erc20Wrapper: ERC20Wrapper;
let erc721Wrapper: ERC721Wrapper;
@@ -91,6 +95,51 @@ describe('Asset Transfer Proxies', () => {
provider,
txDefaults,
);
+ noReturnErc20Token = await DummyNoReturnERC20TokenContract.deployFrom0xArtifactAsync(
+ artifacts.DummyNoReturnERC20Token,
+ provider,
+ txDefaults,
+ constants.DUMMY_TOKEN_NAME,
+ constants.DUMMY_TOKEN_SYMBOL,
+ constants.DUMMY_TOKEN_DECIMALS,
+ constants.DUMMY_TOKEN_TOTAL_SUPPLY,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await noReturnErc20Token.setBalance.sendTransactionAsync(makerAddress, constants.INITIAL_ERC20_BALANCE),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await noReturnErc20Token.approve.sendTransactionAsync(
+ erc20Proxy.address,
+ constants.INITIAL_ERC20_ALLOWANCE,
+ { from: makerAddress },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ multipleReturnErc20Token = await DummyMultipleReturnERC20TokenContract.deployFrom0xArtifactAsync(
+ artifacts.DummyMultipleReturnERC20Token,
+ provider,
+ txDefaults,
+ constants.DUMMY_TOKEN_NAME,
+ constants.DUMMY_TOKEN_SYMBOL,
+ constants.DUMMY_TOKEN_DECIMALS,
+ constants.DUMMY_TOKEN_TOTAL_SUPPLY,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await multipleReturnErc20Token.setBalance.sendTransactionAsync(
+ makerAddress,
+ constants.INITIAL_ERC20_BALANCE,
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await multipleReturnErc20Token.approve.sendTransactionAsync(
+ erc20Proxy.address,
+ constants.INITIAL_ERC20_ALLOWANCE,
+ { from: makerAddress },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
});
beforeEach(async () => {
await blockchainLifecycle.startAsync();
@@ -141,6 +190,65 @@ describe('Asset Transfer Proxies', () => {
);
});
+ it('should successfully transfer tokens that do not return a value', async () => {
+ // Construct ERC20 asset data
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address);
+ // Perform a transfer from makerAddress to takerAddress
+ const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Verify transfer was successful
+ const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance.minus(amount));
+ expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance.plus(amount));
+ });
+
+ it('should successfully transfer tokens and ignore extra assetData', async () => {
+ // Construct ERC20 asset data
+ const extraData = '0102030405060708';
+ const encodedAssetData = `${assetDataUtils.encodeERC20AssetData(zrxToken.address)}${extraData}`;
+ // Perform a transfer from makerAddress to takerAddress
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Verify transfer was successful
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal(
+ erc20Balances[makerAddress][zrxToken.address].minus(amount),
+ );
+ expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal(
+ erc20Balances[takerAddress][zrxToken.address].add(amount),
+ );
+ });
+
it('should do nothing if transferring 0 amount of a token', async () => {
// Construct ERC20 asset data
const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
@@ -189,6 +297,7 @@ describe('Asset Transfer Proxies', () => {
}),
constants.AWAIT_TRANSACTION_MINED_MS,
);
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
// Perform a transfer; expect this to fail.
await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
@@ -198,6 +307,43 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.TransferFailed,
);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
+ it('should throw if allowances are too low and token does not return a value', async () => {
+ // Construct ERC20 asset data
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(noReturnErc20Token.address);
+ // Create allowance less than transfer amount. Set allowance on proxy.
+ const allowance = new BigNumber(0);
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await noReturnErc20Token.approve.sendTransactionAsync(erc20Proxy.address, allowance, {
+ from: makerAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ const initialMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const initialTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ // Perform a transfer; expect this to fail.
+ await expectTransactionFailedAsync(
+ web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ RevertReason.TransferFailed,
+ );
+ const newMakerBalance = await noReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const newTakerBalance = await noReturnErc20Token.balanceOf.callAsync(takerAddress);
+ expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance);
+ expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance);
});
it('should throw if requesting address is not authorized', async () => {
@@ -211,6 +357,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc20Proxy.address,
@@ -219,6 +366,35 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.SenderNotAuthorized,
);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
+ it('should throw if token returns more than 32 bytes', async () => {
+ // Construct ERC20 asset data
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(multipleReturnErc20Token.address);
+ const amount = new BigNumber(10);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ const initialMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const initialTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress);
+ // Perform a transfer; expect this to fail.
+ await expectTransactionFailedAsync(
+ web3Wrapper.sendTransactionAsync({
+ to: erc20Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ RevertReason.TransferFailed,
+ );
+ const newMakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(makerAddress);
+ const newTakerBalance = await multipleReturnErc20Token.balanceOf.callAsync(takerAddress);
+ expect(newMakerBalance).to.be.bignumber.equal(initialMakerBalance);
+ expect(newTakerBalance).to.be.bignumber.equal(initialTakerBalance);
});
});
@@ -247,7 +423,38 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
+ // Perform a transfer from makerAddress to takerAddress
+ const amount = new BigNumber(1);
+ const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ );
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await web3Wrapper.sendTransactionAsync({
+ to: erc721Proxy.address,
+ data,
+ from: exchangeAddress,
+ }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Verify transfer was successful
+ const newOwnerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwnerMakerAsset).to.be.bignumber.equal(takerAddress);
+ });
+
+ it('should successfully transfer tokens and ignore extra assetData', async () => {
+ // Construct ERC721 asset data
+ const extraData = '0102030405060708';
+ const encodedAssetData = `${assetDataUtils.encodeERC721AssetData(
+ erc721Token.address,
+ erc721MakerTokenId,
+ )}${extraData}`;
+ // Verify pre-condition
+ const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -274,7 +481,7 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -304,7 +511,7 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(0);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -313,7 +520,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -321,6 +528,8 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.InvalidAmount,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
it('should throw if transferring > 1 amount of a token', async () => {
@@ -328,7 +537,7 @@ describe('Asset Transfer Proxies', () => {
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
// Verify pre-condition
const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
- expect(ownerMakerAsset).to.be.bignumber.equal(makerAddress);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(500);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -337,7 +546,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -345,11 +554,16 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.InvalidAmount,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
it('should throw if allowances are too low', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+ // Verify pre-condition
+ const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Remove transfer approval for makerAddress.
await web3Wrapper.awaitTransactionSuccessAsync(
await erc721Token.approve.sendTransactionAsync(constants.NULL_ADDRESS, erc721MakerTokenId, {
@@ -365,7 +579,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -373,11 +587,16 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.TransferFailed,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
it('should throw if requesting address is not authorized', async () => {
// Construct ERC721 asset data
const encodedAssetData = assetDataUtils.encodeERC721AssetData(erc721Token.address, erc721MakerTokenId);
+ // Verify pre-condition
+ const ownerMakerAsset = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(ownerMakerAsset).to.be.equal(makerAddress);
// Perform a transfer from makerAddress to takerAddress
const amount = new BigNumber(1);
const data = assetProxyInterface.transferFrom.getABIEncodedTransactionData(
@@ -386,7 +605,7 @@ describe('Asset Transfer Proxies', () => {
takerAddress,
amount,
);
- return expectTransactionFailedAsync(
+ await expectTransactionFailedAsync(
web3Wrapper.sendTransactionAsync({
to: erc721Proxy.address,
data,
@@ -394,6 +613,8 @@ describe('Asset Transfer Proxies', () => {
}),
RevertReason.SenderNotAuthorized,
);
+ const newOwner = await erc721Token.ownerOf.callAsync(erc721MakerTokenId);
+ expect(newOwner).to.be.equal(ownerMakerAsset);
});
});
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 3bb71b58f..acb99eed1 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -8,7 +8,10 @@ import { LogWithDecodedArgs } from 'ethereum-types';
import ethUtil = require('ethereumjs-util');
import * as _ from 'lodash';
-import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
+import {
+ DummyERC20TokenContract,
+ DummyERC20TokenTransferEventArgs,
+} from '../../generated_contract_wrappers/dummy_erc20_token';
import { DummyERC721TokenContract } from '../../generated_contract_wrappers/dummy_erc721_token';
import { DummyNoReturnERC20TokenContract } from '../../generated_contract_wrappers/dummy_no_return_erc20_token';
import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_proxy';
@@ -243,6 +246,40 @@ describe('Exchange core', () => {
RevertReason.ValidatorError,
);
});
+
+ it('should not emit transfer events for transfers where from == to', async () => {
+ const txReceipt = await exchangeWrapper.fillOrderAsync(signedOrder, makerAddress);
+ const logs = txReceipt.logs;
+ const transferLogs = _.filter(
+ logs,
+ log => (log as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).event === 'Transfer',
+ );
+ expect(transferLogs.length).to.be.equal(2);
+ expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal(
+ zrxToken.address,
+ );
+ expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal(
+ makerAddress,
+ );
+ expect((transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal(
+ feeRecipientAddress,
+ );
+ expect(
+ (transferLogs[0] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value,
+ ).to.be.bignumber.equal(signedOrder.makerFee);
+ expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).address).to.be.equal(
+ zrxToken.address,
+ );
+ expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._from).to.be.equal(
+ makerAddress,
+ );
+ expect((transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._to).to.be.equal(
+ feeRecipientAddress,
+ );
+ expect(
+ (transferLogs[1] as LogWithDecodedArgs<DummyERC20TokenTransferEventArgs>).args._value,
+ ).to.be.bignumber.equal(signedOrder.takerFee);
+ });
});
describe('Testing exchange of ERC20 tokens with no return values', () => {
diff --git a/packages/contracts/test/exchange/dispatcher.ts b/packages/contracts/test/exchange/dispatcher.ts
index 81871a680..a8ae897a8 100644
--- a/packages/contracts/test/exchange/dispatcher.ts
+++ b/packages/contracts/test/exchange/dispatcher.ts
@@ -205,6 +205,60 @@ describe('AssetProxyDispatcher', () => {
);
});
+ it('should not dispatch a transfer if amount == 0', async () => {
+ // Register ERC20 proxy
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Construct metadata for ERC20 proxy
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
+
+ // Perform a transfer from makerAddress to takerAddress
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
+ const amount = constants.ZERO_AMOUNT;
+ const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
+ encodedAssetData,
+ makerAddress,
+ takerAddress,
+ amount,
+ { from: owner },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ expect(txReceipt.logs.length).to.be.equal(0);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
+ it('should not dispatch a transfer if from == to', async () => {
+ // Register ERC20 proxy
+ await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.registerAssetProxy.sendTransactionAsync(erc20Proxy.address, { from: owner }),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ // Construct metadata for ERC20 proxy
+ const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
+
+ // Perform a transfer from makerAddress to takerAddress
+ const erc20Balances = await erc20Wrapper.getBalancesAsync();
+ const amount = new BigNumber(10);
+ const txReceipt = await web3Wrapper.awaitTransactionSuccessAsync(
+ await assetProxyDispatcher.publicDispatchTransferFrom.sendTransactionAsync(
+ encodedAssetData,
+ makerAddress,
+ makerAddress,
+ amount,
+ { from: owner },
+ ),
+ constants.AWAIT_TRANSACTION_MINED_MS,
+ );
+ expect(txReceipt.logs.length).to.be.equal(0);
+ const newBalances = await erc20Wrapper.getBalancesAsync();
+ expect(newBalances).to.deep.equal(erc20Balances);
+ });
+
it('should throw if dispatching to unregistered proxy', async () => {
// Construct metadata for ERC20 proxy
const encodedAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
diff --git a/packages/contracts/test/exchange/internal.ts b/packages/contracts/test/exchange/internal.ts
index de381fca3..156e086af 100644
--- a/packages/contracts/test/exchange/internal.ts
+++ b/packages/contracts/test/exchange/internal.ts
@@ -6,10 +6,7 @@ import * as _ from 'lodash';
import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals';
import { artifacts } from '../utils/artifacts';
-import {
- getInvalidOpcodeErrorMessageForCallAsync,
- getRevertReasonOrErrorMessageForSendTransactionAsync,
-} from '../utils/assertions';
+import { getRevertReasonOrErrorMessageForSendTransactionAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { bytes32Values, testCombinatoriallyWithReferenceFuncAsync, uint256Values } from '../utils/combinatorial_utils';
import { constants } from '../utils/constants';
@@ -48,9 +45,9 @@ const overflowErrorForCall = new Error(RevertReason.Uint256Overflow);
describe('Exchange core internal functions', () => {
let testExchange: TestExchangeInternalsContract;
- let invalidOpcodeErrorForCall: Error | undefined;
let overflowErrorForSendTransaction: Error | undefined;
let divisionByZeroErrorForCall: Error | undefined;
+ let roundingErrorForCall: Error | undefined;
before(async () => {
await blockchainLifecycle.startAsync();
@@ -68,12 +65,67 @@ describe('Exchange core internal functions', () => {
await getRevertReasonOrErrorMessageForSendTransactionAsync(RevertReason.Uint256Overflow),
);
divisionByZeroErrorForCall = new Error(RevertReason.DivisionByZero);
- invalidOpcodeErrorForCall = new Error(await getInvalidOpcodeErrorMessageForCallAsync());
+ roundingErrorForCall = new Error(RevertReason.RoundingError);
});
// Note(albrow): Don't forget to add beforeEach and afterEach calls to reset
// the blockchain state for any tests which modify it!
- async function referenceGetPartialAmountFloorAsync(
+ async function referenceIsRoundingErrorFloorAsync(
+ numerator: BigNumber,
+ denominator: BigNumber,
+ target: BigNumber,
+ ): Promise<boolean> {
+ if (denominator.eq(0)) {
+ throw divisionByZeroErrorForCall;
+ }
+ if (numerator.eq(0)) {
+ return false;
+ }
+ if (target.eq(0)) {
+ return false;
+ }
+ const product = numerator.mul(target);
+ const remainder = product.mod(denominator);
+ const remainderTimes1000 = remainder.mul('1000');
+ const isError = remainderTimes1000.gte(product);
+ if (product.greaterThan(MAX_UINT256)) {
+ throw overflowErrorForCall;
+ }
+ if (remainderTimes1000.greaterThan(MAX_UINT256)) {
+ throw overflowErrorForCall;
+ }
+ return isError;
+ }
+
+ async function referenceIsRoundingErrorCeilAsync(
+ numerator: BigNumber,
+ denominator: BigNumber,
+ target: BigNumber,
+ ): Promise<boolean> {
+ if (denominator.eq(0)) {
+ throw divisionByZeroErrorForCall;
+ }
+ if (numerator.eq(0)) {
+ return false;
+ }
+ if (target.eq(0)) {
+ return false;
+ }
+ const product = numerator.mul(target);
+ const remainder = product.mod(denominator);
+ const error = denominator.sub(remainder).mod(denominator);
+ const errorTimes1000 = error.mul('1000');
+ const isError = errorTimes1000.gte(product);
+ if (product.greaterThan(MAX_UINT256)) {
+ throw overflowErrorForCall;
+ }
+ if (errorTimes1000.greaterThan(MAX_UINT256)) {
+ throw overflowErrorForCall;
+ }
+ return isError;
+ }
+
+ async function referenceSafeGetPartialAmountFloorAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
@@ -81,6 +133,10 @@ describe('Exchange core internal functions', () => {
if (denominator.eq(0)) {
throw divisionByZeroErrorForCall;
}
+ const isRoundingError = await referenceIsRoundingErrorFloorAsync(numerator, denominator, target);
+ if (isRoundingError) {
+ throw roundingErrorForCall;
+ }
const product = numerator.mul(target);
if (product.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
@@ -162,19 +218,22 @@ describe('Exchange core internal functions', () => {
// in any mathematical operation in either the reference TypeScript
// implementation or the Solidity implementation of
// calculateFillResults.
+ const makerAssetFilledAmount = await referenceSafeGetPartialAmountFloorAsync(
+ takerAssetFilledAmount,
+ orderTakerAssetAmount,
+ otherAmount,
+ );
+ const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount);
+ const orderMakerAssetAmount = order.makerAssetAmount;
return {
- makerAssetFilledAmount: await referenceGetPartialAmountFloorAsync(
- takerAssetFilledAmount,
- orderTakerAssetAmount,
- otherAmount,
- ),
+ makerAssetFilledAmount,
takerAssetFilledAmount,
- makerFeePaid: await referenceGetPartialAmountFloorAsync(
- takerAssetFilledAmount,
- orderTakerAssetAmount,
+ makerFeePaid: await referenceSafeGetPartialAmountFloorAsync(
+ makerAssetFilledAmount,
+ orderMakerAssetAmount,
otherAmount,
),
- takerFeePaid: await referenceGetPartialAmountFloorAsync(
+ takerFeePaid: await referenceSafeGetPartialAmountFloorAsync(
takerAssetFilledAmount,
orderTakerAssetAmount,
otherAmount,
@@ -198,6 +257,20 @@ describe('Exchange core internal functions', () => {
});
describe('getPartialAmountFloor', async () => {
+ async function referenceGetPartialAmountFloorAsync(
+ numerator: BigNumber,
+ denominator: BigNumber,
+ target: BigNumber,
+ ): Promise<BigNumber> {
+ if (denominator.eq(0)) {
+ throw divisionByZeroErrorForCall;
+ }
+ const product = numerator.mul(target);
+ if (product.greaterThan(MAX_UINT256)) {
+ throw overflowErrorForCall;
+ }
+ return product.dividedToIntegerBy(denominator);
+ }
async function testGetPartialAmountFloorAsync(
numerator: BigNumber,
denominator: BigNumber,
@@ -206,7 +279,7 @@ describe('Exchange core internal functions', () => {
return testExchange.publicGetPartialAmountFloor.callAsync(numerator, denominator, target);
}
await testCombinatoriallyWithReferenceFuncAsync(
- 'getPartialAmount',
+ 'getPartialAmountFloor',
referenceGetPartialAmountFloorAsync,
testGetPartialAmountFloorAsync,
[uint256Values, uint256Values, uint256Values],
@@ -250,76 +323,80 @@ describe('Exchange core internal functions', () => {
);
});
- describe('isRoundingError', async () => {
- async function referenceIsRoundingErrorAsync(
+ describe('safeGetPartialAmountFloor', async () => {
+ async function testSafeGetPartialAmountFloorAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
- ): Promise<boolean> {
+ ): Promise<BigNumber> {
+ return testExchange.publicSafeGetPartialAmountFloor.callAsync(numerator, denominator, target);
+ }
+ await testCombinatoriallyWithReferenceFuncAsync(
+ 'safeGetPartialAmountFloor',
+ referenceSafeGetPartialAmountFloorAsync,
+ testSafeGetPartialAmountFloorAsync,
+ [uint256Values, uint256Values, uint256Values],
+ );
+ });
+
+ describe('safeGetPartialAmountCeil', async () => {
+ async function referenceSafeGetPartialAmountCeilAsync(
+ numerator: BigNumber,
+ denominator: BigNumber,
+ target: BigNumber,
+ ): Promise<BigNumber> {
if (denominator.eq(0)) {
throw divisionByZeroErrorForCall;
}
- if (numerator.eq(0)) {
- return false;
- }
- if (target.eq(0)) {
- return false;
+ const isRoundingError = await referenceIsRoundingErrorCeilAsync(numerator, denominator, target);
+ if (isRoundingError) {
+ throw roundingErrorForCall;
}
const product = numerator.mul(target);
- const remainder = product.mod(denominator);
- const remainderTimes1000 = remainder.mul('1000');
- const isError = remainderTimes1000.gt(product);
- if (product.greaterThan(MAX_UINT256)) {
+ const offset = product.add(denominator.sub(1));
+ if (offset.greaterThan(MAX_UINT256)) {
throw overflowErrorForCall;
}
- if (remainderTimes1000.greaterThan(MAX_UINT256)) {
- throw overflowErrorForCall;
+ const result = offset.dividedToIntegerBy(denominator);
+ if (product.mod(denominator).eq(0)) {
+ expect(result.mul(denominator)).to.be.bignumber.eq(product);
+ } else {
+ expect(result.mul(denominator)).to.be.bignumber.gt(product);
}
- return isError;
+ return result;
}
- async function testIsRoundingErrorAsync(
+ async function testSafeGetPartialAmountCeilAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
- ): Promise<boolean> {
- return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target);
+ ): Promise<BigNumber> {
+ return testExchange.publicSafeGetPartialAmountCeil.callAsync(numerator, denominator, target);
}
await testCombinatoriallyWithReferenceFuncAsync(
- 'isRoundingError',
- referenceIsRoundingErrorAsync,
- testIsRoundingErrorAsync,
+ 'safeGetPartialAmountCeil',
+ referenceSafeGetPartialAmountCeilAsync,
+ testSafeGetPartialAmountCeilAsync,
[uint256Values, uint256Values, uint256Values],
);
});
- describe('isRoundingErrorCeil', async () => {
- async function referenceIsRoundingErrorAsync(
+ describe('isRoundingErrorFloor', async () => {
+ async function testIsRoundingErrorFloorAsync(
numerator: BigNumber,
denominator: BigNumber,
target: BigNumber,
): Promise<boolean> {
- if (denominator.eq(0)) {
- throw divisionByZeroErrorForCall;
- }
- if (numerator.eq(0)) {
- return false;
- }
- if (target.eq(0)) {
- return false;
- }
- const product = numerator.mul(target);
- const remainder = product.mod(denominator);
- const error = denominator.sub(remainder).mod(denominator);
- const errorTimes1000 = error.mul('1000');
- const isError = errorTimes1000.gt(product);
- if (product.greaterThan(MAX_UINT256)) {
- throw overflowErrorForCall;
- }
- if (errorTimes1000.greaterThan(MAX_UINT256)) {
- throw overflowErrorForCall;
- }
- return isError;
+ return testExchange.publicIsRoundingErrorFloor.callAsync(numerator, denominator, target);
}
+ await testCombinatoriallyWithReferenceFuncAsync(
+ 'isRoundingErrorFloor',
+ referenceIsRoundingErrorFloorAsync,
+ testIsRoundingErrorFloorAsync,
+ [uint256Values, uint256Values, uint256Values],
+ );
+ });
+
+ describe('isRoundingErrorCeil', async () => {
async function testIsRoundingErrorCeilAsync(
numerator: BigNumber,
denominator: BigNumber,
@@ -329,7 +406,7 @@ describe('Exchange core internal functions', () => {
}
await testCombinatoriallyWithReferenceFuncAsync(
'isRoundingErrorCeil',
- referenceIsRoundingErrorAsync,
+ referenceIsRoundingErrorCeilAsync,
testIsRoundingErrorCeilAsync,
[uint256Values, uint256Values, uint256Values],
);
diff --git a/packages/contracts/test/exchange/match_orders.ts b/packages/contracts/test/exchange/match_orders.ts
index 8732e20ba..c6e7b494f 100644
--- a/packages/contracts/test/exchange/match_orders.ts
+++ b/packages/contracts/test/exchange/match_orders.ts
@@ -3,6 +3,7 @@ import { assetDataUtils } from '@0xproject/order-utils';
import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import { Web3Wrapper } from '@0xproject/web3-wrapper';
+import * as chai from 'chai';
import * as _ from 'lodash';
import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
@@ -11,8 +12,10 @@ import { ERC20ProxyContract } from '../../generated_contract_wrappers/erc20_prox
import { ERC721ProxyContract } from '../../generated_contract_wrappers/erc721_proxy';
import { ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { ReentrantERC20TokenContract } from '../../generated_contract_wrappers/reentrant_erc20_token';
+import { TestExchangeInternalsContract } from '../../generated_contract_wrappers/test_exchange_internals';
import { artifacts } from '../utils/artifacts';
import { expectTransactionFailedAsync } from '../utils/assertions';
+import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { ERC20Wrapper } from '../utils/erc20_wrapper';
import { ERC721Wrapper } from '../utils/erc721_wrapper';
@@ -23,6 +26,8 @@ import { ERC20BalancesByOwner, ERC721TokenIdsByOwner } from '../utils/types';
import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper';
const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
+chaiSetup.configure();
+const expect = chai.expect;
describe('matchOrders', () => {
let makerAddressLeft: string;
@@ -58,6 +63,8 @@ describe('matchOrders', () => {
let matchOrderTester: MatchOrderTester;
+ let testExchange: TestExchangeInternalsContract;
+
before(async () => {
await blockchainLifecycle.startAsync();
});
@@ -160,6 +167,11 @@ describe('matchOrders', () => {
orderFactoryRight = new OrderFactory(privateKeyRight, defaultOrderParamsRight);
// Set match order tester
matchOrderTester = new MatchOrderTester(exchangeWrapper, erc20Wrapper, erc721Wrapper, zrxToken.address);
+ testExchange = await TestExchangeInternalsContract.deployFrom0xArtifactAsync(
+ artifacts.TestExchangeInternals,
+ provider,
+ txDefaults,
+ );
});
beforeEach(async () => {
await blockchainLifecycle.startAsync();
@@ -173,39 +185,170 @@ describe('matchOrders', () => {
erc721TokenIdsByOwner = await erc721Wrapper.getBalancesAsync();
});
- it('Should give right maker a better price when correct price is not integral', async () => {
+ it('Should transfer correct amounts when right order is fully filled and values pass isRoundingErrorFloor but fail isRoundingErrorCeil', async () => {
+ // Create orders to match
+ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+ makerAddress: makerAddressLeft,
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(17), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(98), 0),
+ feeRecipientAddress: feeRecipientAddressLeft,
+ });
+ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
+ makerAddress: makerAddressRight,
+ makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
+ takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0),
+ feeRecipientAddress: feeRecipientAddressRight,
+ });
+ // Assert is rounding error ceil & not rounding error floor
+ // These assertions are taken from MixinMatchOrders::calculateMatchedFillResults
+ // The rounding error is derived computating how much the left maker will sell.
+ const numerator = signedOrderLeft.makerAssetAmount;
+ const denominator = signedOrderLeft.takerAssetAmount;
+ const target = signedOrderRight.makerAssetAmount;
+ const isRoundingErrorCeil = await testExchange.publicIsRoundingErrorCeil.callAsync(
+ numerator,
+ denominator,
+ target,
+ );
+ expect(isRoundingErrorCeil).to.be.true();
+ const isRoundingErrorFloor = await testExchange.publicIsRoundingErrorFloor.callAsync(
+ numerator,
+ denominator,
+ target,
+ );
+ expect(isRoundingErrorFloor).to.be.false();
+ // Match signedOrderLeft with signedOrderRight
+ // Note that the left maker received a slightly better sell price.
+ // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults.
+ // Because the left maker received a slightly more favorable sell price, the fee
+ // paid by the left taker is slightly higher than that paid by the left maker.
+ // Fees can be thought of as a tax paid by the seller, derived from the sale price.
+ const expectedTransferAmounts = {
+ // Left Maker
+ amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0),
+ amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 0),
+ feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('76.4705882352941176'), 16), // 76.47%
+ // Right Maker
+ amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 0),
+ amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0),
+ feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ // Taker
+ amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(0), 0),
+ feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber('76.5306122448979591'), 16), // 76.53%
+ feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ };
+ await matchOrderTester.matchOrdersAndAssertEffectsAsync(
+ signedOrderLeft,
+ signedOrderRight,
+ takerAddress,
+ erc20BalancesByOwner,
+ erc721TokenIdsByOwner,
+ expectedTransferAmounts,
+ );
+ });
+
+ it('Should transfer correct amounts when left order is fully filled and values pass isRoundingErrorCeil but fail isRoundingErrorFloor', async () => {
// Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
- makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0),
- takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(15), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 0),
feeRecipientAddress: feeRecipientAddressLeft,
});
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
- makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0),
- takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3000), 0),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(14), 0),
+ feeRecipientAddress: feeRecipientAddressRight,
+ });
+ // Assert is rounding error floor & not rounding error ceil
+ // These assertions are taken from MixinMatchOrders::calculateMatchedFillResults
+ // The rounding error is derived computating how much the right maker will buy.
+ const numerator = signedOrderRight.takerAssetAmount;
+ const denominator = signedOrderRight.makerAssetAmount;
+ const target = signedOrderLeft.takerAssetAmount;
+ const isRoundingErrorFloor = await testExchange.publicIsRoundingErrorFloor.callAsync(
+ numerator,
+ denominator,
+ target,
+ );
+ expect(isRoundingErrorFloor).to.be.true();
+ const isRoundingErrorCeil = await testExchange.publicIsRoundingErrorCeil.callAsync(
+ numerator,
+ denominator,
+ target,
+ );
+ expect(isRoundingErrorCeil).to.be.false();
+ // Match signedOrderLeft with signedOrderRight
+ // Note that the right maker received a slightly better purchase price.
+ // This is intentional; see note in MixinMatchOrders.calculateMatchedFillResults.
+ // Because the right maker received a slightly more favorable buy price, the fee
+ // paid by the right taker is slightly higher than that paid by the right maker.
+ // Fees can be thought of as a tax paid by the seller, derived from the sale price.
+ const expectedTransferAmounts = {
+ // Left Maker
+ amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(15), 0),
+ amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 0),
+ feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ // Right Maker
+ amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(90), 0),
+ amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0),
+ feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('92.7835051546391752'), 16), // 92.78%
+ // Taker
+ amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0),
+ feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('92.8571428571428571'), 16), // 92.85%
+ };
+ await matchOrderTester.matchOrdersAndAssertEffectsAsync(
+ signedOrderLeft,
+ signedOrderRight,
+ takerAddress,
+ erc20BalancesByOwner,
+ erc721TokenIdsByOwner,
+ expectedTransferAmounts,
+ );
+ });
+
+ it('Should give right maker a better buy price when rounding', async () => {
+ // Create orders to match
+ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+ makerAddress: makerAddressLeft,
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0),
+ feeRecipientAddress: feeRecipientAddressLeft,
+ });
+ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
+ makerAddress: makerAddressRight,
+ makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
+ takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(83), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(49), 0),
feeRecipientAddress: feeRecipientAddressRight,
});
// Note:
+ // The correct price buy price for the right maker would yield (49/83) * 22 = 12.988 units
+ // of the left maker asset. This gets rounded up to 13, giving the right maker a better price.
+ // Note:
// The maker/taker fee percentage paid on the right order differs because
- // they received different sale prices. Similarly, the right maker pays a
- // slightly higher lower than the right taker.
+ // they received different sale prices. The right maker pays a
+ // fee slightly lower than the right taker.
const expectedTransferAmounts = {
// Left Maker
- amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2000), 0),
- amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0),
+ amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0),
+ amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0),
feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
// Right Maker
- amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1001), 0),
- amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(301), 0),
- feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10.01), 16), // 10.01%
+ amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0),
+ amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0),
+ feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('26.5060240963855421'), 16), // 26.506%
// Taker
- amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1699), 0),
+ amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0),
feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
- feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('10.0333333333333333'), 16), // 10.03%
+ feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('26.5306122448979591'), 16), // 26.531%
};
// Match signedOrderLeft with signedOrderRight
await matchOrderTester.matchOrdersAndAssertEffectsAsync(
@@ -218,7 +361,7 @@ describe('matchOrders', () => {
);
});
- it('Should give left maker a better price when correct price is not integral', async () => {
+ it('Should give left maker a better sell price when rounding', async () => {
// Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
@@ -236,7 +379,8 @@ describe('matchOrders', () => {
});
// Note:
// The maker/taker fee percentage paid on the left order differs because
- // they received different sale prices.
+ // they received different sale prices. The left maker pays a fee
+ // slightly lower than the left taker.
const expectedTransferAmounts = {
// Left Maker
amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(11), 0),
@@ -262,42 +406,87 @@ describe('matchOrders', () => {
);
});
- it('Should transfer correct amounts when right order fill amount deviates from amount derived by `Exchange.fillOrder`', async () => {
+ it('Should give right maker and right taker a favorable fee price when rounding', async () => {
// Create orders to match
const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
makerAddress: makerAddressLeft,
- makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0),
- takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0),
feeRecipientAddress: feeRecipientAddressLeft,
});
const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
makerAddress: makerAddressRight,
makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
- makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(4), 0),
- takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(83), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(49), 0),
feeRecipientAddress: feeRecipientAddressRight,
+ makerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0),
+ takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0),
});
- // TODO: These values will change after implementation of rounding up has been merged
+ // Note:
+ // The maker/taker fee percentage paid on the right order differs because
+ // they received different sale prices. The right maker pays a
+ // fee slightly lower than the right taker.
const expectedTransferAmounts = {
// Left Maker
- amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0),
- amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0),
+ amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(16), 0),
+ amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0),
feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
// Right Maker
- // Note:
- // For order [4,2] valid fill amounts through `Exchange.fillOrder` would be [2, 1] or [4, 2]
- // In this case we have fill amounts of [3, 1] which is attainable through
- // `Exchange.matchOrders` but not `Exchange.fillOrder`
- // Note:
- // The right maker fee differs from the right taker fee because their exchange rate differs.
- // The right maker always receives the better exchange and fee price.
- amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0),
- amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2), 0),
- feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(75), 16), // 75%
+ amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(22), 0),
+ amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(13), 0),
+ feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(2650), 0), // 2650.6 rounded down tro 2650
// Taker
- amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(8), 0),
+ amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(3), 0),
feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(2653), 0), // 2653.1 rounded down to 2653
+ };
+ // Match signedOrderLeft with signedOrderRight
+ await matchOrderTester.matchOrdersAndAssertEffectsAsync(
+ signedOrderLeft,
+ signedOrderRight,
+ takerAddress,
+ erc20BalancesByOwner,
+ erc721TokenIdsByOwner,
+ expectedTransferAmounts,
+ );
+ });
+
+ it('Should give left maker and left taker a favorable fee price when rounding', async () => {
+ // Create orders to match
+ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+ makerAddress: makerAddressLeft,
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(12), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(97), 0),
+ feeRecipientAddress: feeRecipientAddressLeft,
+ makerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0),
+ takerFee: Web3Wrapper.toBaseUnitAmount(new BigNumber(10000), 0),
+ });
+ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
+ makerAddress: makerAddressRight,
+ makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
+ takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0),
+ feeRecipientAddress: feeRecipientAddressRight,
+ });
+ // Note:
+ // The maker/taker fee percentage paid on the left order differs because
+ // they received different sale prices. The left maker pays a
+ // fee slightly lower than the left taker.
+ const expectedTransferAmounts = {
+ // Left Maker
+ amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(11), 0),
+ amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0),
+ feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(9166), 0), // 9166.6 rounded down to 9166
+ // Right Maker
+ amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(89), 0),
+ amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1), 0),
+ feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ // Taker
+ amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(10), 0),
+ feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(9175), 0), // 9175.2 rounded down to 9175
feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
};
// Match signedOrderLeft with signedOrderRight
@@ -311,6 +500,61 @@ describe('matchOrders', () => {
);
});
+ it('Should transfer correct amounts when right order fill amount deviates from amount derived by `Exchange.fillOrder`', async () => {
+ // Create orders to match
+ const signedOrderLeft = await orderFactoryLeft.newSignedOrderAsync({
+ makerAddress: makerAddressLeft,
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1005), 0),
+ feeRecipientAddress: feeRecipientAddressLeft,
+ });
+ const signedOrderRight = await orderFactoryRight.newSignedOrderAsync({
+ makerAddress: makerAddressRight,
+ makerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20TakerAssetAddress),
+ takerAssetData: assetDataUtils.encodeERC20AssetData(defaultERC20MakerAssetAddress),
+ makerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(2126), 0),
+ takerAssetAmount: Web3Wrapper.toBaseUnitAmount(new BigNumber(1063), 0),
+ feeRecipientAddress: feeRecipientAddressRight,
+ });
+ const expectedTransferAmounts = {
+ // Left Maker
+ amountSoldByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1000), 0),
+ amountBoughtByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1005), 0),
+ feePaidByLeftMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ // Right Maker
+ // Notes:
+ // i.
+ // The left order is fully filled by the right order, so the right maker must sell 1005 units of their asset to the left maker.
+ // By selling 1005 units, the right maker should theoretically receive 502.5 units of the left maker's asset.
+ // Since the transfer amount must be an integer, this value must be rounded down to 502 or up to 503.
+ // ii.
+ // If the right order were filled via `Exchange.fillOrder` the respective fill amounts would be [1004, 502] or [1006, 503].
+ // It follows that we cannot trigger a sale of 1005 units of the right maker's asset through `Exchange.fillOrder`.
+ // iii.
+ // For an optimal match, the algorithm must choose either [1005, 502] or [1005, 503] as fill amounts for the right order.
+ // The algorithm favors the right maker when the exchange rate must be rounded, so the final fill for the right order is [1005, 503].
+ // iv.
+ // The right maker fee differs from the right taker fee because their exchange rate differs.
+ // The right maker always receives the better exchange and fee price.
+ amountSoldByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(1005), 0),
+ amountBoughtByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(503), 0),
+ feePaidByRightMaker: Web3Wrapper.toBaseUnitAmount(new BigNumber('47.2718720602069614'), 16), // 47.27%
+ // Taker
+ amountReceivedByTaker: Web3Wrapper.toBaseUnitAmount(new BigNumber(497), 0),
+ feePaidByTakerLeft: Web3Wrapper.toBaseUnitAmount(new BigNumber(100), 16), // 100%
+ feePaidByTakerRight: Web3Wrapper.toBaseUnitAmount(new BigNumber('47.3189087488240827'), 16), // 47.31%
+ };
+ // Match signedOrderLeft with signedOrderRight
+ await matchOrderTester.matchOrdersAndAssertEffectsAsync(
+ signedOrderLeft,
+ signedOrderRight,
+ takerAddress,
+ erc20BalancesByOwner,
+ erc721TokenIdsByOwner,
+ expectedTransferAmounts,
+ );
+ });
+
const reentrancyTest = (functionNames: string[]) => {
_.forEach(functionNames, async (functionName: string, functionId: number) => {
const description = `should not allow matchOrders to reenter the Exchange contract via ${functionName}`;
diff --git a/packages/contracts/test/exchange/signature_validator.ts b/packages/contracts/test/exchange/signature_validator.ts
index da2febfd8..5cc62e777 100644
--- a/packages/contracts/test/exchange/signature_validator.ts
+++ b/packages/contracts/test/exchange/signature_validator.ts
@@ -14,7 +14,7 @@ import { ValidatorContract } from '../../generated_contract_wrappers/validator';
import { WalletContract } from '../../generated_contract_wrappers/wallet';
import { addressUtils } from '../utils/address_utils';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed, expectContractCallFailedWithoutReasonAsync } from '../utils/assertions';
+import { expectContractCallFailedAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { LogDecoder } from '../utils/log_decoder';
@@ -119,7 +119,7 @@ describe('MixinSignatureValidator', () => {
it('should revert when signature is empty', async () => {
const emptySignature = '0x';
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -133,7 +133,7 @@ describe('MixinSignatureValidator', () => {
const unsupportedSignatureType = SignatureType.NSignatureTypes;
const unsupportedSignatureHex = '0x' + Buffer.from([unsupportedSignatureType]).toString('hex');
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -146,7 +146,7 @@ describe('MixinSignatureValidator', () => {
it('should revert when SignatureType=Illegal', async () => {
const unsupportedSignatureHex = '0x' + Buffer.from([SignatureType.Illegal]).toString('hex');
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -173,7 +173,7 @@ describe('MixinSignatureValidator', () => {
const signatureBuffer = Buffer.concat([fillerData, signatureType]);
const signatureHex = ethUtil.bufferToHex(signatureBuffer);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
signedOrder.makerAddress,
@@ -339,7 +339,7 @@ describe('MixinSignatureValidator', () => {
ethUtil.toBuffer(`0x${SignatureType.Wallet}`),
]);
const signatureHex = ethUtil.bufferToHex(signature);
- await expectContractCallFailed(
+ await expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(
orderHashHex,
maliciousWallet.address,
@@ -385,7 +385,7 @@ describe('MixinSignatureValidator', () => {
const signature = Buffer.concat([validatorAddress, signatureType]);
const signatureHex = ethUtil.bufferToHex(signature);
const orderHashHex = orderHashUtils.getOrderHashHex(signedOrder);
- await expectContractCallFailed(
+ await expectContractCallFailedAsync(
signatureValidator.publicIsValidSignature.callAsync(orderHashHex, signerAddress, signatureHex),
RevertReason.ValidatorError,
);
diff --git a/packages/contracts/test/extensions/forwarder.ts b/packages/contracts/test/extensions/forwarder.ts
index 18101d684..8424d01fd 100644
--- a/packages/contracts/test/extensions/forwarder.ts
+++ b/packages/contracts/test/extensions/forwarder.ts
@@ -12,7 +12,11 @@ import { ExchangeContract } from '../../generated_contract_wrappers/exchange';
import { ForwarderContract } from '../../generated_contract_wrappers/forwarder';
import { WETH9Contract } from '../../generated_contract_wrappers/weth9';
import { artifacts } from '../utils/artifacts';
-import { expectTransactionFailedAsync } from '../utils/assertions';
+import {
+ expectContractCreationFailedAsync,
+ expectTransactionFailedAsync,
+ sendTransactionResult,
+} from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { ERC20Wrapper } from '../utils/erc20_wrapper';
@@ -37,6 +41,7 @@ describe(ContractName.Forwarder, () => {
let otherAddress: string;
let defaultMakerAssetAddress: string;
let zrxAssetData: string;
+ let wethAssetData: string;
let weth: DummyERC20TokenContract;
let zrxToken: DummyERC20TokenContract;
@@ -90,7 +95,7 @@ describe(ContractName.Forwarder, () => {
weth = new DummyERC20TokenContract(wethContract.abi, wethContract.address, provider);
erc20Wrapper.addDummyTokenContract(weth);
- const wethAssetData = assetDataUtils.encodeERC20AssetData(wethContract.address);
+ wethAssetData = assetDataUtils.encodeERC20AssetData(wethContract.address);
zrxAssetData = assetDataUtils.encodeERC20AssetData(zrxToken.address);
const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync(
artifacts.Exchange,
@@ -98,8 +103,7 @@ describe(ContractName.Forwarder, () => {
txDefaults,
zrxAssetData,
);
- const exchangeContract = new ExchangeContract(exchangeInstance.abi, exchangeInstance.address, provider);
- exchangeWrapper = new ExchangeWrapper(exchangeContract, provider);
+ exchangeWrapper = new ExchangeWrapper(exchangeInstance, provider);
await exchangeWrapper.registerAssetProxyAsync(erc20Proxy.address, owner);
await exchangeWrapper.registerAssetProxyAsync(erc721Proxy.address, owner);
@@ -162,6 +166,27 @@ describe(ContractName.Forwarder, () => {
await blockchainLifecycle.revertAsync();
});
+ describe('constructor', () => {
+ it('should revert if assetProxy is unregistered', async () => {
+ const exchangeInstance = await ExchangeContract.deployFrom0xArtifactAsync(
+ artifacts.Exchange,
+ provider,
+ txDefaults,
+ zrxAssetData,
+ );
+ return expectContractCreationFailedAsync(
+ (ForwarderContract.deployFrom0xArtifactAsync(
+ artifacts.Forwarder,
+ provider,
+ txDefaults,
+ exchangeInstance.address,
+ zrxAssetData,
+ wethAssetData,
+ ) as any) as sendTransactionResult,
+ RevertReason.UnregisteredAssetProxy,
+ );
+ });
+ });
describe('marketSellOrdersWithEth without extra fees', () => {
it('should fill a single order', async () => {
const ordersWithoutFee = [orderWithoutFee];
diff --git a/packages/contracts/test/libraries/lib_bytes.ts b/packages/contracts/test/libraries/lib_bytes.ts
index 1c497a226..13640a761 100644
--- a/packages/contracts/test/libraries/lib_bytes.ts
+++ b/packages/contracts/test/libraries/lib_bytes.ts
@@ -9,7 +9,7 @@ import * as _ from 'lodash';
import { TestLibBytesContract } from '../../generated_contract_wrappers/test_lib_bytes';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed } from '../utils/assertions';
+import { expectContractCallFailedAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { typeEncodingUtils } from '../utils/type_encoding_utils';
@@ -41,6 +41,8 @@ describe('LibBytes', () => {
const testBytes32B = '0x534877abd8443578526845cdfef020047528759477fedef87346527659aced32';
const testUint256 = new BigNumber(testBytes32, 16);
const testUint256B = new BigNumber(testBytes32B, 16);
+ const testBytes4 = '0xabcdef12';
+ const testByte = '0xab';
let shortData: string;
let shortTestBytes: string;
let shortTestBytesAsBuffer: Buffer;
@@ -101,34 +103,47 @@ describe('LibBytes', () => {
describe('popLastByte', () => {
it('should revert if length is 0', async () => {
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicPopLastByte.callAsync(constants.NULL_BYTES),
RevertReason.LibBytesGreaterThanZeroLengthRequired,
);
});
- it('should pop the last byte from the input and return it', async () => {
+ it('should pop the last byte from the input and return it when array holds more than 1 byte', async () => {
const [newBytes, poppedByte] = await libBytes.publicPopLastByte.callAsync(byteArrayLongerThan32Bytes);
const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -2);
const expectedPoppedByte = `0x${byteArrayLongerThan32Bytes.slice(-2)}`;
expect(newBytes).to.equal(expectedNewBytes);
expect(poppedByte).to.equal(expectedPoppedByte);
});
+ it('should pop the last byte from the input and return it when array is exactly 1 byte', async () => {
+ const [newBytes, poppedByte] = await libBytes.publicPopLastByte.callAsync(testByte);
+ const expectedNewBytes = '0x';
+ expect(newBytes).to.equal(expectedNewBytes);
+ return expect(poppedByte).to.be.equal(testByte);
+ });
});
describe('popLast20Bytes', () => {
it('should revert if length is less than 20', async () => {
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicPopLast20Bytes.callAsync(byteArrayShorterThan20Bytes),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
});
- it('should pop the last 20 bytes from the input and return it', async () => {
+ it('should pop the last 20 bytes from the input and return it when array holds more than 20 bytes', async () => {
const [newBytes, poppedAddress] = await libBytes.publicPopLast20Bytes.callAsync(byteArrayLongerThan32Bytes);
const expectedNewBytes = byteArrayLongerThan32Bytes.slice(0, -40);
const expectedPoppedAddress = `0x${byteArrayLongerThan32Bytes.slice(-40)}`;
expect(newBytes).to.equal(expectedNewBytes);
expect(poppedAddress).to.equal(expectedPoppedAddress);
});
+ it('should pop the last 20 bytes from the input and return it when array is exactly 20 bytes', async () => {
+ const [newBytes, poppedAddress] = await libBytes.publicPopLast20Bytes.callAsync(testAddress);
+ const expectedNewBytes = '0x';
+ const expectedPoppedAddress = testAddress;
+ expect(newBytes).to.equal(expectedNewBytes);
+ expect(poppedAddress).to.equal(expectedPoppedAddress);
+ });
});
describe('equals', () => {
@@ -185,7 +200,7 @@ describe('LibBytes', () => {
describe('deepCopyBytes', () => {
it('should revert if dest is shorter than source', async () => {
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicDeepCopyBytes.callAsync(byteArrayShorterThan32Bytes, byteArrayLongerThan32Bytes),
RevertReason.LibBytesGreaterOrEqualToSourceBytesLengthRequired,
);
@@ -238,7 +253,7 @@ describe('LibBytes', () => {
it('should fail if the byte array is too short to hold an address', async () => {
const shortByteArray = '0xabcdef';
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadAddress.callAsync(shortByteArray, offset),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -246,7 +261,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => {
const byteArray = testAddress;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadAddress.callAsync(byteArray, badOffset),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -282,7 +297,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold an address', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteAddress.callAsync(byteArrayShorterThan20Bytes, offset, testAddress),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -290,7 +305,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold an address', async () => {
const byteArray = byteArrayLongerThan32Bytes;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteAddress.callAsync(byteArray, badOffset, testAddress),
RevertReason.LibBytesGreaterOrEqualTo20LengthRequired,
);
@@ -303,7 +318,7 @@ describe('LibBytes', () => {
const bytes32 = await libBytes.publicReadBytes32.callAsync(testBytes32, testBytes32Offset);
return expect(bytes32).to.be.equal(testBytes32);
});
- it('should successfully read bytes32 when it is offset in the array)', async () => {
+ it('should successfully read bytes32 when it is offset in the array', async () => {
const bytes32ByteArrayBuffer = ethUtil.toBuffer(testBytes32);
const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef');
const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes32ByteArrayBuffer]);
@@ -314,14 +329,14 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a bytes32', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytes32.callAsync(byteArrayShorterThan32Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
});
it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytes32.callAsync(testBytes32, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -357,7 +372,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a bytes32', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytes32.callAsync(byteArrayShorterThan32Bytes, offset, testBytes32),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -365,7 +380,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold a bytes32', async () => {
const byteArray = byteArrayLongerThan32Bytes;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytes32.callAsync(byteArray, badOffset, testBytes32),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -393,7 +408,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a uint256', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadUint256.callAsync(byteArrayShorterThan32Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -403,7 +418,7 @@ describe('LibBytes', () => {
const testUint256AsBuffer = ethUtil.toBuffer(formattedTestUint256);
const byteArray = ethUtil.bufferToHex(testUint256AsBuffer);
const badOffset = new BigNumber(testUint256AsBuffer.byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadUint256.callAsync(byteArray, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -443,7 +458,7 @@ describe('LibBytes', () => {
});
it('should fail if the byte array is too short to hold a uint256', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteUint256.callAsync(byteArrayShorterThan32Bytes, offset, testUint256),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -451,7 +466,7 @@ describe('LibBytes', () => {
it('should fail if the length between the offset and end of the byte array is too short to hold a uint256', async () => {
const byteArray = byteArrayLongerThan32Bytes;
const badOffset = new BigNumber(ethUtil.toBuffer(byteArray).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteUint256.callAsync(byteArray, badOffset, testUint256),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -462,8 +477,9 @@ describe('LibBytes', () => {
// AssertionError: expected promise to be rejected with an error including 'revert' but it was fulfilled with '0x08c379a0'
it('should revert if byte array has a length < 4', async () => {
const byteArrayLessThan4Bytes = '0x010101';
- return expectContractCallFailed(
- libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)),
+ const offset = new BigNumber(0);
+ return expectContractCallFailedAsync(
+ libBytes.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo4LengthRequired,
);
});
@@ -472,6 +488,27 @@ describe('LibBytes', () => {
const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10);
expect(first4Bytes).to.equal(expectedFirst4Bytes);
});
+ it('should successfully read bytes4 when the bytes4 takes up the whole array', async () => {
+ const testBytes4Offset = new BigNumber(0);
+ const bytes4 = await libBytes.publicReadBytes4.callAsync(testBytes4, testBytes4Offset);
+ return expect(bytes4).to.be.equal(testBytes4);
+ });
+ it('should successfully read bytes4 when it is offset in the array', async () => {
+ const bytes4ByteArrayBuffer = ethUtil.toBuffer(testBytes4);
+ const prefixByteArrayBuffer = ethUtil.toBuffer('0xabcdef');
+ const combinedByteArrayBuffer = Buffer.concat([prefixByteArrayBuffer, bytes4ByteArrayBuffer]);
+ const combinedByteArray = ethUtil.bufferToHex(combinedByteArrayBuffer);
+ const testBytes4Offset = new BigNumber(prefixByteArrayBuffer.byteLength);
+ const bytes4 = await libBytes.publicReadBytes4.callAsync(combinedByteArray, testBytes4Offset);
+ return expect(bytes4).to.be.equal(testBytes4);
+ });
+ it('should fail if the length between the offset and end of the byte array is too short to hold a bytes4', async () => {
+ const badOffset = new BigNumber(ethUtil.toBuffer(testBytes4).byteLength);
+ return expectContractCallFailedAsync(
+ libBytes.publicReadBytes4.callAsync(testBytes4, badOffset),
+ RevertReason.LibBytesGreaterOrEqualTo4LengthRequired,
+ );
+ });
});
describe('readBytesWithLength', () => {
@@ -517,28 +554,28 @@ describe('LibBytes', () => {
it('should fail if the byte array is too short to hold the length of a nested byte array', async () => {
// The length of the nested array is 32 bytes. By storing less than 32 bytes, a length cannot be read.
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, offset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
});
it('should fail if we store a nested byte array length, without a nested byte array', async () => {
const offset = new BigNumber(0);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(testBytes32, offset),
RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired,
);
});
it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(byteArrayShorterThan32Bytes).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(byteArrayShorterThan32Bytes, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
});
it('should fail if the length between the offset and end of the byte array is too short to hold the nested byte array', async () => {
const badOffset = new BigNumber(ethUtil.toBuffer(testBytes32).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicReadBytesWithLength.callAsync(testBytes32, badOffset),
RevertReason.LibBytesGreaterOrEqualTo32LengthRequired,
);
@@ -546,7 +583,7 @@ describe('LibBytes', () => {
});
describe('writeBytesWithLength', () => {
- it('should successfully write short, nested array of bytes when it takes up the whole array)', async () => {
+ it('should successfully write short, nested array of bytes when it takes up the whole array', async () => {
const testBytesOffset = new BigNumber(0);
const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength));
const bytesWritten = await libBytes.publicWriteBytesWithLength.callAsync(
@@ -650,15 +687,15 @@ describe('LibBytes', () => {
it('should fail if the byte array is too short to hold the length of a nested byte array', async () => {
const offset = new BigNumber(0);
const emptyByteArray = ethUtil.bufferToHex(new Buffer(1));
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, offset, longData),
RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired,
);
});
- it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array)', async () => {
+ it('should fail if the length between the offset and end of the byte array is too short to hold the length of a nested byte array', async () => {
const emptyByteArray = ethUtil.bufferToHex(new Buffer(shortTestBytesAsBuffer.byteLength));
const badOffset = new BigNumber(ethUtil.toBuffer(shortTestBytesAsBuffer).byteLength);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
libBytes.publicWriteBytesWithLength.callAsync(emptyByteArray, badOffset, shortData),
RevertReason.LibBytesGreaterOrEqualToNestedBytesLengthRequired,
);
diff --git a/packages/contracts/test/multisig/asset_proxy_owner.ts b/packages/contracts/test/multisig/asset_proxy_owner.ts
index 9515941ff..299707512 100644
--- a/packages/contracts/test/multisig/asset_proxy_owner.ts
+++ b/packages/contracts/test/multisig/asset_proxy_owner.ts
@@ -1,4 +1,5 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
+import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
@@ -14,9 +15,11 @@ import { MixinAuthorizableContract } from '../../generated_contract_wrappers/mix
import { TestAssetProxyOwnerContract } from '../../generated_contract_wrappers/test_asset_proxy_owner';
import { artifacts } from '../utils/artifacts';
import {
- expectContractCallFailedWithoutReasonAsync,
- expectContractCreationFailedWithoutReason,
+ expectContractCallFailedAsync,
+ expectContractCreationFailedAsync,
+ expectTransactionFailedAsync,
expectTransactionFailedWithoutReasonAsync,
+ sendTransactionResult,
} from '../utils/assertions';
import { increaseTimeAndMineBlockAsync } from '../utils/block_timestamp';
import { chaiSetup } from '../utils/chai_setup';
@@ -31,6 +34,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
describe('AssetProxyOwner', () => {
let owners: string[];
let authorized: string;
+ let notOwner: string;
const REQUIRED_APPROVALS = new BigNumber(2);
const SECONDS_TIME_LOCKED = new BigNumber(1000000);
@@ -48,7 +52,9 @@ describe('AssetProxyOwner', () => {
before(async () => {
const accounts = await web3Wrapper.getAvailableAddressesAsync();
owners = [accounts[0], accounts[1]];
- const initialOwner = (authorized = accounts[0]);
+ authorized = accounts[2];
+ notOwner = accounts[3];
+ const initialOwner = accounts[0];
erc20Proxy = await MixinAuthorizableContract.deployFrom0xArtifactAsync(
artifacts.MixinAuthorizable,
provider,
@@ -109,8 +115,8 @@ describe('AssetProxyOwner', () => {
});
it('should throw if a null address is included in assetProxyContracts', async () => {
const assetProxyContractAddresses = [erc20Proxy.address, constants.NULL_ADDRESS];
- return expectContractCreationFailedWithoutReason(
- AssetProxyOwnerContract.deployFrom0xArtifactAsync(
+ return expectContractCreationFailedAsync(
+ (AssetProxyOwnerContract.deployFrom0xArtifactAsync(
artifacts.AssetProxyOwner,
provider,
txDefaults,
@@ -118,7 +124,8 @@ describe('AssetProxyOwner', () => {
assetProxyContractAddresses,
REQUIRED_APPROVALS,
SECONDS_TIME_LOCKED,
- ),
+ ) as any) as sendTransactionResult,
+ RevertReason.InvalidAssetProxy,
);
});
});
@@ -148,25 +155,6 @@ describe('AssetProxyOwner', () => {
});
});
- describe('readBytes4', () => {
- it('should revert if byte array has a length < 4', async () => {
- const byteArrayLessThan4Bytes = '0x010101';
- return expectContractCallFailedWithoutReasonAsync(
- testAssetProxyOwner.publicReadBytes4.callAsync(byteArrayLessThan4Bytes, new BigNumber(0)),
- );
- });
- it('should return the first 4 bytes of a byte array of arbitrary length', async () => {
- const byteArrayLongerThan32Bytes =
- '0x0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef';
- const first4Bytes = await testAssetProxyOwner.publicReadBytes4.callAsync(
- byteArrayLongerThan32Bytes,
- new BigNumber(0),
- );
- const expectedFirst4Bytes = byteArrayLongerThan32Bytes.slice(0, 10);
- expect(first4Bytes).to.equal(expectedFirst4Bytes);
- });
- });
-
describe('registerAssetProxy', () => {
it('should throw if not called by multisig', async () => {
const isRegistered = true;
@@ -284,8 +272,12 @@ describe('AssetProxyOwner', () => {
await multiSigWrapper.confirmTransactionAsync(erc721AddAuthorizedAddressTxId, owners[1]);
await increaseTimeAndMineBlockAsync(SECONDS_TIME_LOCKED.toNumber());
await multiSigWrapper.executeTransactionAsync(registerAssetProxyTxId, owners[0]);
- await multiSigWrapper.executeTransactionAsync(erc20AddAuthorizedAddressTxId, owners[0]);
- await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0]);
+ await multiSigWrapper.executeTransactionAsync(erc20AddAuthorizedAddressTxId, owners[0], {
+ gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
+ });
+ await multiSigWrapper.executeTransactionAsync(erc721AddAuthorizedAddressTxId, owners[0], {
+ gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
+ });
});
describe('validRemoveAuthorizedAddressAtIndexTx', () => {
@@ -300,8 +292,9 @@ describe('AssetProxyOwner', () => {
);
const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>;
const txId = log.args.transactionId;
- return expectContractCallFailedWithoutReasonAsync(
+ return expectContractCallFailedAsync(
testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId),
+ RevertReason.InvalidFunctionSelector,
);
});
@@ -335,8 +328,9 @@ describe('AssetProxyOwner', () => {
);
const log = submitTxRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>;
const txId = log.args.transactionId;
- return expectContractCallFailedWithoutReasonAsync(
+ return expectContractCallFailedAsync(
testAssetProxyOwner.testValidRemoveAuthorizedAddressAtIndexTx.callAsync(txId),
+ RevertReason.UnregisteredAssetProxy,
);
});
});
@@ -355,10 +349,11 @@ describe('AssetProxyOwner', () => {
const log = res.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>;
const txId = log.args.transactionId;
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, {
from: owners[1],
}),
+ RevertReason.TxNotFullyConfirmed,
);
});
@@ -377,10 +372,11 @@ describe('AssetProxyOwner', () => {
await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, {
from: owners[1],
}),
+ RevertReason.UnregisteredAssetProxy,
);
});
@@ -399,14 +395,18 @@ describe('AssetProxyOwner', () => {
await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
testAssetProxyOwner.executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, {
from: owners[1],
}),
+ RevertReason.InvalidFunctionSelector,
);
});
- it('should execute removeAuthorizedAddressAtIndex for registered address if fully confirmed', async () => {
+ it('should execute removeAuthorizedAddressAtIndex for registered address if fully confirmed and called by owner', async () => {
+ const isAuthorizedBefore = await erc20Proxy.authorized.callAsync(authorized);
+ expect(isAuthorizedBefore).to.equal(true);
+
const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData(
authorized,
erc20Index,
@@ -429,8 +429,38 @@ describe('AssetProxyOwner', () => {
const isExecuted = tx[3];
expect(isExecuted).to.equal(true);
- const isAuthorized = await erc20Proxy.authorized.callAsync(authorized);
- expect(isAuthorized).to.equal(false);
+ const isAuthorizedAfter = await erc20Proxy.authorized.callAsync(authorized);
+ expect(isAuthorizedAfter).to.equal(false);
+ });
+
+ it('should execute removeAuthorizedAddressAtIndex for registered address if fully confirmed and called by non-owner', async () => {
+ const isAuthorizedBefore = await erc20Proxy.authorized.callAsync(authorized);
+ expect(isAuthorizedBefore).to.equal(true);
+
+ const removeAuthorizedAddressAtIndexData = erc20Proxy.removeAuthorizedAddressAtIndex.getABIEncodedTransactionData(
+ authorized,
+ erc20Index,
+ );
+ const submitRes = await multiSigWrapper.submitTransactionAsync(
+ erc20Proxy.address,
+ removeAuthorizedAddressAtIndexData,
+ owners[0],
+ );
+ const submitLog = submitRes.logs[0] as LogWithDecodedArgs<AssetProxyOwnerSubmissionEventArgs>;
+ const txId = submitLog.args.transactionId;
+
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+
+ const execRes = await multiSigWrapper.executeRemoveAuthorizedAddressAtIndexAsync(txId, notOwner);
+ const execLog = execRes.logs[1] as LogWithDecodedArgs<AssetProxyOwnerExecutionEventArgs>;
+ expect(execLog.args.transactionId).to.be.bignumber.equal(txId);
+
+ const tx = await testAssetProxyOwner.transactions.callAsync(txId);
+ const isExecuted = tx[3];
+ expect(isExecuted).to.equal(true);
+
+ const isAuthorizedAfter = await erc20Proxy.authorized.callAsync(authorized);
+ expect(isAuthorizedAfter).to.equal(false);
});
it('should throw if already executed', async () => {
diff --git a/packages/contracts/test/multisig/multi_sig_with_time_lock.ts b/packages/contracts/test/multisig/multi_sig_with_time_lock.ts
index 8eeeeca6b..0b17c298b 100644
--- a/packages/contracts/test/multisig/multi_sig_with_time_lock.ts
+++ b/packages/contracts/test/multisig/multi_sig_with_time_lock.ts
@@ -1,14 +1,21 @@
import { BlockchainLifecycle } from '@0xproject/dev-utils';
+import { RevertReason } from '@0xproject/types';
import { BigNumber } from '@0xproject/utils';
import * as chai from 'chai';
import { LogWithDecodedArgs } from 'ethereum-types';
+import * as _ from 'lodash';
+import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
import {
+ MultiSigWalletWithTimeLockConfirmationEventArgs,
+ MultiSigWalletWithTimeLockConfirmationTimeSetEventArgs,
MultiSigWalletWithTimeLockContract,
+ MultiSigWalletWithTimeLockExecutionEventArgs,
+ MultiSigWalletWithTimeLockExecutionFailureEventArgs,
MultiSigWalletWithTimeLockSubmissionEventArgs,
} from '../../generated_contract_wrappers/multi_sig_wallet_with_time_lock';
import { artifacts } from '../utils/artifacts';
-import { expectTransactionFailedWithoutReasonAsync } from '../utils/assertions';
+import { expectTransactionFailedAsync, expectTransactionFailedWithoutReasonAsync } from '../utils/assertions';
import { increaseTimeAndMineBlockAsync } from '../utils/block_timestamp';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
@@ -21,6 +28,7 @@ const blockchainLifecycle = new BlockchainLifecycle(web3Wrapper);
// tslint:disable:no-unnecessary-type-assertion
describe('MultiSigWalletWithTimeLock', () => {
let owners: string[];
+ let notOwner: string;
const REQUIRED_APPROVALS = new BigNumber(2);
const SECONDS_TIME_LOCKED = new BigNumber(1000000);
@@ -32,7 +40,8 @@ describe('MultiSigWalletWithTimeLock', () => {
});
before(async () => {
const accounts = await web3Wrapper.getAvailableAddressesAsync();
- owners = [accounts[0], accounts[1]];
+ owners = [accounts[0], accounts[1], accounts[2]];
+ notOwner = accounts[3];
});
let multiSig: MultiSigWalletWithTimeLockContract;
@@ -45,6 +54,171 @@ describe('MultiSigWalletWithTimeLock', () => {
await blockchainLifecycle.revertAsync();
});
+ describe('external_call', () => {
+ it('should be internal', async () => {
+ const secondsTimeLocked = new BigNumber(0);
+ multiSig = await MultiSigWalletWithTimeLockContract.deployFrom0xArtifactAsync(
+ artifacts.MultiSigWalletWithTimeLock,
+ provider,
+ txDefaults,
+ owners,
+ REQUIRED_APPROVALS,
+ secondsTimeLocked,
+ );
+ expect(_.isUndefined((multiSig as any).external_call)).to.be.equal(true);
+ });
+ });
+ describe('confirmTransaction', () => {
+ let txId: BigNumber;
+ beforeEach(async () => {
+ const secondsTimeLocked = new BigNumber(0);
+ multiSig = await MultiSigWalletWithTimeLockContract.deployFrom0xArtifactAsync(
+ artifacts.MultiSigWalletWithTimeLock,
+ provider,
+ txDefaults,
+ owners,
+ REQUIRED_APPROVALS,
+ secondsTimeLocked,
+ );
+ multiSigWrapper = new MultiSigWrapper(multiSig, provider);
+ const destination = notOwner;
+ const data = constants.NULL_BYTES;
+ const txReceipt = await multiSigWrapper.submitTransactionAsync(destination, data, owners[0]);
+ txId = (txReceipt.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockSubmissionEventArgs>).args
+ .transactionId;
+ });
+ it('should revert if called by a non-owner', async () => {
+ await expectTransactionFailedWithoutReasonAsync(multiSigWrapper.confirmTransactionAsync(txId, notOwner));
+ });
+ it('should revert if transaction does not exist', async () => {
+ const nonexistentTxId = new BigNumber(123456789);
+ await expectTransactionFailedWithoutReasonAsync(
+ multiSigWrapper.confirmTransactionAsync(nonexistentTxId, owners[1]),
+ );
+ });
+ it('should revert if transaction is already confirmed by caller', async () => {
+ await expectTransactionFailedWithoutReasonAsync(multiSigWrapper.confirmTransactionAsync(txId, owners[0]));
+ });
+ it('should confirm transaction for caller and log a Confirmation event', async () => {
+ const txReceipt = await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ const log = txReceipt.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockConfirmationEventArgs>;
+ expect(log.event).to.be.equal('Confirmation');
+ expect(log.args.sender).to.be.equal(owners[1]);
+ expect(log.args.transactionId).to.be.bignumber.equal(txId);
+ });
+ it('should revert if fully confirmed', async () => {
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ await expectTransactionFailedAsync(
+ multiSigWrapper.confirmTransactionAsync(txId, owners[2]),
+ RevertReason.TxFullyConfirmed,
+ );
+ });
+ it('should set the confirmation time of the transaction if it becomes fully confirmed', async () => {
+ const txReceipt = await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ const blockNum = await web3Wrapper.getBlockNumberAsync();
+ const timestamp = new BigNumber(await web3Wrapper.getBlockTimestampAsync(blockNum));
+ const log = txReceipt.logs[1] as LogWithDecodedArgs<MultiSigWalletWithTimeLockConfirmationTimeSetEventArgs>;
+ expect(log.args.confirmationTime).to.be.bignumber.equal(timestamp);
+ expect(log.args.transactionId).to.be.bignumber.equal(txId);
+ });
+ });
+ describe('executeTransaction', () => {
+ let txId: BigNumber;
+ const secondsTimeLocked = new BigNumber(1000000);
+ beforeEach(async () => {
+ multiSig = await MultiSigWalletWithTimeLockContract.deployFrom0xArtifactAsync(
+ artifacts.MultiSigWalletWithTimeLock,
+ provider,
+ txDefaults,
+ owners,
+ REQUIRED_APPROVALS,
+ secondsTimeLocked,
+ );
+ multiSigWrapper = new MultiSigWrapper(multiSig, provider);
+ const destination = notOwner;
+ const data = constants.NULL_BYTES;
+ const txReceipt = await multiSigWrapper.submitTransactionAsync(destination, data, owners[0]);
+ txId = (txReceipt.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockSubmissionEventArgs>).args
+ .transactionId;
+ });
+ it('should revert if transaction has not been fully confirmed', async () => {
+ await increaseTimeAndMineBlockAsync(secondsTimeLocked.toNumber());
+ await expectTransactionFailedAsync(
+ multiSigWrapper.executeTransactionAsync(txId, owners[1]),
+ RevertReason.TxNotFullyConfirmed,
+ );
+ });
+ it('should revert if time lock has not passed', async () => {
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ await expectTransactionFailedAsync(
+ multiSigWrapper.executeTransactionAsync(txId, owners[1]),
+ RevertReason.TimeLockIncomplete,
+ );
+ });
+ it('should execute a transaction and log an Execution event if successful and called by owner', async () => {
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ await increaseTimeAndMineBlockAsync(secondsTimeLocked.toNumber());
+ const txReceipt = await multiSigWrapper.executeTransactionAsync(txId, owners[1]);
+ const log = txReceipt.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockExecutionEventArgs>;
+ expect(log.event).to.be.equal('Execution');
+ expect(log.args.transactionId).to.be.bignumber.equal(txId);
+ });
+ it('should execute a transaction and log an Execution event if successful and called by non-owner', async () => {
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ await increaseTimeAndMineBlockAsync(secondsTimeLocked.toNumber());
+ const txReceipt = await multiSigWrapper.executeTransactionAsync(txId, notOwner);
+ const log = txReceipt.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockExecutionEventArgs>;
+ expect(log.event).to.be.equal('Execution');
+ expect(log.args.transactionId).to.be.bignumber.equal(txId);
+ });
+ it('should revert if a required confirmation is revoked before executeTransaction is called', async () => {
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ await increaseTimeAndMineBlockAsync(secondsTimeLocked.toNumber());
+ await multiSigWrapper.revokeConfirmationAsync(txId, owners[0]);
+ await expectTransactionFailedAsync(
+ multiSigWrapper.executeTransactionAsync(txId, owners[1]),
+ RevertReason.TxNotFullyConfirmed,
+ );
+ });
+ it('should revert if transaction has been executed', async () => {
+ await multiSigWrapper.confirmTransactionAsync(txId, owners[1]);
+ await increaseTimeAndMineBlockAsync(secondsTimeLocked.toNumber());
+ const txReceipt = await multiSigWrapper.executeTransactionAsync(txId, owners[1]);
+ const log = txReceipt.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockExecutionEventArgs>;
+ expect(log.args.transactionId).to.be.bignumber.equal(txId);
+ await expectTransactionFailedWithoutReasonAsync(multiSigWrapper.executeTransactionAsync(txId, owners[1]));
+ });
+ it("should log an ExecutionFailure event and not update the transaction's execution state if unsuccessful", async () => {
+ const contractWithoutFallback = await DummyERC20TokenContract.deployFrom0xArtifactAsync(
+ artifacts.DummyERC20Token,
+ provider,
+ txDefaults,
+ constants.DUMMY_TOKEN_NAME,
+ constants.DUMMY_TOKEN_SYMBOL,
+ constants.DUMMY_TOKEN_DECIMALS,
+ constants.DUMMY_TOKEN_TOTAL_SUPPLY,
+ );
+ const data = constants.NULL_BYTES;
+ const value = new BigNumber(10);
+ const submissionTxReceipt = await multiSigWrapper.submitTransactionAsync(
+ contractWithoutFallback.address,
+ data,
+ owners[0],
+ { value },
+ );
+ const newTxId = (submissionTxReceipt.logs[0] as LogWithDecodedArgs<
+ MultiSigWalletWithTimeLockSubmissionEventArgs
+ >).args.transactionId;
+ await multiSigWrapper.confirmTransactionAsync(newTxId, owners[1]);
+ await increaseTimeAndMineBlockAsync(secondsTimeLocked.toNumber());
+ const txReceipt = await multiSigWrapper.executeTransactionAsync(newTxId, owners[1]);
+ const executionFailureLog = txReceipt.logs[0] as LogWithDecodedArgs<
+ MultiSigWalletWithTimeLockExecutionFailureEventArgs
+ >;
+ expect(executionFailureLog.event).to.be.equal('ExecutionFailure');
+ expect(executionFailureLog.args.transactionId).to.be.bignumber.equal(newTxId);
+ });
+ });
describe('changeTimeLock', () => {
describe('initially non-time-locked', async () => {
before(async () => {
@@ -78,8 +252,9 @@ describe('MultiSigWalletWithTimeLock', () => {
const res = await multiSigWrapper.submitTransactionAsync(destination, changeTimeLockData, owners[0]);
const log = res.logs[0] as LogWithDecodedArgs<MultiSigWalletWithTimeLockSubmissionEventArgs>;
const txId = log.args.transactionId;
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }),
+ RevertReason.TxNotFullyConfirmed,
);
});
@@ -94,7 +269,10 @@ describe('MultiSigWalletWithTimeLock', () => {
expect(confirmRes.logs).to.have.length(2);
const blockNum = await web3Wrapper.getBlockNumberAsync();
- const blockInfo = await web3Wrapper.getBlockAsync(blockNum);
+ const blockInfo = await web3Wrapper.getBlockIfExistsAsync(blockNum);
+ if (_.isUndefined(blockInfo)) {
+ throw new Error(`Unexpectedly failed to fetch block at #${blockNum}`);
+ }
const timestamp = new BigNumber(blockInfo.timestamp);
const confirmationTimeBigNum = new BigNumber(await multiSig.confirmationTimes.callAsync(txId));
@@ -147,8 +325,9 @@ describe('MultiSigWalletWithTimeLock', () => {
});
it('should throw if it has enough confirmations but is not past the time lock', async () => {
- return expectTransactionFailedWithoutReasonAsync(
+ return expectTransactionFailedAsync(
multiSig.executeTransaction.sendTransactionAsync(txId, { from: owners[0] }),
+ RevertReason.TimeLockIncomplete,
);
});
diff --git a/packages/contracts/test/tokens/unlimited_allowance_token.ts b/packages/contracts/test/tokens/unlimited_allowance_token.ts
index f2725b408..63680fe9b 100644
--- a/packages/contracts/test/tokens/unlimited_allowance_token.ts
+++ b/packages/contracts/test/tokens/unlimited_allowance_token.ts
@@ -5,7 +5,7 @@ import * as chai from 'chai';
import { DummyERC20TokenContract } from '../../generated_contract_wrappers/dummy_erc20_token';
import { artifacts } from '../utils/artifacts';
-import { expectContractCallFailed } from '../utils/assertions';
+import { expectContractCallFailedAsync } from '../utils/assertions';
import { chaiSetup } from '../utils/chai_setup';
import { constants } from '../utils/constants';
import { provider, txDefaults, web3Wrapper } from '../utils/web3_wrapper';
@@ -54,7 +54,7 @@ describe('UnlimitedAllowanceToken', () => {
it('should throw if owner has insufficient balance', async () => {
const ownerBalance = await token.balanceOf.callAsync(owner);
const amountToTransfer = ownerBalance.plus(1);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
token.transfer.callAsync(spender, amountToTransfer, { from: owner }),
RevertReason.Erc20InsufficientBalance,
);
@@ -93,7 +93,7 @@ describe('UnlimitedAllowanceToken', () => {
await token.approve.sendTransactionAsync(spender, amountToTransfer, { from: owner }),
constants.AWAIT_TRANSACTION_MINED_MS,
);
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
token.transferFrom.callAsync(owner, spender, amountToTransfer, {
from: spender,
}),
@@ -109,7 +109,7 @@ describe('UnlimitedAllowanceToken', () => {
const isSpenderAllowanceInsufficient = spenderAllowance.cmp(amountToTransfer) < 0;
expect(isSpenderAllowanceInsufficient).to.be.true();
- return expectContractCallFailed(
+ return expectContractCallFailedAsync(
token.transferFrom.callAsync(owner, spender, amountToTransfer, {
from: spender,
}),
diff --git a/packages/contracts/test/utils/artifacts.ts b/packages/contracts/test/utils/artifacts.ts
index 5ddb5cc7f..53f2a4e4e 100644
--- a/packages/contracts/test/utils/artifacts.ts
+++ b/packages/contracts/test/utils/artifacts.ts
@@ -4,6 +4,7 @@ import * as AssetProxyOwner from '../../artifacts/AssetProxyOwner.json';
import * as DummyERC20Token from '../../artifacts/DummyERC20Token.json';
import * as DummyERC721Receiver from '../../artifacts/DummyERC721Receiver.json';
import * as DummyERC721Token from '../../artifacts/DummyERC721Token.json';
+import * as DummyMultipleReturnERC20Token from '../../artifacts/DummyMultipleReturnERC20Token.json';
import * as DummyNoReturnERC20Token from '../../artifacts/DummyNoReturnERC20Token.json';
import * as ERC20Proxy from '../../artifacts/ERC20Proxy.json';
import * as ERC721Proxy from '../../artifacts/ERC721Proxy.json';
@@ -37,6 +38,7 @@ export const artifacts = {
DummyERC20Token: (DummyERC20Token as any) as ContractArtifact,
DummyERC721Receiver: (DummyERC721Receiver as any) as ContractArtifact,
DummyERC721Token: (DummyERC721Token as any) as ContractArtifact,
+ DummyMultipleReturnERC20Token: (DummyMultipleReturnERC20Token as any) as ContractArtifact,
DummyNoReturnERC20Token: (DummyNoReturnERC20Token as any) as ContractArtifact,
ERC20Proxy: (ERC20Proxy as any) as ContractArtifact,
ERC721Proxy: (ERC721Proxy as any) as ContractArtifact,
diff --git a/packages/contracts/test/utils/assertions.ts b/packages/contracts/test/utils/assertions.ts
index 61df800c8..3361a751a 100644
--- a/packages/contracts/test/utils/assertions.ts
+++ b/packages/contracts/test/utils/assertions.ts
@@ -159,7 +159,7 @@ export async function expectTransactionFailedWithoutReasonAsync(p: sendTransacti
* @returns a new Promise which will reject if the conditions are not met and
* otherwise resolve with no value.
*/
-export async function expectContractCallFailed<T>(p: Promise<T>, reason: RevertReason): Promise<void> {
+export async function expectContractCallFailedAsync<T>(p: Promise<T>, reason: RevertReason): Promise<void> {
return expect(p).to.be.rejectedWith(reason);
}
@@ -180,7 +180,20 @@ export async function expectContractCallFailedWithoutReasonAsync<T>(p: Promise<T
* @returns a new Promise which will reject if the conditions are not met and
* otherwise resolve with no value.
*/
-export async function expectContractCreationFailedWithoutReason<T>(p: Promise<T>): Promise<void> {
+export async function expectContractCreationFailedAsync<T>(
+ p: sendTransactionResult,
+ reason: RevertReason,
+): Promise<void> {
+ return expectTransactionFailedAsync(p, reason);
+}
+
+/**
+ * Resolves if the contract creation/deployment fails without a revert reason.
+ * @param p a Promise resulting from a contract creation/deployment
+ * @returns a new Promise which will reject if the conditions are not met and
+ * otherwise resolve with no value.
+ */
+export async function expectContractCreationFailedWithoutReasonAsync<T>(p: Promise<T>): Promise<void> {
const errMessage = await _getTransactionFailedErrorMessageAsync();
return expect(p).to.be.rejectedWith(errMessage);
}
diff --git a/packages/contracts/test/utils/block_timestamp.ts b/packages/contracts/test/utils/block_timestamp.ts
index 1159792c4..66c13eed1 100644
--- a/packages/contracts/test/utils/block_timestamp.ts
+++ b/packages/contracts/test/utils/block_timestamp.ts
@@ -35,6 +35,9 @@ export async function increaseTimeAndMineBlockAsync(seconds: number): Promise<nu
* @returns a new Promise which will resolve with the timestamp in seconds.
*/
export async function getLatestBlockTimestampAsync(): Promise<number> {
- const currentBlock = await web3Wrapper.getBlockAsync('latest');
- return currentBlock.timestamp;
+ const currentBlockIfExists = await web3Wrapper.getBlockIfExistsAsync('latest');
+ if (_.isUndefined(currentBlockIfExists)) {
+ throw new Error(`Unable to fetch latest block.`);
+ }
+ return currentBlockIfExists.timestamp;
}
diff --git a/packages/contracts/test/utils/constants.ts b/packages/contracts/test/utils/constants.ts
index ee4378d2e..b9ba8ccb9 100644
--- a/packages/contracts/test/utils/constants.ts
+++ b/packages/contracts/test/utils/constants.ts
@@ -60,6 +60,7 @@ export const constants = {
'MARKET_SELL_ORDERS',
'MATCH_ORDERS',
'CANCEL_ORDER',
+ 'BATCH_CANCEL_ORDERS',
'CANCEL_ORDERS_UP_TO',
'SET_SIGNATURE_VALIDATOR_APPROVAL',
],
diff --git a/packages/contracts/test/utils/multi_sig_wrapper.ts b/packages/contracts/test/utils/multi_sig_wrapper.ts
index e0c27b839..e12a58695 100644
--- a/packages/contracts/test/utils/multi_sig_wrapper.ts
+++ b/packages/contracts/test/utils/multi_sig_wrapper.ts
@@ -6,7 +6,6 @@ import * as _ from 'lodash';
import { AssetProxyOwnerContract } from '../../generated_contract_wrappers/asset_proxy_owner';
import { MultiSigWalletContract } from '../../generated_contract_wrappers/multi_sig_wallet';
-import { constants } from './constants';
import { LogDecoder } from './log_decoder';
export class MultiSigWrapper {
@@ -36,10 +35,19 @@ export class MultiSigWrapper {
const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
return tx;
}
- public async executeTransactionAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> {
+ public async revokeConfirmationAsync(txId: BigNumber, from: string): Promise<TransactionReceiptWithDecodedLogs> {
+ const txHash = await this._multiSig.revokeConfirmation.sendTransactionAsync(txId, { from });
+ const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
+ return tx;
+ }
+ public async executeTransactionAsync(
+ txId: BigNumber,
+ from: string,
+ opts: { gas?: number } = {},
+ ): Promise<TransactionReceiptWithDecodedLogs> {
const txHash = await this._multiSig.executeTransaction.sendTransactionAsync(txId, {
from,
- gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
+ gas: opts.gas,
});
const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
return tx;
@@ -52,7 +60,6 @@ export class MultiSigWrapper {
const txHash = await (this
._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, {
from,
- gas: constants.MAX_EXECUTE_TRANSACTION_GAS,
});
const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash);
return tx;
diff --git a/packages/contracts/test/utils/web3_wrapper.ts b/packages/contracts/test/utils/web3_wrapper.ts
index acb3103b7..d1cd3d387 100644
--- a/packages/contracts/test/utils/web3_wrapper.ts
+++ b/packages/contracts/test/utils/web3_wrapper.ts
@@ -1,5 +1,5 @@
import { devConstants, env, EnvVars, web3Factory } from '@0xproject/dev-utils';
-import { prependSubprovider } from '@0xproject/subproviders';
+import { prependSubprovider, Web3ProviderEngine } from '@0xproject/subproviders';
import { logUtils } from '@0xproject/utils';
import { Web3Wrapper } from '@0xproject/web3-wrapper';
import * as _ from 'lodash';
@@ -47,7 +47,7 @@ const ganacheConfigs = {
};
const providerConfigs = testProvider === ProviderType.Ganache ? ganacheConfigs : gethConfigs;
-export const provider = web3Factory.getRpcProvider(providerConfigs);
+export const provider: Web3ProviderEngine = web3Factory.getRpcProvider(providerConfigs);
const isCoverageEnabled = env.parseBoolean(EnvVars.SolidityCoverage);
const isProfilerEnabled = env.parseBoolean(EnvVars.SolidityProfiler);
const isRevertTraceEnabled = env.parseBoolean(EnvVars.SolidityRevertTrace);
diff --git a/packages/contracts/tsconfig.json b/packages/contracts/tsconfig.json
index 86b33ede7..5fd4c5331 100644
--- a/packages/contracts/tsconfig.json
+++ b/packages/contracts/tsconfig.json
@@ -2,13 +2,11 @@
"extends": "../../tsconfig",
"compilerOptions": {
"outDir": "lib",
- "baseUrl": ".",
- "declaration": false,
- "allowJs": true
+ "rootDir": "."
},
"include": [
"./globals.d.ts",
- "./contract_wrappers",
+ "./generated_contract_wrappers",
"./src/**/*",
"./utils/**/*",
"./test/**/*",