crowetic/protocol
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

@0x/contracts-exchange: Avoid redundant trasfer in fillOrder() when maker/taker is the same as feeRecipient and the assets match.

Browse Source 
`@0x/contracts-exchange`: Swap fill order in `fillOrder()` from maker -> taker to taker -> maker first
This commit is contained in:
Lawrence Forman
2019-05-24 13:29:46 -04:00
committed by Amir Bandeali
parent 9cc8933eec
commit cd08c3e8fa
5 changed files with 131 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
contracts/exchange/CHANGELOG.json
View File

@@ -37,6 +37,14 @@
{
"note": "Incorporate Multi-asset and ERC1155 tests into `fillOrder` and `matchOrders` tests",
"pr": 1819
},
{
"note": "Swap fill order from maker -> taker to taker -> maker",
"pr": 1819
},
{
"note": "Avoid redundant transfer in `fillOrder` when maker/taker is the same as feeRecipient and assets are the same",
"pr": 1819
}
]
},

90
contracts/exchange/contracts/src/MixinExchangeCore.sol
View File

@@ -611,33 +611,67 @@ contract MixinExchangeCore is
)
private
{
_dispatchTransferFrom(
orderHash,
order.makerAssetData,
order.makerAddress,
takerAddress,
fillResults.makerAssetFilledAmount
);
_dispatchTransferFrom(
orderHash,
order.takerAssetData,
takerAddress,
order.makerAddress,
fillResults.takerAssetFilledAmount
);
_dispatchTransferFrom(
orderHash,
order.makerFeeAssetData,
order.makerAddress,
order.feeRecipientAddress,
fillResults.makerFeePaid
);
_dispatchTransferFrom(
orderHash,
order.takerFeeAssetData,
takerAddress,
order.feeRecipientAddress,
fillResults.takerFeePaid
);
if (
order.makerAddress == order.feeRecipientAddress &&
order.makerFeeAssetData.equals(order.takerAssetData)
) {
// Maker is fee recipient and the maker fee asset is the same as the taker asset.
// We can transfer the taker asset and maker fees in one go.
_dispatchTransferFrom(
orderHash,
order.takerAssetData,
takerAddress,
order.makerAddress,
_safeAdd(fillResults.takerAssetFilledAmount, fillResults.makerFeePaid)
);
} else {
// Transfer taker -> maker
_dispatchTransferFrom(
orderHash,
order.takerAssetData,
takerAddress,
order.makerAddress,
fillResults.takerAssetFilledAmount
);
// Transfer maker fee -> feeRecipient
_dispatchTransferFrom(
orderHash,
order.makerFeeAssetData,
order.makerAddress,
order.feeRecipientAddress,
fillResults.makerFeePaid
);
}
if (
takerAddress == order.feeRecipientAddress &&
order.takerFeeAssetData.equals(order.makerAssetData)
) {
// Taker is fee recipient and the taker fee asset is the same as the maker asset.
// We can transfer the maker asset and taker fees in one go.
_dispatchTransferFrom(
orderHash,
order.makerAssetData,
order.makerAddress,
takerAddress,
_safeAdd(fillResults.makerAssetFilledAmount, fillResults.takerFeePaid)
);
} else {
// Transfer maker -> taker
_dispatchTransferFrom(
orderHash,
order.makerAssetData,
order.makerAddress,
takerAddress,
fillResults.makerAssetFilledAmount
);
// Transfer taker fee -> feeRecipient
_dispatchTransferFrom(
orderHash,
order.takerFeeAssetData,
takerAddress,
order.feeRecipientAddress,
fillResults.takerFeePaid
);
}
}
}

2
contracts/exchange/test/fill_order.ts
View File

