Skip to content

Commit f63aebb

Browse files
committed
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.
1 parent ecce859 commit f63aebb

File tree

2 files changed

+11
-42
lines changed

2 files changed

+11
-42
lines changed

lightning-invoice/src/de.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -772,9 +772,6 @@ impl Display for Bolt11ParseError {
772772
Bolt11ParseError::InvalidScriptHashLength => {
773773
f.write_str("fallback script hash has a length unequal 32 bytes")
774774
},
775-
Bolt11ParseError::InvalidRecoveryId => {
776-
f.write_str("recovery id is out of range (should be in [0,3])")
777-
},
778775
Bolt11ParseError::Skip => f.write_str(
779776
"the tagged field has to be skipped because of an unexpected, but allowed property",
780777
),

lightning-invoice/src/lib.rs

Lines changed: 11 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ pub enum Bolt11ParseError {
106106
InvalidSegWitProgramLength,
107107
InvalidPubKeyHashLength,
108108
InvalidScriptHashLength,
109-
InvalidRecoveryId,
110109
// Invalid length, with actual length, expected length, and name of the element
111110
InvalidSliceLength(usize, usize, &'static str),
112111

@@ -1011,31 +1010,19 @@ impl SignedRawBolt11Invoice {
10111010
}
10121011

10131012
/// Checks if the signature is valid for the included payee public key or if none exists if it's
1014-
/// valid for the recovered signature (which should always be true?).
1013+
/// possible to recover the public key from the signature.
10151014
pub fn check_signature(&self) -> bool {
1016-
let included_pub_key = self.raw_invoice.payee_pub_key();
1015+
match self.raw_invoice.payee_pub_key() {
1016+
Some(pk) => {
1017+
let hash = Message::from_digest(self.hash);
10171018

1018-
let mut recovered_pub_key = Option::None;
1019-
if recovered_pub_key.is_none() {
1020-
let recovered = match self.recover_payee_pub_key() {
1021-
Ok(pk) => pk,
1022-
Err(_) => return false,
1023-
};
1024-
recovered_pub_key = Some(recovered);
1025-
}
1019+
let secp_context = Secp256k1::new();
1020+
let verification_result =
1021+
secp_context.verify_ecdsa(&hash, &self.signature.to_standard(), pk);
10261022

1027-
let pub_key =
1028-
included_pub_key.or(recovered_pub_key.as_ref()).expect("One is always present");
1029-
1030-
let hash = Message::from_digest(self.hash);
1031-
1032-
let secp_context = Secp256k1::new();
1033-
let verification_result =
1034-
secp_context.verify_ecdsa(&hash, &self.signature.to_standard(), pub_key);
1035-
1036-
match verification_result {
1037-
Ok(()) => true,
1038-
Err(_) => false,
1023+
verification_result.is_ok()
1024+
},
1025+
None => self.recover_payee_pub_key().is_ok(),
10391026
}
10401027
}
10411028
}
@@ -1410,19 +1397,8 @@ impl Bolt11Invoice {
14101397
}
14111398
}
14121399

1413-
/// Check that the invoice is signed correctly and that key recovery works
1400+
/// Check that the invoice is signed correctly
14141401
pub fn check_signature(&self) -> Result<(), Bolt11SemanticError> {
1415-
match self.signed_invoice.recover_payee_pub_key() {
1416-
Err(bitcoin::secp256k1::Error::InvalidRecoveryId) => {
1417-
return Err(Bolt11SemanticError::InvalidRecoveryId)
1418-
},
1419-
Err(bitcoin::secp256k1::Error::InvalidSignature) => {
1420-
return Err(Bolt11SemanticError::InvalidSignature)
1421-
},
1422-
Err(e) => panic!("no other error may occur, got {:?}", e),
1423-
Ok(_) => {},
1424-
}
1425-
14261402
if !self.signed_invoice.check_signature() {
14271403
return Err(Bolt11SemanticError::InvalidSignature);
14281404
}
@@ -1873,9 +1849,6 @@ pub enum Bolt11SemanticError {
18731849
/// The invoice's features are invalid
18741850
InvalidFeatures,
18751851

1876-
/// The recovery id doesn't fit the signature/pub key
1877-
InvalidRecoveryId,
1878-
18791852
/// The invoice's signature is invalid
18801853
InvalidSignature,
18811854

@@ -1893,7 +1866,6 @@ impl Display for Bolt11SemanticError {
18931866
Bolt11SemanticError::NoPaymentSecret => f.write_str("The invoice is missing the mandatory payment secret"),
18941867
Bolt11SemanticError::MultiplePaymentSecrets => f.write_str("The invoice contains multiple payment secrets"),
18951868
Bolt11SemanticError::InvalidFeatures => f.write_str("The invoice's features are invalid"),
1896-
Bolt11SemanticError::InvalidRecoveryId => f.write_str("The recovery id doesn't fit the signature/pub key"),
18971869
Bolt11SemanticError::InvalidSignature => f.write_str("The invoice's signature is invalid"),
18981870
Bolt11SemanticError::ImpreciseAmount => f.write_str("The invoice's amount was not a whole number of millisatoshis"),
18991871
}

0 commit comments

Comments
 (0)