Skip to content

Commit 0bee7ca

Browse files
joostjagerTheBlueMatt
joostjager
and
TheBlueMatt
committed
attributable failures pre-factor
Co-authored-by: Matt Corallo <[email protected]>
1 parent 5506d3d commit 0bee7ca

File tree

7 files changed

+102
-63
lines changed

7 files changed

+102
-63
lines changed

‎lightning/src/ln/channel.rs‎

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ use crate::ln::interactivetxs::{
3636
TX_COMMON_FIELDS_WEIGHT,
3737
};
3838
use crate::ln::msgs;
39-
use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError};
39+
use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket};
4040
use crate::ln::script::{self, ShutdownScript};
4141
use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails};
4242
use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
@@ -49,7 +49,7 @@ use crate::ln::chan_utils::{
4949
ClosingTransaction, commit_tx_fee_sat,
5050
};
5151
use crate::ln::chan_utils;
52-
use crate::ln::onion_utils::HTLCFailReason;
52+
use crate::ln::onion_utils::{HTLCFailReason};
5353
use crate::chain::BestBlock;
5454
use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator, fee_for_weight};
5555
use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
@@ -4722,7 +4722,7 @@ trait FailHTLCContents {
47224722
impl FailHTLCContents for msgs::OnionErrorPacket {
47234723
type Message = msgs::UpdateFailHTLC;
47244724
fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
4725-
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self }
4725+
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
47264726
}
47274727
fn to_inbound_htlc_state(self) -> InboundHTLCState {
47284728
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
@@ -6041,7 +6041,7 @@ impl<SP: Deref> FundedChannel<SP> where
60416041
require_commitment = true;
60426042
match fail_msg {
60436043
HTLCFailureMsg::Relay(msg) => {
6044-
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
6044+
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay((&msg).into()));
60456045
update_fail_htlcs.push(msg)
60466046
},
60476047
HTLCFailureMsg::Malformed(msg) => {
@@ -6749,7 +6749,7 @@ impl<SP: Deref> FundedChannel<SP> where
67496749
update_fail_htlcs.push(msgs::UpdateFailHTLC {
67506750
channel_id: self.context.channel_id(),
67516751
htlc_id: htlc.htlc_id,
6752-
reason: err_packet.clone()
6752+
reason: err_packet.data.clone(),
67536753
});
67546754
},
67556755
&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
@@ -9903,11 +9903,6 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures)
99039903
const SERIALIZATION_VERSION: u8 = 4;
99049904
const MIN_SERIALIZATION_VERSION: u8 = 4;
99059905

9906-
impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,;
9907-
(0, FailRelay),
9908-
(1, FailMalformed),
9909-
(2, Fulfill),
9910-
);
99119906

