From b1d536e57ac99c7446f50c4f64c4a8b46c3b3830 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 May 2020 16:00:46 -0400 Subject: [PATCH 1/6] Expose private keys from InMemoryChannelKeys publicly As we drop the requirement that all ChannelKeys expose the private keys used, we should have a way to access the private keys in use when using InMemoryChannelKeys. --- lightning/src/chain/keysinterface.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 9de35f0ef43..b7583bb1f7b 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -345,17 +345,17 @@ pub trait KeysInterface: Send + Sync { /// A simple implementation of ChannelKeys that just keeps the private keys in memory. pub struct InMemoryChannelKeys { /// Private key of anchor tx - funding_key: SecretKey, + pub funding_key: SecretKey, /// Local secret key for blinded revocation pubkey - revocation_base_key: SecretKey, + pub revocation_base_key: SecretKey, /// Local secret key used for our balance in remote-broadcasted commitment transactions - payment_key: SecretKey, + pub payment_key: SecretKey, /// Local secret key used in HTLC tx - delayed_payment_base_key: SecretKey, + pub delayed_payment_base_key: SecretKey, /// Local htlc secret key used in commitment tx htlc outputs - htlc_base_key: SecretKey, + pub htlc_base_key: SecretKey, /// Commitment seed - commitment_seed: [u8; 32], + pub commitment_seed: [u8; 32], /// Local public keys and basepoints pub(crate) local_channel_pubkeys: ChannelPublicKeys, /// Remote public keys and base points From 9f7bcfb1ede0ecd8cd635c2bacf1774cc65487b7 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 May 2020 16:02:33 -0400 Subject: [PATCH 2/6] Drop requirement that all ChannelKeys expose the funding privkey --- lightning/src/chain/keysinterface.rs | 3 --- lightning/src/ln/channel.rs | 26 ++++++++++----------- lightning/src/ln/functional_tests.rs | 4 ++-- lightning/src/util/enforcing_trait_impls.rs | 1 - 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index b7583bb1f7b..5859385d9fb 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -195,8 +195,6 @@ impl Readable for SpendableOutputDescriptor { // TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create // ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors. pub trait ChannelKeys : Send+Clone { - /// Gets the private key for the anchor tx - fn funding_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local secret key for blinded revocation pubkey fn revocation_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local secret key used in the to_remote output of remote commitment tx (ie the @@ -416,7 +414,6 @@ impl InMemoryChannelKeys { } impl ChannelKeys for InMemoryChannelKeys { - fn funding_key(&self) -> &SecretKey { &self.funding_key } fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key } fn payment_key(&self) -> &SecretKey { &self.payment_key } fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8d55804f41d..9c8fc343169 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1120,8 +1120,7 @@ impl Channel { /// pays to get_funding_redeemscript().to_v0_p2wsh()). /// Panics if called before accept_channel/new_from_req pub fn get_funding_redeemscript(&self) -> Script { - let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()); - make_funding_redeemscript(&our_funding_key, self.their_funding_pubkey()) + make_funding_redeemscript(&self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey()) } /// Builds the htlc-success or htlc-timeout transaction which spends a given HTLC output @@ -1455,7 +1454,7 @@ impl Channel { log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {}", log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.their_funding_pubkey().serialize()), encode::serialize_hex(&local_initial_commitment_tx), log_bytes!(local_sighash[..]), encode::serialize_hex(&funding_script)); secp_check!(self.secp_ctx.verify(&local_sighash, &sig, self.their_funding_pubkey()), "Invalid funding_created signature from peer"); - let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new()); + let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig.clone(), &self.local_keys.pubkeys().funding_pubkey, self.their_funding_pubkey(), local_keys, self.feerate_per_kw, Vec::new()); let remote_keys = self.build_remote_transaction_keys()?; let remote_initial_commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw, logger).0; @@ -1568,7 +1567,7 @@ impl Channel { let funding_txo_script = funding_redeemscript.to_v0_p2wsh(); macro_rules! create_monitor { () => { { - let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()); + let local_commitment_tx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx.clone(), msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, their_funding_pubkey, local_keys.clone(), self.feerate_per_kw, Vec::new()); let mut channel_monitor = ChannelMonitor::new(self.local_keys.clone(), &self.shutdown_pubkey, self.our_to_self_delay, &self.destination_script, (funding_txo.clone(), funding_txo_script.clone()), @@ -1899,7 +1898,7 @@ impl Channel { let mut monitor_update = ChannelMonitorUpdate { update_id: self.latest_monitor_update_id, updates: vec![ChannelMonitorUpdateStep::LatestLocalCommitmentTXInfo { - commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source), + commitment_tx: LocalCommitmentTransaction::new_missing_local_sig(local_commitment_tx.0, msg.signature.clone(), &self.local_keys.pubkeys().funding_pubkey, &their_funding_pubkey, local_keys, self.feerate_per_kw, htlcs_without_source), htlc_outputs: htlcs_and_sigs }] }; @@ -2825,7 +2824,7 @@ impl Channel { tx.input[0].witness.push(Vec::new()); // First is the multisig dummy - let our_funding_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()).serialize(); + let our_funding_key = self.local_keys.pubkeys().funding_pubkey.serialize(); let their_funding_key = self.their_funding_pubkey().serialize(); if our_funding_key[..] < their_funding_key[..] { tx.input[0].witness.push(our_sig.serialize_der().to_vec()); @@ -3302,6 +3301,7 @@ impl Channel { } let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number); + let local_keys = self.local_keys.pubkeys(); msgs::OpenChannel { chain_hash: chain_hash, @@ -3315,7 +3315,7 @@ impl Channel { feerate_per_kw: fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background) as u32, to_self_delay: self.our_to_self_delay, max_accepted_htlcs: OUR_MAX_HTLCS, - funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), + funding_pubkey: local_keys.funding_pubkey, revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()), delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), @@ -3338,6 +3338,7 @@ impl Channel { } let local_commitment_secret = self.build_local_commitment_secret(self.cur_local_commitment_transaction_number); + let local_keys = self.local_keys.pubkeys(); msgs::AcceptChannel { temporary_channel_id: self.channel_id, @@ -3348,7 +3349,7 @@ impl Channel { minimum_depth: self.minimum_depth, to_self_delay: self.our_to_self_delay, max_accepted_htlcs: OUR_MAX_HTLCS, - funding_pubkey: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), + funding_pubkey: local_keys.funding_pubkey, revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()), delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), @@ -3431,7 +3432,6 @@ impl Channel { } let were_node_one = our_node_id.serialize()[..] < self.their_node_id.serialize()[..]; - let our_bitcoin_key = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()); let msg = msgs::UnsignedChannelAnnouncement { features: ChannelFeatures::known(), @@ -3439,8 +3439,8 @@ impl Channel { short_channel_id: self.get_short_channel_id().unwrap(), node_id_1: if were_node_one { our_node_id } else { self.get_their_node_id() }, node_id_2: if were_node_one { self.get_their_node_id() } else { our_node_id }, - bitcoin_key_1: if were_node_one { our_bitcoin_key } else { self.their_funding_pubkey().clone() }, - bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { our_bitcoin_key }, + bitcoin_key_1: if were_node_one { self.local_keys.pubkeys().funding_pubkey } else { self.their_funding_pubkey().clone() }, + bitcoin_key_2: if were_node_one { self.their_funding_pubkey().clone() } else { self.local_keys.pubkeys().funding_pubkey }, excess_data: Vec::new(), }; @@ -4442,7 +4442,7 @@ mod tests { (0, 0) ); - assert_eq!(PublicKey::from_secret_key(&secp_ctx, chan_keys.funding_key()).serialize()[..], + assert_eq!(chan_keys.pubkeys().funding_pubkey.serialize()[..], hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]); let keys_provider = Keys { chan_keys: chan_keys.clone() }; @@ -4512,7 +4512,7 @@ mod tests { })* assert_eq!(unsigned_tx.1.len(), per_htlc.len()); - localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &PublicKey::from_secret_key(&secp_ctx, chan.local_keys.funding_key()), chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc); + localtx = LocalCommitmentTransaction::new_missing_local_sig(unsigned_tx.0.clone(), their_signature.clone(), &chan_keys.pubkeys().funding_pubkey, chan.their_funding_pubkey(), keys.clone(), chan.feerate_per_kw, per_htlc); let local_sig = chan_keys.sign_local_commitment(&localtx, &chan.secp_ctx).unwrap(); assert_eq!(Signature::from_der(&hex::decode($our_sig_hex).unwrap()[..]).unwrap(), local_sig); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 13125a36dda..5eb3602851e 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -3850,8 +3850,8 @@ fn test_invalid_channel_announcement() { macro_rules! sign_msg { ($unsigned_msg: expr) => { let msghash = Message::from_slice(&Sha256dHash::hash(&$unsigned_msg.encode()[..])[..]).unwrap(); - let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key()); - let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key()); + let as_bitcoin_sig = secp_ctx.sign(&msghash, &as_chan.get_local_keys().inner.funding_key); + let bs_bitcoin_sig = secp_ctx.sign(&msghash, &bs_chan.get_local_keys().inner.funding_key); let as_node_sig = secp_ctx.sign(&msghash, &nodes[0].keys_manager.get_node_secret()); let bs_node_sig = secp_ctx.sign(&msghash, &nodes[1].keys_manager.get_node_secret()); chan_announcement = msgs::ChannelAnnouncement { diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 9a1b3ccf983..41666374836 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -51,7 +51,6 @@ impl EnforcingChannelKeys { } impl ChannelKeys for EnforcingChannelKeys { - fn funding_key(&self) -> &SecretKey { self.inner.funding_key() } fn revocation_base_key(&self) -> &SecretKey { self.inner.revocation_base_key() } fn payment_key(&self) -> &SecretKey { self.inner.payment_key() } fn delayed_payment_base_key(&self) -> &SecretKey { self.inner.delayed_payment_base_key() } From 1a574d205557f6770d00f6f297dfd9d6ff5a53c0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 May 2020 16:03:03 -0400 Subject: [PATCH 3/6] Drop requirement that all ChannelKeys expose the payment_point --- lightning/src/chain/keysinterface.rs | 5 ----- lightning/src/ln/channel.rs | 9 ++++----- lightning/src/ln/channelmonitor.rs | 2 +- lightning/src/ln/functional_tests.rs | 4 ++-- lightning/src/util/enforcing_trait_impls.rs | 1 - 5 files changed, 7 insertions(+), 14 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 5859385d9fb..88040a04b7f 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -197,10 +197,6 @@ impl Readable for SpendableOutputDescriptor { pub trait ChannelKeys : Send+Clone { /// Gets the local secret key for blinded revocation pubkey fn revocation_base_key<'a>(&'a self) -> &'a SecretKey; - /// Gets the local secret key used in the to_remote output of remote commitment tx (ie the - /// output to us in transactions our counterparty broadcasts). - /// Also as part of obscured commitment number. - fn payment_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local htlc secret key used in commitment tx htlc outputs @@ -415,7 +411,6 @@ impl InMemoryChannelKeys { impl ChannelKeys for InMemoryChannelKeys { fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key } - fn payment_key(&self) -> &SecretKey { &self.payment_key } fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key } fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key } fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9c8fc343169..48d6e674d03 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -766,15 +766,14 @@ impl Channel { fn get_commitment_transaction_number_obscure_factor(&self) -> u64 { let mut sha = Sha256::engine(); - let our_payment_point = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()); let their_payment_point = &self.their_pubkeys.as_ref().unwrap().payment_point.serialize(); if self.channel_outbound { - sha.input(&our_payment_point.serialize()); + sha.input(&self.local_keys.pubkeys().payment_point.serialize()); sha.input(their_payment_point); } else { sha.input(their_payment_point); - sha.input(&our_payment_point.serialize()); + sha.input(&self.local_keys.pubkeys().payment_point.serialize()); } let res = Sha256::from_engine(sha).into_inner(); @@ -3317,7 +3316,7 @@ impl Channel { max_accepted_htlcs: OUR_MAX_HTLCS, funding_pubkey: local_keys.funding_pubkey, revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), - payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()), + payment_point: local_keys.payment_point, delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), @@ -3351,7 +3350,7 @@ impl Channel { max_accepted_htlcs: OUR_MAX_HTLCS, funding_pubkey: local_keys.funding_pubkey, revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), - payment_point: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.payment_key()), + payment_point: local_keys.payment_point, delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), diff --git a/lightning/src/ln/channelmonitor.rs b/lightning/src/ln/channelmonitor.rs index 6d10fdb4f05..4f20b1d42fe 100644 --- a/lightning/src/ln/channelmonitor.rs +++ b/lightning/src/ln/channelmonitor.rs @@ -1641,7 +1641,7 @@ impl ChannelMonitor { self.remote_payment_script = { // Note that the Network here is ignored as we immediately drop the address for the // script_pubkey version - let payment_hash160 = WPubkeyHash::hash(&PublicKey::from_secret_key(&self.secp_ctx, &self.keys.payment_key()).serialize()); + let payment_hash160 = WPubkeyHash::hash(&self.keys.pubkeys().payment_point.serialize()); Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0).push_slice(&payment_hash160[..]).into_script() }; diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 5eb3602851e..9513dc66561 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4293,10 +4293,10 @@ macro_rules! check_spendable_outputs { }; let secp_ctx = Secp256k1::new(); let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1); - let remotepubkey = PublicKey::from_secret_key(&secp_ctx, &keys.payment_key()); + let remotepubkey = keys.pubkeys().payment_point; let witness_script = Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey(); let sighash = Message::from_slice(&bip143::SighashComponents::new(&spend_tx).sighash_all(&spend_tx.input[0], &witness_script, output.value)[..]).unwrap(); - let remotesig = secp_ctx.sign(&sighash, &keys.payment_key()); + let remotesig = secp_ctx.sign(&sighash, &keys.inner.payment_key); spend_tx.input[0].witness.push(remotesig.serialize_der().to_vec()); spend_tx.input[0].witness[0].push(SigHashType::All as u8); spend_tx.input[0].witness.push(remotepubkey.serialize().to_vec()); diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 41666374836..ffb05762ae6 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -52,7 +52,6 @@ impl EnforcingChannelKeys { impl ChannelKeys for EnforcingChannelKeys { fn revocation_base_key(&self) -> &SecretKey { self.inner.revocation_base_key() } - fn payment_key(&self) -> &SecretKey { self.inner.payment_key() } fn delayed_payment_base_key(&self) -> &SecretKey { self.inner.delayed_payment_base_key() } fn htlc_base_key(&self) -> &SecretKey { self.inner.htlc_base_key() } fn commitment_seed(&self) -> &[u8; 32] { self.inner.commitment_seed() } From d9f5df99b06841fba7eb4be191d05214b5b1e477 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 May 2020 16:06:28 -0400 Subject: [PATCH 4/6] Drop requirement that all ChannelKeys expose revocaion_basepoint --- lightning/src/chain/keysinterface.rs | 3 --- lightning/src/ln/channel.rs | 8 ++++---- lightning/src/util/enforcing_trait_impls.rs | 4 +--- 3 files changed, 5 insertions(+), 10 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 88040a04b7f..10687b74c88 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -195,8 +195,6 @@ impl Readable for SpendableOutputDescriptor { // TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create // ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors. pub trait ChannelKeys : Send+Clone { - /// Gets the local secret key for blinded revocation pubkey - fn revocation_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local htlc secret key used in commitment tx htlc outputs @@ -410,7 +408,6 @@ impl InMemoryChannelKeys { } impl ChannelKeys for InMemoryChannelKeys { - fn revocation_base_key(&self) -> &SecretKey { &self.revocation_base_key } fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key } fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key } fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 48d6e674d03..f737e56a73d 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1108,11 +1108,11 @@ impl Channel { fn build_remote_transaction_keys(&self) -> Result { //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we //may see payments to it! - let revocation_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()); + let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint; let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()); let their_pubkeys = self.their_pubkeys.as_ref().unwrap(); - Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, &revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys")) + Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys")) } /// Gets the redeemscript for the funding transaction output (ie the funding transaction output @@ -3315,7 +3315,7 @@ impl Channel { to_self_delay: self.our_to_self_delay, max_accepted_htlcs: OUR_MAX_HTLCS, funding_pubkey: local_keys.funding_pubkey, - revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), + revocation_basepoint: local_keys.revocation_basepoint, payment_point: local_keys.payment_point, delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), @@ -3349,7 +3349,7 @@ impl Channel { to_self_delay: self.our_to_self_delay, max_accepted_htlcs: OUR_MAX_HTLCS, funding_pubkey: local_keys.funding_pubkey, - revocation_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.revocation_base_key()), + revocation_basepoint: local_keys.revocation_basepoint, payment_point: local_keys.payment_point, delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index ffb05762ae6..425b2124f9c 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -35,7 +35,6 @@ impl EnforcingChannelKeys { impl EnforcingChannelKeys { fn check_keys(&self, secp_ctx: &Secp256k1, keys: &TxCreationKeys) { - let revocation_base = PublicKey::from_secret_key(secp_ctx, &self.inner.revocation_base_key()); let htlc_base = PublicKey::from_secret_key(secp_ctx, &self.inner.htlc_base_key()); let remote_points = self.inner.remote_channel_pubkeys.as_ref().unwrap(); @@ -44,14 +43,13 @@ impl EnforcingChannelKeys { &keys.per_commitment_point, &remote_points.delayed_payment_basepoint, &remote_points.htlc_basepoint, - &revocation_base, + &self.inner.pubkeys().revocation_basepoint, &htlc_base).unwrap(); if keys != &keys_expected { panic!("derived different per-tx keys") } } } impl ChannelKeys for EnforcingChannelKeys { - fn revocation_base_key(&self) -> &SecretKey { self.inner.revocation_base_key() } fn delayed_payment_base_key(&self) -> &SecretKey { self.inner.delayed_payment_base_key() } fn htlc_base_key(&self) -> &SecretKey { self.inner.htlc_base_key() } fn commitment_seed(&self) -> &[u8; 32] { self.inner.commitment_seed() } From d77e40fa76d7f6f9ab5c5ae91d686ac8b8226b3f Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 May 2020 16:09:47 -0400 Subject: [PATCH 5/6] Drop requirement that ChannelKeys expose delayed_payment_basepoint --- lightning/src/chain/keysinterface.rs | 3 --- lightning/src/ln/channel.rs | 12 ++++++------ lightning/src/ln/functional_tests.rs | 2 +- lightning/src/util/enforcing_trait_impls.rs | 1 - 4 files changed, 7 insertions(+), 11 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 10687b74c88..05e24d8c1b2 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -195,8 +195,6 @@ impl Readable for SpendableOutputDescriptor { // TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create // ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors. pub trait ChannelKeys : Send+Clone { - /// Gets the local secret key used in HTLC-Success/HTLC-Timeout txn and to_local output - fn delayed_payment_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the local htlc secret key used in commitment tx htlc outputs fn htlc_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the commitment seed @@ -408,7 +406,6 @@ impl InMemoryChannelKeys { } impl ChannelKeys for InMemoryChannelKeys { - fn delayed_payment_base_key(&self) -> &SecretKey { &self.delayed_payment_base_key } fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key } fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed } fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f737e56a73d..0c30a7e96cb 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1094,11 +1094,11 @@ impl Channel { /// TODO Some magic rust shit to compile-time check this? fn build_local_transaction_keys(&self, commitment_number: u64) -> Result { let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number)); - let delayed_payment_base = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()); + let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint; let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()); let their_pubkeys = self.their_pubkeys.as_ref().unwrap(); - Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys")) + Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys")) } #[inline] @@ -3317,7 +3317,7 @@ impl Channel { funding_pubkey: local_keys.funding_pubkey, revocation_basepoint: local_keys.revocation_basepoint, payment_point: local_keys.payment_point, - delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), + delayed_payment_basepoint: local_keys.delayed_payment_basepoint, htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), channel_flags: if self.config.announced_channel {1} else {0}, @@ -3351,7 +3351,7 @@ impl Channel { funding_pubkey: local_keys.funding_pubkey, revocation_basepoint: local_keys.revocation_basepoint, payment_point: local_keys.payment_point, - delayed_payment_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.delayed_payment_base_key()), + delayed_payment_basepoint: local_keys.delayed_payment_basepoint, htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) @@ -4476,11 +4476,11 @@ mod tests { // We can't just use build_local_transaction_keys here as the per_commitment_secret is not // derived from a commitment_seed, so instead we copy it here and call // build_commitment_transaction. - let delayed_payment_base = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.delayed_payment_base_key()); + let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint; let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap(); let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret); let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key()); - let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, &delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap(); + let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap(); chan.their_pubkeys = Some(their_pubkeys); diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 9513dc66561..bd1b1d4f029 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -4321,7 +4321,7 @@ macro_rules! check_spendable_outputs { }; let secp_ctx = Secp256k1::new(); let keys = $keysinterface.derive_channel_keys($chan_value, key_derivation_params.0, key_derivation_params.1); - if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, keys.delayed_payment_base_key()) { + if let Ok(delayed_payment_key) = chan_utils::derive_private_key(&secp_ctx, &per_commitment_point, &keys.inner.delayed_payment_base_key) { let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key); let witness_script = chan_utils::get_revokeable_redeemscript(remote_revocation_pubkey, *to_self_delay, &delayed_payment_pubkey); diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 425b2124f9c..03df5af861f 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -50,7 +50,6 @@ impl EnforcingChannelKeys { } impl ChannelKeys for EnforcingChannelKeys { - fn delayed_payment_base_key(&self) -> &SecretKey { self.inner.delayed_payment_base_key() } fn htlc_base_key(&self) -> &SecretKey { self.inner.htlc_base_key() } fn commitment_seed(&self) -> &[u8; 32] { self.inner.commitment_seed() } fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { self.inner.pubkeys() } From e5a74227f63a5b1cdd93c8cd365db99ec9cd48a9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Thu, 28 May 2020 16:13:38 -0400 Subject: [PATCH 6/6] Drop requirement that all ChannelKeys expose htlc_basepoint --- lightning/src/chain/keysinterface.rs | 3 --- lightning/src/ln/channel.rs | 16 ++++++++-------- lightning/src/util/enforcing_trait_impls.rs | 5 +---- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index 05e24d8c1b2..fb7538199d2 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/lightning/src/chain/keysinterface.rs @@ -195,8 +195,6 @@ impl Readable for SpendableOutputDescriptor { // TODO: We should remove Clone by instead requesting a new ChannelKeys copy when we create // ChannelMonitors instead of expecting to clone the one out of the Channel into the monitors. pub trait ChannelKeys : Send+Clone { - /// Gets the local htlc secret key used in commitment tx htlc outputs - fn htlc_base_key<'a>(&'a self) -> &'a SecretKey; /// Gets the commitment seed fn commitment_seed<'a>(&'a self) -> &'a [u8; 32]; /// Gets the local channel public keys and basepoints @@ -406,7 +404,6 @@ impl InMemoryChannelKeys { } impl ChannelKeys for InMemoryChannelKeys { - fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key } fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed } fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { &self.local_channel_pubkeys } fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 0c30a7e96cb..c1cdac3cecc 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1095,10 +1095,10 @@ impl Channel { fn build_local_transaction_keys(&self, commitment_number: u64) -> Result { let per_commitment_point = PublicKey::from_secret_key(&self.secp_ctx, &self.build_local_commitment_secret(commitment_number)); let delayed_payment_base = &self.local_keys.pubkeys().delayed_payment_basepoint; - let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()); + let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint; let their_pubkeys = self.their_pubkeys.as_ref().unwrap(); - Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys")) + Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint), "Local tx keys generation got bogus keys")) } #[inline] @@ -1109,10 +1109,10 @@ impl Channel { //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we //may see payments to it! let revocation_basepoint = &self.local_keys.pubkeys().revocation_basepoint; - let htlc_basepoint = PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()); + let htlc_basepoint = &self.local_keys.pubkeys().htlc_basepoint; let their_pubkeys = self.their_pubkeys.as_ref().unwrap(); - Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, &htlc_basepoint), "Remote tx keys generation got bogus keys")) + Ok(secp_check!(TxCreationKeys::new(&self.secp_ctx, &self.their_cur_commitment_point.unwrap(), &their_pubkeys.delayed_payment_basepoint, &their_pubkeys.htlc_basepoint, revocation_basepoint, htlc_basepoint), "Remote tx keys generation got bogus keys")) } /// Gets the redeemscript for the funding transaction output (ie the funding transaction output @@ -3318,7 +3318,7 @@ impl Channel { revocation_basepoint: local_keys.revocation_basepoint, payment_point: local_keys.payment_point, delayed_payment_basepoint: local_keys.delayed_payment_basepoint, - htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), + htlc_basepoint: local_keys.htlc_basepoint, first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), channel_flags: if self.config.announced_channel {1} else {0}, shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) @@ -3352,7 +3352,7 @@ impl Channel { revocation_basepoint: local_keys.revocation_basepoint, payment_point: local_keys.payment_point, delayed_payment_basepoint: local_keys.delayed_payment_basepoint, - htlc_basepoint: PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.htlc_base_key()), + htlc_basepoint: local_keys.htlc_basepoint, first_per_commitment_point: PublicKey::from_secret_key(&self.secp_ctx, &local_commitment_secret), shutdown_scriptpubkey: OptionalField::Present(if self.config.commit_upfront_shutdown_pubkey { self.get_closing_scriptpubkey() } else { Builder::new().into_script() }) } @@ -4479,8 +4479,8 @@ mod tests { let delayed_payment_base = &chan.local_keys.pubkeys().delayed_payment_basepoint; let per_commitment_secret = SecretKey::from_slice(&hex::decode("1f1e1d1c1b1a191817161514131211100f0e0d0c0b0a09080706050403020100").unwrap()[..]).unwrap(); let per_commitment_point = PublicKey::from_secret_key(&secp_ctx, &per_commitment_secret); - let htlc_basepoint = PublicKey::from_secret_key(&secp_ctx, chan.local_keys.htlc_base_key()); - let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, &htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap(); + let htlc_basepoint = &chan.local_keys.pubkeys().htlc_basepoint; + let keys = TxCreationKeys::new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &their_pubkeys.revocation_basepoint, &their_pubkeys.htlc_basepoint).unwrap(); chan.their_pubkeys = Some(their_pubkeys); diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 03df5af861f..6856805aac4 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/lightning/src/util/enforcing_trait_impls.rs @@ -35,8 +35,6 @@ impl EnforcingChannelKeys { impl EnforcingChannelKeys { fn check_keys(&self, secp_ctx: &Secp256k1, keys: &TxCreationKeys) { - let htlc_base = PublicKey::from_secret_key(secp_ctx, &self.inner.htlc_base_key()); - let remote_points = self.inner.remote_channel_pubkeys.as_ref().unwrap(); let keys_expected = TxCreationKeys::new(secp_ctx, @@ -44,13 +42,12 @@ impl EnforcingChannelKeys { &remote_points.delayed_payment_basepoint, &remote_points.htlc_basepoint, &self.inner.pubkeys().revocation_basepoint, - &htlc_base).unwrap(); + &self.inner.pubkeys().htlc_basepoint).unwrap(); if keys != &keys_expected { panic!("derived different per-tx keys") } } } impl ChannelKeys for EnforcingChannelKeys { - fn htlc_base_key(&self) -> &SecretKey { self.inner.htlc_base_key() } fn commitment_seed(&self) -> &[u8; 32] { self.inner.commitment_seed() } fn pubkeys<'a>(&'a self) -> &'a ChannelPublicKeys { self.inner.pubkeys() } fn key_derivation_params(&self) -> (u64, u64) { self.inner.key_derivation_params() }