Skip to content

Commit 1426197

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 <git@bluematt.me>
1 parent ea0f099 commit 1426197

File tree

7 files changed

+100
-59
lines changed

7 files changed

+100
-59
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 35 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ use crate::ln::interactivetxs::{
3535
TX_COMMON_FIELDS_WEIGHT,
3636
};
3737
use crate::ln::msgs;
38-
use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError};
38+
use crate::ln::msgs::{ClosingSigned, ClosingSignedFeeRange, DecodeError, OnionErrorPacket};
3939
use crate::ln::script::{self, ShutdownScript};
4040
use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo, InboundHTLCDetails, InboundHTLCStateDetails, OutboundHTLCDetails, OutboundHTLCStateDetails};
4141
use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
@@ -50,7 +50,7 @@ use crate::ln::chan_utils::{
5050
#[cfg(splicing)]
5151
use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT;
5252
use crate::ln::chan_utils;
53-
use crate::ln::onion_utils::HTLCFailReason;
53+
use crate::ln::onion_utils::{HTLCFailReason};
5454
use crate::chain::BestBlock;
5555
use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator, fee_for_weight};
5656
use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
@@ -4933,7 +4933,7 @@ trait FailHTLCContents {
49334933
impl FailHTLCContents for msgs::OnionErrorPacket {
49344934
type Message = msgs::UpdateFailHTLC;
49354935
fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
4936-
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self }
4936+
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
49374937
}
49384938
fn to_inbound_htlc_state(self) -> InboundHTLCState {
49394939
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
@@ -6136,7 +6136,7 @@ impl<SP: Deref> FundedChannel<SP> where
61366136
require_commitment = true;
61376137
match fail_msg {
61386138
HTLCFailureMsg::Relay(msg) => {
6139-
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
6139+
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.clone().into()));
61406140
update_fail_htlcs.push(msg)
61416141
},
61426142
HTLCFailureMsg::Malformed(msg) => {
@@ -6844,7 +6844,7 @@ impl<SP: Deref> FundedChannel<SP> where
68446844
update_fail_htlcs.push(msgs::UpdateFailHTLC {
68456845
channel_id: self.context.channel_id(),
68466846
htlc_id: htlc.htlc_id,
6847-
reason: err_packet.clone()
6847+
reason: err_packet.data.clone(),
68486848
});
68496849
},
68506850
&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
@@ -10142,11 +10142,6 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures)
1014210142
const SERIALIZATION_VERSION: u8 = 4;
1014310143
const MIN_SERIALIZATION_VERSION: u8 = 4;
1014410144

10145-
impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,;
10146-
(0, FailRelay),
10147-
(1, FailMalformed),
10148-
(2, Fulfill),
10149-
);
1015010145

1015110146
impl Writeable for ChannelUpdateStatus {
1015210147
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
@@ -10276,7 +10271,20 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1027610271
},
1027710272
&InboundHTLCState::LocalRemoved(ref removal_reason) => {
1027810273
4u8.write(writer)?;
10279-
removal_reason.write(writer)?;
10274+
match removal_reason {
10275+
InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
10276+
0u8.write(writer)?;
10277+
data.write(writer)?;
10278+
},
10279+
InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
10280+
1u8.write(writer)?;
10281+
(hash, code).write(writer)?;
10282+
},
10283+
InboundHTLCRemovalReason::Fulfill(preimage) => {
10284+
2u8.write(writer)?;
10285+
preimage.write(writer)?;
10286+
},
10287+
}
1028010288
},
1028110289
}
1028210290
}
@@ -10355,7 +10363,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1035510363
&HTLCUpdateAwaitingACK::FailHTLC { ref htlc_id, ref err_packet } => {
1035610364
2u8.write(writer)?;
1035710365
htlc_id.write(writer)?;
10358-
err_packet.write(writer)?;
10366+
err_packet.data.write(writer)?;
1035910367
}
1036010368
&HTLCUpdateAwaitingACK::FailMalformedHTLC {
1036110369
htlc_id, failure_code, sha256_of_onion
@@ -10364,10 +10372,9 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1036410372
// `::FailHTLC` variant and write the real malformed error as an optional TLV.
1036510373
malformed_htlcs.push((htlc_id, failure_code, sha256_of_onion));
1036610374

10367-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1036810375
2u8.write(writer)?;
1036910376
htlc_id.write(writer)?;
10370-
dummy_err_packet.write(writer)?;
10377+
Vec::<u8>::new().write(writer)?;
1037110378
}
1037210379
}
1037310380
}
@@ -10613,7 +10620,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1061310620
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(resolution)
1061410621
},
1061510622
3 => InboundHTLCState::Committed,
10616-
4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?),
10623+
4 => {
10624+
let reason = match <u8 as Readable>::read(reader)? {
10625+
0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
10626+
data: Readable::read(reader)?,
10627+
}),
10628+
1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
10629+
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
10630+
_ => return Err(DecodeError::InvalidValue),
10631+
};
10632+
InboundHTLCState::LocalRemoved(reason)
10633+
},
1061710634
_ => return Err(DecodeError::InvalidValue),
1061810635
},
1061910636
});
@@ -10669,7 +10686,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1066910686
},
1067010687
2 => HTLCUpdateAwaitingACK::FailHTLC {
1067110688
htlc_id: Readable::read(reader)?,
10672-
err_packet: Readable::read(reader)?,
10689+
err_packet: OnionErrorPacket {
10690+
data: Readable::read(reader)?,
10691+
},
1067310692
},
1067410693
_ => return Err(DecodeError::InvalidValue),
1067510694
});

‎lightning/src/ln/channelmanager.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4470,11 +4470,12 @@ where
44704470
} else {
44714471
(err_code, &res.0[..])
44724472
};
4473+
let failure = HTLCFailReason::reason(err_code, err_data.to_vec())
4474+
.get_encrypted_failure_packet(shared_secret, &None);
44734475
HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44744476
channel_id: msg.channel_id,
44754477
htlc_id: msg.htlc_id,
4476-
reason: HTLCFailReason::reason(err_code, err_data.to_vec())
4477-
.get_encrypted_failure_packet(shared_secret, &None),
4478+
reason: failure.data,
44784479
})
44794480
}
44804481

@@ -4498,11 +4499,12 @@ where
44984499
}
44994500
))
45004501
}
4502+
let failure = HTLCFailReason::reason($err_code, $data.to_vec())
4503+
.get_encrypted_failure_packet(&shared_secret, &None);
45014504
return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
45024505
channel_id: msg.channel_id,
45034506
htlc_id: msg.htlc_id,
4504-
reason: HTLCFailReason::reason($err_code, $data.to_vec())
4505-
.get_encrypted_failure_packet(&shared_secret, &None),
4507+
reason: failure.data,
45064508
}));
45074509
}
45084510
}
@@ -5881,7 +5883,7 @@ where
58815883
let failure = match htlc_fail {
58825884
HTLCFailureMsg::Relay(fail_htlc) => HTLCForwardInfo::FailHTLC {
58835885
htlc_id: fail_htlc.htlc_id,
5884-
err_packet: fail_htlc.reason,
5886+
err_packet: fail_htlc.into(),
58855887
},
58865888
HTLCFailureMsg::Malformed(fail_malformed_htlc) => HTLCForwardInfo::FailMalformedHTLC {
58875889
htlc_id: fail_malformed_htlc.htlc_id,
@@ -13224,19 +13226,18 @@ impl Writeable for HTLCForwardInfo {
1322413226
FAIL_HTLC_VARIANT_ID.write(w)?;
1322513227
write_tlv_fields!(w, {
1322613228
(0, htlc_id, required),
13227-
(2, err_packet, required),
13229+
(2, err_packet.data, required),
1322813230
});
1322913231
},
1323013232
Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
1323113233
// Since this variant was added in 0.0.119, write this as `::FailHTLC` with an empty error
1323213234
// packet so older versions have something to fail back with, but serialize the real data as
1323313235
// optional TLVs for the benefit of newer versions.
1323413236
FAIL_HTLC_VARIANT_ID.write(w)?;
13235-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1323613237
write_tlv_fields!(w, {
1323713238
(0, htlc_id, required),
1323813239
(1, failure_code, required),
13239-
(2, dummy_err_packet, required),
13240+
(2, Vec::<u8>::new(), required),
1324013241
(3, sha256_of_onion, required),
1324113242
});
1324213243
},
@@ -13266,7 +13267,9 @@ impl Readable for HTLCForwardInfo {
1326613267
} else {
1326713268
Self::FailHTLC {
1326813269
htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
13269-
err_packet: _init_tlv_based_struct_field!(err_packet, required),
13270+
err_packet: crate::ln::msgs::OnionErrorPacket {
13271+
data: _init_tlv_based_struct_field!(err_packet, required),
13272+
},
1327013273
}
1327113274
}
1327213275
},
@@ -16273,7 +16276,7 @@ mod tests {
1627316276
let mut nodes = create_network(1, &node_cfg, &chanmgrs);
1627416277

1627516278
let dummy_failed_htlc = |htlc_id| {
16276-
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }, }
16279+
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } }
1627716280
};
1627816281
let dummy_malformed_htlc = |htlc_id| {
1627916282
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: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -766,7 +766,7 @@ pub struct UpdateFailHTLC {
766766
pub channel_id: ChannelId,
767767
/// The HTLC ID
768768
pub htlc_id: u64,
769-
pub(crate) reason: OnionErrorPacket,
769+
pub(crate) reason: Vec<u8>,
770770
}
771771

772772
/// An [`update_fail_malformed_htlc`] message to be sent to or received from a peer.
@@ -2355,6 +2355,14 @@ pub(crate) struct OnionErrorPacket {
23552355
pub(crate) data: Vec<u8>,
23562356
}
23572357

2358+
impl From<UpdateFailHTLC> for OnionErrorPacket {
2359+
fn from(msg: UpdateFailHTLC) -> Self {
2360+
OnionErrorPacket {
2361+
data: msg.reason,
2362+
}
2363+
}
2364+
}
2365+
23582366
impl fmt::Display for DecodeError {
23592367
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
23602368
match *self {
@@ -3001,13 +3009,6 @@ impl_writeable_msg!(PeerStorageRetrieval, {
30013009
data
30023010
}, {});
30033011

3004-
// Note that this is written as a part of ChannelManager objects, and thus cannot change its
3005-
// serialization format in a way which assumes we know the total serialized length/message end
3006-
// position.
3007-
impl_writeable!(OnionErrorPacket, {
3008-
data
3009-
});
3010-
30113012
// Note that this is written as a part of ChannelManager objects, and thus cannot change its
30123013
// serialization format in a way which assumes we know the total serialized length/message end
30133014
// position.
@@ -3933,7 +3934,7 @@ mod tests {
39333934
use crate::ln::types::ChannelId;
39343935
use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
39353936
use crate::types::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, NodeFeatures};
3936-
use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload};
3937+
use crate::ln::msgs::{self, FinalOnionHopData, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload};
39373938
use crate::ln::msgs::SocketAddress;
39383939
use crate::routing::gossip::{NodeAlias, NodeId};
39393940
use crate::util::ser::{BigSize, FixedLengthReader, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable};
@@ -4932,13 +4933,10 @@ mod tests {
49324933

49334934
#[test]
49344935
fn encoding_update_fail_htlc() {
4935-
let reason = OnionErrorPacket {
4936-
data: [1; 32].to_vec(),
4937-
};
49384936
let update_fail_htlc = msgs::UpdateFailHTLC {
49394937
channel_id: ChannelId::from_bytes([2; 32]),
49404938
htlc_id: 2316138423780173,
4941-
reason
4939+
reason: [1; 32].to_vec()
49424940
};
49434941
let encoded_value = update_fail_htlc.encode();
49444942
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
@@ -407,7 +407,7 @@ where
407407
).map_err(|e| {
408408
let (err_code, err_data) = match e {
409409
HTLCFailureMsg::Malformed(m) => (m.failure_code, Vec::new()),
410-
HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason.data),
410+
HTLCFailureMsg::Relay(r) => (0x4000 | 22, r.reason),
411411
};
412412
let msg = "Failed to decode update add htlc onion";
413413
InboundHTLCErr { msg, err_code, err_data }
@@ -512,11 +512,12 @@ where
512512
}
513513

514514
log_info!(logger, "Failed to accept/forward incoming HTLC: {}", message);
515+
let failure = HTLCFailReason::reason(err_code, data.to_vec())
516+
.get_encrypted_failure_packet(&shared_secret, &trampoline_shared_secret);
515517
return Err(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
516518
channel_id: msg.channel_id,
517519
htlc_id: msg.htlc_id,
518-
reason: HTLCFailReason::reason(err_code, data.to_vec())
519-
.get_encrypted_failure_packet(&shared_secret, &trampoline_shared_secret),
520+
reason: failure.data,
520521
}));
521522
};
522523

0 commit comments

Comments
 (0)