Support scalar tweak to rotate holder funding key during splicing

A scalar tweak applied to the base funding key to obtain the channel's
funding key used in the 2-of-2 multisig. This is used to derive
additional keys from the same secret backing the base `funding_pubkey`,
as we have to rotate keys for each successful splice attempt.

The tweak is computed similar to existing tweaks used in
[BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation),
but rather than using the `per_commitment_point`, we use the txid of the
funding transaction the splice transaction is spending to guarantee
uniqueness, and the `revocation_basepoint` to guarantee only the channel
participants can re-derive the new funding key.

  tweak = SHA256(splice_parent_funding_txid || revocation_basepoint || base_funding_pubkey)
  tweaked_funding_key = base_funding_key + tweak

Author: wpaulino

lightning/src/chain/channelmonitor.rs

@@ -3461,7 +3461,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		let broadcaster_keys = &self.onchain_tx_handler.channel_transaction_parameters
 			.counterparty_parameters.as_ref().unwrap().pubkeys;
 		let countersignatory_keys =
-			&self.onchain_tx_handler.channel_transaction_parameters.holder_pubkeys;
+			self.onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.inner();
 
 		let broadcaster_funding_key = broadcaster_keys.funding_pubkey;
 		let countersignatory_funding_key = countersignatory_keys.funding_pubkey;
@@ -5136,7 +5136,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		if onchain_tx_handler.channel_type_features().supports_anchors_zero_fee_htlc_tx() &&
 			counterparty_payment_script.is_p2wpkh()
 		{
-			let payment_point = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.payment_point;
+			let payment_point = onchain_tx_handler.channel_transaction_parameters.holder_pubkeys.inner().payment_point;
 			counterparty_payment_script =
 				chan_utils::get_to_countersignatory_with_anchors_redeemscript(&payment_point).to_p2wsh();
 		}
@@ -5235,7 +5235,7 @@ mod tests {
 	use crate::ln::types::ChannelId;
 	use crate::types::payment::{PaymentPreimage, PaymentHash};
 	use crate::ln::channel_keys::{DelayedPaymentBasepoint, DelayedPaymentKey, HtlcBasepoint, RevocationBasepoint, RevocationKey};
-	use crate::ln::chan_utils::{self,HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
+	use crate::ln::chan_utils::{self,HTLCOutputInCommitment, ChannelPublicKeys, ChannelTransactionParameters, HolderChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters};
 	use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
 	use crate::ln::functional_test_utils::*;
 	use crate::ln::script::ShutdownScript;
@@ -5415,7 +5415,13 @@ mod tests {
 		let funding_outpoint = OutPoint { txid: Txid::all_zeros(), index: u16::MAX };
 		let channel_id = ChannelId::v1_from_funding_outpoint(funding_outpoint);
 		let channel_parameters = ChannelTransactionParameters {
-			holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+			holder_pubkeys: HolderChannelPublicKeys::new(
+				keys.holder_channel_pubkeys.funding_pubkey,
+				keys.holder_channel_pubkeys.revocation_basepoint,
+				keys.holder_channel_pubkeys.payment_point,
+				keys.holder_channel_pubkeys.delayed_payment_basepoint,
+				keys.holder_channel_pubkeys.htlc_basepoint, None, &secp_ctx,
+			),
 			holder_selected_contest_delay: 66,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
@@ -5667,7 +5673,13 @@ mod tests {
 		let funding_outpoint = OutPoint { txid: Txid::all_zeros(), index: u16::MAX };
 		let channel_id = ChannelId::v1_from_funding_outpoint(funding_outpoint);
 		let channel_parameters = ChannelTransactionParameters {
-			holder_pubkeys: keys.holder_channel_pubkeys.clone(),
+			holder_pubkeys: HolderChannelPublicKeys::new(
+				keys.holder_channel_pubkeys.funding_pubkey,
+				keys.holder_channel_pubkeys.revocation_basepoint,
+				keys.holder_channel_pubkeys.payment_point,
+				keys.holder_channel_pubkeys.delayed_payment_basepoint,
+				keys.holder_channel_pubkeys.htlc_basepoint, None, &secp_ctx,
+			),
 			holder_selected_contest_delay: 66,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {

lightning/src/chain/onchaintx.rs

@@ -665,7 +665,7 @@ impl<ChannelSigner: EcdsaChannelSigner> OnchainTxHandler<ChannelSigner> {
 					}
 
 					// We'll locate an anchor output we can spend within the commitment transaction.
-					let funding_pubkey = &self.channel_transaction_parameters.holder_pubkeys.funding_pubkey;
+					let funding_pubkey = &self.channel_transaction_parameters.holder_pubkeys.inner().funding_pubkey;
 					match chan_utils::get_anchor_output(&tx.0, funding_pubkey) {
 						// An anchor output was found, so we should yield a funding event externally.
 						Some((idx, _)) => {
@@ -1290,7 +1290,7 @@ mod tests {
 	use crate::chain::transaction::OutPoint;
 	use crate::ln::chan_utils::{
 		ChannelPublicKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters,
-		HTLCOutputInCommitment, HolderCommitmentTransaction,
+		HTLCOutputInCommitment, HolderChannelPublicKeys, HolderCommitmentTransaction,
 	};
 	use crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint};
 	use crate::ln::functional_test_utils::create_dummy_block;
@@ -1344,7 +1344,13 @@ mod tests {
 		// Use non-anchor channels so that HTLC-Timeouts are broadcast immediately instead of sent
 		// to the user for external funding.
 		let chan_params = ChannelTransactionParameters {
-			holder_pubkeys: signer.holder_channel_pubkeys.clone(),
+			holder_pubkeys: HolderChannelPublicKeys::new(
+				signer.holder_channel_pubkeys.funding_pubkey,
+				signer.holder_channel_pubkeys.revocation_basepoint,
+				signer.holder_channel_pubkeys.payment_point,
+				signer.holder_channel_pubkeys.delayed_payment_basepoint,
+				signer.holder_channel_pubkeys.htlc_basepoint, None, &secp_ctx,
+			),
 			holder_selected_contest_delay: 66,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {

lightning/src/ln/chan_utils.rs

@@ -35,7 +35,7 @@ use crate::util::transaction_utils;
 
 use bitcoin::locktime::absolute::LockTime;
 use bitcoin::ecdsa::Signature as BitcoinSignature;
-use bitcoin::secp256k1::{SecretKey, PublicKey, Scalar};
+use bitcoin::secp256k1::{SecretKey, PublicKey, Scalar, Verification};
 use bitcoin::secp256k1::{Secp256k1, ecdsa::Signature, Message};
 use bitcoin::{secp256k1, Sequence, Witness};
 
@@ -430,6 +430,26 @@ pub fn derive_private_revocation_key<T: secp256k1::Signing>(secp_ctx: &Secp256k1
 		.expect("Addition only fails if the tweak is the inverse of the key. This is not possible when the tweak commits to the key.")
 }
 
+/// Computes the tweak to apply to the base funding key of a channel.
+///
+/// The tweak is computed similar to existing tweaks used in
+/// [BOLT-3](https://github.com/lightning/bolts/blob/master/03-transactions.md#key-derivation), but
+/// rather than using the `per_commitment_point`, we use the txid of the funding transaction the
+/// splice transaction is spending to guarantee uniqueness, and the `revocation_basepoint` to
+/// guarantee only the channel participants can re-derive the new funding key.
+///
+///   tweak = SHA256(splice_parent_funding_txid || revocation_basepoint || base_funding_pubkey)
+///   tweaked_funding_key = base_funding_key + tweak
+//
+// TODO: Expose a helper on `FundingScope` that calls this.
+pub fn compute_funding_key_tweak(base_funding_pubkey: &PublicKey, revocation_basepoint: &PublicKey, splice_parent_funding_txid: &Txid) -> Scalar {
+	let mut sha = Sha256::engine();
+	sha.input(splice_parent_funding_txid.as_byte_array());
+	sha.input(&revocation_basepoint.serialize());
+	sha.input(&base_funding_pubkey.serialize());
+	Scalar::from_be_bytes(Sha256::from_engine(sha).to_byte_array()).unwrap()
+}
+
 /// The set of public keys which are used in the creation of one commitment transaction.
 /// These are derived from the channel base keys and per-commitment data.
 ///
@@ -470,6 +490,9 @@ impl_writeable_tlv_based!(TxCreationKeys, {
 pub struct ChannelPublicKeys {
 	/// The public key which is used to sign all commitment transactions, as it appears in the
 	/// on-chain channel lock-in 2-of-2 multisig output.
+	///
+	/// NOTE: This key will already have the [`HolderChannelPublicKeys::funding_key_tweak`] applied
+	/// if one existed.
 	pub funding_pubkey: PublicKey,
 	/// The base point which is used (with [`RevocationKey::from_basepoint`]) to derive per-commitment
 	/// revocation keys. This is combined with the per-commitment-secret generated by the
@@ -497,6 +520,102 @@ impl_writeable_tlv_based!(ChannelPublicKeys, {
 	(8, htlc_basepoint, required),
 });
 
+/// The holder's public keys which do not change over the life of a channel, except for the
+/// `funding_pubkey`, which may rotate after each successful splice attempt via the
+/// `funding_key_tweak`.
+#[derive(Clone, Debug, Hash, PartialEq, Eq)]
+pub struct HolderChannelPublicKeys {
+	keys: ChannelPublicKeys,
+	/// A optional scalar tweak applied to the base funding key to obtain the channel's funding key
+	/// used in the 2-of-2 multisig. This is used to derive additional keys from the same secret
+	/// backing the base `funding_pubkey`, as we have to rotate keys for each successful splice
+	/// attempt. The tweak is computed as described in [`compute_funding_key_tweak`].
+	//
+	// TODO: Expose `splice_parent_funding_txid` instead so the signer can re-derive the tweak?
+	// There's no harm in the signer trusting the tweak as long as its funding secret has not
+	// been leaked.
+	pub funding_key_tweak: Option<Scalar>,
+}
+
+// `HolderChannelPublicKeys` may have been previously written as `ChannelPublicKeys` so we have to
+// mimic its serialization.
+impl Writeable for HolderChannelPublicKeys {
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		write_tlv_fields!(writer, {
+			(0, self.keys.funding_pubkey, required),
+			(2, self.keys.revocation_basepoint, required),
+			(4, self.keys.payment_point, required),
+			(6, self.keys.delayed_payment_basepoint, required),
+			(8, self.keys.htlc_basepoint, required),
+			(10, self.funding_key_tweak, option),
+		});
+		Ok(())
+	}
+}
+
+impl Readable for HolderChannelPublicKeys {
+	fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+		let mut funding_pubkey = RequiredWrapper(None);
+		let mut revocation_basepoint = RequiredWrapper(None);
+		let mut payment_point = RequiredWrapper(None);
+		let mut delayed_payment_basepoint = RequiredWrapper(None);
+		let mut htlc_basepoint = RequiredWrapper(None);
+		let mut funding_key_tweak: Option<Scalar> = None;
+
+		read_tlv_fields!(reader, {
+			(0, funding_pubkey, required),
+			(2, revocation_basepoint, required),
+			(4, payment_point, required),
+			(6, delayed_payment_basepoint, required),
+			(8, htlc_basepoint, required),
+			(10, funding_key_tweak, option),
+		});
+
+		Ok(Self {
+			keys: ChannelPublicKeys {
+				funding_pubkey: funding_pubkey.0.unwrap(),
+				revocation_basepoint: revocation_basepoint.0.unwrap(),
+				payment_point: payment_point.0.unwrap(),
+				delayed_payment_basepoint: delayed_payment_basepoint.0.unwrap(),
+				htlc_basepoint: htlc_basepoint.0.unwrap(),
+			},
+			funding_key_tweak,
+		})
+	}
+}
+
+impl HolderChannelPublicKeys {
+	/// Constructs a new instance of [`HolderChannelPublicKeys`].
+	pub fn new<C: Verification>(
+		funding_pubkey: PublicKey, revocation_basepoint: RevocationBasepoint,
+		payment_point: PublicKey, delayed_payment_basepoint: DelayedPaymentBasepoint,
+		htlc_basepoint: HtlcBasepoint, funding_key_tweak: Option<Scalar>, secp: &Secp256k1<C>,
+	) -> Self {
+		let funding_pubkey = funding_key_tweak
+			.map(|tweak| {
+				funding_pubkey
+					.add_exp_tweak(secp, &tweak)
+					.expect("Addition only fails if the tweak is the inverse of the key")
+			})
+			.unwrap_or(funding_pubkey);
+		Self {
+			keys: ChannelPublicKeys {
+				funding_pubkey,
+				revocation_basepoint,
+				payment_point,
+				delayed_payment_basepoint,
+				htlc_basepoint,
+			},
+			funding_key_tweak,
+		}
+	}
+
+	/// Returns the holder's channel public keys as a [`ChannelPublicKeys`].
+	pub fn inner(&self) -> &ChannelPublicKeys {
+		&self.keys
+	}
+}
+
 impl TxCreationKeys {
 	/// Create per-state keys from channel base points and the per-commitment point.
 	/// Key set is asymmetric and can't be used as part of counter-signatory set of transactions.
@@ -869,7 +988,7 @@ pub fn build_anchor_input_witness(funding_key: &PublicKey, funding_sig: &Signatu
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct ChannelTransactionParameters {
 	/// Holder public keys
-	pub holder_pubkeys: ChannelPublicKeys,
+	pub holder_pubkeys: HolderChannelPublicKeys,
 	/// The contest delay selected by the holder, which applies to counterparty-broadcast transactions
 	pub holder_selected_contest_delay: u16,
 	/// Whether the holder is the initiator of this channel.
@@ -933,7 +1052,7 @@ impl ChannelTransactionParameters {
 
 	pub(crate) fn make_funding_redeemscript(&self) -> ScriptBuf {
 		make_funding_redeemscript(
-			&self.holder_pubkeys.funding_pubkey,
+			&self.holder_pubkeys.inner().funding_pubkey,
 			&self.counterparty_parameters.as_ref().unwrap().pubkeys.funding_pubkey
 		)
 	}
@@ -953,7 +1072,10 @@ impl ChannelTransactionParameters {
 			htlc_basepoint: PublicKey::from_slice(&[2; 33]).unwrap().into(),
 		};
 		Self {
-			holder_pubkeys: dummy_keys.clone(),
+			holder_pubkeys: HolderChannelPublicKeys {
+				keys: dummy_keys.clone(),
+				funding_key_tweak: None,
+			},
 			holder_selected_contest_delay: 42,
 			is_outbound_from_holder: true,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
@@ -1050,7 +1172,7 @@ impl<'a> DirectedChannelTransactionParameters<'a> {
 	/// Get the channel pubkeys for the broadcaster
 	pub fn broadcaster_pubkeys(&self) -> &'a ChannelPublicKeys {
 		if self.holder_is_broadcaster {
-			&self.inner.holder_pubkeys
+			self.inner.holder_pubkeys.inner()
 		} else {
 			&self.inner.counterparty_parameters.as_ref().unwrap().pubkeys
 		}
@@ -1061,7 +1183,7 @@ impl<'a> DirectedChannelTransactionParameters<'a> {
 		if self.holder_is_broadcaster {
 			&self.inner.counterparty_parameters.as_ref().unwrap().pubkeys
 		} else {
-			&self.inner.holder_pubkeys
+			self.inner.holder_pubkeys.inner()
 		}
 	}
 
@@ -1149,7 +1271,10 @@ impl HolderCommitmentTransaction {
 			htlc_basepoint: HtlcBasepoint::from(dummy_key.clone())
 		};
 		let channel_parameters = ChannelTransactionParameters {
-			holder_pubkeys: channel_pubkeys.clone(),
+			holder_pubkeys: HolderChannelPublicKeys {
+				keys: channel_pubkeys.clone(),
+				funding_key_tweak: None,
+			},
 			holder_selected_contest_delay: 0,
 			is_outbound_from_holder: false,
 			counterparty_parameters: Some(CounterpartyChannelTransactionParameters { pubkeys: channel_pubkeys.clone(), selected_contest_delay: 0 }),
@@ -1918,7 +2043,7 @@ pub fn get_commitment_transaction_number_obscure_factor(
 
 #[cfg(test)]
 mod tests {
-	use super::{CounterpartyCommitmentSecrets, ChannelPublicKeys};
+	use super::{CounterpartyCommitmentSecrets, ChannelPublicKeys, HolderChannelPublicKeys};
 	use crate::chain;
 	use crate::ln::chan_utils::{get_htlc_redeemscript, get_to_countersignatory_with_anchors_redeemscript, CommitmentTransaction, TxCreationKeys, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, HTLCOutputInCommitment};
 	use bitcoin::secp256k1::{PublicKey, SecretKey, Secp256k1};
@@ -1961,7 +2086,11 @@ mod tests {
 			let counterparty_pubkeys = counterparty_signer.pubkeys().clone();
 			let keys = TxCreationKeys::derive_new(&secp_ctx, &per_commitment_point, delayed_payment_base, htlc_basepoint, &counterparty_pubkeys.revocation_basepoint, &counterparty_pubkeys.htlc_basepoint);
 			let channel_parameters = ChannelTransactionParameters {
-				holder_pubkeys: holder_pubkeys.clone(),
+				holder_pubkeys: HolderChannelPublicKeys::new(
+					holder_pubkeys.funding_pubkey, holder_pubkeys.revocation_basepoint,
+					holder_pubkeys.payment_point, holder_pubkeys.delayed_payment_basepoint,
+					holder_pubkeys.htlc_basepoint, None, &secp_ctx,
+				),
 				holder_selected_contest_delay: 0,
 				is_outbound_from_holder: false,
 				counterparty_parameters: Some(CounterpartyChannelTransactionParameters { pubkeys: counterparty_pubkeys.clone(), selected_contest_delay: 0 }),

lightning/src/ln/channel.rs

@@ -42,7 +42,7 @@ use crate::ln::channel_state::{ChannelShutdownState, CounterpartyForwardingInfo,
 use crate::ln::channelmanager::{self, OpenChannelMessage, PendingHTLCStatus, HTLCSource, SentHTLCId, HTLCFailureMsg, PendingHTLCInfo, RAACommitmentOrder, PaymentClaimDetails, BREAKDOWN_TIMEOUT, MIN_CLTV_EXPIRY_DELTA, MAX_LOCAL_BREAKDOWN_TIMEOUT};
 use crate::ln::chan_utils::{
 	CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight,
-	htlc_timeout_tx_weight, ChannelPublicKeys, CommitmentTransaction,
+	htlc_timeout_tx_weight, ChannelPublicKeys, HolderChannelPublicKeys, CommitmentTransaction,
 	HolderCommitmentTransaction, ChannelTransactionParameters,
 	CounterpartyChannelTransactionParameters, MAX_HTLCS,
 	get_commitment_transaction_number_obscure_factor,
@@ -1693,7 +1693,7 @@ impl FundingScope {
 	}
 
 	fn get_holder_pubkeys(&self) -> &ChannelPublicKeys {
-		&self.channel_transaction_parameters.holder_pubkeys
+		self.channel_transaction_parameters.holder_pubkeys.inner()
 	}
 
 	pub fn get_counterparty_selected_contest_delay(&self) -> Option<u16> {
@@ -2585,7 +2585,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
 			channel_transaction_parameters: ChannelTransactionParameters {
-				holder_pubkeys: pubkeys,
+				holder_pubkeys: HolderChannelPublicKeys::new(
+					pubkeys.funding_pubkey, pubkeys.revocation_basepoint, pubkeys.payment_point,
+					pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, None, &secp_ctx,
+				),
 				holder_selected_contest_delay: config.channel_handshake_config.our_to_self_delay,
 				is_outbound_from_holder: false,
 				counterparty_parameters: Some(CounterpartyChannelTransactionParameters {
@@ -2822,7 +2825,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
 			channel_transaction_parameters: ChannelTransactionParameters {
-				holder_pubkeys: pubkeys,
+				holder_pubkeys: HolderChannelPublicKeys::new(
+					pubkeys.funding_pubkey, pubkeys.revocation_basepoint, pubkeys.payment_point,
+					pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint, None, &secp_ctx,
+				),
 				holder_selected_contest_delay: config.channel_handshake_config.our_to_self_delay,
 				is_outbound_from_holder: true,
 				counterparty_parameters: None,