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.

Sign up for GitHub

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

Jump to bottom

No maxsell/maxbuy recalculation for order replacement case #1999

Closed
raladev opened this issue Nov 19, 2020 · 2 comments · Fixed by #2002
Closed

No maxsell/maxbuy recalculation for order replacement case #1999

raladev opened this issue Nov 19, 2020 · 2 comments · Fixed by #2002
Assignees
@sangaman
Labels
bug Something isn't working P0 life and death priority
Milestone
1.2.2

Comments

@raladev
Copy link
Contributor

raladev commented Nov 19, 2020

Screenshot from 2020-11-20 00-07-56

Case:

  1. Placed order blocks part of max sell/ max buy amount.
  2. But when user tries to replace this orders, xud uses reserved amount of order that should be replaced in tradinglimits calculation.
  3. As a resultm user cant place order

Example:

  1. max sell ETH = 1 ETH, max buy BTC = 0.1 BTC
  2. placeorder sell 0.95 ETH/BTC 0.1 my-id-> order placed
  3. max sell ETH = 1 - 0,95 = 0.05, max buy BTC = 0.1 - 0,095 = 0.005 BTC
  4. placeorder sell 0.948 ETH/BTC 0.1 -r my-id -> err because of max sell and max buy

Expected behavior:
On step 4 (on trading limits check) we should increase maxsell/maxbuy values by 0.95 ETH/ 0.095 BTC (amount that were used in old order).
@raladev raladev added bug Something isn't working P1 top priority labels Nov 19, 2020
@raladev raladev assigned ghost , sangaman and kilrau Nov 19, 2020
@kilrau
Copy link
Contributor

kilrau commented Nov 20, 2020

Related (but different): https://github.com/ExchangeUnion/xud/issues/1991

@kilrau kilrau unassigned ghost and kilrau Nov 20, 2020
@kilrau kilrau added this to the 1.2.2 milestone Nov 20, 2020
@kilrau
Copy link
Contributor

kilrau commented Nov 20, 2020

@sangaman the problem in simple words seems to be: when replacing an order we currently do not remove reserved order amounts of the old order from tradinglimits, which is when the channel capacity is not large enough to cover the second new order, replacing fails.

Fix should be as simple as removing reserved quantity from tradinglimits as intermediate step in the replace process.

@kilrau kilrau added P0 life and death priority and removed P1 top priority labels Nov 23, 2020
sangaman added a commit that referenced this issue Nov 23, 2020
@sangaman
fix: limits account for replaced order quantity
d624c80 
This fixes a bug in the logic to calculate whether a request to place
a new order exceeds the available capacity and trading limits by
properly accounting for the quantity that is being replaced by the new
order request. Previously, this was not accounted for and attempting
to replace an order, even if not changing quantity, could result in
an error due to exceeding the trading capacity.

Closes #1999.
@sangaman sangaman mentioned this issue Nov 23, 2020
Merged
raladev pushed a commit that referenced this issue Nov 23, 2020
@sangaman
fix: limits account for replaced order quantity (#2002)
eadf5d1 
This fixes a bug in the logic to calculate whether a request to place
a new order exceeds the available capacity and trading limits by
properly accounting for the quantity that is being replaced by the new
order request. Previously, this was not accounted for and attempting
to replace an order, even if not changing quantity, could result in
an error due to exceeding the trading capacity.

Closes #1999.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sangaman sangaman

Labels
bug Something isn't working P0 life and death priority
Projects
None yet
Milestone
1.2.2
Development

Successfully merging a pull request may close this issue.

fix: limits account for replaced order quantity ExchangeUnion/xud
3 participants
@sangaman @kilrau @raladev