Handle fallible `release_commitment_secret`

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index be9d2d3ea5893a6a67e37a70e57807aedd0bdc3b..5170c8e6b5fa97ea47da8ce082f662e1e01fb003 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -5500,32 +5500,42 @@ impl<SP: Deref> Channel<SP> where
        fn get_last_revoke_and_ack<L: Deref>(&mut self, logger: &L) -> Option<msgs::RevokeAndACK> where L::Target: Logger {
                debug_assert!(self.context.holder_commitment_point.transaction_number() <= INITIAL_COMMITMENT_NUMBER - 2);
                self.context.holder_commitment_point.try_resolve_pending(&self.context.holder_signer, &self.context.secp_ctx, logger);
-               let per_commitment_secret = self.context.holder_signer.as_ref().release_commitment_secret(self.context.holder_commitment_point.transaction_number() + 2);
-               if let HolderCommitmentPoint::Available { transaction_number: _, current, next: _ } = self.context.holder_commitment_point {
+               let per_commitment_secret = self.context.holder_signer.as_ref()
+                       .release_commitment_secret(self.context.holder_commitment_point.transaction_number() + 2).ok();
+               if let (HolderCommitmentPoint::Available { current, .. }, Some(per_commitment_secret)) =
+                       (self.context.holder_commitment_point, per_commitment_secret) {
                        self.context.signer_pending_revoke_and_ack = false;
-                       Some(msgs::RevokeAndACK {
+                       return Some(msgs::RevokeAndACK {
                                channel_id: self.context.channel_id,
                                per_commitment_secret,
                                next_per_commitment_point: current,
                                #[cfg(taproot)]
                                next_local_nonce: None,
                        })
-               } else {
-                       #[cfg(not(async_signing))] {
-                               panic!("Holder commitment point must be Available when generating revoke_and_ack");
-                       }
-                       #[cfg(async_signing)] {
-                               // Technically if we're at HolderCommitmentPoint::PendingNext,
-                               // we have a commitment point ready to send in an RAA, however we
-                               // choose to wait since if we send RAA now, we could get another
-                               // CS before we have any commitment point available. Blocking our
-                               // RAA here is a convenient way to make sure that post-funding
-                               // we're only ever waiting on one commitment point at a time.
-                               log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
-                                       &self.context.channel_id(), self.context.holder_commitment_point.transaction_number());
-                               self.context.signer_pending_revoke_and_ack = true;
-                               None
-                       }
+               }
+               if !self.context.holder_commitment_point.is_available() {
+                       log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
+                               &self.context.channel_id(), self.context.holder_commitment_point.transaction_number());
+               }
+               if per_commitment_secret.is_none() {
+                       log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment secret for {} is not available",
+                               &self.context.channel_id(), self.context.holder_commitment_point.transaction_number(),
+                               self.context.holder_commitment_point.transaction_number() + 2);
+               }
+               #[cfg(not(async_signing))] {
+                       panic!("Holder commitment point and per commitment secret must be available when generating revoke_and_ack");
+               }
+               #[cfg(async_signing)] {
+                       // Technically if we're at HolderCommitmentPoint::PendingNext,
+                       // we have a commitment point ready to send in an RAA, however we
+                       // choose to wait since if we send RAA now, we could get another
+                       // CS before we have any commitment point available. Blocking our
+                       // RAA here is a convenient way to make sure that post-funding
+                       // we're only ever waiting on one commitment point at a time.
+                       log_trace!(logger, "Last revoke-and-ack pending in channel {} for sequence {} because the next per-commitment point is not available",
+                               &self.context.channel_id(), self.context.holder_commitment_point.transaction_number());
+                       self.context.signer_pending_revoke_and_ack = true;
+                       None
                }
        }
 

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index 6a0343fb53379e40a5d05c47ce71cd37b8c90fca..0694dac994e9cdeb758330f56a6e1dcc484cba59 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -1474,7 +1474,7 @@ fn test_fee_spike_violation_fails_htlc() {
 
                let pubkeys = chan_signer.as_ref().pubkeys();
                (pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
-                chan_signer.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER),
+                chan_signer.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER).unwrap(),
                 chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx).unwrap(),
                 chan_signer.as_ref().pubkeys().funding_pubkey)
        };
@@ -7860,15 +7860,15 @@ fn test_counterparty_raa_skip_no_crash() {
 
                // Make signer believe we got a counterparty signature, so that it allows the revocation
                keys.as_ecdsa().unwrap().get_enforcement_state().last_holder_commitment -= 1;
-               per_commitment_secret = keys.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER);
+               per_commitment_secret = keys.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER).unwrap();
 
                // Must revoke without gaps
                keys.as_ecdsa().unwrap().get_enforcement_state().last_holder_commitment -= 1;
-               keys.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1);
+               keys.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 1).unwrap();
 
                keys.as_ecdsa().unwrap().get_enforcement_state().last_holder_commitment -= 1;
                next_per_commitment_point = PublicKey::from_secret_key(&Secp256k1::new(),
-                       &SecretKey::from_slice(&keys.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2)).unwrap());
+                       &SecretKey::from_slice(&keys.as_ref().release_commitment_secret(INITIAL_COMMITMENT_NUMBER - 2).unwrap()).unwrap());
        }
 
        nodes[1].node.handle_revoke_and_ack(&nodes[0].node.get_our_node_id(),

diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
index d684b4fe68073ab526552d0ff582d16405d1c4ba..5f5ce901e304b75f8c758207057e3c87a5967677 100644 (file)
--- a/lightning/src/sign/mod.rs
+++ b/lightning/src/sign/mod.rs
@@ -752,7 +752,7 @@ pub trait ChannelSigner {
        ///
        /// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards.
        // TODO: return a Result so we can signal a validation error
-       fn release_commitment_secret(&self, idx: u64) -> [u8; 32];
+       fn release_commitment_secret(&self, idx: u64) -> Result<[u8; 32], ()>;
 
        /// Validate the counterparty's signatures on the holder commitment transaction and HTLCs.
        ///
@@ -1361,8 +1361,8 @@ impl ChannelSigner for InMemorySigner {
                Ok(PublicKey::from_secret_key(secp_ctx, &commitment_secret))
        }
 
-       fn release_commitment_secret(&self, idx: u64) -> [u8; 32] {
-               chan_utils::build_commitment_secret(&self.commitment_seed, idx)
+       fn release_commitment_secret(&self, idx: u64) -> Result<[u8; 32], ()> {
+               Ok(chan_utils::build_commitment_secret(&self.commitment_seed, idx))
        }
 
        fn validate_holder_commitment(

diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs
index 13dfd6a83530930d37bfa179ab63b9ea576a7ec9..fb20528f9eebbe4eb90d295184c16191ed5f9c4e 100644 (file)
--- a/lightning/src/util/test_channel_signer.rs
+++ b/lightning/src/util/test_channel_signer.rs
@@ -172,7 +172,7 @@ impl ChannelSigner for TestChannelSigner {
                self.inner.get_per_commitment_point(idx, secp_ctx)
        }
 
-       fn release_commitment_secret(&self, idx: u64) -> [u8; 32] {
+       fn release_commitment_secret(&self, idx: u64) -> Result<[u8; 32], ()> {
                {
                        let mut state = self.state.lock().unwrap();
                        assert!(idx == state.last_holder_revoked_commitment || idx == state.last_holder_revoked_commitment - 1, "can only revoke the current or next unrevoked commitment - trying {}, last revoked {}", idx, state.last_holder_revoked_commitment);