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

BOLT2: both nodes send `update_add_htlc` #473

Merged
merged 1 commit into from Oct 29, 2018

Conversation

Projects
None yet
5 participants
@nayuta-ueno
Contributor

nayuta-ueno commented Sep 3, 2018

issue #472

In this sequence, three commitment_signed and revoke_and_ack are required.

@TheBlueMatt

This comment has been minimized.

Contributor

TheBlueMatt commented Sep 3, 2018

This should not be the case. When B sends A a commitment_signed it should absolutely include any pending update_adds from B.

@nayuta-ueno

This comment has been minimized.

Contributor

nayuta-ueno commented Sep 7, 2018

@TheBlueMatt

When B sends A a commitment_signed it should absolutely include any pending update_adds from B.

When B receive revoke_and_ack(7), B have signed commitment transaction HTLC1, 2 (HTLC3 has only applied).
Therefore 'A' need sends commitment_singed(8) like #472 .

@pm47

pm47 approved these changes Sep 7, 2018 edited

@nayuta-ueno is right, A acknowledges the incoming htlc (3) at step (7), only then will this hltlc be signed by A.

@nayuta-ueno

This comment has been minimized.

Contributor

nayuta-ueno commented Sep 7, 2018

@pm47 Thank you for your review!
(I'm not @ ueno but @nayuta-ueno :)

@TheBlueMatt

Oops, oops, yes, you're right, I'd missed the direction of the last HTLC.

@rustyrussell

This comment has been minimized.

Collaborator

rustyrussell commented Sep 17, 2018

You are correct, the example was unclear. Perhaps deliberately so, as below it says:

what matters is whether both sides have irrevocably committed to a
particular HTLC or not (the final state, above).

Which implies that it's deliberate that one side is committed to the HTLC and the other isn't? We should probably spell out the states at each point for each side, but that's a separate commit.

@sstone

sstone approved these changes Oct 18, 2018

@rustyrussell rustyrussell merged commit 1ddc1a5 into lightningnetwork:master Oct 29, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nayuta-ueno nayuta-ueno deleted the nayuta-ueno:both_add_htlc branch Oct 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment