Provide the signer with a full RawBolt11Invoice to sign

Now that the `lightning` crate depends on the `lightning-invoice` crate, there's no reason to have the `sign_invoice` method take raw base32 field elements as we can now give it a real `RawBolt11Invoice`, which we do here. This simplifies the interface and avoids a serialization-deserialization roundtrip when signing invoices in a validating signer. FIxes lightningdevkit#3227
TheBlueMatt · Aug 9, 2024 · cbbad33 · cbbad33
1 parent b4b5406
commit cbbad33
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 63 deletions.
diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs
@@ -71,6 +71,8 @@ use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer};
 use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner};
 
+use lightning_invoice::RawBolt11Invoice;
+
 use crate::utils::test_logger::{self, Output};
 use crate::utils::test_persister::TestPersister;
 
@@ -79,7 +81,6 @@ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
 use bitcoin::secp256k1::schnorr;
 use bitcoin::secp256k1::{self, Message, PublicKey, Scalar, Secp256k1, SecretKey};
 
-use bech32::u5;
 use std::cmp::{self, Ordering};
 use std::io::Cursor;
 use std::mem;
@@ -332,7 +333,7 @@ impl NodeSigner for KeyProvider {
 	}
 
 	fn sign_invoice(
-		&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient,
+		&self, _invoice: &RawBolt11Invoice, _recipient: Recipient,
 	) -> Result<RecoverableSignature, ()> {
 		unreachable!()
 	}

diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs
@@ -68,6 +68,8 @@ use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, ReadableArgs, Writeable};
 use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner};
 
+use lightning_invoice::RawBolt11Invoice;
+
 use crate::utils::test_logger;
 use crate::utils::test_persister::TestPersister;
 
@@ -76,7 +78,6 @@ use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
 use bitcoin::secp256k1::schnorr;
 use bitcoin::secp256k1::{self, Message, PublicKey, Scalar, Secp256k1, SecretKey};
 
-use bech32::u5;
 use std::cell::RefCell;
 use std::cmp;
 use std::convert::TryInto;
@@ -406,7 +407,7 @@ impl NodeSigner for KeyProvider {
 	}
 
 	fn sign_invoice(
-		&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient,
+		&self, _invoice: &RawBolt11Invoice, _recipient: Recipient,
 	) -> Result<RecoverableSignature, ()> {
 		unreachable!()
 	}

diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs
@@ -1,5 +1,4 @@
 // Imports that need to be added manually
-use bech32::u5;
 use bitcoin::script::ScriptBuf;
 use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::secp256k1::ecdsa::RecoverableSignature;
@@ -27,6 +26,8 @@ use lightning::util::logger::Logger;
 use lightning::util::ser::{Readable, Writeable, Writer};
 use lightning::util::test_channel_signer::TestChannelSigner;
 
+use lightning_invoice::RawBolt11Invoice;
+
 use crate::utils::test_logger;
 
 use std::io::{self, Cursor};
@@ -225,7 +226,7 @@ impl NodeSigner for KeyProvider {
 	}
 
 	fn sign_invoice(
-		&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient,
+		&self, _invoice: &RawBolt11Invoice, _recipient: Recipient,
 	) -> Result<RecoverableSignature, ()> {
 		unreachable!()
 	}

diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs
@@ -5,7 +5,6 @@ use lightning_invoice::{Description, Bolt11InvoiceDescription, Sha256};
 
 use crate::prelude::*;
 
-use bech32::ToBase32;
 use bitcoin::hashes::Hash;
 use crate::chain;
 use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator};
@@ -219,10 +218,8 @@ where
 		Ok(inv) => inv,
 		Err(e) => return Err(SignOrCreationError::CreationError(e))
 	};
-	let hrp_str = raw_invoice.hrp.to_string();
-	let hrp_bytes = hrp_str.as_bytes();
-	let data_without_signature = raw_invoice.data.to_base32();
-	let signed_raw_invoice = raw_invoice.sign(|_| node_signer.sign_invoice(hrp_bytes, &data_without_signature, Recipient::PhantomNode));
+	let signature = node_signer.sign_invoice(&raw_invoice, Recipient::PhantomNode);
+	let signed_raw_invoice = raw_invoice.sign(|_| signature);
 	match signed_raw_invoice {
 		Ok(inv) => Ok(Bolt11Invoice::from_signed(inv).unwrap()),
 		Err(e) => Err(SignOrCreationError::SignError(e))
@@ -571,10 +568,8 @@ fn _create_invoice_from_channelmanager_and_duration_since_epoch_with_payment_has
 		Ok(inv) => inv,
 		Err(e) => return Err(SignOrCreationError::CreationError(e))
 	};
-	let hrp_str = raw_invoice.hrp.to_string();
-	let hrp_bytes = hrp_str.as_bytes();
-	let data_without_signature = raw_invoice.data.to_base32();
-	let signed_raw_invoice = raw_invoice.sign(|_| node_signer.sign_invoice(hrp_bytes, &data_without_signature, Recipient::Node));
+	let signature = node_signer.sign_invoice(&raw_invoice, Recipient::Node);
+	let signed_raw_invoice = raw_invoice.sign(|_| signature);
 	match signed_raw_invoice {
 		Ok(inv) => Ok(Bolt11Invoice::from_signed(inv).unwrap()),
 		Err(e) => Err(SignOrCreationError::SignError(e))

diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
@@ -24,7 +24,6 @@ use bitcoin::sighash::EcdsaSighashType;
 use bitcoin::transaction::Version;
 use bitcoin::transaction::{Transaction, TxIn, TxOut};
 
-use bech32::u5;
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::sha256d::Hash as Sha256dHash;
 use bitcoin::hashes::{Hash, HashEngine};
@@ -37,6 +36,8 @@ use bitcoin::secp256k1::All;
 use bitcoin::secp256k1::{Keypair, PublicKey, Scalar, Secp256k1, SecretKey, Signing};
 use bitcoin::{secp256k1, Psbt, Sequence, Txid, WPubkeyHash, Witness};
 
+use lightning_invoice::RawBolt11Invoice;
+
 use crate::chain::transaction::OutPoint;
 use crate::crypto::utils::{hkdf_extract_expand_twice, sign, sign_with_aux_rand};
 use crate::ln::chan_utils;
@@ -69,7 +70,6 @@ use crate::sign::ecdsa::EcdsaChannelSigner;
 #[cfg(taproot)]
 use crate::sign::taproot::TaprootChannelSigner;
 use crate::util::atomic_counter::AtomicCounter;
-use crate::util::invoice::construct_invoice_preimage;
 use core::convert::TryInto;
 use core::ops::Deref;
 use core::sync::atomic::{AtomicUsize, Ordering};
@@ -867,7 +867,7 @@ pub trait NodeSigner {
 	///
 	/// Errors if the [`Recipient`] variant is not supported by the implementation.
 	fn sign_invoice(
-		&self, hrp_bytes: &[u8], invoice_data: &[u5], recipient: Recipient,
+		&self, invoice: &RawBolt11Invoice, recipient: Recipient,
 	) -> Result<RecoverableSignature, ()>;
 
 	/// Signs the [`TaggedHash`] of a BOLT 12 invoice request.
@@ -2174,17 +2174,14 @@ impl NodeSigner for KeysManager {
 	}
 
 	fn sign_invoice(
-		&self, hrp_bytes: &[u8], invoice_data: &[u5], recipient: Recipient,
+		&self, invoice: &RawBolt11Invoice, recipient: Recipient,
 	) -> Result<RecoverableSignature, ()> {
-		let preimage = construct_invoice_preimage(&hrp_bytes, &invoice_data);
+		let hash = invoice.signable_hash();
 		let secret = match recipient {
 			Recipient::Node => Ok(&self.node_secret),
 			Recipient::PhantomNode => Err(()),
 		}?;
-		Ok(self.secp_ctx.sign_ecdsa_recoverable(
-			&hash_to_message!(&Sha256::hash(&preimage).to_byte_array()),
-			secret,
-		))
+		Ok(self.secp_ctx.sign_ecdsa_recoverable(&hash_to_message!(&hash), secret))
 	}
 
 	fn sign_bolt12_invoice_request(
@@ -2352,17 +2349,14 @@ impl NodeSigner for PhantomKeysManager {
 	}
 
 	fn sign_invoice(
-		&self, hrp_bytes: &[u8], invoice_data: &[u5], recipient: Recipient,
+		&self, invoice: &RawBolt11Invoice, recipient: Recipient,
 	) -> Result<RecoverableSignature, ()> {
-		let preimage = construct_invoice_preimage(&hrp_bytes, &invoice_data);
+		let hash = invoice.signable_hash();
 		let secret = match recipient {
 			Recipient::Node => &self.inner.node_secret,
 			Recipient::PhantomNode => &self.phantom_secret,
 		};
-		Ok(self.inner.secp_ctx.sign_ecdsa_recoverable(
-			&hash_to_message!(&Sha256::hash(&preimage).to_byte_array()),
-			secret,
-		))
+		Ok(self.inner.secp_ctx.sign_ecdsa_recoverable(&hash_to_message!(&hash), secret))
 	}
 
 	fn sign_bolt12_invoice_request(

diff --git a/lightning/src/util/invoice.rs b/lightning/src/util/invoice.rs
diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs
@@ -18,7 +18,6 @@ pub mod ser_macros;
 pub mod errors;
 pub mod ser;
 pub mod message_signing;
-pub mod invoice;
 pub mod persist;
 pub mod scid_utils;
 pub mod sweep;

diff --git a/lightning/src/util/test_utils.rs b/lightning/src/util/test_utils.rs
@@ -65,14 +65,15 @@ use bitcoin::secp256k1::ecdh::SharedSecret;
 use bitcoin::secp256k1::ecdsa::{RecoverableSignature, Signature};
 use bitcoin::secp256k1::schnorr;
 
+use lightning_invoice::RawBolt11Invoice;
+
 use crate::io;
 use crate::prelude::*;
 use core::cell::RefCell;
 use core::time::Duration;
 use crate::sync::{Mutex, Arc};
 use core::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
 use core::mem;
-use bech32::u5;
 use crate::sign::{InMemorySigner, RandomBytes, Recipient, EntropySource, NodeSigner, SignerProvider};
 
 #[cfg(feature = "std")]
@@ -1217,7 +1218,7 @@ impl NodeSigner for TestNodeSigner {
 		Ok(SharedSecret::new(other_key, &node_secret))
 	}
 
-	fn sign_invoice(&self, _: &[u8], _: &[bech32::u5], _: Recipient) -> Result<bitcoin::secp256k1::ecdsa::RecoverableSignature, ()> {
+	fn sign_invoice(&self, _: &RawBolt11Invoice, _: Recipient) -> Result<RecoverableSignature, ()> {
 		unreachable!()
 	}
 
@@ -1270,8 +1271,8 @@ impl NodeSigner for TestKeysInterface {
 		self.backing.get_inbound_payment_key_material()
 	}
 
-	fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5], recipient: Recipient) -> Result<RecoverableSignature, ()> {
-		self.backing.sign_invoice(hrp_bytes, invoice_data, recipient)
+	fn sign_invoice(&self, invoice: &RawBolt11Invoice, recipient: Recipient) -> Result<RecoverableSignature, ()> {
+		self.backing.sign_invoice(invoice, recipient)
 	}
 
 	fn sign_bolt12_invoice_request(