Merge pull request #1272 from lightning-signer/2022-01-sign-invoice-api
authorvalentinewallace <valentinewallace@users.noreply.github.com>
Mon, 24 Jan 2022 16:39:58 +0000 (11:39 -0500)
committerGitHub <noreply@github.com>
Mon, 24 Jan 2022 16:39:58 +0000 (11:39 -0500)
Improve KeysInterface::sign_invoice API

1  2 
fuzz/src/chanmon_consistency.rs
fuzz/src/full_stack.rs
lightning-invoice/src/utils.rs
lightning/src/chain/keysinterface.rs
lightning/src/ln/channel.rs

index 73cbb7a4a7f26634b6f825582d7d2be33612ca9a,7ac628d4ff009b59d3c6efc2d5f4c6b4ea66c470..ef0ae1da82094a9c79812911947a5db9cf428042
@@@ -63,6 -63,7 +63,7 @@@ use std::collections::{HashSet, hash_ma
  use std::sync::{Arc,Mutex};
  use std::sync::atomic;
  use std::io::Cursor;
+ use bitcoin::bech32::u5;
  
  const MAX_FEE: u32 = 10_000;
  struct FuzzEstimator {
@@@ -220,7 -221,7 +221,7 @@@ impl KeysInterface for KeyProvider 
                })
        }
  
-       fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
+       fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
                unreachable!()
        }
  }
@@@ -308,7 -309,7 +309,7 @@@ fn send_payment(source: &ChanMan, dest
                        fee_msat: amt,
                        cltv_expiry_delta: 200,
                }]],
 -              payee: None,
 +              payment_params: None,
        }, payment_hash, &Some(payment_secret)) {
                check_payment_err(err);
                false
@@@ -334,7 -335,7 +335,7 @@@ fn send_hop_payment(source: &ChanMan, m
                        fee_msat: amt,
                        cltv_expiry_delta: 200,
                }]],
 -              payee: None,
 +              payment_params: None,
        }, payment_hash, &Some(payment_secret)) {
                check_payment_err(err);
                false
diff --combined fuzz/src/full_stack.rs
index fd0df5a02e1df1b06639edb3c35672580000f069,c238aca6320b039e2dcba3afbd23fd90e01ed9a9..c5d091f2d21ec7f44885ab91b2f40e8ce907880e
@@@ -38,7 -38,7 +38,7 @@@ use lightning::ln::peer_handler::{Messa
  use lightning::ln::msgs::DecodeError;
  use lightning::ln::script::ShutdownScript;
  use lightning::routing::network_graph::{NetGraphMsgHandler, NetworkGraph};
 -use lightning::routing::router::{find_route, Payee, RouteParameters};
 +use lightning::routing::router::{find_route, PaymentParameters, RouteParameters};
  use lightning::routing::scoring::Scorer;
  use lightning::util::config::UserConfig;
  use lightning::util::errors::APIError;
@@@ -60,6 -60,7 +60,7 @@@ use std::convert::TryInto
  use std::cmp;
  use std::sync::{Arc, Mutex};
  use std::sync::atomic::{AtomicU64,AtomicUsize,Ordering};
+ use bitcoin::bech32::u5;
  
  #[inline]
  pub fn slice_to_be16(v: &[u8]) -> u16 {
@@@ -333,7 -334,7 +334,7 @@@ impl KeysInterface for KeyProvider 
                ))
        }
  
-       fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
+       fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
                unreachable!()
        }
  }
@@@ -446,9 -447,9 +447,9 @@@ pub fn do_test(data: &[u8], logger: &Ar
                        },
                        4 => {
                                let final_value_msat = slice_to_be24(get_slice!(3)) as u64;
 -                              let payee = Payee::from_node_id(get_pubkey!());
 +                              let payment_params = PaymentParameters::from_node_id(get_pubkey!());
                                let params = RouteParameters {
 -                                      payee,
 +                                      payment_params,
                                        final_value_msat,
                                        final_cltv_expiry_delta: 42,
                                };
                        },
                        15 => {
                                let final_value_msat = slice_to_be24(get_slice!(3)) as u64;
 -                              let payee = Payee::from_node_id(get_pubkey!());
 +                              let payment_params = PaymentParameters::from_node_id(get_pubkey!());
                                let params = RouteParameters {
 -                                      payee,
 +                                      payment_params,
                                        final_value_msat,
                                        final_cltv_expiry_delta: 42,
                                };
index 0a987a4fb52a6d2212754ab85197ce9f9dbc4399,189da28d1ebaf0547589d20340fc89ad4e00d68c..390290768de7da76bc94d9f87da58b9e3044669a
@@@ -1,6 -1,6 +1,6 @@@
  //! Convenient utilities to create an invoice.
  
- use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError, RawInvoice};
+ use {CreationError, Currency, DEFAULT_EXPIRY_TIME, Invoice, InvoiceBuilder, SignOrCreationError};
  use payment::{Payer, Router};
  
  use bech32::ToBase32;
@@@ -118,8 -118,7 +118,7 @@@ wher
        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 invoice_preimage = RawInvoice::construct_invoice_preimage(hrp_bytes, &data_without_signature);
