-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
lnrpc: expose linked htlcs in ListChannels #4693
lnrpc: expose linked htlcs in ListChannels #4693
Conversation
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.
Just nits ✨
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.
On second look, itests are panicing.
368e3c8
to
5df6754
Compare
952c623
to
ea16465
Compare
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, going to be very useful to have this info surfaced!
}, | ||
) | ||
|
||
// If the incoming channel id is the special hop.Source |
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.
s/hop.Source/hop. Source
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.
hop.Source
is a symbol :)
// For an incoming htlc, it is the outgoing channel. When the htlc | ||
// originates from this node or this node is the final destination, | ||
// forwarding_channel will be zero. | ||
uint64 forwarding_channel = 6; |
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.
nit: since we can have a nil outgoing circuit, is it possible that we have a forwarded htlc that will have a 0 forwarding channel (temporarily)? Not the most familiar with that part of the switch, if possible perhaps update the comment so that people don't rely on 0 value meaning own payment.
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.
Comment added. I considered making this state explicit, but I don't think it is necessary. Htlcs that need to be forwarded, but haven't even reached the circuit map will also be reported with a zero forwarding_channel
.
ea16465
to
49df2d6
Compare
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 👍
Anything blocking merge on this one? |
No, forgot you don't have merge powers anymore tbh :) Travis looks confused over the "sample configuration check" - happened to me on a PR that was opened before it was added. Restarting the actions now, then will merge if that does the trick, otherwise you may need to push again to jump it. |
@joostjager won't you push again? Looks like sample config check is stuck. |
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 😎
Fixes #4687