-
-
Notifications
You must be signed in to change notification settings - Fork 160
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
[v0.9.0] Custom route hints #2142
Conversation
Looks great so far. Might have some conflicts once External accounts goes in. We'll cut out a feature branch for v0.9.0 to go into while we're still rolling out v0.8.4 Just lmk if you'd like an LNC instance to test against. |
Forgot to update backends/LightingNodeConnect.ts in original PR, so that is done now. Tested it, but I'm a bit confused because with LNC backend the chan id in the actual route hint of the invoice differs (just a little bit for the last digits) to the chan id of the channel that was selected (e.g. 276BDF0000250000 vs. 276bdf0000250018). Not sure if that is a problem, waiting for tests from @kaloudis. |
ac572a3
to
cb6cb41
Compare
After investigating this I found out that this is no LNC specific problem (it was just luck that it didn't happen with LND). The problem was the conversion of the channel id to Number which sometimes leads to a loss of precision since the number is too big for javascript Number. Fortunately the backend also accepts it as string. Also resolved the conflicts. |
25ff8c7
to
46c037f
Compare
46c037f
to
2a34363
Compare
2a34363
to
82fa0f6
Compare
Tested with embedded LND. It generally works but there is an already existing problem with the channel ids. The backend (sometimes?) provides invalid ids so that retrieving channel infos sometimes does not work (edge not found). The cause is probably a conversion to Number which leads to a loss of precision. Example: Channel id from our node to Olympus testnet node: 3090127951852011500 (hex: 2AE257 0000A4 FFEC). FFEC is the tx output index which is wrong (channel point: 5587b84bf0fd43125eb3faea5f786c5c5e84bc06b4711e0b0fff906740fab1a5:0 -> output index is 0). This can be merged, the channel id problem should be fixed separately. |
Have you checked the raw responses from the endpoints? I'm curious if it's an issue with the REST API or our conversion somewhere. |
No, not yet. But again, I think it's really something for another PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
This pull request introduces optional 'custom route hints'. With custom route hints users can now decide which lightning channels are put into an invoice as additional information for the sender to be able to use those channels for the payment. In practice this enables users to top up specific channels by receiving sats via any lightning payment or by using external swap services.
Additionally:
This pull request is categorized as a:
Checklist
yarn run tsc
and made sure my code compiles correctlyyarn run lint
and made sure my code didn’t contain any problematic patternsyarn run prettier
and made sure my code is formatted correctlyyarn run test
and made sure all of the tests passTesting
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
Locales
Third Party Dependencies and Packages
yarn
after this PR is merged inpackage.json
andyarn.lock
have been properly updatedOther: