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

First stab at changes to add lnaddress #433

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

brianoflondon
Copy link
Contributor

Signed-off-by: Brian of London (Dot) [email protected]

I moved the Lightning block info out of the main Value.md because they're separate. Lightning is A value block system not THE value block system.

This is a follow up to pull request #384 about lnurl.

Signed-off-by: Brian of London (Dot) <[email protected]>
@kingonly
Copy link

  1. lnaddress should take priority if exists
  2. The well-known/keysend should refer to the Lightning Address spec. One thing that is missing here is routing hints support.

@brianoflondon
Copy link
Contributor Author

  1. lnaddress should take priority if exists

Ok.

  1. The well-known/keysend should refer to the Lightning Address spec. One thing that is missing here is routing hints support.

I seem to remember route hints being in the spec but can't find any reference to them in the current version. I don't think I've ever seen them in the wild and I must admit that my Node had some unadvertised channels but I stopped using those a while back.

@brianoflondon
Copy link
Contributor Author

I just added the details for a bare endpoint response to signify a server which is acting as a lookup for Keysend.

@joksas
Copy link

joksas commented Feb 23, 2023

This looks great!

Should we also have a custom error returned when the Lightning address does not exist, but the provider still supports keysend? This would eliminate the need of making an additional request to https://example.com/.well-known/keysend.

@MerryOscar suggested having something like "status" = "UNKNOWN". Here's the relevant discussion on podcastindex.social: https://podcastindex.social/@dovydas/109746270989356454

Personally, I think HTTP status 404 with JSON field "tag" = "keysend" would be sufficient.

@brianoflondon
Copy link
Contributor Author

This looks great!

Should we also have a custom error returned when the Lightning address does not exist, but the provider still supports keysend? This would eliminate the need of making an additional request to https://example.com/.well-known/keysend.

@MerryOscar suggested having something like "status" = "UNKNOWN". Here's the relevant discussion on podcastindex.social: https://podcastindex.social/@dovydas/109746270989356454

Personally, I think HTTP status 404 with JSON field "tag" = "keysend" would be sufficient.

I actually send out a 422 if the name isn't a valid Hive account name.

I do not, however, check if the account has been created on Hive (that's a somewhat slow lookup)

You can see this here:

https://api.v4v.app/.well-known/keysend/122345

@joksas
Copy link

joksas commented Feb 24, 2023

I'm just thinking how to make the spec precise, yet not too difficult to implement. @brianoflondon, summarizing what you wrote and adding some more info, would the following work?

Providers (say, example.com) supporting .well-known/keysend lookup scheme should implement the following:

GET https://example.com/.well-known/keysend

GET request to https://example.com/.well-known/keysend should return HTTP status code 200 and a JSON with at least the following fields

  • status (required): "OK"
  • message (required): string describing what information is returned through this lookup scheme

GET https://example.com/.well-known/keysend/{lightningUsername}

GET request to https://example.com/.well-known/keysend/{lightningUsername} should return the following based on the value of {lightningUsername}:

  • If Ligthning address {lightningUsername}@example.com exists, HTTP status code 200 and a JSON with at least the following fields should be returned:
    • status (required): "OK"
    • tag (required): "keysend"
    • pubkey (required): string representing destination node's public key
    • customData (optional): array of CustomData, which has fields
      • customKey (required): number
      • customValue (required): string
  • Optionally, if the syntax for the Lightning address at example.com does not allow username {lightningUsername}, HTTP status code 422 and a JSON with at least the following fields should be returned:
    • status (required): "UNPROCESSABLE_ENTITY"
    • tag (required): "keysend"
    • message (optional): string describing why the username is invalid
  • In any other case, HTTP status code 404 and a JSON with at least the following fields should be returned:
    • status (required): "NOT_FOUND"
    • tag (required): "keysend"

A few other comments:

  • Is the JSON field status really necessary? Does it indicate any extra information that HTTP status code doesn't already provide?
  • Could the message in JSON returned from https://example.com/.well-known/keysend be optional instead? For most services, it's pretty straightforward—they host Lightning wallets and return their details. It's only in special cases like that of v4v.app where the mechanism is different.

@brianoflondon
Copy link
Contributor Author

The status and message field comes from FastAPI which is what I'm using, that's it's default behaviour for 422 and this happens outside of any code I write.

I'll try to take your suggestion and put it in this proposal soon.

@joksas
Copy link

joksas commented Feb 26, 2023

Got it! Well, then maybe this could be simplified by simply having a 404 with an optional message, which might say something like "the string does not match regex xyz"?

@brianoflondon
Copy link
Contributor Author

Got it! Well, then maybe this could be simplified by simply having a 404 with an optional message, which might say something like "the string does not match regex xyz"?

That's a part of FastAPI which I'm sure could be altered but I'm not going to try :-) The way FastAPI generates the docs and handles all the input sanitization is somewhere close to miraculous to my eyes!

@daveajones
Copy link
Contributor

I don't think I've ever seen them in the wild

Are you talking about something like this @brianoflondon ?

https://api.fountain.fm/v1/lnurlp/dave/keysend

@brianoflondon
Copy link
Contributor Author

I don't think I've ever seen them in the wild

Are you talking about something like this @brianoflondon ?

https://api.fountain.fm/v1/lnurlp/dave/keysend

This is what I meant: I've never seen anyone put routing hints in a value block. I can't find any explanation of what these look like in the current spec though I seem to remember they were once in the spec.

The well-known/keysend should refer to the Lightning Address spec. One thing that is missing here is routing hints support.

@daveajones
Copy link
Contributor

The routing hints are the customKey/customValue properties.

@brianoflondon
Copy link
Contributor Author

The routing hints are the customKey/customValue properties.

Maybe that's what you meant in the spec but "routing hints" has a special meaning in Bolt 11 Invoices and concerns nodes connected to the rest of the network by purely private or un-announced channels.

When I started up my V4VApp Voltage node I used private channels and soon realised that in order for funds to get to me the invoices my node generated were huge and the QR codes very detailed. That was because, if you decoded them, you got a list of public node addresses for nodes that I had private channels to. Those were the route hints.

The simple reality is that using a node only connected by these private channels for receiving keysend is not very practical.

Also Keysend payments would only come in on public channels.

https://docs.lightning.engineering/the-lightning-network/payment-lifecycle/understanding-lightning-invoices

r (3): One or more entries containing extra routing information for a private route. 
These routing hints include a 

pubkey (264 bits)
short_channel_id (64 bits)
fee_base_msat (32 bits, big-endian)
fee_proportional_millionths (32 bits, big-endian)
cltv_expiry_delta (16 bits, big-endian)

@adamc199
Copy link

I agree with Brian. Name needs to be different from routingHints. Can be almost anything but that to avoid confusion

@ericpp
Copy link

ericpp commented Mar 25, 2023

I think the current version assumes that the domain name and web server will be accessible. What happens if one or both of those are temporarily or permanently inaccessible/unresolvable? Should it hard fail the split as if the server returned a 404/NOT_FOUND? Should it have a "temporarily inaccessible - retry later" mode?

How should applications handle lnaddress lookup failures? Should unresolvable splits be skipped over or should the entire payment fail with a message to the user?

@brianoflondon
Copy link
Contributor Author

How should applications handle lnaddress lookup failures? Should unresolvable splits be skipped over or should the entire payment fail with a message to the user?

Splits fail, clients just need to handle this regardless of the reason.

Usually apps just fail a specific split.

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.

6 participants