Skip to content

Commit 4d68a4f

Browse files
TheBlueMattwaterson
TheBlueMatt
authored and
waterson
committed
Handle retrying sign_counterparty_commitment inb funding 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 retrying of inbound funding_created signing failures, regenerating the `FundingSigned` message, attempting to re-sign, and sending it to our peers if we succeed.
1 parent 2c4872b commit 4d68a4f

File tree

1 file changed

+55
-52
lines changed

1 file changed

+55
-52
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 55 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -2050,6 +2050,36 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
20502050
next_local_nonce: None,
20512051
})
20522052
}
2053+
2054+
/// Only allowed after [`Self::channel_transaction_parameters`] is set.
2055+
fn get_funding_signed_msg<L: Deref>(&mut self, logger: &L) -> (CommitmentTransaction, Option<msgs::FundingSigned>) where L::Target: Logger {
2056+
let counterparty_keys = self.build_remote_transaction_keys();
2057+
let counterparty_initial_commitment_tx = self.build_commitment_transaction(self.cur_counterparty_commitment_transaction_number + 1, &counterparty_keys, false, false, logger).tx;
2058+
2059+
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
2060+
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
2061+
log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
2062+
&self.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
2063+
2064+
match &self.holder_signer {
2065+
// TODO (arik): move match into calling method for Taproot
2066+
ChannelSignerType::Ecdsa(ecdsa) => {
2067+
let funding_signed = ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
2068+
.map(|(signature, _)| msgs::FundingSigned {
2069+
channel_id: self.channel_id(),
2070+
signature,
2071+
#[cfg(taproot)]
2072+
partial_signature_with_nonce: None,
2073+
})
2074+
.ok();
2075+
self.signer_pending_funding = funding_signed.is_none();
2076+
2077+
// We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
2078+
(counterparty_initial_commitment_tx, funding_signed)
2079+
}
2080+
}
2081+
}
2082+
20532083
}
20542084

20552085
// Internal utility functions for channels
@@ -3854,7 +3884,9 @@ impl<SP: Deref> Channel<SP> where
38543884
let commitment_update = if self.context.signer_pending_commitment_update {
38553885
None
38563886
} else { None };
3857-
let funding_signed = None;
3887+
let funding_signed = if self.context.signer_pending_funding && !self.context.is_outbound() {
3888+
self.context.get_funding_signed_msg(logger).1
3889+
} else { None };
38583890
let funding_created = if self.context.signer_pending_funding && self.context.is_outbound() {
38593891
self.context.get_funding_created_msg(logger)
38603892
} else { None };
@@ -6571,41 +6603,22 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
65716603
self.generate_accept_channel_message()
65726604
}
65736605

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

65776609
let keys = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
65786610
let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &keys, true, false, logger).tx;
6579-
{
6580-
let trusted_tx = initial_commitment_tx.trust();
6581-
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
6582-
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.context.channel_value_satoshis);
6583-
// They sign the holder commitment transaction...
6584-
log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} for channel {}.",
6585-
log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.context.counterparty_funding_pubkey().serialize()),
6586-
encode::serialize_hex(&initial_commitment_bitcoin_tx.transaction), log_bytes!(sighash[..]),
6587-
encode::serialize_hex(&funding_script), &self.context.channel_id());
6588-
secp_check!(self.context.secp_ctx.verify_ecdsa(&sighash, &sig, self.context.counterparty_funding_pubkey()), "Invalid funding_created signature from peer".to_owned());
6589-
}
6590-
6591-
let counterparty_keys = self.context.build_remote_transaction_keys();
6592-
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
6593-
6594-
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
6595-
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
6596-
log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
6597-
&self.context.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
6598-
6599-
match &self.context.holder_signer {
6600-
// TODO (arik): move match into calling method for Taproot
6601-
ChannelSignerType::Ecdsa(ecdsa) => {
6602-
let counterparty_signature = ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.context.secp_ctx)
6603-
.map(|(sig, _)| sig).ok();
6611+
let trusted_tx = initial_commitment_tx.trust();
6612+
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
6613+
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.context.channel_value_satoshis);
6614+
// They sign the holder commitment transaction...
6615+
log_trace!(logger, "Checking funding_created tx signature {} by key {} against tx {} (sighash {}) with redeemscript {} for channel {}.",
6616+
log_bytes!(sig.serialize_compact()[..]), log_bytes!(self.context.counterparty_funding_pubkey().serialize()),
6617+
encode::serialize_hex(&initial_commitment_bitcoin_tx.transaction), log_bytes!(sighash[..]),
6618+
encode::serialize_hex(&funding_script), &self.context.channel_id());
6619+
secp_check!(self.context.secp_ctx.verify_ecdsa(&sighash, &sig, self.context.counterparty_funding_pubkey()), "Invalid funding_created signature from peer".to_owned());
66046620

6605-
// We sign "counterparty" commitment transaction, allowing them to broadcast the tx if they wish.
6606-
Ok((counterparty_initial_commitment_tx, initial_commitment_tx, counterparty_signature))
6607-
}
6608-
}
6621+
Ok(initial_commitment_tx)
66096622
}
66106623

66116624
pub fn funding_created<L: Deref>(
@@ -6632,10 +6645,10 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66326645
let funding_txo = OutPoint { txid: msg.funding_txid, index: msg.funding_output_index };
66336646
self.context.channel_transaction_parameters.funding_outpoint = Some(funding_txo);
66346647
// This is an externally observable change before we finish all our checks. In particular
6635-
// funding_created_signature may fail.
6648+
// check_funding_created_signature may fail.
66366649
self.context.holder_signer.as_mut().provide_channel_parameters(&self.context.channel_transaction_parameters);
66376650

6638-
let (counterparty_initial_commitment_tx, initial_commitment_tx, sig_opt) = match self.funding_created_signature(&msg.signature, logger) {
6651+
let initial_commitment_tx = match self.check_funding_created_signature(&msg.signature, logger) {
66396652
Ok(res) => res,
66406653
Err(ChannelError::Close(e)) => {
66416654
self.context.channel_transaction_parameters.funding_outpoint = None;
@@ -6644,7 +6657,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66446657
Err(e) => {
66456658
// The only error we know how to handle is ChannelError::Close, so we fall over here
66466659
// to make sure we don't continue with an inconsistent state.
6647-
panic!("unexpected error type from funding_created_signature {:?}", e);
6660+
panic!("unexpected error type from check_funding_created_signature {:?}", e);
66486661
}
66496662
};
66506663

@@ -6660,6 +6673,13 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66606673
return Err((self, ChannelError::Close("Failed to validate our commitment".to_owned())));
66616674
}
66626675

6676+
self.context.channel_state = ChannelState::FundingSent as u32;
6677+
self.context.channel_id = funding_txo.to_channel_id();
6678+
self.context.cur_counterparty_commitment_transaction_number -= 1;
6679+
self.context.cur_holder_commitment_transaction_number -= 1;
6680+
6681+
let (counterparty_initial_commitment_tx, funding_signed) = self.context.get_funding_signed_msg(logger);
6682+
66636683
// Now that we're past error-generating stuff, update our local state:
66646684

66656685
let funding_redeemscript = self.context.get_funding_redeemscript();
@@ -6678,16 +6698,11 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66786698

66796699
channel_monitor.provide_initial_counterparty_commitment_tx(
66806700
counterparty_initial_commitment_tx.trust().txid(), Vec::new(),
6681-
self.context.cur_counterparty_commitment_transaction_number,
6701+
self.context.cur_counterparty_commitment_transaction_number + 1,
66826702
self.context.counterparty_cur_commitment_point.unwrap(), self.context.feerate_per_kw,
66836703
counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
66846704
counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
66856705

6686-
self.context.channel_state = ChannelState::FundingSent as u32;
6687-
self.context.channel_id = funding_txo.to_channel_id();
6688-
self.context.cur_counterparty_commitment_transaction_number -= 1;
6689-
self.context.cur_holder_commitment_transaction_number -= 1;
6690-
66916706
log_info!(logger, "Generated funding_signed for peer for channel {}", &self.context.channel_id());
66926707

66936708
// Promote the channel to a full-fledged one now that we have updated the state and have a
@@ -6699,18 +6714,6 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
66996714
let need_channel_ready = channel.check_get_channel_ready(0).is_some();
67006715
channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
67016716

6702-
let funding_signed = if let Some(signature) = sig_opt {
6703-
Some(msgs::FundingSigned {
6704-
channel_id,
6705-
signature,
6706-
#[cfg(taproot)]
6707-
partial_signature_with_nonce: None,
6708-
})
6709-
} else {
6710-
channel.context.signer_pending_funding = true;
6711-
None
6712-
};
6713-
67146717
Ok((channel, funding_signed, channel_monitor))
67156718
}
67166719
}

0 commit comments

Comments
 (0)