Fix channel_reestablish generation/handling around next_remote. #261
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ariard
reviewed
Dec 2, 2018
| msgs::ChannelReestablish { | ||
| channel_id: self.channel_id(), | ||
| next_local_commitment_number: INITIAL_COMMITMENT_NUMBER - self.cur_local_commitment_transaction_number, | ||
| next_remote_commitment_number: INITIAL_COMMITMENT_NUMBER - self.cur_remote_commitment_transaction_number - | ||
| if self.channel_state & (ChannelState::FundingSent as u32 | ChannelState::OurFundingLocked as u32) == (ChannelState::FundingSent as u32) { 1 } else { 0 }, | ||
| // We have to set next_remote_commitment_number to the next revoke_and_ack we expect to |
There was a problem hiding this comment.
Hmm yes bolt is quite unclear on dissociation between next_local_commitment_number and next_remote_commitment_number. Maybe comment could be more meaningful : "next_local_commitment_number is future index of our revocation secret for our next local_commitment_tx, whereas next_remote_commitment_number is present index at which we expect remote node reveals secret for its actual remote commitment tx secret in its next revoke_and_ack" Don't know if it's clearer but at least I've tried :)
There was a problem hiding this comment.
I added a ton more text, hopefully its all clear enough now.
TheBlueMatt
force-pushed
the
2018-11-reestablish-fix
branch
from
dcc7e84 to
23c2cef
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this reestablish actually works in practice and channels in rust-lightning-bitcoinrpc survive restart no problem.