Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added note for bLIP 10 fields removed by #510 #587

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ericpp
Copy link

@ericpp ericpp commented Nov 25, 2023

The spec was changed when it was transferred over from the lightning repo and several unused or underused fields were removed (see #510). This PR adds a note listing the fields that were removed and links to the #510 for more info.

Some apps are still using these fields (particularly value_msat, see: https://reflex.livewire.io/apps) and it would be good to indicate in the spec that they should no longer be used

@ericpp
Copy link
Author

ericpp commented Nov 25, 2023

And since value_msat is still widely used, maybe it should be re-added to the spec? It was removed since the invoice itself already contains the number of received msats.

@johnspurlock
Copy link
Contributor

value_msat would be very useful to include in the spec (it's in the example!), as it can contain millisats (i don't think invoices can or at least do?) - millisats are useful to back into the total amount if you have a small split

and it's a great indication of "what the client meant to send", to be sanity checked against what was actually received

also, as you say, already being sent by many

@johnspurlock
Copy link
Contributor

this was my only recommended note for whoever is in charge of the spec over in https://reflex.livewire.io/apps
image

@ericpp
Copy link
Author

ericpp commented Nov 25, 2023

@johnspurlock Invoices do contain both sat and msat totals:
e.g. lncli output

"value": "34",
"value_msat": "34000",
...
"amt_paid": "34000",
"amt_paid_sat": "34",
"amt_paid_msat": "34000",
...
"htlcs": [
    {
        ...
        "amt_msat": "34000",
    }
]

value_msat might be useful since it contains the original split value before any lightning routing fees are taken out. I think this can still be derived from the HTLCs in the invoice, but it might be more convenient to get it from the TLV. Actually, lightning routing fees are added to the payment amount rather than taken from the payment amount. The split total should be the same as the total listed on the invoice.

On the other hand, getting it from the invoice is better for reporting actual received msat totals, since TLVs can be easily spoofed. It's easy to send a 1 sat payment with a TLV that says I sent a 1M sat payment.

@samsethi
Copy link

Thanks to @johnspurlock we added both value_msat and value_msat_total to the Alby Dashboard. Are we missing anything else John?

Alby__Your_Bitcoin___Nostr_companion_for_the_web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants