Skip to content

Commit 1c64a50

Browse files
committed
Add hold times to update_fulfill_htlc
1 parent bb91bb6 commit 1c64a50

File tree

5 files changed

+278
-56
lines changed

5 files changed

+278
-56
lines changed

fuzz/src/process_onion_failure.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
115115
let path = Path { hops, blinded_tail };
116116

117117
let htlc_source = HTLCSource::OutboundRoute {
118-
path,
118+
path: path.clone(),
119119
session_priv,
120120
first_hop_htlc_msat: 0,
121121
payment_id,
@@ -133,8 +133,19 @@ fn do_test<Out: test_logger::Output>(data: &[u8], out: Out) {
133133
} else {
134134
None
135135
};
136-
let encrypted_packet = OnionErrorPacket { data: failure_data.into(), attribution_data };
136+
let encrypted_packet =
137+
OnionErrorPacket { data: failure_data.into(), attribution_data: attribution_data.clone() };
137138
lightning::ln::process_onion_failure(&secp_ctx, &logger, &htlc_source, encrypted_packet);
139+
140+
if let Some(attribution_data) = attribution_data {
141+
lightning::ln::process_onion_success(
142+
&secp_ctx,
143+
&logger,
144+
&path,
145+
&session_priv,
146+
attribution_data,
147+
);
148+
}
138149
}
139150

140151
/// Method that needs to be added manually, {name}_test

lightning/src/ln/channel.rs

