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

tradinglimits should account for "reserved balance" #1678

Closed
kilrau opened this issue Jun 18, 2020 · 2 comments · Fixed by #1933
Closed

tradinglimits should account for "reserved balance" #1678

kilrau opened this issue Jun 18, 2020 · 2 comments · Fixed by #1933
Assignees
@sangaman
Labels
grpc gRPC API P1 top priority

Comments

@kilrau
Copy link
Contributor

kilrau commented Jun 18, 2020

How it is

Currently, calling tradinglimites calculates & displays total inbound and total outbound channel capacity for a respective assets' channel:

simnet > tradinglimits

Trading Limits:
┌──────────┬───────────────────┬──────────┐
│ Currency │ Max Buy           │ Max Sell │
├──────────┼───────────────────┼──────────┤
│ BTC      │ 2.5               │ 2.5      │
├──────────┼───────────────────┼──────────┤
│ ETH      │ 90071992.54740991 │ 10       │
├──────────┼───────────────────┼──────────┤
│ LTC      │ 5                 │ 5        │
└──────────┴───────────────────┴──────────┘

How it should be

Max Buy/Max sell is reduced by quantity reserved quantity for open orders. We have a separate issue for calculating "reserved balance" and displaying it under getbalance already: #1584
@kilrau kilrau added P2 mid priority P3 low priority and removed P2 mid priority labels Jun 18, 2020
@sangaman sangaman self-assigned this Oct 14, 2020
@sangaman sangaman added the grpc gRPC API label Oct 14, 2020
@sangaman
Copy link
Collaborator

As just discussed, the plan for resolving this issue is:

  1. Remove reserved order amounts from GetBalance - reversing Reserved Balance (show balance reserved by open orders separately) #1584.

  2. Subtract reserved order amounts from the max buy and max sell amounts on the TradingLimits call, e.g. if we have outbound capacity of 1.2 ETH, but we have an order to sell 0.7 ETH, our max sell would be 0.5 ETH.

  3. Return reserved order amounts on the TradingLimits rpc call.

  4. Reject orders that would put our total reserved balance over our total capacity. An exception to this would be for inbound limits for Connext currencies, since we have the ability to dynamically request additional inbound collateral via our "lazy collateral" approach.

  5. Revisit the logic used for accepting market orders as it pertains to our trading limits, we want to ensure that market orders won't make it so that they don't leave us with insufficient balance to cover our outstanding orders.

I'm going to start on this ASAP, points 1-3 above are informational in nature whereas 4-5 changes the way we validate new order placement requests. I'll probably wind up implementing those in separate PRs.

@kilrau kilrau mentioned this issue Oct 14, 2020
Closed
@kilrau kilrau added P1 top priority and removed P3 low priority labels Oct 14, 2020
@sangaman
Copy link
Collaborator

I'm going to open a separate issue for 4 & 5, this issue will just track returning the proper values for the TradingLimits call but not how we enforce limits on the PlaceOrder call.

sangaman added a commit that referenced this issue Oct 15, 2020
@sangaman
feat(rpc): reserved order amount for TradingLimits
53cb231 
This exposes the amounts reserved for open orders on the `TradingLimits`
call and subtracts reserved amounts from the `maxSell` and `maxBuy`
values returned by the call.

It also removes reserved order amounts from GetBalance - reversing #1584.

The logic to calculate max sell and buys is as follows:

1. Sum up all outbound and inbound balances across all channels, as well
as the maximum balances for a single channel, as we still aren't
expecting to use multi-path payments. For Connext there is a single
channel so we only have a single inbound & outbound value per currency.

2. Subtract reserved order amounts from the total inbound and outbound
balances calculated above. E.g. if we have outbound capacity of 1.2 ETH,
but we have an order to sell 0.7 ETH, our max sell would be 0.5 ETH.

3. Return the smaller of the amount calculated in step 2 or the maximum
single channel balance calculated in step 1.

Resolves #1678.
@sangaman sangaman mentioned this issue Oct 15, 2020
Merged
sangaman added a commit that referenced this issue Oct 15, 2020
@sangaman
feat(rpc): reserved order amount for TradingLimits
7f7cd68 
This exposes the amounts reserved for open orders on the `TradingLimits`
call and subtracts reserved amounts from the `maxSell` and `maxBuy`
values returned by the call.

It also removes reserved order amounts from GetBalance - reversing #1584.

The logic to calculate max sell and buys is as follows:

1. Sum up all outbound and inbound balances across all channels, as well
as the maximum balances for a single channel, as we still aren't
expecting to use multi-path payments. For Connext there is a single
channel so we only have a single inbound & outbound value per currency.

2. Subtract reserved order amounts from the total inbound and outbound
balances calculated above. E.g. if we have outbound capacity of 1.2 ETH,
but we have an order to sell 0.7 ETH, our max sell would be 0.5 ETH.

3. Return the smaller of the amount calculated in step 2 or the maximum
single channel balance calculated in step 1.

Resolves #1678.
@ghost ghost closed this as completed in #1933 Oct 15, 2020
@sangaman sangaman mentioned this issue Oct 21, 2020
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sangaman sangaman

Labels
grpc gRPC API P1 top priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(rpc): reserved order amount for TradingLimits ExchangeUnion/xud
2 participants
@sangaman @kilrau