Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions lightning/src/ln/async_signer_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1742,3 +1742,80 @@ fn test_async_splice_initial_commit_sig_waits_for_monitor_before_tx_signatures()
let _ = get_event!(initiator, Event::SplicePending);
let _ = get_event!(acceptor, Event::SplicePending);
}

#[test]
fn test_async_splice_shared_input_signature_released_on_unblock() {
// Test that we can provide the signature of a splice's shared input asynchronously, and check
// that the holding cell is freed after exiting quiescence due to exchanging `tx_signatures`.
let chanmon_cfgs = create_chanmon_cfgs(2);
let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
let nodes = create_network(2, &node_cfgs, &node_chanmgrs);

let channel_id = create_announced_chan_between_nodes(&nodes, 0, 1).2;

let (initiator, acceptor) = (&nodes[0], &nodes[1]);
let initiator_node_id = initiator.node.get_our_node_id();
let acceptor_node_id = acceptor.node.get_our_node_id();

initiator.disable_channel_signer_op(
&acceptor_node_id,
&channel_id,
SignerOp::SignSpliceSharedInput,
);

let outputs = vec![TxOut {
value: Amount::from_sat(1_000),
script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
}];
let contribution = initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap();
negotiate_splice_tx(initiator, acceptor, channel_id, contribution);

let event = get_event!(initiator, Event::FundingTransactionReadyForSigning);
if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = event {
let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap();
initiator
.node
.funding_transaction_signed(&channel_id, &acceptor_node_id, partially_signed_tx)
.unwrap();
}

let initiator_commit_sig = get_htlc_update_msgs(initiator, &acceptor_node_id);
acceptor
.node
.handle_commitment_signed(initiator_node_id, &initiator_commit_sig.commitment_signed[0]);
check_added_monitors(acceptor, 1);

let acceptor_msg_events = acceptor.node.get_and_clear_pending_msg_events();
assert_eq!(acceptor_msg_events.len(), 2, "{acceptor_msg_events:?}");
for msg_event in &acceptor_msg_events {
match msg_event {
MessageSendEvent::UpdateHTLCs { updates, .. } => {
initiator
.node
.handle_commitment_signed(acceptor_node_id, &updates.commitment_signed[0]);
check_added_monitors(initiator, 1);
},
MessageSendEvent::SendTxSignatures { msg, .. } => {
initiator.node.handle_tx_signatures(acceptor_node_id, msg);
},
_ => panic!("Unexpected event"),
}
}

assert!(initiator.node.get_and_clear_pending_msg_events().is_empty());

initiator.enable_channel_signer_op(
&acceptor_node_id,
&channel_id,
SignerOp::SignSpliceSharedInput,
);
initiator.node.signer_unblocked(None);

let tx_signatures =
get_event_msg!(initiator, MessageSendEvent::SendTxSignatures, acceptor_node_id);
acceptor.node.handle_tx_signatures(initiator_node_id, &tx_signatures);

let _ = get_event!(initiator, Event::SplicePending);
let _ = get_event!(acceptor, Event::SplicePending);
}
118 changes: 91 additions & 27 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1205,8 +1205,7 @@ pub(super) struct SignerResumeUpdates {
pub accept_channel: Option<msgs::AcceptChannel>,
pub funding_created: Option<msgs::FundingCreated>,
pub funding_signed: Option<msgs::FundingSigned>,
pub funding_commit_sig: Option<msgs::CommitmentSigned>,
pub tx_signatures: Option<msgs::TxSignatures>,
pub funding_tx_signed: Option<FundingTxSigned>,
pub channel_ready: Option<msgs::ChannelReady>,
pub order: RAACommitmentOrder,
pub closing_signed: Option<msgs::ClosingSigned>,
Expand Down Expand Up @@ -1639,11 +1638,11 @@ where

#[rustfmt::skip]
pub fn signer_maybe_unblocked<L: Logger, CBP>(
&mut self, chain_hash: ChainHash, logger: &L, path_for_release_htlc: CBP
&mut self, chain_hash: ChainHash, best_block_height: u32, logger: &L, path_for_release_htlc: CBP
) -> Result<Option<SignerResumeUpdates>, ChannelError> where CBP: Fn(u64) -> BlindedMessagePath {
match &mut self.phase {
ChannelPhase::Undefined => unreachable!(),
ChannelPhase::Funded(chan) => chan.signer_maybe_unblocked(logger, path_for_release_htlc).map(|r| Some(r)),
ChannelPhase::Funded(chan) => chan.signer_maybe_unblocked(best_block_height, logger, path_for_release_htlc).map(|r| Some(r)),
ChannelPhase::UnfundedOutboundV1(chan) => {
let (open_channel, funding_created) = chan.signer_maybe_unblocked(chain_hash, logger);
Ok(Some(SignerResumeUpdates {
Expand All @@ -1653,8 +1652,7 @@ where
accept_channel: None,
funding_created,
funding_signed: None,
funding_commit_sig: None,
tx_signatures: None,
funding_tx_signed: None,
channel_ready: None,
order: chan.context.resend_order.clone(),
closing_signed: None,
Expand All @@ -1671,8 +1669,7 @@ where
accept_channel,
funding_created: None,
funding_signed: None,
funding_commit_sig: None,
tx_signatures: None,
funding_tx_signed: None,
channel_ready: None,
order: chan.context.resend_order.clone(),
closing_signed: None,
Expand Down Expand Up @@ -2213,17 +2210,29 @@ where

let shared_input_signature =
if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() {
let sig = context.holder_signer.sign_splice_shared_input(
&funding.channel_transaction_parameters,
tx,
splice_input_index as usize,
&context.secp_ctx,
);
Some(sig)
let sig = context
.holder_signer
.sign_splice_shared_input(
&funding.channel_transaction_parameters,
tx,
splice_input_index as usize,
&context.secp_ctx,
)
.ok();
if sig.is_none() {
log_debug!(
logger,
"Splice shared input signature not available, waiting on async signer"
);
}
sig
} else {
None
};
debug_assert_eq!(pending_splice.is_some(), shared_input_signature.is_some());
debug_assert_eq!(
pending_splice.is_some(),
signing_session.unsigned_tx().shared_input_index().is_some()
);

let tx_signatures = msgs::TxSignatures {
channel_id: context.channel_id,
Expand Down Expand Up @@ -9450,6 +9459,8 @@ where
}
}

let awaiting_holder_shared_input_signature =
signing_session.awaiting_holder_shared_input_signature();
let (holder_tx_signatures, funding_tx) =
signing_session.received_tx_signatures(msg).map_err(|msg| ChannelError::Warn(msg))?;

Expand Down Expand Up @@ -9488,6 +9499,11 @@ where
best_block_height,
&logger,
);
} else if awaiting_holder_shared_input_signature {
log_debug!(
logger,
"Waiting for funding transaction shared input signature before finalizing negotiation"
);
} else {
debug_assert!(
false,
Expand Down Expand Up @@ -9882,7 +9898,7 @@ where
/// blocked.
#[rustfmt::skip]
pub fn signer_maybe_unblocked<L: Logger, CBP>(
&mut self, logger: &L, path_for_release_htlc: CBP
&mut self, best_block_height: u32, logger: &L, path_for_release_htlc: CBP
) -> Result<SignerResumeUpdates, ChannelError> where CBP: Fn(u64) -> BlindedMessagePath {
if let Some((commitment_number, commitment_secret)) = self.context.signer_pending_stale_state_verification.clone() {
if let Ok(expected_point) = self
Expand Down Expand Up @@ -9938,16 +9954,65 @@ where
None
};

let tx_signatures = if funding_commit_sig.is_some() {
let mut shared_input_signature_unblocked = false;
{
if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() {
if signing_session.awaiting_holder_shared_input_signature() {
let splice_input_index = signing_session
.unsigned_tx()
.shared_input_index()
.expect("Missing shared input index while awaiting a splice signature");
log_trace!(logger, "Attempting to generate pending splice shared input signature...");
if let Ok(shared_input_signature) = self.context.holder_signer.sign_splice_shared_input(
&self.funding.channel_transaction_parameters,
signing_session.unsigned_tx().tx(),
splice_input_index as usize,
&self.context.secp_ctx,
) {
shared_input_signature_unblocked = true;
signing_session
.provide_holder_shared_input_signature(shared_input_signature)
.map_err(ChannelError::close)?;
}
}
}
}

let mut tx_signatures = None;
let mut funding_tx = None;
if funding_commit_sig.is_some() || shared_input_signature_unblocked {
if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() {
signing_session.holder_tx_signatures().filter(|_| !self.is_awaiting_monitor_update())
if !self.is_awaiting_monitor_update() && !self.context.signer_pending_funding {
tx_signatures = signing_session.holder_tx_signatures();
funding_tx = tx_signatures.as_ref().and_then(|_| signing_session.signed_tx());
}
} else {
debug_assert!(false);
None
}
} else {
None
};
}
Comment on lines +9957 to +9992
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: the return value of provide_holder_shared_input_signature (which includes (Option<TxSignatures>, Option<Transaction>)) is discarded at line 9974 and then re-fetched via signing_session.holder_tx_signatures() and signing_session.signed_tx() at lines 9986-9987. Each call to signed_tx() internally calls holder_tx_signatures() again, resulting in 3 total calls to holder_tx_signatures() (which clones and rebuilds each time). Not a correctness issue, but you could reuse the values from provide_holder_shared_input_signature to avoid redundant cloning.


let mut funding_tx_signed = None;
if funding_commit_sig.is_some() || tx_signatures.is_some() || funding_tx.is_some() {
let mut resumed = FundingTxSigned {
commitment_signed: funding_commit_sig,
counterparty_initial_commitment_signed_result: None,
tx_signatures,
funding_tx: None,
splice_negotiated: None,
splice_locked: None,
};
if let Some(funding_tx) = funding_tx {
let funding_logger = WithChannelContext::from(logger, &self.context, None);
debug_assert!(resumed.tx_signatures.is_some());
self.on_tx_signatures_exchange(
&mut resumed,
funding_tx,
best_block_height,
&funding_logger,
);
}
funding_tx_signed = Some(resumed);
}

// Provide a `channel_ready` message if we need to, but only if we're _not_ still pending
// funding.
Expand Down Expand Up @@ -10013,8 +10078,8 @@ where
if revoke_and_ack.is_some() { "a" } else { "no" },
self.context.resend_order,
if funding_signed.is_some() { "a" } else { "no" },
if funding_commit_sig.is_some() { "a" } else { "no" },
if tx_signatures.is_some() { "a" } else { "no" },
if funding_tx_signed.as_ref().map(|v| v.commitment_signed.is_some()).unwrap_or(false) { "a" } else { "no" },
if funding_tx_signed.as_ref().map(|v| v.tx_signatures.is_some()).unwrap_or(false) { "a" } else { "no" },
if channel_ready.is_some() { "a" } else { "no" },
if closing_signed.is_some() { "a" } else { "no" },
if signed_closing_tx.is_some() { "a" } else { "no" },
Expand All @@ -10027,8 +10092,7 @@ where
accept_channel: None,
funding_created: None,
funding_signed,
funding_commit_sig,
tx_signatures,
funding_tx_signed,
channel_ready,
order: self.context.resend_order.clone(),
closing_signed,
Expand Down
Loading
Loading