From 9c93bd56c25976ede81ffc44bc83e8b3adffc294 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 9 Aug 2024 02:45:55 +0000 Subject: [PATCH] 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 #3227 --- fuzz/src/chanmon_consistency.rs | 5 +++-- fuzz/src/full_stack.rs | 5 +++-- fuzz/src/onion_message.rs | 5 +++-- lightning/src/ln/invoice_utils.rs | 13 ++++--------- lightning/src/sign/mod.rs | 24 +++++++++--------------- lightning/src/util/invoice.rs | 28 ---------------------------- lightning/src/util/mod.rs | 1 - lightning/src/util/test_utils.rs | 9 +++++---- 8 files changed, 27 insertions(+), 63 deletions(-) delete mode 100644 lightning/src/util/invoice.rs diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 06d2f6c84..4322fccb1 100644 --- 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 { unreachable!() } diff --git a/fuzz/src/full_stack.rs b/fuzz/src/full_stack.rs index 55a965217..fa0c29062 100644 --- 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 { unreachable!() } diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index a05551410..48f184af7 100644 --- 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 { unreachable!() } diff --git a/lightning/src/ln/invoice_utils.rs b/lightning/src/ln/invoice_utils.rs index 5e4ba508f..5a793052c 100644 --- 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 index f5fc9bb19..1dd8c90f6 100644 --- 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; /// 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 { - 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 { - 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 deleted file mode 100644 index 4a4baa452..000000000 --- a/lightning/src/util/invoice.rs +++ /dev/null @@ -1,28 +0,0 @@ -//! Low level invoice utilities. - -use bech32::{u5, FromBase32}; - -#[allow(unused)] -use crate::prelude::*; - -/// Construct the invoice's HRP and signatureless data into a preimage to be hashed. -pub fn construct_invoice_preimage(hrp_bytes: &[u8], data_without_signature: &[u5]) -> Vec { - let mut preimage = Vec::::from(hrp_bytes); - - let mut data_part = Vec::from(data_without_signature); - let overhang = (data_part.len() * 5) % 8; - if overhang > 0 { - // add padding if data does not end at a byte boundary - data_part.push(u5::try_from_u8(0).unwrap()); - - // if overhang is in (1..3) we need to add u5(0) padding two times - if overhang < 3 { - data_part.push(u5::try_from_u8(0).unwrap()); - } - } - - preimage.extend_from_slice(&Vec::::from_base32(&data_part) - .expect("No padding error may occur due to appended zero above.")); - preimage -} - diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs index cfcea8379..f19a9b8d8 100644 --- 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 index 113006cbb..f5256064e 100644 --- a/lightning/src/util/test_utils.rs +++ b/lightning/src/util/test_utils.rs @@ -65,6 +65,8 @@ 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; @@ -72,7 +74,6 @@ 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 { + fn sign_invoice(&self, _: &RawBolt11Invoice, _: Recipient) -> Result { 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 { - self.backing.sign_invoice(hrp_bytes, invoice_data, recipient) + fn sign_invoice(&self, invoice: &RawBolt11Invoice, recipient: Recipient) -> Result { + self.backing.sign_invoice(invoice, recipient) } fn sign_bolt12_invoice_request( -- 2.39.5