Lines changed: 84 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ enum FeeUpdateState {
138138
enum InboundHTLCRemovalReason {
139139
FailRelay(msgs::OnionErrorPacket),
140140
FailMalformed(([u8; 32], u16)),
141-
Fulfill(PaymentPreimage),
141+
Fulfill(PaymentPreimage, Option<AttributionData>),
142142
}
143143

144144
/// Represents the resolution status of an inbound HTLC.
@@ -234,7 +234,7 @@ impl From<&InboundHTLCState> for Option<InboundHTLCStateDetails> {
234234
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
235235
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed(_)) =>
236236
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFail),
237-
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) =>
237+
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_, _)) =>
238238
Some(InboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFulfill),
239239
}
240240
}
@@ -266,7 +266,7 @@ impl InboundHTLCState {
266266

267267
fn preimage(&self) -> Option<PaymentPreimage> {
268268
match self {
269-
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(preimage)) => {
269+
InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(preimage, _)) => {
270270
Some(*preimage)
271271
},
272272
_ => None,
@@ -466,6 +466,7 @@ enum HTLCUpdateAwaitingACK {
466466
},
467467
ClaimHTLC {
468468
payment_preimage: PaymentPreimage,
469+
attribution_data: Option<AttributionData>,
469470
htlc_id: u64,
470471
},
471472
FailHTLC {
@@ -6214,7 +6215,7 @@ where
62146215
assert!(!self.context.channel_state.can_generate_new_commitment());
62156216
let mon_update_id = self.context.latest_monitor_update_id; // Forget the ChannelMonitor update
62166217
let fulfill_resp =
6217-
self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, None, logger);
6218+
self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, None, None, logger);
62186219
self.context.latest_monitor_update_id = mon_update_id;
62196220
if let UpdateFulfillFetch::NewClaim { update_blocked, .. } = fulfill_resp {
62206221
assert!(update_blocked); // The HTLC must have ended up in the holding cell.
@@ -6223,7 +6224,8 @@ where
62236224

62246225
fn get_update_fulfill_htlc<L: Deref>(
62256226
&mut self, htlc_id_arg: u64, payment_preimage_arg: PaymentPreimage,
6226-
payment_info: Option<PaymentClaimDetails>, logger: &L,
6227+
payment_info: Option<PaymentClaimDetails>, attribution_data: Option<AttributionData>,
6228+
logger: &L,
62276229
) -> UpdateFulfillFetch
62286230
where
62296231
L::Target: Logger,
@@ -6258,7 +6260,7 @@ where
62586260
match htlc.state {
62596261
InboundHTLCState::Committed => {},
62606262
InboundHTLCState::LocalRemoved(ref reason) => {
6261-
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
6263+
if let &InboundHTLCRemovalReason::Fulfill(_, _) = reason {
62626264
} else {
62636265
log_warn!(logger, "Have preimage and want to fulfill HTLC with payment hash {} we already failed against channel {}", &htlc.payment_hash, &self.context.channel_id());
62646266
debug_assert!(
@@ -6339,6 +6341,7 @@ where
63396341
self.context.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
63406342
payment_preimage: payment_preimage_arg,
63416343
htlc_id: htlc_id_arg,
6344+
attribution_data,
63426345
});
63436346
return UpdateFulfillFetch::NewClaim {
63446347
monitor_update,
@@ -6369,6 +6372,7 @@ where
63696372
);
63706373
htlc.state = InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(
63716374
payment_preimage_arg.clone(),
6375+
attribution_data,
63726376
));
63736377
}
63746378

@@ -6377,13 +6381,20 @@ where
63776381

63786382
pub fn get_update_fulfill_htlc_and_commit<L: Deref>(
63796383
&mut self, htlc_id: u64, payment_preimage: PaymentPreimage,
6380-
payment_info: Option<PaymentClaimDetails>, logger: &L,
6384+
payment_info: Option<PaymentClaimDetails>, attribution_data: Option<AttributionData>,
6385+
logger: &L,
63816386
) -> UpdateFulfillCommitFetch
63826387
where
63836388
L::Target: Logger,
63846389
{
63856390
let release_cs_monitor = self.context.blocked_monitor_updates.is_empty();
6386-
match self.get_update_fulfill_htlc(htlc_id, payment_preimage, payment_info, logger) {
6391+
match self.get_update_fulfill_htlc(
6392+
htlc_id,
6393+
payment_preimage,
6394+
payment_info,
6395+
attribution_data,
6396+
logger,
6397+
) {
63876398
UpdateFulfillFetch::NewClaim {
63886399
mut monitor_update,
63896400
htlc_value_msat,
@@ -6717,7 +6728,7 @@ where
67176728

67186729
pub fn update_fulfill_htlc(
67196730
&mut self, msg: &msgs::UpdateFulfillHTLC,
6720-
) -> Result<(HTLCSource, u64, Option<u64>), ChannelError> {
6731+
) -> Result<(HTLCSource, u64, Option<u64>, Option<Duration>), ChannelError> {
67216732
if self.context.channel_state.is_remote_stfu_sent()
67226733
|| self.context.channel_state.is_quiescent()
67236734
{
@@ -6740,7 +6751,9 @@ where
67406751
msg.htlc_id,
67416752
OutboundHTLCOutcome::Success(msg.payment_preimage),
67426753
)
6743-
.map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat))
6754+
.map(|htlc| {
6755+
(htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat, htlc.send_timestamp)
6756+
})
67446757
}
67456758

67466759
#[rustfmt::skip]
@@ -7276,7 +7289,11 @@ where
72767289
}
72777290
None
72787291
},
7279-
&HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => {
7292+
&HTLCUpdateAwaitingACK::ClaimHTLC {
7293+
ref payment_preimage,
7294+
htlc_id,
7295+
ref attribution_data,
7296+
} => {
72807297
// If an HTLC claim was previously added to the holding cell (via
72817298
// `get_update_fulfill_htlc`, then generating the claim message itself must
72827299
// not fail - any in between attempts to claim the HTLC will have resulted
@@ -7289,8 +7306,13 @@ where
72897306
// We do not bother to track and include `payment_info` here, however.
72907307
let mut additional_monitor_update =
72917308
if let UpdateFulfillFetch::NewClaim { monitor_update, .. } = self
7292-
.get_update_fulfill_htlc(htlc_id, *payment_preimage, None, logger)
7293-
{
7309+
.get_update_fulfill_htlc(
7310+
htlc_id,
7311+
*payment_preimage,
7312+
None,
7313+
attribution_data.clone(),
7314+
logger,
7315+
) {
72947316
monitor_update
72957317
} else {
72967318
unreachable!()
@@ -7507,7 +7529,7 @@ where
75077529
pending_inbound_htlcs.retain(|htlc| {
75087530
if let &InboundHTLCState::LocalRemoved(ref reason) = &htlc.state {
75097531
log_trace!(logger, " ...removing inbound LocalRemoved {}", &htlc.payment_hash);
7510-
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
7532+
if let &InboundHTLCRemovalReason::Fulfill(_, _) = reason {
75117533
value_to_self_msat_diff += htlc.amount_msat as i64;
75127534
}
75137535
*expecting_peer_commitment_signed = true;
@@ -8380,12 +8402,15 @@ where
83808402
failure_code: failure_code.clone(),
83818403
});
83828404
},
8383-
&InboundHTLCRemovalReason::Fulfill(ref payment_preimage) => {
8405+
&InboundHTLCRemovalReason::Fulfill(
8406+
ref payment_preimage,
8407+
ref attribution_data,
8408+
) => {
83848409
update_fulfill_htlcs.push(msgs::UpdateFulfillHTLC {
83858410
channel_id: self.context.channel_id(),
83868411
htlc_id: htlc.htlc_id,
83878412
payment_preimage: payment_preimage.clone(),
8388-
attribution_data: None,
8413+
attribution_data: attribution_data.clone(),
83898414
});
83908415
},
83918416
}
@@ -12457,7 +12482,7 @@ where
1245712482
dropped_inbound_htlcs += 1;
1245812483
}
1245912484
}
12460-
let mut removed_htlc_failure_attribution_data: Vec<&Option<AttributionData>> = Vec::new();
12485+
let mut removed_htlc_attribution_data: Vec<&Option<AttributionData>> = Vec::new();
1246112486
(self.context.pending_inbound_htlcs.len() as u64 - dropped_inbound_htlcs).write(writer)?;
1246212487
for htlc in self.context.pending_inbound_htlcs.iter() {
1246312488
if let &InboundHTLCState::RemoteAnnounced(_) = &htlc.state {
@@ -12489,13 +12514,14 @@ where
1248912514
}) => {
1249012515
0u8.write(writer)?;
1249112516
data.write(writer)?;
12492-
removed_htlc_failure_attribution_data.push(&attribution_data);
12517+
removed_htlc_attribution_data.push(&attribution_data);
1249312518
},
1249412519
InboundHTLCRemovalReason::FailMalformed((hash, code)) => {
1249512520
1u8.write(writer)?;
1249612521
(hash, code).write(writer)?;
1249712522
},
12498-
InboundHTLCRemovalReason::Fulfill(preimage) => {
12523+
InboundHTLCRemovalReason::Fulfill(preimage, _) => {
12524+
// TODO: Persistence
1249912525
2u8.write(writer)?;
1250012526
preimage.write(writer)?;
1250112527
},
@@ -12556,7 +12582,7 @@ where
1255612582
Vec::with_capacity(holding_cell_htlc_update_count);
1255712583
let mut holding_cell_blinding_points: Vec<Option<PublicKey>> =
1255812584
Vec::with_capacity(holding_cell_htlc_update_count);
12559-
let mut holding_cell_failure_attribution_data: Vec<Option<&AttributionData>> =
12585+
let mut holding_cell_attribution_data: Vec<Option<&AttributionData>> =
1256012586
Vec::with_capacity(holding_cell_htlc_update_count);
1256112587
// Vec of (htlc_id, failure_code, sha256_of_onion)
1256212588
let mut malformed_htlcs: Vec<(u64, u16, [u8; 32])> = Vec::new();
@@ -12582,19 +12608,25 @@ where
1258212608
holding_cell_skimmed_fees.push(skimmed_fee_msat);
1258312609
holding_cell_blinding_points.push(blinding_point);
1258412610
},
12585-
&HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, ref htlc_id } => {
12611+
&HTLCUpdateAwaitingACK::ClaimHTLC {
12612+
ref payment_preimage,
12613+
ref htlc_id,
12614+
ref attribution_data,
12615+
} => {
1258612616
1u8.write(writer)?;
1258712617
payment_preimage.write(writer)?;
1258812618
htlc_id.write(writer)?;
12619+
12620+
// Store the attribution data for later writing.
12621+
holding_cell_attribution_data.push(attribution_data.as_ref());
1258912622
},
1259012623
&HTLCUpdateAwaitingACK::FailHTLC { ref htlc_id, ref err_packet } => {
1259112624
2u8.write(writer)?;
1259212625
htlc_id.write(writer)?;
1259312626
err_packet.data.write(writer)?;
1259412627

1259512628
// Store the attribution data for later writing.
12596-
holding_cell_failure_attribution_data
12597-
.push(err_packet.attribution_data.as_ref());
12629+
holding_cell_attribution_data.push(err_packet.attribution_data.as_ref());
1259812630
},
1259912631
&HTLCUpdateAwaitingACK::FailMalformedHTLC {
1260012632
htlc_id,
@@ -12611,7 +12643,7 @@ where
1261112643

1261212644
// Push 'None' attribution data for FailMalformedHTLC, because FailMalformedHTLC uses the same
1261312645
// type 2 and is deserialized as a FailHTLC.
12614-
holding_cell_failure_attribution_data.push(None);
12646+
holding_cell_attribution_data.push(None);
1261512647
},
1261612648
}
1261712649
}
@@ -12814,8 +12846,8 @@ where
1281412846
(51, is_manual_broadcast, option), // Added in 0.0.124
1281512847
(53, funding_tx_broadcast_safe_event_emitted, option), // Added in 0.0.124
1281612848
(54, self.pending_funding, optional_vec), // Added in 0.2
12817-
(55, removed_htlc_failure_attribution_data, optional_vec), // Added in 0.2
12818-
(57, holding_cell_failure_attribution_data, optional_vec), // Added in 0.2
12849+
(55, removed_htlc_attribution_data, optional_vec), // Added in 0.2
12850+
(57, holding_cell_attribution_data, optional_vec), // Added in 0.2
1281912851
(58, self.interactive_tx_signing_session, option), // Added in 0.2
1282012852
(59, self.funding.minimum_depth_override, option), // Added in 0.2
1282112853
(60, self.context.historical_scids, optional_vec), // Added in 0.2
@@ -12910,7 +12942,7 @@ where
1291012942
attribution_data: None,
1291112943
}),
1291212944
1 => InboundHTLCRemovalReason::FailMalformed(Readable::read(reader)?),
12913-
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?),
12945+
2 => InboundHTLCRemovalReason::Fulfill(Readable::read(reader)?, None),
1291412946
_ => return Err(DecodeError::InvalidValue),
1291512947
};
1291612948
InboundHTLCState::LocalRemoved(reason)
@@ -12989,6 +13021,7 @@ where
1298913021
1 => HTLCUpdateAwaitingACK::ClaimHTLC {
1299013022
payment_preimage: Readable::read(reader)?,
1299113023
htlc_id: Readable::read(reader)?,
13024+
attribution_data: None,
1299213025
},
1299313026
2 => HTLCUpdateAwaitingACK::FailHTLC {
1299413027
htlc_id: Readable::read(reader)?,
@@ -13160,8 +13193,8 @@ where
1316013193
let mut pending_outbound_blinding_points_opt: Option<Vec<Option<PublicKey>>> = None;
1316113194
let mut holding_cell_blinding_points_opt: Option<Vec<Option<PublicKey>>> = None;
1316213195

13163-
let mut removed_htlc_failure_attribution_data: Option<Vec<Option<AttributionData>>> = None;
13164-
let mut holding_cell_failure_attribution_data: Option<Vec<Option<AttributionData>>> = None;
13196+
let mut removed_htlc_attribution_data: Option<Vec<Option<AttributionData>>> = None;
13197+
let mut holding_cell_attribution_data: Option<Vec<Option<AttributionData>>> = None;
1316513198

1316613199
let mut malformed_htlcs: Option<Vec<(u64, u16, [u8; 32])>> = None;
1316713200
let mut monitor_pending_update_adds: Option<Vec<msgs::UpdateAddHTLC>> = None;
@@ -13213,8 +13246,8 @@ where
1321313246
(51, is_manual_broadcast, option),
1321413247
(53, funding_tx_broadcast_safe_event_emitted, option),
1321513248
(54, pending_funding, optional_vec), // Added in 0.2
13216-
(55, removed_htlc_failure_attribution_data, optional_vec),
13217-
(57, holding_cell_failure_attribution_data, optional_vec),
13249+
(55, removed_htlc_attribution_data, optional_vec),
13250+
(57, holding_cell_attribution_data, optional_vec),
1321813251
(58, interactive_tx_signing_session, option), // Added in 0.2
1321913252
(59, minimum_depth_override, option), // Added in 0.2
1322013253
(60, historical_scids, optional_vec), // Added in 0.2
@@ -13317,14 +13350,19 @@ where
1331713350
}
1331813351
}
1331913352

