use bitcoin::blockdata::constants::genesis_block;
use bitcoin::hash_types::BlockHash;
use bitcoin::network::constants::Network;
-use chain::channelmonitor::ChannelMonitor;
+use chain::channelmonitor::{ANTI_REORG_DELAY, ChannelMonitor};
use chain::transaction::OutPoint;
use chain::{ChannelMonitorUpdateErr, Listen, Watch};
use ln::channelmanager::{ChannelManager, ChannelManagerReadArgs, RAACommitmentOrder, PaymentSendFailure};
claim_payment(&nodes[1], &[&nodes[0]], payment_preimage);
}
+#[test]
+fn test_fail_htlc_on_broadcast_after_claim() {
+ // In an earlier version of 7e78fa660cec8a73286c94c1073ee588140e7a01 we'd also fail the inbound
+ // channel backwards if we received an HTLC failure after a HTLC fulfillment. Here we test a
+ // specific case of that by having the HTLC failure come from the ChannelMonitor after a dust
+ // HTLC was not included in a confirmed commitment transaction.
+ //
+ // We first forward a payment, then claim it with an update_fulfill_htlc message, closing the
+ // channel immediately before commitment occurs. After the commitment transaction reaches
+ // ANTI_REORG_DELAY confirmations, will will try to fail the HTLC which was already fulfilled.
+ let chanmon_cfgs = create_chanmon_cfgs(3);
+ let node_cfgs = create_node_cfgs(3, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]);
+ let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs);
+
+ create_announced_chan_between_nodes(&nodes, 0, 1, InitFeatures::known(), InitFeatures::known());
+ let chan_id_2 = create_announced_chan_between_nodes(&nodes, 1, 2, InitFeatures::known(), InitFeatures::known()).2;
+
+ let payment_preimage = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 2000).0;
+
+ let bs_txn = get_local_commitment_txn!(nodes[2], chan_id_2);
+ assert_eq!(bs_txn.len(), 1);
+
+ nodes[2].node.claim_funds(payment_preimage);
+ check_added_monitors!(nodes[2], 1);
+ let cs_updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
+ nodes[1].node.handle_update_fulfill_htlc(&nodes[2].node.get_our_node_id(), &cs_updates.update_fulfill_htlcs[0]);
+ let bs_updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
+ check_added_monitors!(nodes[1], 1);
+ expect_payment_forwarded!(nodes[1], Some(1000), false);
+
+ mine_transaction(&nodes[1], &bs_txn[0]);
+ check_closed_event!(nodes[1], 1, ClosureReason::CommitmentTxConfirmed);
+ check_closed_broadcast!(nodes[1], true);
+ connect_blocks(&nodes[1], ANTI_REORG_DELAY - 1);
+ check_added_monitors!(nodes[1], 1);
+ expect_pending_htlcs_forwardable!(nodes[1]);
+
+ nodes[0].node.handle_update_fulfill_htlc(&nodes[1].node.get_our_node_id(), &bs_updates.update_fulfill_htlcs[0]);
+ expect_payment_sent_without_paths!(nodes[0], payment_preimage);
+ commitment_signed_dance!(nodes[0], nodes[1], bs_updates.commitment_signed, true, true);
+ expect_payment_path_successful!(nodes[0]);
+}
+
fn do_update_fee_resend_test(deliver_update: bool, parallel_updates: bool) {
// In early versions we did not handle resending of update_fee on reconnect correctly. The
// chanmon_consistency fuzz target, of course, immediately found it, but we test a few cases
}
}
}
- macro_rules! maybe_update_channel_info {
- ( $target: expr, $src_node: expr) => {
+ macro_rules! check_update_latest {
+ ($target: expr) => {
if let Some(existing_chan_info) = $target.as_ref() {
// The timestamp field is somewhat of a misnomer - the BOLTs use it to
// order updates to ensure you always have the latest one, only
} else {
chan_was_enabled = false;
}
+ }
+ }
+ macro_rules! get_new_channel_info {
+ () => { {
let last_update_message = if msg.excess_data.len() <= MAX_EXCESS_BYTES_FOR_RELAY
{ full_msg.cloned() } else { None };
},
last_update_message
};
- $target = Some(updated_channel_update_info);
- }
+ Some(updated_channel_update_info)
+ } }
}
let msg_hash = hash_to_message!(&Sha256dHash::hash(&msg.encode()[..])[..]);
if msg.flags & 1 == 1 {
dest_node_id = channel.node_one.clone();
+ check_update_latest!(channel.two_to_one);
if let Some((sig, ctx)) = sig_info {
secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_two.as_slice()).map_err(|_| LightningError{
err: "Couldn't parse source node pubkey".to_owned(),
action: ErrorAction::IgnoreAndLog(Level::Debug)
})?, "channel_update");
}
- maybe_update_channel_info!(channel.two_to_one, channel.node_two);
+ channel.two_to_one = get_new_channel_info!();
} else {
dest_node_id = channel.node_two.clone();
+ check_update_latest!(channel.one_to_two);
if let Some((sig, ctx)) = sig_info {
secp_verify_sig!(ctx, &msg_hash, &sig, &PublicKey::from_slice(channel.node_one.as_slice()).map_err(|_| LightningError{
err: "Couldn't parse destination node pubkey".to_owned(),
action: ErrorAction::IgnoreAndLog(Level::Debug)
})?, "channel_update");
}
- maybe_update_channel_info!(channel.one_to_two, channel.node_one);
+ channel.one_to_two = get_new_channel_info!();
}
}
}
impl cmp::Ord for RouteGraphNode {
fn cmp(&self, other: &RouteGraphNode) -> cmp::Ordering {
let other_score = cmp::max(other.lowest_fee_to_peer_through_node, other.path_htlc_minimum_msat)
- .checked_add(other.path_penalty_msat)
- .unwrap_or_else(|| u64::max_value());
+ .saturating_add(other.path_penalty_msat);
let self_score = cmp::max(self.lowest_fee_to_peer_through_node, self.path_htlc_minimum_msat)
- .checked_add(self.path_penalty_msat)
- .unwrap_or_else(|| u64::max_value());
+ .saturating_add(self.path_penalty_msat);
other_score.cmp(&self_score).then_with(|| other.node_id.cmp(&self.node_id))
}
}
self.hops.last().unwrap().0.fee_msat
}
+ fn get_path_penalty_msat(&self) -> u64 {
+ self.hops.first().map(|h| h.0.path_penalty_msat).unwrap_or(u64::max_value())
+ }
+
fn get_total_fee_paid_msat(&self) -> u64 {
if self.hops.len() < 1 {
return 0;
pub(crate) fn get_route<L: Deref, S: Score>(
our_node_pubkey: &PublicKey, payment_params: &PaymentParameters, network_graph: &ReadOnlyNetworkGraph,
first_hops: Option<&[&ChannelDetails]>, final_value_msat: u64, final_cltv_expiry_delta: u32,
- logger: L, scorer: &S, _random_seed_bytes: &[u8; 32]
+ logger: L, scorer: &S, random_seed_bytes: &[u8; 32]
) -> Result<Route, LightningError>
where L::Target: Logger {
let payee_node_id = NodeId::from_pubkey(&payment_params.payee_pubkey);
.entry(short_channel_id)
.or_insert_with(|| $candidate.effective_capacity().as_msat());
- // It is tricky to substract $next_hops_fee_msat from available liquidity here.
+ // It is tricky to subtract $next_hops_fee_msat from available liquidity here.
// It may be misleading because we might later choose to reduce the value transferred
// over these channels, and the channel which was insufficient might become sufficient.
// Worst case: we drop a good channel here because it can't cover the high following
.checked_sub(2*MEDIAN_HOP_CLTV_EXPIRY_DELTA)
.unwrap_or(payment_params.max_total_cltv_expiry_delta - final_cltv_expiry_delta);
let hop_total_cltv_delta = ($next_hops_cltv_delta as u32)
- .checked_add($candidate.cltv_expiry_delta())
- .unwrap_or(u32::max_value());
+ .saturating_add($candidate.cltv_expiry_delta());
let doesnt_exceed_cltv_delta_limit = hop_total_cltv_delta <= max_total_cltv_expiry_delta;
let value_contribution_msat = cmp::min(available_value_contribution_msat, $next_hops_value_contribution);
}
}
- let path_penalty_msat = $next_hops_path_penalty_msat.checked_add(
- scorer.channel_penalty_msat(short_channel_id, amount_to_transfer_over_msat, *available_liquidity_msat,
- &$src_node_id, &$dest_node_id)).unwrap_or_else(|| u64::max_value());
+ let path_penalty_msat = $next_hops_path_penalty_msat.saturating_add(
+ scorer.channel_penalty_msat(short_channel_id, amount_to_transfer_over_msat,
+ *available_liquidity_msat, &$src_node_id, &$dest_node_id));
let new_graph_node = RouteGraphNode {
node_id: $src_node_id,
lowest_fee_to_peer_through_node: total_fee_msat,
// the fees included in $next_hops_path_htlc_minimum_msat, but also
// can't use something that may decrease on future hops.
let old_cost = cmp::max(old_entry.total_fee_msat, old_entry.path_htlc_minimum_msat)
- .checked_add(old_entry.path_penalty_msat)
- .unwrap_or_else(|| u64::max_value());
+ .saturating_add(old_entry.path_penalty_msat);
let new_cost = cmp::max(total_fee_msat, path_htlc_minimum_msat)
- .checked_add(path_penalty_msat)
- .unwrap_or_else(|| u64::max_value());
+ .saturating_add(path_penalty_msat);
if !old_entry.was_processed && new_cost < old_cost {
targets.push(new_graph_node);
.unwrap_or_else(|| CandidateRouteHop::PrivateHop { hint: hop });
let capacity_msat = candidate.effective_capacity().as_msat();
aggregate_next_hops_path_penalty_msat = aggregate_next_hops_path_penalty_msat
- .checked_add(scorer.channel_penalty_msat(hop.short_channel_id, final_value_msat, capacity_msat, &source, &target))
- .unwrap_or_else(|| u64::max_value());
+ .saturating_add(scorer.channel_penalty_msat(hop.short_channel_id, final_value_msat, capacity_msat, &source, &target));
aggregate_next_hops_cltv_delta = aggregate_next_hops_cltv_delta
- .checked_add(hop.cltv_expiry_delta as u32)
- .unwrap_or_else(|| u32::max_value());
+ .saturating_add(hop.cltv_expiry_delta as u32);
if !add_entry!(candidate, source, target, aggregate_next_hops_fee_msat, path_value_msat, aggregate_next_hops_path_htlc_minimum_msat, aggregate_next_hops_path_penalty_msat, aggregate_next_hops_cltv_delta) {
// If this hop was not used then there is no use checking the preceding hops
}
// Sort by total fees and take the best paths.
- payment_paths.sort_by_key(|path| path.get_total_fee_paid_msat());
+ payment_paths.sort_unstable_by_key(|path| path.get_total_fee_paid_msat());
if payment_paths.len() > 50 {
payment_paths.truncate(50);
}
// Draw multiple sufficient routes by randomly combining the selected paths.
let mut drawn_routes = Vec::new();
- for i in 0..payment_paths.len() {
+ let mut prng = ChaCha20::new(random_seed_bytes, &[0u8; 12]);
+ let mut random_index_bytes = [0u8; ::core::mem::size_of::<usize>()];
+
+ let num_permutations = payment_paths.len();
+ for _ in 0..num_permutations {
let mut cur_route = Vec::<PaymentPath>::new();
let mut aggregate_route_value_msat = 0;
// Step (6).
- // TODO: real random shuffle
- // Currently just starts with i_th and goes up to i-1_th in a looped way.
- let cur_payment_paths = [&payment_paths[i..], &payment_paths[..i]].concat();
+ // Do a Fisher-Yates shuffle to create a random permutation of the payment paths
+ for cur_index in (1..payment_paths.len()).rev() {
+ prng.process_in_place(&mut random_index_bytes);
+ let random_index = usize::from_be_bytes(random_index_bytes).wrapping_rem(cur_index+1);
+ payment_paths.swap(cur_index, random_index);
+ }
// Step (7).
- for payment_path in cur_payment_paths {
+ for payment_path in &payment_paths {
cur_route.push(payment_path.clone());
aggregate_route_value_msat += payment_path.get_value_msat();
if aggregate_route_value_msat > final_value_msat {
// also makes routing more reliable.
let mut overpaid_value_msat = aggregate_route_value_msat - final_value_msat;
- // First, drop some expensive low-value paths entirely if possible.
- // Sort by value so that we drop many really-low values first, since
- // fewer paths is better: the payment is less likely to fail.
- // TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
- // so that the sender pays less fees overall. And also htlc_minimum_msat.
- cur_route.sort_by_key(|path| path.get_value_msat());
+ // First, we drop some expensive low-value paths entirely if possible, since fewer
+ // paths is better: the payment is less likely to fail. In order to do so, we sort
+ // by value and fall back to total fees paid, i.e., in case of equal values we
+ // prefer lower cost paths.
+ cur_route.sort_unstable_by(|a, b| {
+ a.get_value_msat().cmp(&b.get_value_msat())
+ // Reverse ordering for fees, so we drop higher-fee paths first
+ .then_with(|| b.get_total_fee_paid_msat().saturating_add(b.get_path_penalty_msat())
+ .cmp(&a.get_total_fee_paid_msat().saturating_add(a.get_path_penalty_msat())))
+ });
+
// We should make sure that at least 1 path left.
let mut paths_left = cur_route.len();
cur_route.retain(|path| {
assert!(cur_route.len() > 0);
// Step (8).
- // Now, substract the overpaid value from the most-expensive path.
+ // Now, subtract the overpaid value from the most-expensive path.
// TODO: this could also be optimized by also sorting by feerate_per_sat_routed,
// so that the sender pays less fees overall. And also htlc_minimum_msat.
- cur_route.sort_by_key(|path| { path.hops.iter().map(|hop| hop.0.candidate.fees().proportional_millionths as u64).sum::<u64>() });
+ cur_route.sort_unstable_by_key(|path| { path.hops.iter().map(|hop| hop.0.candidate.fees().proportional_millionths as u64).sum::<u64>() });
let expensive_payment_path = cur_route.first_mut().unwrap();
- // We already dropped all the small channels above, meaning all the
- // remaining channels are larger than remaining overpaid_value_msat.
+
+ // We already dropped all the small value paths above, meaning all the
+ // remaining paths are larger than remaining overpaid_value_msat.
// Thus, this can't be negative.
let expensive_path_new_value_msat = expensive_payment_path.get_value_msat() - overpaid_value_msat;
expensive_payment_path.update_value_and_recompute_fees(expensive_path_new_value_msat);
// Step (9).
// Select the best route by lowest total fee.
- drawn_routes.sort_by_key(|paths| paths.iter().map(|path| path.get_total_fee_paid_msat()).sum::<u64>());
+ drawn_routes.sort_unstable_by_key(|paths| paths.iter().map(|path| path.get_total_fee_paid_msat()).sum::<u64>());
let mut selected_paths = Vec::<Vec<Result<RouteHop, LightningError>>>::new();
for payment_path in drawn_routes.first().unwrap() {
let mut path = payment_path.hops.iter().map(|(payment_hop, node_features)| {