99129907
impl Writeable for ChannelUpdateStatus {
99139908
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
@@ -10037,7 +10032,20 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1003710032
},
1003810033
&InboundHTLCState::LocalRemoved(ref removal_reason) => {
1003910034
4u8.write(writer)?;
10040-
removal_reason.write(writer)?;
10035+
match removal_reason {
10036+
InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
10037+
0u8.write(writer)?;
10038+
data.write(writer)?;
10039+
},
10040+
InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
10041+
1u8.write(writer)?;
10042+
(hash, code).write(writer)?;
10043+
},
10044+
InboundHTLCRemovalReason::Fulfill(preimage) => {
10045+
2u8.write(writer)?;
10046+
preimage.write(writer)?;
10047+
},
10048+
}
1004110049
},
1004210050
}
1004310051
}
@@ -10116,7 +10124,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1011610124
&HTLCUpdateAwaitingACK::FailHTLC { ref htlc_id, ref err_packet } => {
1011710125
2u8.write(writer)?;
1011810126
htlc_id.write(writer)?;
10119-
err_packet.write(writer)?;
10127+
err_packet.data.write(writer)?;
1012010128
}
1012110129
&HTLCUpdateAwaitingACK::FailMalformedHTLC {
1012210130
htlc_id, failure_code, sha256_of_onion
@@ -10125,10 +10133,9 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1012510133
// `::FailHTLC` variant and write the real malformed error as an optional TLV.
1012610134
malformed_htlcs.push((htlc_id, failure_code, sha256_of_onion));
1012710135

10128-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1012910136
2u8.write(writer)?;
1013010137
htlc_id.write(writer)?;
10131-
dummy_err_packet.write(writer)?;
10138+
Vec::<u8>::new().write(writer)?;
1013210139
}
1013310140
}
1013410141
}
@@ -10374,7 +10381,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1037410381
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(resolution)
1037510382
},
1037610383
3 => InboundHTLCState::Committed,
10377-
4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?),
10384+
4 => {
10385+
let reason = match <u8 as Readable>::read(reader)? {
10386+
0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
10387+
data: Readable::read(reader)?,
10388+
}),
10389+
1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
10390+
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
10391+
_ => return Err(DecodeError::InvalidValue),
10392+
};
10393+
InboundHTLCState::LocalRemoved(reason)
10394+
},
1037810395
_ => return Err(DecodeError::InvalidValue),
1037910396
},
1038010397
});
@@ -10430,7 +10447,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1043010447
},
1043110448
2 => HTLCUpdateAwaitingACK::FailHTLC {
1043210449
htlc_id: Readable::read(reader)?,
10433-
err_packet: Readable::read(reader)?,
10450+
err_packet: OnionErrorPacket {
10451+
data: Readable::read(reader)?,
10452+
},
1043410453
},
1043510454
_ => return Err(DecodeError::InvalidValue),
1043610455
});

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4427,11 +4427,12 @@ where
44274427
} else {
44284428
(err_code, &res.0[..])
44294429
};
4430+
let failure = HTLCFailReason::reason(err_code, err_data.to_vec())
4431+
.get_encrypted_failure_packet(shared_secret, &None);
44304432
HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44314433
channel_id: msg.channel_id,
44324434
htlc_id: msg.htlc_id,
4433-
reason: HTLCFailReason::reason(err_code, err_data.to_vec())
4434-
.get_encrypted_failure_packet(shared_secret, &None),
4435+
reason: failure.data.clone(),
44354436
})
44364437
}
44374438

@@ -4455,11 +4456,12 @@ where
44554456
}
44564457
))
44574458
}
4459+
let failure = HTLCFailReason::reason($err_code, $data.to_vec())
4460+
.get_encrypted_failure_packet(&shared_secret, &None);
44584461
return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44594462
channel_id: msg.channel_id,
44604463
htlc_id: msg.htlc_id,
4461-
reason: HTLCFailReason::reason($err_code, $data.to_vec())
4462-
.get_encrypted_failure_packet(&shared_secret, &None),
4464+
reason: failure.data,
44634465
}));
44644466
}
44654467
}
@@ -5798,7 +5800,7 @@ where
57985800
let failure = match htlc_fail {
57995801
HTLCFailureMsg::Relay(fail_htlc) => HTLCForwardInfo::FailHTLC {
58005802
htlc_id: fail_htlc.htlc_id,
5801-
err_packet: fail_htlc.reason,
5803+
err_packet: (&fail_htlc).into(),
58025804
},
58035805
HTLCFailureMsg::Malformed(fail_malformed_htlc) => HTLCForwardInfo::FailMalformedHTLC {
58045806
htlc_id: fail_malformed_htlc.htlc_id,
@@ -13108,19 +13110,18 @@ impl Writeable for HTLCForwardInfo {
1310813110
FAIL_HTLC_VARIANT_ID.write(w)?;
1310913111
write_tlv_fields!(w, {
1311013112
(0, htlc_id, required),
13111-
(2, err_packet, required),
13113+
(2, err_packet.data, required),
1311213114
});
1311313115
},
1311413116
Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
1311513117
// Since this variant was added in 0.0.119, write this as `::FailHTLC` with an empty error
1311613118
// packet so older versions have something to fail back with, but serialize the real data as
1311713119
// optional TLVs for the benefit of newer versions.
1311813120
FAIL_HTLC_VARIANT_ID.write(w)?;
13119-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1312013121
write_tlv_fields!(w, {
1312113122
(0, htlc_id, required),
1312213123
(1, failure_code, required),
13123-
(2, dummy_err_packet, required),
13124+
(2, Vec::<u8>::new(), required),
1312413125
(3, sha256_of_onion, required),
1312513126
});
1312613127
},
@@ -13150,7 +13151,9 @@ impl Readable for HTLCForwardInfo {
1315013151
} else {
1315113152
Self::FailHTLC {
1315213153
htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
13153-
err_packet: _init_tlv_based_struct_field!(err_packet, required),
13154+
err_packet: crate::ln::msgs::OnionErrorPacket {
13155+
data: _init_tlv_based_struct_field!(err_packet, required),
13156+
},
1315413157
}
1315513158
}
1315613159
},
@@ -14851,7 +14854,7 @@ mod tests {
1485114854
use crate::events::{Event, HTLCDestination, ClosureReason};
1485214855
use crate::ln::types::ChannelId;
1485314856
use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
14854-
use crate::ln::channelmanager::{create_recv_pending_htlc_info, inbound_payment, ChannelConfigOverrides, HTLCForwardInfo, InterceptId, PaymentId, RecipientOnionFields};
14857+
use crate::ln::channelmanager::{RAACommitmentOrder, create_recv_pending_htlc_info, inbound_payment, ChannelConfigOverrides, HTLCForwardInfo, InterceptId, PaymentId, RecipientOnionFields};
1485514858
use crate::ln::functional_test_utils::*;
1485614859
use crate::ln::msgs::{self, BaseMessageHandler, ChannelMessageHandler, AcceptChannel, ErrorAction, MessageSendEvent};
1485714860
use crate::ln::outbound_payment::Retry;
@@ -15070,8 +15073,8 @@ mod tests {
1507015073
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
1507115074

1507215075
create_announced_chan_between_nodes(&nodes, 0, 1);
15073-
15074-
// Since we do not send peer storage, we manually simulate receiving a dummy
15076+
15077+
// Since we do not send peer storage, we manually simulate receiving a dummy
1507515078
// `PeerStorage` from the channel partner.
1507615079
nodes[0].node.handle_peer_storage(nodes[1].node.get_our_node_id(), msgs::PeerStorage{data: vec![0; 100]});
1507715080

@@ -16280,7 +16283,7 @@ mod tests {
1628016283
let mut nodes = create_network(1, &node_cfg, &chanmgrs);
1628116284

1628216285
let dummy_failed_htlc = |htlc_id| {
16283-
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }, }
16286+
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } }
1628416287
};
1628516288
let dummy_malformed_htlc = |htlc_id| {
1628616289
HTLCForwardInfo::FailMalformedHTLC { htlc_id, failure_code: 0x4000, sha256_of_onion: [0; 32] }

‎lightning/src/ln/functional_tests.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7057,7 +7057,7 @@ pub fn test_update_fulfill_htlc_bolt2_update_fail_htlc_before_commitment() {
70577057
let update_msg = msgs::UpdateFailHTLC{
70587058
channel_id: chan.2,
70597059
htlc_id: 0,
7060-
reason: msgs::OnionErrorPacket { data: Vec::new()},
7060+
reason: Vec::new(),
70617061
};
70627062

70637063
nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &update_msg);

‎lightning/src/ln/msgs.rs‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -728,7 +728,7 @@ pub struct UpdateFulfillHTLC {
728728
/// A [`peer_storage`] message that can be sent to or received from a peer.
729729
///
730730
/// This message is used to distribute backup data to peers.
731-
/// If data is lost or corrupted, users can retrieve it through [`PeerStorageRetrieval`]
731+
/// If data is lost or corrupted, users can retrieve it through [`PeerStorageRetrieval`]
732732
/// to recover critical information, such as channel states, for fund recovery.
733733
///
734734
/// [`peer_storage`] is used to send our own encrypted backup data to a peer.
@@ -743,7 +743,7 @@ pub struct PeerStorage {
743743
/// A [`peer_storage_retrieval`] message that can be sent to or received from a peer.
744744
///
745745
/// This message is sent to peers for whom we store backup data.
746-
/// If we receive this message, it indicates that the peer had stored our backup data.
746+
/// If we receive this message, it indicates that the peer had stored our backup data.
747747
/// This data can be used for fund recovery in case of data loss.
748748
///
749749
/// [`peer_storage_retrieval`] is used to send the most recent backup of the peer.
@@ -764,7 +764,7 @@ pub struct UpdateFailHTLC {
764764
pub channel_id: ChannelId,
765765
/// The HTLC ID
766766
pub htlc_id: u64,
767-
pub(crate) reason: OnionErrorPacket,
767+
pub(crate) reason: Vec<u8>,
768768
}
769769

770770
/// An [`update_fail_malformed_htlc`] message to be sent to or received from a peer.
@@ -2300,6 +2300,14 @@ pub(crate) struct OnionErrorPacket {
23002300
pub(crate) data: Vec<u8>,
23012301
}
23022302

2303+
impl From<&UpdateFailHTLC> for OnionErrorPacket {
2304+
fn from(msg: &UpdateFailHTLC) -> Self {
2305+
OnionErrorPacket {
2306+
data: msg.reason.clone(),
2307+
}
2308+
}
2309+
}
2310+
23032311
impl fmt::Display for DecodeError {
23042312
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
23052313
match *self {
@@ -2946,13 +2954,6 @@ impl_writeable_msg!(PeerStorageRetrieval, {
29462954
data
29472955
}, {});
29482956

2949-
// Note that this is written as a part of ChannelManager objects, and thus cannot change its
2950-
// serialization format in a way which assumes we know the total serialized length/message end
2951-
// position.
2952-
impl_writeable!(OnionErrorPacket, {
2953-
data
2954-
});
2955-
29562957
// Note that this is written as a part of ChannelManager objects, and thus cannot change its
29572958
// serialization format in a way which assumes we know the total serialized length/message end
29582959
// position.
@@ -4698,13 +4699,10 @@ mod tests {
46984699

46994700
#[test]
47004701
fn encoding_update_fail_htlc() {
4701-
let reason = OnionErrorPacket {
4702-
data: [1; 32].to_vec(),
4703-
};
47044702
let update_fail_htlc = msgs::UpdateFailHTLC {
47054703
channel_id: ChannelId::from_bytes([2; 32]),
47064704
htlc_id: 2316138423780173,
4707-
reason
4705+
reason: [1; 32].to_vec()
47084706
};
47094707
let encoded_value = update_fail_htlc.encode();
47104708
let target_value = <Vec<u8>>::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034d00200101010101010101010101010101010101010101010101010101010101010101").unwrap();

‎lightning/src/ln/onion_payment.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ where
289289
).map_err(|e| {
290290
let (err_code, err_data) = match e {
291291
HTLCFailureMsg::Malformed(m) => (m.failure_code, Vec::new()),
292-
HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason.data),
292+
HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason),
293293
};
294294
let msg = "Failed to decode update add htlc onion";
295295
InboundHTLCErr { msg, err_code, err_data }
@@ -400,11 +400,12 @@ where
400400
}
401401

402402
log_info!(logger, "Failed to accept/forward incoming HTLC: {}", message);
403+
let failure = HTLCFailReason::reason(err_code, data.to_vec())
404+
.get_encrypted_failure_packet(&shared_secret, &None);
403405
return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
404406
channel_id: msg.channel_id,
405407
htlc_id: msg.htlc_id,
406-
reason: HTLCFailReason::reason(err_code, data.to_vec())
407-
.get_encrypted_failure_packet(&shared_secret, &None),
408+
reason: failure.data,
408409
}));
409410
};
410411

0 commit comments

Comments
 (0)