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

remove _noTakerFee (i.e. disallow StaticCall takerFeeAssetData)

Browse Source
This commit is contained in:
Michael Zhu
2020-01-05 22:38:16 -05:00
committed by Amir
parent 295811ed5a
commit b70cb726c5
5 changed files with 6 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files

33
contracts/exchange-forwarder/contracts/src/MixinExchangeWrapper.sol
View File

@@ -88,10 +88,9 @@ contract MixinExchangeWrapper is
uint256 makerAssetAcquiredAmount
)
{
bool noTakerFee = _noTakerFee(order.takerFee, order.takerFeeAssetData);
// No taker fee or percentage fee
if (
noTakerFee ||
order.takerFee == 0 ||
_areUnderlyingAssetsEqual(order.takerFeeAssetData, order.makerAssetData)
) {
// Attempt to sell the remaining amount of WETH
@@ -106,7 +105,7 @@ contract MixinExchangeWrapper is
// Subtract fee from makerAssetFilledAmount for the net amount acquired.
makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount
.safeSub(noTakerFee ? 0 : singleFillResults.takerFeePaid);
.safeSub(singleFillResults.takerFeePaid);
// WETH fee
} else if (_areUnderlyingAssetsEqual(order.takerFeeAssetData, order.takerAssetData)) {
@@ -231,10 +230,9 @@ contract MixinExchangeWrapper is
uint256 makerAssetAcquiredAmount
)
{
bool noTakerFee = _noTakerFee(order.takerFee, order.takerFeeAssetData);
// No taker fee or WETH fee
if (
noTakerFee ||
order.takerFee == 0 ||
_areUnderlyingAssetsEqual(order.takerFeeAssetData, order.takerAssetData)
) {
// Calculate the remaining amount of takerAsset to sell
@@ -253,7 +251,7 @@ contract MixinExchangeWrapper is
// WETH is also spent on the protocol and taker fees, so we add it here.
wethSpentAmount = singleFillResults.takerAssetFilledAmount
.safeAdd(noTakerFee ? 0 : singleFillResults.takerFeePaid)
.safeAdd(singleFillResults.takerFeePaid)
.safeAdd(singleFillResults.protocolFeePaid);
makerAssetAcquiredAmount = singleFillResults.makerAssetFilledAmount;
@@ -496,27 +494,4 @@ contract MixinExchangeWrapper is
{
return order.makerFeeAssetData.length > 3 && order.makerFeeAssetData.readBytes4(0) == EXCHANGE_V2_ORDER_ID;
}
/// @dev Checks whether one asset is effectively equal to another asset.
/// This is the case if they have the same ERC20Proxy/ERC20BridgeProxy asset data, or if
/// one is the ERC20Bridge equivalent of the other.
/// @param takerFee Byte array encoded for the takerFee asset proxy.
/// @param takerFeeAssetData Byte array encoded for the maker asset proxy.
/// @return Whether or not the underlying assets are equal.
function _noTakerFee(
uint256 takerFee,
bytes memory takerFeeAssetData
)
internal
pure
returns (bool)
{
return (
takerFee == 0 ||
(
takerFeeAssetData.length > 3 &&
takerFeeAssetData.readBytes4(0) == IAssetData(address(0)).StaticCall.selector
)
);
}
}

13
contracts/exchange-forwarder/contracts/test/TestForwarder.sol
View File

@@ -61,17 +61,4 @@ contract TestForwarder is
amount
);
}
function noTakerFee(
uint256 takerFee,
bytes memory takerFeeAssetData
)
public
returns (bool)
{
return _noTakerFee(
takerFee,
takerFeeAssetData
);
}
}

23
contracts/exchange-forwarder/test/asset_test.ts
View File

@@ -292,27 +292,4 @@ blockchainTests.resets('Supported asset type unit tests', env => {
return expect(tx).to.revertWith(expectedError);
});
});
describe('_noTakerFee', () => {
it('returns true if takerFee == 0 and takerFeeAssetData != StaticCall', async () => {
const result = await forwarder.noTakerFee(constants.ZERO_AMOUNT, erc20AssetData).callAsync();
expect(result).to.be.true();
});
it('returns false if takerFee != 0 and takerFeeAssetData != StaticCall', async () => {
const result = await forwarder
.noTakerFee(getRandomInteger(1, constants.MAX_UINT256), erc20AssetData)
.callAsync();
expect(result).to.be.false();
});
it('returns true if takerFee == 0 and takerFeeAssetData == StaticCall', async () => {
const result = await forwarder.noTakerFee(constants.ZERO_AMOUNT, staticCallAssetData).callAsync();
expect(result).to.be.true();
});
it('returns true if takerFee != 0 and takerFeeAssetData == StaticCall', async () => {
const result = await forwarder
.noTakerFee(getRandomInteger(1, constants.MAX_UINT256), staticCallAssetData)
.callAsync();
expect(result).to.be.true();
});
});
});

2
contracts/integrations/test/forwarder/forwarder_test.ts
View File

@@ -319,7 +319,7 @@ blockchainTests('Forwarder integration tests', env => {
const fillableOrder = await maker.signOrderAsync();
await testFactory.marketSellTestAsync([cancelledOrder, fillableOrder], 1.5, { noopOrders: [0] });
});
for (const orderAssetData of ['makerAssetData', 'makerFeeAssetData', 'takerFeeAssetData']) {
for (const orderAssetData of ['makerAssetData', 'makerFeeAssetData']) {
it(`should fill an order with StaticCall ${orderAssetData} if the StaticCall succeeds`, async () => {
const staticCallOrder = await maker.signOrderAsync({
[orderAssetData]: staticCallSuccessAssetData,

5
contracts/integrations/test/forwarder/forwarder_test_factory.ts
View File

@@ -246,10 +246,7 @@ export class ForwarderTestFactory {
const makerFeeFilled = takerAssetFilled.times(order.makerFee).dividedToIntegerBy(order.takerAssetAmount);
makerFee = BigNumber.max(makerFee.minus(makerFeeFilled), 0);
const takerFeeFilled = takerAssetFilled.times(order.takerFee).dividedToIntegerBy(order.takerAssetAmount);
takerFee =
hexUtils.slice(order.takerFeeAssetData, 0, 4) === AssetProxyId.StaticCall
? constants.ZERO_AMOUNT
: BigNumber.max(takerFee.minus(takerFeeFilled), 0);
takerFee = BigNumber.max(takerFee.minus(takerFeeFilled), 0);
makerAssetAmount = makerAssetAmount.plus(bridgeExcessBuyAmount);
let wethSpentAmount = takerAssetAmount.plus(DeploymentManager.protocolFee);