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.

Sign up for GitHub

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

Jump to bottom

Implement the KeySend feature, spontaneous payments #1485

Merged
merged 24 commits into from
Jul 21, 2020
Merged

Implement the KeySend feature, spontaneous payments #1485

merged 24 commits into from
Jul 21, 2020

Conversation

dariuskramer
Copy link
Contributor

What is KeySend?

For an explanation of the feature see the PR from c-lightning.

Testing

Support for receiving and sending is added. This was tested against each Lightning implementations (Sender -> Receiver):

  • Eclair -> Eclair
  • Eclair -> lnd
  • Eclair -> c-lightning
  • lnd -> Eclair

Sending support is not currently merged in c-lightning, so testing in this way c-lightning -> Eclair cannot be performed yet.

@t-bast t-bast self-requested a review July 10, 2020 15:34
@dariuskramer dariuskramer marked this pull request as draft July 10, 2020 15:55
t-bast
t-bast reviewed Jul 10, 2020
View reviewed changes
Copy link
Member

@t-bast t-bast left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Good job, this is very clean for a first PR!
I've put many comments for things you simply couldn't have guessed.

This needs some tests for the payment reception part, here is what comes to mind:

  • test we can receive a single HTLC keysend payment
  • test we can receive a multipart keysend payment (with two HTLCs)
  • test we reject keysend payments when the feature is disabled in nodeParams.features

This will allow you to touch many of our infrastructure code, it's great ;)

eclair-core/src/main/resources/reference.conf Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/Eclair.scala Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/Features.scala Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/wire/Onion.scala Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/payment/receive/MultiPartHandler.scala Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/payment/receive/MultiPartHandler.scala Outdated Show resolved Hide resolved
eclair-core/src/main/scala/fr/acinq/eclair/payment/receive/MultiPartHandler.scala Outdated Show resolved Hide resolved
eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala Show resolved Hide resolved
eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala Outdated Show resolved Hide resolved
t-bast
t-bast reviewed Jul 16, 2020
View reviewed changes
Copy link
Member

@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.

The code looks good, once we have tests we should be good to go!

eclair-core/src/test/scala/fr/acinq/eclair/EclairImplSpec.scala Show resolved Hide resolved
eclair-node/src/main/scala/fr/acinq/eclair/api/Service.scala Outdated Show resolved Hide resolved
Donovan Jean added 8 commits July 21, 2020 14:42
add feature bits
12a2dfa 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
add KeySend type
6860802 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
receive KeySend payment type
0d40241 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
send KeySend payment type
53af861 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
factorize KeySend TLV tag value
0387691 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
use a magic value for finalExpiryDelta
9fcbfee 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
fix failed test, use a different custom record than KeySend
18c5193 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
add test for sendWithPreimage
7491969 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
Donovan Jean added 14 commits July 21, 2020 14:43
fix KeySend feature announcement
6a49c98 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
move related function signature close together
b46323b 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
remove unnecessary KeySend type
8407fa4 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
remove constant for KeySend tag value
9b90158 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
add a new KeySend PaymentType
8ef272a 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
add features in the invoice
51c9dc4 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
receive KeySend payment only if the option is enabled
588000f 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
improve sendWithPreimage to be more functional
826e3cc 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
improve KeySend API type parameter
ee009bd 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
improve tests
46ff2e1 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
be more gentle with user
0dfcd99 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
add test for a KeySend payment in a single HTLC
58a04e2 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
add test for a KeySend payment without the feature activated
493ddf9 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
optimize imports
a8f89d7 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
fix CLTV expiration value (caused by the merge of PR #1483)
fb691c0 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
@dariuskramer dariuskramer marked this pull request as ready for review July 21, 2020 13:39
harmonize test names
c42203c 
Signed-off-by: Donovan Jean <donovan@acinq.fr>
t-bast
t-bast approved these changes Jul 21, 2020
View reviewed changes
Copy link
Member

@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.

LGTM!

@dariuskramer dariuskramer merged commit e6909cf into master Jul 21, 2020
@t-bast t-bast deleted the feature/keysend branch August 11, 2020 07:51
@Ale1ster Ale1ster mentioned this pull request Feb 18, 2021
Closed
pm47 pushed a commit that referenced this pull request May 1, 2021
@dariuskramer @pm47
Implement the KeySend feature, spontaneous payments (#1485)
36ef588 
Support for receiving and sending KeySend payment is added.

For an explanation of the KeySend feature see: ElementsProject/lightning#3611
@t-bast t-bast mentioned this pull request Aug 9, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-bast t-bast t-bast approved these changes

Assignees
No one assigned
Labels
Optech Make Me Famous!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dariuskramer @t-bast