@0x:contracts-exchange Addressed review feedback

This commit is contained in:
Alex Towle 2019-08-29 00:31:30 -07:00
parent 75e6c45285
commit dba0d8469d
34 changed files with 510 additions and 291 deletions

View File

@ -52,13 +52,18 @@ library LibFillResults {
/// @dev Calculates amounts filled and fees paid by maker and taker.
/// @param order to be filled.
/// @param takerAssetFilledAmount Amount of takerAsset that will be filled.
/// @param protocolFeeMultiplier The current protocol fee of the exchange contract.
/// @param gasPrice The gasprice of the transaction. This is provided so that the function call can continue
/// to be pure rather than view.
/// @return fillResults Amounts filled and fees paid by maker and taker.
function calculateFillResults(
LibOrder.Order memory order,
uint256 takerAssetFilledAmount
uint256 takerAssetFilledAmount,
uint256 protocolFeeMultiplier,
uint256 gasPrice
)
internal
view
pure
returns (FillResults memory fillResults)
{
// Compute proportional transfer amounts
@ -79,6 +84,9 @@ library LibFillResults {
order.takerFee
);
// Compute the protocol fee that should be paid for a single fill.
fillResults.protocolFeePaid = gasPrice.safeMul(protocolFeeMultiplier);
return fillResults;
}
@ -90,6 +98,9 @@ library LibFillResults {
/// @param rightOrder Second order to match.
/// @param leftOrderTakerAssetFilledAmount Amount of left order already filled.
/// @param rightOrderTakerAssetFilledAmount Amount of right order already filled.
/// @param protocolFeeMultiplier The current protocol fee of the exchange contract.
/// @param gasPrice The gasprice of the transaction. This is provided so that the function call can continue
/// to be pure rather than view.
/// @param shouldMaximallyFillOrders A value that indicates whether or not this calculation should use
/// the maximal fill order matching strategy.
/// @param matchedFillResults Amounts to fill and fees to pay by maker and taker of matched orders.
@ -98,10 +109,12 @@ library LibFillResults {
LibOrder.Order memory rightOrder,
uint256 leftOrderTakerAssetFilledAmount,
uint256 rightOrderTakerAssetFilledAmount,
uint256 protocolFeeMultiplier,
uint256 gasPrice,
bool shouldMaximallyFillOrders
)
internal
view
pure
returns (MatchedFillResults memory matchedFillResults)
{
// Derive maker asset amounts for left & right orders, given store taker assert amounts
@ -163,6 +176,12 @@ library LibFillResults {
rightOrder.takerFee
);
// Compute the protocol fee that should be paid for a single fill. In this
// case this should be made the protocol fee for both the left and right orders.
uint256 protocolFee = gasPrice.safeMul(protocolFeeMultiplier);
matchedFillResults.left.protocolFeePaid = protocolFee;
matchedFillResults.right.protocolFeePaid = protocolFee;
// Return fill results
return matchedFillResults;
}

View File

@ -29,13 +29,20 @@ contract TestLibFillResults {
function calculateFillResults(
LibOrder.Order memory order,
uint256 takerAssetFilledAmount
uint256 takerAssetFilledAmount,
uint256 protocolFeeMultiplier,
uint256 gasPrice
)
public
view
pure
returns (LibFillResults.FillResults memory fillResults)
{
fillResults = LibFillResults.calculateFillResults(order, takerAssetFilledAmount);
fillResults = LibFillResults.calculateFillResults(
order,
takerAssetFilledAmount,
protocolFeeMultiplier,
gasPrice
);
return fillResults;
}
@ -44,10 +51,12 @@ contract TestLibFillResults {
LibOrder.Order memory rightOrder,
uint256 leftOrderTakerAssetFilledAmount,
uint256 rightOrderTakerAssetFilledAmount,
uint256 protocolFeeMultiplier,
uint256 gasPrice,
bool shouldMaximallyFillOrders
)
public
view
pure
returns (LibFillResults.MatchedFillResults memory matchedFillResults)
{
matchedFillResults = LibFillResults.calculateMatchedFillResults(
@ -55,6 +64,8 @@ contract TestLibFillResults {
rightOrder,
leftOrderTakerAssetFilledAmount,
rightOrderTakerAssetFilledAmount,
protocolFeeMultiplier,
gasPrice,
shouldMaximallyFillOrders
);
return matchedFillResults;

View File

@ -1,4 +1,3 @@
import { constants } from '@0x/contracts-test-utils';
import { ReferenceFunctions } from '@0x/contracts-utils';
import { LibMathRevertErrors } from '@0x/order-utils';
import { FillResults, OrderWithoutDomain } from '@0x/types';
@ -94,7 +93,12 @@ export function addFillResults(a: FillResults, b: FillResults): FillResults {
/**
* Calculates amounts filled and fees paid by maker and taker.
*/
export function calculateFillResults(order: OrderWithoutDomain, takerAssetFilledAmount: BigNumber): FillResults {
export function calculateFillResults(
order: OrderWithoutDomain,
takerAssetFilledAmount: BigNumber,
protocolFeeMultiplier: BigNumber,
gasPrice: BigNumber,
): FillResults {
const makerAssetFilledAmount = safeGetPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
@ -107,6 +111,6 @@ export function calculateFillResults(order: OrderWithoutDomain, takerAssetFilled
takerAssetFilledAmount,
makerFeePaid,
takerFeePaid,
protocolFeePaid: constants.ZERO_AMOUNT, // This field is not calculated in `calculateFillResults` as a gas optimization.
protocolFeePaid: safeMul(protocolFeeMultiplier, gasPrice),
};
}

View File

@ -93,6 +93,8 @@ blockchainTests('LibFillResults', env => {
return ReferenceFunctions.calculateFillResults(
makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount),
takerAssetFilledAmount,
takerAssetFilledAmount, // Using this so that the gas price is distinct from protocolFeeMultiplier
otherAmount,
);
}
@ -102,7 +104,12 @@ blockchainTests('LibFillResults', env => {
otherAmount: BigNumber,
): Promise<FillResults> {
const order = makeOrder(otherAmount, orderTakerAssetAmount, otherAmount, otherAmount);
return libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount);
return libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
takerAssetFilledAmount, // Using this so that the gas price is distinct from protocolFeeMultiplier
otherAmount,
);
}
testCombinatoriallyWithReferenceFunc(
@ -115,6 +122,9 @@ blockchainTests('LibFillResults', env => {
describe('explicit tests', () => {
const MAX_UINT256_ROOT = constants.MAX_UINT256_ROOT;
const DEFAULT_GAS_PRICE = new BigNumber(200000);
const DEFAULT_PROTOCOL_FEE_MULTIPLIER = new BigNumber(150000);
function makeOrder(details?: Partial<Order>): Order {
return _.assign({}, EMPTY_ORDER, details);
}
@ -127,8 +137,18 @@ blockchainTests('LibFillResults', env => {
takerFee: ONE_ETHER.times(0.0025),
});
const takerAssetFilledAmount = ONE_ETHER.dividedToIntegerBy(3);
const expected = ReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount);
const actual = await libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount);
const expected = ReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
);
const actual = await libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
);
expect(actual).to.deep.eq(expected);
});
@ -144,9 +164,14 @@ blockchainTests('LibFillResults', env => {
takerAssetFilledAmount,
order.makerAssetAmount,
);
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if computing `fillResults.makerFeePaid` overflows', async () => {
@ -167,9 +192,14 @@ blockchainTests('LibFillResults', env => {
makerAssetFilledAmount,
order.makerFee,
);
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if computing `fillResults.takerFeePaid` overflows', async () => {
@ -185,9 +215,14 @@ blockchainTests('LibFillResults', env => {
takerAssetFilledAmount,
order.takerFee,
);
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if `order.makerAssetAmount` is 0', async () => {
@ -197,9 +232,14 @@ blockchainTests('LibFillResults', env => {
});
const takerAssetFilledAmount = ONE_ETHER;
const expectedError = new LibMathRevertErrors.DivisionByZeroError();
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if `order.takerAssetAmount` is 0', async () => {
@ -209,9 +249,14 @@ blockchainTests('LibFillResults', env => {
});
const takerAssetFilledAmount = ONE_ETHER;
const expectedError = new LibMathRevertErrors.DivisionByZeroError();
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', async () => {
@ -225,9 +270,14 @@ blockchainTests('LibFillResults', env => {
order.takerAssetAmount,
order.makerAssetAmount,
);
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if there is a rounding error computing `makerFeePaid`', async () => {
@ -247,9 +297,91 @@ blockchainTests('LibFillResults', env => {
order.makerAssetAmount,
order.makerFee,
);
return expect(libsContract.calculateFillResults.callAsync(order, takerAssetFilledAmount)).to.revertWith(
expectedError,
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if there is a rounding error computing `takerFeePaid`', async () => {
const order = makeOrder({
makerAssetAmount: ONE_ETHER,
takerAssetAmount: ONE_ETHER,
takerFee: new BigNumber(100),
});
const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
const makerAssetFilledAmount = ReferenceFunctions.getPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
order.makerAssetAmount,
);
const expectedError = new LibMathRevertErrors.RoundingError(
makerAssetFilledAmount,
order.makerAssetAmount,
order.takerFee,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if computing `fillResults.protocolFeePaid` overflows', async () => {
const order = makeOrder({
makerAssetAmount: ONE_ETHER,
takerAssetAmount: ONE_ETHER.times(2),
makerFee: ONE_ETHER.times(0.0023),
takerFee: ONE_ETHER.times(0.0025),
});
const takerAssetFilledAmount = ONE_ETHER.dividedToIntegerBy(3);
const expectedError = new SafeMathRevertErrors.Uint256BinOpError(
SafeMathRevertErrors.BinOpErrorCodes.MultiplicationOverflow,
DEFAULT_GAS_PRICE,
MAX_UINT256,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
MAX_UINT256,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
it('reverts if there is a rounding error computing `makerFeePaid`', async () => {
const order = makeOrder({
makerAssetAmount: ONE_ETHER,
takerAssetAmount: ONE_ETHER,
makerFee: new BigNumber(100),
});
const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
const makerAssetFilledAmount = ReferenceFunctions.getPartialAmountFloor(
takerAssetFilledAmount,
order.takerAssetAmount,
order.makerAssetAmount,
);
const expectedError = new LibMathRevertErrors.RoundingError(
makerAssetFilledAmount,
order.makerAssetAmount,
order.makerFee,
);
return expect(
libsContract.calculateFillResults.callAsync(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.revertWith(expectedError);
});
});
});
@ -358,14 +490,14 @@ blockchainTests('LibFillResults', env => {
takerAssetFilledAmount: Web3Wrapper.toBaseUnitAmount(10, 18),
makerFeePaid: Web3Wrapper.toBaseUnitAmount(100, 16),
takerFeePaid: Web3Wrapper.toBaseUnitAmount(100, 16),
protocolFeePaid: constants.ZERO_AMOUNT,
protocolFeePaid: Web3Wrapper.toBaseUnitAmount(15, 4),
},
right: {
makerAssetFilledAmount: Web3Wrapper.toBaseUnitAmount(10, 18),
takerAssetFilledAmount: Web3Wrapper.toBaseUnitAmount(2, 18),
makerFeePaid: Web3Wrapper.toBaseUnitAmount(100, 16),
takerFeePaid: Web3Wrapper.toBaseUnitAmount(100, 16),
protocolFeePaid: constants.ZERO_AMOUNT,
protocolFeePaid: Web3Wrapper.toBaseUnitAmount(15, 4),
},
profitInLeftMakerAsset: Web3Wrapper.toBaseUnitAmount(3, 18),
profitInRightMakerAsset: constants.ZERO_AMOUNT,
@ -401,6 +533,8 @@ blockchainTests('LibFillResults', env => {
rightOrder,
leftOrderTakerAssetFilledAmount,
rightOrderTakerAssetFilledAmount,
protocolFeeMultiplier,
gasPrice,
false,
{ from },
);
@ -1072,6 +1206,8 @@ blockchainTests('LibFillResults', env => {
rightOrder,
leftOrderTakerAssetFilledAmount,
rightOrderTakerAssetFilledAmount,
protocolFeeMultiplier,
gasPrice,
true,
{ from },
);

View File

@ -217,18 +217,10 @@ contract MixinExchangeCore is
uint256 takerAssetFilledAmount = LibSafeMath.min256(takerAssetFillAmount, remainingTakerAssetAmount);
// Compute proportional fill amounts
fillResults = LibFillResults.calculateFillResults(order, takerAssetFilledAmount);
fillResults = LibFillResults.calculateFillResults(order, takerAssetFilledAmount, protocolFeeMultiplier, tx.gasprice);
bytes32 orderHash = orderInfo.orderHash;
// Settle order
_settleOrder(
orderHash,
order,
takerAddress,
fillResults
);
// Update exchange internal state
_updateFilledState(
order,
@ -238,6 +230,14 @@ contract MixinExchangeCore is
fillResults
);
// Settle order
_settleOrder(
orderHash,
order,
takerAddress,
fillResults
);
return fillResults;
}
@ -292,8 +292,7 @@ contract MixinExchangeCore is
fillResults.takerAssetFilledAmount,
fillResults.makerFeePaid,
fillResults.takerFeePaid,
fillResults.protocolFeePaid,
msg.value >= fillResults.protocolFeePaid
fillResults.protocolFeePaid
);
}
@ -471,8 +470,7 @@ contract MixinExchangeCore is
// Calculate the protocol fee that should be paid and populate the `protocolFeePaid` field in `fillResults`.
// It's worth noting that we leave this calculation until now so that work isn't wasted if a fee collector
// is not registered in the exchange.
uint256 protocolFee = tx.gasprice.safeMul(protocolFeeMultiplier);
fillResults.protocolFeePaid = protocolFee;
uint256 protocolFee = fillResults.protocolFeePaid;
// If sufficient ether was sent to the contract, the protocol fee should be paid in ETH.
// Otherwise the fee should be paid in WETH. Since the exchange doesn't actually handle
@ -482,6 +480,8 @@ contract MixinExchangeCore is
valuePaid = protocolFee;
}
IStaking(feeCollector).payProtocolFee.value(valuePaid)(order.makerAddress, takerAddress, protocolFee);
} else {
fillResults.protocolFeePaid = 0;
}
}
}

View File

@ -385,19 +385,11 @@ contract MixinMatchOrders is
rightOrder,
leftOrderInfo.orderTakerAssetFilledAmount,
rightOrderInfo.orderTakerAssetFilledAmount,
protocolFeeMultiplier,
tx.gasprice,
shouldMaximallyFillOrders
);
// Settle matched orders. Succeeds or throws.
_settleMatchedOrders(
leftOrderInfo.orderHash,
rightOrderInfo.orderHash,
leftOrder,
rightOrder,
takerAddress,
matchedFillResults
);
// Update exchange state
_updateFilledState(
leftOrder,
@ -414,6 +406,16 @@ contract MixinMatchOrders is
matchedFillResults.right
);
// Settle matched orders. Succeeds or throws.
_settleMatchedOrders(
leftOrderInfo.orderHash,
rightOrderInfo.orderHash,
leftOrder,
rightOrder,
takerAddress,
matchedFillResults
);
return matchedFillResults;
}
@ -492,12 +494,8 @@ contract MixinMatchOrders is
// Pay the protocol fees if there is a registered `protocolFeeCollector` address.
address feeCollector = protocolFeeCollector;
if (feeCollector != address(0)) {
// Calculate the protocol fee that should be paid and populate the `protocolFeePaid` field in the left and
// right `fillResults` of `matchedFillResults`. It's worth noting that we leave this calculation until now
// so that work isn't wasted if a fee collector is not registered in the exchange.
uint256 protocolFee = tx.gasprice.safeMul(protocolFeeMultiplier);
matchedFillResults.left.protocolFeePaid = protocolFee;
matchedFillResults.right.protocolFeePaid = protocolFee;
// Only one of the protocol fees is used because they are identical.
uint256 protocolFee = matchedFillResults.left.protocolFeePaid;
// Create a stack variable for the value that will be sent to the feeCollector when `payProtocolFee` is called.
// This allows a gas optimization where the `leftOrder.makerAddress` only needs be loaded onto the stack once AND
@ -510,7 +508,7 @@ contract MixinMatchOrders is
}
IStaking(feeCollector).payProtocolFee.value(valuePaid)(leftOrder.makerAddress, takerAddress, protocolFee);
// Clear value paid for the next call to `payProtocolFee()`.
// Clear the value paid for the next calculation.
valuePaid = 0;
// Pay the right order's protocol fee.
@ -518,6 +516,9 @@ contract MixinMatchOrders is
valuePaid = protocolFee;
}
IStaking(feeCollector).payProtocolFee.value(valuePaid)(rightOrder.makerAddress, takerAddress, protocolFee);
} else {
matchedFillResults.left.protocolFeePaid = 0;
matchedFillResults.right.protocolFeePaid = 0;
}
// Settle taker fees.

View File

@ -34,21 +34,21 @@ contract MixinProtocolFees is
/// @dev Allows the owner to update the protocol fee multiplier.
/// @param updatedProtocolFeeMultiplier The updated protocol fee multiplier.
function updateProtocolFeeMultiplier(uint256 updatedProtocolFeeMultiplier)
function setProtocolFeeMultiplier(uint256 updatedProtocolFeeMultiplier)
external
onlyOwner
{
emit UpdatedProtocolFeeMultiplier(protocolFeeMultiplier, updatedProtocolFeeMultiplier);
emit ProtocolFeeMultiplier(protocolFeeMultiplier, updatedProtocolFeeMultiplier);
protocolFeeMultiplier = updatedProtocolFeeMultiplier;
}
/// @dev Allows the owner to update the protocolFeeCollector address.
/// @param updatedProtocolFeeCollector The updated protocolFeeCollector contract address.
function updateProtocolFeeCollectorAddress(address updatedProtocolFeeCollector)
function setProtocolFeeCollectorAddress(address updatedProtocolFeeCollector)
external
onlyOwner
{
emit UpdatedProtocolFeeCollectorAddress(protocolFeeCollector, updatedProtocolFeeCollector);
emit ProtocolFeeCollectorAddress(protocolFeeCollector, updatedProtocolFeeCollector);
protocolFeeCollector = updatedProtocolFeeCollector;
}
}

View File

@ -109,7 +109,7 @@ contract MixinWrapperFunctions is
public
payable
nonReentrant
disableRefundUntilEnd
refundFinalBalance
returns (LibFillResults.FillResults[] memory fillResults)
{
uint256 ordersLength = orders.length;
@ -137,7 +137,7 @@ contract MixinWrapperFunctions is
public
payable
nonReentrant
disableRefundUntilEnd
refundFinalBalance
returns (LibFillResults.FillResults[] memory fillResults)
{
uint256 ordersLength = orders.length;
@ -291,7 +291,6 @@ contract MixinWrapperFunctions is
)
public
payable
disableRefundUntilEnd
returns (LibFillResults.FillResults memory fillResults)
{
fillResults = marketSellOrdersNoThrow(orders, takerAssetFillAmount, signatures);
@ -316,7 +315,6 @@ contract MixinWrapperFunctions is
)
public
payable
disableRefundUntilEnd
returns (LibFillResults.FillResults memory fillResults)
{
fillResults = marketBuyOrdersNoThrow(orders, makerAssetFillAmount, signatures);
@ -341,6 +339,22 @@ contract MixinWrapperFunctions is
}
}
/// @dev Fetches information for all passed in orders.
/// @param orders Array of order specifications.
/// @return Array of OrderInfo instances that correspond to each order.
function getOrdersInfo(LibOrder.Order[] memory orders)
public
view
returns (LibOrder.OrderInfo[] memory)
{
uint256 ordersLength = orders.length;
LibOrder.OrderInfo[] memory ordersInfo = new LibOrder.OrderInfo[](ordersLength);
for (uint256 i = 0; i != ordersLength; i++) {
ordersInfo[i] = getOrderInfo(orders[i]);
}
return ordersInfo;
}
/// @dev Fills the input order. Reverts if exact takerAssetFillAmount not filled.
/// @param order Order struct containing order specifications.
/// @param takerAssetFillAmount Desired amount of takerAsset to sell.
@ -367,20 +381,4 @@ contract MixinWrapperFunctions is
}
return fillResults;
}
/// @dev Fetches information for all passed in orders.
/// @param orders Array of order specifications.
/// @return Array of OrderInfo instances that correspond to each order.
function getOrdersInfo(LibOrder.Order[] memory orders)
public
view
returns (LibOrder.OrderInfo[] memory)
{
uint256 ordersLength = orders.length;
LibOrder.OrderInfo[] memory ordersInfo = new LibOrder.OrderInfo[](ordersLength);
for (uint256 i = 0; i != ordersLength; i++) {
ordersInfo[i] = getOrderInfo(orders[i]);
}
return ordersInfo;
}
}