@@ -51,7 +51,7 @@ const defaultFillScenario = {
},
};
describe('FillOrder Tests', () => {
describe.only('FillOrder Tests', () => {
let fillOrderCombinatorialUtils: FillOrderCombinatorialUtils;
before(async () => {

9
contracts/exchange/test/match_orders.ts
View File

@@ -11,14 +11,7 @@ import {
import { ERC1155Contract as ERC1155TokenContract, Erc1155Wrapper as ERC1155Wrapper } from '@0x/contracts-erc1155';
import { DummyERC20TokenContract } from '@0x/contracts-erc20';
import { DummyERC721TokenContract } from '@0x/contracts-erc721';
import {
chaiSetup,
constants,
OrderFactory,
provider,
txDefaults,
web3Wrapper,
} from '@0x/contracts-test-utils';
import { chaiSetup, constants, OrderFactory, provider, txDefaults, web3Wrapper } from '@0x/contracts-test-utils';
import { BlockchainLifecycle } from '@0x/dev-utils';
import { assetDataUtils, ExchangeRevertErrors, orderHashUtils } from '@0x/order-utils';
import { OrderStatus, RevertReason } from '@0x/types';

94
contracts/exchange/test/utils/fill_order_simulator.ts
View File

@@ -71,45 +71,69 @@ export class FillOrderSimulator {
);
try {
// Maker -> Taker
await this._transferSimulator.transferFromAsync(
order.makerAssetData,
order.makerAddress,
takerAddress,
makerAssetFillAmount,
TradeSide.Maker,
TransferType.Trade,
);
// Taker -> Maker
await this._transferSimulator.transferFromAsync(
order.takerAssetData,
takerAddress,
order.makerAddress,
finalTakerAssetFillAmount,
TradeSide.Taker,
TransferType.Trade,
);
// Maker fee -> fee recipient
if (makerFeePaid.isGreaterThan(0)) {
if (order.makerAddress === order.feeRecipientAddress && order.takerAssetData === order.makerFeeAssetData) {
// Transfer combined taker assets and maker fees.
await this._transferSimulator.transferFromAsync(
order.makerFeeAssetData,
order.makerAddress,
order.feeRecipientAddress,
makerFeePaid,
TradeSide.Maker,
TransferType.Fee,
);
}
// Taker fee -> fee recipient
if (takerFeePaid.isGreaterThan(0)) {
await this._transferSimulator.transferFromAsync(
order.takerFeeAssetData,
order.takerAssetData,
takerAddress,
order.feeRecipientAddress,
takerFeePaid,
order.makerAddress,
finalTakerAssetFillAmount.plus(makerFeePaid),
TradeSide.Taker,
TransferType.Fee,
TransferType.Trade,
);
} else {
// Taker -> Maker
await this._transferSimulator.transferFromAsync(
order.takerAssetData,
takerAddress,
order.makerAddress,
finalTakerAssetFillAmount,
TradeSide.Taker,
TransferType.Trade,
);
// Maker fee -> fee recipient
if (makerFeePaid.isGreaterThan(0)) {
await this._transferSimulator.transferFromAsync(
order.makerFeeAssetData,
order.makerAddress,
order.feeRecipientAddress,
makerFeePaid,
TradeSide.Maker,
TransferType.Fee,
);
}
}
if (takerAddress === order.feeRecipientAddress && order.makerAssetData === order.takerFeeAssetData) {
// Transfer combined maker assets and taker fees.
await this._transferSimulator.transferFromAsync(
order.makerAssetData,
order.makerAddress,
takerAddress,
makerAssetFillAmount.plus(takerFeePaid),
TradeSide.Maker,
TransferType.Trade,
);
} else {
// Maker -> Taker
await this._transferSimulator.transferFromAsync(
order.makerAssetData,
order.makerAddress,
takerAddress,
makerAssetFillAmount,
TradeSide.Maker,
TransferType.Trade,
);
// Taker fee -> fee recipient
if (takerFeePaid.isGreaterThan(0)) {
await this._transferSimulator.transferFromAsync(
order.takerFeeAssetData,
takerAddress,
order.feeRecipientAddress,
takerFeePaid,
TradeSide.Taker,
TransferType.Fee,
);
}
}
} catch (err) {
throw new Error(FillOrderError.TransferFailed);