Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for mass order cancellations based on salt value #20

Closed
abandeali1 opened this issue Jan 12, 2018 · 2 comments
Closed

Allow for mass order cancellations based on salt value #20

abandeali1 opened this issue Jan 12, 2018 · 2 comments

Comments

@abandeali1
Copy link
Member

abandeali1 commented Jan 12, 2018

Summary

This change would allow traders to very cheaply cancel multiple orders in a single transaction with a single state update.

Motivation

Currently, each individual order must be cancelled separately, which can be very expensive if required for a large amount of orders. This can prevent market makers from creating multiple orders for fear of large and sudden market moves.

In addition, if a trader forgets or loses their old orders, it is currently not possible to cancel them. This is potentially dangerous, since there is always a chance that other parties are still storing the orders.

Specification

Traders SHOULD treat the salt order field as a timestamp with arbitrary precision. Then we can add the following functionality:

mapping (address => uint256) cancelledBefore;   // mapping of maker => timestamp

function cancelOrdersBefore(uint256 timestamp)
    external
{
    require(cancelledBefore[msg.sender] < timestamp);   // removing this check allows makers to "pause" orders, but adds complexity
    cancelledBefore[msg.sender] = timestamp;
}

function fillOrder(...)
    public
    returns (uint256)
{
    require(order.salt > cancelledBefore[order.maker]);
    // remaining fill logic
}

Note that while it is in the best interest of makers to treat an order's salt as a timestamp, there is no way to enforce this and it should therefore NOT be relied upon for any other functionality (doing so would create incentives for makers to provide inaccurate timestamps).

It would also be useful to allow makers to cancel orders of specific token pairs, but this may add too much extra complexity.

@dekz
Copy link
Member

dekz commented Jan 16, 2018

I like the efficiency this allows.

Since salt is quite an ambiguous term, often not implying any order or structure (same can be said for nonce) and often assumes random, I'm concerned about an account getting stuck at MAX_INT, and not being able to fix it (due to developer error). This might be enough of a concern to allow the user to reset their cancelledBefore back to 0x0.

This salt format would follow the previously used expirationTimestampInSec, or is that not granular enough? If the Relayer choses this could be an issue.

@BMillman19
Copy link

If this requires a change a redeployment of the exchange contract, why not just add it as a new field in the order format?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants