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

Keysend TLV type mapping for sphinx #2

Closed
cdecker opened this issue Jun 16, 2021 · 8 comments
Closed

Keysend TLV type mapping for sphinx #2

cdecker opened this issue Jun 16, 2021 · 8 comments
Assignees

Comments

@cdecker
Copy link
Collaborator

cdecker commented Jun 16, 2021

During our call yesterday @Evanfeenstra mentioned that there might be some customization required to get the keysend and noise delivery working with the sphinx-relay. So I'd like to see how we can map the required parts to the existing fields, and if changes are required. Currently the keysend plugin uses this type to indicate that there is a preimage included in the payload:

https://github.com/ElementsProject/lightning/blob/75720ad0e1690052c108fbf457ded807bf09d6c1/plugins/keysend.c#L10

While noise uses these fields to transfer the information:

https://github.com/lightningd/plugins/blob/775fd8a3c29b0a5a2a4e7003f2f23b8644672348/noise/noise.py#L15-L18

The preimage type is the same and we use a chain of responsibility to fail in keysend if unknown even field types are present, so it hands the HTLC over to the noise plugin if there is a message attached.

Which fields are required by sphinx and what types are used for them? I assume that the message, being encrypted and prepended with the pubkey on the sphinx-relay side, is intended to be binary (which requires a minimal change to the noise plugin anyway).

@cdecker
Copy link
Collaborator Author

cdecker commented Jun 16, 2021

One more thing that came to mind: how do you signal whether the payload is binary or human readable text? Since there is very little to add to our keysend plugin I'm currently adding the messages to it directly instead of going through the noise plugin.

@Evanfeenstra
Copy link
Collaborator

Sphinx uses the key of 133773310 here: https://github.com/stakwork/sphinx-relay/blob/master/src/utils/lightning.ts#L18.

The content is bytes (JS ByteBuffer object)... internally its actually stringified JSON (with the signature appended at the end). We validate the signature externally, it doesn't need to be validated inside the plugin.

@cdecker
Copy link
Collaborator Author

cdecker commented Jun 16, 2021

So signatures are handled completely by sphinx-relay? No need for us to pass the signature along? That makes it easy for us :-)

@cdecker
Copy link
Collaborator Author

cdecker commented Jun 16, 2021

Would it make sense for me to just add a extra_destination_tlv parameter to keysend? That would allow us to basically just reuse the existing keysend plugin. I don't expect it'd take much longer than using the noise plugin and be more efficient on our side. The alternative is to have a custom plugin for sphinx which implements just the send and receive logic to add the field you need.

The main problem with this is that the key you're using is an even key, which means we can't accept it without understanding it in the receiving plugin. The spec says we're supposed to reject payments with even fields we don't implement internally, this is meant as a security measure since we don't know what we're agreeing to by accepting the payment.

Not sure how other implementations handle this.

@Evanfeenstra
Copy link
Collaborator

Dang, my mistake on the even number. I should've read the spec more carefully. I guess i can just switch it 13377331...

extra_destination_tlv on the keysend plugin seems like a good option to me

@cdecker
Copy link
Collaborator Author

cdecker commented Jun 18, 2021

No problem, I have a patch pending for the keysend plugin that allows the operator to specify even TLV numbers to also accept at startup. Then we can just use the existing keysend plugin plus the invoice_payment notification to deliver the extra TLVs to the user application.

Should get it working in a couple of hours.

@cdecker
Copy link
Collaborator Author

cdecker commented Jun 18, 2021

Pull request for keysend + extra-tlvs + extra-tlv-type allowlist is up for review: ElementsProject/lightning#4610

Also mentioned sphinx in the integration test to make sure it fits the use-case we have here: inject a binary field into the payload, and then call the plugin hook we'll use to return notify the app about the incoming payment with the binary field.

As far as I see it this should match exactly our use-case here :-)

@cdecker
Copy link
Collaborator Author

cdecker commented Jun 23, 2021

We deployed the version of c-lightning with extratlvs and routehints support yesterday, and today we added the GRPC wrappers (9e07c93). This also includes a new stream_incoming method which can be used to be notified about incoming payments, along with any extra TLVs sent by the sender. For now it just matches invoices (which also covers keysend since for book-keeping reasons we create an invoice matching the keysend on the fly), but eventually we will add on-chain payment notifications as well (hence the nesting here).

The glcli utility has also been updated, so glcli stream-incoming can be used to tail the incoming payments, and glcli keysend [node_id] [amount] can be used to send spontaneous payments (--extratlvs and --routehints supported as well).

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

No branches or pull requests

2 participants