diff options
| author | Fabio Berger <me@fabioberger.com> | 2018-06-25 17:49:14 +0800 |
|---|---|---|
| committer | Fabio Berger <me@fabioberger.com> | 2018-06-25 17:49:14 +0800 |
| commit | 9b196ba68c9f958cd82ef99ae87fdd87c70441a9 (patch) | |
| tree | 26f265a0293703627fffd1993336a57ee694d670 /packages/contracts/src | |
| parent | 5bfdffda1155e306b17eee0a4e60320c3433d5c4 (diff) | |
| parent | f8bde5ab9b8e5d4ec8b9532dfbf18d1202dbfb29 (diff) | |
| download | dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.tar dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.tar.gz dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.tar.bz2 dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.tar.lz dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.tar.xz dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.tar.zst dexon-sol-tools-9b196ba68c9f958cd82ef99ae87fdd87c70441a9.zip | |
Merge branch 'v2-prototype' into feature/combinatorial-testing
* v2-prototype: (97 commits)
Fix typos in comments
Add modifier and tests for removeAuthorizedAddressAtIndex
Update and add tests
Change removeAuthorizedAddress => removeAuthorizedAddressAtIndex
Move isFunctionRemoveAuthorizedAddress to test
Fix usage of `popLastByte`
Fix LibBytes is a library
Remove `areBytesEqual`
Fix usage of `contentAddress()`
Clean low bits in bytes4
Clean high bits in address
Refactor LibBytes.readBytes4 for consistency
Fix LibBytes.equals
Add trailing garbage testcase for LibBytes.equals
Rename bytes.equals
Add slice and sliceDestructive
Rename bytes.rawAddress and add bytes.contentAddress
Rename read/writeBytesWithLength
Using LibBytes for bytes
Make LibBytes a library
...
# Conflicts:
# packages/contracts/src/utils/constants.ts
# packages/contracts/test/exchange/core.ts
Diffstat (limited to 'packages/contracts/src')
38 files changed, 910 insertions, 703 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol index 8cb4254c5..37c12f861 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinAuthorizable.sol @@ -69,7 +69,7 @@ contract MixinAuthorizable is ); delete authorized[target]; - for (uint i = 0; i < authorities.length; i++) { + for (uint256 i = 0; i < authorities.length; i++) { if (authorities[i] == target) { authorities[i] = authorities[authorities.length - 1]; authorities.length -= 1; @@ -87,8 +87,13 @@ contract MixinAuthorizable is uint256 index ) external + onlyOwner { require( + authorized[target], + TARGET_NOT_AUTHORIZED + ); + require( index < authorities.length, INDEX_OUT_OF_BOUNDS ); diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol index 4af39a00b..0e7f3fc89 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC20Transfer.sol @@ -24,9 +24,10 @@ import "../../tokens/ERC20Token/IERC20Token.sol"; import "./libs/LibTransferErrors.sol"; contract MixinERC20Transfer is - LibBytes, LibTransferErrors { + using LibBytes for bytes; + /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -41,7 +42,7 @@ contract MixinERC20Transfer is internal { // Decode asset data. - address token = readAddress(assetData, 0); + address token = assetData.readAddress(0); // Transfer tokens. // We do a raw call so we can check the success separate diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol index 9dc9e6525..944068bbb 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/MixinERC721Transfer.sol @@ -24,9 +24,10 @@ import "../../tokens/ERC721Token/ERC721Token.sol"; import "./libs/LibTransferErrors.sol"; contract MixinERC721Transfer is - LibBytes, LibTransferErrors { + using LibBytes for bytes; + /// @dev Internal version of `transferFrom`. /// @param assetData Encoded byte array. /// @param from Address to transfer asset from. @@ -53,7 +54,12 @@ contract MixinERC721Transfer is bytes memory receiverData ) = decodeERC721AssetData(assetData); - ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); + ERC721Token(token).safeTransferFrom( + from, + to, + tokenId, + receiverData + ); } /// @dev Decodes ERC721 Asset data. @@ -73,10 +79,10 @@ contract MixinERC721Transfer is ) { // Decode asset data. - token = readAddress(assetData, 0); - tokenId = readUint256(assetData, 20); + token = assetData.readAddress(0); + tokenId = assetData.readUint256(20); if (assetData.length > 52) { - receiverData = readBytes(assetData, 52); + receiverData = assetData.readBytesWithLength(52); } return ( diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol index 7f5f056b5..eb58b3374 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxyOwner/AssetProxyOwner.sol @@ -22,24 +22,24 @@ import "../../multisig/MultiSigWalletWithTimeLock.sol"; import "../../utils/LibBytes/LibBytes.sol"; contract AssetProxyOwner is - LibBytes, MultiSigWalletWithTimeLock { + using LibBytes for bytes; event AssetProxyRegistration(address assetProxyContract, bool isRegistered); // Mapping of AssetProxy contract address => - // if this contract is allowed to call the AssetProxy's removeAuthorizedAddress method without a time lock. + // if this contract is allowed to call the AssetProxy's `removeAuthorizedAddressAtIndex` method without a time lock. mapping (address => bool) public isAssetProxyRegistered; - bytes4 constant REMOVE_AUTHORIZED_ADDRESS_SELECTOR = bytes4(keccak256("removeAuthorizedAddress(address)")); + bytes4 constant REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR = bytes4(keccak256("removeAuthorizedAddressAtIndex(address,uint256)")); - /// @dev Function will revert if the transaction does not call `removeAuthorizedAddress` + /// @dev Function will revert if the transaction does not call `removeAuthorizedAddressAtIndex` /// on an approved AssetProxy contract. - modifier validRemoveAuthorizedAddressTx(uint256 transactionId) { + modifier validRemoveAuthorizedAddressAtIndexTx(uint256 transactionId) { Transaction storage tx = transactions[transactionId]; require(isAssetProxyRegistered[tx.destination]); - require(isFunctionRemoveAuthorizedAddress(tx.data)); + require(tx.data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR); _; } @@ -66,7 +66,7 @@ contract AssetProxyOwner is } /// @dev Registers or deregisters an AssetProxy to be able to execute - /// removeAuthorizedAddress without a timelock. + /// `removeAuthorizedAddressAtIndex` without a timelock. /// @param assetProxyContract Address of AssetProxy contract. /// @param isRegistered Status of approval for AssetProxy contract. function registerAssetProxy(address assetProxyContract, bool isRegistered) @@ -78,13 +78,13 @@ contract AssetProxyOwner is AssetProxyRegistration(assetProxyContract, isRegistered); } - /// @dev Allows execution of removeAuthorizedAddress without time lock. + /// @dev Allows execution of `removeAuthorizedAddressAtIndex` without time lock. /// @param transactionId Transaction ID. - function executeRemoveAuthorizedAddress(uint256 transactionId) + function executeRemoveAuthorizedAddressAtIndex(uint256 transactionId) public notExecuted(transactionId) fullyConfirmed(transactionId) - validRemoveAuthorizedAddressTx(transactionId) + validRemoveAuthorizedAddressAtIndexTx(transactionId) { Transaction storage tx = transactions[transactionId]; tx.executed = true; @@ -95,17 +95,4 @@ contract AssetProxyOwner is tx.executed = false; } } - - /// @dev Compares first 4 bytes of byte array to removeAuthorizedAddress function selector. - /// @param data Transaction data. - /// @return Successful if data is a call to removeAuthorizedAddress. - function isFunctionRemoveAuthorizedAddress(bytes memory data) - public - pure - returns (bool) - { - bytes4 first4Bytes = readFirst4(data); - require(REMOVE_AUTHORIZED_ADDRESS_SELECTOR == first4Bytes); - return true; - } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol index 51f99bafa..d36e9633e 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/Exchange.sol @@ -19,9 +19,9 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; import "./MixinExchangeCore.sol"; import "./MixinSignatureValidator.sol"; -import "./MixinSettlement.sol"; import "./MixinWrapperFunctions.sol"; import "./MixinAssetProxyDispatcher.sol"; import "./MixinTransactions.sol"; @@ -30,7 +30,6 @@ import "./MixinMatchOrders.sol"; contract Exchange is MixinExchangeCore, MixinMatchOrders, - MixinSettlement, MixinSignatureValidator, MixinTransactions, MixinAssetProxyDispatcher, @@ -42,9 +41,9 @@ contract Exchange is // Mixins are instantiated in the order they are inherited constructor (bytes memory _zrxAssetData) public + LibConstants(_zrxAssetData) // @TODO: Remove when we deploy. MixinExchangeCore() MixinMatchOrders() - MixinSettlement(_zrxAssetData) MixinSignatureValidator() MixinTransactions() MixinAssetProxyDispatcher() diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index 9e0246303..b8d6c0722 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,14 +19,12 @@ pragma solidity ^0.4.24; import "../../utils/Ownable/Ownable.sol"; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MAssetProxyDispatcher.sol"; import "../AssetProxy/interfaces/IAssetProxy.sol"; contract MixinAssetProxyDispatcher is Ownable, - LibBytes, LibExchangeErrors, MAssetProxyDispatcher { @@ -47,7 +45,7 @@ contract MixinAssetProxyDispatcher is onlyOwner { // Ensure the existing asset proxy is not unintentionally overwritten - address currentAssetProxy = address(assetProxies[assetProxyId]); + address currentAssetProxy = assetProxies[assetProxyId]; require( oldAssetProxy == currentAssetProxy, ASSET_PROXY_MISMATCH @@ -66,7 +64,11 @@ contract MixinAssetProxyDispatcher is // Add asset proxy and log registration. assetProxies[assetProxyId] = assetProxy; - emit AssetProxySet(assetProxyId, newAssetProxy, oldAssetProxy); + emit AssetProxySet( + assetProxyId, + newAssetProxy, + oldAssetProxy + ); } /// @dev Gets an asset proxy. @@ -77,8 +79,7 @@ contract MixinAssetProxyDispatcher is view returns (address) { - address assetProxy = address(assetProxies[assetProxyId]); - return assetProxy; + return assetProxies[assetProxyId]; } /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. @@ -100,8 +101,18 @@ contract MixinAssetProxyDispatcher is if (amount > 0) { // Lookup assetProxy IAssetProxy assetProxy = assetProxies[assetProxyId]; + // Ensure that assetProxy exists + require( + assetProxy != address(0), + ASSET_PROXY_DOES_NOT_EXIST + ); // transferFrom will either succeed or throw. - assetProxy.transferFrom(assetData, from, to, amount); + assetProxy.transferFrom( + assetData, + from, + to, + amount + ); } } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol index c406354a7..b207b3e57 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -19,25 +19,30 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibOrder.sol"; import "./libs/LibMath.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; -import "./mixins/MSettlement.sol"; import "./mixins/MSignatureValidator.sol"; import "./mixins/MTransactions.sol"; +import "./mixins/MAssetProxyDispatcher.sol"; contract MixinExchangeCore is + LibConstants, LibMath, LibOrder, LibFillResults, LibExchangeErrors, + MAssetProxyDispatcher, MExchangeCore, - MSettlement, MSignatureValidator, MTransactions { + using LibBytes for bytes; + // Mapping of orderHash => amount of takerAsset already bought by maker mapping (bytes32 => uint256) public filled; @@ -221,8 +226,9 @@ contract MixinExchangeCore is // Log order emit Fill( order.makerAddress, - takerAddress, order.feeRecipientAddress, + takerAddress, + msg.sender, fillResults.makerAssetFilledAmount, fillResults.takerAssetFilledAmount, fillResults.makerFeePaid, @@ -251,6 +257,7 @@ contract MixinExchangeCore is emit Cancel( order.makerAddress, order.feeRecipientAddress, + msg.sender, orderHash, order.makerAssetData, order.takerAssetData @@ -306,7 +313,11 @@ contract MixinExchangeCore is // Validate Maker signature (check only if first time seen) if (orderInfo.orderTakerAssetFilledAmount == 0) { require( - isValidSignature(orderInfo.orderHash, order.makerAddress, signature), + isValidSignature( + orderInfo.orderHash, + order.makerAddress, + signature + ), INVALID_ORDER_SIGNATURE ); } @@ -389,4 +400,48 @@ contract MixinExchangeCore is return fillResults; } + + /// @dev Settles an order by transferring assets between counterparties. + /// @param order Order struct containing order specifications. + /// @param takerAddress Address selling takerAsset and buying makerAsset. + /// @param fillResults Amounts to be filled and fees paid by maker and taker. + function settleOrder( + LibOrder.Order memory order, + address takerAddress, + LibFillResults.FillResults memory fillResults + ) + private + { + uint8 makerAssetProxyId = uint8(order.makerAssetData.popLastByte()); + uint8 takerAssetProxyId = uint8(order.takerAssetData.popLastByte()); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + dispatchTransferFrom( + order.makerAssetData, + makerAssetProxyId, + order.makerAddress, + takerAddress, + fillResults.makerAssetFilledAmount + ); + dispatchTransferFrom( + order.takerAssetData, + takerAssetProxyId, + takerAddress, + order.makerAddress, + fillResults.takerAssetFilledAmount + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + order.makerAddress, + order.feeRecipientAddress, + fillResults.makerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + order.feeRecipientAddress, + fillResults.takerFeePaid + ); + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index 517b743fe..e36fcc2c5 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,23 +14,27 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; +import "./libs/LibConstants.sol"; +import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; import "./mixins/MMatchOrders.sol"; -import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; +import "./mixins/MAssetProxyDispatcher.sol"; contract MixinMatchOrders is + LibConstants, LibMath, LibExchangeErrors, + MAssetProxyDispatcher, MExchangeCore, MMatchOrders, - MSettlement, MTransactions { + using LibBytes for bytes; /// @dev Match two complementary orders that have a profitable spread. /// Each order is filled at their respective price point. However, the calculations are @@ -224,4 +228,89 @@ contract MixinMatchOrders is // Return fill results return matchedFillResults; } + + /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. + /// @param leftOrder First matched order. + /// @param rightOrder Second matched order. + /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. + /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. + function settleMatchedOrders( + LibOrder.Order memory leftOrder, + LibOrder.Order memory rightOrder, + address takerAddress, + LibFillResults.MatchedFillResults memory matchedFillResults + ) + private + { + uint8 leftMakerAssetProxyId = uint8(leftOrder.makerAssetData.popLastByte()); + uint8 rightMakerAssetProxyId = uint8(rightOrder.makerAssetData.popLastByte()); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + // Order makers and taker + dispatchTransferFrom( + leftOrder.makerAssetData, + leftMakerAssetProxyId, + leftOrder.makerAddress, + rightOrder.makerAddress, + matchedFillResults.right.takerAssetFilledAmount + ); + dispatchTransferFrom( + rightOrder.makerAssetData, + rightMakerAssetProxyId, + rightOrder.makerAddress, + leftOrder.makerAddress, + matchedFillResults.left.takerAssetFilledAmount + ); + dispatchTransferFrom( + leftOrder.makerAssetData, + leftMakerAssetProxyId, + leftOrder.makerAddress, + takerAddress, + matchedFillResults.leftMakerAssetSpreadAmount + ); + + // Maker fees + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + leftOrder.makerAddress, + leftOrder.feeRecipientAddress, + matchedFillResults.left.makerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + rightOrder.makerAddress, + rightOrder.feeRecipientAddress, + matchedFillResults.right.makerFeePaid + ); + + // Taker fees + if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + leftOrder.feeRecipientAddress, + safeAdd( + matchedFillResults.left.takerFeePaid, + matchedFillResults.right.takerFeePaid + ) + ); + } else { + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + leftOrder.feeRecipientAddress, + matchedFillResults.left.takerFeePaid + ); + dispatchTransferFrom( + zrxAssetData, + ZRX_PROXY_ID, + takerAddress, + rightOrder.feeRecipientAddress, + matchedFillResults.right.takerFeePaid + ); + } + } } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol deleted file mode 100644 index 29a9c87bd..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ /dev/null @@ -1,182 +0,0 @@ -/* - - 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; -pragma experimental ABIEncoderV2; - -import "../../utils/LibBytes/LibBytes.sol"; -import "./libs/LibMath.sol"; -import "./libs/LibFillResults.sol"; -import "./libs/LibOrder.sol"; -import "./libs/LibExchangeErrors.sol"; -import "./mixins/MMatchOrders.sol"; -import "./mixins/MSettlement.sol"; -import "./mixins/MAssetProxyDispatcher.sol"; - -contract MixinSettlement is - LibBytes, - LibMath, - LibExchangeErrors, - MMatchOrders, - MSettlement, - MAssetProxyDispatcher -{ - // ZRX address encoded as a byte array. - // This will be constant throughout the life of the Exchange contract, - // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_ASSET_DATA; - uint8 constant ZRX_PROXY_ID = 1; - - /// TODO: _zrxAssetData should be a constant in production. - /// @dev Constructor sets the metadata that will be used for paying ZRX fees. - /// @param _zrxAssetData Byte array containing ERC20 proxy id concatenated with address of ZRX. - constructor (bytes memory _zrxAssetData) - public - { - ZRX_ASSET_DATA = _zrxAssetData; - } - - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal - { - uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); - bytes memory zrxAssetData = ZRX_ASSET_DATA; - dispatchTransferFrom( - order.makerAssetData, - makerAssetProxyId, - order.makerAddress, - takerAddress, - fillResults.makerAssetFilledAmount - ); - dispatchTransferFrom( - order.takerAssetData, - takerAssetProxyId, - takerAddress, - order.makerAddress, - fillResults.takerAssetFilledAmount - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - order.makerAddress, - order.feeRecipientAddress, - fillResults.makerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - order.feeRecipientAddress, - fillResults.takerFeePaid - ); - } - - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - internal - { - uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); - bytes memory zrxAssetData = ZRX_ASSET_DATA; - // Order makers and taker - dispatchTransferFrom( - leftOrder.makerAssetData, - leftMakerAssetProxyId, - leftOrder.makerAddress, - rightOrder.makerAddress, - matchedFillResults.right.takerAssetFilledAmount - ); - dispatchTransferFrom( - rightOrder.makerAssetData, - rightMakerAssetProxyId, - rightOrder.makerAddress, - leftOrder.makerAddress, - matchedFillResults.left.takerAssetFilledAmount - ); - dispatchTransferFrom( - leftOrder.makerAssetData, - leftMakerAssetProxyId, - leftOrder.makerAddress, - takerAddress, - matchedFillResults.leftMakerAssetSpreadAmount - ); - - // Maker fees - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - leftOrder.makerAddress, - leftOrder.feeRecipientAddress, - matchedFillResults.left.makerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - rightOrder.makerAddress, - rightOrder.feeRecipientAddress, - matchedFillResults.right.makerFeePaid - ); - - // Taker fees - if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - leftOrder.feeRecipientAddress, - safeAdd( - matchedFillResults.left.takerFeePaid, - matchedFillResults.right.takerFeePaid - ) - ); - } else { - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - leftOrder.feeRecipientAddress, - matchedFillResults.left.takerFeePaid - ); - dispatchTransferFrom( - zrxAssetData, - ZRX_PROXY_ID, - takerAddress, - rightOrder.feeRecipientAddress, - matchedFillResults.right.takerFeePaid - ); - } - } -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 8ad15aaff..cbb55bfce 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -26,14 +26,15 @@ import "./interfaces/IWallet.sol"; import "./interfaces/IValidator.sol"; contract MixinSignatureValidator is - LibBytes, LibExchangeErrors, MSignatureValidator, MTransactions { + using LibBytes for bytes; + // Personal message headers string constant ETH_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n32"; - string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x41"; + string constant TREZOR_PERSONAL_MESSAGE = "\x19Ethereum Signed Message:\n\x20"; // Mapping of hash => signer => signed mapping (bytes32 => mapping (address => bool)) public preSigned; @@ -43,43 +44,52 @@ contract MixinSignatureValidator is /// @dev Approves a hash on-chain using any valid signature type. /// After presigning a hash, the preSign signature type will become valid for that hash and signer. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof that the hash has been signed by signer. function preSign( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external { require( - isValidSignature(hash, signer, signature), + isValidSignature( + hash, + signerAddress, + signature + ), INVALID_SIGNATURE ); - preSigned[hash][signer] = true; + preSigned[hash][signerAddress] = true; } /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf. - /// @param validator Address of Validator contract. + /// @param validatorAddress Address of Validator contract. /// @param approval Approval or disapproval of Validator contract. function setSignatureValidatorApproval( - address validator, + address validatorAddress, bool approval ) external { - address signer = getCurrentContextAddress(); - allowedValidators[signer][validator] = approval; + address signerAddress = getCurrentContextAddress(); + allowedValidators[signerAddress][validatorAddress] = approval; + emit SignatureValidatorApproval( + signerAddress, + validatorAddress, + approval + ); } /// @dev Verifies that a hash has been signed by the given signer. /// @param hash Any 32 byte hash. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof that the hash has been signed by signer. /// @return True if the address recovered from the provided signature matches the input signer address. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes memory signature ) public @@ -92,8 +102,15 @@ contract MixinSignatureValidator is LENGTH_GREATER_THAN_0_REQUIRED ); + // Ensure signature is supported + uint8 signatureTypeRaw = uint8(signature.popLastByte()); + require( + signatureTypeRaw < uint8(SignatureType.NSignatureTypes), + SIGNATURE_UNSUPPORTED + ); + // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); + SignatureType signatureType = SignatureType(signatureTypeRaw); // Variables are not scoped in Solidity. uint8 v; @@ -128,10 +145,10 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover(hash, v, r, s); - isValid = signer == recovered; + isValid = signerAddress == recovered; return isValid; // Signed using web3.eth_sign @@ -141,15 +158,15 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover( keccak256(abi.encodePacked(ETH_PERSONAL_MESSAGE, hash)), v, r, s ); - isValid = signer == recovered; + isValid = signerAddress == recovered; return isValid; // Implicitly signed by caller. @@ -165,13 +182,13 @@ contract MixinSignatureValidator is signature.length == 0, LENGTH_0_REQUIRED ); - isValid = signer == msg.sender; + isValid = signerAddress == msg.sender; return isValid; // Signature verified by wallet contract. // If used with an order, the maker of the order is the wallet contract. } else if (signatureType == SignatureType.Wallet) { - isValid = IWallet(signer).isValidSignature(hash, signature); + isValid = IWallet(signerAddress).isValidSignature(hash, signature); return isValid; // Signature verified by validator contract. @@ -183,21 +200,23 @@ contract MixinSignatureValidator is // | 0x14 + x | 1 | Signature type is always "\x06" | } else if (signatureType == SignatureType.Validator) { // Pop last 20 bytes off of signature byte array. - address validator = popLast20Bytes(signature); + + address validatorAddress = signature.popLast20Bytes(); + // Ensure signer has approved validator. - if (!allowedValidators[signer][validator]) { + if (!allowedValidators[signerAddress][validatorAddress]) { return false; } - isValid = IValidator(validator).isValidSignature( + isValid = IValidator(validatorAddress).isValidSignature( hash, - signer, + signerAddress, signature ); return isValid; // Signer signed hash previously using the preSign function. } else if (signatureType == SignatureType.PreSigned) { - isValid = preSigned[hash][signer]; + isValid = preSigned[hash][signerAddress]; return isValid; // Signature from Trezor hardware wallet. @@ -214,20 +233,15 @@ contract MixinSignatureValidator is LENGTH_65_REQUIRED ); v = uint8(signature[0]); - r = readBytes32(signature, 1); - s = readBytes32(signature, 33); + r = signature.readBytes32(1); + s = signature.readBytes32(33); recovered = ecrecover( keccak256(abi.encodePacked(TREZOR_PERSONAL_MESSAGE, hash)), v, r, s ); - isValid = signer == recovered; - return isValid; - - // Signer signed hash previously using the preSign function - } else if (signatureType == SignatureType.PreSigned) { - isValid = preSigned[hash][signer]; + isValid = signerAddress == recovered; return isValid; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol index f1332363c..20a4a12df 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinTransactions.sol @@ -41,37 +41,47 @@ contract MixinTransactions is bytes32 constant EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH = keccak256(abi.encodePacked( "ZeroExTransaction(", "uint256 salt,", - "address signer,", + "address signerAddress,", "bytes data", ")" )); /// @dev Calculates EIP712 hash of the Transaction. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. - /// @param signer Address of transaction signer. + /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @return EIP712 hash of the Transaction. - function hashZeroExTransaction(uint256 salt, address signer, bytes data) + function hashZeroExTransaction( + uint256 salt, + address signerAddress, + bytes memory data + ) internal pure - returns (bytes32) + returns (bytes32 result) { - return keccak256(abi.encode( - EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH, - salt, - signer, - keccak256(data) - )); + bytes32 schemaHash = EIP712_ZEROEX_TRANSACTION_SCHEMA_HASH; + bytes32 dataHash = keccak256(data); + assembly { + let memPtr := mload(64) + mstore(memPtr, schemaHash) + mstore(add(memPtr, 32), salt) + mstore(add(memPtr, 64), and(signerAddress, 0xffffffffffffffffffffffffffffffffffffffff)) + mstore(add(memPtr, 96), dataHash) + result := keccak256(memPtr, 128) + } + + return result; } /// @dev Executes an exchange method call in the context of signer. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. - /// @param signer Address of transaction signer. + /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @param signature Proof of signer transaction by signer. function executeTransaction( uint256 salt, - address signer, + address signerAddress, bytes data, bytes signature ) @@ -83,7 +93,11 @@ contract MixinTransactions is REENTRANCY_ILLEGAL ); - bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction(salt, signer, data)); + bytes32 transactionHash = hashEIP712Message(hashZeroExTransaction( + salt, + signerAddress, + data + )); // Validate transaction has not been executed require( @@ -92,15 +106,19 @@ contract MixinTransactions is ); // Transaction always valid if signer is sender of transaction - if (signer != msg.sender) { + if (signerAddress != msg.sender) { // Validate signature require( - isValidSignature(transactionHash, signer, signature), + isValidSignature( + transactionHash, + signerAddress, + signature + ), INVALID_TX_SIGNATURE ); // Set the current transaction signer - currentContextAddress = signer; + currentContextAddress = signerAddress; } // Execute transaction diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol index 02aa9776e..511463309 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ISignatureValidator.sol @@ -22,32 +22,32 @@ contract ISignatureValidator { /// @dev Approves a hash on-chain using any valid signature type. /// After presigning a hash, the preSign signature type will become valid for that hash and signer. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof that the hash has been signed by signer. function preSign( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external; /// @dev Approves/unnapproves a Validator contract to verify signatures on signer's behalf. - /// @param validator Address of Validator contract. + /// @param validatorAddress Address of Validator contract. /// @param approval Approval or disapproval of Validator contract. function setSignatureValidatorApproval( - address validator, + address validatorAddress, bool approval ) external; /// @dev Verifies that a signature is valid. /// @param hash Message hash that is signed. - /// @param signer Address of signer. + /// @param signerAddress Address of signer. /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes memory signature ) public diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol index 2f9a5bc7c..a7cab8f55 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/ITransactions.sol @@ -21,12 +21,12 @@ contract ITransactions { /// @dev Executes an exchange method call in the context of signer. /// @param salt Arbitrary number to ensure uniqueness of transaction hash. - /// @param signer Address of transaction signer. + /// @param signerAddress Address of transaction signer. /// @param data AbiV2 encoded calldata. /// @param signature Proof of signer transaction by signer. function executeTransaction( uint256 salt, - address signer, + address signerAddress, bytes data, bytes signature ) diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol index 3e5ccc190..0b1796a66 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/interfaces/IValidator.sol @@ -22,12 +22,12 @@ contract IValidator { /// @dev Verifies that a signature is valid. /// @param hash Message hash that is signed. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol new file mode 100644 index 000000000..4a9452448 --- /dev/null +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibConstants.sol @@ -0,0 +1,37 @@ +/* + + 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; + +contract LibConstants { + + // Asset data for ZRX token. Used for fee transfers. + // @TODO: Hardcode constant when we deploy. Currently + // not constant to make testing easier. + bytes public ZRX_ASSET_DATA; + + // Proxy Id for ZRX token. + uint8 constant ZRX_PROXY_ID = 1; + + // @TODO: Remove when we deploy. + constructor (bytes memory zrxAssetData) + public + { + ZRX_ASSET_DATA = zrxAssetData; + } +} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol index adf27bec3..a43f0f927 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -20,42 +20,45 @@ pragma solidity ^0.4.24; contract LibExchangeErrors { /// Order validation errors /// - string constant ORDER_UNFILLABLE = "ORDER_UNFILLABLE"; // Order cannot be filled. - string constant INVALID_MAKER = "INVALID_MAKER"; // Invalid makerAddress. - string constant INVALID_TAKER = "INVALID_TAKER"; // Invalid takerAddress. - string constant INVALID_SENDER = "INVALID_SENDER"; // Invalid `msg.sender`. - string constant INVALID_ORDER_SIGNATURE = "INVALID_ORDER_SIGNATURE"; // Signature validation failed. + string constant ORDER_UNFILLABLE = "ORDER_UNFILLABLE"; // Order cannot be filled. + string constant INVALID_MAKER = "INVALID_MAKER"; // Invalid makerAddress. + string constant INVALID_TAKER = "INVALID_TAKER"; // Invalid takerAddress. + string constant INVALID_SENDER = "INVALID_SENDER"; // Invalid `msg.sender`. + string constant INVALID_ORDER_SIGNATURE = "INVALID_ORDER_SIGNATURE"; // Signature validation failed. /// fillOrder validation errors /// - string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0. - string constant ROUNDING_ERROR = "ROUNDING_ERROR"; // Rounding error greater than 0.1% of takerAssetFillAmount. + string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0. + string constant ROUNDING_ERROR = "ROUNDING_ERROR"; // Rounding error greater than 0.1% of takerAssetFillAmount. /// Signature validation errors /// - string constant INVALID_SIGNATURE = "INVALID_SIGNATURE"; // Signature validation failed. - string constant SIGNATURE_ILLEGAL = "SIGNATURE_ILLEGAL"; // Signature type is illegal. - string constant SIGNATURE_UNSUPPORTED = "SIGNATURE_UNSUPPORTED"; // Signature type unsupported. + string constant INVALID_SIGNATURE = "INVALID_SIGNATURE"; // Signature validation failed. + string constant SIGNATURE_ILLEGAL = "SIGNATURE_ILLEGAL"; // Signature type is illegal. + string constant SIGNATURE_UNSUPPORTED = "SIGNATURE_UNSUPPORTED"; // Signature type unsupported. /// cancelOrdersUptTo errors /// - string constant INVALID_NEW_ORDER_EPOCH = "INVALID_NEW_ORDER_EPOCH"; // Specified salt must be greater than or equal to existing orderEpoch. + string constant INVALID_NEW_ORDER_EPOCH = "INVALID_NEW_ORDER_EPOCH"; // Specified salt must be greater than or equal to existing orderEpoch. /// fillOrKillOrder errors /// - string constant COMPLETE_FILL_FAILED = "COMPLETE_FILL_FAILED"; // Desired takerAssetFillAmount could not be completely filled. + string constant COMPLETE_FILL_FAILED = "COMPLETE_FILL_FAILED"; // Desired takerAssetFillAmount could not be completely filled. /// matchOrders errors /// - string constant NEGATIVE_SPREAD_REQUIRED = "NEGATIVE_SPREAD_REQUIRED"; // Matched orders must have a negative spread. + string constant NEGATIVE_SPREAD_REQUIRED = "NEGATIVE_SPREAD_REQUIRED"; // Matched orders must have a negative spread. /// Transaction errors /// - string constant REENTRANCY_ILLEGAL = "REENTRANCY_ILLEGAL"; // Recursive reentrancy is not allowed. - string constant INVALID_TX_HASH = "INVALID_TX_HASH"; // Transaction has already been executed. - string constant INVALID_TX_SIGNATURE = "INVALID_TX_SIGNATURE"; // Signature validation failed. - string constant FAILED_EXECUTION = "FAILED_EXECUTION"; // Transaction execution failed. + string constant REENTRANCY_ILLEGAL = "REENTRANCY_ILLEGAL"; // Recursive reentrancy is not allowed. + string constant INVALID_TX_HASH = "INVALID_TX_HASH"; // Transaction has already been executed. + string constant INVALID_TX_SIGNATURE = "INVALID_TX_SIGNATURE"; // Signature validation failed. + string constant FAILED_EXECUTION = "FAILED_EXECUTION"; // Transaction execution failed. /// registerAssetProxy errors /// - string constant ASSET_PROXY_MISMATCH = "ASSET_PROXY_MISMATCH"; // oldAssetProxy proxy does not match currentAssetProxy. - string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // newAssetProxyId does not match given assetProxyId. + string constant ASSET_PROXY_MISMATCH = "ASSET_PROXY_MISMATCH"; // oldAssetProxy proxy does not match currentAssetProxy. + string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // newAssetProxyId does not match given assetProxyId. + + /// dispatchTransferFrom errors /// + string constant ASSET_PROXY_DOES_NOT_EXIST = "ASSET_PROXY_DOES_NOT_EXIST"; // No assetProxy registered at given id. /// Length validation errors /// string constant LENGTH_GREATER_THAN_0_REQUIRED = "LENGTH_GREATER_THAN_0_REQUIRED"; // Byte array must have a length greater than 0. - string constant LENGTH_0_REQUIRED = "LENGTH_1_REQUIRED"; // Byte array must have a length of 1. - string constant LENGTH_65_REQUIRED = "LENGTH_66_REQUIRED"; // Byte array must have a length of 66. + string constant LENGTH_0_REQUIRED = "LENGTH_0_REQUIRED"; // Byte array must have a length of 0. + string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; // Byte array must have a length of 65. } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol index b7550d6d2..63f1b8c87 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibFillResults.sol @@ -25,16 +25,16 @@ contract LibFillResults is { struct FillResults { - uint256 makerAssetFilledAmount; - uint256 takerAssetFilledAmount; - uint256 makerFeePaid; - uint256 takerFeePaid; + uint256 makerAssetFilledAmount; // Total amount of makerAsset(s) filled. + uint256 takerAssetFilledAmount; // Total amount of takerAsset(s) filled. + uint256 makerFeePaid; // Total amount of ZRX paid by maker(s) to feeRecipient(s). + uint256 takerFeePaid; // Total amount of ZRX paid by taker to feeRecipients(s). } struct MatchedFillResults { - FillResults left; - FillResults right; - uint256 leftMakerAssetSpreadAmount; + FillResults left; // Amounts filled and fees paid of left order. + FillResults right; // Amounts filled and fees paid of right order. + uint256 leftMakerAssetSpreadAmount; // Spread between price of left and right order, denominated in the left order's makerAsset, paid to taker. } /// @dev Adds properties of both FillResults instances. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol index bfc7aaae0..f3f1e9277 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibOrder.sol @@ -55,27 +55,24 @@ contract LibOrder is } struct Order { - address makerAddress; - address takerAddress; - address feeRecipientAddress; - address senderAddress; - uint256 makerAssetAmount; - uint256 takerAssetAmount; - uint256 makerFee; - uint256 takerFee; - uint256 expirationTimeSeconds; - uint256 salt; - bytes makerAssetData; - bytes takerAssetData; + address makerAddress; // Address that created the order. + address takerAddress; // Address that is allowed to fill the order. If set to 0, any address is allowed to fill the order. + address feeRecipientAddress; // Address that will recieve fees when order is filled. + address senderAddress; // Address that is allowed to call Exchange contract methods that affect this order. If set to 0, any address is allowed to call these methods. + uint256 makerAssetAmount; // Amount of makerAsset being offered by maker. Must be greater than 0. + uint256 takerAssetAmount; // Amount of takerAsset being bid on by maker. Must be greater than 0. + uint256 makerFee; // Amount of ZRX paid to feeRecipient by maker when order is filled. If set to 0, no transfer of ZRX from maker to feeRecipient will be attempted. + uint256 takerFee; // Amount of ZRX paid to feeRecipient by taker when order is filled. If set to 0, no transfer of ZRX from taker to feeRecipient will be attempted. + uint256 expirationTimeSeconds; // Timestamp in seconds at which order expires. + uint256 salt; // Arbitrary number to facilitate uniqueness of the order's hash. + bytes makerAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring makerAsset. The last byte references the id of this proxy. + bytes takerAssetData; // Encoded data that can be decoded by a specified proxy contract when transferring takerAsset. The last byte references the id of this proxy. } struct OrderInfo { - // See LibStatus for a complete description of order statuses - uint8 orderStatus; - // Keccak-256 EIP712 hash of the order - bytes32 orderHash; - // Amount of order that has been filled - uint256 orderTakerAssetFilledAmount; + uint8 orderStatus; // Status that describes order's validity and fillability. + bytes32 orderHash; // EIP712 hash of the order (see LibOrder.getOrderHash). + uint256 orderTakerAssetFilledAmount; // Amount of order that has already been filled. } /// @dev Calculates Keccak-256 hash of the order. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol index d16a830f4..788f42c60 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -27,9 +27,9 @@ contract MAssetProxyDispatcher is // Logs registration of new asset proxy event AssetProxySet( - uint8 id, - address newAssetProxy, - address oldAssetProxy + uint8 id, // Id of new registered AssetProxy. + address newAssetProxy, // Address of new registered AssetProxy. + address oldAssetProxy // Address of AssetProxy that was overwritten at given id (or null address). ); /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol index fb345294c..6e406e1c4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol @@ -28,32 +28,34 @@ contract MExchangeCore is { // Fill event is emitted whenever an order is filled. event Fill( - address indexed makerAddress, - address takerAddress, - address indexed feeRecipientAddress, - uint256 makerAssetFilledAmount, - uint256 takerAssetFilledAmount, - uint256 makerFeePaid, - uint256 takerFeePaid, - bytes32 indexed orderHash, - bytes makerAssetData, - bytes takerAssetData + address indexed makerAddress, // Address that created the order. + address indexed feeRecipientAddress, // Address that received fees. + address takerAddress, // Address that filled the order. + address senderAddress, // Address that called the Exchange contract (msg.sender). + uint256 makerAssetFilledAmount, // Amount of makerAsset sold by maker and bought by taker. + uint256 takerAssetFilledAmount, // Amount of takerAsset sold by taker and bought by maker. + uint256 makerFeePaid, // Amount of ZRX paid to feeRecipient by maker. + uint256 takerFeePaid, // Amount of ZRX paid to feeRecipient by taker. + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash). + bytes makerAssetData, // Encoded data specific to makerAsset. + bytes takerAssetData // Encoded data specific to takerAsset. ); // Cancel event is emitted whenever an individual order is cancelled. event Cancel( - address indexed makerAddress, - address indexed feeRecipientAddress, - bytes32 indexed orderHash, - bytes makerAssetData, - bytes takerAssetData + address indexed makerAddress, // Address that created the order. + address indexed feeRecipientAddress, // Address that would have recieved fees if order was filled. + address senderAddress, // Address that called the Exchange contract (msg.sender). + bytes32 indexed orderHash, // EIP712 hash of order (see LibOrder.getOrderHash). + bytes makerAssetData, // Encoded data specific to makerAsset. + bytes takerAssetData // Encoded data specific to takerAsset. ); // CancelUpTo event is emitted whenever `cancelOrdersUpTo` is executed succesfully. event CancelUpTo( - address indexed makerAddress, - address indexed senderAddress, - uint256 orderEpoch + address indexed makerAddress, // Orders cancelled must have been created by this address. + address indexed senderAddress, // Orders cancelled must have a `senderAddress` equal to this address. + uint256 orderEpoch // Orders specified makerAddress and senderAddress with a salt <= this value are considered cancelled. ); /// @dev Updates state with results of a fill order. @@ -121,4 +123,5 @@ contract MExchangeCore is internal pure returns (LibFillResults.FillResults memory fillResults); + } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol index e52963a26..abe7c3596 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MMatchOrders.sol @@ -54,4 +54,5 @@ contract MMatchOrders is internal pure returns (LibFillResults.MatchedFillResults memory matchedFillResults); + } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol deleted file mode 100644 index 2c403162f..000000000 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSettlement.sol +++ /dev/null @@ -1,49 +0,0 @@ -/* - - 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 "../libs/LibOrder.sol"; -import "../libs/LibFillResults.sol"; - -contract MSettlement { - - /// @dev Settles an order by transferring assets between counterparties. - /// @param order Order struct containing order specifications. - /// @param takerAddress Address selling takerAsset and buying makerAsset. - /// @param fillResults Amounts to be filled and fees paid by maker and taker. - function settleOrder( - LibOrder.Order memory order, - address takerAddress, - LibFillResults.FillResults memory fillResults - ) - internal; - - /// @dev Settles matched order by transferring appropriate funds between order makers, taker, and fee recipient. - /// @param leftOrder First matched order. - /// @param rightOrder Second matched order. - /// @param takerAddress Address that matched the orders. The taker receives the spread between orders as profit. - /// @param matchedFillResults Struct holding amounts to transfer between makers, taker, and fee recipients. - function settleMatchedOrders( - LibOrder.Order memory leftOrder, - LibOrder.Order memory rightOrder, - address takerAddress, - LibFillResults.MatchedFillResults memory matchedFillResults - ) - internal; -} diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol index 5e286e43a..6cc1d7a10 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MSignatureValidator.sol @@ -23,16 +23,23 @@ import "../interfaces/ISignatureValidator.sol"; contract MSignatureValidator is ISignatureValidator { + event SignatureValidatorApproval( + address indexed signerAddress, // Address that approves or disapproves a contract to verify signatures. + address indexed validatorAddress, // Address of signature validator contract. + bool approved // Approval or disapproval of validator contract. + ); + // Allowed signature types. enum SignatureType { - Illegal, // 0x00, default value - Invalid, // 0x01 - EIP712, // 0x02 - EthSign, // 0x03 - Caller, // 0x04 - Wallet, // 0x05 - Validator, // 0x06 - PreSigned, // 0x07 - Trezor // 0x08 + Illegal, // 0x00, default value + Invalid, // 0x01 + EIP712, // 0x02 + EthSign, // 0x03 + Caller, // 0x04 + Wallet, // 0x05 + Validator, // 0x06 + PreSigned, // 0x07 + Trezor, // 0x08 + NSignatureTypes // 0x09, number of signature types. Always leave at end. } } diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol new file mode 100644 index 000000000..2abcd17a0 --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyOwner/TestAssetProxyOwner.sol @@ -0,0 +1,56 @@ +/* + + 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 "../../protocol/AssetProxyOwner/AssetProxyOwner.sol"; + +contract TestAssetProxyOwner is + AssetProxyOwner +{ + constructor( + address[] memory _owners, + address[] memory _assetProxyContracts, + uint256 _required, + uint256 _secondsTimeLocked + ) + public + AssetProxyOwner(_owners, _assetProxyContracts, _required, _secondsTimeLocked) + { + } + + function testValidRemoveAuthorizedAddressAtIndexTx(uint256 id) + public + validRemoveAuthorizedAddressAtIndexTx(id) + returns (bool) + { + // Do nothing. We expect reverts through the modifier + return true; + } + + /// @dev Compares first 4 bytes of byte array to `removeAuthorizedAddressAtIndex` function selector. + /// @param data Transaction data. + /// @return Successful if data is a call to `removeAuthorizedAddressAtIndex`. + function isFunctionRemoveAuthorizedAddressAtIndex(bytes memory data) + public + pure + returns (bool) + { + return data.readBytes4(0) == REMOVE_AUTHORIZED_ADDRESS_AT_INDEX_SELECTOR; + } +} diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 6f1898acd..f45faaf36 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -21,9 +21,9 @@ pragma experimental ABIEncoderV2; import "../../utils/LibBytes/LibBytes.sol"; -contract TestLibBytes is - LibBytes -{ +contract TestLibBytes { + + using LibBytes for bytes; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -33,7 +33,7 @@ contract TestLibBytes is pure returns (bytes memory, bytes1 result) { - result = popLastByte(b); + result = b.popLastByte(); return (b, result); } @@ -45,7 +45,7 @@ contract TestLibBytes is pure returns (bytes memory, address result) { - result = popLast20Bytes(b); + result = b.popLast20Bytes(); return (b, result); } @@ -53,12 +53,23 @@ contract TestLibBytes is /// @param lhs First byte array to compare. /// @param rhs Second byte array to compare. /// @return True if arrays are the same. False otherwise. - function publicAreBytesEqual(bytes memory lhs, bytes memory rhs) + function publicEquals(bytes memory lhs, bytes memory rhs) public pure returns (bool equal) { - equal = areBytesEqual(lhs, rhs); + equal = lhs.equals(rhs); + return equal; + } + + function publicEqualsPop1(bytes memory lhs, bytes memory rhs) + public + pure + returns (bool equal) + { + lhs.popLastByte(); + rhs.popLastByte(); + equal = lhs.equals(rhs); return equal; } @@ -73,7 +84,7 @@ contract TestLibBytes is pure returns (bytes memory) { - deepCopyBytes(dest, source); + LibBytes.deepCopyBytes(dest, source); return dest; } @@ -89,7 +100,7 @@ contract TestLibBytes is pure returns (address result) { - result = readAddress(b, index); + result = b.readAddress(index); return result; } @@ -106,7 +117,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeAddress(b, index, input); + b.writeAddress(index, input); return b; } @@ -122,7 +133,7 @@ contract TestLibBytes is pure returns (bytes32 result) { - result = readBytes32(b, index); + result = b.readBytes32(index); return result; } @@ -139,7 +150,7 @@ contract TestLibBytes is pure returns (bytes memory) { - writeBytes32(b, index, input); + b.writeBytes32(index, input); return b; } @@ -155,7 +166,7 @@ contract TestLibBytes is pure returns (uint256 result) { - result = readUint256(b, index); + result = b.readUint256(index); return result; } @@ -172,19 +183,23 @@ contract TestLibBytes is pure returns (bytes memory) { - writeUint256(b, index, input); + b.writeUint256(index, input); return b; } - /// @dev Reads the first 4 bytes from a byte array of arbitrary length. - /// @param b Byte array to read first 4 bytes from. - /// @return First 4 bytes of data. - function publicReadFirst4(bytes memory b) + /// @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 pure returns (bytes4 result) { - result = readFirst4(b); + result = b.readBytes4(index); return result; } @@ -192,7 +207,7 @@ contract TestLibBytes is /// @param b Byte array containing nested bytes. /// @param index Index of nested bytes. /// @return result Nested bytes. - function publicReadBytes( + function publicReadBytesWithLength( bytes memory b, uint256 index ) @@ -200,7 +215,7 @@ contract TestLibBytes is pure returns (bytes memory result) { - result = readBytes(b, index); + result = b.readBytesWithLength(index); return result; } @@ -209,7 +224,7 @@ contract TestLibBytes is /// @param index Index in byte array of <input>. /// @param input bytes to insert. /// @return b Updated input byte array - function publicWriteBytes( + function publicWriteBytesWithLength( bytes memory b, uint256 index, bytes memory input @@ -218,7 +233,37 @@ contract TestLibBytes is pure returns (bytes memory) { - writeBytes(b, index, input); + b.writeBytesWithLength(index, input); return b; } + + /// @dev Copies a block of memory from one location to another. + /// @param mem Memory contents we want to apply memCopy to + /// @param dest Destination offset into <mem>. + /// @param source Source offset into <mem>. + /// @param length Length of bytes to copy from <source> to <dest> + /// @return mem Memory contents after calling memCopy. + function testMemcpy( + bytes mem, + uint256 dest, + uint256 source, + uint256 length + ) + public // not external, we need input in memory + pure + returns (bytes) + { + // Sanity check. Overflows are not checked. + require(source + length <= mem.length); + require(dest + length <= mem.length); + + // Get pointer to memory contents + uint256 offset = mem.contentAddress(); + + // Execute memCopy adjusted for memory array location + LibBytes.memCopy(offset + dest, offset + source, length); + + // Return modified memory contents + return mem; + } } diff --git a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol b/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol deleted file mode 100644 index b7e2e06b8..000000000 --- a/packages/contracts/src/contracts/current/test/TestLibMem/TestLibMem.sol +++ /dev/null @@ -1,56 +0,0 @@ -/* - - 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 "../../utils/LibMem/LibMem.sol"; - -contract TestLibMem is - LibMem -{ - - /// @dev Copies a block of memory from one location to another. - /// @param mem Memory contents we want to apply memCopy to - /// @param dest Destination offset into <mem>. - /// @param source Source offset into <mem>. - /// @param length Length of bytes to copy from <source> to <dest> - /// @return mem Memory contents after calling memCopy. - function testMemcpy( - bytes mem, - uint256 dest, - uint256 source, - uint256 length - ) - public // not external, we need input in memory - pure - returns (bytes) - { - // Sanity check. Overflows are not checked. - require(source + length <= mem.length); - require(dest + length <= mem.length); - - // Get pointer to memory contents - uint256 offset = getMemAddress(mem) + 32; - - // Execute memCopy adjusted for memory array location - memCopy(offset + dest, offset + source, length); - - // Return modified memory contents - return mem; - } -} diff --git a/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol new file mode 100644 index 000000000..f9271bf7a --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestValidator/TestValidator.sol @@ -0,0 +1,52 @@ +/* + + 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 "../../protocol/Exchange/interfaces/IValidator.sol"; + +contract TestValidator is + IValidator +{ + + // The single valid signer for this wallet. + address VALID_SIGNER; + + /// @dev constructs a new `TestValidator` with a single valid signer. + /// @param validSigner The sole, valid signer. + constructor (address validSigner) public { + VALID_SIGNER = validSigner; + } + + /// @dev Verifies that a signature is valid. `signer` must match `VALID_SIGNER`. + /// @param hash Message hash that is signed. + /// @param signerAddress Address that should have signed the given hash. + /// @param signature Proof of signing. + /// @return Validity of signature. + function isValidSignature( + bytes32 hash, + address signerAddress, + bytes signature + ) + external + view + returns (bool isValid) + { + return (signerAddress == VALID_SIGNER); + } +} diff --git a/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol new file mode 100644 index 000000000..17dee9e9c --- /dev/null +++ b/packages/contracts/src/contracts/current/test/TestWallet/TestWallet.sol @@ -0,0 +1,65 @@ +/* + + 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 "../../protocol/Exchange/interfaces/IWallet.sol"; +import "../../utils/LibBytes/LibBytes.sol"; + +contract TestWallet is + IWallet +{ + using LibBytes for bytes; + + string constant LENGTH_65_REQUIRED = "LENGTH_65_REQUIRED"; + + // The owner of this wallet. + address WALLET_OWNER; + + /// @dev constructs a new `TestWallet` with a single owner. + /// @param walletOwner The owner of this wallet. + constructor (address walletOwner) public { + WALLET_OWNER = walletOwner; + } + + /// @dev Validates an EIP712 signature. + /// The signer must match the owner of this wallet. + /// @param hash Message hash that is signed. + /// @param eip712Signature Proof of signing. + /// @return Validity of signature. + function isValidSignature( + bytes32 hash, + bytes eip712Signature + ) + external + view + returns (bool isValid) + { + require( + eip712Signature.length == 65, + LENGTH_65_REQUIRED + ); + + uint8 v = uint8(eip712Signature[0]); + bytes32 r = eip712Signature.readBytes32(1); + bytes32 s = eip712Signature.readBytes32(33); + address recoveredAddress = ecrecover(hash, v, r, s); + isValid = WALLET_OWNER == recoveredAddress; + return isValid; + } +} diff --git a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol index 460c9ea42..d35815474 100644 --- a/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol +++ b/packages/contracts/src/contracts/current/test/Whitelist/Whitelist.sol @@ -116,18 +116,18 @@ contract Whitelist is /// @dev Verifies signer is same as signer of current Ethereum transaction. /// NOTE: This function can currently be used to validate signatures coming from outside of this contract. /// Extra safety checks can be added for a production contract. - /// @param signer Address that should have signed the given hash. + /// @param signerAddress Address that should have signed the given hash. /// @param signature Proof of signing. /// @return Validity of order signature. function isValidSignature( bytes32 hash, - address signer, + address signerAddress, bytes signature ) external view returns (bool isValid) { - return signer == tx.origin; + return signerAddress == tx.origin; } } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 10d7ce41a..e4cbf318b 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -18,11 +18,8 @@ pragma solidity ^0.4.24; -import "../LibMem/LibMem.sol"; - -contract LibBytes is - LibMem -{ +library LibBytes { + using LibBytes for bytes; // Revert reasons string constant GREATER_THAN_ZERO_LENGTH_REQUIRED = "GREATER_THAN_ZERO_LENGTH_REQUIRED"; @@ -31,6 +28,187 @@ contract LibBytes is string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"; + string constant FROM_LESS_THAN_TO_REQUIRED = "FROM_LESS_THAN_TO_REQUIRED"; + string constant TO_LESS_THAN_LENGTH_REQUIRED = "TO_LESS_THAN_LENGTH_REQUIRED"; + + /// @dev Gets the memory address for a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of byte array. This + /// points to the header of the byte array which contains + /// the length. + function rawAddress(bytes memory input) + internal + pure + returns (uint256 memoryAddress) + { + assembly { + memoryAddress := input + } + return memoryAddress; + } + + /// @dev Gets the memory address for the contents of a byte array. + /// @param input Byte array to lookup. + /// @return memoryAddress Memory address of the contents of the byte array. + function contentAddress(bytes memory input) + internal + pure + returns (uint256 memoryAddress) + { + assembly { + memoryAddress := add(input, 32) + } + return memoryAddress; + } + + /// @dev Copies `length` bytes from memory location `source` to `dest`. + /// @param dest memory address to copy bytes to. + /// @param source memory address to copy bytes from. + /// @param length number of bytes to copy. + function memCopy( + uint256 dest, + uint256 source, + uint256 length + ) + internal + pure + { + if (length < 32) { + // Handle a partial word by reading destination and masking + // off the bits we are interested in. + // This correctly handles overlap, zero lengths and source == dest + assembly { + let mask := sub(exp(256, sub(32, length)), 1) + let s := and(mload(source), not(mask)) + let d := and(mload(dest), mask) + mstore(dest, or(s, d)) + } + } else { + // Skip the O(length) loop when source == dest. + if (source == dest) { + return; + } + + // For large copies we copy whole words at a time. The final + // word is aligned to the end of the range (instead of after the + // previous) to handle partial words. So a copy will look like this: + // + // #### + // #### + // #### + // #### + // + // We handle overlap in the source and destination range by + // changing the copying direction. This prevents us from + // overwriting parts of source that we still need to copy. + // + // This correctly handles source == dest + // + if (source > dest) { + assembly { + // We subtract 32 from `sEnd` and `dEnd` because it + // is easier to compare with in the loop, and these + // are also the addresses we need for copying the + // last bytes. + length := sub(length, 32) + let sEnd := add(source, length) + let dEnd := add(dest, length) + + // Remember the last 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the last bytes in + // source already due to overlap. + let last := mload(sEnd) + + // Copy whole words front to back + // Note: the first check is always true, + // this could have been a do-while loop. + for {} lt(source, sEnd) {} { + mstore(dest, mload(source)) + source := add(source, 32) + dest := add(dest, 32) + } + + // Write the last 32 bytes + mstore(dEnd, last) + } + } else { + assembly { + // We subtract 32 from `sEnd` and `dEnd` because those + // are the starting points when copying a word at the end. + length := sub(length, 32) + let sEnd := add(source, length) + let dEnd := add(dest, length) + + // Remember the first 32 bytes of source + // This needs to be done here and not after the loop + // because we may have overwritten the first bytes in + // source already due to overlap. + let first := mload(source) + + // Copy whole words back to front + // We use a signed comparisson here to allow dEnd to become + // negative (happens when source and dest < 32). Valid + // addresses in local memory will never be larger than + // 2**255, so they can be safely re-interpreted as signed. + // Note: the first check is always true, + // this could have been a do-while loop. + for {} slt(dest, dEnd) {} { + mstore(dEnd, mload(sEnd)) + sEnd := sub(sEnd, 32) + dEnd := sub(dEnd, 32) + } + + // Write the first 32 bytes + mstore(dest, first) + } + } + } + } + + /// @dev Returns a slices from a byte array. + /// @param b The byte array to take a slice from. + /// @param from The starting index for the slice (inclusive). + /// @param to The final index for the slice (exclusive). + /// @return result The slice containing bytes at indices [from, to) + function slice(bytes memory b, uint256 from, uint256 to) + internal + pure + returns (bytes memory result) + { + require(from <= to, FROM_LESS_THAN_TO_REQUIRED); + require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED); + + // Create a new bytes structure and copy contents + result = new bytes(to - from); + memCopy( + result.contentAddress(), + b.contentAddress() + from, + result.length); + return result; + } + + /// @dev Returns a slice from a byte array without preserving the input. + /// @param b The byte array to take a slice from. Will be destroyed in the process. + /// @param from The starting index for the slice (inclusive). + /// @param to The final index for the slice (exclusive). + /// @return result The slice containing bytes at indices [from, to) + /// @dev When `from == 0`, the original array will match the slice. In other cases its state will be corrupted. + function sliceDestructive(bytes memory b, uint256 from, uint256 to) + internal + pure + returns (bytes memory result) + { + require(from <= to, FROM_LESS_THAN_TO_REQUIRED); + require(to < b.length, TO_LESS_THAN_LENGTH_REQUIRED); + + // Create a new bytes structure around [from, to) in-place. + assembly { + result := add(b, from) + mstore(result, sub(to, from)) + } + return result; + } /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -80,6 +258,24 @@ contract LibBytes is return result; } + /// @dev Tests equality of two byte arrays. + /// @param lhs First byte array to compare. + /// @param rhs Second byte array to compare. + /// @return True if arrays are the same. False otherwise. + function equals( + bytes memory lhs, + bytes memory rhs + ) + internal + pure + returns (bool equal) + { + // Keccak gas cost is 30 + numWords * 6. This is a cheap way to compare. + // We early exit on unequal lengths, but keccak would also correctly + // handle this. + return lhs.length == rhs.length && keccak256(lhs) == keccak256(rhs); + } + /// @dev Reads an address from a position in a byte array. /// @param b Byte array containing an address. /// @param index Index in byte array of address. @@ -145,6 +341,10 @@ contract LibBytes is // 2. Load 32-byte word from memory // 3. Apply 12-byte mask to obtain extra bytes occupying word of memory where we'll store the address let neighbors := and(mload(add(b, index)), 0xffffffffffffffffffffffff0000000000000000000000000000000000000000) + + // Make sure input address is clean. + // (Solidity does not guarantee this) + input := and(input, 0xffffffffffffffffffffffffffffffffffffffff) // Store the neighbors and address into memory mstore(add(b, index), xor(input, neighbors)) @@ -234,20 +434,26 @@ contract LibBytes is writeBytes32(b, index, bytes32(input)); } - /// @dev Reads the first 4 bytes from a byte array of arbitrary length. - /// @param b Byte array to read first 4 bytes from. - /// @return First 4 bytes of data. - function readFirst4(bytes memory b) + /// @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 pure returns (bytes4 result) { require( - b.length >= 4, + b.length >= index + 4, GREATER_OR_EQUAL_TO_4_LENGTH_REQUIRED ); 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; } @@ -256,7 +462,7 @@ contract LibBytes is /// @param b Byte array containing nested bytes. /// @param index Index of nested bytes. /// @return result Nested bytes. - function readBytes( + function readBytesWithLength( bytes memory b, uint256 index ) @@ -278,8 +484,8 @@ contract LibBytes is // Allocate memory and copy value to result result = new bytes(nestedBytesLength); memCopy( - getMemAddress(result) + 32, // +32 skips array length - getMemAddress(b) + index + 32, + result.contentAddress(), + b.contentAddress() + index, nestedBytesLength ); @@ -290,7 +496,7 @@ contract LibBytes is /// @param b Byte array to insert <input> into. /// @param index Index in byte array of <input>. /// @param input bytes to insert. - function writeBytes( + function writeBytesWithLength( bytes memory b, uint256 index, bytes memory input @@ -307,47 +513,12 @@ contract LibBytes is // Copy <input> into <b> memCopy( - getMemAddress(b) + 32 + index, // +32 to skip length of <b> - getMemAddress(input), // includes length of <input> - input.length + 32 // +32 bytes to store <input> length + b.contentAddress() + index, + input.rawAddress(), // includes length of <input> + input.length + 32 // +32 bytes to store <input> length ); } - /// @dev Tests equality of two byte arrays. - /// @param lhs First byte array to compare. - /// @param rhs Second byte array to compare. - /// @return True if arrays are the same. False otherwise. - function areBytesEqual( - bytes memory lhs, - bytes memory rhs - ) - internal - pure - returns (bool equal) - { - assembly { - // Get the number of words occupied by <lhs> - let lenFullWords := div(add(mload(lhs), 0x1F), 0x20) - - // Add 1 to the number of words, to account for the length field - lenFullWords := add(lenFullWords, 0x1) - - // Test equality word-by-word. - // Terminates early if there is a mismatch. - for {let i := 0} lt(i, lenFullWords) {i := add(i, 1)} { - let lhsWord := mload(add(lhs, mul(i, 0x20))) - let rhsWord := mload(add(rhs, mul(i, 0x20))) - equal := eq(lhsWord, rhsWord) - if eq(equal, 0) { - // Break - i := lenFullWords - } - } - } - - return equal; - } - /// @dev Performs a deep copy of a byte array onto another byte array of greater than or equal length. /// @param dest Byte array that will be overwritten with source bytes. /// @param source Byte array to copy onto dest bytes. @@ -365,8 +536,8 @@ contract LibBytes is GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED ); memCopy( - getMemAddress(dest) + 32, // +32 to skip length of <dest> - getMemAddress(source) + 32, // +32 to skip length of <source> + dest.contentAddress(), + source.contentAddress(), sourceLen ); } diff --git a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol b/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol deleted file mode 100644 index 97fb5fb0f..000000000 --- a/packages/contracts/src/contracts/current/utils/LibMem/LibMem.sol +++ /dev/null @@ -1,142 +0,0 @@ -/* - - 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; - -contract LibMem -{ - - /// @dev Gets the memory address for a byte array. - /// @param input Byte array to lookup. - /// @return memoryAddress Memory address of byte array. - function getMemAddress(bytes memory input) - internal - pure - returns (uint256 memoryAddress) - { - assembly { - memoryAddress := input - } - return memoryAddress; - } - - /// @dev Copies `length` bytes from memory location `source` to `dest`. - /// @param dest memory address to copy bytes to. - /// @param source memory address to copy bytes from. - /// @param length number of bytes to copy. - function memCopy( - uint256 dest, - uint256 source, - uint256 length - ) - internal - pure - { - if (length < 32) { - // Handle a partial word by reading destination and masking - // off the bits we are interested in. - // This correctly handles overlap, zero lengths and source == dest - assembly { - let mask := sub(exp(256, sub(32, length)), 1) - let s := and(mload(source), not(mask)) - let d := and(mload(dest), mask) - mstore(dest, or(s, d)) - } - } else { - // Skip the O(length) loop when source == dest. - if (source == dest) { - return; - } - - // For large copies we copy whole words at a time. The final - // word is aligned to the end of the range (instead of after the - // previous) to handle partial words. So a copy will look like this: - // - // #### - // #### - // #### - // #### - // - // We handle overlap in the source and destination range by - // changing the copying direction. This prevents us from - // overwriting parts of source that we still need to copy. - // - // This correctly handles source == dest - // - if (source > dest) { - assembly { - // We subtract 32 from `sEnd` and `dEnd` because it - // is easier to compare with in the loop, and these - // are also the addresses we need for copying the - // last bytes. - length := sub(length, 32) - let sEnd := add(source, length) - let dEnd := add(dest, length) - - // Remember the last 32 bytes of source - // This needs to be done here and not after the loop - // because we may have overwritten the last bytes in - // source already due to overlap. - let last := mload(sEnd) - - // Copy whole words front to back - // Note: the first check is always true, - // this could have been a do-while loop. - for {} lt(source, sEnd) {} { - mstore(dest, mload(source)) - source := add(source, 32) - dest := add(dest, 32) - } - - // Write the last 32 bytes - mstore(dEnd, last) - } - } else { - assembly { - // We subtract 32 from `sEnd` and `dEnd` because those - // are the starting points when copying a word at the end. - length := sub(length, 32) - let sEnd := add(source, length) - let dEnd := add(dest, length) - - // Remember the first 32 bytes of source - // This needs to be done here and not after the loop - // because we may have overwritten the first bytes in - // source already due to overlap. - let first := mload(source) - - // Copy whole words back to front - // We use a signed comparisson here to allow dEnd to become - // negative (happens when source and dest < 32). Valid - // addresses in local memory will never be larger than - // 2**255, so they can be safely re-interpreted as signed. - // Note: the first check is always true, - // this could have been a do-while loop. - for {} slt(dest, dEnd) {} { - mstore(dEnd, mload(sEnd)) - sEnd := sub(sEnd, 32) - dEnd := sub(dEnd, 32) - } - - // Write the first 32 bytes - mstore(dest, first) - } - } - } - } -} diff --git a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol index 296c6c856..99b93d0d2 100644 --- a/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol +++ b/packages/contracts/src/contracts/current/utils/Ownable/Ownable.sol @@ -13,6 +13,9 @@ import "./IOwnable.sol"; contract Ownable is IOwnable { address public owner; + // Revert reasons + string constant ONLY_CONTRACT_OWNER = "ONLY_CONTRACT_OWNER"; + constructor () public { @@ -22,7 +25,7 @@ contract Ownable is IOwnable { modifier onlyOwner() { require( msg.sender == owner, - "Only contract owner is allowed to call this method." + ONLY_CONTRACT_OWNER ); _; } diff --git a/packages/contracts/src/utils/artifacts.ts b/packages/contracts/src/utils/artifacts.ts index 7ba467708..a46bab9ae 100644 --- a/packages/contracts/src/utils/artifacts.ts +++ b/packages/contracts/src/utils/artifacts.ts @@ -13,10 +13,12 @@ import * as MultiSigWallet from '../artifacts/MultiSigWallet.json'; import * as MultiSigWalletWithTimeLock from '../artifacts/MultiSigWalletWithTimeLock.json'; import * as TestAssetDataDecoders from '../artifacts/TestAssetDataDecoders.json'; import * as TestAssetProxyDispatcher from '../artifacts/TestAssetProxyDispatcher.json'; +import * as TestAssetProxyOwner from '../artifacts/TestAssetProxyOwner.json'; import * as TestLibBytes from '../artifacts/TestLibBytes.json'; -import * as TestLibMem from '../artifacts/TestLibMem.json'; import * as TestLibs from '../artifacts/TestLibs.json'; import * as TestSignatureValidator from '../artifacts/TestSignatureValidator.json'; +import * as TestValidator from '../artifacts/TestValidator.json'; +import * as TestWallet from '../artifacts/TestWallet.json'; import * as TokenRegistry from '../artifacts/TokenRegistry.json'; import * as EtherToken from '../artifacts/WETH9.json'; import * as Whitelist from '../artifacts/Whitelist.json'; @@ -35,12 +37,14 @@ export const artifacts = { MixinAuthorizable: (MixinAuthorizable as any) as ContractArtifact, MultiSigWallet: (MultiSigWallet as any) as ContractArtifact, MultiSigWalletWithTimeLock: (MultiSigWalletWithTimeLock as any) as ContractArtifact, + TestAssetProxyOwner: (TestAssetProxyOwner as any) as ContractArtifact, TestAssetProxyDispatcher: (TestAssetProxyDispatcher as any) as ContractArtifact, TestAssetDataDecoders: (TestAssetDataDecoders as any) as ContractArtifact, TestLibBytes: (TestLibBytes as any) as ContractArtifact, - TestLibMem: (TestLibMem as any) as ContractArtifact, TestLibs: (TestLibs as any) as ContractArtifact, TestSignatureValidator: (TestSignatureValidator as any) as ContractArtifact, + TestValidator: (TestValidator as any) as ContractArtifact, + TestWallet: (TestWallet as any) as ContractArtifact, TokenRegistry: (TokenRegistry as any) as ContractArtifact, Whitelist: (Whitelist as any) as ContractArtifact, ZRX: (ZRX as any) as ContractArtifact, diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index c8b684c41..6999146c7 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -29,6 +29,11 @@ export const constants = { ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', ERC20_PROXY_ID: 1, ERC721_PROXY_ID: 2, + EXCHANGE_LENGTH_GREATER_THAN_0_REQUIRED: 'LENGTH_GREATER_THAN_0_REQUIRED', + EXCHANGE_SIGNATURE_UNSUPPORTED: 'SIGNATURE_UNSUPPORTED', + EXCHANGE_SIGNATURE_ILLEGAL: 'SIGNATURE_ILLEGAL', + EXCHANGE_LENGTH_0_REQUIRED: 'LENGTH_0_REQUIRED', + EXCHANGE_LENGTH_65_REQUIRED: 'LENGTH_65_REQUIRED', TESTRPC_NETWORK_ID: 50, // Note(albrow): In practice V8 and most other engines limit the minimum // interval for setInterval to 10ms. We still set it to 0 here in order to diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index 38dcb891a..ad68c8ff4 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -215,7 +215,7 @@ export class ExchangeWrapper { ): Promise<TransactionReceiptWithDecodedLogs> { const txHash = await this._exchange.executeTransaction.sendTransactionAsync( signedTx.salt, - signedTx.signer, + signedTx.signerAddress, signedTx.data, signedTx.signature, { from }, diff --git a/packages/contracts/src/utils/multi_sig_wrapper.ts b/packages/contracts/src/utils/multi_sig_wrapper.ts index f0098bd5e..b0d4fa8ab 100644 --- a/packages/contracts/src/utils/multi_sig_wrapper.ts +++ b/packages/contracts/src/utils/multi_sig_wrapper.ts @@ -40,13 +40,15 @@ export class MultiSigWrapper { const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } - public async executeRemoveAuthorizedAddressAsync( + public async executeRemoveAuthorizedAddressAtIndexAsync( txId: BigNumber, from: string, ): Promise<TransactionReceiptWithDecodedLogs> { // tslint:disable-next-line:no-unnecessary-type-assertion const txHash = await (this - ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddress.sendTransactionAsync(txId, { from }); + ._multiSig as AssetProxyOwnerContract).executeRemoveAuthorizedAddressAtIndex.sendTransactionAsync(txId, { + from, + }); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; } diff --git a/packages/contracts/src/utils/transaction_factory.ts b/packages/contracts/src/utils/transaction_factory.ts index 19ef4e1bf..348c0715d 100644 --- a/packages/contracts/src/utils/transaction_factory.ts +++ b/packages/contracts/src/utils/transaction_factory.ts @@ -9,7 +9,7 @@ const EIP712_ZEROEX_TRANSACTION_SCHEMA: EIP712Schema = { name: 'ZeroExTransaction', parameters: [ { name: 'salt', type: EIP712Types.Uint256 }, - { name: 'signer', type: EIP712Types.Address }, + { name: 'signerAddress', type: EIP712Types.Address }, { name: 'data', type: EIP712Types.Bytes }, ], }; @@ -25,10 +25,10 @@ export class TransactionFactory { } public newSignedTransaction(data: string, signatureType: SignatureType = SignatureType.EthSign): SignedTransaction { const salt = generatePseudoRandomSalt(); - const signer = `0x${this._signerBuff.toString('hex')}`; + const signerAddress = `0x${this._signerBuff.toString('hex')}`; const executeTransactionData = { salt, - signer, + signerAddress, data, }; const executeTransactionHashBuff = EIP712Utils.structHash( diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts index 7d9217c50..b792bb90a 100644 --- a/packages/contracts/src/utils/types.ts +++ b/packages/contracts/src/utils/types.ts @@ -92,7 +92,6 @@ export enum ContractName { Arbitrage = 'Arbitrage', TestAssetDataDecoders = 'TestAssetDataDecoders', TestAssetProxyDispatcher = 'TestAssetProxyDispatcher', - TestLibMem = 'TestLibMem', TestLibs = 'TestLibs', TestSignatureValidator = 'TestSignatureValidator', ERC20Proxy = 'ERC20Proxy', @@ -100,6 +99,7 @@ export enum ContractName { DummyERC721Receiver = 'DummyERC721Receiver', DummyERC721Token = 'DummyERC721Token', TestLibBytes = 'TestLibBytes', + TestWallet = 'TestWallet', Authorizable = 'Authorizable', Whitelist = 'Whitelist', } @@ -107,7 +107,7 @@ export enum ContractName { export interface SignedTransaction { exchangeAddress: string; salt: BigNumber; - signer: string; + signerAddress: string; data: string; signature: string; } |