Skip to content

Commit f051dff

Browse files
TheBlueMattwaterson
authored andcommitted
Handle sign_counterparty_commitment failing during inb funding
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). Here we add initial handling of sign_counterparty_commitment failing during inbound channel funding, setting a flag in `ChannelContext` which indicates we should retry sending the `funding_signed` later. We don't yet add any ability to do that retry.
1 parent 91760b2 commit f051dff

File tree

2 files changed

+34
-22
lines changed

2 files changed

+34
-22
lines changed

lightning/src/ln/channel.rs

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6545,7 +6545,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
65456545
self.generate_accept_channel_message()
65466546
}
65476547

6548-
fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(CommitmentTransaction, CommitmentTransaction, Signature), ChannelError> where L::Target: Logger {
6548+
fn funding_created_signature<L: Deref>(&mut self, sig: &Signature, logger: &L) -> Result<(CommitmentTransaction, CommitmentTransaction, Option<Signature>), ChannelError> where L::Target: Logger {
65496549
let funding_script = self.context.get_funding_redeemscript();
65506550

65516551
let keys = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
@@ -6574,7 +6574,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
65746574
// TODO (arik): move match into calling method for Taproot
65756575
ChannelSignerType::Ecdsa(ecdsa) => {
65766576
let counterparty_signature = ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
6577-
.map_err(|_| ChannelError::Close("Failed to get signatures for new commitment_signed".to_owned()))?.0;
6577+
.map(|(sig, _)| sig).ok();
65786578

65796579
// We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
65806580
Ok((counterparty_initial_commitment_tx, initial_commitment_tx, counterparty_signature))
@@ -6584,7 +6584,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
65846584

65856585
pub fn funding_created<L: Deref>(
65866586
mut self, msg: &msgs::FundingCreated, best_block: BestBlock, signer_provider: &SP, logger: &L
6587-
) -> Result<(Channel<SP>, msgs::FundingSigned, ChannelMonitor<<SP::Target as SignerProvider>::Signer>), (Self, ChannelError)>
6587+
) -> Result<(Channel<SP>, Option<msgs::FundingSigned>, ChannelMonitor<<SP::Target as SignerProvider>::Signer>), (Self, ChannelError)>
65886588
where
65896589
L::Target: Logger
65906590
{
@@ -6609,7 +6609,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66096609
// funding_created_signature may fail.
66106610
self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
66116611

6612-
let (counterparty_initial_commitment_tx, initial_commitment_tx, signature) = match self.funding_created_signature(&msg.signature, logger) {
6612+
let (counterparty_initial_commitment_tx, initial_commitment_tx, sig_opt) = match self.funding_created_signature(&msg.signature, logger) {
66136613
Ok(res) => res,
66146614
Err(ChannelError::Close(e)) => {
66156615
self.context.channel_transaction_parameters.funding_outpoint = None;
@@ -6673,12 +6673,19 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66736673
let need_channel_ready = channel.check_get_channel_ready(0).is_some();
66746674
channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
66756675

6676-
Ok((channel, msgs::FundingSigned {
6677-
channel_id,
6678-
signature,
6679-
#[cfg(taproot)]
6680-
partial_signature_with_nonce: None,
6681-
}, channel_monitor))
6676+
let funding_signed = if let Some(signature) = sig_opt {
6677+
Some(msgs::FundingSigned {
6678+
channel_id,
6679+
signature,
6680+
#[cfg(taproot)]
6681+
partial_signature_with_nonce: None,
6682+
})
6683+
} else {
6684+
channel.context.signer_pending_funding = true;
6685+
None
6686+
};
6687+
6688+
Ok((channel, funding_signed, channel_monitor))
66826689
}
66836690
}
66846691

@@ -7761,7 +7768,7 @@ mod tests {
77617768
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
77627769

77637770
// Node B --> Node A: funding signed
7764-
let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger).unwrap();
7771+
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
77657772

77667773
// Put some inbound and outbound HTLCs in A's channel.
77677774
let htlc_amount_msat = 11_092_000; // put an amount below A's effective dust limit but above B's.
@@ -7888,7 +7895,7 @@ mod tests {
78887895
let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
78897896

78907897
// Node B --> Node A: funding signed
7891-
let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger).unwrap();
7898+
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
78927899

78937900
// Now disconnect the two nodes and check that the commitment point in
78947901
// Node B's channel_reestablish message is sane.
@@ -8076,7 +8083,7 @@ mod tests {
80768083
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
80778084

80788085
// Node B --> Node A: funding signed
8079-
let _ = node_a_chan.funding_signed(&funding_signed_msg, best_block, &&keys_provider, &&logger).unwrap();
8086+
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
80808087

80818088
// Make sure that receiving a channel update will update the Channel as expected.
80828089
let update = ChannelUpdate {

lightning/src/ln/channelmanager.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5677,7 +5677,7 @@ where
56775677

56785678
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
56795679
let peer_state = &mut *peer_state_lock;
5680-
let (chan, funding_msg, monitor) =
5680+
let (chan, funding_msg_opt, monitor) =
56815681
match peer_state.channel_by_id.remove(&msg.temporary_channel_id) {
56825682
Some(ChannelPhase::UnfundedInboundV1(inbound_chan)) => {
56835683
match inbound_chan.funding_created(msg, best_block, &self.signer_provider, &self.logger) {
@@ -5700,16 +5700,19 @@ where
57005700
None => return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}", counterparty_node_id), msg.temporary_channel_id))
57015701
};
57025702

5703-
match peer_state.channel_by_id.entry(funding_msg.channel_id) {
5703+
match peer_state.channel_by_id.entry(chan.context.channel_id()) {
57045704
hash_map::Entry::Occupied(_) => {
5705-
Err(MsgHandleErrInternal::send_err_msg_no_close("Already had channel with the new channel_id".to_owned(), funding_msg.channel_id))
5705+
Err(MsgHandleErrInternal::send_err_msg_no_close(
5706+
"Already had channel with the new channel_id".to_owned(),
5707+
chan.context.channel_id()
5708+
))
57065709
},
57075710
hash_map::Entry::Vacant(e) => {
57085711
match self.id_to_peer.lock().unwrap().entry(chan.context.channel_id()) {
57095712
hash_map::Entry::Occupied(_) => {
57105713
return Err(MsgHandleErrInternal::send_err_msg_no_close(
57115714
"The funding_created message had the same funding_txid as an existing channel - funding is not possible".to_owned(),
5712-
funding_msg.channel_id))
5715+
chan.context.channel_id()))
57135716
},
57145717
hash_map::Entry::Vacant(i_e) => {
57155718
i_e.insert(chan.context.get_counterparty_node_id());
@@ -5720,11 +5723,13 @@ where
57205723
// hasn't persisted to disk yet - we can't lose money on a transaction that we haven't
57215724
// accepted payment from yet. We do, however, need to wait to send our channel_ready
57225725
// until we have persisted our monitor.
5723-
let new_channel_id = funding_msg.channel_id;
5724-
peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
5725-
node_id: counterparty_node_id.clone(),
5726-
msg: funding_msg,
5727-
});
5726+
let new_channel_id = chan.context.channel_id();
5727+
if let Some(msg) = funding_msg_opt {
5728+
peer_state.pending_msg_events.push(events::MessageSendEvent::SendFundingSigned {
5729+
node_id: counterparty_node_id.clone(),
5730+
msg,
5731+
});
5732+
}
57285733

57295734
let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
57305735

0 commit comments

Comments
 (0)