Skip to content

Commit 8dfc1e5

Browse files
TheBlueMattwaterson
authored andcommitted
Handle retrying sign_counterparty_commitment failures
If sign_counterparty_commitment fails (i.e. because the signer is temporarily disconnected), this really indicates that we should retry the message sending which required the signature later, rather than force-closing the channel (which probably won't even work if the signer is missing). This commit adds initial retrying of failures, specifically regenerating commitment updates, attempting to re-sign the `CommitmentSigned` message, and sending it to our peers if we succed.
1 parent 0e3f6b6 commit 8dfc1e5

File tree

2 files changed

+94
-0
lines changed

2 files changed

+94
-0
lines changed

lightning/src/ln/channel.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -533,6 +533,15 @@ pub(super) struct MonitorRestoreUpdates {
533533
pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
534534
}
535535

536+
/// The return value of `signer_maybe_unblocked`
537+
#[allow(unused)]
538+
pub(super) struct SignerResumeUpdates {
539+
pub commitment_update: Option<msgs::CommitmentUpdate>,
540+
pub funding_signed: Option<msgs::FundingSigned>,
541+
pub funding_created: Option<msgs::FundingCreated>,
542+
pub channel_ready: Option<msgs::ChannelReady>,
543+
}
544+
536545
/// The return value of `channel_reestablish`
537546
pub(super) struct ReestablishResponses {
538547
pub channel_ready: Option<msgs::ChannelReady>,
@@ -3912,6 +3921,31 @@ impl<SP: Deref> Channel<SP> where
39123921
Ok(())
39133922
}
39143923

3924+
/// Indicates that the signer may have some signatures for us, so we should retry if we're
3925+
/// blocked.
3926+
#[allow(unused)]
3927+
pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> SignerResumeUpdates where L::Target: Logger {
3928+
let commitment_update = if self.context.signer_pending_commitment_update {
3929+
self.get_last_commitment_update_for_send(logger).ok()
3930+
} else { None };
3931+
let funding_signed = None;
3932+
let channel_ready = None;
3933+
let funding_created = None;
3934+
3935+
log_trace!(logger, "Signer unblocked with {} commitment_update, {} funding_signed, {} funding_created, and {} channel_ready",
3936+
if commitment_update.is_some() { "a" } else { "no" },
3937+
if funding_signed.is_some() { "a" } else { "no" },
3938+
if funding_created.is_some() { "a" } else { "no" },
3939+
if channel_ready.is_some() { "a" } else { "no" });
3940+
3941+
SignerResumeUpdates {
3942+
commitment_update,
3943+
funding_signed,
3944+
funding_created,
3945+
channel_ready,
3946+
}
3947+
}
3948+
39153949
fn get_last_revoke_and_ack(&self) -> msgs::RevokeAndACK {
39163950
let next_per_commitment_point = self.context.holder_signer.as_ref().get_per_commitment_point(self.context.cur_holder_commitment_transaction_number, &self.context.secp_ctx);
39173951
let per_commitment_secret = self.context.holder_signer.as_ref().release_commitment_secret(self.context.cur_holder_commitment_transaction_number + 2);

lightning/src/ln/channelmanager.rs

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7227,6 +7227,66 @@ where
72277227
has_update
72287228
}
72297229

7230+
/// When a call to a [`ChannelSigner`] method returns an error, this indicates that the signer
7231+
/// is (temporarily) unavailable, and the operation should be retried later.
7232+
///
7233+
/// This method allows for that retry - either checking for any signer-pending messages to be
7234+
/// attempted in every channel, or in the specifically provided channel.
7235+
///
7236+
/// [`ChannelSigner`]: crate::sign::ChannelSigner
7237+
#[cfg(test)] // This is only implemented for one signer method, and should be private until we
7238+
// actually finish implementing it fully.
7239+
pub fn signer_unblocked(&self, channel_opt: Option<(PublicKey, ChannelId)>) {
7240+
let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
7241+
7242+
let unblock_chan = |phase: &mut ChannelPhase<SP>, pending_msg_events: &mut Vec<MessageSendEvent>| {
7243+
let node_id = phase.context().get_counterparty_node_id();
7244+
if let ChannelPhase::Funded(chan) = phase {
7245+
let msgs = chan.signer_maybe_unblocked(&self.logger);
7246+
if let Some(updates) = msgs.commitment_update {
7247+
pending_msg_events.push(events::MessageSendEvent::UpdateHTLCs {
7248+
node_id,
7249+
updates,
7250+
});
7251+
}
7252+
if let Some(msg) = msgs.funding_signed {
7253+
pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
7254+
node_id,
7255+
msg,
7256+
});
7257+
}
7258+
if let Some(msg) = msgs.funding_created {
7259+
pending_msg_events.push(events::MessageSendEvent::SendFundingCreated {
7260+
node_id,
7261+
msg,
7262+
});
7263+
}
7264+
if let Some(msg) = msgs.channel_ready {
7265+
send_channel_ready!(self, pending_msg_events, chan, msg);
7266+
}
7267+
}
7268+
};
7269+
7270+
let per_peer_state = self.per_peer_state.read().unwrap();
7271+
if let Some((counterparty_node_id, channel_id)) = channel_opt {
7272+
if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) {
7273+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
7274+
let peer_state = &mut *peer_state_lock;
7275+
if let Some(chan) = peer_state.channel_by_id.get_mut(&channel_id) {
7276+
unblock_chan(chan, &mut peer_state.pending_msg_events);
7277+
}
7278+
}
7279+
} else {
7280+
for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
7281+
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
7282+
let peer_state = &mut *peer_state_lock;
7283+
for (_, chan) in peer_state.channel_by_id.iter_mut() {
7284+
unblock_chan(chan, &mut peer_state.pending_msg_events);
7285+
}
7286+
}
7287+
}
7288+
}
7289+
72307290
/// Check whether any channels have finished removing all pending updates after a shutdown
72317291
/// exchange and can now send a closing_signed.
72327292
/// Returns whether any closing_signed messages were generated.

0 commit comments

Comments
 (0)