Provide preimages to signer

Author: devrandom

lightning/src/chain/keysinterface.rs

@@ -34,7 +34,7 @@ use util::{byte_utils, transaction_utils};
 use util::ser::{Writeable, Writer, Readable};
 
 use chain::transaction::OutPoint;
-use ln::chan_utils;
+use ln::{chan_utils, PaymentPreimage};
 use ln::chan_utils::{HTLCOutputInCommitment, make_funding_redeemscript, ChannelPublicKeys, HolderCommitmentTransaction, ChannelTransactionParameters, CommitmentTransaction, ClosingTransaction};
 use ln::msgs::UnsignedChannelAnnouncement;
 use ln::script::ShutdownScript;
@@ -226,7 +226,14 @@ pub trait BaseSign {
 	/// secret won't leave us without a broadcastable holder transaction.
 	/// Policy checks should be implemented in this function, including checking the amount
 	/// sent to us and checking the HTLCs.
-	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) -> Result<(), ()>;
+	///
+	/// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
+	/// A validating signer should ensure that an HTLC output is removed only when the matching
+	/// preimage is provided, or when the value to holder is restored.
+	///
+	/// NOTE: all the relevant preimages will be provided, but there may also be additional
+	/// irrelevant or duplicate preimages.
+	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, preimages: Vec<PaymentPreimage>) -> Result<(), ()>;
 	/// Gets the holder's channel public keys and basepoints
 	fn pubkeys(&self) -> &ChannelPublicKeys;
 	/// Gets an arbitrary identifier describing the set of keys which are provided back to you in
@@ -240,9 +247,16 @@ pub trait BaseSign {
 	///
 	/// Policy checks should be implemented in this function, including checking the amount
 	/// sent to us and checking the HTLCs.
+	///
+	/// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
+	/// A validating signer should ensure that an HTLC output is removed only when the matching
+	/// preimage is provided, or when the value to holder is restored.
+	///
+	/// NOTE: all the relevant preimages will be provided, but there may also be additional
+	/// irrelevant or duplicate preimages.
 	//
 	// TODO: Document the things someone using this interface should enforce before signing.
-	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
+	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()>;
 	/// Validate the counterparty's revocation.
 	///
 	/// This is required in order for the signer to make sure that the state has moved
@@ -601,14 +615,14 @@ impl BaseSign for InMemorySigner {
 		chan_utils::build_commitment_secret(&self.commitment_seed, idx)
 	}
 
-	fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction) -> Result<(), ()> {
+	fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction, _preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
 		Ok(())
 	}
 
 	fn pubkeys(&self) -> &ChannelPublicKeys { &self.holder_channel_pubkeys }
 	fn channel_keys_id(&self) -> [u8; 32] { self.channel_keys_id }
 
-	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, _preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
 		let trusted_tx = commitment_tx.trust();
 		let keys = trusted_tx.keys();
 

lightning/src/ln/channel.rs

@@ -330,6 +330,7 @@ struct CommitmentStats<'a> {
 	htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
 	local_balance_msat: u64, // local balance before fees but considering dust limits
 	remote_balance_msat: u64, // remote balance before fees but considering dust limits
+	preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
 }
 
 /// Used when calculating whether we or the remote can afford an additional HTLC.
@@ -1298,6 +1299,8 @@ impl<Signer: Sign> Channel<Signer> {
 			}
 		}
 
