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

fix(lnd): add jstype to channel ids #1036

Merged
merged 1 commit into from
Jun 17, 2019
Merged

fix(lnd): add jstype to channel ids #1036

merged 1 commit into from
Jun 17, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jun 16, 2019

In this PR we're adding jstype = JS_STRING back to lnd proto definitions. They were originally added via #747 and removed when the hold invoices feature was merged (35ae4f6#diff-b455b61269f578b306ff6d7f238d0aa7).

This fixes the UnknownNextPeer output described in #984 when using SendToRoute.

@ghost ghost requested review from sangaman and offerm June 16, 2019 17:11
Copy link
Contributor

@offerm offerm left a comment

Choose a reason for hiding this comment

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

Looks good to me if this was the solution used in the past.

Only question I have is for @sangaman - Why lightningnetwork/lnd#2352 was closed? Did you find a different solution ?

@ghost
Copy link
Author

ghost commented Jun 17, 2019

I was wondering the same about lightningnetwork/lnd#2352. It would be great to have some kind of mechanism in place to prevent this from happening in the future.

@offerm offerm merged commit fd11eef into master Jun 17, 2019
@ghost ghost deleted the fix/lnd-proto branch June 17, 2019 09:19
@sangaman
Copy link
Collaborator

Only question I have is for @sangaman - Why lightningnetwork/lnd#2352 was closed? Did you find a different solution ?

I must have deleted the branch by accident after the PR didn't get picked up after several months. I'm going to rebase the branch and reopen the PR as it would be helpful to prevent these sorts of things in the future.

@sangaman sangaman assigned ghost Jun 25, 2019
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.

2 participants