fixup! improve the API of the derived fields in CommitmentTransaction

Author: devrandom

lightning/src/chain/channelmonitor.rs

@@ -969,23 +969,23 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 
 		let secp_ctx = Secp256k1::new();
 
-		let txid = initial_holder_commitment_tx.untrusted_txid();
-
 		// block for Rust 1.34 compat
 		let (holder_commitment_tx, current_holder_commitment_number) = {
-			let commitment_tx = &initial_holder_commitment_tx.inner;
-			let tx_keys = commitment_tx.untrusted_key_derivation();
+			let trusted_tx = initial_holder_commitment_tx.trust();
+			let txid = trusted_tx.txid();
+
+			let tx_keys = trusted_tx.keys();
 			let holder_commitment_tx = HolderSignedTx {
 				txid,
 				revocation_key: tx_keys.revocation_key,
 				a_htlc_key: tx_keys.broadcaster_htlc_key,
 				b_htlc_key: tx_keys.countersignatory_htlc_key,
 				delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
 				per_commitment_point: tx_keys.per_commitment_point,
-				feerate_per_kw: commitment_tx.feerate_per_kw(),
+				feerate_per_kw: trusted_tx.feerate_per_kw(),
 				htlc_outputs: Vec::new(), // There are never any HTLCs in the initial commitment transactions
 			};
-			(holder_commitment_tx, commitment_tx.commitment_number())
+			(holder_commitment_tx, trusted_tx.commitment_number())
 		};
 		onchain_tx_handler.provide_latest_holder_tx(initial_holder_commitment_tx);
 
@@ -1143,21 +1143,20 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 	/// up-to-date as our holder commitment transaction is updated.
 	/// Panics if set_on_holder_tx_csv has never been called.
 	fn provide_latest_holder_commitment_tx(&mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Signature>, Option<HTLCSource>)>) -> Result<(), MonitorUpdateError> {
-		let txid = holder_commitment_tx.untrusted_txid();
-
 		// block for Rust 1.34 compat
 		let mut new_holder_commitment_tx = {
-			let commitment_tx = &holder_commitment_tx.inner;
-			let tx_keys = &commitment_tx.untrusted_key_derivation();
-			self.current_holder_commitment_number = commitment_tx.commitment_number();
+			let trusted_tx = holder_commitment_tx.trust();
+			let txid = trusted_tx.txid();
+			let tx_keys = trusted_tx.keys();
+			self.current_holder_commitment_number = trusted_tx.commitment_number();
 			HolderSignedTx {
 				txid,
 				revocation_key: tx_keys.revocation_key,
 				a_htlc_key: tx_keys.broadcaster_htlc_key,
 				b_htlc_key: tx_keys.countersignatory_htlc_key,
 				delayed_payment_key: tx_keys.broadcaster_delayed_payment_key,
 				per_commitment_point: tx_keys.per_commitment_point,
-				feerate_per_kw: commitment_tx.feerate_per_kw(),
+				feerate_per_kw: trusted_tx.feerate_per_kw(),
 				htlc_outputs,
 			}
 		};
@@ -1249,11 +1248,11 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
 		}
 		for update in updates.updates.iter() {
 			match update {
-				ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx: commitment_info, htlc_outputs } => {
+				ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs } => {
 					log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info");
 					if self.lockdown_from_offchain { panic!(); }
-					self.provide_latest_holder_commitment_tx(commitment_info.clone(), htlc_outputs.clone())?
-				},
+					self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone())?
+				}
 				ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_revocation_point } => {
 					log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info");
 					self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_revocation_point, logger)

lightning/src/chain/keysinterface.rs

@@ -231,8 +231,6 @@ pub trait ChannelKeys : Send+Clone {
 	/// Note that if signing fails or is rejected, the channel will be force-closed.
 	//
 	// TODO: Document the things someone using this interface should enforce before signing.
-	// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
-	// making the callee generate it via some util function we expose)!
 	fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
 
 	/// Create a signature for a holder's commitment transaction. This will only ever be called with
@@ -241,7 +239,6 @@ pub trait ChannelKeys : Send+Clone {
 	/// An external signer implementation should check that the commitment has not been revoked.
 	//
 	// TODO: Document the things someone using this interface should enforce before signing.
-	// TODO: Add more input vars to enable better checking (preferably removing commitment_tx and
 	fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
 
 	/// Same as sign_holder_commitment, but exists only for tests to get access to holder commitment
@@ -260,10 +257,7 @@ pub trait ChannelKeys : Send+Clone {
 	/// ChannelMonitor decided to broadcast before it had been updated to the latest.
 	///
 	/// Either an Err should be returned, or a Vec with one entry for each HTLC which exists in
-	/// holder_commitment_tx. For those HTLCs which have transaction_output_index set to None
-	/// (implying they were considered dust at the time the commitment transaction was negotiated),
-	/// a corresponding None should be included in the return value. All other positions in the
-	/// return value must contain a signature.
+	/// holder_commitment_tx.
 	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()>;
 
 	/// Create a signature for the given input in a transaction spending an HTLC or commitment
@@ -430,8 +424,7 @@ impl InMemoryChannelKeys {
 
 	/// The contest_delay value specified by our counterparty and applied on holder-broadcastable
 	/// transactions, ie the amount of time that we have to wait to recover our funds if we
-	/// broadcast a transaction. You'll likely want to pass this to the
-	/// ln::chan_utils::build*_transaction functions when signing holder's transactions.
+	/// broadcast a transaction.
 	/// Will panic if ready_channel wasn't called.
 	pub fn counterparty_selected_contest_delay(&self) -> u16 { self.channel_parameters.as_ref().unwrap().counterparty_parameters.as_ref().unwrap().selected_contest_delay }
 
@@ -463,15 +456,15 @@ impl ChannelKeys for InMemoryChannelKeys {
 	fn key_derivation_params(&self) -> (u64, u64) { self.key_derivation_params }
 
 	fn sign_counterparty_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
-		let keys = commitment_tx.untrusted_key_derivation();
+		let trusted_tx = commitment_tx.trust();
+		let keys = trusted_tx.keys();
 
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
 
-		let built_tx = commitment_tx.untrusted_built_transaction();
+		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_txid = commitment_tx.untrusted_txid();
+		let commitment_txid = built_tx.txid;
 
 		let mut htlc_sigs = Vec::with_capacity(commitment_tx.htlcs().len());
 		for htlc in commitment_tx.htlcs() {
@@ -491,28 +484,24 @@ impl ChannelKeys for InMemoryChannelKeys {
 	fn sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
 		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 built_tx = commitment_tx.inner.untrusted_built_transaction();
-		let sig = built_tx.sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
+		let sig = commitment_tx.trust().built_transaction().sign(&self.funding_key, &funding_redeemscript, self.channel_value_satoshis, secp_ctx);
 		let htlc_sigs_o = self.sign_holder_commitment_htlc_transactions(&commitment_tx, secp_ctx)?;
 		let htlc_sigs = htlc_sigs_o.iter().map(|o| o.unwrap()).collect();
 
 		Ok((sig, htlc_sigs))
 	}
 
 	#[cfg(any(test,feature = "unsafe_revoked_tx_signing"))]
-	fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
+	fn unsafe_sign_holder_commitment<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
 		let funding_pubkey = PublicKey::from_secret_key(secp_ctx, &self.funding_key);
 		let channel_funding_redeemscript = make_funding_redeemscript(&funding_pubkey, &self.counterparty_pubkeys().funding_pubkey);
-
-		let built_tx = holder_commitment_tx.inner.untrusted_built_transaction();
-		Ok(built_tx.sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
+		Ok(commitment_tx.trust().built_transaction().sign(&self.funding_key, &channel_funding_redeemscript, self.channel_value_satoshis, secp_ctx))
 	}
 
 	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
 		let channel_parameters = self.make_channel_parameters();
-		let channel_parameters = channel_parameters.as_holder_broadcastable();
-		commitment_tx.inner.get_htlc_sigs(&self.htlc_base_key, &channel_parameters, secp_ctx)
+		let trusted_tx = commitment_tx.trust();
+		trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)
 	}
 
 	fn sign_justice_transaction<T: secp256k1::Signing + secp256k1::Verification>(&self, justice_tx: &Transaction, input: usize, amount: u64, per_commitment_key: &SecretKey, htlc: &Option<HTLCOutputInCommitment>, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {