Changed the returndata for batchMatchOrders

This commit is contained in:
James Towle
2019-07-09 17:38:11 -05:00
committed by Amir Bandeali
parent 1c1d257bd9
commit ffa32f7610
4 changed files with 134 additions and 46 deletions

View File

@@ -1,6 +1,6 @@
/*
Copyright 2018 ZeroEx Intl.
Copyright 2019 ZeroEx Intl.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.

View File

@@ -296,17 +296,10 @@ contract MixinMatchOrders is
_rrevert(BatchMatchOrdersError(BatchMatchOrdersErrorCodes.INCOMPATIBLE_RIGHT_ORDERS));
}
// Without simulating all of the order matching, this program cannot know how many
// matches there will be. To ensure that batchMatchedFillResults has enough memory
// allocated for the left and the right side, we will allocate enough space for the
// maximum amount of matches (the left side length added to the right side length
// minus 1).
uint256 maxLength = leftOrders.length + rightOrders.length - 1;
batchMatchedFillResults.left = new LibFillResults.FillResults[](maxLength);
batchMatchedFillResults.right = new LibFillResults.FillResults[](maxLength);
batchMatchedFillResults.left = new LibFillResults.FillResults[](leftOrders.length);
batchMatchedFillResults.right = new LibFillResults.FillResults[](rightOrders.length);
// Set up initial indices.
uint256 matchCount;
uint256 leftIdx = 0;
uint256 rightIdx = 0;
@@ -315,10 +308,12 @@ contract MixinMatchOrders is
LibOrder.Order memory rightOrder = rightOrders[0];
LibOrder.OrderInfo memory leftOrderInfo = getOrderInfo(leftOrder);
LibOrder.OrderInfo memory rightOrderInfo = getOrderInfo(rightOrder);
LibFillResults.FillResults memory leftFillResults;
LibFillResults.FillResults memory rightFillResults;
// Loop infinitely (until broken inside of the loop), but keep a counter of how
// many orders have been matched.
for (matchCount = 0;;) {
for (;;) {
// Match the two orders that are pointed to by the left and right indices
LibFillResults.MatchedFillResults memory matchResults = _matchOrders(
leftOrder,
@@ -338,10 +333,18 @@ contract MixinMatchOrders is
matchResults.right.takerAssetFilledAmount
);
// Add the matchResults and the profit made during the match to the
// batchMatchedFillResults for this batch.
batchMatchedFillResults.left[matchCount] = matchResults.left;
batchMatchedFillResults.right[matchCount] = matchResults.right;
// Aggregate the new fill results with the previous fill results for the current orders.
_addFillResults(
leftFillResults,
matchResults.left
);
_addFillResults(
rightFillResults,
matchResults.right
);
// Update the profit in the left and right maker assets using the profits from
// the match.
batchMatchedFillResults.profitInLeftMakerAsset = _safeAdd(
batchMatchedFillResults.profitInLeftMakerAsset,
matchResults.profitInLeftMakerAsset
@@ -351,13 +354,19 @@ contract MixinMatchOrders is
matchResults.profitInRightMakerAsset
);
// Increment the number of matches
matchCount++;
// If the leftOrder is filled, update the leftIdx, leftOrder, and leftSignature,
// or break out of the loop if there are no more leftOrders to match.
if (leftOrderInfo.orderTakerAssetFilledAmount >= leftOrder.takerAssetAmount) {
// Update the batched fill results once the leftIdx is updated.
batchMatchedFillResults.left[leftIdx] = leftFillResults;
// Clear the intermediate fill results value.
leftFillResults = LibFillResults.FillResults(0, 0, 0, 0);
// If all of the right orders have been filled, break out of the loop.
// Otherwise, update the current right order.
if (++leftIdx == leftOrders.length) {
// Update the right batched fill results
batchMatchedFillResults.right[rightIdx] = rightFillResults;
break;
} else {
leftOrder = leftOrders[leftIdx];
@@ -368,7 +377,16 @@ contract MixinMatchOrders is
// If the rightOrder is filled, update the rightIdx, rightOrder, and rightSignature,
// or break out of the loop if there are no more rightOrders to match.
if (rightOrderInfo.orderTakerAssetFilledAmount >= rightOrder.takerAssetAmount) {
// Update the batched fill results once the rightIdx is updated.
batchMatchedFillResults.right[rightIdx] = rightFillResults;
// Clear the intermediate fill results value.
rightFillResults = LibFillResults.FillResults(0, 0, 0, 0);
// If all of the right orders have been filled, break out of the loop.
// Otherwise, update the current right order.
if (++rightIdx == rightOrders.length) {
// Update the left batched fill results
batchMatchedFillResults.left[leftIdx] = leftFillResults;
break;
} else {
rightOrder = rightOrders[rightIdx];
@@ -377,12 +395,6 @@ contract MixinMatchOrders is
}
}
// Update the lengths of the fill results for batchMatchResults
assembly {
mstore(mload(batchMatchedFillResults), matchCount)
mstore(mload(add(batchMatchedFillResults, 32)), matchCount)
}
// Return the fill results from the batch match
return batchMatchedFillResults;
}

View File

@@ -3305,6 +3305,7 @@ describe('matchOrders', () => {
leftMakerAssetSoldByLeftMakerAmount: Web3Wrapper.toBaseUnitAmount(2, 0),
leftMakerFeeAssetPaidByLeftMakerAmount: Web3Wrapper.toBaseUnitAmount(100, 16), // 100%
// Right Maker
leftMakerAssetBoughtByRightMakerAmount: Web3Wrapper.toBaseUnitAmount(2, 0),
rightMakerAssetSoldByRightMakerAmount: Web3Wrapper.toBaseUnitAmount(1, 0),
rightMakerFeeAssetPaidByRightMakerAmount: Web3Wrapper.toBaseUnitAmount(50, 16), // 50%
// Taker
@@ -3316,6 +3317,7 @@ describe('matchOrders', () => {
leftMakerAssetSoldByLeftMakerAmount: Web3Wrapper.toBaseUnitAmount(2, 0),
leftMakerFeeAssetPaidByLeftMakerAmount: Web3Wrapper.toBaseUnitAmount(100, 16), // 50%
// Right Maker
leftMakerAssetBoughtByRightMakerAmount: Web3Wrapper.toBaseUnitAmount(2, 0),
rightMakerAssetSoldByRightMakerAmount: Web3Wrapper.toBaseUnitAmount(1, 0),
rightMakerFeeAssetPaidByRightMakerAmount: Web3Wrapper.toBaseUnitAmount(50, 16), // 50%
// Taker

View File

@@ -3,6 +3,7 @@ import {
BatchMatchedFillResults,
chaiSetup,
ERC1155HoldingsByOwner,
FillResults,
MatchedFillResults,
OrderStatus,
} from '@0x/contracts-test-utils';
@@ -65,6 +66,8 @@ export interface MatchResults {
export interface BatchMatchResults {
matches: MatchResults[];
filledAmounts: Array<[SignedOrder, BigNumber, string]>;
leftFilledResults: FillEventArgs[];
rightFilledResults: FillEventArgs[];
}
export interface ERC1155Holdings {
@@ -418,17 +421,23 @@ function simulateBatchMatchOrders(
transferAmounts: Array<Partial<MatchTransferAmounts>>,
): BatchMatchResults {
// Initialize variables
let lastLeftIdx = 0;
let lastRightIdx = 0;
let leftIdx = 0;
let rightIdx = 0;
let lastLeftIdx = -1;
let lastRightIdx = -1;
let matchedOrders: MatchedOrders;
const batchMatchResults: BatchMatchResults = {
matches: [],
filledAmounts: [],
leftFilledResults: [],
rightFilledResults: [],
};
// Loop over all of the matched pairs from the round
for (let i = 0; i < matchPairs.length; i++) {
const leftIdx = matchPairs[i][0];
const rightIdx = matchPairs[i][1];
leftIdx = matchPairs[i][0];
rightIdx = matchPairs[i][1];
// Construct a matched order out of the current left and right orders
matchedOrders = {
leftOrder: orders.leftOrders[leftIdx],
@@ -436,6 +445,7 @@ function simulateBatchMatchOrders(
leftOrderTakerAssetFilledAmount: orders.leftOrdersTakerAssetFilledAmounts[leftIdx],
rightOrderTakerAssetFilledAmount: orders.rightOrdersTakerAssetFilledAmounts[rightIdx],
};
// If there has been a match recorded and one or both of the side indices have not changed,
// replace the side's taker asset filled amount
if (batchMatchResults.matches.length > 0) {
@@ -462,8 +472,7 @@ function simulateBatchMatchOrders(
]);
}
}
lastLeftIdx = leftIdx;
lastRightIdx = rightIdx;
// Add the latest match to the batch match results
batchMatchResults.matches.push(
simulateMatchOrders(
@@ -473,7 +482,31 @@ function simulateBatchMatchOrders(
toFullMatchTransferAmounts(transferAmounts[i]),
),
);
// Update the left and right fill results
if (lastLeftIdx === leftIdx) {
addFillResults(batchMatchResults.leftFilledResults[leftIdx], getLastMatch(batchMatchResults).fills[0]);
} else {
batchMatchResults.leftFilledResults.push({ ...getLastMatch(batchMatchResults).fills[0] });
}
if (lastRightIdx === rightIdx) {
addFillResults(batchMatchResults.rightFilledResults[rightIdx], getLastMatch(batchMatchResults).fills[1]);
} else {
batchMatchResults.rightFilledResults.push({ ...getLastMatch(batchMatchResults).fills[1] });
}
lastLeftIdx = leftIdx;
lastRightIdx = rightIdx;
}
for (let i = leftIdx + 1; i < orders.leftOrders.length; i++) {
batchMatchResults.leftFilledResults.push(emptyFillEventArgs());
}
for (let i = rightIdx + 1; i < orders.rightOrders.length; i++) {
batchMatchResults.rightFilledResults.push(emptyFillEventArgs());
}
// The two orders indexed by lastLeftIdx and lastRightIdx were potentially
// filled; however, the TakerAssetFilledAmounts that pertain to these orders
// will not have been added to batchMatchResults, so we need to write them
@@ -488,6 +521,7 @@ function simulateBatchMatchOrders(
getLastMatch(batchMatchResults).orders.rightOrderTakerAssetFilledAmount || ZERO,
'right',
]);
// Return the batch match results
return batchMatchResults;
}
@@ -592,6 +626,7 @@ function simulateMatchOrders(
orders.rightOrder.takerFeeAssetData,
matchResults,
);
return matchResults;
}
@@ -1018,6 +1053,23 @@ function getUpdatedBalances(batchMatchResults: BatchMatchResults, initialTokenBa
.reduce((totalBalances, balances) => aggregateBalances(totalBalances, balances, initialTokenBalances));
}
/**
* Add a new fill results object to a total fill results object destructively.
* @param total The total fill results that should be updated.
* @param fill The new fill results that should be used to accumulate.
*/
function addFillResults(total: FillEventArgs, fill: FillEventArgs): void {
// Ensure that the total and fill are compatibe fill events
expect(total.orderHash).to.be.eq(fill.orderHash);
expect(total.makerAddress).to.be.eq(fill.makerAddress);
expect(total.takerAddress).to.be.eq(fill.takerAddress);
// Add the fill results together
total.makerAssetFilledAmount = total.makerAssetFilledAmount.plus(fill.makerAssetFilledAmount);
total.takerAssetFilledAmount = total.takerAssetFilledAmount.plus(fill.takerAssetFilledAmount);
total.makerFeePaid = total.makerFeePaid.plus(fill.makerFeePaid);
total.takerFeePaid = total.takerFeePaid.plus(fill.takerFeePaid);
}
/**
* Takes a `totalBalances`, a `balances`, and an `initialBalances`, subtracts the `initialBalances
* from the `balances`, and then adds the result to `totalBalances`.
@@ -1069,23 +1121,7 @@ function convertToBatchMatchResults(results: BatchMatchResults): BatchMatchedFil
profitInLeftMakerAsset: ZERO,
profitInRightMakerAsset: ZERO,
};
// Create the batchMatchedFillResults by aggreagating the data from the simulations matches
for (const match of results.matches) {
expect(match.fills.length).to.be.eq(2);
// Include the matches results in the left fills of the batch match results.
batchMatchedFillResults.left.push({
makerAssetFilledAmount: match.fills[0].makerAssetFilledAmount,
takerAssetFilledAmount: match.fills[0].takerAssetFilledAmount,
makerFeePaid: match.fills[0].makerFeePaid,
takerFeePaid: match.fills[0].takerFeePaid,
});
// Include the matches results in the right fills of the batch match results.
batchMatchedFillResults.right.push({
makerAssetFilledAmount: match.fills[1].makerAssetFilledAmount,
takerAssetFilledAmount: match.fills[1].takerAssetFilledAmount,
makerFeePaid: match.fills[1].makerFeePaid,
takerFeePaid: match.fills[1].takerFeePaid,
});
const leftSpread = match.fills[0].makerAssetFilledAmount.minus(match.fills[1].takerAssetFilledAmount);
// If the left maker spread is positive for match, update the profitInLeftMakerAsset
if (leftSpread.isGreaterThan(ZERO)) {
@@ -1101,6 +1137,12 @@ function convertToBatchMatchResults(results: BatchMatchResults): BatchMatchedFil
);
}
}
for (const fill of results.leftFilledResults) {
batchMatchedFillResults.left.push(convertToFillResults(fill));
}
for (const fill of results.rightFilledResults) {
batchMatchedFillResults.right.push(convertToFillResults(fill));
}
return batchMatchedFillResults;
}
@@ -1141,4 +1183,36 @@ function convertToMatchResults(result: MatchResults): MatchedFillResults {
};
return matchedFillResults;
}
/**
* Converts a fill event args object to the associated FillResults object.
* @param result The result to be converted to a FillResults object.
* @return The converted value.
*/
function convertToFillResults(result: FillEventArgs): FillResults {
const fillResults: FillResults = {
makerAssetFilledAmount: result.makerAssetFilledAmount,
takerAssetFilledAmount: result.takerAssetFilledAmount,
makerFeePaid: result.makerFeePaid,
takerFeePaid: result.takerFeePaid,
};
return fillResults;
}
/**
* Creates an empty FillEventArgs object.
* @return The empty FillEventArgs object.
*/
function emptyFillEventArgs(): FillEventArgs {
const empty: FillEventArgs = {
orderHash: '',
makerAddress: '',
takerAddress: '',
makerAssetFilledAmount: new BigNumber(0),
takerAssetFilledAmount: new BigNumber(0),
makerFeePaid: new BigNumber(0),
takerFeePaid: new BigNumber(0),
};
return empty;
}
// tslint:disable-line:max-file-line-count