Change get_per_commitment_point to return result type

Includes simple changes to test util signers and tests, as well as
handling the error case for get_per_commitment_point in
HolderCommitmentPoint. This leaves a couple `.expect`s in places
that will be addressed in a separate PR for handling funding.

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 6d72bf0eb9a979484669d1b7a3d519212da4214f..9a824b2d6a67d09b7c0a2159720b78a8f5de186e 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -962,8 +962,12 @@ impl HolderCommitmentPoint {
        {
                HolderCommitmentPoint::Available {
                        transaction_number: INITIAL_COMMITMENT_NUMBER,
-                       current: signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER, secp_ctx),
-                       next: signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, secp_ctx),
+                       // TODO(async_signing): remove this expect with the Uninitialized variant
+                       current: signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER, secp_ctx)
+                               .expect("Signer must be able to provide initial commitment point"),
+                       // TODO(async_signing): remove this expect with the Uninitialized variant
+                       next: signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, secp_ctx)
+                               .expect("Signer must be able to provide second commitment point"),
                }
        }
 
@@ -1001,9 +1005,12 @@ impl HolderCommitmentPoint {
                where SP::Target: SignerProvider, L::Target: Logger
        {
                if let HolderCommitmentPoint::PendingNext { transaction_number, current } = self {
-                       let next = signer.as_ref().get_per_commitment_point(*transaction_number - 1, secp_ctx);
-                       log_trace!(logger, "Retrieved next per-commitment point {}", *transaction_number - 1);
-                       *self = HolderCommitmentPoint::Available { transaction_number: *transaction_number, current: *current, next };
+                       if let Ok(next) = signer.as_ref().get_per_commitment_point(*transaction_number - 1, secp_ctx) {
+                               log_trace!(logger, "Retrieved next per-commitment point {}", *transaction_number - 1);
+                               *self = HolderCommitmentPoint::Available { transaction_number: *transaction_number, current: *current, next };
+                       } else {
+                               log_trace!(logger, "Next per-commitment point {} is pending", transaction_number);
+                       }
                }
        }
 
@@ -5622,7 +5629,9 @@ impl<SP: Deref> Channel<SP> where
 
                let our_commitment_transaction = INITIAL_COMMITMENT_NUMBER - self.context.holder_commitment_point.transaction_number() - 1;
                if msg.next_remote_commitment_number > 0 {
-                       let expected_point = self.context.holder_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.context.secp_ctx);
+                       let expected_point = self.context.holder_signer.as_ref()
+                               .get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - msg.next_remote_commitment_number + 1, &self.context.secp_ctx)
+                               .expect("TODO: async signing is not yet supported for per commitment points upon channel reestablishment");
                        let given_secret = SecretKey::from_slice(&msg.your_last_per_commitment_secret)
                                .map_err(|_| ChannelError::close("Peer sent a garbage channel_reestablish with unparseable secret key".to_owned()))?;
                        if expected_point != PublicKey::from_secret_key(&self.context.secp_ctx, &given_secret) {
@@ -8230,10 +8239,12 @@ impl<SP: Deref> OutboundV2Channel<SP> where SP::Target: SignerProvider {
 
                let first_per_commitment_point = self.context.holder_signer.as_ref()
                        .get_per_commitment_point(self.context.holder_commitment_point.transaction_number(),
-                               &self.context.secp_ctx);
+                               &self.context.secp_ctx)
+                               .expect("TODO: async signing is not yet supported for commitment points in v2 channel establishment");
                let second_per_commitment_point = self.context.holder_signer.as_ref()
                        .get_per_commitment_point(self.context.holder_commitment_point.transaction_number() - 1,
-                               &self.context.secp_ctx);
+                               &self.context.secp_ctx)
+                               .expect("TODO: async signing is not yet supported for commitment points in v2 channel establishment");
                let keys = self.context.get_holder_pubkeys();
 
                msgs::OpenChannelV2 {
@@ -8380,9 +8391,11 @@ impl<SP: Deref> InboundV2Channel<SP> where SP::Target: SignerProvider {
        /// [`msgs::AcceptChannelV2`]: crate::ln::msgs::AcceptChannelV2
        fn generate_accept_channel_v2_message(&self) -> msgs::AcceptChannelV2 {
                let first_per_commitment_point = self.context.holder_signer.as_ref().get_per_commitment_point(
-                       self.context.holder_commitment_point.transaction_number(), &self.context.secp_ctx);
+                       self.context.holder_commitment_point.transaction_number(), &self.context.secp_ctx)
+                       .expect("TODO: async signing is not yet supported for commitment points in v2 channel establishment");
                let second_per_commitment_point = self.context.holder_signer.as_ref().get_per_commitment_point(
-                       self.context.holder_commitment_point.transaction_number() - 1, &self.context.secp_ctx);
+                       self.context.holder_commitment_point.transaction_number() - 1, &self.context.secp_ctx)
+                       .expect("TODO: async signing is not yet supported for commitment points in v2 channel establishment");
                let keys = self.context.get_holder_pubkeys();
 
                msgs::AcceptChannelV2 {
@@ -9334,14 +9347,16 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
                        (Some(current), Some(next)) => HolderCommitmentPoint::Available {
                                transaction_number: cur_holder_commitment_transaction_number, current, next
                        },
-                       (Some(current), _) => HolderCommitmentPoint::Available {
+                       (Some(current), _) => HolderCommitmentPoint::PendingNext {
                                transaction_number: cur_holder_commitment_transaction_number, current,
-                               next: holder_signer.get_per_commitment_point(cur_holder_commitment_transaction_number - 1, &secp_ctx),
                        },
-                       (_, _) => HolderCommitmentPoint::Available {
-                               transaction_number: cur_holder_commitment_transaction_number,
-                               current: holder_signer.get_per_commitment_point(cur_holder_commitment_transaction_number, &secp_ctx),
-                               next: holder_signer.get_per_commitment_point(cur_holder_commitment_transaction_number - 1, &secp_ctx),
+                       (_, _) => {
+                               // TODO(async_signing): remove this expect with the Uninitialized variant
+                               let current = holder_signer.get_per_commitment_point(cur_holder_commitment_transaction_number, &secp_ctx)
+                                       .expect("Must be able to derive the current commitment point upon channel restoration");
+                               HolderCommitmentPoint::PendingNext {
+                                       transaction_number: cur_holder_commitment_transaction_number, current,
+                               }
                        },
                };
 

diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs
index e56fbf76c5ead5ce4ee5a752c4d3fbc5370e2758..6a0343fb53379e40a5d05c47ce71cd37b8c90fca 100644 (file)
--- a/lightning/src/ln/functional_tests.rs
+++ b/lightning/src/ln/functional_tests.rs
@@ -741,7 +741,7 @@ fn test_update_fee_that_funder_cannot_afford() {
                (pubkeys.revocation_basepoint, pubkeys.htlc_basepoint,
                 pubkeys.funding_pubkey)
        };
-       let (remote_delayed_payment_basepoint, remote_htlc_basepoint,remote_point, remote_funding) = {
+       let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_point, remote_funding) = {
                let per_peer_state = nodes[1].node.per_peer_state.read().unwrap();
                let chan_lock = per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap();
                let remote_chan = chan_lock.channel_by_id.get(&chan.2).map(
@@ -750,7 +750,7 @@ fn test_update_fee_that_funder_cannot_afford() {
                let chan_signer = remote_chan.get_signer();
                let pubkeys = chan_signer.as_ref().pubkeys();
                (pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
-                chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx),
+                chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap(),
                 pubkeys.funding_pubkey)
        };
 
@@ -1475,7 +1475,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().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx),
+                chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 2, &secp_ctx).unwrap(),
                 chan_signer.as_ref().pubkeys().funding_pubkey)
        };
        let (remote_delayed_payment_basepoint, remote_htlc_basepoint, remote_point, remote_funding) = {
@@ -1487,7 +1487,7 @@ fn test_fee_spike_violation_fails_htlc() {
                let chan_signer = remote_chan.get_signer();
                let pubkeys = chan_signer.as_ref().pubkeys();
                (pubkeys.delayed_payment_basepoint, pubkeys.htlc_basepoint,
-                chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx),
+                chan_signer.as_ref().get_per_commitment_point(INITIAL_COMMITMENT_NUMBER - 1, &secp_ctx).unwrap(),
                 chan_signer.as_ref().pubkeys().funding_pubkey)
        };
 

diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
index 885b8840b7650d9f4f49f811195cdef09c2822a9..d684b4fe68073ab526552d0ff582d16405d1c4ba 100644 (file)
--- a/lightning/src/sign/mod.rs
+++ b/lightning/src/sign/mod.rs
@@ -725,12 +725,23 @@ impl HTLCDescriptor {
 
 /// A trait to handle Lightning channel key material without concretizing the channel type or
 /// the signature mechanism.
+///
+/// Several methods allow error types to be returned to support async signing. This feature
+/// is not yet complete, and panics may occur in certain situations when returning errors
+/// for these methods.
 pub trait ChannelSigner {
        /// Gets the per-commitment point for a specific commitment number
        ///
        /// Note that the commitment number starts at `(1 << 48) - 1` and counts backwards.
-       fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>)
-               -> PublicKey;
+       ///
+       /// If the signer returns `Err`, then the user is responsible for either force-closing the channel
+       /// or calling `ChannelManager::signer_unblocked` (this method is only available when the
+       /// `async_signing` cfg flag is enabled) once the signature is ready.
+       ///
+       // TODO: link to `signer_unblocked` once the cfg flag is removed
+       fn get_per_commitment_point(
+               &self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>,
+       ) -> Result<PublicKey, ()>;
 
        /// Gets the commitment secret for a specific commitment number as part of the revocation process
        ///
@@ -1343,11 +1354,11 @@ impl EntropySource for InMemorySigner {
 impl ChannelSigner for InMemorySigner {
        fn get_per_commitment_point(
                &self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>,
-       ) -> PublicKey {
+       ) -> Result<PublicKey, ()> {
                let commitment_secret =
                        SecretKey::from_slice(&chan_utils::build_commitment_secret(&self.commitment_seed, idx))
                                .unwrap();
-               PublicKey::from_secret_key(secp_ctx, &commitment_secret)
+               Ok(PublicKey::from_secret_key(secp_ctx, &commitment_secret))
        }
 
        fn release_commitment_secret(&self, idx: u64) -> [u8; 32] {

diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs
index 28647982bdad2641975bc81779e89bedfb734fa5..aa491b04bd8a8581f34bd594ca74cabefb7ba314 100644 (file)
--- a/lightning/src/util/test_channel_signer.rs
+++ b/lightning/src/util/test_channel_signer.rs
@@ -164,7 +164,9 @@ impl TestChannelSigner {
 }
 
 impl ChannelSigner for TestChannelSigner {
-       fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>) -> PublicKey {
+       fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<PublicKey, ()> {
+               // TODO: implement a mask in EnforcementState to let you test signatures being
+               // unavailable
                self.inner.get_per_commitment_point(idx, secp_ctx)
        }