View File

@ -25,9 +25,6 @@ import "@0x/contracts-exchange-libs/contracts/src/LibFillResults.sol";
contract IExchangeCore {
// keccak256("Fill(address,address,bytes32,address,address,uint256,uint256,uint256,uint256,uint256,bool,bytes,bytes,bytes,bytes)")
bytes32 internal constant FILL_EVENT_TOPIC = 0x266de417a663e51231ccdf89b2794cea06fde5e2c433d76473160b32d31fd867;
// Fill event is emitted whenever an order is filled.
event Fill(
address indexed makerAddress, // Address that created the order.
@ -43,8 +40,7 @@ contract IExchangeCore {
uint256 takerAssetFilledAmount, // Amount of takerAsset sold by taker and bought by maker.
uint256 makerFeePaid, // Amount of makerFeeAssetData paid to feeRecipient by maker.
uint256 takerFeePaid, // Amount of takerFeeAssetData paid to feeRecipient by taker.
uint256 protocolFeePaid, // Amount of eth or weth paid to the staking contract.
bool isProtocolFeePaidInWei // Indicates whether the protocol fee is paid in Wei (ETH) or WETH.
uint256 protocolFeePaid // Amount of eth or weth paid to the staking contract.
);
// Cancel event is emitted whenever an individual order is cancelled.

View File

@ -25,18 +25,18 @@ contract IProtocolFees {
bytes internal constant WETH_ASSET_DATA = hex"f47261b0";
// Logs updates to the protocol fee multiplier.
event UpdatedProtocolFeeMultiplier(uint256 oldProtocolFeeMultiplier, uint256 updatedProtocolFeeMultiplier);
event ProtocolFeeMultiplier(uint256 oldProtocolFeeMultiplier, uint256 updatedProtocolFeeMultiplier);
// Logs updates to the protocolFeeCollector address.
event UpdatedProtocolFeeCollectorAddress(address oldProtocolFeeCollector, address updatedProtocolFeeCollector);
event ProtocolFeeCollectorAddress(address oldProtocolFeeCollector, address updatedProtocolFeeCollector);
/// @dev Allows the owner to update the protocol fee multiplier.
/// @param updatedProtocolFeeMultiplier The updated protocol fee multiplier.
function updateProtocolFeeMultiplier(uint256 updatedProtocolFeeMultiplier)
function setProtocolFeeMultiplier(uint256 updatedProtocolFeeMultiplier)
external;
/// @dev Allows the owner to update the protocolFeeCollector address.
/// @param updatedProtocolFeeCollector The updated protocolFeeCollector contract address.
function updateProtocolFeeCollectorAddress(address updatedProtocolFeeCollector)
function setProtocolFeeCollectorAddress(address updatedProtocolFeeCollector)
external;
}

View File

@ -1,50 +0,0 @@
/*
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.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
pragma solidity ^0.5.9;
contract IStakingManager {
// The proxy id of the weth asset proxy.
bytes internal constant WETH_ASSET_DATA = hex"f47261b0";
// Logs updates to the protocol fee multiplier.
event UpdatedProtocolFeeMultiplier(uint256 oldProtocolFeeMultiplier, uint256 updatedProtocolFeeMultiplier);
// Logs updates to the staking address.
event UpdatedStakingAddress(address oldStaking, address updatedStaking);
// Logs updates to the weth address.
event UpdatedWethAddress(address oldWeth, address updatedWeth);
/// @dev Allows the owner to update the protocol fee multiplier.
/// @param updatedProtocolFeeMultiplier The updated protocol fee multiplier.
function updateProtocolFeeMultiplier(uint256 updatedProtocolFeeMultiplier)
external;
/// @dev Allows the owner to update the staking address.
/// @param updatedStaking The updated staking contract address.
function updateStakingAddress(address updatedStaking)
external;
/// @dev Allows the owner to update the WETH address.
/// @param updatedWeth The updated WETH contract address.
function updateWethAddress(address updatedWeth)
external;
}

View File

@ -156,4 +156,12 @@ contract IWrapperFunctions {
/// @param orders Array of order specifications.
function batchCancelOrders(LibOrder.Order[] memory orders)
public;
/// @dev Fetches information for all passed in orders
/// @param orders Array of order specifications.
/// @return Array of OrderInfo instances that correspond to each order.
function getOrdersInfo(LibOrder.Order[] memory orders)
public
view
returns (LibOrder.OrderInfo[] memory);
}

View File

@ -15,7 +15,6 @@
limitations under the License.
*/
pragma solidity ^0.5.9;
pragma experimental ABIEncoderV2;
@ -300,14 +299,6 @@ contract TestProtocolFeesReceiver {
// Execute the test.
_;
// Clear exchange state
testProtocolFees.setProtocolFeeCollector(address(0));
testProtocolFees.setProtocolFeeMultiplier(0);
msg.sender.transfer(address(this).balance);
// Clear this contract's state
delete testLogs;
}
/// @dev Constructs an order with a specified maker address.

View File

@ -35,7 +35,7 @@
"compile:truffle": "truffle compile"
},
"config": {
"abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxy|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|IProtocolFees|ISignatureValidator|IStakingManager|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinProtocolFees|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestProtocolFees|TestProtocolFeesReceiver|TestSignatureValidator|TestTransactions|TestValidatorWallet|TestWrapperFunctions|Whitelist).json",
"abis": "./generated-artifacts/@(Exchange|ExchangeWrapper|IAssetProxy|IAssetProxyDispatcher|IEIP1271Wallet|IExchange|IExchangeCore|IMatchOrders|IProtocolFees|ISignatureValidator|ITransactions|ITransferSimulator|IWallet|IWrapperFunctions|IsolatedExchange|LibExchangeRichErrorDecoder|MixinAssetProxyDispatcher|MixinExchangeCore|MixinMatchOrders|MixinProtocolFees|MixinSignatureValidator|MixinTransactions|MixinTransferSimulator|MixinWrapperFunctions|ReentrancyTester|TestAssetProxyDispatcher|TestExchangeInternals|TestLibExchangeRichErrorDecoder|TestProtocolFees|TestProtocolFeesReceiver|TestSignatureValidator|TestTransactions|TestValidatorWallet|TestWrapperFunctions|Whitelist).json",
"abis:comment": "This list is auto-generated by contracts-gen. Don't edit manually."
},
"repository": {

View File

@ -16,7 +16,6 @@ import * as IMatchOrders from '../generated-artifacts/IMatchOrders.json';
import * as IProtocolFees from '../generated-artifacts/IProtocolFees.json';
import * as ISignatureValidator from '../generated-artifacts/ISignatureValidator.json';
import * as IsolatedExchange from '../generated-artifacts/IsolatedExchange.json';
import * as IStakingManager from '../generated-artifacts/IStakingManager.json';
import * as ITransactions from '../generated-artifacts/ITransactions.json';
import * as ITransferSimulator from '../generated-artifacts/ITransferSimulator.json';
import * as IWallet from '../generated-artifacts/IWallet.json';
@ -61,7 +60,6 @@ export const artifacts = {
IMatchOrders: IMatchOrders as ContractArtifact,
IProtocolFees: IProtocolFees as ContractArtifact,
ISignatureValidator: ISignatureValidator as ContractArtifact,
IStakingManager: IStakingManager as ContractArtifact,
ITransactions: ITransactions as ContractArtifact,
ITransferSimulator: ITransferSimulator as ContractArtifact,
IWallet: IWallet as ContractArtifact,

View File

@ -13,7 +13,6 @@ export * from '../generated-wrappers/i_exchange_core';
export * from '../generated-wrappers/i_match_orders';
export * from '../generated-wrappers/i_protocol_fees';
export * from '../generated-wrappers/i_signature_validator';
export * from '../generated-wrappers/i_staking_manager';
export * from '../generated-wrappers/i_transactions';
export * from '../generated-wrappers/i_transfer_simulator';
export * from '../generated-wrappers/i_wallet';

View File

@ -8,6 +8,7 @@ import { artifacts as erc20Artifacts } from '@0x/contracts-erc20';
import { artifacts as erc721Artifacts } from '@0x/contracts-erc721';
import { ReferenceFunctions as LibReferenceFunctions } from '@0x/contracts-exchange-libs';
import {
constants,
expect,
FillEventArgs,
filterLogsToArguments,
@ -58,7 +59,13 @@ export class FillOrderWrapper {
const balanceStore = LocalBalanceStore.create(initBalanceStore);
const takerAssetFillAmount =
opts.takerAssetFillAmount !== undefined ? opts.takerAssetFillAmount : signedOrder.takerAssetAmount;
const fillResults = LibReferenceFunctions.calculateFillResults(signedOrder, takerAssetFillAmount);
// TODO(jalextowle): Change this if the integration tests take protocol fees into account.
const fillResults = LibReferenceFunctions.calculateFillResults(
signedOrder,
takerAssetFillAmount,
constants.ZERO_AMOUNT,
constants.ZERO_AMOUNT,
);
const fillEvent = FillOrderWrapper.simulateFillEvent(signedOrder, takerAddress, fillResults);
// Taker -> Maker
balanceStore.transferAsset(

View File

@ -0,0 +1,18 @@
import { chaiSetup, getCodesizeFromArtifact } from '@0x/contracts-test-utils';
import * as chai from 'chai';
chaiSetup.configure();
const expect = chai.expect;
import { artifacts } from '../src';
describe('Contract Size Checks', () => {
const MAX_CODE_SIZE = 24576;
describe('Exchange', () => {
it('should have a codesize less than the maximum', async () => {
const actualSize = getCodesizeFromArtifact(artifacts.Exchange);
expect(actualSize).to.be.lt(MAX_CODE_SIZE);
});
});
});

View File

@ -23,6 +23,8 @@ blockchainTests('Exchange core internal functions', env => {
let testExchange: TestExchangeInternalsContract;
let logDecoder: LogDecoder;
let senderAddress: string;
const DEFAULT_PROTOCOL_MULTIPLIER = new BigNumber(150000);
const DEFAULT_GAS_PRICE = new BigNumber(200000);
before(async () => {
const accounts = await env.getAccountAddressesAsync();
@ -169,12 +171,16 @@ blockchainTests('Exchange core internal functions', env => {
orderTakerAssetFilledAmount: BigNumber,
takerAddress: string,
takerAssetFillAmount: BigNumber,
// protocolFeeMultiplier: BigNumber,
// gasPrice: BigNumber,
// isProtocolFeePaidInEth: boolean,
protocolFeeMultiplier: BigNumber,
gasPrice: BigNumber,
): Promise<void> {
const orderHash = randomHash();
const fillResults = LibReferenceFunctions.calculateFillResults(order, takerAssetFillAmount);
const fillResults = LibReferenceFunctions.calculateFillResults(
order,
takerAssetFillAmount,
protocolFeeMultiplier,
gasPrice,
);
const expectedFilledState = orderTakerAssetFilledAmount.plus(takerAssetFillAmount);
// const opts = isProtocolFeePaidInEth
// ? { value: fillResults.protocolFeePaid }
@ -189,7 +195,6 @@ blockchainTests('Exchange core internal functions', env => {
orderHash,
orderTakerAssetFilledAmount,
fillResults,
// opts,
),
);
// Grab the new `filled` state for this order.
@ -209,31 +214,22 @@ blockchainTests('Exchange core internal functions', env => {
expect(fillEvent.args.takerAssetFilledAmount).to.bignumber.eq(fillResults.takerAssetFilledAmount);
expect(fillEvent.args.makerFeePaid).to.bignumber.eq(fillResults.makerFeePaid);
expect(fillEvent.args.takerFeePaid).to.bignumber.eq(fillResults.takerFeePaid);
// expect(fillEvent.args.protocolFeePaid).to.bignumber.eq(fillResults.protocolFeePaid);
// expect(fillEvent.args.isProtocolFeePaidInEth).to.eq(isProtocolFeePaidInEth);
expect(fillEvent.args.makerAssetData).to.eq(order.makerAssetData);
expect(fillEvent.args.takerAssetData).to.eq(order.takerAssetData);
expect(fillEvent.args.makerFeeAssetData).to.eq(order.makerFeeAssetData);
expect(fillEvent.args.takerFeeAssetData).to.eq(order.takerFeeAssetData);
expect(fillEvent.args.protocolFeePaid).to.bignumber.eq(fillResults.protocolFeePaid);
}
it('emits a `Fill` event and updates `filled` state correctly if protocol fee is paid in ETH', async () => {
const order = makeOrder();
return testUpdateFilledStateAsync(
order,
order.takerAssetAmount.times(0.1),
randomAddress(),
order.takerAssetAmount.times(0.25),
);
});
it('emits a `Fill` event and updates `filled` state correctly if protocol fee is paid in WETH', async () => {
it('emits a `Fill` event and updates `filled` state correctly', async () => {
const order = makeOrder();
return testUpdateFilledStateAsync(
order,
order.takerAssetAmount.times(0.1),
randomAddress(),
order.takerAssetAmount.times(0.25),
DEFAULT_PROTOCOL_MULTIPLIER,
DEFAULT_GAS_PRICE,
);
});

View File

@ -37,6 +37,8 @@ blockchainTests('Isolated fillOrder() tests', env => {
makerFeeAssetData: createGoodAssetData(),
takerFeeAssetData: createGoodAssetData(),
};
const DEFAULT_GAS_PRICE = new BigNumber(20000);
const DEFAULT_PROTOCOL_FEE_MULTIPLIER = new BigNumber(15000);
let takerAddress: string;
let notTakerAddress: string;
let exchange: IsolatedExchangeWrapper;
@ -69,7 +71,13 @@ blockchainTests('Isolated fillOrder() tests', env => {
signature?: string,
): Promise<FillOrderAndAssertResultsResults> {
const orderInfo = await exchange.getOrderInfoAsync(order);
const efr = calculateExpectedFillResults(order, orderInfo, takerAssetFillAmount);
const efr = calculateExpectedFillResults(
order,
orderInfo,
takerAssetFillAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
);
const eoi = calculateExpectedOrderInfo(order, orderInfo, efr);
const efb = calculateExpectedFillBalances(order, efr);
// Fill the order.
@ -106,11 +114,15 @@ blockchainTests('Isolated fillOrder() tests', env => {
order: Order,
orderInfo: OrderInfo,
takerAssetFillAmount: BigNumber,
protocolFeeMultiplier: BigNumber,
gasPrice: BigNumber,
): FillResults {
const remainingTakerAssetAmount = order.takerAssetAmount.minus(orderInfo.orderTakerAssetFilledAmount);
return LibReferenceFunctions.calculateFillResults(
order,
BigNumber.min(takerAssetFillAmount, remainingTakerAssetAmount),
protocolFeeMultiplier,
gasPrice,
);
}

View File

@ -42,7 +42,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay protocol fee in WETH when too little value is sent', async () => {
it('should not forward ETH when too little value is sent', async () => {
await testProtocolFeesReceiver.testFillOrderProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -92,7 +92,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay protocol fee in WETH twice when too little value is sent', async () => {
it('should not forward ETH twice when too little value is sent', async () => {
await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -104,7 +104,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay protocol fee in ETH and then WETH when the correct value is sent', async () => {
it('should pay protocol fee in ETH and then not forward ETH when exactly one protocol fee is sent', async () => {
await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -116,7 +116,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay protocol fee in ETH when extra value is sent and then pay in WETH', async () => {
it('should pay protocol fee in ETH and then not forward ETH when a bit more than one protocol fee is sent', async () => {
await testProtocolFeesReceiver.testMatchOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -167,7 +167,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay one protocol fee in WETH when too little ETH is sent and only one order is in the batch', async () => {
it('should not forward ETH when less than one protocol fee is sent and only one order is in the batch', async () => {
await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -206,7 +206,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay both protocol fees in WETH when an insuffiecent amount of ETH for one protocol fee is sent', async () => {
it('should not forward ETH twice when an insuffiecent amount of ETH for one protocol fee is sent', async () => {
await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -219,7 +219,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay a protocol in ETH and then a fee in WETH when exactly one protocol fee in ETH is sent', async () => {
it('should pay a protocol in ETH and not forward ETH for the second when exactly one protocol fee in ETH is sent', async () => {
await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
@ -245,7 +245,7 @@ blockchainTests('Protocol Fee Payments', env => {
);
});
it('should pay two protocol fees in ETH and one in WETH when exactly two protocol fees in ETH is sent', async () => {
it('should pay two protocol fees in ETH and then not forward ETH for a third when exactly two protocol fees in ETH is sent', async () => {
await testProtocolFeesReceiver.testBatchFillOrdersProtocolFees.awaitTransactionSuccessAsync(
testProtocolFees.address,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,

View File

@ -1,18 +1,17 @@
import { blockchainTests, constants, expect, LogDecoder } from '@0x/contracts-test-utils';
import { blockchainTests, constants, expect } from '@0x/contracts-test-utils';
import { BigNumber, OwnableRevertErrors } from '@0x/utils';
import { LogWithDecodedArgs } from 'ethereum-types';
import {
artifacts,
ExchangeContract,
ExchangeUpdatedProtocolFeeCollectorAddressEventArgs,
ExchangeUpdatedProtocolFeeMultiplierEventArgs,
ExchangeProtocolFeeCollectorAddressEventArgs,
ExchangeProtocolFeeMultiplierEventArgs,
} from '../src';
blockchainTests.resets('MixinProtocolFees', env => {
let accounts: string[];
let exchange: ExchangeContract;
let logDecoder: LogDecoder;
let nonOwner: string;
let owner: string;
let protocolFeeCollector: string;
@ -37,60 +36,59 @@ blockchainTests.resets('MixinProtocolFees', env => {
{},
new BigNumber(1337),
);
// Configure the log decoder
logDecoder = new LogDecoder(env.web3Wrapper, artifacts);
});
blockchainTests.resets('updateProtocolFeeMultiplier', () => {
blockchainTests.resets('setProtocolFeeMultiplier', () => {
it('should revert if msg.sender != owner', async () => {
const expectedError = new OwnableRevertErrors.OnlyOwnerError(nonOwner, owner);
// Ensure that the transaction reverts with the expected rich error.
const tx = exchange.updateProtocolFeeCollectorAddress.sendTransactionAsync(protocolFeeCollector, {
const tx = exchange.setProtocolFeeCollectorAddress.sendTransactionAsync(protocolFeeCollector, {
from: nonOwner,
});
return expect(tx).to.revertWith(expectedError);
});
it('should succeed and emit an UpdatedProtocolFeeMultiplier event if msg.sender == owner', async () => {
// Call the `updateProtocolFeeMultiplier()` function and get the receipt.
const receipt = await logDecoder.getTxWithDecodedLogsAsync(
await exchange.updateProtocolFeeMultiplier.sendTransactionAsync(protocolFeeMultiplier, {
it('should succeed and emit an ProtocolFeeMultiplier event if msg.sender == owner', async () => {
// Call the `setProtocolFeeMultiplier()` function and get the receipt.
const receipt = await exchange.setProtocolFeeMultiplier.awaitTransactionSuccessAsync(
protocolFeeMultiplier,
{
from: owner,
}),
},
);
// Verify that the protocolFeeCollector address was actually updated to the correct address.
const updated = await exchange.protocolFeeMultiplier.callAsync();
expect(updated).bignumber.to.be.eq(protocolFeeMultiplier);
// Ensure that the correct `UpdatedProtocolFeeCollectorAddress` event was logged.
// Ensure that the correct `ProtocolFeeCollectorAddress` event was logged.
// tslint:disable:no-unnecessary-type-assertion
const updatedEvent = receipt.logs[0] as LogWithDecodedArgs<ExchangeUpdatedProtocolFeeMultiplierEventArgs>;
expect(updatedEvent.event).to.be.eq('UpdatedProtocolFeeMultiplier');
const updatedEvent = receipt.logs[0] as LogWithDecodedArgs<ExchangeProtocolFeeMultiplierEventArgs>;
expect(updatedEvent.event).to.be.eq('ProtocolFeeMultiplier');
expect(updatedEvent.args.oldProtocolFeeMultiplier).bignumber.to.be.eq(constants.ZERO_AMOUNT);
expect(updatedEvent.args.updatedProtocolFeeMultiplier).bignumber.to.be.eq(protocolFeeMultiplier);
});
});
blockchainTests.resets('updateProtocolFeeCollectorAddress', () => {
blockchainTests.resets('setProtocolFeeCollectorAddress', () => {
it('should revert if msg.sender != owner', async () => {
const expectedError = new OwnableRevertErrors.OnlyOwnerError(nonOwner, owner);
// Ensure that the transaction reverts with the expected rich error.
const tx = exchange.updateProtocolFeeCollectorAddress.sendTransactionAsync(protocolFeeCollector, {
const tx = exchange.setProtocolFeeCollectorAddress.sendTransactionAsync(protocolFeeCollector, {
from: nonOwner,
});
return expect(tx).to.revertWith(expectedError);
});
it('should succeed and emit an UpdatedProtocolFeeCollectorAddress event if msg.sender == owner', async () => {
// Call the `updateProtocolFeeCollectorAddress()` function and get the receipt.
const receipt = await logDecoder.getTxWithDecodedLogsAsync(
await exchange.updateProtocolFeeCollectorAddress.sendTransactionAsync(protocolFeeCollector, {
it('should succeed and emit an ProtocolFeeCollectorAddress event if msg.sender == owner', async () => {
// Call the `setProtocolFeeCollectorAddress()` function and get the receipt.
const receipt = await exchange.setProtocolFeeCollectorAddress.awaitTransactionSuccessAsync(
protocolFeeCollector,
{
from: owner,
}),
},
);
// Verify that the protocolFeeCollector address was actually updated to the correct address.
@ -99,10 +97,8 @@ blockchainTests.resets('MixinProtocolFees', env => {
// Ensure that the correct `UpdatedProtocolFeeCollectorAddress` event was logged.
// tslint:disable:no-unnecessary-type-assertion
const updatedEvent = receipt.logs[0] as LogWithDecodedArgs<
ExchangeUpdatedProtocolFeeCollectorAddressEventArgs
>;
expect(updatedEvent.event).to.be.eq('UpdatedProtocolFeeCollectorAddress');
const updatedEvent = receipt.logs[0] as LogWithDecodedArgs<ExchangeProtocolFeeCollectorAddressEventArgs>;
expect(updatedEvent.event).to.be.eq('ProtocolFeeCollectorAddress');
expect(updatedEvent.args.oldProtocolFeeCollector).to.be.eq(constants.NULL_ADDRESS);
expect(updatedEvent.args.updatedProtocolFeeCollector).to.be.eq(protocolFeeCollector);
});

View File

@ -7,6 +7,8 @@ import * as _ from 'lodash';
describe('Reference functions', () => {
const ONE_ETHER = constants.ONE_ETHER;
const DEFAULT_GAS_PRICE = new BigNumber(2);
const DEFAULT_PROTOCOL_FEE_MULTIPLIER = new BigNumber(150);
const EMPTY_ORDER: Order = {
senderAddress: constants.NULL_ADDRESS,
makerAddress: constants.NULL_ADDRESS,
@ -42,9 +44,14 @@ describe('Reference functions', () => {
takerAssetFilledAmount,
order.makerAssetAmount,
);
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if computing `fillResults.makerFeePaid` overflows', () => {
@ -65,9 +72,14 @@ describe('Reference functions', () => {
makerAssetFilledAmount,
order.makerFee,
);
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if computing `fillResults.takerFeePaid` overflows', () => {
@ -83,9 +95,14 @@ describe('Reference functions', () => {
takerAssetFilledAmount,
order.takerFee,
);
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if `order.makerAssetAmount` is 0', () => {
@ -95,9 +112,14 @@ describe('Reference functions', () => {
});
const takerAssetFilledAmount = ONE_ETHER;
const expectedError = new LibMathRevertErrors.DivisionByZeroError();
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if `order.takerAssetAmount` is 0', () => {
@ -107,9 +129,14 @@ describe('Reference functions', () => {
});
const takerAssetFilledAmount = ONE_ETHER;
const expectedError = new LibMathRevertErrors.DivisionByZeroError();
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if there is a rounding error computing `makerAsssetFilledAmount`', () => {
@ -123,9 +150,14 @@ describe('Reference functions', () => {
order.takerAssetAmount,
order.makerAssetAmount,
);
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if there is a rounding error computing `makerFeePaid`', () => {
@ -145,9 +177,14 @@ describe('Reference functions', () => {
order.makerAssetAmount,
order.makerFee,
);
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if there is a rounding error computing `takerFeePaid`', () => {
@ -167,9 +204,36 @@ describe('Reference functions', () => {
order.makerAssetAmount,
order.takerFee,
);
return expect(() => LibReferenceFunctions.calculateFillResults(order, takerAssetFilledAmount)).to.throw(
expectedError.message,
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
DEFAULT_PROTOCOL_FEE_MULTIPLIER,
DEFAULT_GAS_PRICE,
),
).to.throw(expectedError.message);
});
it('reverts if there is an overflow computing `protocolFeePaid`', () => {
const order = makeOrder({
makerAssetAmount: ONE_ETHER,
takerAssetAmount: ONE_ETHER,
takerFee: constants.ZERO_AMOUNT,
});
const takerAssetFilledAmount = order.takerAssetAmount.dividedToIntegerBy(3);
const expectedError = new SafeMathRevertErrors.Uint256BinOpError(
SafeMathRevertErrors.BinOpErrorCodes.MultiplicationOverflow,
MAX_UINT256_ROOT,
MAX_UINT256_ROOT,
);
return expect(() =>
LibReferenceFunctions.calculateFillResults(
order,
takerAssetFilledAmount,
MAX_UINT256_ROOT,
MAX_UINT256_ROOT,
),
).to.throw(expectedError.message);
});
});
});

View File

@ -9,8 +9,8 @@ export const constants = {
ExchangeFunctionName.RegisterAssetProxy,
ExchangeFunctionName.SimulateDispatchTransferFromCalls,
ExchangeFunctionName.TransferOwnership,
ExchangeFunctionName.UpdateProtocolFeeMultiplier,
ExchangeFunctionName.UpdateProtocolFeeCollectorAddress,
ExchangeFunctionName.SetProtocolFeeMultiplier,
ExchangeFunctionName.SetProtocolFeeCollectorAddress,
],
SINGLE_FILL_FN_NAMES: [
ExchangeFunctionName.FillOrder,

View File

@ -32,6 +32,6 @@ export enum ExchangeFunctionName {
SetSignatureValidatorApproval = 'setSignatureValidatorApproval',
SimulateDispatchTransferFromCalls = 'simulateDispatchTransferFromCalls',
TransferOwnership = 'transferOwnership',
UpdateProtocolFeeMultiplier = 'updateProtocolFeeMultiplier',
UpdateProtocolFeeCollectorAddress = 'updateProtocolFeeCollectorAddress',
SetProtocolFeeMultiplier = 'setProtocolFeeMultiplier',
SetProtocolFeeCollectorAddress = 'setProtocolFeeCollectorAddress',
}

View File

@ -91,7 +91,7 @@ blockchainTests.resets('Exchange wrappers', env => {
);
// Set the protocol fee multiplier of the exchange
await exchange.updateProtocolFeeMultiplier.awaitTransactionSuccessAsync(PROTOCOL_FEE_MULTIPLIER, {
await exchange.setProtocolFeeMultiplier.awaitTransactionSuccessAsync(PROTOCOL_FEE_MULTIPLIER, {
from: owner,
});

View File

@ -62,7 +62,7 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
);
// Set the protocol fee multiplier.
await testContract.updateProtocolFeeMultiplier.awaitTransactionSuccessAsync(protocolFeeMultiplier, {
await testContract.setProtocolFeeMultiplier.awaitTransactionSuccessAsync(protocolFeeMultiplier, {
from: owner,
});
});
@ -1320,50 +1320,50 @@ blockchainTests('Exchange wrapper functions unit tests.', env => {
const tx = txHelper.getResultAndReceiptAsync(testContract.batchCancelOrders, orders);
return expect(tx).to.revertWith(expectedError);
});
});
describe('getOrdersInfo', () => {
// Computes the expected (fake) order info generated by the `TestWrapperFunctions` contract.
function getExpectedOrderInfo(order: Order): OrderInfo {
const MAX_ORDER_STATUS = OrderStatus.Cancelled as number;
return {
orderHash: getExpectedOrderHash(order),
// Lower uint128 of `order.salt` is the `orderTakerAssetFilledAmount`.
orderTakerAssetFilledAmount: order.salt.mod(new BigNumber(2).pow(128)),
// High byte of `order.salt` is the `orderStatus`.
orderStatus:
order.salt.dividedToIntegerBy(new BigNumber(2).pow(248)).toNumber() % (MAX_ORDER_STATUS + 1),
};
}
describe('getOrdersInfo', () => {
// Computes the expected (fake) order info generated by the `TestWrapperFunctions` contract.
function getExpectedOrderInfo(order: Order): OrderInfo {
const MAX_ORDER_STATUS = OrderStatus.Cancelled as number;
return {
orderHash: getExpectedOrderHash(order),
// Lower uint128 of `order.salt` is the `orderTakerAssetFilledAmount`.
orderTakerAssetFilledAmount: order.salt.mod(new BigNumber(2).pow(128)),
// High byte of `order.salt` is the `orderStatus`.
orderStatus:
order.salt.dividedToIntegerBy(new BigNumber(2).pow(248)).toNumber() % (MAX_ORDER_STATUS + 1),
};
}
it('works with no orders', async () => {
const infos = await testContract.getOrdersInfo.callAsync([]);
expect(infos.length).to.eq(0);
});
it('works with no orders', async () => {
const infos = await testContract.getOrdersInfo.callAsync([]);
expect(infos.length).to.eq(0);
});
it('works with one order', async () => {
const orders = [randomOrder()];
const expectedResult = orders.map(getExpectedOrderInfo);
const actualResult = await testContract.getOrdersInfo.callAsync(orders);
expect(actualResult).to.deep.eq(expectedResult);
});
it('works with one order', async () => {
const orders = [randomOrder()];
const expectedResult = orders.map(getExpectedOrderInfo);
const actualResult = await testContract.getOrdersInfo.callAsync(orders);
expect(actualResult).to.deep.eq(expectedResult);
});
it('works with many orders', async () => {
const NUM_ORDERS = 16;
const orders = _.times(NUM_ORDERS, () => randomOrder());
const expectedResult = orders.map(getExpectedOrderInfo);
const actualResult = await testContract.getOrdersInfo.callAsync(orders);
expect(actualResult).to.deep.eq(expectedResult);
});
it('works with many orders', async () => {
const NUM_ORDERS = 16;
const orders = _.times(NUM_ORDERS, () => randomOrder());
const expectedResult = orders.map(getExpectedOrderInfo);
const actualResult = await testContract.getOrdersInfo.callAsync(orders);
expect(actualResult).to.deep.eq(expectedResult);
});
it('works with duplicate orders', async () => {
const NUM_UNIQUE_ORDERS = 4;
const CLONE_COUNT = 2;
const uniqueOrders = _.times(NUM_UNIQUE_ORDERS, () => randomOrder());
const orders = _.shuffle(_.flatten(_.times(CLONE_COUNT, () => uniqueOrders)));
const expectedResult = orders.map(getExpectedOrderInfo);
const actualResult = await testContract.getOrdersInfo.callAsync(orders);
expect(actualResult).to.deep.eq(expectedResult);
});
it('works with duplicate orders', async () => {
const NUM_UNIQUE_ORDERS = 4;
const CLONE_COUNT = 2;
const uniqueOrders = _.times(NUM_UNIQUE_ORDERS, () => randomOrder());
const orders = _.shuffle(_.flatten(_.times(CLONE_COUNT, () => uniqueOrders)));
const expectedResult = orders.map(getExpectedOrderInfo);
const actualResult = await testContract.getOrdersInfo.callAsync(orders);
expect(actualResult).to.deep.eq(expectedResult);
});
});
});

View File

@ -13,7 +13,6 @@
"generated-artifacts/IMatchOrders.json",
"generated-artifacts/IProtocolFees.json",
"generated-artifacts/ISignatureValidator.json",
"generated-artifacts/IStakingManager.json",
"generated-artifacts/ITransactions.json",
"generated-artifacts/ITransferSimulator.json",
"generated-artifacts/IWallet.json",

View File

@ -56,6 +56,7 @@ contract MixinExchangeFees is
{
using LibSafeMath for uint256;
/// TODO(jalextowle): Add WETH to protocol fees. Should this be unwrapped?
/// @dev Pays a protocol fee in ETH.
/// Only a known 0x exchange can call this method. See (MixinExchangeManager).
/// @param makerAddress The address of the order's maker.

View File

@ -191,7 +191,13 @@ export class Simulation {
const feeAmount = p.protocolFeesByMaker[i];
// TODO(jalextowle): I'll need to fix this once I make my PR on protocol fees. The arguments
// I'm adding are just placeholders for now.
await this._stakingWrapper.payProtocolFeeAsync(makerAddress, makerAddress, feeAmount, feeAmount, p.exchangeAddress);
await this._stakingWrapper.payProtocolFeeAsync(
makerAddress,
makerAddress,
feeAmount,
feeAmount,
p.exchangeAddress,
);
}
// validate fees per pool
let expectedTotalFeesThisEpoch = new BigNumber(0);

View File

@ -0,0 +1,8 @@
import { ContractArtifact } from 'ethereum-types';
/**
* Get the codesize of a provided artifact.
*/
export function getCodesizeFromArtifact(artifact: ContractArtifact): number {
return (artifact.compilerOutput.evm.bytecode.object.length - 2) / 2;
}

View File

@ -48,3 +48,4 @@ export {
} from './types';
export { blockchainTests, BlockchainTestsEnvironment, describe } from './mocha_blockchain';
export { chaiSetup, expect } from './chai_setup';
export { getCodesizeFromArtifact } from './codesize';

View File

@ -27,7 +27,7 @@ contract Refundable {
modifier refundFinalBalance {
_;
if (!shouldNotRefund) {
refundNonzeroBalance();
_refundNonzeroBalance();
}
}
@ -38,11 +38,11 @@ contract Refundable {
shouldNotRefund = true;
_;
shouldNotRefund = false;
refundNonzeroBalance();
_refundNonzeroBalance();
}
}
function refundNonzeroBalance()
function _refundNonzeroBalance()
internal
{
uint256 balance = address(this).balance;

View File

@ -28,7 +28,7 @@ contract TestRefundable is
external
payable
{
refundNonzeroBalance();
_refundNonzeroBalance();
}
function setShouldNotRefund(bool shouldNotRefundNew)