Skip to content

Interpret sync_complete in reply_channel_range #790

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Feb 4, 2021
Merged

Interpret sync_complete in reply_channel_range #790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions lightning/src/ln/msgs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,9 +602,8 @@ pub struct ReplyChannelRange {
pub first_blocknum: u32,
/// The number of blocks included in the range of the reply
pub number_of_blocks: u32,
/// Indicates if the query recipient maintains up-to-date channel
/// information for the chain_hash
pub full_information: bool,
/// True when this is the final reply for a query
pub sync_complete: bool,
/// The short_channel_ids in the channel range
pub short_channel_ids: Vec<u64>,
}
Expand Down Expand Up @@ -1727,7 +1726,7 @@ impl Readable for ReplyChannelRange {
let chain_hash: BlockHash = Readable::read(r)?;
let first_blocknum: u32 = Readable::read(r)?;
let number_of_blocks: u32 = Readable::read(r)?;
let full_information: bool = Readable::read(r)?;
let sync_complete: bool = Readable::read(r)?;

// We expect the encoding_len to always includes the 1-byte
// encoding_type and that short_channel_ids are 8-bytes each
Expand Down Expand Up @@ -1755,7 +1754,7 @@ impl Readable for ReplyChannelRange {
chain_hash,
first_blocknum,
number_of_blocks,
full_information,
sync_complete,
short_channel_ids
})
}
Expand All @@ -1768,7 +1767,7 @@ impl Writeable for ReplyChannelRange {
self.chain_hash.write(w)?;
self.first_blocknum.write(w)?;
self.number_of_blocks.write(w)?;
self.full_information.write(w)?;
self.sync_complete.write(w)?;

encoding_len.write(w)?;
(EncodingType::Uncompressed as u8).write(w)?;
Expand Down Expand Up @@ -2569,7 +2568,7 @@ mod tests {
chain_hash: expected_chain_hash,
first_blocknum: 756230,
number_of_blocks: 1500,
full_information: true,
sync_complete: true,
short_channel_ids: vec![0x000000000000008e, 0x0000000000003c69, 0x000000000045a6c4],
};

Expand All @@ -2582,7 +2581,7 @@ mod tests {
assert_eq!(reply_channel_range.chain_hash, expected_chain_hash);
assert_eq!(reply_channel_range.first_blocknum, 756230);
assert_eq!(reply_channel_range.number_of_blocks, 1500);
assert_eq!(reply_channel_range.full_information, true);
assert_eq!(reply_channel_range.sync_complete, true);
assert_eq!(reply_channel_range.short_channel_ids[0], 0x000000000000008e);
assert_eq!(reply_channel_range.short_channel_ids[1], 0x0000000000003c69);
assert_eq!(reply_channel_range.short_channel_ids[2], 0x000000000045a6c4);
Expand Down
31 changes: 2 additions & 29 deletions lightning/src/routing/network_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,18 +264,7 @@ impl<C: Deref + Sync + Send, L: Deref + Sync + Send> RoutingMessageHandler for N
/// does not match our chain_hash will be rejected when the announcement is
/// processed.
fn handle_reply_channel_range(&self, their_node_id: &PublicKey, msg: ReplyChannelRange) -> Result<(), LightningError> {
log_debug!(self.logger, "Handling reply_channel_range peer={}, first_blocknum={}, number_of_blocks={}, full_information={}, scids={}", log_pubkey!(their_node_id), msg.first_blocknum, msg.number_of_blocks, msg.full_information, msg.short_channel_ids.len(),);

// Validate that the remote node maintains up-to-date channel
// information for chain_hash. Some nodes use the full_information
// flag to indicate multi-part messages so we must check whether
// we received SCIDs as well.
if !msg.full_information && msg.short_channel_ids.len() == 0 {
return Err(LightningError {
err: String::from("Received reply_channel_range with no information available"),
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(),);
Copy link

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.

Copy link
Collaborator

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.


log_debug!(self.logger, "Sending query_short_channel_ids peer={}, batch_size={}", log_pubkey!(their_node_id), msg.short_channel_ids.len());
let mut pending_events = self.pending_events.lock().unwrap();
Expand Down Expand Up @@ -2015,7 +2004,7 @@ mod tests {
{
let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, ReplyChannelRange {
chain_hash,
full_information: true,
sync_complete: true,
first_blocknum: 0,
number_of_blocks: 2000,
short_channel_ids: vec![
Expand Down Expand Up @@ -2048,22 +2037,6 @@ mod tests {
_ => panic!("expected MessageSendEvent::SendShortIdsQuery"),
}
}

// Test receipt of a reply that indicates the remote node does not maintain up-to-date
// information for the chain_hash. Because of discrepancies in implementation we use
// full_information=false and short_channel_ids=[] as the signal.
{
// Handle the reply indicating the peer was unable to fulfill our request.
let result = net_graph_msg_handler.handle_reply_channel_range(&node_id_1, ReplyChannelRange {
chain_hash,
full_information: false,
first_blocknum: 1000,
number_of_blocks: 100,
short_channel_ids: vec![],
});
assert!(result.is_err());
assert_eq!(result.err().unwrap().err, "Received reply_channel_range with no information available");
}
}

#[test]
Expand Down