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

Remove partial cancels

Browse Source
This commit is contained in:
Amir Bandeali
2018-03-23 12:17:54 -07:00
parent 3f76985435
commit 8f809e3a29
8 changed files with 35 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
View File

@@ -41,7 +41,7 @@ contract MixinExchangeCore is
{
// Mappings of orderHash => amounts of takerTokenAmount filled or cancelled.
mapping (bytes32 => uint256) public filled;
mapping (bytes32 => uint256) public cancelled;
mapping (bytes32 => bool) public cancelled;
// Mapping of makerAddress => lowest salt an order can have in order to be fillable
// Orders with a salt less than their maker's epoch are considered cancelled
@@ -65,8 +65,6 @@ contract MixinExchangeCore is
address indexed feeRecipientAddress,
address makerTokenAddress,
address takerTokenAddress,
uint256 makerTokenCancelledAmount,
uint256 takerTokenCancelledAmount,
bytes32 indexed orderHash
);
@@ -94,9 +92,15 @@ contract MixinExchangeCore is
// Compute the order hash
bytes32 orderHash = getOrderHash(order);
// Check if order has been cancelled
if (cancelled[orderHash]) {
LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash);
return 0;
}
// Validate order and maker only if first time seen
// TODO: Read filled and cancelled only once
if (filled[orderHash] == 0 && cancelled[orderHash] == 0) {
if (filled[orderHash] == 0) {
require(order.makerTokenAmount > 0);
require(order.takerTokenAmount > 0);
require(isValidSignature(orderHash, order.makerAddress, signature));
@@ -115,14 +119,14 @@ contract MixinExchangeCore is
}
// Validate order availability
uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(orderHash));
takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount);
if (takerTokenFilledAmount == 0) {
LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash);
uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, filled[orderHash]);
if (remainingTakerTokenAmount == 0) {
LogError(uint8(Errors.ORDER_FULLY_FILLED), orderHash);
return 0;
}
// Validate fill order rounding
takerTokenFilledAmount = min256(takerTokenFillAmount, remainingTakerTokenAmount);
if (isRoundingError(takerTokenFilledAmount, order.takerTokenAmount, order.makerTokenAmount)) {
LogError(uint8(Errors.ROUNDING_ERROR_TOO_LARGE), orderHash);
return 0;
@@ -157,15 +161,12 @@ contract MixinExchangeCore is
return takerTokenFilledAmount;
}
/// @dev Cancels the input order.
/// @dev After calling, the order can not be filled anymore.
/// @param order Order struct containing order specifications.
/// @param takerTokenCancelAmount Desired amount of takerToken to cancel in order.
/// @return Amount of takerToken cancelled.
function cancelOrder(
Order order,
uint256 takerTokenCancelAmount)
/// @return True if the order state changed to cancelled. False if the transaction was already cancelled or expired.
function cancelOrder(Order order)
public
returns (uint256 takerTokenCancelledAmount)
returns (bool)
{
// Compute the order hash
bytes32 orderHash = getOrderHash(order);
@@ -173,33 +174,28 @@ contract MixinExchangeCore is
// Validate the order
require(order.makerTokenAmount > 0);
require(order.takerTokenAmount > 0);
require(takerTokenCancelAmount > 0);
require(order.makerAddress == msg.sender);
if (block.timestamp >= order.expirationTimeSeconds) {
LogError(uint8(Errors.ORDER_EXPIRED), orderHash);
return 0;
return false;
}
uint256 remainingTakerTokenAmount = safeSub(order.takerTokenAmount, getUnavailableTakerTokenAmount(orderHash));
takerTokenCancelledAmount = min256(takerTokenCancelAmount, remainingTakerTokenAmount);
if (takerTokenCancelledAmount == 0) {
if (cancelled[orderHash]) {
LogError(uint8(Errors.ORDER_FULLY_FILLED_OR_CANCELLED), orderHash);
return 0;
return false;
}
cancelled[orderHash] = safeAdd(cancelled[orderHash], takerTokenCancelledAmount);
cancelled[orderHash] = true;
LogCancel(
order.makerAddress,
order.feeRecipientAddress,
order.makerTokenAddress,
order.takerTokenAddress,
getPartialAmount(takerTokenCancelledAmount, order.takerTokenAmount, order.makerTokenAmount),
takerTokenCancelledAmount,
orderHash
);
return takerTokenCancelledAmount;
return true;
}
/// @param salt Orders created with a salt less or equal to this value will be cancelled.
@@ -233,15 +229,4 @@ contract MixinExchangeCore is
isError = errPercentageTimes1000000 > 1000;
return isError;
}
/// @dev Calculates the sum of values already filled and cancelled for a given order.
/// @param orderHash The Keccak-256 hash of the given order.
/// @return Sum of values already filled and cancelled.
function getUnavailableTakerTokenAmount(bytes32 orderHash)
public view
returns (uint256 unavailableTakerTokenAmount)
{
unavailableTakerTokenAmount = safeAdd(filled[orderHash], cancelled[orderHash]);
return unavailableTakerTokenAmount;
}
}

10
packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
View File

