Return optional recovered BOLT11 payee keys
What changed, and why it matters
This commit changes how a Bitcoin Lightning invoice library exposes the payee's public key. Previously, a function called recover_payee_pub_key always returned a public key, even when mathematically recovering it from the invoice signature actually failed. Now it returns an optional value (None when recovery fails), and callers are directed to a safer accessor. The change prevents callers from silently getting a wrong or misleading key, which could matter for payment security decisions.
Review downstream callers of recover_payee_pub_key to ensure they handle the new Option<PublicKey> return type and do not rely on the previous always-succeeding behavior. Prefer get_payee_pub_key for obtaining the invoice's intended payee key.
Security signals we found
API semantics changed from non-optional to optional return for a cryptographic recovery operation
Previous behavior could silently return an explicitly included key instead of a recovered key
New behavior surfaces recovery failure rather than masking it
Documentation now directs callers to canonical accessor get_payee_pub_key
Test demonstrates recovery failure case with a tampered recovery ID
Evidence from the diff
In rust-lightning’s lightning-invoice crate, recover_payee_pub_key() previously delegated to get_payee_pub_key(), which would fall back to the explicitly included n-field payee public key when signature recovery failed. The patch makes recover_payee_pub_key() return Option
Changed components
lightning-invoice/src/lib.rsBolt11Invoice::recover_payee_pub_keyBolt11Invoice::get_payee_pub_keyBolt11Invoice::payee_pub_keyInspect captured patch +23 / −13
diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs
index e6150cd..3826adc 100644
--- a/lightning-invoice/src/lib.rs
+++ b/lightning-invoice/src/lib.rs
@@ -1478,7 +1478,7 @@ impl Bolt11Invoice {
unreachable!("ensured by constructor");
}
- /// Get the payee's public key if one was included in the invoice
+ /// Get the payee's public key if one was explicitly included in the invoice's `n` field.
pub fn payee_pub_key(&self) -> Option<&PublicKey> {
self.signed_invoice.payee_pub_key().map(|x| &x.0)
}
@@ -1498,12 +1498,13 @@ impl Bolt11Invoice {
self.signed_invoice.features()
}
- /// Get the invoice's payee public key.
+ /// Recover the payee's public key from the invoice signature.
///
- /// This uses the explicitly included payee public key, if present, otherwise it recovers the
- /// payee public key from the signature. Prefer [`Self::get_payee_pub_key`] for clarity.
- pub fn recover_payee_pub_key(&self) -> PublicKey {
- self.get_payee_pub_key()
+ /// This attempts signature recovery regardless of whether a payee public key was explicitly
+ /// included in the invoice's `n` field. Recovery can fail for a valid invoice with an included
+ /// `n` field, so [`Self::get_payee_pub_key`] should be used to obtain the invoice's payee key.
+ pub fn recover_payee_pub_key(&self) -> Option<PublicKey> {
+ self.signed_invoice.recover_payee_pub_key().ok().map(|p| p.0)
}
/// Get the invoice's payee public key, preferring an explicitly included payee public key and
@@ -1511,9 +1512,7 @@ impl Bolt11Invoice {
pub fn get_payee_pub_key(&self) -> PublicKey {
match self.payee_pub_key() {
Some(pk) => *pk,
- None => {
- self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
- },
+ None => self.recover_payee_pub_key().expect("was checked by constructor"),
}
}
@@ -2063,7 +2062,7 @@ mod test {
}
#[test]
- fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ fn recover_payee_pub_key_returns_signature_recovery_result() {
use crate::{
Bolt11Invoice, Bolt11InvoiceSignature, Currency, InvoiceBuilder, PaymentHash,
PaymentSecret, SignedRawBolt11Invoice,
@@ -2076,17 +2075,28 @@ mod test {
let private_key = SecretKey::from_slice(&[42; 32]).unwrap();
let public_key = PublicKey::from_secret_key(&secp_ctx, &private_key);
- let invoice = InvoiceBuilder::new(Currency::Bitcoin)
+ let invoice_without_payee_pub_key = InvoiceBuilder::new(Currency::Bitcoin)
.description("Test".to_string())
.payment_hash(PaymentHash([0; 32]))
.payment_secret(PaymentSecret([21; 32]))
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+ assert_eq!(invoice_without_payee_pub_key.recover_payee_pub_key(), Some(public_key));
+ assert_eq!(invoice_without_payee_pub_key.get_payee_pub_key(), public_key);
+
+ let invoice_with_payee_pub_key = InvoiceBuilder::new(Currency::Bitcoin)
+ .description("Test".to_string())
+ .payment_hash(PaymentHash([1; 32]))
+ .payment_secret(PaymentSecret([21; 32]))
.payee_pub_key(public_key)
.min_final_cltv_expiry_delta(144)
.duration_since_epoch(Duration::from_secs(1234567))
.build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
.unwrap();
- let signed_raw = invoice.into_signed_raw();
+ let signed_raw = invoice_with_payee_pub_key.into_signed_raw();
let (raw_invoice, hash, signature) = signed_raw.into_parts();
let (_orig_rid, sig_bytes) = signature.0.serialize_compact();
let bad_rid = RecoveryId::from_i32(2).unwrap();
@@ -2099,7 +2109,7 @@ mod test {
let bad_invoice = Bolt11Invoice::from_signed(bad_signed_raw).unwrap();
assert_eq!(bad_invoice.payee_pub_key(), Some(&public_key));
- assert_eq!(bad_invoice.recover_payee_pub_key(), public_key);
+ assert_eq!(bad_invoice.recover_payee_pub_key(), None);
assert_eq!(bad_invoice.get_payee_pub_key(), public_key);
}
Why this scored 47/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.