diff options
Diffstat (limited to 'packages/contracts')
4 files changed, 95 insertions, 90 deletions
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 <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. - 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 <dest> - getMemAddress(source) + 32, // +32 to skip length of <source> - 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 <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. + 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 <dest> + getMemAddress(source) + 32, // +32 to skip length of <source> + 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); diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts index 5371ae1d1..abba1ac4f 100644 --- a/packages/contracts/test/exchange/wrapper.ts +++ b/packages/contracts/test/exchange/wrapper.ts @@ -712,6 +712,10 @@ describe('Exchange wrappers', () => { ); await exchangeWrapper.marketSellOrdersNoThrowAsync(signedOrders, takerAddress, { takerAssetFillAmount, + // HACK(albrow): We need to hardcode the gas estimate here because + // the Geth gas estimator doesn't work with the way we use + // delegatecall and swallow errors. + gas: 6000000, }); const newBalances = await erc20Wrapper.getBalancesAsync(); |