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 HTLCs rexmit out of order #4124

Conversation

rustyrussell
Copy link
Contributor

As noted in this morning's spec call, I tracked it down.

@rustyrussell rustyrussell added this to the v0.9.2 milestone Oct 13, 2020
Copy link
Member

@cdecker cdecker left a comment

Choose a reason for hiding this comment

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

ACK 324b846

channeld/full_channel.c Outdated Show resolved Hide resolved
tests/test_connection.py Outdated Show resolved Hide resolved
tests/test_connection.py Outdated Show resolved Hide resolved
tests/test_connection.py Outdated Show resolved Hide resolved
@cdecker cdecker force-pushed the guilt/htlcs-rexmit-out-of-order branch from d5243d4 to 324b846 Compare October 13, 2020 19:59
We didn't care, but other implementations (particularly lnd) do.  And it
does violate the spec.

(We need to use skip not xfail on the test which catches this, since
xfail doesn't seem to stop errors reported by cleanup)

(Includes Christian's typo fix!)

Signed-off-by: Rusty Russell <[email protected]>
@rustyrussell rustyrussell force-pushed the guilt/htlcs-rexmit-out-of-order branch from fed7c71 to f417840 Compare October 14, 2020 00:28
@rustyrussell
Copy link
Contributor Author

OK, wove @cdecker fixes into commits, so self-ack:

Ack f417840

[ Includes tidyups by Christian Decker ]

Signed-off-by: Rusty Russell <[email protected]>
We had one report of this, and then Eugene and Roasbeef of Lightning
Labs confirmed it; they saw misordered HTLCs on reconnection too.

Since we didn't enforce this when we receive HTLCs, we never noticed :(

Fixes: ElementsProject#3920
Signed-off-by: Rusty Russell <[email protected]>
Changelog-Fixed: Protocol: fixed retransmission order of multiple new HTLCs (causing channel close with LND)
@rustyrussell rustyrussell force-pushed the guilt/htlcs-rexmit-out-of-order branch from f417840 to 7f4b11e Compare October 14, 2020 05:42
@rustyrussell rustyrussell merged commit a3c3044 into ElementsProject:master Oct 14, 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