LN-Zap / zap-android

Zap Wallet - Native android lightning wallet for node operators focused on user experience and ease of use ⚡️
MIT License
165 stars 49 forks source link

Add specific zero-amount-payment-request error message #364

Closed vindard closed 3 years ago

vindard commented 3 years ago

Description

This PR introduces a more specific error message for the error case where a payment requests has an invalid zero-amount attached to it.

The prior error message suggests a more general and broad failure of the payment request that is very difficult to understand and correct by the end-user.

Note: The original error code was removed in a separate commit since it no longer is used anywhere else. If this isn't ok, we can simply drop that commit and force-push the branch (or revert the specific commit)

Motivation and Context

Issue: #363

How Has This Been Tested?

Has not been tested

Types of changes

Checklist:

vindard commented 3 years ago

I just found a similar issue/fix across in the Zap-Desktop repo: https://github.com/LN-Zap/zap-desktop/pull/3385

michaelWuensch commented 3 years ago

@vindard Hi, thank you very much for the pull request! The old message is indeed confusing. Can you please use the wording from Zap desktop? I like it more as it explains why 0 amount invoices are not supported.

vindard commented 3 years ago

@vindard Hi, thank you very much for the pull request! The old message is indeed confusing. Can you please use the wording from Zap desktop? I like it more as it explains why 0 amount invoices are not supported.

Sure no probs, change made here

michaelWuensch commented 3 years ago

@vindard Ah sorry, i just looked at the screenshot of that pull request you linked which showed another message that I actually meant. And I like it more:

"This is a zero amount payment request. Zap does not support zero amount payment requests due to security concerns associated with them."

Would you mind changing it again? I want to explain why we don't support them, not just say it is unsupported.

vindard commented 3 years ago

@vindard Ah sorry, i just looked at the screenshot of that pull request you linked which showed another message that I actually meant. And I like it more:

"This is a zero amount payment request. Zap does not support zero amount payment requests due to security concerns associated with them."

Would you mind changing it again? I want to explain why we don't support them, not just say it is unsupported.

Ah makes sense, changed here