+		let mut preimages: Vec<PaymentPreimage> = Vec::new();
+
 		for ref htlc in self.pending_outbound_htlcs.iter() {
 			let (include, state_name) = match htlc.state {
 				OutboundHTLCState::LocalAnnounced(_) => (generated_by_local, "LocalAnnounced"),
@@ -1307,6 +1310,17 @@ impl<Signer: Sign> Channel<Signer> {
 				OutboundHTLCState::AwaitingRemovedRemoteRevoke(_) => (false, "AwaitingRemovedRemoteRevoke"),
 			};
 
+			let preimage_opt = match htlc.state {
+				OutboundHTLCState::RemoteRemoved(OutboundHTLCOutcome::Success(p)) => p,
+				OutboundHTLCState::AwaitingRemoteRevokeToRemove(OutboundHTLCOutcome::Success(p)) => p,
+				OutboundHTLCState::AwaitingRemovedRemoteRevoke(OutboundHTLCOutcome::Success(p)) => p,
+				_ => None,
+			};
+
+			if let Some(preimage) = preimage_opt {
+				preimages.push(preimage);
+			}
+
 			if include {
 				add_htlc_output!(htlc, true, Some(&htlc.source), state_name);
 				local_htlc_total_msat += htlc.amount_msat;
@@ -1411,6 +1425,7 @@ impl<Signer: Sign> Channel<Signer> {
 			htlcs_included,
 			local_balance_msat: value_to_self_msat as u64,
 			remote_balance_msat: value_to_remote_msat as u64,
+			preimages
 		}
 	}
 
@@ -1882,7 +1897,7 @@ impl<Signer: Sign> Channel<Signer> {
 		log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
 			log_bytes!(self.channel_id()), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
 
-		let counterparty_signature = self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx)
+		let counterparty_signature = self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0;
 
 		// We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
@@ -1936,7 +1951,7 @@ impl<Signer: Sign> Channel<Signer> {
 			self.counterparty_funding_pubkey()
 		);
 
-		self.holder_signer.validate_holder_commitment(&holder_commitment_tx)
+		self.holder_signer.validate_holder_commitment(&holder_commitment_tx, Vec::new())
 			.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
 
 		// Now that we're past error-generating stuff, update our local state:
@@ -2013,7 +2028,7 @@ impl<Signer: Sign> Channel<Signer> {
 			self.counterparty_funding_pubkey()
 		);
 
-		self.holder_signer.validate_holder_commitment(&holder_commitment_tx)
+		self.holder_signer.validate_holder_commitment(&holder_commitment_tx, Vec::new())
 			.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
 
 
@@ -2682,7 +2697,7 @@ impl<Signer: Sign> Channel<Signer> {
 		);
 
 		let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number - 1, &self.secp_ctx);
-		self.holder_signer.validate_holder_commitment(&holder_commitment_tx)
+		self.holder_signer.validate_holder_commitment(&holder_commitment_tx, commitment_stats.preimages)
 			.map_err(|_| (None, ChannelError::Close("Failed to validate our commitment".to_owned())))?;
 		let per_commitment_secret = self.holder_signer.release_commitment_secret(self.cur_holder_commitment_transaction_number + 1);
 
@@ -4529,7 +4544,7 @@ impl<Signer: Sign> Channel<Signer> {
 	fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
 		let counterparty_keys = self.build_remote_transaction_keys()?;
 		let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
-		Ok(self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, &self.secp_ctx)
+		Ok(self.holder_signer.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0)
 	}
 
@@ -4994,7 +5009,7 @@ impl<Signer: Sign> Channel<Signer> {
 				htlcs.push(htlc);
 			}
 
-			let res = self.holder_signer.sign_counterparty_commitment(&commitment_stats.tx, &self.secp_ctx)
+			let res = self.holder_signer.sign_counterparty_commitment(&commitment_stats.tx, commitment_stats.preimages, &self.secp_ctx)
 				.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?;
 			signature = res.0;
 			htlc_signatures = res.1;

lightning/src/ln/functional_tests.rs

@@ -730,7 +730,7 @@ fn test_update_fee_that_funder_cannot_afford() {
 			&mut htlcs,
 			&local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
 		);
-		local_chan_signer.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
+		local_chan_signer.sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap()
 	};
 
 	let commit_signed_msg = msgs::CommitmentSigned {
@@ -1466,7 +1466,7 @@ fn test_fee_spike_violation_fails_htlc() {
 			&mut vec![(accepted_htlc_info, ())],
 			&local_chan.channel_transaction_parameters.as_counterparty_broadcastable()
 		);
-		local_chan_signer.sign_counterparty_commitment(&commitment_tx, &secp_ctx).unwrap()
+		local_chan_signer.sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap()
 	};
 
 	let commit_signed_msg = msgs::CommitmentSigned {

lightning/src/util/enforcing_trait_impls.rs

@@ -8,7 +8,7 @@
 // licenses.
 
 use ln::chan_utils::{HTLCOutputInCommitment, ChannelPublicKeys, HolderCommitmentTransaction, CommitmentTransaction, ChannelTransactionParameters, TrustedCommitmentTransaction, ClosingTransaction};
-use ln::{chan_utils, msgs};
+use ln::{chan_utils, msgs, PaymentPreimage};
 use chain::keysinterface::{Sign, InMemorySigner, BaseSign};
 
 use prelude::*;
@@ -102,7 +102,7 @@ impl BaseSign for EnforcingSigner {
 		self.inner.release_commitment_secret(idx)
 	}
 
-	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) -> Result<(), ()> {
+	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, _preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
 		let mut state = self.state.lock().unwrap();
 		let idx = holder_tx.commitment_number();
 		assert!(idx == state.last_holder_commitment || idx == state.last_holder_commitment - 1, "expecting to validate the current or next holder commitment - trying {}, current {}", idx, state.last_holder_commitment);
@@ -113,7 +113,7 @@ impl BaseSign for EnforcingSigner {
 	fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
 	fn channel_keys_id(&self) -> [u8; 32] { self.inner.channel_keys_id() }
 
-	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
+	fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
 		self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx);
 
 		{
@@ -129,7 +129,7 @@ impl BaseSign for EnforcingSigner {
 			state.last_counterparty_commitment = cmp::min(last_commitment_number, actual_commitment_number)
 		}
 
-		Ok(self.inner.sign_counterparty_commitment(commitment_tx, secp_ctx).unwrap())
+		Ok(self.inner.sign_counterparty_commitment(commitment_tx, preimages, secp_ctx).unwrap())
 	}
 
 	fn validate_counterparty_revocation(&self, idx: u64, _secret: &SecretKey) -> Result<(), ()> {