nathanielhourt commented 5 years ago

BSIP: 0061
Title: Operation to Update Limit Orders
Authors:
  Nathan Hourt <nat.hourt@gmail.com>
Contributors and Reviewers:
  TBD
Status: Draft
Type: Protocol
Created: 2019-02-22
Discussion: https://github.com/bitshares/bsips/issues/150
Worker: Unpaid

Abstract

This BSIP proposes a new operation, limit_order_update_operation, which will allow direct modification of a limit order. This operation will be smaller to serialize than the cancel/create operation combination presently required to achieve the same result, as it must only store new field values rather than all field values. Moreover, the update operation will require lower processing and RAM consumption. In return for this reduction in consumption, the overall fee charged for an update operation can be set lower than the equivalent cancel/create combo, thus incentivizing market makers to keep freshly updated orders at the top of the books.

Motivation

At present, the BitShares protocol does not allow direct modification of limit orders in the markets. Instead, to modify a limit order, the order must be canceled and and a new one created in its place. This results in greater-than-necessary overhead when updating a limit order, an action which is performed by market maker robots with great frequency (hundreds of times per day, per market). Additionally, the cancel/create combination requires payment of two fees, which has led to some pressure from market makers for BitShares to provide a cheaper mechanism for maintaining fresh order walls in the markets.

Rationale

The specification below was chosen on the following criteria:

Minimal storage space for historical data (i.e. serialized operation size)
- Cancel/create combination requires two distinct operations, and explicit restatement of all order parameters
- Update operation requires only one operation, which elides all fields which are not being updated
Minimal processing
- No deletion/construction is necessary, object is modified in place
- Cancel/create requires two separate modifications of balances; update requires zero or one
- Order matching is only triggered if order is moved past previous top of book
Minimal RAM consumption for undo storage
- Object modification requires slightly less space than object deletion and creation
- Balances and account statistics are not updated unless necessary, avoiding unnecessary undo storage
Finer granularity of fees
- Low fee if only the order object is modified
- Higher fee if balances must be updated as well

Specification

A complete implementation of this BSIP is available for review here.

The limit_order_update_operation contains the following fields:

       asset fee;
       account_id_type seller;
       limit_order_id_type order;
       optional<price> new_price;
       optional<share_type> delta_amount_to_sell;
       optional<time_point_sec> new_expiration;

       extensions_type extensions;

A single new evaluator is required, which shall check that the new field values are valid for the order being updated, and update the order and balances as necessary, then trigger order matching only if the price was modified such that the order has moved past the previous top-of-book order.

No new indexes are required. No modifications to existing indexes are required. No new database objects are defined. No modifications to existing database objects are required.

Discussion and Summary for Shareholders

The changes proposed in this BSIP are easily implemented, and the only downside apparent to the author is the definition of a new operation type, which slightly increases the complexity of creating a new implementation of the BitShares protocol. Thus, the question must be considered, is the new operation of significant value to the community, and will it be used?

The new operation is of greatest value to market makers, as these individuals typically run robots which must monitor the markets and maintain competitively priced orders on both sides of the market. These orders must be updated regularly, especially for BitAsset markets. The addition of an operation to modify limit orders will reduce the cost of market making, thus increasing the incentive for users to provide this service. Additionally, it reduces the requirements for processing and storing market maker activity.

In conclusion, this BSIP proposes a very small modification to the protocol with minimal downside, and a meaningful upside for market makers and reduced requirements for all nodes which process the markets.

Copyright

Intellectual Property is BS. Those who wish to pretend otherwise are hereby directed to treat this document as public domain.

Actually, delta_amount_to_sell complicates things a bit, at least with a negative delta, because we'll have to check if the remainder is to be considered dust. Since the order is cancelled anyway if the remainder is considered dust, this would allow abuse as a cheaper version of limit_order_cancel. Perhaps we should only allow increasing the amount to avoid this problem?

ryanRfox commented 5 years ago

Assigned as BSIP61
@nathanhourt suggesting to update the BSIP Discussion URL in the header to "https://github.com/bitshares/bsips/issues/150"
Suggesting to include Issue 1604 in the (to-be-added) "See Also" section near the end of the BSIP (reference BSIP Template)
Suggesting further drafting of the specification in this Issue, then return to 1604 for implementation
I will update the README.md shortly

pmconrad commented 5 years ago

I've been thinking about the dust issue and realized that the dust check will also have to be applied when the price is modified. So to avoid abuse we should disallow the order to be auto-cancelled after update.

Another question: is it better to specify the new amount_to_sell as an absolute value or as a delta? In other words, what is the desired behaviour if the existing order gets partially filled before the update enters the chain?

nathanielhourt commented 5 years ago

I can add a dust-check, if needed; how should such a check work?

