Skip to content

Commit c35f024

Browse files
committed
Refactor: unify invoice builder response paths (remove respond_with* split)
This refactor removes the separate `respond_with` / `respond_with_no_std` variants and replaces them with a single unified `respond_using_derived_keys(created_at)` API. Reasoning: - Upcoming recurrence logic requires setting `invoice_recurrence_basetime` based on the invoice’s `created_at` timestamp. - For consistency with Offer and Refund builders, we want a single method that accepts an explicit `created_at` value at the callsite. - The only real difference between the std/no_std response paths was how `created_at` was sourced; once it becomes a parameter, the split becomes unnecessary. This change consolidates the response flow, reduces API surface, and makes future recurrence-related changes simpler and more uniform across Offer, InvoiceRequest, and Refund builders.
1 parent b67ef21 commit c35f024

File tree

9 files changed

+98
-209
lines changed

9 files changed

+98
-209
lines changed

fuzz/src/invoice_request_deser.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
// licenses.
99

1010
use crate::utils::test_logger;
11+
use bitcoin::blockdata::constants::genesis_block;
1112
use bitcoin::secp256k1::{self, Keypair, Parity, PublicKey, Secp256k1, SecretKey};
1213
use core::convert::TryFrom;
14+
use core::time::Duration;
1315
use lightning::blinded_path::payment::{
1416
BlindedPaymentPath, Bolt12OfferContext, ForwardTlvs, PaymentConstraints, PaymentContext,
1517
PaymentForwardNode, PaymentRelay, ReceiveTlvs,
@@ -145,6 +147,8 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
145147
.unwrap();
146148

147149
let payment_hash = PaymentHash([42; 32]);
150+
let genesis_block = genesis_block(network);
151+
let now = Duration::from_secs(genesis_block.header.time as u64);
148152
invoice_request.respond_with(vec![payment_path], payment_hash)?.build()
149153
}
150154

fuzz/src/refund_deser.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@
88
// licenses.
99

1010
use crate::utils::test_logger;
11+
use bitcoin::blockdata::constants::genesis_block;
1112
use bitcoin::secp256k1::{self, Keypair, PublicKey, Secp256k1, SecretKey};
1213
use core::convert::TryFrom;
14+
use core::time::Duration;
1315
use lightning::blinded_path::payment::{
1416
BlindedPaymentPath, Bolt12RefundContext, ForwardTlvs, PaymentConstraints, PaymentContext,
1517
PaymentForwardNode, PaymentRelay, ReceiveTlvs,
@@ -109,7 +111,9 @@ fn build_response<T: secp256k1::Signing + secp256k1::Verification>(
109111
.unwrap();
110112

111113
let payment_hash = PaymentHash([42; 32]);
112-
refund.respond_with(vec![payment_path], payment_hash, signing_pubkey)?.build()
114+
let genesis_block = genesis_block(network);
115+
let now = Duration::from_secs(genesis_block.header.time as u64);
116+
refund.respond_with(vec![payment_path], payment_hash, signing_pubkey, now)?.build()
113117
}
114118

115119
pub fn refund_deser_test<Out: test_logger::Output>(data: &[u8], out: Out) {

lightning/src/ln/channelmanager.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13236,6 +13236,13 @@ where
1323613236

1323713237
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
1323813238

13239+
#[cfg(not(feature = "std"))]
13240+
let created_at = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64);
13241+
#[cfg(feature = "std")]
13242+
let created_at = std::time::SystemTime::now()
13243+
.duration_since(std::time::SystemTime::UNIX_EPOCH)
13244+
.expect("SystemTime::now() should come after SystemTime::UNIX_EPOCH");
13245+
1323913246
let builder = self.flow.create_invoice_builder_from_refund(
1324013247
&self.router,
1324113248
entropy,
@@ -13245,6 +13252,7 @@ where
1324513252
self.create_inbound_payment(Some(amount_msats), relative_expiry, None)
1324613253
.map_err(|()| Bolt12SemanticError::InvalidAmount)
1324713254
},
13255+
created_at,
1324813256
)?;
1324913257

