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

bolt2: give the possibility to the sender to be outdated #1428

Conversation

vincenzopalazzo
Copy link
Contributor

@vincenzopalazzo vincenzopalazzo commented Apr 16, 2022

This PR gives the possibility to the remote peer to be outdated when is trying to reconnect to us.

I will open this in the draft because it is missing the test case, and also because I'm new in the code base and I can make some wrong assumptions.

Fixes #1207

Signed-off-by: Vincenzo Palazzo vincenzopalazzodev@gmail.com

This commit give the possibility to the remote peer to be outdate when is trying to reconnect to us.

Fixes lightningdevkit#1207

Signed-off-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
@codecov-commenter
Copy link

codecov-commenter commented Apr 16, 2022

Codecov Report

Merging #1428 (9fd0292) into main (83595db) will decrease coverage by 0.04%.
The diff coverage is 29.62%.

@@            Coverage Diff             @@
##             main    #1428      +/-   ##
==========================================
- Coverage   90.84%   90.79%   -0.05%     
==========================================
  Files          74       74              
  Lines       41288    41314      +26     
  Branches    41288    41314      +26     
==========================================
+ Hits        37507    37511       +4     
- Misses       3781     3803      +22     
Impacted Files Coverage Δ
lightning/src/ln/channelmanager.rs 84.57% <20.00%> (-0.09%) ⬇️
lightning/src/ln/channel.rs 88.03% <31.81%> (-0.34%) ⬇️
lightning/src/ln/functional_tests.rs 97.08% <0.00%> (-0.07%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83595db...9fd0292. Read the comment docs.

@TheBlueMatt
Copy link
Collaborator

Hmm, I believe at this point we've already sent our reestablish message (we do it immediately on connect, so it should fly before we receive any messages from the peer). Instead, I think we can return a ChannelError::Warn so that we sent the counterparty a warning message.

Further, I think we actually really want to keep the PeerDisconnected flag in this case. This will effectively mean we'll force-close the channel if the peer tries to use the channel, and it'll mean we keep the channel marked disabled until the peer sends a new reestablish message. Because I'm too lazy to carefully review the rest of the function, the simplest way to do this would be to simply add the "are they behind" check immediately before the line where we clear the PeerDisconnected bit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Don'e force-close if a peer is behind our latest state on reestablish
3 participants