Use BOLT11 invoice payee keys for payment params
What changed, and why it matters
This commit fixes a bug in how Lightning payment parameters are built from BOLT11 invoices. Previously, the code always tried to recover the payee's public key from the invoice signature, even when the invoice already explicitly included the payee's key. Signature recovery can fail or produce the wrong key in some legitimate cases, such as when the recovery ID byte is altered. The fix makes payment routing use the explicitly included payee key when available, falling back to signature recovery only when needed. This prevents potential payment routing failures or misrouting caused by relying on recoverable signatures.
Review and merge the patch. After deployment, ensure that any code paths constructing PaymentParameters from BOLT11 invoices use the updated method. Consider auditing other uses of recover_payee_pub_key across the codebase to confirm they should still perform recovery rather than preferring explicit keys.
Security signals we found
BOLT11 invoice payee public key derivation changed from signature recovery to explicit key preference
PaymentParameters::from_bolt11_invoice now uses get_payee_pub_key instead of recover_payee_pub_key
Test cases demonstrate handling of invoices with valid included payee key but invalid recovery ID
Commit message notes signature recovery may legitimately be unavailable
Finding attributed to external discovery by Project Loupe
Evidence from the diff
The change updates PaymentParameters::from_bolt11_invoice and related test helpers to call invoice.get_payee_pub_key() instead of invoice.recover_payee_pub_key(). The recover_payee_pub_key method is now a thin wrapper that delegates to get_payee_pub_key, which prefers an explicitly included payee_pub_key (the n tagged field in BOLT11) and only falls back to ECDSA signature recovery when absent. The commit adds tests that construct invoices with a valid included payee key but an invalid signature recovery ID, proving that payment parameters still resolve to the correct payee public key. This addresses a correctness issue where signature recovery could fail or yield an incorrect public key despite a valid invoice.
Changed components
lightning-invoice/src/lib.rslightning/src/ln/invoice_utils.rslightning/src/routing/router.rsBolt11Invoice payee public key resolutionPaymentParameters construction from BOLT11 invoicesInspect captured patch +105 / −9
diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs
index 6c18e60..e6150cd 100644
--- a/lightning-invoice/src/lib.rs
+++ b/lightning-invoice/src/lib.rs
@@ -1498,17 +1498,22 @@ impl Bolt11Invoice {
self.signed_invoice.features()
}
- /// Recover the payee's public key (only to be used if none was included in the invoice)
+ /// Get the invoice's payee public key.
+ ///
+ /// 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.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ self.get_payee_pub_key()
}
- /// Recover the payee's public key if one was included in the invoice, otherwise return the
- /// recovered public key from the signature
+ /// Get the invoice's payee public key, preferring an explicitly included payee public key and
+ /// falling back to recovering the key from the signature.
pub fn get_payee_pub_key(&self) -> PublicKey {
match self.payee_pub_key() {
Some(pk) => *pk,
- None => self.recover_payee_pub_key(),
+ None => {
+ self.signed_invoice.recover_payee_pub_key().expect("was checked by constructor").0
+ },
}
}
@@ -2057,6 +2062,47 @@ mod test {
assert!(new_signed.check_signature());
}
+ #[test]
+ fn recover_payee_pub_key_uses_included_payee_pub_key() {
+ use crate::{
+ Bolt11Invoice, Bolt11InvoiceSignature, Currency, InvoiceBuilder, PaymentHash,
+ PaymentSecret, SignedRawBolt11Invoice,
+ };
+ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+ use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
+ use core::time::Duration;
+
+ let secp_ctx = Secp256k1::new();
+ 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)
+ .description("Test".to_string())
+ .payment_hash(PaymentHash([0; 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 (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();
+ let bad_sig = RecoverableSignature::from_compact(&sig_bytes, bad_rid).unwrap();
+ let bad_signed_raw = SignedRawBolt11Invoice {
+ raw_invoice,
+ hash,
+ signature: Bolt11InvoiceSignature(bad_sig),
+ };
+ 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.get_payee_pub_key(), public_key);
+ }
+
#[test]
fn test_check_feature_bits() {
use crate::TaggedField::*;
diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs
index 98996fa..10cda06 100644
--- a/lightning/src/ln/invoice_utils.rs
+++ b/lightning/src/ln/invoice_utils.rs
@@ -1281,7 +1281,7 @@ mod test {
assert!(!invoice.features().unwrap().supports_basic_mpp());
let payment_params = PaymentParameters::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
invoice.min_final_cltv_expiry_delta() as u32,
)
.with_bolt11_features(invoice.features().unwrap().clone())
@@ -1347,7 +1347,7 @@ mod test {
payment_secret,
payment_amt,
payment_preimage_opt,
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
);
do_claim_payment_along_route(ClaimAlongRouteArgs::new(
&nodes[0],
diff --git a/lightning/src/routing/router.rs b/lightning/src/routing/router.rs
index 364bd86..99d9623 100644
--- a/lightning/src/routing/router.rs
+++ b/lightning/src/routing/router.rs
@@ -1177,7 +1177,7 @@ impl PaymentParameters {
/// [`PaymentParameters::expiry_time`].
pub fn from_bolt11_invoice(invoice: &Bolt11Invoice) -> Self {
let mut payment_params = Self::from_node_id(
- invoice.recover_payee_pub_key(),
+ invoice.get_payee_pub_key(),
invoice.min_final_cltv_expiry_delta() as u32,
)
.with_route_hints(invoice.route_hints())
@@ -4094,7 +4094,7 @@ mod tests {
use crate::routing::gossip::{EffectiveCapacity, NetworkGraph, NodeId, P2PGossipSync};
use crate::routing::router::{
add_random_cltv_offset, build_route_from_hops_internal, default_node_features, get_route,
- BlindedPathCandidate, BlindedTail, CandidateRouteHop, InFlightHtlcs, Path,
+ BlindedPathCandidate, BlindedTail, CandidateRouteHop, InFlightHtlcs, Path, Payee,
PaymentParameters, PublicHopCandidate, Route, RouteHint, RouteHintHop, RouteHop,
RouteParameters, RoutingFees, ScorerAccountingForInFlightHtlcs,
DEFAULT_MAX_TOTAL_CLTV_EXPIRY_DELTA, MAX_PATH_LENGTH_ESTIMATE,
@@ -4113,6 +4113,8 @@ mod tests {
use crate::util::test_utils as ln_test_utils;
use bitcoin::amount::Amount;
+ use bitcoin::bech32::primitives::decode::CheckedHrpstring;
+ use bitcoin::bech32::{ByteIterExt, Fe32IterExt};
use bitcoin::constants::ChainHash;
use bitcoin::hashes::Hash;
use bitcoin::hex::FromHex;
@@ -4124,10 +4126,58 @@ mod tests {
use bitcoin::transaction::TxOut;
use chacha20_poly1305::chacha20::ChaCha20;
use chacha20_poly1305::{Key, Nonce};
+ use lightning_invoice::{Bolt11Bech32, Bolt11Invoice, Currency, InvoiceBuilder};
use crate::io::Cursor;
use crate::prelude::*;
use crate::sync::{Arc, Mutex};
+ use crate::types::payment::{PaymentHash, PaymentSecret};
+
+ fn invoice_with_included_payee_pub_key_and_bad_recovery_id() -> (Bolt11Invoice, PublicKey) {
+ let secp_ctx = Secp256k1::new();
+ 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)
+ .description("Test".to_string())
+ .amount_milli_satoshis(1000)
+ .payment_hash(PaymentHash([0; 32]))
+ .payment_secret(PaymentSecret([21; 32]))
+ .payee_pub_key(public_key)
+ .min_final_cltv_expiry_delta(144)
+ .duration_since_epoch(core::time::Duration::from_secs(1234567))
+ .build_signed(|hash| secp_ctx.sign_ecdsa_recoverable(hash, &private_key))
+ .unwrap();
+
+ let invoice_string = invoice.to_string();
+ let parsed = CheckedHrpstring::new::<Bolt11Bech32>(&invoice_string).unwrap();
+ let hrp = parsed.hrp();
+ let mut data: Vec<_> = parsed.fe32_iter::<&mut dyn Iterator<Item = u8>>().collect();
+ let signature_start = data.len() - 104;
+ let mut signature_bytes: Vec<u8> =
+ data[signature_start..].iter().copied().fes_to_bytes().collect();
+ signature_bytes[64] = 2;
+ let signature_data: Vec<_> = signature_bytes.into_iter().bytes_to_fes().collect();
+ data.splice(signature_start.., signature_data);
+
+ let bad_invoice_string = data
+ .into_iter()
+ .with_checksum::<bitcoin::bech32::Bech32>(&hrp)
+ .chars()
+ .collect::<String>();
+ (bad_invoice_string.parse().unwrap(), public_key)
+ }
+
+ #[test]
+ fn payment_params_from_bolt11_invoice_uses_included_payee_pub_key() {
+ let (invoice, public_key) = invoice_with_included_payee_pub_key_and_bad_recovery_id();
+ let payment_params = PaymentParameters::from_bolt11_invoice(&invoice);
+
+ match payment_params.payee {
+ Payee::Clear { node_id, .. } => assert_eq!(node_id, public_key),
+ Payee::Blinded { .. } => panic!("BOLT11 invoice should create a clear payee"),
+ }
+ }
#[rustfmt::skip]
fn get_channel_details(short_channel_id: Option<u64>, node_id: PublicKey,
Why this scored 63/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.