Skip to content

Commit 894c91e

Browse files
joostjagerTheBlueMatt
joostjager
and
TheBlueMatt
committed
Attributable failures pre-factor
This commits prepares the persistence layer for the addition of attribution data. Changes: - Expand InboundHTLCRemovalReason serialization instead of using the macro. When attribution data is added, it can't just be serialized along with the existing fields because it would break backwards compatibility. Instead the new field needs to go into the tlv block. - Stop using OnionErrorPacket in the UpdateFailHTLC message. When attribution data is added to OnionErrorPacket, it would not be serialized for the wire properly because also here the new field needs to go in the tlv extension of the message. - Prepare HTLCFailReasonRepr serialization for that addition of attribution data. Co-authored-by: Matt Corallo <[email protected]>
1 parent 5506d3d commit 894c91e

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)