Add deposit/withdraw wallet calls to SwapClient

[kilrau]

Add to SwapClient interface, implement for LNDClient only:

• deposit <currency> (p2wkh only is fine, for other options users can fall back to lncli)
• withdraw <amount> <currency> <destination> [fee]: fee in sat/lit per byte

[kilrau]

Another idea is to unify channelbalance and walletbalance into one balance call which shows both in separated tables. The naming could be wallet/tradeable or committed/non-committedt.

[krrprr]

I'm working on this issue.

[ghost]

I'm working on this issue.

Awesome, for the initial iteration I'd go for deposit and walletbalance commands. We can even split those into separate PRs.

[kilrau]

Thinking about this again, I believe it's nicer to simplify the current channelbalance call to balance and show the walletbalance there, instead of yet another grpc call:

┌──────────┬─────────┬─────────────────┬────────────────┐
│ Currency │ Balance │ Pending Balance │ Wallet Balance │
├──────────┼─────────┼─────────────────┼────────────────┤
│ BTC      │ 1       │ 0.1             │ 2.3            │
├──────────┼─────────┼─────────────────┼────────────────┤
│ LTC      │ 5       │ 10              │ 0.04           │
├──────────┼─────────┼─────────────────┼────────────────┤
│ WETH     │ 25      │ 0               │ 0              │
├──────────┼─────────┼─────────────────┼────────────────┤
│ DAI      │ 500     │ 0               │ 0              │
└──────────┴─────────┴─────────────────┴────────────────┘

What do you think? @krrprr @erkarl

[ghost]

@kilrau I'm OK with your proposed solution.

[krrprr]

@kilrau I like this approach.

[kilrau]

Great! Updated the issue.

[krrprr]

Should the wallet balance column display the value of total_balance or are we interested only in confirmed_balance?

[kilrau]

confirmed_balance only

[kilrau]

Since raiden lacks wallet functionality, I'm wondering what to do. Should we go for a geth web3 call and eth confirmations equal to 10 minutes (lnd defines confirmed as >= 1 confirmations)?

[sangaman]

How about something like:

┌──────────┬─────────┬─────────────────┬──────────────────┐
│ Currency │ Balance │ Channel Balance │ Wallet Balance   │
├──────────┼─────────┼─────────────────┼──────────────────┤
│ BTC      │ 5       │ 2               │ 3 (1 unconfirmed)│
├──────────┼─────────┼─────────────────┼──────────────────┤
│ LTC      │ 20      │ 12 (3 pending)  │ 8                │
├──────────┼─────────┼─────────────────┼──────────────────┤

I figure that "pending" balance may be confused with "unconfirmed" balance like a transaction into your wallet that's not in a block yet. Calling WalletBalance in lnd gives us the confirmed and unconfirmed amounts, so we can differentiate.

Splitting up the data into 6 columns would get pretty wide, and most of the time the pending/unconfirmed balances will be 0, so we can stick to the 4 columns above with parentheses as needed.

[kilrau]

Agree on the table still not being 100% clear, since you read "Balance" as total balance whereas I meant "Channel Balance" - only tradable balance is "balance". Anyhow, that might not hold for all users. Also agree that 2 columns are much better! Including a new Total Balance, here the hopefully final revision:

┌──────────┬─────────┬───────────────────┬──────────────────┐
│ Currency │ Total   │ Channel Balance   │ Wallet Balance   │
│          │ Balance │ (Tradable)        │ (Not Tradable)   │
├──────────┼─────────┼───────────────────┼──────────────────┤
│ BTC      │ 2.83    │ 0.1 (0.43 pending)| 2.3              │
├──────────┼─────────┼───────────────────┼──────────────────┤
│ LTC      │ 10.04   │ 10                │ 0.04             │
├──────────┼─────────┼───────────────────┼──────────────────┤
│ WETH     │ 123.42  │ 25                │ 0 (98.42 pending)│
├──────────┼─────────┼───────────────────┼──────────────────┤
│ DAI      │ 500     │ 500               │ 0                │
└──────────┴─────────┴───────────────────┴──────────────────┘

"pending" balance may be confused with "unconfirmed" balance like a transaction into your wallet

"pending" and "unconfirmed" are the same thing, a transaction which is yet to receive enough block confirmations to be regarded as final by lnd/raiden. No matter if it's tx into the wallet or a channel funding tx. I like the generic "pending" better, since since it's sometimes several confirmations before a tx is regarded as final, depending on the chain. So "unconfirmed" is confusing. Let's also not forget, that we have not-so-technical users which will understand "pending" better.

That all ok with you? @sangaman

[sangaman]

I don't think we need to specify which is tradable and which is not tradable, although specifying "total balance" is fine.

"pending" and "unconfirmed" are the same thing,

But they are different, at least how I understand them and how they are used in lnd. "Unconfirmed" is an on-chain transaction into an lnd wallet that has been broadcast but which has not received the minimum number of confirmations. "Pending" (as in "pending channel") is a channel funding transaction that has been broadcast but has not received the minimum number of confirmations - this is consistent with the usage in lnd. "Pending" balance will be available for swaps & lightning payments imminently and without further action.

For regular on-chain transactions that don't involve payment channels, I think "unconfirmed" is widely used (by wallets, exchanges, block explorers, etc...) to describe a transaction that hasn't been confirmed yet, whereas I never see "pending" used to describe such a transaction.

I would also expect any pending or unconfirmed balance to be included in the "total" balance - for one thing it may appear that the user has lost funds in cases where they have just opened or closed a channel and their "total balance" as reported by xud decreases.

I do think it would be good to be explicit in the comments/descriptions for these terms in our API documentation, though.

[krrprr]

What about including pending/unconfirmed balance in the total balance in the way it is displayed in channel/wallet balance? Then it's even clearer as there are probably several interpretations on what to expect from "total balance".

[sangaman]

Personally I think "Total Balance" is fairly clear that it adds up all types of balances, and there's already a breakdown of what is pending/unconfirmed to the right of it, so I don't think we'd need to break it down again with the total column (there's not much space in a terminal so we need to be succinct).

[kilrau]

To wrap this up:

I would also expect any pending or unconfirmed balance to be included in the "total" balance

Yes, of course. Good catch - my mistake!

I don't think we need to specify which is tradable and which is not tradable,

I think we should - I believe it's it's a valuable hint for anybody who is not familiar with how things work "under the hood". And we can still remove it if people should feel annoyed.

"pending" and "unconfirmed" are the same thing
"Unconfirmed" is an on-chain transaction into an lnd wallet that has been broadcast but which has not received the minimum number of confirmations.

Actually you are right, they are different:

• pending: tx didn't receive required amount of confirmations yet.
• unconfirmed: tx is sitting in the mempool and did not receive ANY confirmation yet.
  Also, we had exactly the same discussion how to label regular bitcoin on-chain transactions in a wallet project I did before: Unconfirmed (and then showing number of confirmations in orange and green once the required confirmations were met) vs. Pending/Complete. We finally went for Pending/Complete since these two states are simpler, less technical and more widely understood. To back this up, a simple Google Search for "pending bitcoin transaction" has ~847k results vs "unconfirmed bitcoin transaction" ~414k results. So I am convinced Pending/Complete is the better choice in both cases.

[krrprr]

I think adding tradable / not tradable makes sense. It does not take up that much space as it is only specified in the header. But it can be a good hint for somebody.

Unconfirmed seems actually accurate in the wallet balance context as the unconfirmedBalance in the lnd rpc's WalletBalanceResponse has 0 confirmations as the confirmedBalance has at least 1. Pending may be more widely understood indeed. So, what's the final decision - should we go for Pending? @kilrau @sangaman

[kilrau]

Yes, Pending.

• more widely understood (see above)
• simpler. Only two states (Pending/Complete), whereas Complete signifies a transaction that met the amount of confirmations required by a particular wallet to regard it as "final" and allow it to be "used". This can be different from wallet implementation to wallet implementation, and definitely can be more than one confirmation (even though for lnd it's one, or even zero if a flag is used).

So unless @sangaman shoots in a veto today, let's go for Pending/Complete.

[sangaman]

OK I'll see how it looks with pending & the "tradable" designations, thanks.