I did consider using new_amount_to_sell rather than a delta, but I went with the delta because the first thing the backend has to do with a flat amount is calculate the delta anyways. I figured we could just push that step to the frontend. I also suspect that the delta will generally be a smaller number than the flat amount, which might translate to fewer bytes in the serial operation.

And to your last question, I think a delta is more consistent with the principle of least surprise in the case of the order getting partially filled in the last moment before the update hits the chain. If I specify a new amount to go into the order which is only slightly off from the old amount, and then right before that processes, my order gets almost completely filled, the chain will automatically withdraw the entire deficit from my account to fill the order all the way back up to my specified new amount. This could be confusing to the user, who had more money taken from his account than he expected.

I recall also a principle in BitShares operation design that the adjustment to an account's balance is always explicit from the operation directly, without examining chain state. A delta fulfills that, but a new amount does not.

As to using a share_type rather than an asset, great point; I'll update the spec.

pmconrad commented 5 years ago

I can add a dust-check, if needed; how should such a check work?

https://github.com/bitshares/bitshares-core/blob/2.0.190219/libraries/chain/db_market.cpp#L301

I agree that the delta is better. I can imagine that a market maker might want to update the order to a new absolute value, independent from whether it was previously filled or not though. He can just do another update then of course.

nathanielhourt commented 5 years ago

So... funny thing about using share_type rather than asset... doing so violates the principle I just described, the ability to determine the adjustment to the account balance by looking at just the operation and not requiring chain state. You can't tell what the adjustment to the account balance is by looking at the operation because you don't know what asset is being moved unless you look at chain state. :man_facepalming:

So in light of the above, do we consider it better to use the couple bytes for the asset and comply with the principle?

pmconrad commented 5 years ago

Since this is not the only place where we have redundancy I think it's better to uphold the principle.

nathanielhourt commented 5 years ago

Agreed.

Vis-a-vis the dust check, it seems the actual check is order.amount_to_receive().amount == 0 -- so should the check simply be that if the operation specifies a negative delta amount, assert that after applying the delta, the order's amount_to_receive() should not be zero?

nathanielhourt commented 5 years ago

Added dust check: https://github.com/nathanhourt/bitshares-2/commit/1c81eaf01c7cbebffb5500c570525868048ba264

Dust check: if either the price or the amount is updated, check that the new amount to receive will be nonzero.

xeroc commented 5 years ago

For sake for discussion, can we look into implications for order creation refund? Does it make a difference whether to

cancel an existing order, or
set the expiration time to few seconds to have it expire?

abitmore commented 5 years ago

Need to describe fee refunding mechanism. See https://github.com/bitshares/bsips/blob/master/bsip-0026.md

nathanielhourt commented 5 years ago

As @pmconrad has noted, the intent of this operation is not to allow a cheaper way to cancel orders. For this reason, perhaps we should require that, if new_expiration is set, it must be later than the old one? So you can update your order to expire later, but not sooner?

This would make sense, as the intended user of this operation is market makers, who probably place orders with near expirations (possibly as short as 5 minutes) as these orders are automatically generated and the maker would not want the order to remain on the books after they go stale, in the event that the bot crashes/etc. When the maker updates the order, they may wish to bump the expiration back another 5 minutes, but they would be unlikely to desire to move the expiration closer.

Shall I add a requirement that the new expiration be later than the old one?

nathanielhourt commented 5 years ago

Additionally, do we wish to accumulate fees from order updates into the deferred fees for later potential refund?

For my part, I expect this operation to be set with a very low processing fee which is non-refundable; however, if there is substantial consensus desiring a (presumably higher) refundable fee, I'll be happy to implement it.

abitmore commented 5 years ago

I don't think updating expiration matters.

I expect this operation to be set with a very low processing fee which is non-refundable

I agree. Actually, amount of fees of the "cancel/create operation combination" mentioned in OP is single cancellation fee since creation fee would be refunded, thus the description is misleading.

pmconrad commented 5 years ago

Shall I add a requirement that the new expiration be later than the old one?

Sounds good.

I expect this operation to be set with a very low processing fee which is non-refundable

Also agree.

abitmore commented 5 years ago

Shall I add a requirement that the new expiration be later than the old one?

Sounds good.

Why not allow it? It's legit (imho) that a user wants to free some RAM earlier, she won't pay less for doing so thus no harm to the chain.

abitmore commented 5 years ago

What's the rationale of having a high balance-updating fee but a low price-updating fee?

My take:

if an order has been partially filled, to update it, the user should pay a fee which is around order creation fee, to avoid changing a dust order to a new order with less cost;
if an order has not been partially filled, to update it, the user should pay a fee which is around order cancellation fee.

