Skip to content

Commit

Permalink
Remove payment_release_secret from async payments messages.
Browse files Browse the repository at this point in the history
This field isn't necessary because we already authenticate the messages via the
blinded reply paths payment_id, nonce and HMAC.
  • Loading branch information
valentinewallace committed Sep 13, 2024
1 parent 4bcf53e commit 6e27aec
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 49 deletions.
5 changes: 1 addition & 4 deletions fuzz/src/onion_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,7 @@ impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {
Some(resp) => resp,
None => return None,
};
Some((
ReleaseHeldHtlc { payment_release_secret: message.payment_release_secret },
responder.respond(),
))
Some((ReleaseHeldHtlc {}, responder.respond()))
}
fn release_held_htlc(&self, _message: ReleaseHeldHtlc, _context: AsyncPaymentsContext) {}
}
Expand Down
22 changes: 9 additions & 13 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4359,8 +4359,8 @@ where
invoice, payment_id, features, best_block_height, &*self.entropy_source,
&self.pending_events
);
let payment_release_secret = match outbound_pmts_res {
Ok(secret) => secret,
match outbound_pmts_res {
Ok(()) => {},
Err(Bolt12PaymentError::UnexpectedInvoice) | Err(Bolt12PaymentError::DuplicateInvoice) => {
res = outbound_pmts_res.map(|_| ());
return NotifyOption::SkipPersistNoEvents
Expand Down Expand Up @@ -4397,9 +4397,7 @@ where
destination: Destination::BlindedPath(invoice_path.clone()),
reply_path: reply_path.clone(),
};
let message = AsyncPaymentsMessage::HeldHtlcAvailable(
HeldHtlcAvailable { payment_release_secret }
);
let message = AsyncPaymentsMessage::HeldHtlcAvailable(HeldHtlcAvailable {});
pending_async_payments_messages.push((message, instructions));
});

Expand All @@ -4411,16 +4409,15 @@ where

