From 75b87723fa08acd239eee39e47f8d558df1d1b4b Mon Sep 17 00:00:00 2001 From: Orbital Date: Wed, 24 Jan 2024 15:00:23 -0600 Subject: [PATCH] multi: split off uir signing portion of create_invoice_request into a method We split the unsigned invoice request (uir) signing portion into another function to make it easier to mock. Earlier we had hardcoded a signature for our lndk_offers.rs unit tests. But now that we've changed the uir being signed (by adding verification metadata), the hardcoded signature is invalid. We refactor our mocking to get rid of this hardcoded signature data completely. --- src/lib.rs | 2 +- src/lnd.rs | 8 +++- src/lndk_offers.rs | 116 +++++++++++++++++++++++++++------------------ 3 files changed, 78 insertions(+), 48 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f91aa684..5fbe05f9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -183,7 +183,7 @@ enum OfferState { pub struct OfferHandler { active_offers: Mutex>, pending_messages: Mutex>>, - messenger_utils: MessengerUtilities, + pub messenger_utils: MessengerUtilities, expanded_key: ExpandedKey, } diff --git a/src/lnd.rs b/src/lnd.rs index 1eb6161b..5c198cae 100644 --- a/src/lnd.rs +++ b/src/lnd.rs @@ -1,3 +1,4 @@ +use crate::OfferError; use async_trait::async_trait; use bitcoin::bech32::u5; use bitcoin::hashes::sha256::Hash; @@ -8,7 +9,7 @@ use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1}; use futures::executor::block_on; use lightning::ln::msgs::UnsignedGossipMessage; use lightning::offers::invoice::UnsignedBolt12Invoice; -use lightning::offers::invoice_request::UnsignedInvoiceRequest; +use lightning::offers::invoice_request::{InvoiceRequest, UnsignedInvoiceRequest}; use lightning::sign::{KeyMaterial, NodeSigner, Recipient}; use std::cell::RefCell; use std::collections::HashMap; @@ -195,6 +196,11 @@ pub trait MessageSigner { merkle_hash: Hash, tag: String, ) -> Result, Status>; + fn sign_uir( + &mut self, + key_loc: KeyLocator, + unsigned_invoice_req: UnsignedInvoiceRequest, + ) -> Result>; } /// PeerConnector provides a layer of abstraction over the LND API for connecting to a peer. diff --git a/src/lndk_offers.rs b/src/lndk_offers.rs index aba2c6d1..59bde552 100644 --- a/src/lndk_offers.rs +++ b/src/lndk_offers.rs @@ -190,24 +190,9 @@ impl OfferHandler { // To create a valid invoice request, we also need to sign it. This is spawned in a blocking // task because we need to call block_on on sign_message so that sign_closure can be a // synchronous closure. - task::spawn_blocking(move || { - let sign_closure = |msg: &UnsignedInvoiceRequest| { - let tagged_hash = msg.as_ref(); - let tag = tagged_hash.tag().to_string(); - - let signature = - block_on(signer.sign_message(key_loc, tagged_hash.merkle_root(), tag)) - .map_err(|_| Secp256k1Error::InvalidSignature)?; - - Signature::from_slice(&signature) - }; - - unsigned_invoice_req - .sign(sign_closure) - .map_err(OfferError::SignError) - }) - .await - .unwrap() + task::spawn_blocking(move || signer.sign_uir(key_loc, unsigned_invoice_req)) + .await + .unwrap() } /// create_reply_path creates a blinded path to provide to the offer maker when requesting an @@ -462,12 +447,34 @@ impl MessageSigner for Client { let resp_inner = resp.into_inner(); Ok(resp_inner.signature) } + + fn sign_uir( + &mut self, + key_loc: KeyLocator, + unsigned_invoice_req: UnsignedInvoiceRequest, + ) -> Result> { + let sign_closure = |msg: &UnsignedInvoiceRequest| { + let tagged_hash = msg.as_ref(); + let tag = tagged_hash.tag().to_string(); + + let signature = block_on(self.sign_message(key_loc, tagged_hash.merkle_root(), tag)) + .map_err(|_| Secp256k1Error::InvalidSignature)?; + + Signature::from_slice(&signature) + }; + + unsigned_invoice_req + .sign(sign_closure) + .map_err(OfferError::SignError) + } } #[cfg(test)] mod tests { use super::*; - use bitcoin::secp256k1::{KeyPair, Secp256k1, SecretKey}; + use bitcoin::secp256k1::{Error as Secp256k1Error, KeyPair, Secp256k1, SecretKey}; + use core::convert::Infallible; + use lightning::offers::merkle::SignError; use lightning::offers::offer::{OfferBuilder, Quantity}; use mockall::mock; use std::collections::HashMap; @@ -498,8 +505,23 @@ mod tests { "0313ba7ccbd754c117962b9afab6c2870eb3ef43f364a9f6c43d0fabb4553776ba".to_string() } - fn get_signature() -> String { - "28b937976a29c15827433086440b36c2bec6ca5bd977557972dca8641cd59ffba50daafb8ee99a19c950976b46f47d9e7aa716652e5657dfc555b82eff467f18".to_string() + fn get_invoice_request(offer: Offer, amount: u64) -> InvoiceRequest { + let secp_ctx = Secp256k1::new(); + let keys = KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); + let pubkey = PublicKey::from(keys); + offer + .request_invoice(vec![42; 64], pubkey) + .unwrap() + .chain(Network::Regtest) + .unwrap() + .amount_msats(amount) + .unwrap() + .build() + .unwrap() + .sign::<_, Infallible>(|message| { + Ok(secp_ctx.sign_schnorr_no_aux_rand(message.as_ref().as_digest(), &keys)) + }) + .expect("failed verifying signature") } mock! { @@ -509,6 +531,7 @@ mod tests { impl MessageSigner for TestBolt12Signer { async fn derive_key(&mut self, key_loc: KeyLocator) -> Result, Status>; async fn sign_message(&mut self, key_loc: KeyLocator, merkle_hash: Hash, tag: String) -> Result, Status>; + fn sign_uir(&mut self, key_loc: KeyLocator, unsigned_invoice_req: UnsignedInvoiceRequest) -> Result>; } } @@ -528,25 +551,27 @@ mod tests { let mut signer_mock = MockTestBolt12Signer::new(); signer_mock.expect_derive_key().returning(|_| { - Ok(PublicKey::from_str(&get_pubkey()) - .unwrap() - .serialize() - .to_vec()) + let pubkey = PublicKey::from_str(&get_pubkey()).unwrap(); + Ok(pubkey.serialize().to_vec()) }); - signer_mock.expect_sign_message().returning(|_, _, _| { - Ok(Signature::from_str(&get_signature()) - .unwrap() - .as_ref() - .to_vec()) - }); + let offer = decode(get_offer()).unwrap(); + let offer_amount = offer.amount().unwrap(); + let amount = match offer_amount { + Amount::Bitcoin { amount_msats } => *amount_msats, + _ => panic!("unexpected amount type"), + }; + + signer_mock + .expect_sign_uir() + .returning(move |_, _| Ok(get_invoice_request(offer.clone(), amount))); let offer = decode(get_offer()).unwrap(); let handler = OfferHandler::new(); - assert!(handler - .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000) - .await - .is_ok()) + let resp = handler + .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, amount) + .await; + assert!(resp.is_ok()) } #[tokio::test] @@ -557,17 +582,14 @@ mod tests { .expect_derive_key() .returning(|_| Err(Status::unknown("error testing"))); - signer_mock.expect_sign_message().returning(|_, _, _| { - Ok(Signature::from_str(&get_signature()) - .unwrap() - .as_ref() - .to_vec()) - }); + signer_mock + .expect_sign_uir() + .returning(move |_, _| Ok(get_invoice_request(decode(get_offer()).unwrap(), 10000))); let offer = decode(get_offer()).unwrap(); let handler = OfferHandler::new(); assert!(handler - .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000) + .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000,) .await .is_err()) } @@ -583,14 +605,16 @@ mod tests { .to_vec()) }); - signer_mock - .expect_sign_message() - .returning(|_, _, _| Err(Status::unknown("error testing"))); + signer_mock.expect_sign_uir().returning(move |_, _| { + Err(OfferError::SignError(SignError::Signing( + Secp256k1Error::InvalidSignature, + ))) + }); let offer = decode(get_offer()).unwrap(); let handler = OfferHandler::new(); assert!(handler - .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000) + .create_invoice_request(signer_mock, offer, vec![], Network::Regtest, 10000,) .await .is_err()) }