Skip to content

Commit

Permalink
Attributable failures
Browse files Browse the repository at this point in the history
  • Loading branch information
@joostjager
joostjager committed Mar 5, 2025
1 parent 5d75b36 commit 898cd51
Show file tree
Hide file tree
Showing 8 changed files with 627 additions and 156 deletions.
64 changes: 58 additions & 6 deletions lightning/src/ln/channel.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ use crate::ln::chan_utils::{
ClosingTransaction, commit_tx_fee_sat,
};
use crate::ln::chan_utils;
use crate::ln::onion_utils::{HTLCFailReason};
use crate::ln::onion_utils::{HTLCFailReason, ATTRIBUTION_DATA_LEN};
use crate::chain::BestBlock;
use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator, fee_for_weight};
use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
Expand Down Expand Up @@ -4718,7 +4718,7 @@ trait FailHTLCContents {
impl FailHTLCContents for msgs::OnionErrorPacket {
type Message = msgs::UpdateFailHTLC;
fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data, attribution_data: self.attribution_data }
}
fn to_inbound_htlc_state(self) -> InboundHTLCState {
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
Expand Down Expand Up @@ -6746,6 +6746,7 @@ impl<SP: Deref> FundedChannel<SP> where
channel_id: self.context.channel_id(),
htlc_id: htlc.htlc_id,
reason: err_packet.data.clone(),
attribution_data: err_packet.attribution_data,
});
},
&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
Expand Down Expand Up @@ -9998,6 +9999,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
dropped_inbound_htlcs += 1;
}
}
let mut removed_htlc_failure_attribution_data: Vec<&Option<[u8; ATTRIBUTION_DATA_LEN]>> = Vec::new();
(self.context.pending_inbound_htlcs.len() as u64 - dropped_inbound_htlcs).write(writer)?;
for htlc in self.context.pending_inbound_htlcs.iter() {
if let &InboundHTLCState::RemoteAnnounced(_) = &htlc.state {
Expand All @@ -10023,9 +10025,10 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
&InboundHTLCState::LocalRemoved(ref removal_reason) => {
4u8.write(writer)?;
match removal_reason {
InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data, attribution_data }) => {
0u8.write(writer)?;
data.write(writer)?;
removed_htlc_failure_attribution_data.push(&attribution_data);
},
InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
1u8.write(writer)?;
Expand Down Expand Up @@ -10087,10 +10090,11 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider

let mut holding_cell_skimmed_fees: Vec<Option<u64>> = Vec::new();
let mut holding_cell_blinding_points: Vec<Option<PublicKey>> = Vec::new();
let mut holding_cell_failure_attribution_data: Vec<(u32, [u8; ATTRIBUTION_DATA_LEN])> = Vec::new();
// Vec of (htlc_id, failure_code, sha256_of_onion)
let mut malformed_htlcs: Vec<(u64, u16, [u8; 32])> = Vec::new();
(self.context.holding_cell_htlc_updates.len() as u64).write(writer)?;
for update in self.context.holding_cell_htlc_updates.iter() {
for (i, update) in self.context.holding_cell_htlc_updates.iter().enumerate() {
match update {
&HTLCUpdateAwaitingACK::AddHTLC {
ref amount_msat, ref cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet,
Expand All @@ -10115,6 +10119,13 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
2u8.write(writer)?;
htlc_id.write(writer)?;
err_packet.data.write(writer)?;

// Store the attribution data for later writing. Include the holding cell htlc update index because
// FailMalformedHTLC is stored with the same type 2 and we wouldn't be able to distinguish the two
// when reading back in.
if let Some(attribution_data ) = err_packet.attribution_data {
holding_cell_failure_attribution_data.push((i as u32, attribution_data));
}
}
&HTLCUpdateAwaitingACK::FailMalformedHTLC {
htlc_id, failure_code, sha256_of_onion
Expand Down Expand Up @@ -10298,6 +10309,8 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
(49, self.context.local_initiated_shutdown, option), // Added in 0.0.122
(51, is_manual_broadcast, option), // Added in 0.0.124
(53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124
(55, removed_htlc_failure_attribution_data, optional_vec), // Added in 0.2
(57, holding_cell_failure_attribution_data, optional_vec), // Added in 0.2
});

Ok(())
Expand Down Expand Up @@ -10375,6 +10388,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
let reason = match <u8 as Readable>::read(reader)? {
0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
data: Readable::read(reader)?,
attribution_data: None,
}),
1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
Expand Down Expand Up @@ -10439,6 +10453,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
htlc_id: Readable::read(reader)?,
err_packet: OnionErrorPacket {
data: Readable::read(reader)?,
attribution_data: None,
},
},
_ => return Err(DecodeError::InvalidValue),
Expand Down Expand Up @@ -10582,6 +10597,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
let mut pending_outbound_blinding_points_opt: Option<Vec<Option<PublicKey>>> = None;
let mut holding_cell_blinding_points_opt: Option<Vec<Option<PublicKey>>> = None;

let mut removed_htlc_failure_attribution_data: Option<Vec<Option<[u8; ATTRIBUTION_DATA_LEN]>>> = None;
let mut holding_cell_failure_attribution_data: Option<Vec<(u32, [u8; ATTRIBUTION_DATA_LEN])>> = None;

let mut malformed_htlcs: Option<Vec<(u64, u16, [u8; 32])>> = None;
let mut monitor_pending_update_adds: Option<Vec<msgs::UpdateAddHTLC>> = None;

Expand Down Expand Up @@ -10624,6 +10642,8 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
(49, local_initiated_shutdown, option),
(51, is_manual_broadcast, option),
(53, funding_tx_broadcast_safe_event_emitted, option),
(55, removed_htlc_failure_attribution_data, optional_vec),
(57, holding_cell_failure_attribution_data, optional_vec),
});

let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
Expand Down Expand Up @@ -10705,6 +10725,38 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
if iter.next().is_some() { return Err(DecodeError::InvalidValue) }
}

if let Some(attribution_datas) = removed_htlc_failure_attribution_data {
let mut removed_htlc_relay_failures =
pending_inbound_htlcs.iter_mut().filter_map(|status|
if let InboundHTLCState::LocalRemoved(ref mut reason) = &mut status.state {
if let InboundHTLCRemovalReason::FailRelay(ref mut packet) = reason {
Some(&mut packet.attribution_data)
} else {
None
}
} else {
None
}
);

for attribution_data in attribution_datas {
*removed_htlc_relay_failures.next().ok_or(DecodeError::InvalidValue)? = attribution_data;
}
if removed_htlc_relay_failures.next().is_some() { return Err(DecodeError::InvalidValue); }
}

if let Some(attribution_datas) = holding_cell_failure_attribution_data {
for (i, attribution_data) in attribution_datas {
let update = holding_cell_htlc_updates.get_mut(i as usize).ok_or(DecodeError::InvalidValue)?;

if let HTLCUpdateAwaitingACK::FailHTLC { htlc_id: _, ref mut err_packet } = update {
err_packet.attribution_data = Some(attribution_data);
} else {
return Err(DecodeError::InvalidValue);
}
}
}

if let Some(malformed_htlcs) = malformed_htlcs {
for (malformed_htlc_id, failure_code, sha256_of_onion) in malformed_htlcs {
let htlc_idx = holding_cell_htlc_updates.iter().position(|htlc| {
Expand Down Expand Up @@ -10902,7 +10954,7 @@ mod tests {
use bitcoin::transaction::{Transaction, TxOut, Version};
use bitcoin::opcodes;
use bitcoin::network::Network;
use crate::ln::onion_utils::INVALID_ONION_BLINDING;
use crate::ln::onion_utils::{ATTRIBUTION_DATA_LEN, INVALID_ONION_BLINDING};
use crate::types::payment::{PaymentHash, PaymentPreimage};
use crate::ln::channel_keys::{RevocationKey, RevocationBasepoint};
use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
Expand Down Expand Up @@ -11538,7 +11590,7 @@ mod tests {
htlc_id: 0,
};
let dummy_holding_cell_failed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailHTLC {
htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }
htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42], attribution_data: Some([1; ATTRIBUTION_DATA_LEN]) }
};
let dummy_holding_cell_malformed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailMalformedHTLC {
htlc_id, failure_code: INVALID_ONION_BLINDING, sha256_of_onion: [0; 32],
Expand Down
97 changes: 93 additions & 4 deletions lightning/src/ln/channelmanager.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ use crate::routing::gossip::NodeId;
use crate::routing::router::{BlindedTail, InFlightHtlcs, Path, Payee, PaymentParameters, RouteParameters, RouteParametersConfig, Router, FixedRouter, Route};
use crate::ln::onion_payment::{check_incoming_htlc_cltv, create_recv_pending_htlc_info, create_fwd_pending_htlc_info, decode_incoming_update_add_htlc_onion, InboundHTLCErr, NextPacketDetails};
use crate::ln::msgs;
use crate::ln::onion_utils;
use crate::ln::onion_utils::{self, ATTRIBUTION_DATA_LEN};
use crate::ln::onion_utils::{HTLCFailReason, INVALID_ONION_BLINDING};
use crate::ln::msgs::{ChannelMessageHandler, CommitmentUpdate, DecodeError, LightningError};
#[cfg(test)]
Expand Down Expand Up @@ -4418,6 +4418,7 @@ where
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: failure.data.clone(),
attribution_data: failure.attribution_data,
})
}

