minor fixes

This commit is contained in:
Michael Zhu 2019-07-22 16:46:07 -07:00
parent 688209e272
commit a1aee7111a
4 changed files with 20 additions and 17 deletions

View File

@ -181,24 +181,17 @@ contract MixinExchangeWrapper is
) )
{ {
uint256 ordersLength = orders.length; uint256 ordersLength = orders.length;
for (uint256 i = 0; i != ordersLength; i++) { for (uint256 i = 0; i != ordersLength; i++) {
require( require(
orders[i].makerAssetData.equals(orders[0].makerAssetData), orders[i].makerAssetData.equals(orders[0].makerAssetData),
"MAKER_ASSET_MISMATCH" "MAKER_ASSET_MISMATCH"
); );
// Calculate the remaining amount of makerAsset to buy // Calculate the remaining amount of takerAsset to sell
uint256 remainingMakerAssetFillAmount = _safeSub(makerAssetFillAmount, makerAssetAcquiredAmount);
// Convert the remaining amount of makerAsset to buy into remaining amount
// of takerAsset to sell, assuming entire amount can be sold in the current order.
// We round up because the exchange rate computed by fillOrder rounds in favor
// of the Maker. In this case we want to overestimate the amount of takerAsset.
uint256 remainingTakerAssetFillAmount = _getPartialAmountCeil( uint256 remainingTakerAssetFillAmount = _getPartialAmountCeil(
orders[i].takerAssetAmount, orders[i].takerAssetAmount,
orders[i].makerAssetAmount, orders[i].makerAssetAmount,
remainingMakerAssetFillAmount _safeSub(makerAssetFillAmount, makerAssetAcquiredAmount)
); );
// Attempt to sell the remaining amount of takerAsset // Attempt to sell the remaining amount of takerAsset
@ -207,7 +200,6 @@ contract MixinExchangeWrapper is
remainingTakerAssetFillAmount, remainingTakerAssetFillAmount,
signatures[i] signatures[i]
); );
// Update amounts filled and fees paid by maker and taker // Update amounts filled and fees paid by maker and taker
if (orders[i].makerAssetData.equals(orders[i].takerFeeAssetData)) { if (orders[i].makerAssetData.equals(orders[i].takerFeeAssetData)) {
makerAssetAcquiredAmount = _safeAdd( makerAssetAcquiredAmount = _safeAdd(
@ -221,23 +213,23 @@ contract MixinExchangeWrapper is
} else { } else {
makerAssetAcquiredAmount = _safeAdd( makerAssetAcquiredAmount = _safeAdd(
makerAssetAcquiredAmount, makerAssetAcquiredAmount,
_safeSub(singleFillResults.makerAssetFilledAmount, singleFillResults.takerFeePaid) singleFillResults.makerAssetFilledAmount
); );
wethSpentAmount = _safeAdd( wethSpentAmount = _safeAdd(
wethSpentAmount, wethSpentAmount,
singleFillResults.takerAssetFilledAmount _safeAdd(singleFillResults.takerAssetFilledAmount, singleFillResults.takerFeePaid)
); );
} }
_addFillResults(totalFillResults, singleFillResults); _addFillResults(totalFillResults, singleFillResults);
// Stop execution if the entire amount of makerAsset has been bought // Stop execution if the entire amount of makerAsset has been bought
if (makerAssetAcquiredAmount >= makerAssetFillAmount) { if (totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount) {
break; break;
} }
} }
require( require(
makerAssetAcquiredAmount >= makerAssetFillAmount, totalFillResults.makerAssetFilledAmount >= makerAssetFillAmount,
"COMPLETE_FILL_FAILED" "COMPLETE_FILL_FAILED"
); );
return (totalFillResults, wethSpentAmount, makerAssetAcquiredAmount); return (totalFillResults, wethSpentAmount, makerAssetAcquiredAmount);

View File

@ -31,4 +31,5 @@ contract LibForwarderErrors {
string constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY = "DEFAULT_FUNCTION_WETH_CONTRACT_ONLY"; // Fallback function may only be used for WETH withdrawals. string constant DEFAULT_FUNCTION_WETH_CONTRACT_ONLY = "DEFAULT_FUNCTION_WETH_CONTRACT_ONLY"; // Fallback function may only be used for WETH withdrawals.
string constant INVALID_MSG_VALUE = "INVALID_MSG_VALUE"; // msg.value must be greater than 0. string constant INVALID_MSG_VALUE = "INVALID_MSG_VALUE"; // msg.value must be greater than 0.
string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Amount must equal 1. string constant INVALID_AMOUNT = "INVALID_AMOUNT"; // Amount must equal 1.
string constant MAKER_ASSET_MISMATCH = "MAKER_ASSET_MISMATCH"; // Maker asset must be the same across all orders in a given input
} }

View File

@ -135,11 +135,13 @@ export class ForwarderTestFactory {
expectedResults.takerAssetFillAmount, expectedResults.takerAssetFillAmount,
forwarderFeeOptions.baseFeePercentage, forwarderFeeOptions.baseFeePercentage,
); );
const feePercentage = ForwarderTestFactory.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, forwarderFeeOptions.baseFeePercentage);
const ethValue = expectedResults.takerAssetFillAmount const ethValue = expectedResults.takerAssetFillAmount
.plus(expectedResults.wethFees) .plus(expectedResults.wethFees)
.plus(expectedResults.maxOversoldWeth)
.plus(ethSpentOnForwarderFee) .plus(ethSpentOnForwarderFee)
.plus(ethValueAdjustment); .plus(ethValueAdjustment);
const feePercentage = ForwarderTestFactory.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, forwarderFeeOptions.baseFeePercentage);
if (ethValueAdjustment.isNegative()) { if (ethValueAdjustment.isNegative()) {
return expectTransactionFailedAsync( return expectTransactionFailedAsync(
@ -190,10 +192,12 @@ export class ForwarderTestFactory {
expectedResults.takerAssetFillAmount, expectedResults.takerAssetFillAmount,
forwarderFeeOptions.baseFeePercentage, forwarderFeeOptions.baseFeePercentage,
); );
const feePercentage = ForwarderTestFactory.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, forwarderFeeOptions.baseFeePercentage);
const ethValue = expectedResults.takerAssetFillAmount const ethValue = expectedResults.takerAssetFillAmount
.plus(expectedResults.wethFees) .plus(expectedResults.wethFees)
.plus(expectedResults.maxOversoldWeth)
.plus(ethSpentOnForwarderFee); .plus(ethSpentOnForwarderFee);
const feePercentage = ForwarderTestFactory.getPercentageOfValue(constants.PERCENTAGE_DENOMINATOR, forwarderFeeOptions.baseFeePercentage);
const tx = await this._forwarderWrapper.marketSellOrdersWithEthAsync( const tx = await this._forwarderWrapper.marketSellOrdersWithEthAsync(
orders, orders,
@ -233,7 +237,11 @@ export class ForwarderTestFactory {
const fowarderFeeRecipientEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(this._forwarderFeeRecipientAddress); const fowarderFeeRecipientEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(this._forwarderFeeRecipientAddress);
const newBalances = await this._erc20Wrapper.getBalancesAsync(); const newBalances = await this._erc20Wrapper.getBalancesAsync();
expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); expectBalanceWithin(
takerEthBalanceAfter,
takerEthBalanceBefore.minus(totalEthSpent).minus(expectedResults.maxOversoldWeth),
takerEthBalanceBefore.minus(totalEthSpent),
);
if (ethSpentOnForwarderFee.gt(0)) { if (ethSpentOnForwarderFee.gt(0)) {
expect(fowarderFeeRecipientEthBalanceAfter).to.be.bignumber.equal( expect(fowarderFeeRecipientEthBalanceAfter).to.be.bignumber.equal(
forwarderFeeRecipientEthBalanceBefore.plus(ethSpentOnForwarderFee), forwarderFeeRecipientEthBalanceBefore.plus(ethSpentOnForwarderFee),

View File

@ -340,6 +340,8 @@ export enum RevertReason {
TargetNotEven = 'TARGET_NOT_EVEN', TargetNotEven = 'TARGET_NOT_EVEN',
UnexpectedStaticCallResult = 'UNEXPECTED_STATIC_CALL_RESULT', UnexpectedStaticCallResult = 'UNEXPECTED_STATIC_CALL_RESULT',
TransfersSuccessful = 'TRANSFERS_SUCCESSFUL', TransfersSuccessful = 'TRANSFERS_SUCCESSFUL',
// Forwader
MakerAssetMismatch = 'MAKER_ASSET_MISMATCH',
} }
export enum StatusCodes { export enum StatusCodes {