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.

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

Ignore partially-pruned channels during routing #3038

Merged
merged 1 commit into from
May 21, 2024
Merged

Ignore partially-pruned channels during routing #3038

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
8 changes: 6 additions & 2 deletions lightning/src/routing/gossip.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,7 @@ impl ChannelInfo {
/// Returns a [`DirectedChannelInfo`] for the channel directed to the given `target` from a
/// returned `source`, or `None` if `target` is not one of the channel's counterparties.
pub fn as_directed_to(&self, target: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> {
if self.one_to_two.is_none() || self.two_to_one.is_none() { return None; }
let (direction, source, outbound) = {
if target == &self.node_one {
(self.two_to_one.as_ref(), &self.node_two, false)
Expand All @@ -888,12 +889,14 @@ impl ChannelInfo {
return None;
}
};
direction.map(|dir| (DirectedChannelInfo::new(self, dir, outbound), source))
let dir = direction.expect("We checked that both directions are available at the start");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, one quick question.

Usage of expect here would create an explicit panic in case direction.is_none().
But would explicitly panicking in a pub fn be a good idea?

Should we consider returning a Result<Ok, Err>, instead of panicking within the function?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the text in the expect, we expect this to be unreachable :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! Right!
Then it makes sense! :)

Some((DirectedChannelInfo::new(self, dir, outbound), source))
}

/// Returns a [`DirectedChannelInfo`] for the channel directed from the given `source` to a
/// returned `target`, or `None` if `source` is not one of the channel's counterparties.
pub fn as_directed_from(&self, source: &NodeId) -> Option<(DirectedChannelInfo, &NodeId)> {
if self.one_to_two.is_none() || self.two_to_one.is_none() { return None; }
let (direction, target, outbound) = {
if source == &self.node_one {
(self.one_to_two.as_ref(), &self.node_two, true)
Expand All @@ -903,7 +906,8 @@ impl ChannelInfo {
return None;
}
};
direction.map(|dir| (DirectedChannelInfo::new(self, dir, outbound), target))
let dir = direction.expect("We checked that both directions are available at the start");
Some((DirectedChannelInfo::new(self, dir, outbound), target))
}

/// Returns a [`ChannelUpdateInfo`] based on the direction implied by the channel_flag.
Expand Down
192 changes: 120 additions & 72 deletions lightning/src/routing/router.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5466,6 +5466,18 @@ mod tests {
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
update_channel(&gossip_sync, &secp_ctx, &privkeys[3], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 5,
timestamp: 2,
flags: 3, // disable direction 1
cltv_expiry_delta: 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: 200_000,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});

// Path via {node7, node2, node4} is channels {12, 13, 6, 11}.
// Add 100 sats to the capacities of {12, 13}, because these channels
Expand Down Expand Up @@ -5643,6 +5655,18 @@ mod tests {
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
update_channel(&gossip_sync, &secp_ctx, &privkeys[3], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 5,
timestamp: 2,
flags: 3, // disable direction 1
cltv_expiry_delta: 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: 200_000,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});

// Path via {node7, node2, node4} is channels {12, 13, 6, 11}.
// Add 100 sats to the capacities of {12, 13}, because these channels
Expand Down Expand Up @@ -5816,6 +5840,18 @@ mod tests {
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
update_channel(&gossip_sync, &secp_ctx, &privkeys[3], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 5,
timestamp: 2,
flags: 3, // Disable direction 1
cltv_expiry_delta: 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: 100_000,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});

// Path via {node7, node2, node4} is channels {12, 13, 6, 11}.
// All channels should be 100 sats capacity. But for the fee experiment,
Expand Down Expand Up @@ -6208,92 +6244,104 @@ mod tests {
let payment_params = PaymentParameters::from_node_id(nodes[6], 42);

add_channel(&gossip_sync, &secp_ctx, &our_privkey, &privkeys[1], ChannelFeatures::from_le_bytes(id_to_feature_flags(6)), 6);
update_channel(&gossip_sync, &secp_ctx, &our_privkey, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 6,
timestamp: 1,
flags: 0,
cltv_expiry_delta: (6 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
for (key, flags) in [(&our_privkey, 0), (&privkeys[1], 3)] {
update_channel(&gossip_sync, &secp_ctx, key, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 6,
timestamp: 1,
flags,
cltv_expiry_delta: (6 << 4) | 0,
Copy link
Member

@shaavan shaavan May 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also a tangential question, but why are we writing cltv_expiry_delta: (x<<4) | 0 in this test when it is the same as writing cltv_expiry_delta: (x<<4)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also |1 in some places and it just keeps the same formatting.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright! That makes sense!
Thanks for the clarification! :)

htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
}
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[1], NodeFeatures::from_le_bytes(id_to_feature_flags(1)), 0);

add_channel(&gossip_sync, &secp_ctx, &privkeys[1], &privkeys[4], ChannelFeatures::from_le_bytes(id_to_feature_flags(5)), 5);
update_channel(&gossip_sync, &secp_ctx, &privkeys[1], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 5,
timestamp: 1,
flags: 0,
cltv_expiry_delta: (5 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 100,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
for (key, flags) in [(&privkeys[1], 0), (&privkeys[4], 3)] {
update_channel(&gossip_sync, &secp_ctx, key, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 5,
timestamp: 1,
flags,
cltv_expiry_delta: (5 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 100,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
}
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[4], NodeFeatures::from_le_bytes(id_to_feature_flags(4)), 0);

add_channel(&gossip_sync, &secp_ctx, &privkeys[4], &privkeys[3], ChannelFeatures::from_le_bytes(id_to_feature_flags(4)), 4);
update_channel(&gossip_sync, &secp_ctx, &privkeys[4], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 4,
timestamp: 1,
flags: 0,
cltv_expiry_delta: (4 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
for (key, flags) in [(&privkeys[4], 0), (&privkeys[3], 3)] {
update_channel(&gossip_sync, &secp_ctx, key, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 4,
timestamp: 1,
flags,
cltv_expiry_delta: (4 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
}
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[3], NodeFeatures::from_le_bytes(id_to_feature_flags(3)), 0);

add_channel(&gossip_sync, &secp_ctx, &privkeys[3], &privkeys[2], ChannelFeatures::from_le_bytes(id_to_feature_flags(3)), 3);
update_channel(&gossip_sync, &secp_ctx, &privkeys[3], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 3,
timestamp: 1,
flags: 0,
cltv_expiry_delta: (3 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
for (key, flags) in [(&privkeys[3], 0), (&privkeys[2], 3)] {
update_channel(&gossip_sync, &secp_ctx, key, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 3,
timestamp: 1,
flags,
cltv_expiry_delta: (3 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
}
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[2], NodeFeatures::from_le_bytes(id_to_feature_flags(2)), 0);

add_channel(&gossip_sync, &secp_ctx, &privkeys[2], &privkeys[4], ChannelFeatures::from_le_bytes(id_to_feature_flags(2)), 2);
update_channel(&gossip_sync, &secp_ctx, &privkeys[2], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 2,
timestamp: 1,
flags: 0,
cltv_expiry_delta: (2 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
for (key, flags) in [(&privkeys[2], 0), (&privkeys[4], 3)] {
update_channel(&gossip_sync, &secp_ctx, key, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 2,
timestamp: 1,
flags,
cltv_expiry_delta: (2 << 4) | 0,
htlc_minimum_msat: 0,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
}

add_channel(&gossip_sync, &secp_ctx, &privkeys[4], &privkeys[6], ChannelFeatures::from_le_bytes(id_to_feature_flags(1)), 1);
update_channel(&gossip_sync, &secp_ctx, &privkeys[4], UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 1,
timestamp: 1,
flags: 0,
cltv_expiry_delta: (1 << 4) | 0,
htlc_minimum_msat: 100,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
for (key, flags) in [(&privkeys[4], 0), (&privkeys[6], 3)] {
update_channel(&gossip_sync, &secp_ctx, key, UnsignedChannelUpdate {
chain_hash: ChainHash::using_genesis_block(Network::Testnet),
short_channel_id: 1,
timestamp: 1,
flags,
cltv_expiry_delta: (1 << 4) | 0,
htlc_minimum_msat: 100,
htlc_maximum_msat: MAX_VALUE_MSAT,
fee_base_msat: 0,
fee_proportional_millionths: 0,
excess_data: Vec::new()
});
}
add_or_update_node(&gossip_sync, &secp_ctx, &privkeys[6], NodeFeatures::from_le_bytes(id_to_feature_flags(6)), 0);

{
Expand Down
Loading