refactor(invoice): align signature checks with BOLT11 semantics
What the developer wrote
Authored by Erick Cestari
refactor(invoice): align signature checks with BOLT11 semantics
Simplify `check_signature` logic to follow the BOLT11 rules discussed:
- If an `n` field is present, verify the signature against the included
pubkey using `secp256k1_ecdsa_verify`, which enforces normalized
low-S form.
- If no `n` field is present, rely on `secp256k1_ecdsa_recover` to
extract the pubkey. Recovery accepts both high-S and low-S signatures,
matching existing implementations (lnd, c-lightning).
This avoids redundant recovery+verify checks while preserving
interoperability.
What changed, and why it matters
This commit changes how Bitcoin Lightning invoices (BOLT11) are validated. Previously, the code always tried to recover the signer's public key from the signature and also verified it, which could reject some signatures that other Lightning implementations accept. Now, if the invoice explicitly includes a public key, it only verifies the signature against that key (and requires a strict low-S signature). If no public key is included, it recovers the public key from the signature, accepting both high-S and low-S signatures like lnd and c-lightning. This is described as an interoperability fix, not a security vulnerability, but it slightly weakens signature strictness in the no-pubkey case.
Review whether accepting high-S signatures in the recovery-only path is acceptable for your security model and BOLT11 compliance. Monitor for any related vulnerability reports or follow-up commits that add tests or further harden signature handling. No immediate patch action is indicated by the commit itself.
Security signals we found
Signature verification logic changed from verify+recover to branch on presence of n field
Recovery-only path now accepts high-S signatures, matching lnd/c-lightning behavior
Verification path with explicit pubkey enforces low-S normalization via to_standard()
Removal of InvalidRecoveryId error handling reduces strictness
No explicit security bug or CVE referenced in commit message
Evidence from the diff
The patch refactors check_signature in lightning-invoice/src/lib.rs. The old path always called recover_payee_pub_key() and then verify_ecdsa. The new path branches: if payee_pub_key() (n field) is present, it verifies via secp256k1_ecdsa_verify using to_standard() (low-S normalized); if absent, it only checks that recovery succeeds. It removes InvalidRecoveryId error variants and the redundant recovery-then-verify flow. The stated goal is BOLT11 semantic alignment and interoperability with lnd/c-lightning, which accept high-S signatures when recovering.
Changed components
lightning-invoice/src/lib.rslightning-invoice/src/de.rsSignedRawBolt11Invoice::check_signatureBolt11Invoice::check_signatureBOLT11 invoice parsing and signature validationInspect captured patch +11 / −42
diff --git a/lightning-invoice/src/de.rs b/lightning-invoice/src/de.rs
index fd3c4ad..a4e3cf7 100644
--- a/lightning-invoice/src/de.rs
+++ b/lightning-invoice/src/de.rs
@@ -772,9 +772,6 @@ impl Display for Bolt11ParseError {
Bolt11ParseError::InvalidScriptHashLength => {
f.write_str("fallback script hash has a length unequal 32 bytes")
},
- Bolt11ParseError::InvalidRecoveryId => {
- f.write_str("recovery id is out of range (should be in [0,3])")
- },
Bolt11ParseError::Skip => f.write_str(
"the tagged field has to be skipped because of an unexpected, but allowed property",
),
diff --git a/lightning-invoice/src/lib.rs b/lightning-invoice/src/lib.rs
index fda7c10..ecffcf8 100644
--- a/lightning-invoice/src/lib.rs
+++ b/lightning-invoice/src/lib.rs
@@ -106,7 +106,6 @@ pub enum Bolt11ParseError {
InvalidSegWitProgramLength,
InvalidPubKeyHashLength,
InvalidScriptHashLength,
- InvalidRecoveryId,
// Invalid length, with actual length, expected length, and name of the element
InvalidSliceLength(usize, usize, &'static str),
@@ -1011,31 +1010,19 @@ impl SignedRawBolt11Invoice {
}
/// Checks if the signature is valid for the included payee public key or if none exists if it's
- /// valid for the recovered signature (which should always be true?).
+ /// possible to recover the public key from the signature.
pub fn check_signature(&self) -> bool {
- let included_pub_key = self.raw_invoice.payee_pub_key();
+ match self.raw_invoice.payee_pub_key() {
+ Some(pk) => {
+ let hash = Message::from_digest(self.hash);
- let mut recovered_pub_key = Option::None;
- if recovered_pub_key.is_none() {
- let recovered = match self.recover_payee_pub_key() {
- Ok(pk) => pk,
- Err(_) => return false,
- };
- recovered_pub_key = Some(recovered);
- }
+ let secp_context = Secp256k1::new();
+ let verification_result =
+ secp_context.verify_ecdsa(&hash, &self.signature.to_standard(), pk);
- let pub_key =
- included_pub_key.or(recovered_pub_key.as_ref()).expect("One is always present");
-
- let hash = Message::from_digest(self.hash);
-
- let secp_context = Secp256k1::new();
- let verification_result =
- secp_context.verify_ecdsa(&hash, &self.signature.to_standard(), pub_key);
-
- match verification_result {
- Ok(()) => true,
- Err(_) => false,
+ verification_result.is_ok()
+ },
+ None => self.recover_payee_pub_key().is_ok(),
}
}
}
@@ -1410,19 +1397,8 @@ impl Bolt11Invoice {
}
}
- /// Check that the invoice is signed correctly and that key recovery works
+ /// Check that the invoice is signed correctly
pub fn check_signature(&self) -> Result<(), Bolt11SemanticError> {
- match self.signed_invoice.recover_payee_pub_key() {
- Err(bitcoin::secp256k1::Error::InvalidRecoveryId) => {
- return Err(Bolt11SemanticError::InvalidRecoveryId)
- },
- Err(bitcoin::secp256k1::Error::InvalidSignature) => {
- return Err(Bolt11SemanticError::InvalidSignature)
- },
- Err(e) => panic!("no other error may occur, got {:?}", e),
- Ok(_) => {},
- }
-
if !self.signed_invoice.check_signature() {
return Err(Bolt11SemanticError::InvalidSignature);
}
@@ -1873,9 +1849,6 @@ pub enum Bolt11SemanticError {
/// The invoice's features are invalid
InvalidFeatures,
- /// The recovery id doesn't fit the signature/pub key
- InvalidRecoveryId,
-
/// The invoice's signature is invalid
InvalidSignature,
@@ -1893,7 +1866,6 @@ impl Display for Bolt11SemanticError {
Bolt11SemanticError::NoPaymentSecret => f.write_str("The invoice is missing the mandatory payment secret"),
Bolt11SemanticError::MultiplePaymentSecrets => f.write_str("The invoice contains multiple payment secrets"),
Bolt11SemanticError::InvalidFeatures => f.write_str("The invoice's features are invalid"),
- Bolt11SemanticError::InvalidRecoveryId => f.write_str("The recovery id doesn't fit the signature/pub key"),
Bolt11SemanticError::InvalidSignature => f.write_str("The invoice's signature is invalid"),
Bolt11SemanticError::ImpreciseAmount => f.write_str("The invoice's amount was not a whole number of millisatoshis"),
}
Why this scored 35/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.