Skip to content

Commit 417230a

Browse files
committed
Verify the holder provided valid witnesses and uses SIGHASH_ALL
LDK checks the following: * Each input spends an output that is one of P2WPKH, P2WSH, or P2TR. These were already checked by LDK when the inputs to be contributed were provided. * All signatures use the `SIGHASH_ALL` sighash type. * P2WPKH and P2TR key path spends are valid (verifies signatures) NOTE: * When checking P2WSH spends, LDK tries to decode 70-72 byte witness elements as ECDSA signatures with a sighash flag. If the internal DER-decoding fails, then LDK just assumes it wasn't a signature and carries with checks. If the element can be decoded as an ECDSA signature, the the sighash flag must be `SIGHASH_ALL`. * When checking P2TR script-path spends, LDK assumes all elements of exactly 65 bytes with the last byte matching any valid sighash flag byte are schnorr signatures and checks that the sighash type is `SIGHASH_ALL`. If the last byte is not any valid sighash flag, the element is assumed not to be a signature and is ignored. Elements of 64 bytes are not checked because if they were schnorr signatures then they would implicitly be `SIGHASH_DEFAULT` which is an alias of `SIGHASH_ALL`.
1 parent ede3b09 commit 417230a

File tree

3 files changed

+555
-32
lines changed

3 files changed

+555
-32
lines changed

lightning/src/ln/channel.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7625,11 +7625,12 @@ where
76257625
}
76267626
}
76277627

7628-
fn verify_interactive_tx_signatures(&mut self, _witnesses: &Vec<Witness>) {
7629-
if let Some(ref mut _signing_session) = self.interactive_tx_signing_session {
7630-
// Check that sighash_all was used:
7631-
// TODO(dual_funding): Check sig for sighash
7632-
}
7628+
#[cfg(splicing)]
7629+
fn is_splicing(&self) -> bool {
7630+
self.pending_splice
7631+
.as_ref()
7632+
.and_then(|pending_splice| Some(pending_splice.funding.is_some()))
7633+
.unwrap_or(false)
76337634
}
76347635

76357636
pub fn funding_transaction_signed<L: Deref>(
@@ -7638,14 +7639,21 @@ where
76387639
where
76397640
L::Target: Logger,
76407641
{
7641-
self.verify_interactive_tx_signatures(&witnesses);
76427642
if let Some(ref mut signing_session) = self.interactive_tx_signing_session {
7643+
signing_session.verify_interactive_tx_signatures(&self.context.secp_ctx, &witnesses)?;
76437644
let logger = WithChannelContext::from(logger, &self.context, None);
76447645
if let Some(holder_tx_signatures) = signing_session
76457646
.provide_holder_witnesses(self.context.channel_id, witnesses)
76467647
.map_err(|err| APIError::APIMisuseError { err })?
76477648
{
7648-
if self.is_awaiting_initial_mon_persist() {
7649+
#[cfg(splicing)]
7650+
let is_monitor_update_in_progress = self.is_awaiting_initial_mon_persist()
7651+
|| self.is_splicing()
7652+
&& self.context.channel_state.is_monitor_update_in_progress();
7653+
#[cfg(not(splicing))]
7654+
let is_monitor_update_in_progress = self.is_awaiting_initial_mon_persist();
7655+
7656+
if is_monitor_update_in_progress {
76497657
log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
76507658
self.context.monitor_pending_tx_signatures = Some(holder_tx_signatures);
76517659
return Ok(None);

lightning/src/ln/channelmanager.rs

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5899,19 +5899,32 @@ where
58995899
/// counterparty's signature(s) the funding transaction will automatically be broadcast via the
59005900
/// [`BroadcasterInterface`] provided when this `ChannelManager` was constructed.
59015901
///
5902-
/// `SIGHASH_ALL` MUST be used for all signatures when providing signatures.
5903-
///
5904-
/// <div class="warning">
5905-
/// WARNING: LDK makes no attempt to prevent the counterparty from using non-standard inputs which
5906-
/// will prevent the funding transaction from being relayed on the bitcoin network and hence being
5907-
/// confirmed.
5908-
/// </div>
5902+
/// `SIGHASH_ALL` MUST be used for all signatures when providing signatures, otherwise your
5903+
/// funds can be held hostage!
5904+
///
5905+
/// LDK checks the following:
5906+
/// * Each input spends an output that is one of P2WPKH, P2WSH, or P2TR.
5907+
/// These were already checked by LDK when the inputs to be contributed were provided.
5908+
/// * All signatures use the `SIGHASH_ALL` sighash type.
5909+
/// * P2WPKH and P2TR key path spends are valid (verifies signatures)
5910+
///
5911+
/// NOTE:
5912+
/// * When checking P2WSH spends, LDK tries to decode 70-72 byte witness elements as ECDSA
5913+
/// signatures with a sighash flag. If the internal DER-decoding fails, then LDK just
5914+
/// assumes it wasn't a signature and carries with checks. If the element can be decoded
5915+
/// as an ECDSA signature, the the sighash flag must be `SIGHASH_ALL`.
5916+
/// * When checking P2TR script-path spends, LDK assumes all elements of exactly 65 bytes
5917+
/// with the last byte matching any valid sighash flag byte are schnorr signatures and checks
5918+
/// that the sighash type is `SIGHASH_ALL`. If the last byte is not any valid sighash flag, the
5919+
/// element is assumed not to be a signature and is ignored. Elements of 64 bytes are not
5920+
/// checked because if they were schnorr signatures then they would implicitly be `SIGHASH_DEFAULT`
5921+
/// which is an alias of `SIGHASH_ALL`.
59095922
///
59105923
/// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect
59115924
/// `counterparty_node_id` is provided.
59125925
///
59135926
/// Returns [`APIMisuseError`] when a channel is not in a state where it is expecting funding
5914-
/// signatures.
5927+
/// signatures or if any of the checks described above fail.
59155928
///
59165929
/// [`ChannelUnavailable`]: APIError::ChannelUnavailable
59175930
/// [`APIMisuseError`]: APIError::APIMisuseError

0 commit comments

Comments
 (0)