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

BOLT #2: order htlc_signatures by BIP69 + increasing CLTV. #491

Merged
merged 2 commits into from Jan 22, 2019

Conversation

@rustyrussell
Copy link
Collaborator

commented Oct 21, 2018

We express it has how the outputs are ordered, but the only way you can
detect that is by the htlc_signatures order, which is the part which really
matters.

I finally reproduced this, BTW, which is why I'm digging it up!

Closes: #448
Signed-off-by: Rusty Russell rusty@rustcorp.com.au

@rustyrussell rustyrussell requested a review from pm47 Oct 21, 2018

@pm47

pm47 approved these changes Oct 21, 2018

Copy link
Collaborator

left a comment

This is the simplest way to fix #448 I think.

I have just one bikeshed comment, mainly to try out the new suggestion github feature.

02-peer-protocol.md Outdated Show resolved Hide resolved
@TheBlueMatt

This comment has been minimized.

Copy link
Collaborator

commented Oct 29, 2018

Can we just throw out the reference to BIP 69 and write out the full context there? (#456). This is really actually just a bug in BIP 69 as it should specify everything you need, but its a very poorly-written BIP.

@rustyrussell rustyrussell added this to the v1.0 milestone Nov 26, 2018

@cfromknecht cfromknecht referenced this pull request Nov 28, 2018
@rustyrussell

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 29, 2018

Implemented in c-lightning. Waiting on second implementation by LND or Eclair...

@araspitzu

This comment has been minimized.

Copy link
Contributor

commented Dec 27, 2018

What about adding test vectors for the resulting htlc transactions? Although we should test against the htlc_signatures (in commitment_signed) having a test vector that specify the order of the htcl-tx associated with the commit-tx would be helpful for the implementers.

@rustyrussell

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 7, 2019

We should definitely have more test vectors, though this should be applied meanwhile IMHO.

rustyrussell and others added some commits Oct 21, 2018

BOLT #2: order htlc_signatures by BIP69 + increasing CLTV.
We express it has how the outputs are ordered, but the only way you can
detect that is by the htlc_signatures order, which is the part which really
matters.

I finally reproduced this, BTW, which is why I'm digging it up!

Closes: #448
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Update 02-peer-protocol.md
Co-Authored-By: rustyrussell <rusty@rustcorp.com.au>

@cdecker cdecker force-pushed the rustyrussell:bip69-plus branch from 594d502 to abe027c Jan 22, 2019

@cdecker

This comment has been minimized.

Copy link
Collaborator

commented Jan 22, 2019

Merging as decided during the 2019/01/21 IRC meeting.

@cdecker cdecker merged commit 1f4538c into lightningnetwork:master Jan 22, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.