#[cfg(async_payments)]
fn send_payment_for_static_invoice(
&self, payment_id: PaymentId, payment_release_secret: [u8; 32]
&self, payment_id: PaymentId
) -> Result<(), Bolt12PaymentError> {
let best_block_height = self.best_block.read().unwrap().height;
let mut res = Ok(());
PersistenceNotifierGuard::optionally_notify(self, || {
let outbound_pmts_res = self.pending_outbound_payments.send_payment_for_static_invoice(
payment_id, payment_release_secret, &self.router, self.list_usable_channels(),
|| self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, &self,
&self.secp_ctx, best_block_height, &self.logger, &self.pending_events,
|args| self.send_payment_along_path(args)
payment_id, &self.router, self.list_usable_channels(), || self.compute_inflight_htlcs(),
&self.entropy_source, &self.node_signer, &self, &self.secp_ctx, best_block_height,
&self.logger, &self.pending_events, |args| self.send_payment_along_path(args)
);
match outbound_pmts_res {
Err(Bolt12PaymentError::UnexpectedInvoice) | Err(Bolt12PaymentError::DuplicateInvoice) => {
Expand Down Expand Up @@ -11239,10 +11236,9 @@ where
#[cfg(async_payments)] {
let AsyncPaymentsContext::OutboundPayment { payment_id, hmac, nonce } = _context;
if payment_id.verify_for_async_payment(hmac, nonce, &self.inbound_payment_key).is_err() { return }
if let Err(e) = self.send_payment_for_static_invoice(payment_id, _message.payment_release_secret) {
if let Err(e) = self.send_payment_for_static_invoice(payment_id) {
log_trace!(
self.logger, "Failed to release held HTLC with payment id {} and release secret {:02x?}: {:?}",
payment_id, _message.payment_release_secret, e
self.logger, "Failed to release held HTLC with payment id {}: {:?}", payment_id, e
);
}
}
Expand Down
24 changes: 7 additions & 17 deletions lightning/src/ln/outbound_payment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ pub(crate) enum PendingOutboundPayment {
payment_hash: PaymentHash,
keysend_preimage: PaymentPreimage,
retry_strategy: Retry,
payment_release_secret: [u8; 32],
route_params: RouteParameters,
},
Retryable {
Expand Down Expand Up @@ -984,7 +983,7 @@ impl OutboundPayments {
&self, invoice: &StaticInvoice, payment_id: PaymentId, features: Bolt12InvoiceFeatures,
best_block_height: u32, entropy_source: ES,
pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>
) -> Result<[u8; 32], Bolt12PaymentError> where ES::Target: EntropySource {
) -> Result<(), Bolt12PaymentError> where ES::Target: EntropySource {
macro_rules! abandon_with_entry {
($payment: expr, $reason: expr) => {
$payment.get_mut().mark_abandoned($reason);
Expand Down Expand Up @@ -1029,7 +1028,6 @@ impl OutboundPayments {
};
let keysend_preimage = PaymentPreimage(entropy_source.get_secure_random_bytes());
let payment_hash = PaymentHash(Sha256::hash(&keysend_preimage.0).to_byte_array());
let payment_release_secret = entropy_source.get_secure_random_bytes();
let pay_params = PaymentParameters::from_static_invoice(invoice);
let mut route_params = RouteParameters::from_payment_params_and_value(pay_params, amount_msat);
route_params.max_total_routing_fee_msat = *max_total_routing_fee_msat;
Expand All @@ -1046,10 +1044,9 @@ impl OutboundPayments {
payment_hash,
keysend_preimage,
retry_strategy: *retry_strategy,
payment_release_secret,
route_params,
};
return Ok(payment_release_secret)
return Ok(())
},
_ => return Err(Bolt12PaymentError::DuplicateInvoice),
},
Expand All @@ -1061,9 +1058,9 @@ impl OutboundPayments {
pub(super) fn send_payment_for_static_invoice<
R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP, L: Deref
>(
&self, payment_id: PaymentId, payment_release_secret: [u8; 32], router: &R,
first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
node_id_lookup: &NL, secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
&self, payment_id: PaymentId, router: &R, first_hops: Vec<ChannelDetails>, inflight_htlcs: IH,
entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL,
secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
send_payment_along_path: SP,
) -> Result<(), Bolt12PaymentError>
Expand All @@ -1080,12 +1077,8 @@ impl OutboundPayments {
match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
hash_map::Entry::Occupied(entry) => match entry.get() {
PendingOutboundPayment::StaticInvoiceReceived {
payment_hash, payment_release_secret: release_secret, route_params, retry_strategy,
keysend_preimage, ..
payment_hash, route_params, retry_strategy, keysend_preimage, ..
} => {
if payment_release_secret != *release_secret {
return Err(Bolt12PaymentError::UnexpectedInvoice)
}
(*payment_hash, *keysend_preimage, route_params.clone(), *retry_strategy)
},
_ => return Err(Bolt12PaymentError::DuplicateInvoice),
Expand Down Expand Up @@ -2196,8 +2189,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
(0, payment_hash, required),
(2, keysend_preimage, required),
(4, retry_strategy, required),
(6, payment_release_secret, required),
(8, route_params, required),
(6, route_params, required),
},
);

Expand Down Expand Up @@ -2785,7 +2777,6 @@ mod tests {
payment_hash,
keysend_preimage: PaymentPreimage([0; 32]),
retry_strategy: Retry::Attempts(0),
payment_release_secret: [0; 32],
route_params,
};
outbounds.insert(payment_id, outbound);
Expand Down Expand Up @@ -2832,7 +2823,6 @@ mod tests {
payment_hash,
keysend_preimage: PaymentPreimage([0; 32]),
retry_strategy: Retry::Attempts(0),
payment_release_secret: [0; 32],
route_params,
};
outbounds.insert(payment_id, outbound);
Expand Down
19 changes: 4 additions & 15 deletions lightning/src/onion_message/async_payments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,11 @@ pub enum AsyncPaymentsMessage {
/// accompanying this onion message should be used to send a [`ReleaseHeldHtlc`] response, which
/// will cause the upstream HTLC to be released.
#[derive(Clone, Debug)]
pub struct HeldHtlcAvailable {
/// The secret that will be used by the recipient of this message to release the held HTLC.
pub payment_release_secret: [u8; 32],
}
pub struct HeldHtlcAvailable {}

/// Releases the HTLC corresponding to an inbound [`HeldHtlcAvailable`] message.
#[derive(Clone, Debug)]
pub struct ReleaseHeldHtlc {
/// Used to release the HTLC held upstream if it matches the corresponding
/// [`HeldHtlcAvailable::payment_release_secret`].
pub payment_release_secret: [u8; 32],
}
pub struct ReleaseHeldHtlc {}

impl OnionMessageContents for ReleaseHeldHtlc {
fn tlv_type(&self) -> u64 {
Expand All @@ -88,13 +81,9 @@ impl OnionMessageContents for ReleaseHeldHtlc {
}
}

impl_writeable_tlv_based!(HeldHtlcAvailable, {
(0, payment_release_secret, required),
});
impl_writeable_tlv_based!(HeldHtlcAvailable, {});

impl_writeable_tlv_based!(ReleaseHeldHtlc, {
(0, payment_release_secret, required),
});
impl_writeable_tlv_based!(ReleaseHeldHtlc, {});

impl AsyncPaymentsMessage {
/// Returns whether `tlv_type` corresponds to a TLV record for async payment messages.
Expand Down

0 comments on commit 6e27aec

Please sign in to comment.