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 bLIP #892

Closed
wants to merge 20 commits into from
Closed

Conversation

valentinewallace
Copy link
Contributor

Currently rebased on #884

The portions regarding the keysend feature bit and MPP probably need more iteration, but wanted to get a draft out.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for writing this up, good to have a spec for this.

Receiver:
* if completing the payment, MUST fulfill the HTLC using the TLV-provided
payment preimage
* if failing the payment, SHOULD error with `PERM|15
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should specify reason -> value, not just "if you fail for any reason, use this code".

blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
Copy link

@ariard ariard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good!

* SHOULD only send payments to nodes advertising feature bit 55 in the node
context starting June 1, 2022

Receiver:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MUST reject the keysend if the preimage is not matching the payment hash ?
MUST reject the keysend if a payment data field is present ?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can even the deanonymization attack in the lack of preimage verification in the rational section.

* if failing the payment, SHOULD error with `PERM|15
incorrect_or_unknown_payment_details`.
* MUST advertise feature bit 55 in the node context only starting June 1, 2022

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should include somewhere that TLV type 8 is from now occupied in the tlv_payload by keysend_preimage ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean TLV type 5482373484? Maybe in this section?

blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
Copy link
Collaborator

@t-bast t-bast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, it will be useful to have this in a shared document 👍

blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
## bLIP Reserved Feature Bits
| Bits | Name | Description | Context | Dependencies | Link |
|-------|----------------------------------|-----------------------------------------------------------|----------|-------------------|---------------------------------------|
| 55 | `keysend` | Node supports receiving keysend payments | N | | [bLIP #0002][blip-0002] |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are people relying on this specific value? It's a bit of a waste that it used a value so low, especially for a feature that will be obsolete once replaced by AMP...it would be nice if bLIPs used higher values for feature to avoid mixing official bolt features and bLIP features (but maybe that's just my OCD speaking)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tradeoff with higher feature bits, is that since we don't do any sparse encoding, higher feature bits (say in the tens of thousands) require the entire network to send around more data (looots of zeros onthe wire).

* MUST include a TLV record keyed by type `5482373484` with a TLV value of a
randomly generated and cryptographically-secure 32-byte value that serves as
the HTLC payment preimage
* MUST NOT set a `payment_data` field
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's payment_data here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From this section. I clarified a bit.

@valentinewallace valentinewallace marked this pull request as ready for review August 17, 2021 16:37
blips/blip-0002.mediawiki Outdated Show resolved Hide resolved
@Roasbeef
Copy link
Collaborator

Roasbeef commented Dec 6, 2021

Should be moved to https://github.com/lightning/blips

@valentinewallace
Copy link
Contributor Author

Should be moved to https://github.com/lightning/blips

Thanks, moving!

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.

7 participants