diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 612b2eb1aa0..fbdac3bcd68 100644 --- 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); + } } } @@ -5611,7 +5618,7 @@ impl Channel 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"); 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) { @@ -9313,14 +9320,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 2a5f5a86952..0b60943d2b1 100644 --- 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 885b8840b76..0ce3379107b 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -729,8 +729,12 @@ 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) - -> PublicKey; + /// + /// If the signer returns `Err`, then the user is responsible for either force-closing the channel + /// or retrying once the signature is ready. + fn get_per_commitment_point( + &self, idx: u64, secp_ctx: &Secp256k1, + ) -> Result; /// Gets the commitment secret for a specific commitment number as part of the revocation process /// @@ -1343,11 +1347,11 @@ impl EntropySource for InMemorySigner { impl ChannelSigner for InMemorySigner { fn get_per_commitment_point( &self, idx: u64, secp_ctx: &Secp256k1, - ) -> PublicKey { + ) -> Result { 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 884acc2c2ea..3145d636b37 100644 --- a/lightning/src/util/test_channel_signer.rs +++ b/lightning/src/util/test_channel_signer.rs @@ -166,7 +166,9 @@ impl TestChannelSigner { } impl ChannelSigner for TestChannelSigner { - fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> PublicKey { + fn get_per_commitment_point(&self, idx: u64, secp_ctx: &Secp256k1) -> Result { + // TODO: implement a mask in EnforcementState to let you test signatures being + // unavailable self.inner.get_per_commitment_point(idx, secp_ctx) }