13320-
if let Some(attribution_data_list) = removed_htlc_failure_attribution_data {
13353+
if let Some(attribution_data_list) = removed_htlc_attribution_data {
1332113354
let mut removed_htlc_relay_failures =
1332213355
pending_inbound_htlcs.iter_mut().filter_map(|status| {
13323-
if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailRelay(
13324-
ref mut packet,
13325-
)) = &mut status.state
13326-
{
13327-
Some(&mut packet.attribution_data)
13356+
if let InboundHTLCState::LocalRemoved(reason) = &mut status.state {
13357+
match reason {
13358+
InboundHTLCRemovalReason::FailRelay(ref mut packet) => {
13359+
Some(&mut packet.attribution_data)
13360+
},
13361+
InboundHTLCRemovalReason::Fulfill(_, ref mut attribution_data) => {
13362+
Some(attribution_data)
13363+
},
13364+
_ => None,
13365+
}
1332813366
} else {
1332913367
None
1333013368
}
@@ -13339,18 +13377,17 @@ where
1333913377
}
1334013378
}
1334113379

13342-
if let Some(attribution_data_list) = holding_cell_failure_attribution_data {
13380+
if let Some(attribution_data_list) = holding_cell_attribution_data {
1334313381
let mut holding_cell_failures =
13344-
holding_cell_htlc_updates.iter_mut().filter_map(|upd| {
13345-
if let HTLCUpdateAwaitingACK::FailHTLC {
13382+
holding_cell_htlc_updates.iter_mut().filter_map(|upd| match upd {
13383+
HTLCUpdateAwaitingACK::FailHTLC {
1334613384
err_packet: OnionErrorPacket { ref mut attribution_data, .. },
1334713385
..
13348-
} = upd
13349-
{
13386+
} => Some(attribution_data),
13387+
HTLCUpdateAwaitingACK::ClaimHTLC { attribution_data, .. } => {
1335013388
Some(attribution_data)
13351-
} else {
13352-
None
13353-
}
13389+
},
13390+
_ => None,
1335413391
});
1335513392

1335613393
for attribution_data in attribution_data_list {
@@ -13567,7 +13604,7 @@ where
1356713604
}
1356813605
}
1356913606

13570-
fn duration_since_epoch() -> Option<Duration> {
13607+
pub(crate) fn duration_since_epoch() -> Option<Duration> {
1357113608
#[cfg(not(feature = "std"))]
1357213609
let now = None;
1357313610

@@ -14339,6 +14376,7 @@ mod tests {
1433914376
let dummy_holding_cell_claim_htlc = HTLCUpdateAwaitingACK::ClaimHTLC {
1434014377
payment_preimage: PaymentPreimage([42; 32]),
1434114378
htlc_id: 0,
14379+
attribution_data: None,
1434214380
};
1434314381
let dummy_holding_cell_failed_htlc = |htlc_id| HTLCUpdateAwaitingACK::FailHTLC {
1434414382
htlc_id,

0 commit comments

Comments
 (0)