diff options
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; } |