From bee53fa04a7895ca53672675112e46d863929069 Mon Sep 17 00:00:00 2001
From: Devrandom <c1.devrandom@niftybox.net>
Date: Wed, 8 Jan 2020 11:22:45 -0800
Subject: [PATCH] Provide redeem scripts to sign_remote_commitment

---
 lightning/src/chain/keysinterface.rs        |  5 +-
 lightning/src/ln/channel.rs                 | 56 +++++++++++++--------
 lightning/src/util/enforcing_trait_impls.rs |  6 ++-
 lightning/src/util/transaction_utils.rs     | 10 ++++
 4 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs
index fae609cc..081ed7c3 100644
--- a/lightning/src/chain/keysinterface.rs
+++ b/lightning/src/chain/keysinterface.rs
@@ -142,7 +142,7 @@ pub trait ChannelKeys : Send {
 	/// 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_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()>;
+	fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>, redeem_scripts: &Vec<Script>) -> Result<(Signature, Vec<Signature>), ()>;
 
 	/// Create a signature for a (proposed) closing transaction.
 	///
@@ -184,8 +184,9 @@ impl ChannelKeys for InMemoryChannelKeys {
 	fn htlc_base_key(&self) -> &SecretKey { &self.htlc_base_key }
 	fn commitment_seed(&self) -> &[u8; 32] { &self.commitment_seed }
 
-	fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+	fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>, redeem_scripts: &Vec<Script>) -> Result<(Signature, Vec<Signature>), ()> {
 		if commitment_tx.input.len() != 1 { return Err(()); }
+		if commitment_tx.output.len() != redeem_scripts.len() { return Err(()); }
 		let commitment_sighash = hash_to_message!(&bip143::SighashComponents::new(&commitment_tx).sighash_all(&commitment_tx.input[0], &channel_funding_redeemscript, channel_value_satoshis)[..]);
 		let commitment_sig = secp_ctx.sign(&commitment_sighash, &self.funding_key);
 
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index bba65e52..241a8b0d 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -800,7 +800,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 	/// Note that below-dust HTLCs are included in the third return value, but not the second, and
 	/// sources are provided only for outbound HTLCs in the third return value.
 	#[inline]
-	fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>) {
+	fn build_commitment_transaction(&self, commitment_number: u64, keys: &TxCreationKeys, local: bool, generated_by_local: bool, feerate_per_kw: u64) -> (Transaction, usize, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, Vec<Script>) {
 		let obscured_commitment_transaction_number = self.get_commitment_transaction_number_obscure_factor() ^ (INITIAL_COMMITMENT_NUMBER - commitment_number);
 
 		let txins = {
@@ -814,7 +814,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			ins
 		};
 
-		let mut txouts: Vec<(TxOut, Option<(HTLCOutputInCommitment, Option<&HTLCSource>)>)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
+		let mut txouts: Vec<(TxOut, Option<(HTLCOutputInCommitment, Option<&HTLCSource>)>, Script)> = Vec::with_capacity(self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len() + 2);
 		let mut included_dust_htlcs: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::new();
 
 		let dust_limit_satoshis = if local { self.our_dust_limit_satoshis } else { self.their_dust_limit_satoshis };
@@ -842,10 +842,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 					let htlc_in_tx = get_htlc_in_commitment!($htlc, true);
 					if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_TIMEOUT_TX_WEIGHT / 1000) {
 						log_trace!(self, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+						let script = chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys);
 						txouts.push((TxOut {
-							script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
+							script_pubkey: script.to_v0_p2wsh(),
 							value: $htlc.amount_msat / 1000
-						}, Some((htlc_in_tx, $source))));
+						}, Some((htlc_in_tx, $source)), script));
 					} else {
 						log_trace!(self, "   ...including {} {} dust HTLC {} (hash {}) with value {} due to dust limit", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
 						included_dust_htlcs.push((htlc_in_tx, $source));
@@ -854,10 +855,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 					let htlc_in_tx = get_htlc_in_commitment!($htlc, false);
 					if $htlc.amount_msat / 1000 >= dust_limit_satoshis + (feerate_per_kw * HTLC_SUCCESS_TX_WEIGHT / 1000) {
 						log_trace!(self, "   ...including {} {} HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
+						let script = chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys);
 						txouts.push((TxOut { // "received HTLC output"
-							script_pubkey: chan_utils::get_htlc_redeemscript(&htlc_in_tx, &keys).to_v0_p2wsh(),
+							script_pubkey: script.to_v0_p2wsh(),
 							value: $htlc.amount_msat / 1000
-						}, Some((htlc_in_tx, $source))));
+						}, Some((htlc_in_tx, $source)), script));
 					} else {
 						log_trace!(self, "   ...including {} {} dust HTLC {} (hash {}) with value {}", if $outbound { "outbound" } else { "inbound" }, $state_name, $htlc.htlc_id, log_bytes!($htlc.payment_hash.0), $htlc.amount_msat);
 						included_dust_htlcs.push((htlc_in_tx, $source));
@@ -957,25 +959,29 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 
 		if value_to_a >= (dust_limit_satoshis as i64) {
 			log_trace!(self, "   ...including {} output with value {}", if local { "to_local" } else { "to_remote" }, value_to_a);
+			let script = chan_utils::get_revokeable_redeemscript(&keys.revocation_key,
+			                                                     if local { self.their_to_self_delay } else { self.our_to_self_delay },
+			                                                     &keys.a_delayed_payment_key);
 			txouts.push((TxOut {
-				script_pubkey: chan_utils::get_revokeable_redeemscript(&keys.revocation_key,
-				                                                       if local { self.their_to_self_delay } else { self.our_to_self_delay },
-				                                                       &keys.a_delayed_payment_key).to_v0_p2wsh(),
+				script_pubkey: script.to_v0_p2wsh(),
 				value: value_to_a as u64
-			}, None));
+			}, None, script));
 		}
 
 		if value_to_b >= (dust_limit_satoshis as i64) {
 			log_trace!(self, "   ...including {} output with value {}", if local { "to_remote" } else { "to_local" }, value_to_b);
+			let script = Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
+				.push_slice(&Hash160::hash(&keys.b_payment_key.serialize())[..])
+				.into_script();
+			// p2wpkh doesn't need a redeem script. Provide a placeholder.
+			let redeem_script = Builder::new().into_script();
 			txouts.push((TxOut {
-				script_pubkey: Builder::new().push_opcode(opcodes::all::OP_PUSHBYTES_0)
-				                             .push_slice(&Hash160::hash(&keys.b_payment_key.serialize())[..])
-				                             .into_script(),
+				script_pubkey: script,
 				value: value_to_b as u64
-			}, None));
+			}, None, redeem_script));
 		}
 
