Xud: Inbound/outbound channel balance check

[kilrau]

On placeOrder, check over all open payment channels if inbound (Buy) and outbound (sell) balance is enough to execute the order. One hop only. If not, return 'channel balance not enough` event & print.

[kilrau]

Summary:

Error: Channels with balance below trade amount result in <coin> <incoming/outgoing> channel balance not sufficient. "Balance check" is actually an error message and is a subset of a swap error. Error event via stream, placeOrder sync returns immediately. Printed on CLI.

Error: Entering an order for a non-existing channel immediately returns <coin> <incoming/outgoing> channel not existent and doesn't allow to issue order. Blocking.

Hint: Channel balance is also checked immediately after entering an order and presents a warning via event + CLI if channel balance not sufficient: <coin> <incoming/outgoing> channel balance currently not sufficient. Your order will be added to the order book but currently can only be matched with internal orders. Continue? Y/n Y/n on CLI only. Non-blocking.

[kilrau]

How about this one? @erkarl

[erkarl]

@kilrau I'll take a look!

[sangaman]

I'm sorry I hadn't looked at this issue in a while, do we really want to require a direct channel with sufficient balance for swaps? I figure that as long as we can find a route, we should be good to proceed.

[erkarl]

@sangaman agreed. That's what the current implementation is doing, right?

@kilrau do we also really require the channel & channelbalance checks upon placing the order? Upon implementing #650 I got the feeling that it would be redundant. We're verifying that the route with sufficient balance exists before starting the swap anyway.

Having channel & channelbalance checks upon placing the order would protect against orderbook spoofing, but I feel like this is something the reputation system should handle, eventually.

[sangaman]

That's what the current implementation is doing, right?

I think right now we only check for a route when deciding whether to accept a deal but I believe in #650 you're adding the check before we initiate a deal as well which is good I think.

[kilrau]

we really want to require a direct channel with sufficient balance for swaps

We don't require a direct channel, a route is fine - sry if this was not clear. We check if the balance of OUR channel with the next peer is sufficient, no matter if the next peer is the final destination or not. We obviously can't balances after the first hop. @sangaman @kilrau

do we also really require the channel & channelbalance checks upon placing the order? Upon implementing #650 I got the feeling that it would be redundant

I'd suggest to do it, but only print a warning/hint as I mentioned above. Reason is unintended behavior, like match with an internal orders whereas peer orders were targeted. Mainly for CLI.

[erkarl]

@kilrau @sangaman

As discussed during the call:
a) The first part of this issue is done in #650 (route checks before swapping)
b) I've been trying to figure out how to implement the channel balance sanity checks upon placeOrder, but haven't found a good solution, yet. One idea that I had was that this feature would be for CLI buy/sell only in order to provide users feedback.

[kilrau]

Ok how would it work if it's CLI only. As @sangaman said, it needs to pull channel info from LND & Co

[erkarl]

@kilrau the buy/sell command would internally call xud's channelbalance and break the execution if the balance doesn't exist or warn the user if there's not enough local balance.

[kilrau]

Would that significantly delay the CLI response?

[erkarl]

It depends on what we consider significant. It's a tradeoff between UX and performance.
channelBalance: 2.356ms using my local environment (your milage may vary). For humans it doesn't really matter.

If the CLI is used programatically (in which case they should really be using gRPC instead) then ~2ms might make a big difference.

[sangaman]

What Karl says makes sense, it wouldn't be too hard to just call channelbalance and do a quick comparison. I have a couple of concerns though:

1. The check doesn't assure us of very much, sufficient channel balance is no guarantee that we'd actually have a route, nor that we have sufficient incoming capacity to receive the other side of the swap.

2. I'm not sure prompting y/n if an order can only be matched internally makes sense from the cli perspective. As I see it the cli will either be used for testing purposes (where we're not worried about internal matches happening) or by end-users (as opposed to exchanges) where it doesn't make any sense for them to be trading against themselves.

3. To the extent that this is a valuable safeguard, I think it makes sense to employ it on a broader scope and not just via the cli. Maybe xud should make these sanity checks internally - it's not in our interest to advertise orders that can't be swapped. For example, we shouldn't broadcast orders if we don't have channels open with some minimum capacity. We already do some checks on initialization like verifying lnd is in sync with the chain. I believe checking our incoming capacity would be a bit more complicated - calling listchannels and then iterating over each one - but still doable. Getting this done right wouldn't be simple but it seems worthwhile, and once that's in place we could return a grpc error for the placeorder calls indicating that the order can't be swapped without needing to make an extra call like channelbalance.

Also, blocking the order in case we don't have channels seems unnecessary, it could hinder testing, and I could imagine wanting to be able to add orders to xud even before channels are set up. I think a warning message might make more sense.

[erkarl]

@sangaman

1. True. It would just ensure that the channel exists and that there's enough outbound balance (only 1 side of the swap).

2. I'm now leaning towards this approach as well. Correctly done sanity checks along with the reputation system should eventually make the orderbook quite reliable.

[kilrau]

Ok , let's leave the CLI check (and prompt) out then. Thanks for your feedback!

[erkarl]

Opened #675 for the second part.