xud
xud copied to clipboard
ExchangeUnion
feat: adding new flag to trade all possible quantity for buy/sell
I'm not able to issue a market buy all order when the peer order's offered quantity is higher than we can fill:
➜ xud git:(feature/buy-sell-all-grpc) ✗ ./bin/xucli orderbook BTC/DAI Trading pair: BTC/DAI ┌───────────────────────────────────────┬───────────────────────────────────────┐ │ Buy │ Sell │ ├───────────────────┬───────────────────┼───────────────────┬───────────────────┤ │ Quantity │ Price │ Price │ Quantity │ ├───────────────────┼───────────────────┼───────────────────┼───────────────────┤ │ │ │ 100000 │ 0.02 │ └───────────────────┴───────────────────┴───────────────────┴───────────────────┘ ➜ xud git:(feature/buy-sell-all-grpc) ✗ ./bin/xucli getbalance Balance: ┌──────────┬───────────────┬────────────────────────────┬───────────────────────────────┐ │ Currency │ Total Balance │ Channel Balance (Tradable) │ Wallet Balance (Not Tradable) │ ├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤ │ BTC │ 465.55099411 │ 0.08608198 │ 465.46491213 │ ├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤ │ DAI │ 6000 │ 1500 │ 4500 │ ├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤ │ ETH │ 34.99859449 │ 10 │ 24.99859449 │ ├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤ │ USDT │ 2000 │ 0 │ 2000 │ └──────────┴───────────────┴────────────────────────────┴───────────────────────────────┘ ➜ xud git:(feature/buy-sell-all-grpc) ✗ ./bin/xucli buy all BTC/DAI mkt 9 FAILED_PRECONDITION: DAI outbound balance of 150000000000 is not sufficient for order amount of 200000000000
How should we behave for such cases? I thought it was okay, also are you able to do same without all command, by specifying a quantity?
How should we behave for such cases?
We should place an order with 1500 DAI, filling the existing order partially.
How should we behave for such cases?
We should place an order with 1500 DAI, filling the existing order partially.
Sorry if I overlook something but how, in the orderbook the price is 100.000 DAI for 0.02 BTC (2000 DAI), when we try to buy it all with 6.000 DAI (our balance) even actually we have 1.500 channel balance, we can't fill it, we don't have enough funds, so it throws the error, also it comes from the underlying buy command, nothing I changed.
Looks there was a misunderstanding what issue #1877 is supposed to do: it is not about buying all available quantity in the order book, but about
issue an order with the max possible quantity
(that your balance allows).
Todo:
- [x] rename
alltomax - [x]
sell max BTC/DAI mktin above example is equivalent tosell 0.08608198 BTC/DAI mkt - [x]
sell max BTC/DAI 10123in above example is equivalent tosell 0.08608198 BTC/DAI 10123 - [x]
buy max BTC/DAI mktin above example is equivalent tobuy 0.015 BTC/DAI mkt - [x]
buy max BTC/DAI 10123in above example is equivalent tobuy 0.14818 BTC/DAI 10123
The behaviour of the order type limit or market is unchanged, e.g. remainder of market orders being discarded, remainder of limit orders entering the order book. Let's ignore routing fees for the moment, still have to think how to tackle these.
Implemented below queries with tests;
rename all to max sell max BTC/DAI mkt in above example is equivalent to sell 0.08608198 BTC/DAI mkt sell max BTC/DAI 10123 in above example is equivalent to sell 0.08608198 BTC/DAI 10123 buy max BTC/DAI mkt in above example is equivalent to buy 0.015 BTC/DAI mkt
But for last one:
buy max BTC/DAI 10123 in above example is equivalent to buy 0.14818 BTC/DAI mkt
Does it mean market really? Or was it a typo? @kilrau
Looks like a typo.
It should be equivalent of buy (max_btc_qty_that_depends_on_my_current_dai_balance AND order_price) BTC/DAI price - buy 0.14818 BTC/DAI 10123
-
[x] u should not use channel balance from
getbalancefor max LTC/BTC amount because it does not contain reserved amount. use tradinglimits instead of that. Screenshot from 2020-10-05 14-20-59 -
[x] something wrong with qty calculation for fourth case - it is halved for each next trade (see orderbook and calculated qty). Screenshot from 2020-10-05 15-16-27 max_xud.log max_utils.log
-
[x] for fourth case u should not determine qty as
min{max_available_from_my_qty, max_available_in_orderbook}, u should always use max_available_from_my_qty. Why? just to have no exhausted logic, buy/sell command will do everything that necessary and u dont need to use max available qty in orderbook as a bound. WDYT @kilrau @rsercano ?
For now in following case u will place buy 0.002 BTC/DAI mkt instead of buy 0.015 BTC/DAI mkt
Trading pair: BTC/DAI
┌───────────────────────────────────────┬───────────────────────────────────────┐
│ Buy │ Sell │
├───────────────────┬───────────────────┼───────────────────┬───────────────────┤
│ Quantity │ Price │ Price │ Quantity │
├───────────────────┼───────────────────┼───────────────────┼───────────────────┤
│ │ │ 100000 │ 0.002 │
└───────────────────┴───────────────────┴───────────────────┴───────────────────┘
➜ xud git:(feature/buy-sell-all-grpc) ✗ ./bin/xucli getbalance
Balance:
┌──────────┬───────────────┬────────────────────────────┬───────────────────────────────┐
│ Currency │ Total Balance │ Channel Balance (Tradable) │ Wallet Balance (Not Tradable) │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ BTC │ 465.55099411 │ 0.08608198 │ 465.46491213 │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ DAI │ 6000 │ 1500 │ 4500 │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ ETH │ 34.99859449 │ 10 │ 24.99859449 │
├──────────┼───────────────┼────────────────────────────┼───────────────────────────────┤
│ USDT │ 2000 │ 0 │ 2000 │
└──────────┴───────────────┴────────────────────────────┴───────────────────────────────┘
~
-
[ ] qty is 0 if orderbook is empty as a result i got incorrect error Screenshot from 2020-10-05 14-36-01
-
[ ] also,it is first time when i see [SERVICE] in logs and it seems this log should be moved to trace level (on same level with placeorder call).
Thanks for checking @raladev, but I'm a bit confused:
-
I changed balance with tradinglimits.
-
For third case (I guess you meant third instead of fourth), here's the calculation:
- Iterate over orderbook (because this's a mkt buy and we must know what we can buy)
- For each order starting from the cheapest one;
- Calculate
maxBuyableFromThisPriceif order quantity in the order was infinitive by:currentBalance/order.price - Calculate buyable amount by taking account the order's quantity in the orderbook by:
(maxBuyableFromThisPrice > order.quantity) ? order.quantity : maxBuyableFromThisPrice. Which shortly means if our max buyable is higher than order's quantity then use order's quantity otherwise use maximum buyable quantity which has been calculated in the previous step with our balance. - Sum calculated quantity.
- Subtract
order.price * calculatedQuantityfromcurrentBalancefor next iteration. - End iteration if
currentBalance === 0
- Calculate
-
For now in following case u will place buy 0.002 BTC/DAI mkt instead of buy 0.015 BTC/DAI mkt I tried this again but this's not correct, it places 0.015 BTC/DAI.
-
qty is 0 if orderbook is empty as a result i got incorrect error. What do you mean? I didn't change the buy/sell process.
-
also,it is first time when i see [SERVICE] in logs and it seems this log should be moved to trace level (on same level with placeorder call). You're right. I'll remove it as soon as we complete our tests, and this PR is ready to merge.
qty is 0 if orderbook is empty as a result i got incorrect error.
What do you mean? I didn't change the buy/sell process
max flag for buy max DAI/USDT mkt when orderbook is empty means buy 0 DAI/USDT mkt for now
100 sat error is correct for buy 0 DAI/USDT mkt
BUT it is not correct buy max DAI/USDT mkt because i as a user did not set any sat amount, So that should be another err.
Which one? Something like max flag can not be used for market order now because order books is empty @kilrau @rsercano
Sounds good to me @raladev what about other points?
finished with retesting.
u should not use channel balance from getbalance for max LTC/BTC amount because it does not contain reserved amount. use tradinglimits instead of that.
Fixed
something wrong with qty calculation for fourth case - it is halved for each next trade (see orderbook and calculated qty).
Fixed
I tried this again but this's not correct, it places 0.015 BTC/DAI.
Yeap, my mistake. marked this point as Done.
- [ ] Also there is new bug for buy call (for sell is ok) - u should use max_buy btc bound for that.
In case of BTC/USDT (Lnd/Connext) u should use max buy btc value for buy order and max sell value for sell order AND u cant use max buy bound of USDT for calculation because it is connext currency and this bound is not real bound (it can be increased by collateralization request)
sell max BTC/USDT 10000 - use max sell bound for btc buy max BTC/USDT 10000 - use min(max amount of BTC that i can get using max sell bound for USDT, buy max BTC bound ) BTC
In case of ETH/BTC (Connext/Lnd): sell max ETH/BTC 0.0001 - use min(max amount of ETH that i can get using max buy bound for BTC, max sell bound for eth ) ETH buy max ETH/BTC 0.0001 - use max sell bound for btc to calculate eth amount
In case of USDT/DAI (Connext/Connext): sell max USDT/DAI 2 - use max sell bound for usdt buy max USDT/DAI 2 - use max sell bound for DAI to calculate USDT amount
In case of LTC/BTC (Lnd/Lnd): sell max LTC/BTC 200 - use min(max amount of LTC that i can get using max buy bound for BTC, sell max LTC bound ) LTC buy max LTC/BTC 200 - use min(max amount of LTC that i can get using max sell bound for BTC, buy max LTC bound ) LTC
Why we cant use min-max formula for connext currencies? Becaouse Xud does not know about real buy bound for connext asset, but we can wait https://github.com/connext/rest-api-client/issues/98 and after that we can use max collateral amount in min-max as a buy bound for Connext asset (but not as a value in tradinglimits).
But even if we will wait for it, there is also a case with nobalancechecks flag:
when i have no this flag my orders will be blocked by max collateral amount (15 ETH), but if i have this flag there is not collateral check and i can use amount bigger then max collateral amount (and swap will be successful), so for this case our max buy ETH bound is free balance of indra node.
simnet > sell 25 ETH/BTC 0.01
matched 25 ETH @ 0.01 with peer MovieSuspect order e49fa580-07f2-11eb-9165-e76ad59ebc21, attempting swap...
successfully swapped 25 ETH with peer order e49fa580-07f2-11eb-9165-e76ad59ebc21
simnet > getbalance
So, We need to discuss how we want to handle this @rsercano @kilrau
Hello @raladev this was way more complicated than first I thought but eventually I guess I've done with the implementation, currently it only misses nobalancecheck flag.
Could you please check current state?
p.s. I added test for each and every case you told in the previous comment also for market buy/sell.
-
checked limit orders behavior, works fine for all swap clients
-
[ ] https://github.com/ExchangeUnion/xud/pull/1922#issuecomment-704311772 still have 100 sat err for limit and market orders. Screenshot from 2020-10-19 16-15-51 Screenshot from 2020-10-19 15-53-01
-
[ ] also it seems we need to use same approach that we use now for limit orders (see screen with err), we can use max buy for connext currencies. Screenshot from 2020-10-19 16-05-18
In such case u just should not use max buy bound for usdt, it seems like logic error in the code (u buy btc and sell usdt, u should use sell bound of usdt)
Trading Limits:
┌──────────┬───────────────┬──────────┬───────────────────┬──────────────────┐
│ Currency │ Max Buy │ Max Sell │ Reserved Outbound │ Reserved Inbound │
├──────────┼───────────────┼──────────┼───────────────────┼──────────────────┤
│ BTC │ 2.4499095 │ 2.45 │ 0 │ 0 │
├──────────┼───────────────┼──────────┼───────────────────┼──────────────────┤
│ DAI │ 2624.99999999 │ 1500 │ 0 │ 0 │
├──────────┼───────────────┼──────────┼───────────────────┼──────────────────┤
│ ETH │ 0.1 │ 0 │ 0 │ 0 │
├──────────┼───────────────┼──────────┼───────────────────┼──────────────────┤
│ LTC │ 4.899638 │ 4.9 │ 0 │ 0 │
├──────────┼───────────────┼──────────┼───────────────────┼──────────────────┤
│ USDT │ 4116 │ 1000 │ 0 │ 0 │
└──────────┴───────────────┴──────────┴───────────────────┴──────────────────┘
simnet > listorders btc/usdt
Trading pair: BTC/USDT
┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐
│ Buy │ Sell │
├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤
│ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │
├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤
│ 0.01135065 │ 11408.6504 │ NestGrid │ 0.39602302 │ 11874.3096 │ NestGrid │
└───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘
simnet > buy 0.39 BTC/USDT mkt
9 FAILED_PRECONDITION: USDT outbound balance of 100000000000 is not sufficient for order amount of 463098074400
simnet > buy 0 max BTC/USDT mkt
price must be numeric for limit orders or "mkt"/"market" for market orders
simnet > buy max BTC/USDT mkt
9 FAILED_PRECONDITION: USDT outbound balance of 100000000000 is not sufficient for order amount of 411600000000
simnet >
sell max BTC/USDT mkt - use max sell bound for btc
buy max BTC/USDT mkt - use min(max amount of BTC that i can get using max sell bound for USDT, buy max BTC bound ) BTC
In case of ETH/BTC (Connext/Lnd):
sell max ETH/BTC mkt - use min(max amount of ETH that i can get using max buy bound for BTC, max sell bound for eth ) ETH
buy max ETH/BTC mkt - use max sell bound for btc to calculate eth amount
In case of USDT/DAI (Connext/Connext):
sell max USDT/DAI mkt - use max sell bound for usdt
buy max USDT/DAI mkt - use max sell bound for DAI to calculate USDT amount
In case of LTC/BTC (Lnd/Lnd):
sell max LTC/BTC mkt - use min(max amount of LTC that i can get using max buy bound for BTC, sell max LTC bound ) LTC
buy max LTC/BTC mkt - use min(max amount of LTC that i can get using max sell bound for BTC, buy max LTC bound ) LTC
@raladev I fixed logic of market orders for max, could you please recheck.
For 100 sat error: That means I need to pull this logic from orderbook.placeLimitOrder/placeMarketOrder to Service. I'm not sure about that although you're right currently it doesn't seem very good.
So waiting confirmation for this.
p.s. I rebased from master.
- [ ] assertion err during mkt order filling (something definetly wrong in qty calculation -
8276396.689944529 quantity)
Trading pair: BTC/USDT
┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐
│ Buy │ Sell │
├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤
│ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │
├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤
│ 0.01065863 │ 11608.7272 │ NestGrid │ 0.38919755 │ 12082.5528 │ NestGrid │
└───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘
Trading pair: ETH/BTC
┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐
│ Buy │ Sell │
├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤
│ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │
├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤
│ 14.1075 │ 0.03062206 │ SheriffSolution │ 1.2818432 │ 0.03187194 │ SheriffSolution │
└───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘
Trading pair: LTC/BTC
┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐
│ Buy │ Sell │
├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤
│ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │
├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤
│ 10 │ 0.004 │ OzoneYellow │ 10 │ 0.0042 │ OzoneYellow │
└───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘
Trading pair: USDT/DAI
┌────────────────────────────────────────────────┬────────────────────────────────────────────────┐
│ Buy │ Sell │
├───────────────┬─────────────┬──────────────────┼───────────────┬─────────────┬──────────────────┤
│ Quantity │ Price │ Alias │ Quantity │ Price │ Alias │
├───────────────┼─────────────┼──────────────────┼───────────────┼─────────────┼──────────────────┤
│ 500 │ 1 │ SheriffSolution │ 500 │ 1.01 │ SheriffSolution │
└───────────────┴─────────────┴──────────────────┴───────────────┴─────────────┴──────────────────┘
simnet > tradinglimits
Trading Limits:
┌──────────┬───────────┬──────────┬───────────────────┬──────────────────┐
│ Currency │ Max Buy │ Max Sell │ Reserved Outbound │ Reserved Inbound │
├──────────┼───────────┼──────────┼───────────────────┼──────────────────┤
│ BTC │ 2.4499095 │ 2.45 │ 0 │ 0 │
├──────────┼───────────┼──────────┼───────────────────┼──────────────────┤
│ DAI │ 100 │ 1500 │ 0 │ 0 │
├──────────┼───────────┼──────────┼───────────────────┼──────────────────┤
│ ETH │ 0.1 │ 10 │ 0 │ 0 │
├──────────┼───────────┼──────────┼───────────────────┼──────────────────┤
│ LTC │ 4.899638 │ 4.9 │ 0 │ 0 │
├──────────┼───────────┼──────────┼───────────────────┼──────────────────┤
│ USDT │ 100 │ 1000 │ 0 │ 0 │
└──────────┴───────────┴──────────┴───────────────────┴──────────────────┘
simnet > buy max BTC/USDT mkt
13 INTERNAL: Assertion failed
simnet > buy max BTC/USDT mkt
13 INTERNAL: Assertion failed
simnet > buy max BTC/USDT mkt
13 INTERNAL: Assertion failed
simnet >
20/10/2020 13:25:17.607 [ORDERBOOK] debug: reduced order 33d381a0-12d7-11eb-9dbe-b5b356f20d8a by 8276396.689944529 quantity while matching order b1da3b70-12d7-11eb-95e8-dd9ce37d5507
20/10/2020 13:25:17.608 [ORDERBOOK] debug: matched with peer 03cecb5ecd7f49902092cbf636b514ba3281aaf1bef95cfb249565ef65804c73f5 (NestGrid), executing swap on taker b1da3b70-12d7-11eb-95e8-dd9ce37d5507 and maker 33d381a0-12d7-11eb-9dbe-b5b356f20d8a for 8276396.689944529
20/10/2020 13:25:17.610 [SWAPS] debug: New deal: {"takerCltvDelta":40,"rHash":"8fcdc06e16f36438564ee5a804f8017e1da8e81675cda49ee4aa0ec32d074a8a","orderId":"33d381a0-12d7-11eb-9dbe-b5b356f20d8a","pairId":"BTC/USDT","proposedQuantity":8276396.689944529,"rPreimage":"d500919bdd6810dd4f017796e93794fcafc715507689cb2184924e0c63e9c55b","takerCurrency":"BTC","makerCurrency":"USDT","takerAmount":8276396.689944529,"makerAmount":100000000000,"takerUnits":8276396,"makerUnits":1000000000,"destination":"indra8BVnK5rCtz3sLpjVJY7Nfu7siw4bGZk6Pr9sEmDQ9eCtn36ExB","peerPubKey":"03cecb5ecd7f49902092cbf636b514ba3281aaf1bef95cfb249565ef65804c73f5","localId":"b1da3b70-12d7-11eb-95e8-dd9ce37d5507","price":12082.5528,"isBuy":false,"phase":0,"state":0,"role":0,"createTime":1603200317610}
20/10/2020 13:25:17.610 [ORDERBOOK] error: swap between orders 33d381a0-12d7-11eb-9dbe-b5b356f20d8a & b1da3b70-12d7-11eb-95e8-dd9ce37d5507 failed due to undefined
20/10/2020 13:25:17.610 [ORDERBOOK] error: swap for 8276396.689944529 failed during order matching due to unexpected error: AssertionError: Assertion failed
at new goog.asserts.AssertionError (/app/node_modules/google-protobuf/google-protobuf.js:81:876)
at Object.goog.asserts.doAssertFailure_ (/app/node_modules/google-protobuf/google-protobuf.js:82:257)
at Object.goog.asserts.assert (/app/node_modules/google-protobuf/google-protobuf.js:83:83)
at jspb.BinaryEncoder.writeUnsignedVarint64 (/app/node_modules/google-protobuf/google-protobuf.js:457:77)
at jspb.BinaryWriter.writeUnsignedVarint64_ (/app/node_modules/google-protobuf/google-protobuf.js:478:507)
at jspb.BinaryWriter.writeUint64 (/app/node_modules/google-protobuf/google-protobuf.js:483:471)
at Function.proto.xudp2p.SwapRequestPacket.serializeBinaryToWriter (/app/dist/proto/xudp2p_pb.js:4076:12)
at proto.xudp2p.SwapRequestPacket.serializeBinary (/app/dist/proto/xudp2p_pb.js:4053:34)
at SwapRequestPacket.serialize (/app/dist/p2p/packets/types/SwapRequestPacket.js:40:28)
at SwapRequestPacket.Packet.toRaw (/app/dist/p2p/packets/Packet.js:52:37)
20/10/2020 13:25:17.611 [RPC] error: call /xudrpc.Xud/PlaceOrder errored with code 2: Assertion failed
I added exact same test case for BTC/USDT, it's getting calculated as 0.08276396689944529, can't get the conclusion you're at @raladev
EDIT: 8276396.689944529 can this be in satoshis? Because it seems correct to me, it's 1000 (USDT) / 12082.5528 (Order price)
@raladev @kilrau could you please point out what to do for this PR then according to the @sangaman's review.
Agree with @sangaman . Let's do the following:
- only allow
maxflag for limit orders, remove all unnecessary order book checks as @sangaman mentioned above -> simplify to the best degree possible. Throw a meaningfulmax flag can't be used for market orderserror. - once done, real world tests on simnet by @raladev to see by how much the calculation is off - routing fees and channel reserve probably both need to be accounted for on lightning
Changed flag to be only usable with limit orders, and throwing error in case of market orders, removed market tests. @raladev could you please check ?
Regarding swap clients @raladev can explain better than me @sangaman
- I don't understand why we're concerned with the swap client type when determining the max quantity?
because connext currencies have different way for max amount calculation because of not-real max-buy bound - https://github.com/ExchangeUnion/xud/pull/1922#issuecomment-704409010
because connext currencies have different way for max amount calculation because of not-real max-buy bound - #1922 (comment)
Gotcha. If we want to ignore the connext maximum capacity since it can be dynamically increased, for purposes of the code/logic, I think it's better to make a method like getMaximumInboundCapacity for the SwapClient class and have Connext always return POSITIVE_INFINITY or something similar. Basically the current logic with 4 different if statements dpeneidng on the swap client types involved is hard to follow and seems fragile especially if we add or change swap clients in the future.
Also worth noting that in such cases the order will fail with an error due to insufficient collateral and will say to try again in a little while, I think that's fine but wanted to make sure we were all aware.
- [x] remove max flag example for mkt order from the help
xucli buy max BTC/USDT mkt place a market order to buy max BTC for USDT
-
[ ] still need to fix assertion bug (it appears because of initial qty and qty values - truncate on one of the calculation steps is required) Screenshot from 2020-10-21 16-49-37 Screenshot from 2020-10-21 16-48-00
-
[ ] sat 100 err for limit orders (https://github.com/ExchangeUnion/xud/pull/1922#issuecomment-712152518) Screenshot from 2020-10-21 16-50-22
- I'm concerned that routing fees are going to make this feature unreliable in the real world, if we're really trying to use every last satoshi in our channel then we won't have any left over for routing fees.
not talking about routing fees, but i cant swap for full amount even with directly connected lnd node and i dont know why @sangaman
simnet > lndbtc-lncli listchannels
{
"channels": [
{
"active": true,
"remote_pubkey": "0252f366111259996a101c5a82c880e9c95c8278d7e668c40163c0db656161cbbf",
"channel_point": "ce47babe9783eb188568ca944aa445b9d2a53f7dbc577c2206e82eabe7f908ca:0",
"chan_id": "184711356396666880",
"capacity": "2000000",
"local_balance": "64935",
"remote_balance": "1926014",
"commit_fee": "9051",
"commit_weight": "724",
"fee_per_kw": "12500",
"unsettled_balance": "0",
"total_satoshis_sent": "926014",
"total_satoshis_received": "0",
"num_updates": "4",
"pending_htlcs": [
],
"csv_delay": 240,
"private": false,
"initiator": true,
"chan_status_flags": "ChanStatusDefault",
"local_chan_reserve_sat": "20000",
"remote_chan_reserve_sat": "20000",
"static_remote_key": true,
"commitment_type": "STATIC_REMOTE_KEY",
"lifetime": "4163",
"uptime": "4163",
"close_address": "",
"push_amount_sat": "1000000",
"thaw_height": 0,
"local_constraints": {
"csv_delay": 240,
"chan_reserve_sat": "20000",
"dust_limit_sat": "573",
"max_pending_amt_msat": "1980000000",
"min_htlc_msat": "1",
"max_accepted_htlcs": 483
},
"remote_constraints": {
"csv_delay": 240,
"chan_reserve_sat": "20000",
"dust_limit_sat": "573",
"max_pending_amt_msat": "1980000000",
"min_htlc_msat": "1",
"max_accepted_htlcs": 483
}
}
]
}
because connext currencies have different way for max amount calculation because of not-real max-buy bound - #1922 (comment)
Gotcha. If we want to ignore the connext maximum capacity since it can be dynamically increased, for purposes of the code/logic, I think it's better to make a method like
getMaximumInboundCapacityfor theSwapClientclass and have Connext always returnPOSITIVE_INFINITYor something similar. Basically the current logic with 4 different if statements dpeneidng on the swap client types involved is hard to follow and seems fragile especially if we add or change swap clients in the future.Also worth noting that in such cases the order will fail with an error due to insufficient collateral and will say to try again in a little while, I think that's fine but wanted to make sure we were all aware.
I couldn't get how creating getMaximumInboundCapacity would resolve it, since there's a calculation depending on this, and it would still require 4 if statements, if I'm not mistaken?
- [x] remove max flag example for mkt order from the help
xucli buy max BTC/USDT mkt place a market order to buy max BTC for USDT
- [ ] still need to fix assertion bug (it appears because of initial qty and qty values - truncate on one of the calculation steps is required) Screenshot from 2020-10-21 16-49-37 Screenshot from 2020-10-21 16-48-00
- [ ] sat 100 err for limit orders (#1922 (comment)) Screenshot from 2020-10-21 16-50-22
@sangaman could you please clarify what to do for point 3? I'm not sure if it's okay to pull order quantity check to here.
@raladev for point 2 I still need clarification of how much of decimals to be truncated @sangaman
I couldn't get how creating
getMaximumInboundCapacitywould resolve it, since there's a calculation depending on this, and it would still require 4 if statements, if I'm not mistaken?
No you shouldn't need any if statements, you would just assign your maxBuyable variables to the value returned by getMaximumInboundCapacity by the swap client (rather than the max buy from trading limits), and each swap client could implement it separately with connext returning an unlimited amount. The idea is just to have generic logic in the way we deal with swap clients.
Another option is to make connext return an effectively unlimited amount for its inbound/buy trading limits, but I'm not sure that's a good idea, especially since we will reject connext buy orders that exceed the inbound limit.
@sangaman could you please clarify what to do for point 3? I'm not sure if it's okay to pull order quantity check to here.
Do you mean point 3 re: fees? I don't have a great solution for it, it's not a concern with direct channels like we're doing here for tests but it will be any time there are hops involved.
@raladev for point 2 I still need clarification of how much of decimals to be truncated @sangaman
We don't use fractional amounts for quantity and the grpc layer is using integers for those values so you'd need to remove any decimal places from the amounts calculated when you divide by price.
I am still unsure about the usefulness or reliability of this feature overall...
I couldn't get how creating
getMaximumInboundCapacitywould resolve it, since there's a calculation depending on this, and it would still require 4 if statements, if I'm not mistaken?No you shouldn't need any if statements, you would just assign your
maxBuyablevariables to the value returned bygetMaximumInboundCapacityby the swap client (rather than the max buy from trading limits), and each swap client could implement it separately with connext returning an unlimited amount. The idea is just to have generic logic in the way we deal with swap clients.Another option is to make connext return an effectively unlimited amount for its inbound/buy trading limits, but I'm not sure that's a good idea, especially since we will reject connext buy orders that exceed the inbound limit.
@sangaman could you please clarify what to do for point 3? I'm not sure if it's okay to pull order quantity check to here.
Do you mean point 3 re: fees? I don't have a great solution for it, it's not a concern with direct channels like we're doing here for tests but it will be any time there are hops involved.
@raladev for point 2 I still need clarification of how much of decimals to be truncated @sangaman
We don't use fractional amounts for
quantityand the grpc layer is using integers for those values so you'd need to remove any decimal places from the amounts calculated when you divide by price.I am still unsure about the usefulness or reliability of this feature overall...
For feature reliability I'm not sure about mainnet too if we talk about other fees etc. But for current implementation I guess I couldn't explain it well.
Currently maxBuyable for a swap client completely depends on quote's swap client and even if I implement getMaximumInboundCapacity into swap clients, I would still need to write 4 ifs to check because calculation completely depends on that according to the @raladev's points.