Sign splice shared input when producing holder tx_signatures
What the developer wrote
Authored by Wilmer Paulino
Sign splice shared input when producing holder tx_signatures
We also remove the `Result` to make it clear that this method does not
support async operations yet and rename the method to clarify that it is
only intended to be used for the shared input of a splice.
What changed, and why it matters
This commit fixes a missing signature in the Lightning splicing feature. When a user and their peer splice a channel (replace the old funding transaction with a new one), the old funding output is spent as a shared input in the new transaction. Previously, when the holder produced their tx_signatures message, they left the shared_input_signature field as None, even though the protocol requires them to sign that shared input. The commit adds that signature, renames the signer method to make its purpose clearer, and removes the Result return type because the operation is synchronous. It also adds state checks so funding signatures are only accepted when the channel is actually expecting them, and turns a debug-only assertion about signing failures into a logged warning.
Treat this as a functional/protocol bug fix with possible security implications for splicing. Users relying on splicing should upgrade to a release containing this commit. No immediate emergency response is indicated absent a disclosed exploit, but downstream integrators should verify their custom EcdsaChannelSigner implementations are updated for the renamed method and changed return type.
Security signals we found
Missing cryptographic signature on a splice shared input in holder-generated TxSignatures
Protocol correctness fix for Lightning splicing interactive transaction signing
Added state-guard checks before accepting funding signatures (API misuse errors)
Changed infallible signer API from Result to direct Signature return
Replaced debug_assert with log_warn for signing failures in channel manager
Evidence from the diff
In channel.rs, funding_transaction_signed now checks that the channel is in an interactive-signing state and, under the splicing feature flag, that a pending splice is awaiting signatures before proceeding. It then calls sign_splice_shared_input (formerly sign_splicing_funding_input) on the holder signer when a splice shared input index exists, and places the returned Signature into msgs::TxSignatures::shared_input_signature. The trait method in EcdsaChannelSigner was renamed and changed from Result
Changed components
lightning/src/ln/channel.rslightning/src/ln/channelmanager.rslightning/src/sign/ecdsa.rslightning/src/sign/mod.rslightning/src/util/dyn_signer.rslightning/src/util/test_channel_signer.rsInspect captured patch +64 / −16
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 9c27405..75e597d 100644
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -7984,6 +7984,35 @@ where
pub fn funding_transaction_signed(
&mut self, funding_txid_signed: Txid, witnesses: Vec<Witness>,
) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), APIError> {
+ if !self.context.channel_state.is_interactive_signing() {
+ let err =
+ format!("Channel {} not expecting funding signatures", self.context.channel_id);
+ return Err(APIError::APIMisuseError { err });
+ }
+ if self.context.channel_state.is_our_tx_signatures_ready() {
+ let err =
+ format!("Channel {} already received funding signatures", self.context.channel_id);
+ return Err(APIError::APIMisuseError { err });
+ }
+ #[cfg(splicing)]
+ if let Some(pending_splice) = self.pending_splice.as_ref() {
+ if !pending_splice
+ .funding_negotiation
+ .as_ref()
+ .map(|funding_negotiation| {
+ matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures(_))
+ })
+ .unwrap_or(false)
+ {
+ debug_assert!(false);
+ let err = format!(
+ "Channel {} with pending splice is not expecting funding signatures yet",
+ self.context.channel_id
+ );
+ return Err(APIError::APIMisuseError { err });
+ }
+ }
+
let (tx_signatures_opt, funding_tx_opt) = self
.interactive_tx_signing_session
.as_mut()
@@ -8001,11 +8030,31 @@ where
});
}
+ let shared_input_signature = if let Some(splice_input_index) =
+ signing_session.unsigned_tx().shared_input_index()
+ {
+ let sig = match &self.context.holder_signer {
+ ChannelSignerType::Ecdsa(signer) => signer.sign_splice_shared_input(
+ &self.funding.channel_transaction_parameters,
+ &tx,
+ splice_input_index as usize,
+ &self.context.secp_ctx,
+ ),
+ #[cfg(taproot)]
+ ChannelSignerType::Taproot(_) => todo!(),
+ };
+ Some(sig)
+ } else {
+ None
+ };
+ #[cfg(splicing)]
+ debug_assert_eq!(self.pending_splice.is_some(), shared_input_signature.is_some());
+
let tx_signatures = msgs::TxSignatures {
channel_id: self.context.channel_id,
tx_hash: funding_txid_signed,
witnesses,
- shared_input_signature: None,
+ shared_input_signature,
};
signing_session
.provide_holder_witnesses(tx_signatures, &self.context.secp_ctx)
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 28f770b..075bce3 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -9044,7 +9044,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
Ok((None, _)) => {
debug_assert!(false, "If our tx_signatures is empty, then we should send it first!");
},
- Err(err) => debug_assert!(false, "We should not error here but we got: {:?}", err),
+ Err(err) => {
+ log_warn!(logger, "Failed signing interactive funding transaction: {err:?}");
+ },
}
}
}
diff --git a/lightning/src/sign/ecdsa.rs b/lightning/src/sign/ecdsa.rs
index a25d5df..bfa3a27 100644
--- a/lightning/src/sign/ecdsa.rs
+++ b/lightning/src/sign/ecdsa.rs
@@ -242,7 +242,7 @@ pub trait EcdsaChannelSigner: ChannelSigner {
msg: &UnsignedChannelAnnouncement, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<Signature, ()>;
- /// Signs the input of a splicing funding transaction with our funding key.
+ /// Signs the shared input of a splice transaction with our funding key.
///
/// In splicing, the previous funding transaction output is spent as the input of
/// the new funding transaction, and is a 2-of-2 multisig.
@@ -253,11 +253,8 @@ pub trait EcdsaChannelSigner: ChannelSigner {
///
/// `input_index`: The index of the input within the new funding transaction `tx`,
/// spending the previous funding transaction's output
- ///
- /// This method is *not* asynchronous. If an `Err` is returned, the channel will be immediately
- /// closed.
- fn sign_splicing_funding_input(
+ fn sign_splice_shared_input(
&self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction,
input_index: usize, secp_ctx: &Secp256k1<secp256k1::All>,
- ) -> Result<Signature, ()>;
+ ) -> Signature;
}
diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs
index abdb03c..67b602a 100644
--- a/lightning/src/sign/mod.rs
+++ b/lightning/src/sign/mod.rs
@@ -1753,10 +1753,10 @@ impl EcdsaChannelSigner for InMemorySigner {
Ok(secp_ctx.sign_ecdsa(&msghash, &funding_key))
}
- fn sign_splicing_funding_input(
+ fn sign_splice_shared_input(
&self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction,
input_index: usize, secp_ctx: &Secp256k1<secp256k1::All>,
- ) -> Result<Signature, ()> {
+ ) -> Signature {
assert!(channel_parameters.is_populated(), "Channel parameters must be fully populated");
assert_eq!(
tx.input[input_index].previous_output,
@@ -1782,7 +1782,7 @@ impl EcdsaChannelSigner for InMemorySigner {
)
.unwrap()[..];
let msg = hash_to_message!(sighash);
- Ok(sign(secp_ctx, &msg, &funding_key))
+ sign(secp_ctx, &msg, &funding_key)
}
}
diff --git a/lightning/src/util/dyn_signer.rs b/lightning/src/util/dyn_signer.rs
index baa4eed..8bba856 100644
--- a/lightning/src/util/dyn_signer.rs
+++ b/lightning/src/util/dyn_signer.rs
@@ -159,8 +159,8 @@ delegate!(DynSigner, EcdsaChannelSigner, inner,
secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()>,
fn sign_holder_htlc_transaction(, htlc_tx: &Transaction, input: usize,
htlc_descriptor: &HTLCDescriptor, secp_ctx: &Secp256k1<All>) -> Result<Signature, ()>,
- fn sign_splicing_funding_input(, channel_parameters: &ChannelTransactionParameters,
- tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1<All>) -> Result<Signature, ()>
+ fn sign_splice_shared_input(, channel_parameters: &ChannelTransactionParameters,
+ tx: &Transaction, input_index: usize, secp_ctx: &Secp256k1<All>) -> Signature
);
delegate!(DynSigner, ChannelSigner,
diff --git a/lightning/src/util/test_channel_signer.rs b/lightning/src/util/test_channel_signer.rs
index 7e89650..0b472b9 100644
--- a/lightning/src/util/test_channel_signer.rs
+++ b/lightning/src/util/test_channel_signer.rs
@@ -484,11 +484,11 @@ impl EcdsaChannelSigner for TestChannelSigner {
self.inner.sign_channel_announcement_with_funding_key(channel_parameters, msg, secp_ctx)
}
- fn sign_splicing_funding_input(
+ fn sign_splice_shared_input(
&self, channel_parameters: &ChannelTransactionParameters, tx: &Transaction,
input_index: usize, secp_ctx: &Secp256k1<secp256k1::All>,
- ) -> Result<Signature, ()> {
- self.inner.sign_splicing_funding_input(channel_parameters, tx, input_index, secp_ctx)
+ ) -> Signature {
+ self.inner.sign_splice_shared_input(channel_parameters, tx, input_index, secp_ctx)
}
}
Why this scored 57/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.