1325013258
let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?;
@@ -15407,6 +15415,13 @@ where
1540715415
Err(_) => return None,
1540815416
};
1540915417

15418+
#[cfg(not(feature = "std"))]
15419+
let created_at = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64);
15420+
#[cfg(feature = "std")]
15421+
let created_at = std::time::SystemTime::now()
15422+
.duration_since(std::time::SystemTime::UNIX_EPOCH)
15423+
.expect("SystemTime::now() should come after SystemTime::UNIX_EPOCH");
15424+
1541015425
let get_payment_info = |amount_msats, relative_expiry| {
1541115426
self.create_inbound_payment(
1541215427
Some(amount_msats),
@@ -15422,6 +15437,7 @@ where
1542215437
&request,
1542315438
self.list_usable_channels(),
1542415439
get_payment_info,
15440+
created_at
1542515441
);
1542615442

1542715443
match result {
@@ -15446,6 +15462,7 @@ where
1544615462
&request,
1544715463
self.list_usable_channels(),
1544815464
get_payment_info,
15465+
created_at
1544915466
);
1545015467

1545115468
match result {

lightning/src/ln/offers_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2337,7 +2337,7 @@ fn fails_paying_invoice_with_unknown_required_features() {
23372337

23382338
let invoice = match verified_invoice_request {
23392339
InvoiceRequestVerifiedFromOffer::DerivedKeys(request) => {
2340-
request.respond_using_derived_keys_no_std(payment_paths, payment_hash, created_at).unwrap()
2340+
request.respond_using_derived_keys(payment_paths, payment_hash, created_at).unwrap()
23412341
.features_unchecked(Bolt12InvoiceFeatures::unknown())
23422342
.build_and_sign(&secp_ctx).unwrap()
23432343
},

lightning/src/ln/outbound_payment.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3206,7 +3206,7 @@ mod tests {
32063206
.build().unwrap()
32073207
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
32083208
.build_and_sign().unwrap()
3209-
.respond_with_no_std(payment_paths(), payment_hash(), created_at).unwrap()
3209+
.respond_with(payment_paths(), payment_hash(), created_at).unwrap()
32103210
.build().unwrap()
32113211
.sign(recipient_sign).unwrap();
32123212

@@ -3253,7 +3253,7 @@ mod tests {
32533253
.build().unwrap()
32543254
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
32553255
.build_and_sign().unwrap()
3256-
.respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap()
3256+
.respond_with(payment_paths(), payment_hash(), now()).unwrap()
32573257
.build().unwrap()
32583258
.sign(recipient_sign).unwrap();
32593259

@@ -3316,7 +3316,7 @@ mod tests {
33163316
.build().unwrap()
33173317
.request_invoice(&expanded_key, nonce, &secp_ctx, payment_id).unwrap()
33183318
.build_and_sign().unwrap()
3319-
.respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap()
3319+
.respond_with(payment_paths(), payment_hash(), now()).unwrap()
33203320
.build().unwrap()
33213321
.sign(recipient_sign).unwrap();
33223322

lightning/src/offers/flow.rs

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -932,7 +932,7 @@ where
932932
/// This is not exported to bindings users as builder patterns don't map outside of move semantics.
933933
pub fn create_invoice_builder_from_refund<'a, ES: Deref, R: Deref, F>(
934934
&'a self, router: &R, entropy_source: ES, refund: &'a Refund,
935-
usable_channels: Vec<ChannelDetails>, get_payment_info: F,
935+
usable_channels: Vec<ChannelDetails>, get_payment_info: F, created_at: Duration,
936936
) -> Result<InvoiceBuilder<'a, DerivedSigningPubkey>, Bolt12SemanticError>
937937
where
938938
ES::Target: EntropySource,
@@ -963,18 +963,7 @@ where
963963
)
964964
.map_err(|_| Bolt12SemanticError::MissingPaths)?;
965965

966-
#[cfg(feature = "std")]
967966
let builder = refund.respond_using_derived_keys(
968-
payment_paths,
969-
payment_hash,
970-
expanded_key,
971-
entropy,
972-
)?;
973-
974-
#[cfg(not(feature = "std"))]
975-
let created_at = Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64);
976-
#[cfg(not(feature = "std"))]
977-
let builder = refund.respond_using_derived_keys_no_std(
978967
payment_paths,
979968
payment_hash,
980969
created_at,
@@ -1001,7 +990,7 @@ where
1001990
/// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`].
1002991
pub fn create_invoice_builder_from_invoice_request_with_keys<'a, R: Deref, F>(
1003992
&self, router: &R, invoice_request: &'a VerifiedInvoiceRequest<DerivedSigningPubkey>,
1004-
usable_channels: Vec<ChannelDetails>, get_payment_info: F,
993+
usable_channels: Vec<ChannelDetails>, get_payment_info: F, created_at: Duration,
1005994
) -> Result<(InvoiceBuilder<'a, DerivedSigningPubkey>, MessageContext), Bolt12SemanticError>
1006995
where
1007996
R::Target: Router,
@@ -1030,15 +1019,9 @@ where
10301019
)
10311020
.map_err(|_| Bolt12SemanticError::MissingPaths)?;
10321021

1033-
#[cfg(feature = "std")]
1034-
let builder = invoice_request.respond_using_derived_keys(payment_paths, payment_hash);
1035-
#[cfg(not(feature = "std"))]
1036-
let builder = invoice_request.respond_using_derived_keys_no_std(
1037-
payment_paths,
1038-
payment_hash,
1039-
Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64),
1040-
);
1041-
let builder = builder.map(|b| InvoiceBuilder::from(b).allow_mpp())?;
1022+
let builder = invoice_request
1023+
.respond_using_derived_keys(payment_paths, payment_hash, created_at)
1024+
.map(|b| InvoiceBuilder::from(b).allow_mpp())?;
10421025

10431026
let context = MessageContext::Offers(OffersContext::InboundPayment { payment_hash });
10441027

@@ -1061,7 +1044,7 @@ where
10611044
/// - The [`InvoiceBuilder`] could not be created from the [`InvoiceRequest`].
10621045
pub fn create_invoice_builder_from_invoice_request_without_keys<'a, R: Deref, F>(
10631046
&self, router: &R, invoice_request: &'a VerifiedInvoiceRequest<ExplicitSigningPubkey>,
1064-
usable_channels: Vec<ChannelDetails>, get_payment_info: F,
1047+
usable_channels: Vec<ChannelDetails>, get_payment_info: F, created_at: Duration,
10651048
) -> Result<(InvoiceBuilder<'a, ExplicitSigningPubkey>, MessageContext), Bolt12SemanticError>
10661049
where
10671050
R::Target: Router,
@@ -1090,16 +1073,9 @@ where
10901073
)
10911074
.map_err(|_| Bolt12SemanticError::MissingPaths)?;
10921075

1093-
#[cfg(feature = "std")]
1094-
let builder = invoice_request.respond_with(payment_paths, payment_hash);
1095-
#[cfg(not(feature = "std"))]
1096-
let builder = invoice_request.respond_with_no_std(
1097-
payment_paths,
1098-
payment_hash,
1099-
Duration::from_secs(self.highest_seen_timestamp.load(Ordering::Acquire) as u64),
1100-
);
1101-
1102-
let builder = builder.map(|b| InvoiceBuilder::from(b).allow_mpp())?;
1076+
let builder = invoice_request
1077+
.respond_with(payment_paths, payment_hash, created_at)
1078+
.map(|b| InvoiceBuilder::from(b).allow_mpp())?;
11031079

11041080
let context = MessageContext::Offers(OffersContext::InboundPayment { payment_hash });
11051081

0 commit comments

Comments
 (0)