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

pay: fix handling of legacy vs tlv encoding. #4035

Merged

Conversation

rustyrussell
Copy link
Contributor

As revealed by the failure of tests in #3936, where we ended up trying
to send a partial payment using legacy style, we are not handling
style properly.

  1. BOLT9 has features, so we can know that the destination supports
    MPP. We may not have seen a node_announcement.
  2. We can't assume that nodes inside routehints support TLV.
  3. We can't assume direct peers support TLV.

The keysend code tried to fix this up, so I'm not sure that this caused
the issue in #3968, though.

Signed-off-by: Rusty Russell [email protected]

As revealed by the failure of tests in ElementsProject#3936, where we ended up trying
to send a partial payment using legacy style, we are not handling
style properly.

1. BOLT9 has features, so we can *know* that the destination supports
   MPP.  We may not have seen a node_announcement.
2. We can't assume that nodes inside routehints support TLV.
3. We can't assume direct peers support TLV.

The keysend code tried to fix this up, so I'm not sure that this caused
the issue in ElementsProject#3968, though.

Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: `pay` will now make reliable multi-part payments to nodes it doesn't have a node_announcement for.
@cdecker
Copy link
Member

cdecker commented Sep 10, 2020

ACK 06cc838

@rustyrussell rustyrussell merged commit 191355e into ElementsProject:master Sep 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants