-
Notifications
You must be signed in to change notification settings - Fork 371
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
Interpret sync_complete in reply_channel_range #790
Conversation
LN Spec PR lightningdevkit#826 changes full_information to indicate completion of a sequence of reply_channel_range messages.
Codecov Report
@@ Coverage Diff @@
## main #790 +/- ##
==========================================
- Coverage 90.79% 90.78% -0.02%
==========================================
Files 38 38
Lines 23168 23157 -11
==========================================
- Hits 21036 21023 -13
- Misses 2132 2134 +2
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review ACK 77690fa
action: ErrorAction::IgnoreError, | ||
}); | ||
} | ||
log_debug!(self.logger, "Handling reply_channel_range peer={}, first_blocknum={}, number_of_blocks={}, sync_complete={}, scids={}", log_pubkey!(their_node_id), msg.first_blocknum, msg.number_of_blocks, msg.sync_complete, msg.short_channel_ids.len(),); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay if I follow correctly, the fact we're stateless means we can't qualify if the range is effectively complete and sync_complete
sets properly. But we don't care as peer was able to equivocate before, logging is best we can do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right - we don't care if a given message indicates the end or not - we just respond to them as they come in.
Addresses changes to
reply_channel_range
per point 1 of lightning/bolts#826This PR
full_information
tosync_complete
full_information=false
as a failure response