This repository has been archived by the owner on Mar 23, 2021. It is now read-only.
Sdk lightning actions #2107
Merged
Sdk lightning actions #2107
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
D4nte
force-pushed
the
sdk-lightning-actions
branch
5 times, most recently
from
0e34835 to
9dd23ac
Compare
bors bot
added a commit
to comit-network/comit-js-sdk
that referenced
this pull request
Mar 3, 2020
141: Add Lightning support for actions r=D4nte a=D4nte This allows the use of comit-sdk to make current comit-rs e2e lnd tests pass. See comit-network/comit-rs#2107 for the e2e side. Resolves #118. Co-authored-by: Franck Royer <[email protected]>
D4nte
force-pushed
the
sdk-lightning-actions
branch
from
9dd23ac to
0b63a61
Compare
D4nte
commented
Mar 3, 2020
D4nte
commented
Mar 3, 2020
D4nte
commented
Mar 3, 2020
| ); | ||
|
|
||
| const satAmount = "10000"; | ||
| const finalCltvDelta = 40; // This MUST be 40 to match Bob's invoice. Further investigation needed. |
There was a problem hiding this comment.
I propose to investigate that in a follow-up PR as we would need to play with AddHoldInvoice and understand whether we can manipulate the finalCtlvDelta of an invoice when adding it or whether it's a global lnd parameter.
There was a problem hiding this comment.
Ok, this is now resolved locally. The cltvExpiry of addHoldInvoice needs to match the finalCltvDelta on sendPayment when paying the invoice.
Which means that we need to add this cltvExpiry to the ln-add-hold-invoice action payload.
There was a problem hiding this comment.
Please open an issue.
D4nte
commented
Mar 3, 2020
added 11 commits
March 4, 2020 10:49
D4nte
force-pushed
the
sdk-lightning-actions
branch
from
3e782d7 to
7e0dcc7
Compare
thomaseizinger
suggested changes
Mar 3, 2020
| ); | ||
|
|
||
| const satAmount = "10000"; | ||
| const finalCltvDelta = 40; // This MUST be 40 to match Bob's invoice. Further investigation needed. |
There was a problem hiding this comment.
Please open an issue.
D4nte
added a commit
to comit-network/spikes
that referenced
this pull request
Mar 4, 2020
This is from the learnings we got working on: - comit-network/comit-rs#2107 - comit-network/comit-js-sdk#141
This comment has been minimized.
This comment has been minimized.
tcharding
approved these changes
Mar 4, 2020
thomaseizinger
approved these changes
Mar 4, 2020
|
bors r+ |
|
bors r+ |
|
Already running a review |
Build succeeded |
bors bot
added a commit
that referenced
this pull request
Mar 5, 2020
2155: Follow-up of #2107 r=mergify[bot] a=D4nte Also integrates changes from comit-network/comit-js-sdk#145. 2168: Upgrade node to version 12 r=mergify[bot] a=bonomat @thomaseizinger found an issue describing incompatibility of the node version we are using on circleCI and jest: jestjs/jest#9453. The tests are running locally. So let's upgrade circleCI as well. Co-authored-by: Franck Royer <[email protected]> Co-authored-by: Philipp Hoenisch <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Proof that comit-network/comit-js-sdk#141 works.
Adds a test that does the flow as expected by HALight using hold invoices.
Ready for review.