Expand All @@ -4443,10 +4444,12 @@ where
}
let failure = HTLCFailReason::reason($err_code, $data.to_vec())
.get_encrypted_failure_packet(&shared_secret, &None);

return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
channel_id: msg.channel_id,
htlc_id: msg.htlc_id,
reason: failure.data,
attribution_data: failure.attribution_data,
}));
}
}
Expand Down Expand Up @@ -12837,11 +12840,15 @@ impl_writeable_tlv_based!(PendingHTLCInfo, {
impl Writeable for HTLCFailureMsg {
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
match self {
HTLCFailureMsg::Relay(msgs::UpdateFailHTLC { channel_id, htlc_id, reason }) => {
HTLCFailureMsg::Relay(msgs::UpdateFailHTLC { channel_id, htlc_id, reason, attribution_data }) => {
0u8.write(writer)?;
channel_id.write(writer)?;
htlc_id.write(writer)?;
reason.write(writer)?;

// This code will only ever be hit for legacy data that is re-serialized. It isn't necessary to try
// writing out attribution data, because it can never be present.
debug_assert!(attribution_data.is_none());
},
HTLCFailureMsg::Malformed(msgs::UpdateFailMalformedHTLC {
channel_id, htlc_id, sha256_of_onion, failure_code
Expand All @@ -12866,6 +12873,7 @@ impl Readable for HTLCFailureMsg {
channel_id: Readable::read(reader)?,
htlc_id: Readable::read(reader)?,
reason: Readable::read(reader)?,
attribution_data: None,
}))
},
1 => {
Expand Down Expand Up @@ -13096,6 +13104,7 @@ impl Writeable for HTLCForwardInfo {
write_tlv_fields!(w, {
(0, htlc_id, required),
(2, err_packet.data, required),
(5, err_packet.attribution_data, option),
});
},
Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
Expand Down Expand Up @@ -13126,8 +13135,12 @@ impl Readable for HTLCForwardInfo {
(1, malformed_htlc_failure_code, option),
(2, err_packet, required),
(3, sha256_of_onion, option),
(5, attribution_data, option),
});
if let Some(failure_code) = malformed_htlc_failure_code {
if attribution_data.is_some() {
return Err(DecodeError::InvalidValue);
}
Self::FailMalformedHTLC {
htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
failure_code,
Expand All @@ -13138,6 +13151,7 @@ impl Readable for HTLCForwardInfo {
htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
err_packet: crate::ln::msgs::OnionErrorPacket {
data: _init_tlv_based_struct_field!(err_packet, required),
attribution_data: _init_tlv_based_struct_field!(attribution_data, option),
},
}
}
Expand Down Expand Up @@ -14837,7 +14851,8 @@ mod tests {
use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
use core::sync::atomic::Ordering;
use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
use crate::ln::types::ChannelId;
use crate::ln::onion_utils::ATTRIBUTION_DATA_LEN;
use crate::ln::types::ChannelId;
use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
use crate::ln::channelmanager::{RAACommitmentOrder, create_recv_pending_htlc_info, inbound_payment, ChannelConfigOverrides, HTLCForwardInfo, InterceptId, PaymentId, RecipientOnionFields};
use crate::ln::functional_test_utils::*;
Expand Down Expand Up @@ -15311,6 +15326,80 @@ mod tests {
nodes[1].logger.assert_log_contains("lightning::ln::channelmanager", "Payment preimage didn't match payment hash", 1);
}

#[test]
fn test_htlc_localremoved_persistence() {
let chanmon_cfgs: Vec<TestChanMonCfg> = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);

let persister;
let chain_monitor;
let deserialized_chanmgr;

// Send a keysend payment that fails because of a preimage mismatch.
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let mut nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let payer_pubkey = nodes[0].node.get_our_node_id();
let payee_pubkey = nodes[1].node.get_our_node_id();

let _chan = create_chan_between_nodes(&nodes[0], &nodes[1]);
let route_params = RouteParameters::from_payment_params_and_value(
PaymentParameters::for_keysend(payee_pubkey, 40, false), 10_000);
let network_graph = nodes[0].network_graph;
let first_hops = nodes[0].node.list_usable_channels();
let scorer = test_utils::TestScorer::new();
let random_seed_bytes = chanmon_cfgs[1].keys_manager.get_secure_random_bytes();
let route = find_route(
&payer_pubkey, &route_params, &network_graph, Some(&first_hops.iter().collect::<Vec<_>>()),
nodes[0].logger, &scorer, &Default::default(), &random_seed_bytes
).unwrap();

let test_preimage = PaymentPreimage([42; 32]);
let mismatch_payment_hash = PaymentHash([43; 32]);
let session_privs = nodes[0].node.test_add_new_pending_payment(mismatch_payment_hash,
RecipientOnionFields::spontaneous_empty(), PaymentId(mismatch_payment_hash.0), &route).unwrap();
nodes[0].node.test_send_payment_internal(&route, mismatch_payment_hash,
RecipientOnionFields::spontaneous_empty(), Some(test_preimage), PaymentId(mismatch_payment_hash.0), None, session_privs).unwrap();
check_added_monitors!(nodes[0], 1);

let updates = get_htlc_update_msgs!(nodes[0], nodes[1].node.get_our_node_id());
nodes[1].node.handle_update_add_htlc(nodes[0].node.get_our_node_id(), &updates.update_add_htlcs[0]);
commitment_signed_dance!(nodes[1], nodes[0], &updates.commitment_signed, false);
expect_pending_htlcs_forwardable!(nodes[1]);
expect_htlc_handling_failed_destinations!(nodes[1].node.get_and_clear_pending_events(), &[HTLCDestination::FailedPayment { payment_hash: mismatch_payment_hash }]);
check_added_monitors(&nodes[1], 1);

// Save the update_fail_htlc message for later comparison.
let msgs = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id());
let htlc_fail_msg = msgs.update_fail_htlcs[0].clone();

// Reload node.
nodes[0].node.peer_disconnected(nodes[1].node.get_our_node_id());
nodes[1].node.peer_disconnected(nodes[0].node.get_our_node_id());

let monitor_encoded = get_monitor!(nodes[1], _chan.3).encode();
reload_node!(nodes[1], nodes[1].node.encode(), &[&monitor_encoded], persister, chain_monitor, deserialized_chanmgr);

nodes[0].node.peer_connected(nodes[1].node.get_our_node_id(), &msgs::Init {
features: nodes[1].node.init_features(), networks: None, remote_network_address: None
}, true).unwrap();
let reestablish_1 = get_chan_reestablish_msgs!(nodes[0], nodes[1]);
assert_eq!(reestablish_1.len(), 1);
nodes[1].node.peer_connected(nodes[0].node.get_our_node_id(), &msgs::Init {
features: nodes[0].node.init_features(), networks: None, remote_network_address: None
}, false).unwrap();
let reestablish_2 = get_chan_reestablish_msgs!(nodes[1], nodes[0]);
assert_eq!(reestablish_2.len(), 1);
nodes[0].node.handle_channel_reestablish(nodes[1].node.get_our_node_id(), &reestablish_2[0]);
handle_chan_reestablish_msgs!(nodes[0], nodes[1]);
nodes[1].node.handle_channel_reestablish(nodes[0].node.get_our_node_id(), &reestablish_1[0]);

// Assert that same failure message is resent after reload.
let msgs = handle_chan_reestablish_msgs!(nodes[1], nodes[0]);
let htlc_fail_msg_after_reload = msgs.2.unwrap().update_fail_htlcs[0].clone();
assert_eq!(htlc_fail_msg, htlc_fail_msg_after_reload);
}

#[test]
fn test_multi_hop_missing_secret() {
let chanmon_cfgs = create_chanmon_cfgs(4);
Expand Down Expand Up @@ -16269,7 +16358,7 @@ mod tests {
let mut nodes = create_network(1, &node_cfg, &chanmgrs);

let dummy_failed_htlc = |htlc_id| {
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } }
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42], attribution_data: Some([0; ATTRIBUTION_DATA_LEN]) } }
};
let dummy_malformed_htlc = |htlc_id| {
HTLCForwardInfo::FailMalformedHTLC { htlc_id, failure_code: 0x4000, sha256_of_onion: [0; 32] }
Expand Down
2 changes: 2 additions & 0 deletions lightning/src/ln/functional_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use crate::chain::chaininterface::LowerBoundedFeeEstimator;
use crate::chain::channelmonitor;
use crate::chain::channelmonitor::{Balance, ChannelMonitorUpdateStep, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, COUNTERPARTY_CLAIMABLE_WITHIN_BLOCKS_PINNABLE};
use crate::chain::transaction::OutPoint;
use crate::ln::onion_utils::ATTRIBUTION_DATA_LEN;
use crate::sign::{ecdsa::EcdsaChannelSigner, EntropySource, OutputSpender, SignerProvider};
use crate::events::bump_transaction::WalletSource;
use crate::events::{Event, FundingInfo, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentPurpose, ClosureReason, HTLCDestination, PaymentFailureReason};
Expand Down Expand Up @@ -7055,6 +7056,7 @@ pub fn test_update_fulfill_htlc_bolt2_update_fail_htlc_before_commitment() {
channel_id: chan.2,
htlc_id: 0,
reason: Vec::new(),
attribution_data: Some([0; ATTRIBUTION_DATA_LEN])
};

nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &update_msg);
Expand Down
Loading

0 comments on commit 898cd51

Please sign in to comment.