When signing the transaction, since the user don't know chain state, she should pay order creation fee, the chain can refund immediately when applying the operation, if found the updating order hasn't been filled.

When there is a fee schedule change after an order is created, when updating the order, ideally new fee schedule should apply, that said, we can refund the original order creation fee then charge the updating fee (defer it).

nathanielhourt commented 5 years ago

What's the rationale of having a high balance-updating fee but a low price-updating fee?

Updating the price and/or expiration requires only modifying the order object. Updating the balance requires also updating the account balance, and possibly the account statistics objects as well.

if an order has been partially filled, to update it, the user should pay a fee which is around order creation fee, to avoid changing a dust order to a new order with less cost;

But changing a dust order to a new order should cost less than creating a new order... The operation is smaller, and it recycles already-available RAM rather than allocating new space on the heap.

The purpose of this proposal is to make market making more efficient, both in terms of blockchain processing, and in terms of fees paid. Those who will use this operation will probably execute it well over a thousand times per day. It makes good sense to streamline their operations for lighter processing, as well as to give them a lower fee for being bulk buyers. They're our biggest customers -- we ought to treat them well.

abitmore commented 5 years ago

On the other hand, they're already bloating the chain due to the low fee. While they're earning money, the chain is being operated at a loss. Consider the chain as a for-profit business but not a charity, fees should be on a certain level.

But changing a dust order to a new order should cost less than creating a new order... The operation is smaller, and it recycles already-available RAM rather than allocating new space on the heap.

The chain charges the first fill of every order an order creation fee, this is the main income from order processing. When a partially filled order (dust) is cancelled then replaced, it means new potential income. If we allow partially filled order to be updated/renewed with a cheaper fee, it means less income when the order is filled next time.

pmconrad commented 5 years ago

The idea here is that market making improves liquidity, better liquidity attracts more traders, which boosts volume, which generates more fees.

Presumably a market maker will mostly cancel and re-create unfilled limit orders, which means the chain earns 1 cancel fee for an expensive limit_order_create and a cheap limit_order_cancel, whereas limit_order_update will be much cheaper to validate + execute than the other two and take less bandwidth. Note that even if the order has been partially filled there has been at least one other order created and fully filled, which earns the chain at least one limit_order_create fee.

abitmore commented 5 years ago

Anyway, I've never seen such a feature in traditional exchanges, although in some exchanges it's possible to update an order's size to smaller (but no price change). That said, (I guess) this feature is not common, thus introducing it may probably confuse average traders. How would the UI look like? @clockworkgr @sschiessl-bcp

sschiessl-bcp commented 5 years ago

@sschiessl-bcp For the UI I imagine that you have an edit button in open orders, that pops out the Buy component as a modal

pmconrad commented 5 years ago

Since it is mostly meant for market-makers (who will be running bots), it need not be present in the UI. Except if the UI team want to have it.

cloud-8 commented 5 years ago

A great addition to the protocol, this is often used on many of the popular CEX's and would make Bitshares more attractive to active traders.

sschiessl-bcp commented 5 years ago

Since it is mostly meant for market-makers (who will be running bots), it need not be present in the UI. Except if the UI team want to have it.

UI team wants to support all calls, yet this is certainly targeting bots. A traditional exchange also would need this as order replacing doesn't cost anything.

sschiessl-bcp commented 5 years ago

Should we forward this into a PR?

abitmore commented 5 years ago

@sschiessl-bcp shared an article https://hackernoon.com/the-ideal-crypto-trading-api-b1bbb2675875. IMHO it means it's better to have a "replace" operation rather than an "update" operation, which basically combines a limit_order_cancel_operation and a limit_order_create_operation into one operation. With this approach, we can reuse most of current code, and logic about fees would be clear as well.

abitmore commented 5 years ago

Instead of adding a new operation, we can add an extension to limit_order_create_operation to indicate which order to be replaced.

block-chained commented 5 years ago

Is there a possibility to add "lock order" capability, so for example a user can place an order that cannot be revoked? E.g. to provide trustless automated token-to-token exchange cmart contract.

nathanielhourt commented 5 years ago

If you're looking for an exchange smart contract with predefined parties, you probably want HTLCs, which are currently supported. An irrevocable market order could be taken by anyone in the market, which is different.

An irrevocable order is certainly possible; it could be added if there is sufficient support for the idea, but it would require a separate BSIP.

ryanRfox commented 4 years ago

@nathanhourt this feels "done" to me (leaving out the bits about irrevocable order, which needs a distinct discussion). Please PR with your latest text.

abitmore commented 4 years ago

Draft (#215) merged. Closing.

bitshares / bsips

BSIP61: Operation to Update Limit Orders #150

Abstract

Motivation

Rationale

Specification

Discussion and Summary for Shareholders

Copyright

See Also