Allow BaseSign validation functions to return an Err

Author: devrandom

lightning/src/chain/keysinterface.rs

@@ -218,7 +218,7 @@ 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);
+	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) -> 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
@@ -239,7 +239,7 @@ pub trait BaseSign {
 	///
 	/// This is required in order for the signer to make sure that the state has moved
 	/// forward and it is safe to sign the next counterparty commitment.
-	fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey);
+	fn validate_counterparty_revocation(&self, idx: u64, secret: &SecretKey) -> Result<(), ()>;
 
 	/// Create a signatures for a holder's commitment transaction and its claiming HTLC transactions.
 	/// This will only ever be called with a non-revoked commitment_tx.  This will be called with the
@@ -573,7 +573,8 @@ impl BaseSign for InMemorySigner {
 		chan_utils::build_commitment_secret(&self.commitment_seed, idx)
 	}
 
-	fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction) {
+	fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction) -> Result<(), ()> {
+		Ok(())
 	}
 
 	fn pubkeys(&self) -> &ChannelPublicKeys { &self.holder_channel_pubkeys }
@@ -602,7 +603,8 @@ impl BaseSign for InMemorySigner {
 		Ok((commitment_sig, htlc_sigs))
 	}
 
-	fn validate_counterparty_revocation(&self, _idx: u64, _secret: &SecretKey) {
+	fn validate_counterparty_revocation(&self, _idx: u64, _secret: &SecretKey) -> Result<(), ()> {
+		Ok(())
 	}
 
 	fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {

lightning/src/ln/channel.rs

@@ -1791,7 +1791,8 @@ 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)
+			.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
 
 		// Now that we're past error-generating stuff, update our local state:
 
@@ -1867,7 +1868,9 @@ 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)
+			.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
+
 
 		let funding_redeemscript = self.get_funding_redeemscript();
 		let funding_txo = self.get_funding_txo().unwrap();
@@ -2505,7 +2508,8 @@ 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)
+			.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);
 
 		// Update state now that we've passed all the can-fail calls...
@@ -2770,7 +2774,8 @@ impl<Signer: Sign> Channel<Signer> {
 		self.holder_signer.validate_counterparty_revocation(
 			self.cur_counterparty_commitment_transaction_number + 1,
 			&secret
-		);
+		).map_err(|_| ChannelError::Close("Failed to validate revocation from peer".to_owned()))?;
+
 		self.commitment_secrets.provide_secret(self.cur_counterparty_commitment_transaction_number + 1, msg.per_commitment_secret)
 			.map_err(|_| ChannelError::Close("Previous secrets did not match new one".to_owned()))?;
 		self.latest_monitor_update_id += 1;

lightning/src/util/enforcing_trait_impls.rs

@@ -100,11 +100,12 @@ impl BaseSign for EnforcingSigner {
 		self.inner.release_commitment_secret(idx)
 	}
 
-	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) {
+	fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction) -> 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);
 		state.last_holder_commitment = idx;
+		Ok(())
 	}
 
 	fn pubkeys(&self) -> &ChannelPublicKeys { self.inner.pubkeys() }
@@ -129,10 +130,11 @@ impl BaseSign for EnforcingSigner {
 		Ok(self.inner.sign_counterparty_commitment(commitment_tx, secp_ctx).unwrap())
 	}
 
-	fn validate_counterparty_revocation(&self, idx: u64, _secret: &SecretKey) {
+	fn validate_counterparty_revocation(&self, idx: u64, _secret: &SecretKey) -> Result<(), ()> {
 		let mut state = self.state.lock().unwrap();
 		assert!(idx == state.last_counterparty_revoked_commitment || idx == state.last_counterparty_revoked_commitment - 1, "expecting to validate the current or next counterparty revocation - trying {}, current {}", idx, state.last_counterparty_revoked_commitment);
 		state.last_counterparty_revoked_commitment = idx;
+		Ok(())
 	}
 
 	fn sign_holder_commitment_and_htlcs(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {