Skip to content

Commit 0462dbe

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 8d392ad commit 0462dbe

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
@@ -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};
@@ -4856,7 +4856,7 @@ trait FailHTLCContents {
48564856
impl FailHTLCContents for msgs::OnionErrorPacket {
48574857
type Message = msgs::UpdateFailHTLC;
48584858
fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
4859-
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self }
4859+
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
48604860
}
48614861
fn to_inbound_htlc_state(self) -> InboundHTLCState {
48624862
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
@@ -6070,7 +6070,7 @@ impl<SP: Deref> FundedChannel<SP> where
60706070
require_commitment = true;
60716071
match fail_msg {
60726072
HTLCFailureMsg::Relay(msg) => {
6073-
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
6073+
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay((&msg).into()));
60746074
update_fail_htlcs.push(msg)
60756075
},
60766076
HTLCFailureMsg::Malformed(msg) => {
@@ -6778,7 +6778,7 @@ impl<SP: Deref> FundedChannel<SP> where
67786778
update_fail_htlcs.push(msgs::UpdateFailHTLC {
67796779
channel_id: self.context.channel_id(),
67806780
htlc_id: htlc.htlc_id,
6781-
reason: err_packet.clone()
6781+
reason: err_packet.data.clone(),
67826782
});
67836783
},
67846784
&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
@@ -9932,11 +9932,6 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures)
99329932
const SERIALIZATION_VERSION: u8 = 4;
99339933
const MIN_SERIALIZATION_VERSION: u8 = 4;
99349934

9935-
impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,;
9936-
(0, FailRelay),
9937-
(1, FailMalformed),
9938-
(2, Fulfill),
9939-
);
99409935

99419936
impl Writeable for ChannelUpdateStatus {
99429937
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
@@ -10066,7 +10061,20 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1006610061
},
1006710062
&InboundHTLCState::LocalRemoved(ref removal_reason) => {
1006810063
4u8.write(writer)?;
10069-
removal_reason.write(writer)?;
10064+
match removal_reason {
10065+
InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
10066+
0u8.write(writer)?;
10067+
data.write(writer)?;
10068+
},
10069+
InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
10070+
1u8.write(writer)?;
10071+
(hash, code).write(writer)?;
10072+
},
10073+
InboundHTLCRemovalReason::Fulfill(preimage) => {
10074+
2u8.write(writer)?;
10075+
preimage.write(writer)?;
10076+
},
10077+
}
1007010078
},
1007110079
}
1007210080
}
@@ -10145,7 +10153,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1014510153
&HTLCUpdateAwaitingACK::FailHTLC { ref htlc_id, ref err_packet } => {
1014610154
2u8.write(writer)?;
1014710155
htlc_id.write(writer)?;
10148-
err_packet.write(writer)?;
10156+
err_packet.data.write(writer)?;
1014910157
}
1015010158
&HTLCUpdateAwaitingACK::FailMalformedHTLC {
1015110159
htlc_id, failure_code, sha256_of_onion
@@ -10154,10 +10162,9 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1015410162
// `::FailHTLC` variant and write the real malformed error as an optional TLV.
1015510163
malformed_htlcs.push((htlc_id, failure_code, sha256_of_onion));
1015610164

10157-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1015810165
2u8.write(writer)?;
1015910166
htlc_id.write(writer)?;
10160-
dummy_err_packet.write(writer)?;
10167+
Vec::<u8>::new().write(writer)?;
1016110168
}
1016210169
}
1016310170
}
@@ -10403,7 +10410,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1040310410
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(resolution)
1040410411
},
1040510412
3 => InboundHTLCState::Committed,
10406-
4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?),
10413+
4 => {
10414+
let reason = match <u8 as Readable>::read(reader)? {
10415+
0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
10416+
data: Readable::read(reader)?,
10417+
}),
10418+
1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
10419+
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
10420+
_ => return Err(DecodeError::InvalidValue),
10421+
};
10422+
InboundHTLCState::LocalRemoved(reason)
10423+
},
1040710424
_ => return Err(DecodeError::InvalidValue),
1040810425
},
1040910426
});
@@ -10459,7 +10476,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1045910476
},
1046010477
2 => HTLCUpdateAwaitingACK::FailHTLC {
1046110478
htlc_id: Readable::read(reader)?,
10462-
err_packet: Readable::read(reader)?,
10479+
err_packet: OnionErrorPacket {
10480+
data: Readable::read(reader)?,
10481+
},
1046310482
},
1046410483
_ => return Err(DecodeError::InvalidValue),
1046510484
});

‎lightning/src/ln/channelmanager.rs‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4435,11 +4435,12 @@ where
44354435
} else {
44364436
(err_code, &res.0[..])
44374437
};
4438+
let failure = HTLCFailReason::reason(err_code, err_data.to_vec())
4439+
.get_encrypted_failure_packet(shared_secret, &None);
44384440
HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44394441
channel_id: msg.channel_id,
44404442
htlc_id: msg.htlc_id,
4441-
reason: HTLCFailReason::reason(err_code, err_data.to_vec())
4442-
.get_encrypted_failure_packet(shared_secret, &None),
4443+
reason: failure.data.clone(),
44434444
})
44444445
}
44454446

@@ -4463,11 +4464,12 @@ where
44634464
}
44644465
))
44654466
}
4467+
let failure = HTLCFailReason::reason($err_code, $data.to_vec())
4468+
.get_encrypted_failure_packet(&shared_secret, &None);
44664469
return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44674470
channel_id: msg.channel_id,
44684471
htlc_id: msg.htlc_id,
4469-
reason: HTLCFailReason::reason($err_code, $data.to_vec())
4470-
.get_encrypted_failure_packet(&shared_secret, &None),
4472+
reason: failure.data,
44714473
}));
44724474
}
44734475
}
@@ -5822,7 +5824,7 @@ where
58225824
let failure = match htlc_fail {
58235825
HTLCFailureMsg::Relay(fail_htlc) => HTLCForwardInfo::FailHTLC {
58245826
htlc_id: fail_htlc.htlc_id,
5825-
err_packet: fail_htlc.reason,
5827+
err_packet: (&fail_htlc).into(),
58265828
},
58275829
HTLCFailureMsg::Malformed(fail_malformed_htlc) => HTLCForwardInfo::FailMalformedHTLC {
58285830
htlc_id: fail_malformed_htlc.htlc_id,
@@ -13127,19 +13129,18 @@ impl Writeable for HTLCForwardInfo {
1312713129
FAIL_HTLC_VARIANT_ID.write(w)?;
1312813130
write_tlv_fields!(w, {
1312913131
(0, htlc_id, required),
13130-
(2, err_packet, required),
13132+
(2, err_packet.data, required),
1313113133
});
1313213134
},
1313313135
Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
1313413136
// Since this variant was added in 0.0.119, write this as `::FailHTLC` with an empty error
1313513137
// packet so older versions have something to fail back with, but serialize the real data as
1313613138
// optional TLVs for the benefit of newer versions.
1313713139
FAIL_HTLC_VARIANT_ID.write(w)?;
13138-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1313913140
write_tlv_fields!(w, {
1314013141
(0, htlc_id, required),
1314113142
(1, failure_code, required),
13142-
(2, dummy_err_packet, required),
13143+
(2, Vec::<u8>::new(), required),
1314313144
(3, sha256_of_onion, required),
1314413145
});
1314513146
},
@@ -13169,7 +13170,9 @@ impl Readable for HTLCForwardInfo {
1316913170
} else {
1317013171
Self::FailHTLC {
1317113172
htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
13172-
err_packet: _init_tlv_based_struct_field!(err_packet, required),
13173+
err_packet: crate::ln::msgs::OnionErrorPacket {
13174+
data: _init_tlv_based_struct_field!(err_packet, required),
13175+
},
1317313176
}
1317413177
}
1317513178
},
@@ -16310,7 +16313,7 @@ mod tests {
1631016313
let mut nodes = create_network(1, &node_cfg, &chanmgrs);
1631116314

1631216315
let dummy_failed_htlc = |htlc_id| {
16313-
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }, }
16316+
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } }
1631416317
};
1631516318
let dummy_malformed_htlc = |htlc_id| {
1631616319
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.clone(),
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)