-       let signed_raw_invoice = raw_invoice.sign(|_| keys_manager.sign_invoice(invoice_preimage));
+       let signed_raw_invoice = raw_invoice.sign(|_| keys_manager.sign_invoice(hrp_bytes, &data_without_signature));
        match signed_raw_invoice {
                Ok(inv) => Ok(Invoice::from_signed(inv).unwrap()),
                Err(e) => Err(SignOrCreationError::SignError(e))
@@@ -198,7 -197,7 +197,7 @@@ mod test 
        use lightning::ln::functional_test_utils::*;
        use lightning::ln::features::InitFeatures;
        use lightning::ln::msgs::ChannelMessageHandler;
 -      use lightning::routing::router::{Payee, RouteParameters, find_route};
 +      use lightning::routing::router::{PaymentParameters, RouteParameters, find_route};
        use lightning::util::events::MessageSendEventsProvider;
        use lightning::util::test_utils;
        use utils::create_invoice_from_channelmanager_and_duration_since_epoch;
                assert_eq!(invoice.min_final_cltv_expiry(), MIN_FINAL_CLTV_EXPIRY as u64);
                assert_eq!(invoice.description(), InvoiceDescription::Direct(&Description("test".to_string())));
  
 -              let payee = Payee::from_node_id(invoice.recover_payee_pub_key())
 +              let payment_params = PaymentParameters::from_node_id(invoice.recover_payee_pub_key())
                        .with_features(invoice.features().unwrap().clone())
                        .with_route_hints(invoice.route_hints());
 -              let params = RouteParameters {
 -                      payee,
 +              let route_params = RouteParameters {
 +                      payment_params,
                        final_value_msat: invoice.amount_milli_satoshis().unwrap(),
                        final_cltv_expiry_delta: invoice.min_final_cltv_expiry() as u32,
                };
                let logger = test_utils::TestLogger::new();
                let scorer = test_utils::TestScorer::with_fixed_penalty(0);
                let route = find_route(
 -                      &nodes[0].node.get_our_node_id(), &params, network_graph,
 +                      &nodes[0].node.get_our_node_id(), &route_params, network_graph,
                        Some(&first_hops.iter().collect::<Vec<_>>()), &logger, &scorer,
                ).unwrap();
  
index 9a46351c7b4174c7b279684b99b2837400e7b30a,c44c3e82e1c4d705ce3a28f10adf9956df2b701a..7538d0a83033c03f2f92f17a0dab8ff5c9be1ec5
@@@ -18,6 -18,7 +18,7 @@@ use bitcoin::network::constants::Networ
  use bitcoin::util::bip32::{ExtendedPrivKey, ExtendedPubKey, ChildNumber};
  use bitcoin::util::bip143;
  
+ use bitcoin::bech32::u5;
  use bitcoin::hashes::{Hash, HashEngine};
  use bitcoin::hashes::sha256::HashEngine as Sha256State;
  use bitcoin::hashes::sha256::Hash as Sha256;
@@@ -42,6 -43,7 +43,7 @@@ use prelude::*
  use core::sync::atomic::{AtomicUsize, Ordering};
  use io::{self, Error};
  use ln::msgs::{DecodeError, MAX_VALUE_MSAT};
+ use util::invoice::construct_invoice_preimage;
  
  /// Used as initial key material, to be expanded into multiple secret keys (but not to be used
  /// directly). This is used within LDK to encrypt/decrypt inbound payment data.
@@@ -398,11 -400,12 +400,12 @@@ pub trait KeysInterface 
        /// you've read all of the provided bytes to ensure no corruption occurred.
        fn read_chan_signer(&self, reader: &[u8]) -> Result<Self::Signer, DecodeError>;
  
-       /// Sign an invoice's preimage (note that this is the preimage of the invoice, not the HTLC's
-       /// preimage). By parameterizing by the preimage instead of the hash, we allow implementors of
+       /// Sign an invoice.
+       /// By parameterizing by the raw invoice bytes instead of the hash, we allow implementors of
        /// this trait to parse the invoice and make sure they're signing what they expect, rather than
        /// blindly signing the hash.
-       fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()>;
+       /// The hrp is ascii bytes, while the invoice data is base32.
+       fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()>;
  
        /// Get secret key material as bytes for use in encrypting and decrypting inbound payment data.
        ///
@@@ -526,8 -529,7 +529,8 @@@ impl InMemorySigner 
        /// described by descriptor, returning the witness stack for the input.
        ///
        /// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
 -      /// or is not spending the outpoint described by `descriptor.outpoint`.
 +      /// is not spending the outpoint described by `descriptor.outpoint`,
 +      /// or if an output descriptor script_pubkey does not match the one we can spend.
        pub fn sign_counterparty_payment_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &StaticPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
                // TODO: We really should be taking the SigHashCache as a parameter here instead of
                // spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
                let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
                let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
                let remotesig = secp_ctx.sign(&sighash, &self.payment_key);
 +              let payment_script = bitcoin::Address::p2wpkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Bitcoin).unwrap().script_pubkey();
 +
 +              if payment_script != descriptor.output.script_pubkey  { return Err(()); }
  
                let mut witness = Vec::with_capacity(2);
                witness.push(remotesig.serialize_der().to_vec());
        /// described by descriptor, returning the witness stack for the input.
        ///
        /// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
 -      /// is not spending the outpoint described by `descriptor.outpoint`, or does not have a
 -      /// sequence set to `descriptor.to_self_delay`.
 +      /// is not spending the outpoint described by `descriptor.outpoint`, does not have a
 +      /// sequence set to `descriptor.to_self_delay`, or if an output descriptor
 +      /// script_pubkey does not match the one we can spend.
        pub fn sign_dynamic_p2wsh_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &DelayedPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
                // TODO: We really should be taking the SigHashCache as a parameter here instead of
                // spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
                let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
                let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
                let local_delayedsig = secp_ctx.sign(&sighash, &delayed_payment_key);
 +              let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Bitcoin).script_pubkey();
 +
 +              if descriptor.output.script_pubkey != payment_script { return Err(()); }
  
                let mut witness = Vec::with_capacity(3);
                witness.push(local_delayedsig.serialize_der().to_vec());
@@@ -935,9 -930,8 +938,9 @@@ impl KeysManager 
        /// output to the given change destination (if sufficient change value remains). The
        /// transaction will have a feerate, at least, of the given value.
        ///
 -      /// Returns `Err(())` if the output value is greater than the input value minus required fee or
 -      /// if a descriptor was duplicated.
 +      /// Returns `Err(())` if the output value is greater than the input value minus required fee,
 +      /// if a descriptor was duplicated, or if an output descriptor script_pubkey 
 +      /// does not match the one we can spend.
        ///
        /// We do not enforce that outputs meet the dust limit or that any output scripts are standard.
        ///
                                                        self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
                                                        descriptor.channel_keys_id));
                                        }
 -                                      spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
 +                                      spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?;
                                },
                                SpendableOutputDescriptor::DelayedPaymentOutput(descriptor) => {
                                        if keys_cache.is_none() || keys_cache.as_ref().unwrap().1 != descriptor.channel_keys_id {
                                                        self.derive_channel_keys(descriptor.channel_value_satoshis, &descriptor.channel_keys_id),
                                                        descriptor.channel_keys_id));
                                        }
 -                                      spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
 +                                      spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx)?;
                                },
                                SpendableOutputDescriptor::StaticOutput { ref output, .. } => {
                                        let derivation_idx = if output.script_pubkey == self.destination_script {
                                                assert_eq!(pubkey.key, self.shutdown_pubkey);
                                        }
                                        let witness_script = bitcoin::Address::p2pkh(&pubkey, Network::Testnet).script_pubkey();
 +                                      let payment_script = bitcoin::Address::p2wpkh(&pubkey, Network::Testnet).expect("uncompressed key found").script_pubkey();
 +
 +                                      if payment_script != output.script_pubkey { return Err(()); };
 +
                                        let sighash = hash_to_message!(&bip143::SigHashCache::new(&spend_tx).signature_hash(input_idx, &witness_script, output.value, SigHashType::All)[..]);
                                        let sig = secp_ctx.sign(&sighash, &secret.private_key.key);
                                        spend_tx.input[input_idx].witness.push(sig.serialize_der().to_vec());
@@@ -1105,8 -1095,9 +1108,9 @@@ impl KeysInterface for KeysManager 
                InMemorySigner::read(&mut io::Cursor::new(reader))
        }
  
-       fn sign_invoice(&self, invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> {
-               Ok(self.secp_ctx.sign_recoverable(&hash_to_message!(&Sha256::hash(&invoice_preimage)), &self.get_node_secret()))
+       fn sign_invoice(&self, hrp_bytes: &[u8], invoice_data: &[u5]) -> Result<RecoverableSignature, ()> {
+               let preimage = construct_invoice_preimage(&hrp_bytes, &invoice_data);
+               Ok(self.secp_ctx.sign_recoverable(&hash_to_message!(&Sha256::hash(&preimage)), &self.get_node_secret()))
        }
  }
  
index e5dae3ac7121fbcdaafee7b9d6fa48961a5dd3a5,19f73ba65450017fd63e0b6ce1d3eededc8d21a4..4b1f4a7d1c46005b00df61a66eccdcf4975b4026
@@@ -5840,6 -5840,7 +5840,7 @@@ mod tests 
        use bitcoin::hashes::Hash;
        use bitcoin::hash_types::{Txid, WPubkeyHash};
        use core::num::NonZeroU8;
+       use bitcoin::bech32::u5;
        use sync::Arc;
        use prelude::*;
  
                }
                fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
                fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }
-               fn sign_invoice(&self, _invoice_preimage: Vec<u8>) -> Result<RecoverableSignature, ()> { panic!(); }
+               fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> { panic!(); }
        }
  
        fn public_from_secret_hex(secp_ctx: &Secp256k1<All>, hex: &str) -> PublicKey {
                                first_hop_htlc_msat: 548,
                                payment_id: PaymentId([42; 32]),
                                payment_secret: None,
 -                              payee: None,
 +                              payment_params: None,
                        }
                });