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

lnd sends a commit_sig that doesn't include any updates #1127

Closed
pm47 opened this issue Apr 25, 2018 · 10 comments
Closed

lnd sends a commit_sig that doesn't include any updates #1127

pm47 opened this issue Apr 25, 2018 · 10 comments
Assignees
Labels
commitments Commitment transactions containing the state of the channel p2p Code related to the peer-to-peer behaviour
Milestone

Comments

@pm47
Copy link

pm47 commented Apr 25, 2018

Background

Still tracking down occasional issues between eclair and lnd, I think I was able to isolate this one on lnd's side.

A long-lived channel was closed because, according to eclair, lnd sent a commit_sig that didn't include any changes, which is a protocol violation.

Your environment

  • yalls mainnet nodeid=03e50492eab4107a773141bb419e107bda3de3d55652e6e1a41225f06a0bbf2d56
  • channelid=5002e7b1fe9243c716e6004ee7d9bfcf95b2ec15c1a1ddcfd80ae4b5f06bd29d
  • version of lnd: 0.4.1-beta commit=8af80bfc5cdebcd323ed2a50ea0497df6e9bc6e4 (info from @alexbosworth)

Actual behaviour

  lnd                eclair
   |                   |
   |        ...        | 
   |                   | <--- idle (no htlc on either side) 
   |                   |
   |<----- add-1 ------|
   |<------ sig -------|
   |<----- add-2 ------|
   |------- rev ------>| <--- lnd acks add-1
   |------- sig ------>| <--- lnd signs add-1
   |<------ sig -------|
   |------- rev ------>| <--- lnd acks add-2
   |<------ rev -------|
   |------ ful-1 ----->|
   |------- sig ------>| <--- lnd signs add-1 + add-2 + ful-1 = add-2
   |<------ rev -------|
   |<------ sig -------|
   |------ ful-2 ----->|
   |------- sig ------>| <--- lnd signs add-2 + ful-2 = no htlcs
   |------- rev ------>|
   |<------ rev -------| <--- eclair acks that there are no more htlcs
   |<------ sig -------|
   |------- rev ------>| <--- lnd acks that there are no htlcs on eclair's side
   |------- sig ------>| <--- this signature doesn't include anything new
   |<------ err -------| <--- cannot sign without changes!
   |                   |
@meshcollider meshcollider added p2p Code related to the peer-to-peer behaviour commitments Commitment transactions containing the state of the channel labels Apr 27, 2018
@Roasbeef Roasbeef added this to the 0.4.2-beta milestone May 2, 2018
@halseth
Copy link
Contributor

halseth commented May 2, 2018

@pm47 Thanks for the detailed description! I have been able to reproduce the scenario, and will have a fix up in a bit.

@n1bor
Copy link

n1bor commented May 17, 2018

Does this result in a loss of funds to the LND node? Just occurred on channel channelId=88275f320965874c1bc2010dd694ff53cccf7ebf1710ae3f418a592d7016cdb9 and the other side thinks they have lost funds - but I am assuming they just need to wait for the timeouts?

@Roasbeef
Copy link
Member

Roasbeef commented May 17, 2018 via email

@n1bor
Copy link

n1bor commented May 17, 2018

Looks like Eclair published it - as LND breached the rules so eclair closed channel.

And reassured them that I did not steel them! https://blockchain.info/tx-index/348778808 I assume is the tx that spent the timeouts? So they should own bc1q4mnhlnkk48ds0uy0pftz2jpv5z0nsaqv8r8dyy.

@Roasbeef
Copy link
Member

All funds in that channel have been fully swept. This is the commitment transaction: https://www.smartbit.com.au/tx/5b9d9d742ab6c07fa26d6d74100ccaa216650d08103d76e968ae5f6d787bd527

@n1bor
Copy link

n1bor commented May 19, 2018

Counterpart confirmed that they did not lose any funds (just some fees). And that they were on old version of lnd so is the same issue - that is already fixed.
Also my audit patch on eclair was very useful - see output here https://lightningconductor.net/balances I as was very sure my balances were all as expected.

Better tools for users to reconcile balances in channels+bitcond wallet to what was send/received/earned in fees etc.. seem to be needed - as once scale up manually trawling logs will become impossible.

@pm47
Copy link
Author

pm47 commented Jun 15, 2018

Unfortunately I'm still running into this issue.

This happened between this node and ours. I don't know what version it runs or who is behind it, but since it is fairly big I suppose it is well maintained and up-to-date.

Here is what happened:

  lnd                eclair
   |                   |
   |        ...        | 
   |                   | <--- idle (6 pending htlcs on both sides) 
   |                   |
   |                   |
   |<----- add-1 ------|
   |<------ sig -------|
   |------- rev ------>| <--- lnd acks add-1
   |<----- add-2 ------|
   |------- sig ------>| <--- lnd signs add-1
   |<------ sig -------|
   |<------ rev -------|
   |------- rev ------>| <--- lnd acks add-2
   |                   |
   |----- fail-1  ---->|
   |------- sig ------>| <--- lnd signs add-2 + fail-1
   |<------ rev -------|
   |<------ sig -------|
   |----- fail-2  ---->|
   |------- sig ------>| <--- lnd signs fail-2
   |------- rev ------>|
   |<------ rev -------|
   |<------ sig -------|
   |------- rev ------>|
   |------- sig ------>| <--- this signature doesn't include anything new
   |<------ err -------| <--- cannot sign without changes!
   |                   |

@Roasbeef
Copy link
Member

Roasbeef commented Jun 15, 2018 via email

@n1bor
Copy link

n1bor commented Jun 15, 2018

@pm47 wondering if we need to be more lenient? Feels a bit like the IE/Firefox/Chrome arguments over html. I.e. should it be valid XML or should we just do the best we can. As all users care about is channels staying up not if an implementation sticks to the specs.
So what harm is there if we get sent a sig with no changes? Obviously if you get 100 in a row close the channel but otherwise just keep it up?

@pm47
Copy link
Author

pm47 commented Jun 15, 2018

Not sure if I would call 64 channels big lol.

By capacity they are, apparently, but whatever. Maybe all their channels got closed because of this bug? 😝

@pm47 wondering if we need to be more lenient? Feels a bit like the IE/Firefox/Chrome arguments over html.

I hope eclair isn't IE in your analogy ;-). It's much more low level than that, more like what if they spoke different TCP: that just wouldn't work. Besides, nobody here is arguing that this issue shouldn't be fixed, and actually it has been fixed. I guess they just haven't yet updated their node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
commitments Commitment transactions containing the state of the channel p2p Code related to the peer-to-peer behaviour
Projects
None yet
Development

No branches or pull requests

5 participants