@@ -262,17 +262,11 @@ contract MixinWrapperFunctions is
/// @dev Synchronously cancels multiple orders in a single transaction.
/// @param orders Array of orders.
/// @param takerTokenCancelAmounts Array of desired amounts of takerToken to cancel in orders.
function batchCancelOrders(
Order[] orders,
uint256[] takerTokenCancelAmounts)
function batchCancelOrders(Order[] orders)
public
{
for (uint256 i = 0; i < orders.length; i++) {
cancelOrder(
orders[i],
takerTokenCancelAmounts[i]
);
cancelOrder(orders[i]);
}
}

6
packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
View File

@@ -30,11 +30,9 @@ contract MExchangeCore is LibOrder {
public
returns (uint256 takerTokenFilledAmount);
function cancelOrder(
Order order,
uint256 takerTokenCancelAmount)
function cancelOrder(Order order)
public
returns (uint256 takerTokenCancelledAmount);
returns (bool);
function cancelOrdersUpTo(uint256 salt)
external;

23
packages/contracts/src/utils/exchange_wrapper.ts
View File

@@ -34,17 +34,9 @@ export class ExchangeWrapper {
const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
return tx;
}
public async cancelOrderAsync(
signedOrder: SignedOrder,
from: string,
opts: { takerTokenCancelAmount?: BigNumber } = {},
): Promise<TransactionReceiptWithDecodedLogs> {
const params = orderUtils.createCancel(signedOrder, opts.takerTokenCancelAmount);
const txHash = await this._exchange.cancelOrder.sendTransactionAsync(
params.order,
params.takerTokenCancelAmount,
{ from },
);
public async cancelOrderAsync(signedOrder: SignedOrder, from: string): Promise<TransactionReceiptWithDecodedLogs> {
const params = orderUtils.createCancel(signedOrder);
const txHash = await this._exchange.cancelOrder.sendTransactionAsync(params.order, { from });
const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
return tx;
}
@@ -156,14 +148,9 @@ export class ExchangeWrapper {
public async batchCancelOrdersAsync(
orders: SignedOrder[],
from: string,
opts: { takerTokenCancelAmounts?: BigNumber[] } = {},
): Promise<TransactionReceiptWithDecodedLogs> {
const params = formatters.createBatchCancel(orders, opts.takerTokenCancelAmounts);
const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(
params.orders,
params.takerTokenCancelAmounts,
{ from },
);
const params = formatters.createBatchCancel(orders);
const txHash = await this._exchange.batchCancelOrders.sendTransactionAsync(params.orders, { from });
const tx = await this._getTxWithDecodedExchangeLogsAsync(txHash);
return tx;
}

6
packages/contracts/src/utils/formatters.ts
View File

@@ -55,10 +55,9 @@ export const formatters = {
});
return marketFillOrders;
},
createBatchCancel(signedOrders: SignedOrder[], takerTokenCancelAmounts: BigNumber[] = []) {
createBatchCancel(signedOrders: SignedOrder[]) {
const batchCancel: BatchCancelOrders = {
orders: [],
takerTokenCancelAmounts,
};
_.forEach(signedOrders, signedOrder => {
batchCancel.orders.push({
@@ -74,9 +73,6 @@ export const formatters = {
expirationTimeSeconds: signedOrder.expirationTimeSeconds,
salt: signedOrder.salt,
});
if (takerTokenCancelAmounts.length < signedOrders.length) {
batchCancel.takerTokenCancelAmounts.push(signedOrder.takerTokenAmount);
}
});
return batchCancel;
},

1
packages/contracts/src/utils/types.ts
View File

@@ -25,7 +25,6 @@ export interface MarketFillOrders {
export interface BatchCancelOrders {
orders: OrderStruct[];
takerTokenCancelAmounts: BigNumber[];
}
export interface CancelOrdersBefore {

69
packages/contracts/test/exchange/core.ts
View File

@@ -631,16 +631,6 @@ describe('Exchange', () => {
return expect(exWrapper.cancelOrderAsync(signedOrder, makerAddress)).to.be.rejectedWith(constants.REVERT);
});
it('should throw if takerTokenCancelAmount is 0', async () => {
signedOrder = orderFactory.newSignedOrder();
return expect(
exWrapper.cancelOrderAsync(signedOrder, makerAddress, {
takerTokenCancelAmount: new BigNumber(0),
}),
).to.be.rejectedWith(constants.REVERT);
});
it('should be able to cancel a full order', async () => {
await exWrapper.cancelOrderAsync(signedOrder, makerAddress);
await exWrapper.fillOrderAsync(signedOrder, takerAddress, {
@@ -651,75 +641,22 @@ describe('Exchange', () => {
expect(newBalances).to.be.deep.equal(balances);
});
it('should be able to cancel part of an order', async () => {
const takerTokenCancelAmount = signedOrder.takerTokenAmount.div(2);
await exWrapper.cancelOrderAsync(signedOrder, makerAddress, {
takerTokenCancelAmount,
});
const res = await exWrapper.fillOrderAsync(signedOrder, takerAddress, {
takerTokenFillAmount: signedOrder.takerTokenAmount,
});
const log = logDecoder.decodeLogOrThrow(res.logs[0]) as LogWithDecodedArgs<LogFillContractEventArgs>;
expect(log.args.takerTokenFilledAmount).to.be.bignumber.equal(
signedOrder.takerTokenAmount.minus(takerTokenCancelAmount),
);
const newBalances = await dmyBalances.getAsync();
const cancelMakerTokenAmount = takerTokenCancelAmount
.times(signedOrder.makerTokenAmount)
.dividedToIntegerBy(signedOrder.takerTokenAmount);
const makerFeePaid = signedOrder.makerFeeAmount
.times(cancelMakerTokenAmount)
.dividedToIntegerBy(signedOrder.makerTokenAmount);
const takerFeePaid = signedOrder.takerFeeAmount
.times(cancelMakerTokenAmount)
.dividedToIntegerBy(signedOrder.makerTokenAmount);
expect(newBalances[makerAddress][signedOrder.makerTokenAddress]).to.be.bignumber.equal(
balances[makerAddress][signedOrder.makerTokenAddress].minus(cancelMakerTokenAmount),
);
expect(newBalances[makerAddress][signedOrder.takerTokenAddress]).to.be.bignumber.equal(
balances[makerAddress][signedOrder.takerTokenAddress].add(takerTokenCancelAmount),
);
expect(newBalances[makerAddress][zrx.address]).to.be.bignumber.equal(
balances[makerAddress][zrx.address].minus(makerFeePaid),
);
expect(newBalances[takerAddress][signedOrder.takerTokenAddress]).to.be.bignumber.equal(
balances[takerAddress][signedOrder.takerTokenAddress].minus(takerTokenCancelAmount),
);
expect(newBalances[takerAddress][signedOrder.makerTokenAddress]).to.be.bignumber.equal(
balances[takerAddress][signedOrder.makerTokenAddress].add(cancelMakerTokenAmount),
);
expect(newBalances[takerAddress][zrx.address]).to.be.bignumber.equal(
balances[takerAddress][zrx.address].minus(takerFeePaid),
);
expect(newBalances[feeRecipientAddress][zrx.address]).to.be.bignumber.equal(
balances[feeRecipientAddress][zrx.address].add(makerFeePaid.add(takerFeePaid)),
);
});
it('should log 1 event with correct arguments', async () => {
const divisor = 2;
const res = await exWrapper.cancelOrderAsync(signedOrder, makerAddress, {
takerTokenCancelAmount: signedOrder.takerTokenAmount.div(divisor),
});
const res = await exWrapper.cancelOrderAsync(signedOrder, makerAddress);
expect(res.logs).to.have.length(1);
const log = logDecoder.decodeLogOrThrow(res.logs[0]) as LogWithDecodedArgs<LogCancelContractEventArgs>;
const logArgs = log.args;
const expectedCancelledMakerTokenAmount = signedOrder.makerTokenAmount.div(divisor);
const expectedCancelledTakerTokenAmount = signedOrder.takerTokenAmount.div(divisor);
expect(signedOrder.makerAddress).to.be.equal(logArgs.makerAddress);
expect(signedOrder.feeRecipientAddress).to.be.equal(logArgs.feeRecipientAddress);
expect(signedOrder.makerTokenAddress).to.be.equal(logArgs.makerTokenAddress);
expect(signedOrder.takerTokenAddress).to.be.equal(logArgs.takerTokenAddress);
expect(expectedCancelledMakerTokenAmount).to.be.bignumber.equal(logArgs.makerTokenCancelledAmount);
expect(expectedCancelledTakerTokenAmount).to.be.bignumber.equal(logArgs.takerTokenCancelledAmount);
expect(orderUtils.getOrderHashHex(signedOrder)).to.be.equal(logArgs.orderHash);
});
it('should not log events if no value is cancelled', async () => {
it('should log an error if already cancelled', async () => {
await exWrapper.cancelOrderAsync(signedOrder, makerAddress);
const res = await exWrapper.cancelOrderAsync(signedOrder, makerAddress);
@@ -729,7 +666,7 @@ describe('Exchange', () => {
expect(errCode).to.be.equal(ExchangeContractErrs.ERROR_ORDER_FULLY_FILLED_OR_CANCELLED);
});
it('should not log events if order is expired', async () => {
it('should log error if order is expired', async () => {
signedOrder = orderFactory.newSignedOrder({
expirationTimeSeconds: new BigNumber(Math.floor((Date.now() - 10000) / 1000)),
});

4
packages/contracts/test/exchange/wrapper.ts
View File

@@ -724,9 +724,7 @@ describe('Exchange', () => {
describe('batchCancelOrders', () => {
it('should be able to cancel multiple signedOrders', async () => {
const takerTokenCancelAmounts = _.map(signedOrders, signedOrder => signedOrder.takerTokenAmount);
await exWrapper.batchCancelOrdersAsync(signedOrders, makerAddress, {
takerTokenCancelAmounts,
});
await exWrapper.batchCancelOrdersAsync(signedOrders, makerAddress);
await exWrapper.batchFillOrdersAsync(signedOrders, takerAddress, {
takerTokenFillAmounts: takerTokenCancelAmounts,