Generate local signatures with additional randomness
authorWilmer Paulino <wilmer@wilmerpaulino.com>
Wed, 19 Apr 2023 22:08:29 +0000 (15:08 -0700)
committerWilmer Paulino <wilmer@wilmerpaulino.com>
Thu, 20 Apr 2023 19:14:21 +0000 (12:14 -0700)
Previously, our local signatures would always be deterministic, whether
we'd grind for low R value signatures or not. For peers supporting
SegWit, Bitcoin Core will generally use a transaction's witness-txid, as
opposed to its txid, to advertise transactions. Therefore, to ensure a
transaction has the best chance to propagate across node mempools in the
network, each of its broadcast attempts should have a unique/distinct
witness-txid, which we can achieve by introducing random nonce data when
generating local signatures, such that they are no longer deterministic.

lightning/src/chain/keysinterface.rs
lightning/src/ln/chan_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/ln/payment_tests.rs
lightning/src/util/crypto.rs

index 8d522a3b46dd80a7cfe9e83d9f211480b9d47490..f291f4c8151da43e84840271d66d314f242f07fa 100644 (file)
@@ -32,7 +32,7 @@ use bitcoin::secp256k1::ecdsa::RecoverableSignature;
 use bitcoin::{PackedLockTime, secp256k1, Sequence, Witness};
 
 use crate::util::transaction_utils;
-use crate::util::crypto::{hkdf_extract_expand_twice, sign};
+use crate::util::crypto::{hkdf_extract_expand_twice, sign, sign_with_aux_rand};
 use crate::util::ser::{Writeable, Writer, Readable, ReadableArgs};
 use crate::chain::transaction::OutPoint;
 #[cfg(anchors)]
@@ -713,7 +713,7 @@ impl InMemorySigner {
                let remotepubkey = self.pubkeys().payment_point;
                let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, inner: remotepubkey}, Network::Testnet).script_pubkey();
                let sighash = hash_to_message!(&sighash::SighashCache::new(spend_tx).segwit_signature_hash(input_idx, &witness_script, descriptor.output.value, EcdsaSighashType::All).unwrap()[..]);
-               let remotesig = sign(secp_ctx, &sighash, &self.payment_key);
+               let remotesig = sign_with_aux_rand(secp_ctx, &sighash, &self.payment_key, &self);
                let payment_script = bitcoin::Address::p2wpkh(&::bitcoin::PublicKey{compressed: true, inner: remotepubkey}, Network::Bitcoin).unwrap().script_pubkey();
 
                if payment_script != descriptor.output.script_pubkey { return Err(()); }
@@ -749,7 +749,7 @@ impl InMemorySigner {
                let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
                let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
                let sighash = hash_to_message!(&sighash::SighashCache::new(spend_tx).segwit_signature_hash(input_idx, &witness_script, descriptor.output.value, EcdsaSighashType::All).unwrap()[..]);
-               let local_delayedsig = sign(secp_ctx, &sighash, &delayed_payment_key);
+               let local_delayedsig = sign_with_aux_rand(secp_ctx, &sighash, &delayed_payment_key, &self);
                let payment_script = bitcoin::Address::p2wsh(&witness_script, Network::Bitcoin).script_pubkey();
 
                if descriptor.output.script_pubkey != payment_script { return Err(()); }
@@ -810,7 +810,7 @@ impl EcdsaChannelSigner for InMemorySigner {
                let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
                let built_tx = trusted_tx.built_transaction();
-               let commitment_sig = built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let commitment_sig = built_tx.sign_counterparty_commitment(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx);
                let commitment_txid = built_tx.txid;
 
                let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
@@ -835,9 +835,9 @@ impl EcdsaChannelSigner for InMemorySigner {
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
                let trusted_tx = commitment_tx.trust();
-               let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
                let channel_parameters = self.get_channel_parameters();
-               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
+               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
                Ok((sig, htlc_sigs))
        }
 
@@ -846,9 +846,9 @@ impl EcdsaChannelSigner for InMemorySigner {
                let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
                let funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
                let trusted_tx = commitment_tx.trust();
-               let sig = trusted_tx.built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+               let sig = trusted_tx.built_transaction().sign_holder_commitment(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, &self, secp_ctx);
                let channel_parameters = self.get_channel_parameters();
-               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)?;
+               let htlc_sigs = trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), &self, secp_ctx)?;
                Ok((sig, htlc_sigs))
        }
 
@@ -862,7 +862,7 @@ impl EcdsaChannelSigner for InMemorySigner {
                };
                let mut sighash_parts = sighash::SighashCache::new(justice_tx);
                let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
-               return Ok(sign(secp_ctx, &sighash, &revocation_key))
+               return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self))
        }
 
        fn sign_justice_revoked_htlc(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
@@ -876,7 +876,7 @@ impl EcdsaChannelSigner for InMemorySigner {
                };
                let mut sighash_parts = sighash::SighashCache::new(justice_tx);
                let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
-               return Ok(sign(secp_ctx, &sighash, &revocation_key))
+               return Ok(sign_with_aux_rand(secp_ctx, &sighash, &revocation_key, &self))
        }
 
        #[cfg(anchors)]
@@ -894,7 +894,7 @@ impl EcdsaChannelSigner for InMemorySigner {
                let our_htlc_private_key = chan_utils::derive_private_key(
                        &secp_ctx, &per_commitment_point, &self.htlc_base_key
                );
-               Ok(sign(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key))
+               Ok(sign_with_aux_rand(&secp_ctx, &hash_to_message!(sighash), &our_htlc_private_key, &self))
        }
 
        fn sign_counterparty_htlc_transaction(&self, htlc_tx: &Transaction, input: usize, amount: u64, per_commitment_point: &PublicKey, htlc: &HTLCOutputInCommitment, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
@@ -905,7 +905,7 @@ impl EcdsaChannelSigner for InMemorySigner {
                let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, self.opt_anchors(), &counterparty_htlcpubkey, &htlcpubkey, &revocation_pubkey);
                let mut sighash_parts = sighash::SighashCache::new(htlc_tx);
                let sighash = hash_to_message!(&sighash_parts.segwit_signature_hash(input, &witness_script, amount, EcdsaSighashType::All).unwrap()[..]);
-               Ok(sign(secp_ctx, &sighash, &htlc_key))
+               Ok(sign_with_aux_rand(secp_ctx, &sighash, &htlc_key, &self))
        }
 
        fn sign_closing_transaction(&self, closing_tx: &ClosingTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
@@ -921,7 +921,7 @@ impl EcdsaChannelSigner for InMemorySigner {
                let sighash = sighash::SighashCache::new(&*anchor_tx).segwit_signature_hash(
                        input, &witness_script, ANCHOR_OUTPUT_VALUE_SATOSHI, EcdsaSighashType::All,
                ).unwrap();
-               Ok(sign(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key))
+               Ok(sign_with_aux_rand(secp_ctx, &hash_to_message!(&sighash[..]), &self.funding_key, &self))
        }
 
        fn sign_channel_announcement_with_funding_key(
@@ -1273,7 +1273,7 @@ impl KeysManager {
                                        if payment_script != output.script_pubkey { return Err(()); };
 
                                        let sighash = hash_to_message!(&sighash::SighashCache::new(&spend_tx).segwit_signature_hash(input_idx, &witness_script, output.value, EcdsaSighashType::All).unwrap()[..]);
-                                       let sig = sign(secp_ctx, &sighash, &secret.private_key);
+                                       let sig = sign_with_aux_rand(secp_ctx, &sighash, &secret.private_key, &self);
                                        let mut sig_ser = sig.serialize_der().to_vec();
                                        sig_ser.push(EcdsaSighashType::All as u8);
                                        spend_tx.input[input_idx].witness.push(sig_ser);
index 533da9772a2c8db922794546b696e4bae32a7f94..9699cc78a1a3253a7635277d22c133a3517b2804 100644 (file)
@@ -21,6 +21,7 @@ use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::hashes::ripemd160::Hash as Ripemd160;
 use bitcoin::hash_types::{Txid, PubkeyHash};
 
+use crate::chain::keysinterface::EntropySource;
 use crate::ln::{PaymentHash, PaymentPreimage};
 use crate::ln::msgs::DecodeError;
 use crate::util::ser::{Readable, Writeable, Writer};
@@ -39,7 +40,7 @@ use crate::util::transaction_utils::sort_outputs;
 use crate::ln::channel::{INITIAL_COMMITMENT_NUMBER, ANCHOR_OUTPUT_VALUE_SATOSHI};
 use core::ops::Deref;
 use crate::chain;
-use crate::util::crypto::sign;
+use crate::util::crypto::{sign, sign_with_aux_rand};
 
 /// Maximum number of one-way in-flight HTLC (protocol-level value).
 pub const MAX_HTLCS: u16 = 483;
@@ -1081,12 +1082,20 @@ impl BuiltCommitmentTransaction {
                hash_to_message!(sighash)
        }
 
-       /// Sign a transaction, either because we are counter-signing the counterparty's transaction or
-       /// because we are about to broadcast a holder transaction.
-       pub fn sign<T: secp256k1::Signing>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
+       /// Signs the counterparty's commitment transaction.
+       pub fn sign_counterparty_commitment<T: secp256k1::Signing>(&self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64, secp_ctx: &Secp256k1<T>) -> Signature {
                let sighash = self.get_sighash_all(funding_redeemscript, channel_value_satoshis);
                sign(secp_ctx, &sighash, funding_key)
        }
+
+       /// Signs the holder commitment transaction because we are about to broadcast it.
+       pub fn sign_holder_commitment<T: secp256k1::Signing, ES: Deref>(
+               &self, funding_key: &SecretKey, funding_redeemscript: &Script, channel_value_satoshis: u64,
+               entropy_source: &ES, secp_ctx: &Secp256k1<T>
+       ) -> Signature where ES::Target: EntropySource {
+               let sighash = self.get_sighash_all(funding_redeemscript, channel_value_satoshis);
+               sign_with_aux_rand(secp_ctx, &sighash, funding_key, entropy_source)
+       }
 }
 
 /// This class tracks the per-transaction information needed to build a closing transaction and will
@@ -1563,7 +1572,10 @@ impl<'a> TrustedCommitmentTransaction<'a> {
        /// The returned Vec has one entry for each HTLC, and in the same order.
        ///
        /// This function is only valid in the holder commitment context, it always uses EcdsaSighashType::All.
-       pub fn get_htlc_sigs<T: secp256k1::Signing>(&self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
+       pub fn get_htlc_sigs<T: secp256k1::Signing, ES: Deref>(
+               &self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters,
+               entropy_source: &ES, secp_ctx: &Secp256k1<T>,
+       ) -> Result<Vec<Signature>, ()> where ES::Target: EntropySource {
                let inner = self.inner;
                let keys = &inner.keys;
                let txid = inner.built.txid;
@@ -1577,7 +1589,7 @@ impl<'a> TrustedCommitmentTransaction<'a> {
                        let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, self.opt_anchors(), &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);
 
                        let sighash = hash_to_message!(&sighash::SighashCache::new(&htlc_tx).segwit_signature_hash(0, &htlc_redeemscript, this_htlc.amount_msat / 1000, EcdsaSighashType::All).unwrap()[..]);
-                       ret.push(sign(secp_ctx, &sighash, &holder_htlc_key));
+                       ret.push(sign_with_aux_rand(secp_ctx, &sighash, &holder_htlc_key, entropy_source));
                }
                Ok(ret)
        }
index f2b05b6e1ede4fa05d1fd8eee35679f82e0c88e6..401667f31f6dc80611fb3ef7e8422ff8f0994fbf 100644 (file)
@@ -3461,7 +3461,7 @@ fn test_htlc_ignore_latest_remote_commitment() {
 
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(node_txn.len(), 3);
-       assert_eq!(node_txn[0], node_txn[1]);
+       assert_eq!(node_txn[0].txid(), node_txn[1].txid());
 
        let mut header = BlockHeader { version: 0x20000000, prev_blockhash: nodes[1].best_block_hash(), merkle_root: TxMerkleNode::all_zeros(), time: 42, bits: 42, nonce: 42 };
        connect_block(&nodes[1], &Block { header, txdata: vec![node_txn[0].clone(), node_txn[1].clone()]});
@@ -9248,7 +9248,7 @@ fn do_test_tx_confirmed_skipping_blocks_immediate_broadcast(test_height_before_t
                // We should broadcast an HTLC transaction spending our funding transaction first
                let spending_txn = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
                assert_eq!(spending_txn.len(), 2);
-               assert_eq!(spending_txn[0], node_txn[0]);
+               assert_eq!(spending_txn[0].txid(), node_txn[0].txid());
                check_spends!(spending_txn[1], node_txn[0]);
                // We should also generate a SpendableOutputs event with the to_self output (as its
                // timelock is up).
index 65b0fc00628a6a5237591f9bcc58647a5575948a..51811ec632943efb373031bc87eacab3feecfb69 100644 (file)
@@ -343,7 +343,7 @@ fn do_retry_with_no_persist(confirm_before_reload: bool) {
        if !confirm_before_reload {
                let as_broadcasted_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
                assert_eq!(as_broadcasted_txn.len(), 1);
-               assert_eq!(as_broadcasted_txn[0], as_commitment_tx);
+               assert_eq!(as_broadcasted_txn[0].txid(), as_commitment_tx.txid());
        } else {
                assert!(nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().is_empty());
        }
@@ -684,7 +684,7 @@ fn do_test_dup_htlc_onchain_fails_on_reload(persist_manager_post_event: bool, co
        connect_blocks(&nodes[0], TEST_FINAL_CLTV + LATENCY_GRACE_PERIOD_BLOCKS + 1);
        let node_txn = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
        assert_eq!(node_txn.len(), 3);
-       assert_eq!(node_txn[0], node_txn[1]);
+       assert_eq!(node_txn[0].txid(), node_txn[1].txid());
        check_spends!(node_txn[1], funding_tx);
        check_spends!(node_txn[2], node_txn[1]);
        let timeout_txn = vec![node_txn[2].clone()];
index 2f2d33b29f7ea53c0a2dda084ac7a139583c6345..d4d15cfa3045a9fb09b4400b12302b9984d5d49f 100644 (file)
@@ -3,6 +3,10 @@ use bitcoin::hashes::hmac::{Hmac, HmacEngine};
 use bitcoin::hashes::sha256::Hash as Sha256;
 use bitcoin::secp256k1::{Message, Secp256k1, SecretKey, ecdsa::Signature, Signing};
 
+use crate::chain::keysinterface::EntropySource;
+
+use core::ops::Deref;
+
 macro_rules! hkdf_extract_expand {
        ($salt: expr, $ikm: expr) => {{
                let mut hmac = HmacEngine::<Sha256>::new($salt);
@@ -46,3 +50,19 @@ pub fn sign<C: Signing>(ctx: &Secp256k1<C>, msg: &Message, sk: &SecretKey) -> Si
        let sig = ctx.sign_ecdsa(msg, sk);
        sig
 }
+
+#[inline]
+pub fn sign_with_aux_rand<C: Signing, ES: Deref>(
+       ctx: &Secp256k1<C>, msg: &Message, sk: &SecretKey, entropy_source: &ES
+) -> Signature where ES::Target: EntropySource {
+       #[cfg(feature = "grind_signatures")]
+       let sig = loop {
+               let sig = ctx.sign_ecdsa_with_noncedata(msg, sk, &entropy_source.get_secure_random_bytes());
+               if sig.serialize_compact()[0] < 0x80 {
+                       break sig;
+               }
+       };
+       #[cfg(not(feature = "grind_signatures"))]
+       let sig = ctx.sign_ecdsa_with_noncedata(msg, sk, &entropy_source.get_secure_random_bytes());
+       sig
+}