Provide inbound HTLC preimages to the EcdsaChannelSigner

The VLS signer has a desire to see preimages for resolved forwarded
HTLCs when they are first claimed by us, even if that claim was for
the inbound edge (where claiming strictly increases our balance).

Luckily, providing that information is rather trivial, which we do
here.

Fixes #2356

Author: TheBlueMatt

lightning/src/ln/channel.rs

@@ -479,7 +479,8 @@ 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
+	outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
+	inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
 }
 
 /// Used when calculating whether we or the remote can afford an additional HTLC.
@@ -1393,6 +1394,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			}
 		}
 
+		let mut inbound_htlc_preimages: Vec<PaymentPreimage> = Vec::new();
+
 		for ref htlc in self.pending_inbound_htlcs.iter() {
 			let (include, state_name) = match htlc.state {
 				InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
@@ -1410,7 +1413,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 				match &htlc.state {
 					&InboundHTLCState::LocalRemoved(ref reason) => {
 						if generated_by_local {
-							if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
+							if let &InboundHTLCRemovalReason::Fulfill(preimage) = reason {
+								inbound_htlc_preimages.push(preimage);
 								value_to_self_msat_offset += htlc.amount_msat as i64;
 							}
 						}
@@ -1420,7 +1424,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			}
 		}
 
-		let mut preimages: Vec<PaymentPreimage> = Vec::new();
+
+		let mut outbound_htlc_preimages: Vec<PaymentPreimage> = Vec::new();
 
 		for ref htlc in self.pending_outbound_htlcs.iter() {
 			let (include, state_name) = match htlc.state {
@@ -1439,7 +1444,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			};
 
 			if let Some(preimage) = preimage_opt {
-				preimages.push(preimage);
+				outbound_htlc_preimages.push(preimage);
 			}
 
 			if include {
@@ -1545,7 +1550,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 			htlcs_included,
 			local_balance_msat: value_to_self_msat as u64,
 			remote_balance_msat: value_to_remote_msat as u64,
-			preimages
+			inbound_htlc_preimages,
+			outbound_htlc_preimages,
 		}
 	}
 
@@ -3208,7 +3214,7 @@ impl<SP: Deref> Channel<SP> where
 			self.context.counterparty_funding_pubkey()
 		);
 
-		self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.preimages)
+		self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.outgoing_htlc_preimages)
 			.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
 
 		// Update state now that we've passed all the can-fail calls...
@@ -5709,8 +5715,12 @@ impl<SP: Deref> Channel<SP> where
 						htlcs.push(htlc);
 					}
 
-					let res = ecdsa.sign_counterparty_commitment(&commitment_stats.tx, commitment_stats.preimages, &self.context.secp_ctx)
-						.map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?;
+					let res = ecdsa.sign_counterparty_commitment(
+							&commitment_stats.tx,
+							commitment_stats.incoming_htlc_preimages,
+							commitment_stats.outgoing_htlc_preimages,
+							&self.context.secp_ctx,
+						).map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?;
 					signature = res.0;
 					htlc_signatures = res.1;
 

lightning/src/sign/ecdsa.rs

@@ -29,16 +29,18 @@ pub trait EcdsaChannelSigner: ChannelSigner {
 	/// 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.
+	/// The preimages of outgoing and incoming HTLCs that were fulfilled since the last commitment
+	/// are provided. A validating signer should ensure that an outgoing HTLC output is removed
+	/// only when the matching preimage is provided and after the corresponding incoming HTLC has
+	/// been removed for forwarded payments.
 	///
 	/// Note that 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,
-		preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>
+		incoming_preimages: Vec<PaymentPreimage>, outgoing_preimages: Vec<PaymentPreimage>,
+		secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<(Signature, Vec<Signature>), ()>;
 	/// Validate the counterparty's revocation.
 	///

lightning/src/sign/taproot.rs

@@ -27,16 +27,18 @@ pub trait TaprootChannelSigner: ChannelSigner {
 	/// 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.
+	/// The preimages of outgoing and incoming HTLCs that were fulfilled since the last commitment
+	/// are provided. A validating signer should ensure that an outgoing HTLC output is removed
+	/// only when the matching preimage is provided and after the corresponding incoming HTLC has
+	/// been removed for forwarded payments.
 	///
 	/// Note that 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 partially_sign_counterparty_commitment(&self, counterparty_nonce: PublicNonce,
-		commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>,
+		commitment_tx: &CommitmentTransaction,
+		incoming_preimages: Vec<PaymentPreimage>, outgoing_preimages: Vec<PaymentPreimage>,
 		secp_ctx: &Secp256k1<secp256k1::All>,
 	) -> Result<(PartialSignatureWithNonce, Vec<Signature>), ()>;