OrderValidationUtils: don't parse fee asset data when there are no fees specified (#2218)

* exhibit bug: empty fee asset data induces revert

* fix bug: don't parse fee asset data unless fee>0

* Update DevUtils Kovan contract address
This commit is contained in:
F. Eugene Aumson 2019-09-30 17:53:21 -04:00 committed by GitHub
parent d21f978def
commit c051e11a49
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 4 deletions

View File

@ -86,9 +86,6 @@ contract OrderValidationUtils is
takerAssetAmount
);
} else {
// Get the transferable amount of the `makerFeeAsset`
uint256 transferableMakerFeeAssetAmount = getTransferableAssetAmount(makerAddress, order.makerFeeAssetData);
// If `makerFee` is 0, the % that can be filled is (transferableMakerAssetAmount / makerAssetAmount)
if (makerFee == 0) {
transferableTakerAssetAmount = LibMath.getPartialAmountFloor(
@ -100,6 +97,9 @@ contract OrderValidationUtils is
// If `makerAsset` does not equal `makerFeeAsset`, the % that can be filled is the lower of
// (transferableMakerAssetAmount / makerAssetAmount) and (transferableMakerAssetFeeAmount / makerFee)
} else {
// Get the transferable amount of the `makerFeeAsset`
uint256 transferableMakerFeeAssetAmount = getTransferableAssetAmount(makerAddress, order.makerFeeAssetData);
uint256 transferableMakerToTakerAmount = LibMath.getPartialAmountFloor(
transferableMakerAssetAmount,
order.makerAssetAmount,

View File

@ -410,6 +410,28 @@ describe('OrderValidationUtils/OrderTransferSimulatorUtils', () => {
signedOrder.takerAssetAmount.minus(takerAssetFillAmount),
);
});
it('should return correct info even when there are no fees specified', async () => {
signedOrder = await orderFactory.newSignedOrderAsync({
makerFee: new BigNumber(0),
takerFee: new BigNumber(0),
makerFeeAssetData: '0x',
takerFeeAssetData: '0x',
});
await erc20Token.setBalance.awaitTransactionSuccessAsync(makerAddress, signedOrder.makerAssetAmount);
await erc20Token.approve.awaitTransactionSuccessAsync(erc20Proxy.address, signedOrder.makerAssetAmount, {
from: makerAddress,
});
const [
orderInfo,
fillableTakerAssetAmount,
isValidSignature,
] = await devUtils.getOrderRelevantState.callAsync(signedOrder, signedOrder.signature);
expect(orderInfo.orderHash).to.equal(orderHashUtils.getOrderHashHex(signedOrder));
expect(orderInfo.orderStatus).to.equal(OrderStatus.Fillable);
expect(orderInfo.orderTakerAssetFilledAmount).to.bignumber.equal(constants.ZERO_AMOUNT);
expect(fillableTakerAssetAmount).to.bignumber.equal(signedOrder.takerAssetAmount);
expect(isValidSignature).to.equal(true);
});
});
describe('getOrderRelevantStates', async () => {
it('should return the correct information for multiple orders', async () => {

View File

@ -95,7 +95,7 @@ const networkToAddresses: { [networkId: number]: ContractAddresses } = {
multiAssetProxy: '0xf6313a772c222f51c28f2304c0703b8cf5428fd8',
staticCallProxy: '0x48e94bdb9033640d45ea7c721e25f380f8bffa43',
erc1155Proxy: '0x64517fa2b480ba3678a2a3c0cf08ef7fd4fad36f',
devUtils: '0xa5f985e8df08e17e88001dd85724707bcb80e413',
devUtils: '0xb1863ac46ae23ec55d6eeb8ecc8815655ee638a8',
},
// NetworkId 50 represents our Ganache snapshot generated from migrations.
50: {