fixup! remove dust handling from TrustedCommitmentTransaction.get_htlc_sigs

Author: devrandom

lightning/src/chain/keysinterface.rs

@@ -258,7 +258,7 @@ pub trait ChannelKeys : Send+Clone {
 	///
 	/// Either an Err should be returned, or a Vec with one entry for each HTLC which exists in
 	/// 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>>, ()>;
+	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()>;
 
 	/// Create a signature for the given input in a transaction spending an HTLC or commitment
 	/// transaction output when our counterparty broadcasts an old state.
@@ -485,9 +485,7 @@ impl ChannelKeys for InMemoryChannelKeys {
 		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 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();
-
+		let htlc_sigs = self.sign_holder_commitment_htlc_transactions(&commitment_tx, secp_ctx)?;
 		Ok((sig, htlc_sigs))
 	}
 
@@ -498,7 +496,7 @@ impl ChannelKeys for InMemoryChannelKeys {
 		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>>, ()> {
+	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
 		let channel_parameters = self.make_channel_parameters();
 		let trusted_tx = commitment_tx.trust();
 		trusted_tx.get_htlc_sigs(&self.htlc_base_key, &channel_parameters.as_holder_broadcastable(), secp_ctx)

lightning/src/ln/chan_utils.rs

@@ -1150,27 +1150,22 @@ impl<'a> TrustedCommitmentTransaction<'a> {
 	/// Get a signature for each HTLC which was included in the commitment transaction (ie for
 	/// which HTLCOutputInCommitment::transaction_output_index.is_some()).
 	///
-	/// The returned Vec has one entry for each HTLC, and in the same order. For HTLCs which were
-	/// considered dust and not included, a None entry exists, for all others a signature is
-	/// included.
-	pub fn get_htlc_sigs<T: secp256k1::Signing>(&self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
+	/// The returned Vec has one entry for each HTLC, and in the same order.
+	pub fn get_htlc_sigs<T: secp256k1::Signing>(&self, htlc_base_key: &SecretKey, channel_parameters: &DirectedChannelTransactionParameters, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
 		let inner = self.inner;
 		let keys = &inner.keys;
 		let txid = inner.built.txid;
 		let mut ret = Vec::with_capacity(inner.htlcs.len());
 		let holder_htlc_key = derive_private_key(secp_ctx, &inner.keys.per_commitment_point, htlc_base_key).map_err(|_| ())?;
 
 		for this_htlc in inner.htlcs.iter() {
-			if this_htlc.transaction_output_index.is_some() {
-				let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
+			assert!(this_htlc.transaction_output_index.is_some());
+			let htlc_tx = build_htlc_transaction(&txid, inner.feerate_per_kw, channel_parameters.contest_delay(), &this_htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
 
-				let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);
+			let htlc_redeemscript = get_htlc_redeemscript_with_explicit_keys(&this_htlc, &keys.broadcaster_htlc_key, &keys.countersignatory_htlc_key, &keys.revocation_key);
 
-				let sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, this_htlc.amount_msat / 1000, SigHashType::All)[..]);
-				ret.push(Some(secp_ctx.sign(&sighash, &holder_htlc_key)));
-			} else {
-				ret.push(None);
-			}
+			let sighash = hash_to_message!(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, this_htlc.amount_msat / 1000, SigHashType::All)[..]);
+			ret.push(secp_ctx.sign(&sighash, &holder_htlc_key));
 		}
 		Ok(ret)
 	}

lightning/src/ln/channel.rs

@@ -4780,18 +4780,14 @@ mod tests {
 					assert_eq!((htlc_sig.0).0.transaction_output_index, Some($htlc_idx), "output index");
 
 					let signature = Signature::from_der(&hex::decode($htlc_sig_hex).unwrap()[..]).unwrap();
-					assert_eq!(Some(signature), *(htlc_sig.1).1, "htlc sig");
+					assert_eq!(signature, *(htlc_sig.1).1, "htlc sig");
 					let index = (htlc_sig.1).0;
 					let channel_parameters = chan.channel_transaction_parameters.as_holder_broadcastable();
 					let trusted_tx = holder_commitment_tx.trust();
-					assert_eq!(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1.as_ref().unwrap(), &preimage))[..],
+					assert_eq!(serialize(&trusted_tx.get_signed_htlc_tx(&channel_parameters, index, &(htlc_sig.0).1, (htlc_sig.1).1, &preimage))[..],
 							hex::decode($htlc_tx_hex).unwrap()[..], "htlc tx");
 				})*
-				loop {
-					let htlc_sig = htlc_sig_iter.next();
-					if htlc_sig.is_none() { break; }
-					assert!((htlc_sig.unwrap().1).1.is_none());
-				}
+				assert!(htlc_sig_iter.next().is_none());
 			} }
 		}
 

lightning/src/ln/onchaintx.rs

@@ -896,8 +896,6 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 	fn sign_latest_holder_htlcs(&mut self) {
 		if let Some(ref holder_commitment) = self.holder_commitment {
 			if let Ok(sigs) = self.key_storage.sign_holder_commitment_htlc_transactions(holder_commitment, &self.secp_ctx) {
-				// since we are no longer maintaining dust HTLCs info in OnchainTxHandler, all signatures must be present
-				let sigs: Vec<Signature> = sigs.iter().map(|o| o.unwrap()).collect();
 				self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
 			}
 		}
@@ -906,8 +904,6 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
 	fn sign_prev_holder_htlcs(&mut self) {
 		if let Some(ref holder_commitment) = self.prev_holder_commitment {
 			if let Ok(sigs) = self.key_storage.sign_holder_commitment_htlc_transactions(holder_commitment, &self.secp_ctx) {
-				// since we are no longer maintaining dust HTLCs info in OnchainTxHandler, all signatures must be present
-				let sigs: Vec<Signature> = sigs.iter().map(|o| o.unwrap()).collect();
 				self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
 			}
 		}

lightning/src/util/enforcing_trait_impls.rs

@@ -85,7 +85,7 @@ impl ChannelKeys for EnforcingChannelKeys {
 		Ok(self.inner.unsafe_sign_holder_commitment(holder_commitment_tx, secp_ctx).unwrap())
 	}
 
-	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Option<Signature>>, ()> {
+	fn sign_holder_commitment_htlc_transactions<T: secp256k1::Signing + secp256k1::Verification>(&self, holder_commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<T>) -> Result<Vec<Signature>, ()> {
 		let commitment_tx = self.verify_holder_commitment_tx(holder_commitment_tx, secp_ctx);
 		let commitment_txid = commitment_tx.txid();
 		let holder_csv = self.inner.counterparty_selected_contest_delay();