`ChannelManager::fail_htlc_backwards`' bool return value is quite
confusing - just because it returns false doesn't mean the payment
wasn't (already) failed. Worse, in some race cases around shutdown
where a payment was claimed before an unclean shutdown and then
retried on startup, `fail_htlc_backwards` could return true even
though (a duplicate copy of the same payment) was claimed, but the
claim event has not been seen by the user yet.
While its possible to use it correctly, its somewhat confusing to
have a return value at all, and definitely lends itself to misuse.
Instead, we should push users towards a model where they don't care
if `fail_htlc_backwards` succeeds - either they've locally marked
the payment as failed (prior to seeing any `PaymentReceived`
events) and will fail any attempts to pay it, or they have not and
the payment is still receivable until its timeout time is reached.
We can revisit this decision based on user feedback, but will need
to very carefully document the potential failure modes here if we
do.
events::Event::PaymentReceived { payment_hash, .. } => {
if claim_set.insert(payment_hash.0) {
if $fail {
events::Event::PaymentReceived { payment_hash, .. } => {
if claim_set.insert(payment_hash.0) {
if $fail {
- assert!(nodes[$node].fail_htlc_backwards(&payment_hash));
+ nodes[$node].fail_htlc_backwards(&payment_hash);
} else {
nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
}
} else {
nodes[$node].claim_funds(PaymentPreimage(payment_hash.0));
}
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
// Fail the payment backwards, failing the monitor update on nodes[1]'s receipt of the RAA
- assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
+ nodes[2].node.fail_htlc_backwards(&payment_hash_1);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
send_payment(&nodes[0], &[&nodes[1], &nodes[2]], 5000000);
let (_, payment_hash_1, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], 1000000);
- assert!(nodes[2].node.fail_htlc_backwards(&payment_hash_1));
+ nodes[2].node.fail_htlc_backwards(&payment_hash_1);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
payment_preimage,
};
if second_fails {
payment_preimage,
};
if second_fails {
- assert!(nodes[2].node.fail_htlc_backwards(&payment_hash));
+ nodes[2].node.fail_htlc_backwards(&payment_hash);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
/// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect
/// after a PaymentReceived event, failing the HTLC back to its origin and freeing resources
/// along the path (including in our own channel on which we received it).
/// Indicates that the preimage for payment_hash is unknown or the received amount is incorrect
/// after a PaymentReceived event, failing the HTLC back to its origin and freeing resources
/// along the path (including in our own channel on which we received it).
- /// Returns false if no payment was found to fail backwards, true if the process of failing the
- /// HTLC backwards has been started.
- pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash) -> bool {
+ ///
+ /// Note that in some cases around unclean shutdown, it is possible the payment may have
+ /// already been claimed by you via [`ChannelManager::claim_funds`] prior to you seeing (a
+ /// second copy of) the [`events::Event::PaymentReceived`] event. Alternatively, the payment
+ /// may have already been failed automatically by LDK if it was nearing its expiration time.
+ ///
+ /// While LDK will never claim a payment automatically on your behalf (i.e. without you calling
+ /// [`ChannelManager::claim_funds`]), you should still monitor for
+ /// [`events::Event::PaymentClaimed`] events even for payments you intend to fail, especially on
+ /// startup during which time claims that were in-progress at shutdown may be replayed.
+ pub fn fail_htlc_backwards(&self, payment_hash: &PaymentHash) {
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
let mut channel_state = Some(self.channel_state.lock().unwrap());
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
let mut channel_state = Some(self.channel_state.lock().unwrap());
HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data });
}
HTLCSource::PreviousHopData(htlc.prev_hop), payment_hash,
HTLCFailReason::Reason { failure_code: 0x4000 | 15, data: htlc_msat_height_data });
}
- true
- } else { false }
}
/// Gets an HTLC onion failure code and error data for an `UPDATE` error, given the error code
}
/// Gets an HTLC onion failure code and error data for an `UPDATE` error, given the error code
for path in expected_paths.iter() {
assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
}
for path in expected_paths.iter() {
assert_eq!(path.last().unwrap().node.get_our_node_id(), expected_paths[0].last().unwrap().node.get_our_node_id());
}
- assert!(expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
+ expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash);
expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap());
pass_failed_payment_back(origin_node, expected_paths, skip_last, our_payment_hash);
expect_pending_htlcs_forwardable!(expected_paths[0].last().unwrap());
pass_failed_payment_back(origin_node, expected_paths, skip_last, our_payment_hash);
}
// Ensure that fail_htlc_backwards is idempotent.
}
// Ensure that fail_htlc_backwards is idempotent.
- assert!(!expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash));
+ expected_paths[0].last().unwrap().node.fail_htlc_backwards(&our_payment_hash);
assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_events().is_empty());
assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
check_added_monitors!(expected_paths[0].last().unwrap(), 0);
assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_events().is_empty());
assert!(expected_paths[0].last().unwrap().node.get_and_clear_pending_msg_events().is_empty());
check_added_monitors!(expected_paths[0].last().unwrap(), 0);
let (_, second_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], value);
let (_, third_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], value);
let (_, second_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], value);
let (_, third_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1], &nodes[2]], value);
- assert!(nodes[2].node.fail_htlc_backwards(&first_payment_hash));
+ nodes[2].node.fail_htlc_backwards(&first_payment_hash);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
let bs_raa = commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false, true, false, true);
// Drop the last RAA from 3 -> 2
let bs_raa = commitment_signed_dance!(nodes[1], nodes[2], updates.commitment_signed, false, true, false, true);
// Drop the last RAA from 3 -> 2
- assert!(nodes[2].node.fail_htlc_backwards(&second_payment_hash));
+ nodes[2].node.fail_htlc_backwards(&second_payment_hash);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
nodes[2].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_raa);
check_added_monitors!(nodes[2], 1);
nodes[2].node.handle_revoke_and_ack(&nodes[1].node.get_our_node_id(), &as_raa);
check_added_monitors!(nodes[2], 1);
- assert!(nodes[2].node.fail_htlc_backwards(&third_payment_hash));
+ nodes[2].node.fail_htlc_backwards(&third_payment_hash);
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
expect_pending_htlcs_forwardable!(nodes[2]);
check_added_monitors!(nodes[2], 1);
let updates = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id());
// Now fail back three of the over-dust-limit and three of the under-dust-limit payments in one go.
// Fail 0th below-dust, 4th above-dust, 8th above-dust, 10th below-dust HTLCs
// Now fail back three of the over-dust-limit and three of the under-dust-limit payments in one go.
// Fail 0th below-dust, 4th above-dust, 8th above-dust, 10th below-dust HTLCs
- assert!(nodes[4].node.fail_htlc_backwards(&payment_hash_1));
- assert!(nodes[4].node.fail_htlc_backwards(&payment_hash_3));
- assert!(nodes[4].node.fail_htlc_backwards(&payment_hash_5));
- assert!(nodes[4].node.fail_htlc_backwards(&payment_hash_6));
+ nodes[4].node.fail_htlc_backwards(&payment_hash_1);
+ nodes[4].node.fail_htlc_backwards(&payment_hash_3);
+ nodes[4].node.fail_htlc_backwards(&payment_hash_5);
+ nodes[4].node.fail_htlc_backwards(&payment_hash_6);
check_added_monitors!(nodes[4], 0);
expect_pending_htlcs_forwardable!(nodes[4]);
check_added_monitors!(nodes[4], 1);
check_added_monitors!(nodes[4], 0);
expect_pending_htlcs_forwardable!(nodes[4]);
check_added_monitors!(nodes[4], 1);
commitment_signed_dance!(nodes[3], nodes[4], four_removes.commitment_signed, false);
// Fail 3rd below-dust and 7th above-dust HTLCs
commitment_signed_dance!(nodes[3], nodes[4], four_removes.commitment_signed, false);
// Fail 3rd below-dust and 7th above-dust HTLCs
- assert!(nodes[5].node.fail_htlc_backwards(&payment_hash_2));
- assert!(nodes[5].node.fail_htlc_backwards(&payment_hash_4));
+ nodes[5].node.fail_htlc_backwards(&payment_hash_2);
+ nodes[5].node.fail_htlc_backwards(&payment_hash_4);
check_added_monitors!(nodes[5], 0);
expect_pending_htlcs_forwardable!(nodes[5]);
check_added_monitors!(nodes[5], 1);
check_added_monitors!(nodes[5], 0);
expect_pending_htlcs_forwardable!(nodes[5]);
check_added_monitors!(nodes[5], 1);
// actually revoked.
let htlc_value = if use_dust { 50000 } else { 3000000 };
let (_, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], htlc_value);
// actually revoked.
let htlc_value = if use_dust { 50000 } else { 3000000 };
let (_, our_payment_hash, _) = route_payment(&nodes[0], &[&nodes[1]], htlc_value);
- assert!(nodes[1].node.fail_htlc_backwards(&our_payment_hash));
+ nodes[1].node.fail_htlc_backwards(&our_payment_hash);
expect_pending_htlcs_forwardable!(nodes[1]);
check_added_monitors!(nodes[1], 1);
expect_pending_htlcs_forwardable!(nodes[1]);
check_added_monitors!(nodes[1], 1);
let as_prev_commitment_tx = get_local_commitment_txn!(nodes[0], chan.2);
// Fail one HTLC to prune it in the will-be-latest-local commitment tx
let as_prev_commitment_tx = get_local_commitment_txn!(nodes[0], chan.2);
// Fail one HTLC to prune it in the will-be-latest-local commitment tx
- assert!(nodes[1].node.fail_htlc_backwards(&payment_hash_2));
+ nodes[1].node.fail_htlc_backwards(&payment_hash_2);
check_added_monitors!(nodes[1], 0);
expect_pending_htlcs_forwardable!(nodes[1]);
check_added_monitors!(nodes[1], 1);
check_added_monitors!(nodes[1], 0);
expect_pending_htlcs_forwardable!(nodes[1]);
check_added_monitors!(nodes[1], 1);
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();
expect_payment_received!(nodes[1], payment_hash, payment_secret, recv_amt_msat);
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();
expect_payment_received!(nodes[1], payment_hash, payment_secret, recv_amt_msat);
- assert!(nodes[1].node.fail_htlc_backwards(&payment_hash));
+ nodes[1].node.fail_htlc_backwards(&payment_hash);
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();
expect_pending_htlcs_forwardable_ignore!(nodes[1]);
nodes[1].node.process_pending_htlc_forwards();