From 608ed12a5b0a457ecd5f0b0f5f52bd6e351e4dce Mon Sep 17 00:00:00 2001 From: Devrandom Date: Fri, 20 Aug 2021 21:57:18 +0200 Subject: [PATCH] Allow BaseSign validation functions to return an Err --- lightning/src/chain/keysinterface.rs | 10 ++++++---- lightning/src/ln/channel.rs | 13 +++++++++---- lightning/src/util/enforcing_trait_impls.rs | 6 ++++-- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/lightning/src/chain/keysinterface.rs b/lightning/src/chain/keysinterface.rs index ee7d4d659..44a03d09f 100644 --- a/lightning/src/chain/keysinterface.rs +++ b/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) -> Result<(Signature, Vec), ()> { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9725650f9..e2a19618b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1791,7 +1791,8 @@ impl Channel { 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 Channel { 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 Channel { ); 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 Channel { 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; diff --git a/lightning/src/util/enforcing_trait_impls.rs b/lightning/src/util/enforcing_trait_impls.rs index 7daacf363..b7dfe7670 100644 --- a/lightning/src/util/enforcing_trait_impls.rs +++ b/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) -> Result<(Signature, Vec), ()> { -- 2.39.5