-		transaction_utils::sort_outputs(&mut txouts, |a, b| {
+		transaction_utils::sort_outputs2(&mut txouts, |a, b| {
 			if let &Some(ref a_htlc) = a {
 				if let &Some(ref b_htlc) = b {
 					a_htlc.0.cltv_expiry.cmp(&b_htlc.0.cltv_expiry)
@@ -990,9 +996,11 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		});
 
 		let mut outputs: Vec<TxOut> = Vec::with_capacity(txouts.len());
+		let mut scripts: Vec<Script> = Vec::with_capacity(txouts.len());
 		let mut htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)> = Vec::with_capacity(txouts.len() + included_dust_htlcs.len());
 		for (idx, mut out) in txouts.drain(..).enumerate() {
 			outputs.push(out.0);
+			scripts.push(out.2);
 			if let Some((mut htlc, source_option)) = out.1.take() {
 				htlc.transaction_output_index = Some(idx as u32);
 				htlcs_included.push((htlc, source_option));
@@ -1006,7 +1014,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 			lock_time: ((0x20 as u32) << 8*3) | ((obscured_commitment_transaction_number & 0xffffffu64) as u32),
 			input: txins,
 			output: outputs,
-		}, non_dust_htlc_count, htlcs_included)
+		}, non_dust_htlc_count, htlcs_included, scripts)
 	}
 
 	#[inline]
@@ -1423,8 +1431,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		let localtx = LocalCommitmentTransaction::new_missing_local_sig(local_initial_commitment_tx, sig, &PublicKey::from_secret_key(&self.secp_ctx, self.local_keys.funding_key()), self.their_funding_pubkey.as_ref().unwrap());
 
 		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).0;
-		let remote_signature = self.local_keys.sign_remote_commitment(self.channel_value_satoshis, &self.get_funding_redeemscript(), self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), self.our_to_self_delay, &self.secp_ctx)
+		let commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw);
+		let remote_initial_commitment_tx = commitment_tx.0;
+		let scripts = commitment_tx.3;
+		let remote_signature = self.local_keys.sign_remote_commitment(self.channel_value_satoshis, &self.get_funding_redeemscript(), self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), self.our_to_self_delay, &self.secp_ctx, &scripts)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?.0;
 
 		// We sign the "remote" commitment transaction, allowing them to broadcast the tx if they wish.
@@ -1743,7 +1753,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 		let mut local_commitment_tx = {
 			let mut commitment_tx = self.build_commitment_transaction(self.cur_local_commitment_transaction_number, &local_keys, true, false, feerate_per_kw);
 			let htlcs_cloned: Vec<_> = commitment_tx.2.drain(..).map(|htlc| (htlc.0, htlc.1.map(|h| h.clone()))).collect();
-			(commitment_tx.0, commitment_tx.1, htlcs_cloned)
+			(commitment_tx.0, commitment_tx.1, htlcs_cloned, commitment_tx.3)
 		};
 		let local_commitment_txid = local_commitment_tx.0.txid();
 		let local_sighash = hash_to_message!(&bip143::SighashComponents::new(&local_commitment_tx.0).sighash_all(&local_commitment_tx.0.input[0], &funding_script, self.channel_value_satoshis)[..]);
@@ -3149,8 +3159,10 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 	/// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
 	fn get_outbound_funding_created_signature(&mut self) -> Result<(Signature, Transaction), ChannelError> {
 		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).0;
-		Ok((self.local_keys.sign_remote_commitment(self.channel_value_satoshis, &self.get_funding_redeemscript(), self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), self.our_to_self_delay, &self.secp_ctx)
+		let commitment_tx = self.build_commitment_transaction(self.cur_remote_commitment_transaction_number, &remote_keys, false, false, self.feerate_per_kw);
+		let remote_initial_commitment_tx = commitment_tx.0;
+		let scripts = commitment_tx.3;
+		Ok((self.local_keys.sign_remote_commitment(self.channel_value_satoshis, &self.get_funding_redeemscript(), self.feerate_per_kw, &remote_initial_commitment_tx, &remote_keys, &Vec::new(), self.our_to_self_delay, &self.secp_ctx, &scripts)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?.0, remote_initial_commitment_tx))
 	}
 
@@ -3458,7 +3470,7 @@ impl<ChanSigner: ChannelKeys> Channel<ChanSigner> {
 				htlcs.push(htlc);
 			}
 
-			let res = self.local_keys.sign_remote_commitment(self.channel_value_satoshis, &self.get_funding_redeemscript(), feerate_per_kw, &remote_commitment_tx.0, &remote_keys, &htlcs, self.our_to_self_delay, &self.secp_ctx)
+			let res = self.local_keys.sign_remote_commitment(self.channel_value_satoshis, &self.get_funding_redeemscript(), feerate_per_kw, &remote_commitment_tx.0, &remote_keys, &htlcs, self.our_to_self_delay, &self.secp_ctx, &remote_commitment_tx.3)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed"))?;
 			signature = res.0;
 			htlc_signatures = res.1;
diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs
index e9c82c1b..1586af1b 100644
--- a/lightning/src/util/enforcing_trait_impls.rs
+++ b/lightning/src/util/enforcing_trait_impls.rs
@@ -35,8 +35,10 @@ 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 sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_script: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>) -> Result<(Signature, Vec<Signature>), ()> {
+	fn sign_remote_commitment<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_script: &Script, feerate_per_kw: u64, commitment_tx: &Transaction, keys: &TxCreationKeys, htlcs: &[&HTLCOutputInCommitment], to_self_delay: u16, secp_ctx: &Secp256k1<T>, redeem_scripts: &Vec<Script>) -> Result<(Signature, Vec<Signature>), ()> {
 		if commitment_tx.input.len() != 1 { panic!(); }
+		if commitment_tx.output.len() != redeem_scripts.len() { panic!(); }
+
 		let obscured_commitment_transaction_number = (commitment_tx.lock_time & 0xffffff) as u64 | ((commitment_tx.input[0].sequence as u64 & 0xffffff) << 3*8);
 
 		{
@@ -49,7 +51,7 @@ impl ChannelKeys for EnforcingChannelKeys {
 			commitment_data.1 = cmp::max(commitment_number, commitment_data.1)
 		}
 
-		Ok(self.inner.sign_remote_commitment(channel_value_satoshis, channel_funding_script, feerate_per_kw, commitment_tx, keys, htlcs, to_self_delay, secp_ctx).unwrap())
+		Ok(self.inner.sign_remote_commitment(channel_value_satoshis, channel_funding_script, feerate_per_kw, commitment_tx, keys, htlcs, to_self_delay, secp_ctx, redeem_scripts).unwrap())
 	}
 
 	fn sign_closing_transaction<T: secp256k1::Signing>(&self, channel_value_satoshis: u64, channel_funding_redeemscript: &Script, closing_tx: &Transaction, secp_ctx: &Secp256k1<T>) -> Result<Signature, ()> {
diff --git a/lightning/src/util/transaction_utils.rs b/lightning/src/util/transaction_utils.rs
index a7c1e6bc..95d14236 100644
--- a/lightning/src/util/transaction_utils.rs
+++ b/lightning/src/util/transaction_utils.rs
@@ -12,6 +12,16 @@ pub fn sort_outputs<T, C : Fn(&T, &T) -> Ordering>(outputs: &mut Vec<(TxOut, T)>
 	});
 }
 
+pub fn sort_outputs2<T, T1, C : Fn(&T, &T) -> Ordering>(outputs: &mut Vec<(TxOut, T, T1)>, tie_breaker: C) {
+	outputs.sort_unstable_by(|a, b| {
+		a.0.value.cmp(&b.0.value).then_with(|| {
+			a.0.script_pubkey[..].cmp(&b.0.script_pubkey[..]).then_with(|| {
+				tie_breaker(&a.1, &b.1)
+			})
+		})
+	});
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;
-- 
2.30.2