f return witness instead of modifying the transaction

Author: TheBlueMatt

lightning/src/chain/keysinterface.rs

@@ -503,13 +503,11 @@ impl InMemoryChannelKeys {
 	}
 
 	/// Sign the single input of spend_tx at index `input_idx` which spends the output
-	/// described by descriptor.
+	/// described by descriptor, returning the witness stack for the input.
 	///
 	/// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
 	/// or is not spending the outpoint described by `descriptor.outpoint`.
-	///
-	/// (C-not exported) as bindings don't support modifying a Transaction parameter
-	pub fn sign_counterparty_payment_input<C: Signing>(&self, spend_tx: &mut Transaction, input_idx: usize, descriptor: &StaticCounterpartyPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<(), ()> {
+	pub fn sign_counterparty_payment_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &StaticCounterpartyPaymentOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
 		// TODO: We really should be taking the SigHashCache as a parameter here instead of
 		// spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
 		// so that we can check them. This requires upstream rust-bitcoin changes (as well as
@@ -520,23 +518,23 @@ impl InMemoryChannelKeys {
 
 		let remotepubkey = self.pubkeys().payment_point;
 		let witness_script = bitcoin::Address::p2pkh(&::bitcoin::PublicKey{compressed: true, key: remotepubkey}, Network::Testnet).script_pubkey();
-		let sighash = hash_to_message!(&bip143::SigHashCache::new(&*spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
+		let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
 		let remotesig = secp_ctx.sign(&sighash, &self.payment_key);
-		spend_tx.input[input_idx].witness.push(remotesig.serialize_der().to_vec());
-		spend_tx.input[input_idx].witness[0].push(SigHashType::All as u8);
-		spend_tx.input[input_idx].witness.push(remotepubkey.serialize().to_vec());
-		Ok(())
+
+		let mut witness = Vec::with_capacity(2);
+		witness.push(remotesig.serialize_der().to_vec());
+		witness[0].push(SigHashType::All as u8);
+		witness.push(remotepubkey.serialize().to_vec());
+		Ok(witness)
 	}
 
 	/// Sign the single input of spend_tx at index `input_idx` which spends the output
-	/// described by descriptor.
+	/// described by descriptor, returning the witness stack for the input.
 	///
 	/// Returns an Err if the input at input_idx does not exist, has a non-empty script_sig,
 	/// is not spending the outpoint described by `descriptor.outpoint`, or does not have a
 	/// sequence set to `descriptor.to_self_delay`.
-	///
-	/// (C-not exported) as bindings don't support modifying a Transaction parameter
-	pub fn sign_dynamic_p2wsh_input<C: Signing>(&self, spend_tx: &mut Transaction, input_idx: usize, descriptor: &DynamicP2WSHOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<(), ()> {
+	pub fn sign_dynamic_p2wsh_input<C: Signing>(&self, spend_tx: &Transaction, input_idx: usize, descriptor: &DynamicP2WSHOutputDescriptor, secp_ctx: &Secp256k1<C>) -> Result<Vec<Vec<u8>>, ()> {
 		// TODO: We really should be taking the SigHashCache as a parameter here instead of
 		// spend_tx, but ideally the SigHashCache would expose the transaction's inputs read-only
 		// so that we can check them. This requires upstream rust-bitcoin changes (as well as
@@ -550,13 +548,15 @@ impl InMemoryChannelKeys {
 			.expect("We constructed the payment_base_key, so we can only fail here if the RNG is busted.");
 		let delayed_payment_pubkey = PublicKey::from_secret_key(&secp_ctx, &delayed_payment_key);
 		let witness_script = chan_utils::get_revokeable_redeemscript(&descriptor.revocation_pubkey, descriptor.to_self_delay, &delayed_payment_pubkey);
-		let sighash = hash_to_message!(&bip143::SigHashCache::new(&*spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
+		let sighash = hash_to_message!(&bip143::SigHashCache::new(spend_tx).signature_hash(input_idx, &witness_script, descriptor.output.value, SigHashType::All)[..]);
 		let local_delayedsig = secp_ctx.sign(&sighash, &delayed_payment_key);
-		spend_tx.input[input_idx].witness.push(local_delayedsig.serialize_der().to_vec());
-		spend_tx.input[input_idx].witness[0].push(SigHashType::All as u8);
-		spend_tx.input[input_idx].witness.push(vec!()); //MINIMALIF
-		spend_tx.input[input_idx].witness.push(witness_script.clone().into_bytes());
-		Ok(())
+
+		let mut witness = Vec::with_capacity(3);
+		witness.push(local_delayedsig.serialize_der().to_vec());
+		witness[0].push(SigHashType::All as u8);
+		witness.push(vec!()); //MINIMALIF
+		witness.push(witness_script.clone().into_bytes());
+		Ok(witness)
 	}
 }
 
@@ -977,15 +977,15 @@ impl KeysManager {
 							self.derive_channel_keys(descriptor.channel_value_satoshis, descriptor.key_derivation_params.0, descriptor.key_derivation_params.1),
 							descriptor.key_derivation_params));
 					}
-					keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&mut spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
+					spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_counterparty_payment_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
 				},
 				SpendableOutputDescriptor::DynamicOutputP2WSH(descriptor) => {
 					if keys_cache.is_none() || keys_cache.as_ref().unwrap().1 != descriptor.key_derivation_params {
 						keys_cache = Some((
 							self.derive_channel_keys(descriptor.channel_value_satoshis, descriptor.key_derivation_params.0, descriptor.key_derivation_params.1),
 							descriptor.key_derivation_params));
 					}
-					keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&mut spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
+					spend_tx.input[input_idx].witness = keys_cache.as_ref().unwrap().0.sign_dynamic_p2wsh_input(&spend_tx, input_idx, &descriptor, &secp_ctx).unwrap();
 				},
 				SpendableOutputDescriptor::StaticOutput { ref output, .. } => {
 					let derivation_idx = if output.script_pubkey == self.destination_script {