aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol57
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol10
-rw-r--r--packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol6
-rw-r--r--packages/contracts/src/utils/exchange_wrapper.ts23
-rw-r--r--packages/contracts/src/utils/formatters.ts6
-rw-r--r--packages/contracts/src/utils/types.ts1
-rw-r--r--packages/contracts/test/exchange/core.ts69
-rw-r--r--packages/contracts/test/exchange/wrapper.ts4
8 files changed, 35 insertions, 141 deletions
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
index acbed1ad7..a0603ec78 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinExchangeCore.sol
@@ -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;
- }
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
index 358649aa1..39f4784cb 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/MixinWrapperFunctions.sol
@@ -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]);
}
}
diff --git a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
index 6768e2376..a495434e6 100644
--- a/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
+++ b/packages/contracts/src/contracts/current/protocol/Exchange/mixins/MExchangeCore.sol
@@ -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;
diff --git a/packages/contracts/src/utils/exchange_wrapper.ts b/packages/contracts/src/utils/exchange_wrapper.ts
index 3f56fd52f..bbe9a1baf 100644
--- a/packages/contracts/src/utils/exchange_wrapper.ts
+++ b/packages/contracts/src/utils/exchange_wrapper.ts
@@ -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;
}
diff --git a/packages/contracts/src/utils/formatters.ts b/packages/contracts/src/utils/formatters.ts
index 4f6116f06..2b261f967 100644
--- a/packages/contracts/src/utils/formatters.ts
+++ b/packages/contracts/src/utils/formatters.ts
@@ -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;
},
diff --git a/packages/contracts/src/utils/types.ts b/packages/contracts/src/utils/types.ts
index 06fe23824..460d0a58a 100644
--- a/packages/contracts/src/utils/types.ts
+++ b/packages/contracts/src/utils/types.ts
@@ -25,7 +25,6 @@ export interface MarketFillOrders {
export interface BatchCancelOrders {
orders: OrderStruct[];
- takerTokenCancelAmounts: BigNumber[];
}
export interface CancelOrdersBefore {
diff --git a/packages/contracts/test/exchange/core.ts b/packages/contracts/test/exchange/core.ts
index 69d059256..3919d43d8 100644
--- a/packages/contracts/test/exchange/core.ts
+++ b/packages/contracts/test/exchange/core.ts
@@ -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)),
});
diff --git a/packages/contracts/test/exchange/wrapper.ts b/packages/contracts/test/exchange/wrapper.ts
index 02e57b922..b87f45387 100644
--- a/packages/contracts/test/exchange/wrapper.ts
+++ b/packages/contracts/test/exchange/wrapper.ts
@@ -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,