Skip to content

Commit 5d75b36

Browse files
attributable failures pre-factor
Co-authored-by: Matt Corallo <[email protected]>
1 parent 7188d5b commit 5d75b36

File tree

7 files changed

+231
-128
lines changed

7 files changed

+231
-128
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};
@@ -4718,7 +4718,7 @@ trait FailHTLCContents {
47184718
impl FailHTLCContents for msgs::OnionErrorPacket {
47194719
type Message = msgs::UpdateFailHTLC;
47204720
fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message {
4721-
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self }
4721+
msgs::UpdateFailHTLC { htlc_id, channel_id, reason: self.data }
47224722
}
47234723
fn to_inbound_htlc_state(self) -> InboundHTLCState {
47244724
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(self))
@@ -6037,7 +6037,7 @@ impl<SP: Deref> FundedChannel<SP> where
60376037
require_commitment = true;
60386038
match fail_msg {
60396039
HTLCFailureMsg::Relay(msg) => {
6040-
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(msg.reason.clone()));
6040+
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay((&msg).into()));
60416041
update_fail_htlcs.push(msg)
60426042
},
60436043
HTLCFailureMsg::Malformed(msg) => {
@@ -6745,7 +6745,7 @@ impl<SP: Deref> FundedChannel<SP> where
67456745
update_fail_htlcs.push(msgs::UpdateFailHTLC {
67466746
channel_id: self.context.channel_id(),
67476747
htlc_id: htlc.htlc_id,
6748-
reason: err_packet.clone()
6748+
reason: err_packet.data.clone(),
67496749
});
67506750
},
67516751
&InboundHTLCRemovalReason::FailMalformed((ref sha256_of_onion, ref failure_code)) => {
@@ -9893,11 +9893,6 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures)
98939893
const SERIALIZATION_VERSION: u8 = 4;
98949894
const MIN_SERIALIZATION_VERSION: u8 = 4;
98959895

9896-
impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,;
9897-
(0, FailRelay),
9898-
(1, FailMalformed),
9899-
(2, Fulfill),
9900-
);
99019896

99029897
impl Writeable for ChannelUpdateStatus {
99039898
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
@@ -10027,7 +10022,20 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1002710022
},
1002810023
&InboundHTLCState::LocalRemoved(ref removal_reason) => {
1002910024
4u8.write(writer)?;
10030-
removal_reason.write(writer)?;
10025+
match removal_reason {
10026+
InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket { data }) => {
10027+
0u8.write(writer)?;
10028+
data.write(writer)?;
10029+
},
10030+
InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
10031+
1u8.write(writer)?;
10032+
(hash, code).write(writer)?;
10033+
},
10034+
InboundHTLCRemovalReason::Fulfill(preimage) => {
10035+
2u8.write(writer)?;
10036+
preimage.write(writer)?;
10037+
},
10038+
}
1003110039
},
1003210040
}
1003310041
}
@@ -10106,7 +10114,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1010610114
&HTLCUpdateAwaitingACK::FailHTLC { ref htlc_id, ref err_packet } => {
1010710115
2u8.write(writer)?;
1010810116
htlc_id.write(writer)?;
10109-
err_packet.write(writer)?;
10117+
err_packet.data.write(writer)?;
1011010118
}
1011110119
&HTLCUpdateAwaitingACK::FailMalformedHTLC {
1011210120
htlc_id, failure_code, sha256_of_onion
@@ -10115,10 +10123,9 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1011510123
// `::FailHTLC` variant and write the real malformed error as an optional TLV.
1011610124
malformed_htlcs.push((htlc_id, failure_code, sha256_of_onion));
1011710125

10118-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1011910126
2u8.write(writer)?;
1012010127
htlc_id.write(writer)?;
10121-
dummy_err_packet.write(writer)?;
10128+
Vec::<u8>::new().write(writer)?;
1012210129
}
1012310130
}
1012410131
}
@@ -10364,7 +10371,17 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1036410371
InboundHTLCState::AwaitingAnnouncedRemoteRevoke(resolution)
1036510372
},
1036610373
3 => InboundHTLCState::Committed,
10367-
4 => InboundHTLCState::LocalRemoved(Readable::read(reader)?),
10374+
4 => {
10375+
let reason = match <u8 as Readable>::read(reader)? {
10376+
0 => InboundHTLCRemovalReason::FailRelay(msgs::OnionErrorPacket {
10377+
data: Readable::read(reader)?,
10378+
}),
10379+
1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
10380+
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
10381+
_ => return Err(DecodeError::InvalidValue),
10382+
};
10383+
InboundHTLCState::LocalRemoved(reason)
10384+
},
1036810385
_ => return Err(DecodeError::InvalidValue),
1036910386
},
1037010387
});
@@ -10420,7 +10437,9 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1042010437
},
1042110438
2 => HTLCUpdateAwaitingACK::FailHTLC {
1042210439
htlc_id: Readable::read(reader)?,
10423-
err_packet: Readable::read(reader)?,
10440+
err_packet: OnionErrorPacket {
10441+
data: Readable::read(reader)?,
10442+
},
1042410443
},
1042510444
_ => return Err(DecodeError::InvalidValue),
1042610445
});

