Convert OutboundHTLCOutcome::Success fields to struct
What changed, and why it matters
This commit is a straightforward internal code cleanup: it changes a Rust enum variant from storing two unnamed fields to storing two named fields. It does not alter what data is stored, how payments are validated, or any network-visible behavior. There is no security fix or vulnerability here.
No security action needed. Treat as a normal refactoring commit.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors OutboundHTLCOutcome::Success from a tuple variant Success(PaymentPreimage, Option
Changed components
lightning/src/ln/channel.rsInspect captured patch +49 / −29
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 8e40061..5279cff 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -353,11 +353,11 @@ impl From<&OutboundHTLCState> for OutboundHTLCStateDetails {
// the state yet.
OutboundHTLCState::RemoteRemoved(_) =>
OutboundHTLCStateDetails::Committed,
- OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(_, _)) =>
+ OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success{..}) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveSuccess,
OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Failure(_)) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFailure,
- OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(_, _)) =>
+ OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success{..}) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveSuccess,
OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Failure(_)) =>
OutboundHTLCStateDetails::AwaitingRemoteRevokeToRemoveFailure,
@@ -392,9 +392,9 @@ impl OutboundHTLCState {
#[rustfmt::skip]
fn preimage(&self) -> Option<PaymentPreimage> {
match self {
- OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(preimage, _))
- | OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(preimage, _))
- | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(preimage, _)) => {
+ OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success{preimage, ..})
+ | OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success{preimage, ..})
+ | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success{preimage, ..}) => {
Some(*preimage)
},
_ => None,
@@ -407,14 +407,17 @@ impl OutboundHTLCState {
enum OutboundHTLCOutcome {
/// We started always filling in the preimages here in 0.0.105, and the requirement
/// that the preimages always be filled in was added in 0.2.
- Success(PaymentPreimage, Option<AttributionData>),
+ Success {
+ preimage: PaymentPreimage,
+ attribution_data: Option<AttributionData>,
+ },
Failure(HTLCFailReason),
}
impl<'a> Into<Option<&'a HTLCFailReason>> for &'a OutboundHTLCOutcome {
fn into(self) -> Option<&'a HTLCFailReason> {
match self {
- OutboundHTLCOutcome::Success(_, _) => None,
+ OutboundHTLCOutcome::Success { .. } => None,
OutboundHTLCOutcome::Failure(ref r) => Some(r),
}
}
@@ -4604,10 +4607,10 @@ where
.pending_outbound_htlcs
.iter()
.filter(|OutboundHTLCOutput { state, .. }| match (state, local) {
- (OutboundHTLCState::RemoteRemoved(Success(_, _)), true) => true,
- (OutboundHTLCState::RemoteRemoved(Success(_, _)), false) => false,
- (OutboundHTLCState::AwaitingRemoteRevokeToRemove(Success(_, _)), _) => true,
- (OutboundHTLCState::AwaitingRemovedRemoteRevoke(Success(_, _)), _) => true,
+ (OutboundHTLCState::RemoteRemoved(Success { .. }), true) => true,
+ (OutboundHTLCState::RemoteRemoved(Success { .. }), false) => false,
+ (OutboundHTLCState::AwaitingRemoteRevokeToRemove(Success { .. }), _) => true,
+ (OutboundHTLCState::AwaitingRemovedRemoteRevoke(Success { .. }), _) => true,
_ => false,
})
.map(|OutboundHTLCOutput { amount_msat, .. }| amount_msat)
@@ -7766,8 +7769,8 @@ where
fn mark_outbound_htlc_removed(&mut self, htlc_id: u64, outcome: OutboundHTLCOutcome) -> Result<&OutboundHTLCOutput, ChannelError> {
for htlc in self.context.pending_outbound_htlcs.iter_mut() {
if htlc.htlc_id == htlc_id {
- if let OutboundHTLCOutcome::Success(ref payment_preimage, ..) = outcome {
- let payment_hash = PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array());
+ if let OutboundHTLCOutcome::Success { ref preimage, .. } = outcome {
+ let payment_hash = PaymentHash(Sha256::hash(&preimage.0[..]).to_byte_array());
if payment_hash != htlc.payment_hash {
return Err(ChannelError::close(format!("Remote tried to fulfill HTLC ({}) with an incorrect preimage", htlc_id)));
}
@@ -7808,8 +7811,10 @@ where
));
}
- let outcome =
- OutboundHTLCOutcome::Success(msg.payment_preimage, msg.attribution_data.clone());
+ let outcome = OutboundHTLCOutcome::Success {
+ preimage: msg.payment_preimage,
+ attribution_data: msg.attribution_data.clone(),
+ };
self.mark_outbound_htlc_removed(msg.htlc_id, outcome).map(|htlc| {
(htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat, htlc.send_timestamp)
})
@@ -8197,9 +8202,12 @@ where
log_trace!(logger, "Updating HTLC {} to AwaitingRemoteRevokeToRemove due to commitment_signed in channel {}.",
&htlc.payment_hash, &self.context.channel_id);
// Swap against a dummy variant to avoid a potentially expensive clone of `OutboundHTLCOutcome::Failure(HTLCFailReason)`
- let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None);
+ let mut reason = OutboundHTLCOutcome::Success {
+ preimage: PaymentPreimage([0u8; 32]),
+ attribution_data: None,
+ };
mem::swap(outcome, &mut reason);
- if let OutboundHTLCOutcome::Success(preimage, _) = reason {
+ if let OutboundHTLCOutcome::Success { preimage, .. } = reason {
// If a user (a) receives an HTLC claim using LDK 0.0.104 or before, then (b)
// upgrades to LDK 0.0.114 or later before the HTLC is fully resolved, we could
// have a `Success(None)` reason. In this case we could forget some HTLC
@@ -8672,7 +8680,7 @@ where
});
revoked_htlcs.push((htlc.source.clone(), htlc.payment_hash, reason));
},
- OutboundHTLCOutcome::Success(_, attribution_data) => {
+ OutboundHTLCOutcome::Success { attribution_data, .. } => {
// Even though a fast track was taken for fulfilled HTLCs to the incoming side, we still
// pass along attribution data here so that we can include hold time information in the
// final PaymentPathSuccessful events.
@@ -8781,7 +8789,10 @@ where
{
log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", &htlc.payment_hash);
// Swap against a dummy variant to avoid a potentially expensive clone of `OutboundHTLCOutcome::Failure(HTLCFailReason)`
- let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None);
+ let mut reason = OutboundHTLCOutcome::Success {
+ preimage: PaymentPreimage([0u8; 32]),
+ attribution_data: None,
+ };
mem::swap(outcome, &mut reason);
htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(reason);
require_commitment = true;
@@ -12720,7 +12731,7 @@ where
if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut outcome) = &mut htlc.state {
log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", &htlc.payment_hash);
// Swap against a dummy variant to avoid a potentially expensive clone of `OutboundHTLCOutcome::Failure(HTLCFailReason)`
- let mut reason = OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None);
+ let mut reason = OutboundHTLCOutcome::Success { preimage:PaymentPreimage([0u8; 32]), attribution_data:None };
mem::swap(outcome, &mut reason);
htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(reason);
}
@@ -14552,7 +14563,7 @@ where
},
&OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref outcome) => {
3u8.write(writer)?;
- if let OutboundHTLCOutcome::Success(preimage, attribution_data) = outcome {
+ if let OutboundHTLCOutcome::Success { preimage, attribution_data } = outcome {
preimages.push(Some(preimage));
fulfill_attribution_data.push(attribution_data);
}
@@ -14561,7 +14572,7 @@ where
},
&OutboundHTLCState::AwaitingRemovedRemoteRevoke(ref outcome) => {
4u8.write(writer)?;
- if let OutboundHTLCOutcome::Success(preimage, attribution_data) = outcome {
+ if let OutboundHTLCOutcome::Success { preimage, attribution_data } = outcome {
preimages.push(Some(preimage));
fulfill_attribution_data.push(attribution_data);
}
@@ -14998,7 +15009,10 @@ where
let outcome = match option {
Some(r) => OutboundHTLCOutcome::Failure(r),
// Initialize this variant with a dummy preimage, the actual preimage will be filled in further down
- None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None),
+ None => OutboundHTLCOutcome::Success {
+ preimage: PaymentPreimage([0u8; 32]),
+ attribution_data: None,
+ },
};
OutboundHTLCState::RemoteRemoved(outcome)
},
@@ -15007,7 +15021,10 @@ where
let outcome = match option {
Some(r) => OutboundHTLCOutcome::Failure(r),
// Initialize this variant with a dummy preimage, the actual preimage will be filled in further down
- None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None),
+ None => OutboundHTLCOutcome::Success {
+ preimage: PaymentPreimage([0u8; 32]),
+ attribution_data: None,
+ },
};
OutboundHTLCState::AwaitingRemoteRevokeToRemove(outcome)
},
@@ -15016,7 +15033,10 @@ where
let outcome = match option {
Some(r) => OutboundHTLCOutcome::Failure(r),
// Initialize this variant with a dummy preimage, the actual preimage will be filled in further down
- None => OutboundHTLCOutcome::Success(PaymentPreimage([0u8; 32]), None),
+ None => OutboundHTLCOutcome::Success {
+ preimage: PaymentPreimage([0u8; 32]),
+ attribution_data: None,
+ },
};
OutboundHTLCState::AwaitingRemovedRemoteRevoke(outcome)
},
@@ -15303,14 +15323,14 @@ where
let mut fulfill_attribution_data_iter = fulfill_attribution_data.map(Vec::into_iter);
for htlc in pending_outbound_htlcs.iter_mut() {
match &mut htlc.state {
- OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(
+ OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success {
ref mut preimage,
ref mut attribution_data,
- ))
- | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(
+ })
+ | OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success {
ref mut preimage,
ref mut attribution_data,
- )) => {
+ }) => {
// This variant was initialized like this further above
debug_assert_eq!(preimage, &PaymentPreimage([0u8; 32]));
// Flatten and unwrap the preimage; they are always set starting in 0.2.
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.