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: Add routehints argument to the keysend command #4611

Merged
merged 4 commits into from
Jul 15, 2021

Conversation

cdecker
Copy link
Member

@cdecker cdecker commented Jun 18, 2021

So far we could not reach non-publicly reachable nodes with the
keysend command since we couldn't compute a route to them. With this
change we can add some routehints that we may have gotten from an
address book or a previous invoice, and make it more likely that we
can reach the destination.

Builds on top of #4610 to avoid rebase conflicts, but I can separate them out if that's preferred. This PR consists only of the last 3 commits.

Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 553fd61

Trivial changes only.

Have no firm opinion on rebase or not, that's @niftynei ....

common/json_tok.c Outdated Show resolved Hide resolved
name, json_strdup(tmpctx, buffer, tok));
}

*ris = tal_arr(cmd, struct route_info *, 0);
Copy link
Contributor

Choose a reason for hiding this comment

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

Here too, tok->size?

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried this but there are just too many levels of indirection and my head started hurting: we're passing a pointer of position in an array, which should then be initialized in the callee, and is itself a variable length array. What would the canonical way of doing this be? And is there a trick to make this clearer in the code (parentheses, ...)?

plugins/keysend.c Outdated Show resolved Hide resolved
plugins/keysend.c Outdated Show resolved Hide resolved
tests/test_pay.py Outdated Show resolved Hide resolved
tests/test_pay.py Outdated Show resolved Hide resolved
@cdecker cdecker force-pushed the keysend-routehints branch 2 times, most recently from 15bb806 to 8b22989 Compare June 25, 2021 11:42
@niftynei niftynei added this to the v0.10.1 milestone Jun 28, 2021
@rustyrussell
Copy link
Contributor

Ack: b3b3f53

Incoming HTLCs are rejected by the HTLC logic if the payload contains
an even type that `lightningd` doesn't recognize. This is to prevent
us from accidentally accepting a payment that has extra semantics
attached (for example if we get a keysend payment and don't know what
to do with the TLV field containing the message we should reject it,
otherwise the overall semantics of the message delivery fail).
We'll start passing routehints manually to keysend to reach non-public
nodes as well.
So far we could not reach non-publicly reachable nodes with the
keysend command since we couldn't compute a route to them. With this
change we can add some routehints that we may have gotten from an
address book or a previous invoice, and make it more likely that we
can reach the destination.

Changelog-Added: keysend: `keysend` can now reach non-public nodes by providing the `routehints` argument if they are known.
@niftynei
Copy link
Collaborator

ACK 4b42c3c

@niftynei niftynei merged commit 0f18f20 into ElementsProject:master Jul 15, 2021
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.

3 participants