lightning/src/ln/channelmanager.rs

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4412,11 +4412,12 @@ where
44124412
} else {
44134413
(err_code, &res.0[..])
44144414
};
4415+
let failure = HTLCFailReason::reason(err_code, err_data.to_vec())
4416+
.get_encrypted_failure_packet(shared_secret, &None);
44154417
HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44164418
channel_id: msg.channel_id,
44174419
htlc_id: msg.htlc_id,
4418-
reason: HTLCFailReason::reason(err_code, err_data.to_vec())
4419-
.get_encrypted_failure_packet(shared_secret, &None),
4420+
reason: failure.data.clone(),
44204421
})
44214422
}
44224423

@@ -4440,11 +4441,12 @@ where
44404441
}
44414442
))
44424443
}
4444+
let failure = HTLCFailReason::reason($err_code, $data.to_vec())
4445+
.get_encrypted_failure_packet(&shared_secret, &None);
44434446
return PendingHTLCStatus::Fail(HTLCFailureMsg::Relay(msgs::UpdateFailHTLC {
44444447
channel_id: msg.channel_id,
44454448
htlc_id: msg.htlc_id,
4446-
reason: HTLCFailReason::reason($err_code, $data.to_vec())
4447-
.get_encrypted_failure_packet(&shared_secret, &None),
4449+
reason: failure.data,
44484450
}));
44494451
}
44504452
}
@@ -5783,7 +5785,7 @@ where
57835785
let failure = match htlc_fail {
57845786
HTLCFailureMsg::Relay(fail_htlc) => HTLCForwardInfo::FailHTLC {
57855787
htlc_id: fail_htlc.htlc_id,
5786-
err_packet: fail_htlc.reason,
5788+
err_packet: (&fail_htlc).into(),
57875789
},
57885790
HTLCFailureMsg::Malformed(fail_malformed_htlc) => HTLCForwardInfo::FailMalformedHTLC {
57895791
htlc_id: fail_malformed_htlc.htlc_id,
@@ -13093,19 +13095,18 @@ impl Writeable for HTLCForwardInfo {
1309313095
FAIL_HTLC_VARIANT_ID.write(w)?;
1309413096
write_tlv_fields!(w, {
1309513097
(0, htlc_id, required),
13096-
(2, err_packet, required),
13098+
(2, err_packet.data, required),
1309713099
});
1309813100
},
1309913101
Self::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => {
1310013102
// Since this variant was added in 0.0.119, write this as `::FailHTLC` with an empty error
1310113103
// packet so older versions have something to fail back with, but serialize the real data as
1310213104
// optional TLVs for the benefit of newer versions.
1310313105
FAIL_HTLC_VARIANT_ID.write(w)?;
13104-
let dummy_err_packet = msgs::OnionErrorPacket { data: Vec::new() };
1310513106
write_tlv_fields!(w, {
1310613107
(0, htlc_id, required),
1310713108
(1, failure_code, required),
13108-
(2, dummy_err_packet, required),
13109+
(2, Vec::<u8>::new(), required),
1310913110
(3, sha256_of_onion, required),
1311013111
});
1311113112
},
@@ -13135,7 +13136,9 @@ impl Readable for HTLCForwardInfo {
1313513136
} else {
1313613137
Self::FailHTLC {
1313713138
htlc_id: _init_tlv_based_struct_field!(htlc_id, required),
13138-
err_packet: _init_tlv_based_struct_field!(err_packet, required),
13139+
err_packet: crate::ln::msgs::OnionErrorPacket {
13140+
data: _init_tlv_based_struct_field!(err_packet, required),
13141+
},
1313913142
}
1314013143
}
1314113144
},
@@ -14836,7 +14839,7 @@ mod tests {
1483614839
use crate::events::{Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, ClosureReason};
1483714840
use crate::ln::types::ChannelId;
1483814841
use crate::types::payment::{PaymentPreimage, PaymentHash, PaymentSecret};
14839-
use crate::ln::channelmanager::{create_recv_pending_htlc_info, inbound_payment, ChannelConfigOverrides, HTLCForwardInfo, InterceptId, PaymentId, RecipientOnionFields};
14842+
use crate::ln::channelmanager::{RAACommitmentOrder, create_recv_pending_htlc_info, inbound_payment, ChannelConfigOverrides, HTLCForwardInfo, InterceptId, PaymentId, RecipientOnionFields};
1484014843
use crate::ln::functional_test_utils::*;
1484114844
use crate::ln::msgs::{self, AcceptChannel, ErrorAction};
1484214845
use crate::ln::msgs::ChannelMessageHandler;
@@ -15056,8 +15059,8 @@ mod tests {
1505615059
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
1505715060

1505815061
create_announced_chan_between_nodes(&nodes, 0, 1);
15059-
15060-
// Since we do not send peer storage, we manually simulate receiving a dummy
15062+
15063+
// Since we do not send peer storage, we manually simulate receiving a dummy
1506115064
// `PeerStorage` from the channel partner.
1506215065
nodes[0].node.handle_peer_storage(nodes[1].node.get_our_node_id(), msgs::PeerStorage{data: vec![0; 100]});
1506315066

@@ -16266,7 +16269,7 @@ mod tests {
1626616269
let mut nodes = create_network(1, &node_cfg, &chanmgrs);
1626716270

1626816271
let dummy_failed_htlc = |htlc_id| {
16269-
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] }, }
16272+
HTLCForwardInfo::FailHTLC { htlc_id, err_packet: msgs::OnionErrorPacket { data: vec![42] } }
1627016273
};
1627116274
let dummy_malformed_htlc = |htlc_id| {
1627216275
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
@@ -7054,7 +7054,7 @@ pub fn test_update_fulfill_htlc_bolt2_update_fail_htlc_before_commitment() {
70547054
let update_msg = msgs::UpdateFailHTLC{
70557055
channel_id: chan.2,
70567056
htlc_id: 0,
7057-
reason: msgs::OnionErrorPacket { data: Vec::new()},
7057+
reason: Vec::new(),
70587058
};
70597059

70607060
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
@@ -729,7 +729,7 @@ pub struct UpdateFulfillHTLC {
729729
/// A [`peer_storage`] message that can be sent to or received from a peer.
730730
///
731731
/// This message is used to distribute backup data to peers.
732-
/// If data is lost or corrupted, users can retrieve it through [`PeerStorageRetrieval`]
732+
/// If data is lost or corrupted, users can retrieve it through [`PeerStorageRetrieval`]
733733
/// to recover critical information, such as channel states, for fund recovery.
734734
///
735735
/// [`peer_storage`] is used to send our own encrypted backup data to a peer.
@@ -744,7 +744,7 @@ pub struct PeerStorage {
744744
/// A [`peer_storage_retrieval`] message that can be sent to or received from a peer.
745745
///
746746
/// This message is sent to peers for whom we store backup data.
747-
/// If we receive this message, it indicates that the peer had stored our backup data.
747+
/// If we receive this message, it indicates that the peer had stored our backup data.
748748
/// This data can be used for fund recovery in case of data loss.
749749
///
750750
/// [`peer_storage_retrieval`] is used to send the most recent backup of the peer.
@@ -765,7 +765,7 @@ pub struct UpdateFailHTLC {
765765
pub channel_id: ChannelId,
766766
/// The HTLC ID
767767
pub htlc_id: u64,
768-
pub(crate) reason: OnionErrorPacket,
768+
pub(crate) reason: Vec<u8>,
769769
}
770770

771771
/// An [`update_fail_malformed_htlc`] message to be sent to or received from a peer.
@@ -2038,6 +2038,14 @@ pub(crate) struct OnionErrorPacket {
20382038
pub(crate) data: Vec<u8>,
20392039
}
20402040

2041+
impl From<&UpdateFailHTLC> for OnionErrorPacket {
2042+
fn from(msg: &UpdateFailHTLC) -> Self {
2043+
OnionErrorPacket {
2044+
data: msg.reason.clone(),
2045+
}
2046+
}
2047+
}
2048+
20412049
impl fmt::Display for DecodeError {
20422050
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
20432051
match *self {
@@ -2684,13 +2692,6 @@ impl_writeable_msg!(PeerStorageRetrieval, {
26842692
data
26852693
}, {});
26862694

2687-
// Note that this is written as a part of ChannelManager objects, and thus cannot change its
2688-
// serialization format in a way which assumes we know the total serialized length/message end
2689-
// position.
2690-
impl_writeable!(OnionErrorPacket, {
2691-
data
2692-
});
2693-
26942695
// Note that this is written as a part of ChannelManager objects, and thus cannot change its
26952696
// serialization format in a way which assumes we know the total serialized length/message end
26962697
// position.
@@ -4436,13 +4437,10 @@ mod tests {
44364437

44374438
#[test]
44384439
fn encoding_update_fail_htlc() {
4439-
let reason = OnionErrorPacket {
4440-
data: [1; 32].to_vec(),
4441-
};
44424440
let update_fail_htlc = msgs::UpdateFailHTLC {
44434441
channel_id: ChannelId::from_bytes([2; 32]),
44444442
htlc_id: 2316138423780173,
4445-
reason
4443+
reason: [1; 32].to_vec()
44464444
};
44474445
let encoded_value = update_fail_htlc.encode();
44484446
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)