Error messaging for send amount too small

[chitowncrispy]

Description

I added some error messaging to the UI when the user is trying to send an amount that is too small for the fee estimation service to estimate the fee.

I needed to allow the fee estimation completion block to return a result set that contained both the success and failure scenarios. This allows us to respond to both scenarios all the way down to the UI level in the LoadingAmountView. In this view, I look for a specific error message (not ideal) and then based upon that I tailor a more user-friendly message to display to the user.

I think I may have found a potential error in either our code or in the LND API where they are not properly returning errors back in the response. As far as I can tell the errors are all Code=1 and in the localizedDescription they all come through as code = Unknown. This may be something we should report back to the LND team so they can better pass that through to the apps for error handling.

Motivation and Context

As I have been testing the app I have been seeing that when I have a small number of sats I want to send the fee estimate was "-". This didn't make much sense to me and until I dug into the error that the API was returning I wasn't sure why that was happening. Thus, I put up this PR to make it a bit more user and noob friendly.

How Has This Been Tested?

On testnet I was opening the send view and then typing in a small number of sats (<150 sats). I would type it out, delete it, type it again, etc... All of which would show the error string. I would then put in an amount large enough to trigger a fee estimate (>1000 sats) and the estimate would show.

Screenshots (if appropriate):

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• [x] My code follows the code style of this project.
• [ ] My change requires a change to the documentation.
• [ ] I have updated the documentation accordingly.
• [x] I have read the CONTRIBUTING document.
• [ ] I have added tests to cover my changes.
• [x] All new and existing tests passed.

[codecov-io]

Codecov Report

Merging #297 into master will decrease coverage by 0.04%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #297      +/-   ##
==========================================
- Coverage   11.01%   10.97%   -0.05%     
==========================================
  Files         234      234              
  Lines        9330     9369      +39     
==========================================
  Hits         1028     1028              
- Misses       8302     8341      +39

Impacted Files	Coverage Δ
Library/Generated/strings.swift	6.97% <ø> (ø)	:arrow_up:
Lightning/Services/TransactionService.swift	18.75% <0%> (-1.94%)	:arrow_down:
...ry/Scenes/ModalDetail/Send/LoadingAmountView.swift	0% <0%> (ø)	:arrow_up:
...ibrary/Scenes/ModalDetail/Send/SendViewModel.swift	0% <0%> (ø)	:arrow_up:
Library/Views/OnChainFeeView/OnChainFeeView.swift	0% <0%> (ø)	:arrow_up:
Library/Scenes/LndLog/LndLogViewController.swift	0% <0%> (ø)	:arrow_up:
...y/Scenes/Background/BackgroundViewController.swift	0% <0%> (ø)	:arrow_up:
...cenes/ManageWallets/WalletConfigurationStore.swift	0% <0%> (ø)	:arrow_up:
Library/Scenes/Node/NodeHeaderView.swift	0% <0%> (ø)	:arrow_up:
... and 1 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8320cf8...a04e48d. Read the comment docs.

[ottosuess]

sorry i broke your pr: https://github.com/LN-Zap/zap-iOS/commit/e8b5799125292e420846914eb43618dc71c52ff4

lnd's routes endpoint now only returns a single route. I changed that in our api to make it more obvious. A rebase should be very simple though.

[ottosuess]

when i enter an amount bigger than my on chain funds i get some sort of error indication at the top:

maybe when the funds are below the dust limit we should display the error on the top too. just to be consistent? (because it's not specific to fee estimation but to sending funds in general)

any thoughts? @sohomang @JimmyMow @ole1220

[sohomang]

@ole1220 Agreed! It'd also provide feedback before clicking Send so it's a good thing

What about handling it the same way we handle a channel opening ? i.e. updating the On-chain balance: X sats label to Minimum sending amount: X sats as the user types his chosen amount.

[chitowncrispy]

[chitowncrispy]

@sohomang there isn't a way for me to tell what the minimum amount of sats that we can send is. Since that is the case I added some more UI tweaks in the screenshots above based upon the state. Let me know how these look now.

@ottosuess do you know of a way to get the minimum amount of sats that we can send?

[sohomang]

@chitowncrispy looks 🎇! Could we make it so when no amount is typed yet, no error message appears? Like:

• No amount: On-chain balance: X
• Dust amount: Send amount too small
• Correct amount: On-chain balance: X
• Too much: Same but in red

[chitowncrispy]

@sohomang

[JimmyMow]

So where did we settle on this @chitowncrispy @ottosuess @sohomang? No error in the fee estimation, rather the following for the amount input?

• No amount: On-chain balance: X
• Dust amount: Send amount too small (in red)
• Correct amount: On-chain balance: X
• Too much: On-chain balance: X (in red)

[ottosuess]

yeah, imo the error at the top is enough.

[chitowncrispy]

Ok, so we don't want the error in the fee estimate label? Only up top where we have the subtitle for the amount entered?

[JimmyMow]

Yeah @chitowncrispy that seems to be the consensus. I personally like that UX as well

[chitowncrispy]

[chitowncrispy]

@ottosuess @JimmyMow @sohomang I've updated the code for the desired behavior and attached the screenshots of each state.

[ottosuess]

merged in 7dace9e2532da36faa84ee2aeb3ca1e8eda42d00