From 3cc30f91a99578b626d95811a26ee7b19f404455 Mon Sep 17 00:00:00 2001 From: Alex Browne Date: Mon, 11 Jun 2018 17:04:47 -0700 Subject: Speedup awaitTransactionMinedAsync and reduce polling interval in contracts tests --- packages/contracts/src/utils/constants.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index af3f26d82..5e336589f 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -27,7 +27,10 @@ export const constants = { ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, - AWAIT_TRANSACTION_MINED_MS: 100, + // 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 + // ensure we always use the minimum interval. + AWAIT_TRANSACTION_MINED_MS: 0, MAX_ETHERTOKEN_WITHDRAW_GAS: 43000, MAX_TOKEN_TRANSFERFROM_GAS: 80000, MAX_TOKEN_APPROVE_GAS: 60000, -- cgit v1.2.3 From ee8c9b764d0ee153efa91075b35f3192b72be119 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sat, 9 Jun 2018 19:01:28 -0700 Subject: Pop id from assetData before dispatching to AssetProxies --- .../current/protocol/AssetProxy/ERC20Proxy.sol | 37 +-------------- .../current/protocol/AssetProxy/ERC721Proxy.sol | 34 ++++---------- .../AssetProxy/libs/LibAssetProxyErrors.sol | 5 --- .../Exchange/MixinAssetProxyDispatcher.sol | 14 +++--- .../protocol/Exchange/MixinExchangeCore.sol | 7 +-- .../current/protocol/Exchange/MixinMatchOrders.sol | 18 ++++---- .../current/protocol/Exchange/MixinSettlement.sol | 42 ++++++++++++----- .../protocol/Exchange/MixinWrapperFunctions.sol | 52 ++++++++++++---------- .../protocol/Exchange/libs/LibExchangeErrors.sol | 1 - .../Exchange/mixins/MAssetProxyDispatcher.sol | 2 + .../TestAssetDataDecoders.sol | 21 --------- .../TestAssetProxyDispatcher.sol | 3 +- .../contracts/current/utils/LibBytes/LibBytes.sol | 24 ++++++++++ packages/contracts/src/utils/exchange_wrapper.ts | 2 +- packages/contracts/src/utils/formatters.ts | 16 ++++++- packages/contracts/src/utils/order_utils.ts | 3 ++ 16 files changed, 132 insertions(+), 149 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol index dd25bf41a..ddcd78e93 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC20Proxy.sol @@ -47,16 +47,7 @@ contract ERC20Proxy is internal { // Decode asset data. - ( - uint8 proxyId, - address token - ) = decodeERC20AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); + address token = readAddress(assetData, 0); // Transfer tokens. bool success = IERC20Token(token).transferFrom(from, to, amount); @@ -75,30 +66,4 @@ contract ERC20Proxy is { return PROXY_ID; } - - /// @dev Decodes ERC20 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC20 proxy id. - /// @return token ERC20 token address. - function decodeERC20AssetData(bytes memory assetData) - internal - pure - returns ( - uint8 proxyId, - address token - ) - { - // Validate encoded data length - uint256 length = assetData.length; - require( - length == 21, - LENGTH_21_REQUIRED - ); - - // Decode data - token = readAddress(assetData, 0); - proxyId = uint8(assetData[length - 1]); - - return (proxyId, token); - } } diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol index 25136133d..861fac2c1 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/ERC721Proxy.sol @@ -46,30 +46,22 @@ contract ERC721Proxy is ) internal { + // There exists only 1 of each token. + require( + amount == 1, + INVALID_AMOUNT + ); + // Decode asset data. ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) = decodeERC721AssetData(assetData); - - // Data must be intended for this proxy. - require( - proxyId == PROXY_ID, - ASSET_PROXY_ID_MISMATCH - ); - - // There exists only 1 of each token. - require( - amount == 1, - INVALID_AMOUNT - ); - // Transfer token. Saves gas by calling safeTransferFrom only // when there is receiverData present. Either succeeds or throws. - if(receiverData.length > 0) { + if (receiverData.length > 0) { ERC721Token(token).safeTransferFrom(from, to, tokenId, receiverData); } else { ERC721Token(token).transferFrom(from, to, tokenId); @@ -97,29 +89,19 @@ contract ERC721Proxy is internal pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { - // Validate encoded data length - uint256 length = assetData.length; - require( - length >= 53, - LENGTH_AT_LEAST_53_REQUIRED - ); - // Decode asset data. token = readAddress(assetData, 0); tokenId = readUint256(assetData, 20); - if (length > 53) { + if (assetData.length > 52) { receiverData = readBytes(assetData, 52); } - proxyId = uint8(assetData[length - 1]); return ( - proxyId, token, tokenId, receiverData diff --git a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol index 80180a0d9..65bdacdb7 100644 --- a/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/AssetProxy/libs/LibAssetProxyErrors.sol @@ -27,11 +27,6 @@ contract LibAssetProxyErrors { string constant AUTHORIZED_ADDRESS_MISMATCH = "AUTHORIZED_ADDRESS_MISMATCH"; // Address at index does not match given target address. /// AssetProxy errors /// - string constant ASSET_PROXY_ID_MISMATCH = "ASSET_PROXY_ID_MISMATCH"; // Proxy id in metadata does not match this proxy id. string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Transfer amount must equal 1. string constant TRANSFER_FAILED = "TRANSFER_FAILED"; // Transfer failed. - - /// Length validation errors /// - string constant LENGTH_21_REQUIRED = "LENGTH_21_REQUIRED"; // Byte array must have a length of 21. - string constant LENGTH_AT_LEAST_53_REQUIRED = "LENGTH_AT_LEAST_53_REQUIRED"; // Byte array must have a length of at least 53. } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol index e77d81c06..9e0246303 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinAssetProxyDispatcher.sol @@ -19,12 +19,14 @@ 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 { @@ -81,11 +83,13 @@ contract MixinAssetProxyDispatcher is /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. /// @param assetData Byte array encoded for the respective asset proxy. + /// @param assetProxyId Id of assetProxy to dispach to. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount @@ -94,16 +98,8 @@ contract MixinAssetProxyDispatcher is { // Do nothing if no amount should be transferred. if (amount > 0) { - - // Lookup asset proxy - uint256 length = assetData.length; - require( - length > 0, - LENGTH_GREATER_THAN_0_REQUIRED - ); - uint8 assetProxyId = uint8(assetData[length - 1]); + // Lookup assetProxy IAssetProxy assetProxy = assetProxies[assetProxyId]; - // transferFrom will either succeed or throw. 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 12b57d99f..0a0f0209a 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol @@ -108,9 +108,6 @@ contract MixinExchangeCore is // Compute proportional fill amounts fillResults = calculateFillResults(order, takerAssetFilledAmount); - // Settle order - settleOrder(order, takerAddress, fillResults); - // Update exchange internal state updateFilledState( order, @@ -119,6 +116,10 @@ contract MixinExchangeCore is orderInfo.orderTakerAssetFilledAmount, fillResults ); + + // Settle order + settleOrder(order, takerAddress, fillResults); + return fillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol index ed76287e0..517b743fe 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinMatchOrders.sol @@ -14,7 +14,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -25,7 +24,6 @@ import "./mixins/MSettlement.sol"; import "./mixins/MTransactions.sol"; contract MixinMatchOrders is - LibBytes, LibMath, LibExchangeErrors, MExchangeCore, @@ -94,14 +92,6 @@ contract MixinMatchOrders is rightSignature ); - // Settle matched orders. Succeeds or throws. - settleMatchedOrders( - leftOrder, - rightOrder, - takerAddress, - matchedFillResults - ); - // Update exchange state updateFilledState( leftOrder, @@ -117,6 +107,14 @@ contract MixinMatchOrders is rightOrderInfo.orderTakerAssetFilledAmount, matchedFillResults.right ); + + // Settle matched orders. Succeeds or throws. + settleMatchedOrders( + leftOrder, + rightOrder, + takerAddress, + matchedFillResults + ); return matchedFillResults; } diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 83e9dfdf4..f0caf7446 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -19,6 +19,7 @@ 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"; @@ -28,6 +29,7 @@ import "./mixins/MSettlement.sol"; import "./mixins/MAssetProxyDispatcher.sol"; contract MixinSettlement is + LibBytes, LibMath, LibExchangeErrors, MMatchOrders, @@ -37,7 +39,7 @@ contract MixinSettlement is // ZRX metadata used for fee transfers. // This will be constant throughout the life of the Exchange contract, // since ZRX will always be transferred via the ERC20 AssetProxy. - bytes internal ZRX_PROXY_DATA; + bytes internal ZRX_ASSET_DATA; /// @dev Gets the ZRX metadata used for fee transfers. function zrxAssetData() @@ -45,7 +47,7 @@ contract MixinSettlement is view returns (bytes memory) { - return ZRX_PROXY_DATA; + return ZRX_ASSET_DATA; } /// TODO: _zrxAssetData should be a constant in production. @@ -54,7 +56,7 @@ contract MixinSettlement is constructor (bytes memory _zrxAssetData) public { - ZRX_PROXY_DATA = _zrxAssetData; + ZRX_ASSET_DATA = _zrxAssetData; } /// @dev Settles an order by transferring assets between counterparties. @@ -68,26 +70,34 @@ contract MixinSettlement is ) internal { + uint8 makerAssetProxyId = uint8(popByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popByte(order.takerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + uint8 zrxProxyId = uint8(popByte(zrxAssetData)); dispatchTransferFrom( order.makerAssetData, + makerAssetProxyId, order.makerAddress, takerAddress, fillResults.makerAssetFilledAmount ); dispatchTransferFrom( order.takerAssetData, + takerAssetProxyId, takerAddress, order.makerAddress, fillResults.takerAssetFilledAmount ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid @@ -107,21 +117,28 @@ contract MixinSettlement is ) internal { + uint8 leftMakerAssetProxyId = uint8(popByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popByte(rightOrder.makerAssetData)); + bytes memory zrxAssetData = ZRX_ASSET_DATA; + uint8 zrxProxyId = uint8(popByte(zrxAssetData)); // 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 @@ -129,13 +146,15 @@ contract MixinSettlement is // Maker fees dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -144,7 +163,8 @@ contract MixinSettlement is // Taker fees if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -154,13 +174,15 @@ contract MixinSettlement is ); } else { dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( - ZRX_PROXY_DATA, + zrxAssetData, + zrxProxyId, takerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.takerFeePaid diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index e09f80bcc..cd5e26fb7 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -335,12 +335,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. + // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); + } // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -379,12 +380,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being sold by taker must be the same for each order - // TODO: optimize by only using takerAssetData for first order. - require( - areBytesEqual(orders[i].takerAssetData, orders[0].takerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being sold by taker is the same for each order. + // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. + // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); + } // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -422,12 +424,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); + } // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); @@ -474,12 +477,13 @@ contract MixinWrapperFunctions is { for (uint256 i = 0; i < orders.length; i++) { - // Token being bought by taker must be the same for each order - // TODO: optimize by only using makerAssetData for first order. - require( - areBytesEqual(orders[i].makerAssetData, orders[0].makerAssetData), - ASSET_DATA_MISMATCH - ); + // We assume that asset being bought by taker is the same for each order. + // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. + // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. + uint256 next = i + 1; + if (next != orders.length) { + deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); + } // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); 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 2b4bbeec4..48dd0f8be 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/libs/LibExchangeErrors.sol @@ -25,7 +25,6 @@ contract LibExchangeErrors { 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 ASSET_DATA_MISMATCH = "ASSET_DATA_MISMATCH"; // Asset data must be the same for each order. /// fillOrder validation errors /// string constant INVALID_TAKER_AMOUNT = "INVALID_TAKER_AMOUNT"; // takerAssetFillAmount cannot equal 0. 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 82eafb529..d16a830f4 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MAssetProxyDispatcher.sol @@ -34,11 +34,13 @@ contract MAssetProxyDispatcher is /// @dev Forwards arguments to assetProxy and calls `transferFrom`. Either succeeds or throws. /// @param assetData Byte array encoded for the respective asset proxy. + /// @param assetProxyId Id of assetProxy to dispach to. /// @param from Address to transfer token from. /// @param to Address to transfer token to. /// @param amount Amount of token to transfer. function dispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount diff --git a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol index 2c6a8fdb0..5987291d2 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetDataDecoders/TestAssetDataDecoders.sol @@ -23,26 +23,8 @@ import "../../protocol/AssetProxy/ERC20Proxy.sol"; import "../../protocol/AssetProxy/ERC721Proxy.sol"; contract TestAssetDataDecoders is - ERC20Proxy, ERC721Proxy { - - /// @dev Decodes ERC20 Asset data. - /// @param assetData Encoded byte array. - /// @return proxyId Intended ERC20 proxy id. - /// @return token ERC20 token address. - function publicDecodeERC20Data(bytes memory assetData) - public - pure - returns ( - uint8 proxyId, - address token - ) - { - (proxyId, token) = decodeERC20AssetData(assetData); - return (proxyId, token); - } - /// @dev Decodes ERC721 Asset data. /// @param assetData Encoded byte array. /// @return proxyId Intended ERC721 proxy id. @@ -54,21 +36,18 @@ contract TestAssetDataDecoders is public pure returns ( - uint8 proxyId, address token, uint256 tokenId, bytes memory receiverData ) { ( - proxyId, token, tokenId, receiverData ) = decodeERC721AssetData(assetData); return ( - proxyId, token, tokenId, receiverData diff --git a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol index 2ae69e0ef..d469a07f0 100644 --- a/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol +++ b/packages/contracts/src/contracts/current/test/TestAssetProxyDispatcher/TestAssetProxyDispatcher.sol @@ -24,11 +24,12 @@ import "../../protocol/Exchange/MixinAssetProxyDispatcher.sol"; contract TestAssetProxyDispatcher is MixinAssetProxyDispatcher { function publicDispatchTransferFrom( bytes memory assetData, + uint8 assetProxyId, address from, address to, uint256 amount) public { - dispatchTransferFrom(assetData, from, to, amount); + dispatchTransferFrom(assetData, assetProxyId, from, to, amount); } } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index d1d10476f..aac0ffc31 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -30,6 +30,7 @@ contract LibBytes is string constant GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED"; string constant GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED"; + string constant GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED = "GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED"; /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. @@ -114,6 +115,29 @@ contract LibBytes is 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. + function deepCopyBytes( + bytes memory dest, + bytes memory source + ) + internal + pure + { + uint256 sourceLen = source.length; + // Dest length must be >= source length, or some bytes would not be copied. + require( + dest.length >= sourceLen, + GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED + ); + memCopy( + getMemAddress(dest) + 32, // +32 to skip length of + getMemAddress(source) + 32, // +32 to skip length of + sourceLen + ); + } + /// @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. diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index a8ca5183e..6603538b9 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -165,7 +165,7 @@ export class ExchangeWrapper { public async marketBuyOrdersNoThrowAsync( orders: SignedOrder[], from: string, - opts: { makerAssetFillAmount: BigNumber }, + opts: { makerAssetFillAmount: BigNumber; gas?: number }, ): Promise { const params = formatters.createMarketBuyOrders(orders, opts.makerAssetFillAmount); const txHash = await this._exchange.marketBuyOrdersNoThrow.sendTransactionAsync( diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts index 1035f2d7c..b25dec27c 100644 --- a/packages/contracts/src/utils/formatters.ts +++ b/packages/contracts/src/utils/formatters.ts @@ -28,8 +28,14 @@ export const formatters = { signatures: [], takerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.takerAssetData = `0x${_.repeat( + '0', + signedOrders[0].takerAssetData.length - 2, + )}`; + } marketSellOrders.orders.push(orderWithoutExchangeAddress); marketSellOrders.signatures.push(signedOrder.signature); }); @@ -41,8 +47,14 @@ export const formatters = { signatures: [], makerAssetFillAmount, }; - _.forEach(signedOrders, signedOrder => { + _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); + if (i !== 0) { + orderWithoutExchangeAddress.makerAssetData = `0x${_.repeat( + '0', + signedOrders[0].makerAssetData.length - 2, + )}`; + } marketBuyOrders.orders.push(orderWithoutExchangeAddress); marketBuyOrders.signatures.push(signedOrder.signature); }); diff --git a/packages/contracts/src/utils/order_utils.ts b/packages/contracts/src/utils/order_utils.ts index 2a8791e4c..a9f994d80 100644 --- a/packages/contracts/src/utils/order_utils.ts +++ b/packages/contracts/src/utils/order_utils.ts @@ -1,6 +1,7 @@ import { OrderWithoutExchangeAddress, SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; +import { constants } from './constants'; import { CancelOrder, MatchOrder } from './types'; export const orderUtils = { @@ -43,6 +44,8 @@ export const orderUtils = { leftSignature: signedOrderLeft.signature, rightSignature: signedOrderRight.signature, }; + fill.right.makerAssetData = constants.NULL_BYTES; + fill.right.takerAssetData = constants.NULL_BYTES; return fill; }, }; -- cgit v1.2.3 From 764b1c35cb7bf763deeb5db34ebe36d6e973b409 Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 10 Jun 2018 16:09:07 -0700 Subject: Add tests for deepCopyBytes and missing write methods from LibBytes --- .../contracts/current/test/TestLibBytes/TestLibBytes.sol | 15 +++++++++++++++ packages/contracts/src/utils/constants.ts | 1 + 2 files changed, 16 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index 22c84504c..abce0cb22 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -62,6 +62,21 @@ contract TestLibBytes is 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. + function publicDeepCopyBytes( + bytes memory dest, + bytes memory source + ) + public + pure + returns (bytes memory) + { + deepCopyBytes(dest, source); + return dest; + } + /// @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. diff --git a/packages/contracts/src/utils/constants.ts b/packages/contracts/src/utils/constants.ts index 5e336589f..ec3c8fd36 100644 --- a/packages/contracts/src/utils/constants.ts +++ b/packages/contracts/src/utils/constants.ts @@ -24,6 +24,7 @@ export const constants = { LIB_BYTES_GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_20_LENGTH_REQUIRED', LIB_BYTES_GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED', LIB_BYTES_GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_NESTED_BYTES_LENGTH_REQUIRED', + LIB_BYTES_GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED: 'GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED', ERC20_INSUFFICIENT_BALANCE: 'Insufficient balance to complete transfer.', ERC20_INSUFFICIENT_ALLOWANCE: 'Insufficient allowance to complete transfer.', TESTRPC_NETWORK_ID: 50, -- cgit v1.2.3 From 5910bec52e0664f70d5dc98ce8303ec5373107ba Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Sun, 10 Jun 2018 21:13:59 -0700 Subject: Make ZRX_PROXY_ID constant rather than popping it from ZRX_ASSET_DATA --- .../current/protocol/Exchange/MixinSettlement.sol | 28 +++++++--------------- 1 file changed, 9 insertions(+), 19 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index f0caf7446..69b70112f 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -36,19 +36,11 @@ contract MixinSettlement is MSettlement, MAssetProxyDispatcher { - // ZRX metadata used for fee transfers. + // 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; - - /// @dev Gets the ZRX metadata used for fee transfers. - function zrxAssetData() - external - view - returns (bytes memory) - { - return 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. @@ -73,7 +65,6 @@ contract MixinSettlement is uint8 makerAssetProxyId = uint8(popByte(order.makerAssetData)); uint8 takerAssetProxyId = uint8(popByte(order.takerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; - uint8 zrxProxyId = uint8(popByte(zrxAssetData)); dispatchTransferFrom( order.makerAssetData, makerAssetProxyId, @@ -90,14 +81,14 @@ contract MixinSettlement is ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, order.makerAddress, order.feeRecipientAddress, fillResults.makerFeePaid ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, order.feeRecipientAddress, fillResults.takerFeePaid @@ -120,7 +111,6 @@ contract MixinSettlement is uint8 leftMakerAssetProxyId = uint8(popByte(leftOrder.makerAssetData)); uint8 rightMakerAssetProxyId = uint8(popByte(rightOrder.makerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; - uint8 zrxProxyId = uint8(popByte(zrxAssetData)); // Order makers and taker dispatchTransferFrom( leftOrder.makerAssetData, @@ -147,14 +137,14 @@ contract MixinSettlement is // Maker fees dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, leftOrder.makerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.makerFeePaid ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, rightOrder.makerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.makerFeePaid @@ -164,7 +154,7 @@ contract MixinSettlement is if (leftOrder.feeRecipientAddress == rightOrder.feeRecipientAddress) { dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, safeAdd( @@ -175,14 +165,14 @@ contract MixinSettlement is } else { dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, leftOrder.feeRecipientAddress, matchedFillResults.left.takerFeePaid ); dispatchTransferFrom( zrxAssetData, - zrxProxyId, + ZRX_PROXY_ID, takerAddress, rightOrder.feeRecipientAddress, matchedFillResults.right.takerFeePaid -- cgit v1.2.3 From 2f96cb257c0f7280f8b578eed6a3c1711749c0e9 Mon Sep 17 00:00:00 2001 From: Greg Hysen Date: Mon, 11 Jun 2018 11:58:18 -0700 Subject: Looks up the memory location of makerAssetData/takerAssetData --- .../src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol | 2 ++ 1 file changed, 2 insertions(+) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index cd5e26fb7..88f916179 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -174,6 +174,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(10, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of + sourceOffset := mload(add(order, 0x140)) // makerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) @@ -193,6 +194,7 @@ contract MixinWrapperFunctions is mstore(add(dataAreaStart, mul(11, 0x20)), sub(dataAreaEnd, dataAreaStart)) // Calculate length of + sourceOffset := mload(add(order, 0x160)) // takerAssetData arrayLenBytes := mload(sourceOffset) sourceOffset := add(sourceOffset, 0x20) arrayLenWords := div(add(arrayLenBytes, 0x1F), 0x20) -- cgit v1.2.3 From a0a90afbc0962eb70b2abb3d24aef80a8d8a822d Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Mon, 11 Jun 2018 19:44:27 -0700 Subject: Pass gas in to marketBuyOrdersNoThrow --- packages/contracts/src/utils/exchange_wrapper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts index 6603538b9..4cc8f0b89 100644 --- a/packages/contracts/src/utils/exchange_wrapper.ts +++ b/packages/contracts/src/utils/exchange_wrapper.ts @@ -172,7 +172,7 @@ export class ExchangeWrapper { params.orders, params.makerAssetFillAmount, params.signatures, - { from }, + { from, gas: opts.gas }, ); const tx = await this._logDecoder.getTxWithDecodedLogsAsync(txHash); return tx; -- cgit v1.2.3 From 3a5f3e8b55f4b9733ef542281ee71c4fdc9cd39d Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 12 Jun 2018 11:43:19 -0700 Subject: Unpop byte rather than making deep copy --- .../protocol/Exchange/MixinWrapperFunctions.sol | 54 +++++----- .../contracts/current/utils/LibBytes/LibBytes.sol | 116 ++++++++++----------- packages/contracts/src/utils/formatters.ts | 11 +- 3 files changed, 91 insertions(+), 90 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol index 88f916179..a7849f4cb 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol @@ -19,7 +19,6 @@ pragma solidity ^0.4.24; pragma experimental ABIEncoderV2; -import "../../utils/LibBytes/LibBytes.sol"; import "./libs/LibMath.sol"; import "./libs/LibOrder.sol"; import "./libs/LibFillResults.sol"; @@ -27,7 +26,6 @@ import "./libs/LibExchangeErrors.sol"; import "./mixins/MExchangeCore.sol"; contract MixinWrapperFunctions is - LibBytes, LibMath, LibFillResults, LibExchangeErrors, @@ -335,15 +333,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being sold by taker is the same for each order. - // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. - // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); - } + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -355,6 +351,14 @@ contract MixinWrapperFunctions is signatures[i] ); + // HACK: the proxyId is "popped" from the byte array before a fill is settled + // by subtracting from the length of the array. Since the popped byte is + // still in memory, we can "unpop" it by incrementing the length of the byte array. + assembly { + let len := mload(takerAssetData) + mstore(takerAssetData, add(len, 1)) + } + // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -380,15 +384,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory takerAssetData = orders[0].takerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being sold by taker is the same for each order. - // Rather than passing this in as calldata, we copy the takerAssetData from the first order onto all later orders. - // We cannot reference the same takerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].takerAssetData, orders[i].takerAssetData); - } + // Rather than passing this in as calldata, we use the takerAssetData from the first order in all later orders. + orders[i].takerAssetData = takerAssetData; // Calculate the remaining amount of takerAsset to sell uint256 remainingTakerAssetFillAmount = safeSub(takerAssetFillAmount, totalFillResults.takerAssetFilledAmount); @@ -424,15 +426,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being bought by taker is the same for each order. // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. - // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); - } + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); @@ -452,6 +452,14 @@ contract MixinWrapperFunctions is signatures[i] ); + // HACK: the proxyId is "popped" from the byte array before a fill is settled + // by subtracting from the length of the array. Since the popped byte is + // still in memory, we can "unpop" it by incrementing the length of the byte array. + assembly { + let len := mload(makerAssetData) + mstore(makerAssetData, add(len, 1)) + } + // Update amounts filled and fees paid by maker and taker addFillResults(totalFillResults, singleFillResults); @@ -477,15 +485,13 @@ contract MixinWrapperFunctions is public returns (FillResults memory totalFillResults) { + bytes memory makerAssetData = orders[0].makerAssetData; + for (uint256 i = 0; i < orders.length; i++) { // We assume that asset being bought by taker is the same for each order. // Rather than passing this in as calldata, we copy the makerAssetData from the first order onto all later orders. - // We cannot reference the same makerAssetData byte array because the array is modified when a trade is settled. - uint256 next = i + 1; - if (next != orders.length) { - deepCopyBytes(orders[next].makerAssetData, orders[i].makerAssetData); - } + orders[i].makerAssetData = makerAssetData; // Calculate the remaining amount of makerAsset to buy uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index aac0ffc31..339270a57 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -80,64 +80,6 @@ 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 areBytesEqual( - bytes memory lhs, - bytes memory rhs - ) - internal - pure - returns (bool equal) - { - assembly { - // Get the number of words occupied by - 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. - function deepCopyBytes( - bytes memory dest, - bytes memory source - ) - internal - pure - { - uint256 sourceLen = source.length; - // Dest length must be >= source length, or some bytes would not be copied. - require( - dest.length >= sourceLen, - GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED - ); - memCopy( - getMemAddress(dest) + 32, // +32 to skip length of - getMemAddress(source) + 32, // +32 to skip length of - sourceLen - ); - } - /// @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. @@ -370,4 +312,62 @@ contract LibBytes is input.length + 32 // +32 bytes to store 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 + 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. + function deepCopyBytes( + bytes memory dest, + bytes memory source + ) + internal + pure + { + uint256 sourceLen = source.length; + // Dest length must be >= source length, or some bytes would not be copied. + require( + dest.length >= sourceLen, + GREATER_OR_EQUAL_TO_SOURCE_BYTES_LENGTH_REQUIRED + ); + memCopy( + getMemAddress(dest) + 32, // +32 to skip length of + getMemAddress(source) + 32, // +32 to skip length of + sourceLen + ); + } } diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts index b25dec27c..32e4787d6 100644 --- a/packages/contracts/src/utils/formatters.ts +++ b/packages/contracts/src/utils/formatters.ts @@ -2,6 +2,7 @@ import { SignedOrder } from '@0xproject/types'; import { BigNumber } from '@0xproject/utils'; import * as _ from 'lodash'; +import { constants } from './constants'; import { orderUtils } from './order_utils'; import { BatchCancelOrders, BatchFillOrders, MarketBuyOrders, MarketSellOrders } from './types'; @@ -31,10 +32,7 @@ export const formatters = { _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); if (i !== 0) { - orderWithoutExchangeAddress.takerAssetData = `0x${_.repeat( - '0', - signedOrders[0].takerAssetData.length - 2, - )}`; + orderWithoutExchangeAddress.takerAssetData = constants.NULL_BYTES; } marketSellOrders.orders.push(orderWithoutExchangeAddress); marketSellOrders.signatures.push(signedOrder.signature); @@ -50,10 +48,7 @@ export const formatters = { _.forEach(signedOrders, (signedOrder, i) => { const orderWithoutExchangeAddress = orderUtils.getOrderWithoutExchangeAddress(signedOrder); if (i !== 0) { - orderWithoutExchangeAddress.makerAssetData = `0x${_.repeat( - '0', - signedOrders[0].makerAssetData.length - 2, - )}`; + orderWithoutExchangeAddress.makerAssetData = constants.NULL_BYTES; } marketBuyOrders.orders.push(orderWithoutExchangeAddress); marketBuyOrders.signatures.push(signedOrder.signature); -- cgit v1.2.3 From 0917fa0d75b7328c156af2ffafa641ae09286e2e Mon Sep 17 00:00:00 2001 From: Amir Bandeali Date: Tue, 12 Jun 2018 15:29:18 -0700 Subject: Rename popByte and popAddress --- .../src/contracts/current/protocol/Exchange/MixinSettlement.sol | 8 ++++---- .../current/protocol/Exchange/MixinSignatureValidator.sol | 4 ++-- .../src/contracts/current/test/TestLibBytes/TestLibBytes.sol | 8 ++++---- .../contracts/src/contracts/current/utils/LibBytes/LibBytes.sol | 4 ++-- 4 files changed, 12 insertions(+), 12 deletions(-) (limited to 'packages/contracts/src') diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol index 69b70112f..29a9c87bd 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSettlement.sol @@ -62,8 +62,8 @@ contract MixinSettlement is ) internal { - uint8 makerAssetProxyId = uint8(popByte(order.makerAssetData)); - uint8 takerAssetProxyId = uint8(popByte(order.takerAssetData)); + uint8 makerAssetProxyId = uint8(popLastByte(order.makerAssetData)); + uint8 takerAssetProxyId = uint8(popLastByte(order.takerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; dispatchTransferFrom( order.makerAssetData, @@ -108,8 +108,8 @@ contract MixinSettlement is ) internal { - uint8 leftMakerAssetProxyId = uint8(popByte(leftOrder.makerAssetData)); - uint8 rightMakerAssetProxyId = uint8(popByte(rightOrder.makerAssetData)); + uint8 leftMakerAssetProxyId = uint8(popLastByte(leftOrder.makerAssetData)); + uint8 rightMakerAssetProxyId = uint8(popLastByte(rightOrder.makerAssetData)); bytes memory zrxAssetData = ZRX_ASSET_DATA; // Order makers and taker dispatchTransferFrom( diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol index 48a0c5552..1a556dfe2 100644 --- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol +++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinSignatureValidator.sol @@ -93,7 +93,7 @@ contract MixinSignatureValidator is ); // Pop last byte off of signature byte array. - SignatureType signatureType = SignatureType(uint8(popByte(signature))); + SignatureType signatureType = SignatureType(uint8(popLastByte(signature))); // Variables are not scoped in Solidity. uint8 v; @@ -183,7 +183,7 @@ 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 = popAddress(signature); + address validator = popLast20Bytes(signature); // Ensure signer has approved validator. if (!allowedValidators[signer][validator]) { return false; diff --git a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol index abce0cb22..6f1898acd 100644 --- a/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol +++ b/packages/contracts/src/contracts/current/test/TestLibBytes/TestLibBytes.sol @@ -28,24 +28,24 @@ contract TestLibBytes is /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function publicPopByte(bytes memory b) + function publicPopLastByte(bytes memory b) public pure returns (bytes memory, bytes1 result) { - result = popByte(b); + result = popLastByte(b); return (b, result); } /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function publicPopAddress(bytes memory b) + function publicPopLast20Bytes(bytes memory b) public pure returns (bytes memory, address result) { - result = popAddress(b); + result = popLast20Bytes(b); return (b, result); } diff --git a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol index 339270a57..10d7ce41a 100644 --- a/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol +++ b/packages/contracts/src/contracts/current/utils/LibBytes/LibBytes.sol @@ -35,7 +35,7 @@ contract LibBytes is /// @dev Pops the last byte off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The byte that was popped off. - function popByte(bytes memory b) + function popLastByte(bytes memory b) internal pure returns (bytes1 result) @@ -59,7 +59,7 @@ contract LibBytes is /// @dev Pops the last 20 bytes off of a byte array by modifying its length. /// @param b Byte array that will be modified. /// @return The 20 byte address that was popped off. - function popAddress(bytes memory b) + function popLast20Bytes(bytes memory b) internal pure returns (address result) -- cgit v1.2.3