Skip to content

Commit ac3fdbf

Browse files
committed
Only promote V2 unfunded to Channel when receiving initial commitment signed
1 parent 6790187 commit ac3fdbf

File tree

3 files changed

+134
-80
lines changed

3 files changed

+134
-80
lines changed

Diff for: lightning/src/ln/channel.rs

+35-23
Original file line numberDiff line numberDiff line change
@@ -1801,15 +1801,19 @@ pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider
18011801
},
18021802
};
18031803

1804-
let funding_ready_for_sig_event = None;
1805-
if signing_session.local_inputs_count() == 0 {
1804+
let funding_ready_for_sig_event = if signing_session.local_inputs_count() == 0 {
18061805
debug_assert_eq!(our_funding_satoshis, 0);
18071806
if signing_session.provide_holder_witnesses(context.channel_id, Vec::new()).is_err() {
18081807
debug_assert!(
18091808
false,
18101809
"Zero inputs were provided & zero witnesses were provided, but a count mismatch was somehow found",
18111810
);
1811+
return Err(ChannelError::Close((
1812+
"V2 channel rejected due to sender error".into(),
1813+
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
1814+
)));
18121815
}
1816+
None
18131817
} else {
18141818
// TODO(dual_funding): Send event for signing if we've contributed funds.
18151819
// Inform the user that SIGHASH_ALL must be used for all signatures when contributing
@@ -1825,7 +1829,15 @@ pub(super) trait InteractivelyFunded<SP: Deref> where SP::Target: SignerProvider
18251829
// will prevent the funding transaction from being relayed on the bitcoin network and hence being
18261830
// confirmed.
18271831
// </div>
1828-
}
1832+
debug_assert!(
1833+
false,
1834+
"We don't support users providing inputs but somehow we had more than zero inputs",
1835+
);
1836+
return Err(ChannelError::Close((
1837+
"V2 channel rejected due to sender error".into(),
1838+
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) }
1839+
)));
1840+
};
18291841

18301842
context.channel_state = ChannelState::FundingNegotiated;
18311843

@@ -5693,7 +5705,7 @@ impl<SP: Deref> Channel<SP> where
56935705
}
56945706
}
56955707

5696-
pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<(Option<msgs::TxSignatures>, Option<Transaction>), ChannelError>
5708+
pub fn tx_signatures<L: Deref>(&mut self, msg: &msgs::TxSignatures, logger: &L) -> Result<Option<msgs::TxSignatures>, ChannelError>
56975709
where L::Target: Logger
56985710
{
56995711
if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
@@ -5730,12 +5742,10 @@ impl<SP: Deref> Channel<SP> where
57305742
// for spending. Doesn't seem to be anything in rust-bitcoin.
57315743
}
57325744

5733-
let (tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg.clone())
5745+
let (tx_signatures_opt, funding_tx) = signing_session.received_tx_signatures(msg.clone())
57345746
.map_err(|_| ChannelError::Warn("Witness count did not match contributed input count".to_string()))?;
5735-
if funding_tx_opt.is_some() {
5736-
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
5737-
}
5738-
self.context.funding_transaction = funding_tx_opt.clone();
5747+
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
5748+
self.context.funding_transaction = Some(funding_tx);
57395749

57405750
self.context.next_funding_txid = None;
57415751

@@ -5745,10 +5755,10 @@ impl<SP: Deref> Channel<SP> where
57455755
if tx_signatures_opt.is_some() && self.context.channel_state.is_monitor_update_in_progress() {
57465756
log_debug!(logger, "Not sending tx_signatures: a monitor update is in progress. Setting monitor_pending_tx_signatures.");
57475757
self.context.monitor_pending_tx_signatures = tx_signatures_opt;
5748-
return Ok((None, None));
5758+
return Ok(None);
57495759
}
57505760

5751-
Ok((tx_signatures_opt, funding_tx_opt))
5761+
Ok(tx_signatures_opt)
57525762
} else {
57535763
Err(ChannelError::Close((
57545764
"Unexpected tx_signatures. No funding transaction awaiting signatures".to_string(),
@@ -8726,6 +8736,8 @@ pub(super) struct OutboundV2Channel<SP: Deref> where SP::Target: SignerProvider
87268736
pub dual_funding_context: DualFundingChannelContext,
87278737
/// The current interactive transaction construction session under negotiation.
87288738
interactive_tx_constructor: Option<InteractiveTxConstructor>,
8739+
/// The signing session created after `tx_complete` handling
8740+
pub interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
87298741
}
87308742

87318743
impl<SP: Deref> OutboundV2Channel<SP> where SP::Target: SignerProvider {
@@ -8785,6 +8797,7 @@ impl<SP: Deref> OutboundV2Channel<SP> where SP::Target: SignerProvider {
87858797
our_funding_inputs: funding_inputs,
87868798
},
87878799
interactive_tx_constructor: None,
8800+
interactive_tx_signing_session: None,
87888801
};
87898802
Ok(chan)
87908803
}
@@ -8852,13 +8865,11 @@ impl<SP: Deref> OutboundV2Channel<SP> where SP::Target: SignerProvider {
88528865
}
88538866
}
88548867

8855-
pub fn into_channel(self, signing_session: InteractiveTxSigningSession) -> Result<Channel<SP>, ChannelError>{
8856-
let channel = Channel {
8868+
pub fn into_channel(self) -> Channel<SP> {
8869+
Channel {
88578870
context: self.context,
8858-
interactive_tx_signing_session: Some(signing_session),
8859-
};
8860-
8861-
Ok(channel)
8871+
interactive_tx_signing_session: self.interactive_tx_signing_session,
8872+
}
88628873
}
88638874
}
88648875

@@ -8869,6 +8880,8 @@ pub(super) struct InboundV2Channel<SP: Deref> where SP::Target: SignerProvider {
88698880
pub dual_funding_context: DualFundingChannelContext,
88708881
/// The current interactive transaction construction session under negotiation.
88718882
interactive_tx_constructor: Option<InteractiveTxConstructor>,
8883+
/// The signing session created after `tx_complete` handling
8884+
pub interactive_tx_signing_session: Option<InteractiveTxSigningSession>,
88728885
}
88738886

88748887
impl<SP: Deref> InboundV2Channel<SP> where SP::Target: SignerProvider {
@@ -8970,6 +8983,7 @@ impl<SP: Deref> InboundV2Channel<SP> where SP::Target: SignerProvider {
89708983
context,
89718984
dual_funding_context,
89728985
interactive_tx_constructor,
8986+
interactive_tx_signing_session: None,
89738987
unfunded_context: UnfundedChannelContext { unfunded_channel_age_ticks: 0 },
89748988
})
89758989
}
@@ -9046,13 +9060,11 @@ impl<SP: Deref> InboundV2Channel<SP> where SP::Target: SignerProvider {
90469060
self.generate_accept_channel_v2_message()
90479061
}
90489062

9049-
pub fn into_channel(self, signing_session: InteractiveTxSigningSession) -> Result<Channel<SP>, ChannelError>{
9050-
let channel = Channel {
9063+
pub fn into_channel(self) -> Channel<SP> {
9064+
Channel {
90519065
context: self.context,
9052-
interactive_tx_signing_session: Some(signing_session),
9053-
};
9054-
9055-
Ok(channel)
9066+
interactive_tx_signing_session: self.interactive_tx_signing_session,
9067+
}
90569068
}
90579069
}
90589070

Diff for: lightning/src/ln/channelmanager.rs

+94-38
Original file line numberDiff line numberDiff line change
@@ -8305,7 +8305,7 @@ where
83058305
peer_state.pending_msg_events.push(msg_send_event);
83068306
};
83078307
if let Some(mut signing_session) = signing_session_opt {
8308-
let (commitment_signed, funding_ready_for_sig_event_opt) = match chan_phase_entry.get_mut() {
8308+
let (commitment_signed, funding_ready_for_sig_event_opt) = match channel_phase {
83098309
ChannelPhase::UnfundedOutboundV2(chan) => {
83108310
chan.funding_tx_constructed(&mut signing_session, &self.logger)
83118311
},
@@ -8316,18 +8316,17 @@ where
83168316
"Got a tx_complete message with no interactive transaction construction expected or in-progress"
83178317
.into())),
83188318
}.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?;
8319-
let (channel_id, channel_phase) = chan_phase_entry.remove_entry();
8320-
let channel = match channel_phase {
8321-
ChannelPhase::UnfundedOutboundV2(chan) => chan.into_channel(signing_session),
8322-
ChannelPhase::UnfundedInboundV2(chan) => chan.into_channel(signing_session),
8319+
match channel_phase {
8320+
ChannelPhase::UnfundedOutboundV2(chan) => chan.interactive_tx_signing_session = Some(signing_session),
8321+
ChannelPhase::UnfundedInboundV2(chan) => chan.interactive_tx_signing_session = Some(signing_session),
83238322
_ => {
83248323
debug_assert!(false); // It cannot be another variant as we are in the `Ok` branch of the above match.
8325-
Err(ChannelError::Warn(
8324+
let err = ChannelError::Warn(
83268325
"Got a tx_complete message with no interactive transaction construction expected or in-progress"
8327-
.into()))
8326+
.into());
8327+
return Err(MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id));
83288328
},
8329-
}.map_err(|err| MsgHandleErrInternal::send_err_msg_no_close(format!("{}", err), msg.channel_id))?;
8330-
peer_state.channel_by_id.insert(channel_id, ChannelPhase::Funded(channel));
8329+
}
83318330
if let Some(funding_ready_for_sig_event) = funding_ready_for_sig_event_opt {
83328331
let mut pending_events = self.pending_events.lock().unwrap();
83338332
pending_events.push_back((funding_ready_for_sig_event, None));
@@ -8370,14 +8369,14 @@ where
83708369
match channel_phase {
83718370
ChannelPhase::Funded(chan) => {
83728371
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
8373-
let (tx_signatures_opt, funding_tx_opt) = try_chan_phase_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_phase_entry);
8372+
let tx_signatures_opt = try_chan_phase_entry!(self, peer_state, chan.tx_signatures(msg, &&logger), chan_phase_entry);
83748373
if let Some(tx_signatures) = tx_signatures_opt {
83758374
peer_state.pending_msg_events.push(events::MessageSendEvent::SendTxSignatures {
83768375
node_id: *counterparty_node_id,
83778376
msg: tx_signatures,
83788377
});
83798378
}
8380-
if let Some(ref funding_tx) = funding_tx_opt {
8379+
if let Some(ref funding_tx) = chan.context.unbroadcasted_funding() {
83818380
self.tx_broadcaster.broadcast_transactions(&[funding_tx]);
83828381
{
83838382
let mut pending_events = self.pending_events.lock().unwrap();
@@ -8840,46 +8839,103 @@ where
88408839
})?;
88418840
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
88428841
let peer_state = &mut *peer_state_lock;
8843-
match peer_state.channel_by_id.entry(msg.channel_id) {
8842+
let (channel_id, mut chan) = match peer_state.channel_by_id.entry(msg.channel_id) {
88448843
hash_map::Entry::Occupied(mut chan_phase_entry) => {
8845-
if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() {
8846-
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
8847-
let funding_txo = chan.context.get_funding_txo();
8848-
8849-
if chan.interactive_tx_signing_session.is_some() {
8850-
let monitor = try_chan_phase_entry!(
8851-
self, peer_state, chan.commitment_signed_initial_v2(msg, best_block, &self.signer_provider, &&logger),
8852-
chan_phase_entry);
8853-
let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
8854-
if let Ok(persist_state) = monitor_res {
8855-
handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
8856-
per_peer_state, chan, INITIAL_MONITOR);
8844+
let channel_phase = chan_phase_entry.get_mut();
8845+
match channel_phase {
8846+
ChannelPhase::UnfundedOutboundV2(chan) => {
8847+
if chan.interactive_tx_signing_session.is_some() {
8848+
let (channel_id, mut channel_phase) = chan_phase_entry.remove_entry();
8849+
match channel_phase {
8850+
ChannelPhase::UnfundedOutboundV2(chan) => {
8851+
(channel_id, chan.into_channel())
8852+
}
8853+
_ => {
8854+
debug_assert!(false, "The channel phase was not UnfundedOutboundV2");
8855+
let err = ChannelError::close(
8856+
"Closing due to unexpected sender error".into());
8857+
return Err(convert_chan_phase_err!(self, peer_state, err, &mut channel_phase,
8858+
&channel_id).1)
8859+
}
8860+
}
88578861
} else {
8858-
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
8859-
log_error!(logger, "Persisting initial ChannelMonitor failed, implying the funding outpoint was duplicated");
8860-
try_chan_phase_entry!(self, peer_state, Err(ChannelError::Close(
8861-
(
8862-
"Channel funding outpoint was a duplicate".to_owned(),
8863-
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
8864-
)
8865-
)), chan_phase_entry)
8862+
return try_chan_phase_entry!(self, peer_state, Err(ChannelError::close(
8863+
"Got a commitment_signed message for a V2 channel before funding transaction constructed!".into())), chan_phase_entry);
88668864
}
8867-
} else {
8865+
},
8866+
ChannelPhase::UnfundedInboundV2(chan) => {
8867+
// TODO(dual_funding): This should be somewhat DRYable when #3418 is merged.
8868+
if chan.interactive_tx_signing_session.is_some() {
8869+
let (channel_id, mut channel_phase) = chan_phase_entry.remove_entry();
8870+
match channel_phase {
8871+
ChannelPhase::UnfundedInboundV2(chan) => {
8872+
(channel_id, chan.into_channel())
8873+
}
8874+
_ => {
8875+
debug_assert!(false, "The channel phase was not UnfundedInboundV2");
8876+
let err = ChannelError::close(
8877+
"Closing due to unexpected sender error".into());
8878+
return Err(convert_chan_phase_err!(self, peer_state, err, &mut channel_phase,
8879+
&channel_id).1)
8880+
}
8881+
}
8882+
} else {
8883+
return try_chan_phase_entry!(self, peer_state, Err(ChannelError::close(
8884+
"Got a commitment_signed message for a V2 channel before funding transaction constructed!".into())), chan_phase_entry);
8885+
}
8886+
},
8887+
ChannelPhase::Funded(chan) => {
8888+
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
8889+
let funding_txo = chan.context.get_funding_txo();
88688890
let monitor_update_opt = try_chan_phase_entry!(
88698891
self, peer_state, chan.commitment_signed(msg, &&logger), chan_phase_entry);
88708892
if let Some(monitor_update) = monitor_update_opt {
88718893
handle_new_monitor_update!(self, funding_txo.unwrap(), monitor_update, peer_state_lock,
88728894
peer_state, per_peer_state, chan);
88738895
}
8896+
return Ok(())
8897+
},
8898+
_ => {
8899+
return try_chan_phase_entry!(self, peer_state, Err(ChannelError::close(
8900+
"Got a commitment_signed message for an unfunded channel!".into())), chan_phase_entry);
88748901
}
8875-
Ok(())
8876-
} else {
8877-
return try_chan_phase_entry!(self, peer_state, Err(ChannelError::close(
8878-
"Got a commitment_signed message for an unfunded channel!".into())), chan_phase_entry);
88798902
}
88808903
},
8881-
hash_map::Entry::Vacant(_) => 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.channel_id))
8904+
hash_map::Entry::Vacant(_) => return Err(MsgHandleErrInternal::send_err_msg_no_close(
8905+
format!("Got a message for a channel from the wrong node! No such channel for the passed counterparty_node_id {}",
8906+
counterparty_node_id), msg.channel_id))
8907+
};
8908+
let logger = WithChannelContext::from(&self.logger, &chan.context, None);
8909+
let monitor = match chan.commitment_signed_initial_v2(msg, best_block, &self.signer_provider, &&logger) {
8910+
Ok(monitor) => monitor,
8911+
Err(err) => return Err(convert_chan_phase_err!(self, peer_state, err, &mut ChannelPhase::Funded(chan), &channel_id).1),
8912+
};
8913+
let monitor_res = self.chain_monitor.watch_channel(monitor.get_funding_txo().0, monitor);
8914+
if let Ok(persist_state) = monitor_res {
8915+
let mut occupied_entry = peer_state.channel_by_id.entry(channel_id).insert(ChannelPhase::Funded(chan));
8916+
let channel_phase_entry = occupied_entry.get_mut();
8917+
match channel_phase_entry {
8918+
ChannelPhase::Funded(chan) => { handle_new_monitor_update!(self, persist_state, peer_state_lock, peer_state,
8919+
per_peer_state, chan, INITIAL_MONITOR); },
8920+
channel_phase => {
8921+
debug_assert!(false, "Expected a ChannelPhase::Funded");
8922+
let err = ChannelError::close(
8923+
"Closing due to unexpected sender error".into());
8924+
return Err(convert_chan_phase_err!(self, peer_state, err, channel_phase,
8925+
&channel_id).1)
8926+
},
8927+
}
8928+
} else {
8929+
log_error!(logger, "Persisting initial ChannelMonitor failed, implying the funding outpoint was duplicated");
8930+
let err = ChannelError::Close(
8931+
(
8932+
"Channel funding outpoint was a duplicate".to_owned(),
8933+
ClosureReason::HolderForceClosed { broadcasted_latest_txn: Some(false) },
8934+
)
8935+
);
8936+
return Err(convert_chan_phase_err!(self, peer_state, err, &mut ChannelPhase::Funded(chan), &channel_id).1);
88828937
}
8938+
Ok(())
88838939
}
88848940

88858941
fn push_decode_update_add_htlcs(&self, mut update_add_htlcs: (u64, Vec<msgs::UpdateAddHTLC>)) {

Diff for: lightning/src/ln/interactivetxs.rs

+5-19
Original file line numberDiff line numberDiff line change
@@ -320,10 +320,7 @@ impl InteractiveTxSigningSession {
320320
/// unsigned transaction.
321321
pub fn received_tx_signatures(
322322
&mut self, tx_signatures: TxSignatures,
323-
) -> Result<(Option<TxSignatures>, Option<Transaction>), ()> {
324-
if self.counterparty_sent_tx_signatures {
325-
return Ok((None, None));
326-
};
323+
) -> Result<(Option<TxSignatures>, Transaction), ()> {
327324
if self.remote_inputs_count() != tx_signatures.witnesses.len() {
328325
return Err(());
329326
}
@@ -336,13 +333,7 @@ impl InteractiveTxSigningSession {
336333
None
337334
};
338335

339-
let funding_tx = if self.holder_tx_signatures.is_some() {
340-
Some(self.finalize_funding_tx())
341-
} else {
342-
None
343-
};
344-
345-
Ok((holder_tx_signatures, funding_tx))
336+
Ok((holder_tx_signatures, self.finalize_funding_tx()))
346337
}
347338

348339
/// Provides the holder witnesses for the unsigned transaction.
@@ -351,7 +342,7 @@ impl InteractiveTxSigningSession {
351342
/// unsigned transaction.
352343
pub fn provide_holder_witnesses(
353344
&mut self, channel_id: ChannelId, witnesses: Vec<Witness>,
354-
) -> Result<Option<TxSignatures>, ()> {
345+
) -> Result<(), ()> {
355346
if self.local_inputs_count() != witnesses.len() {
356347
return Err(());
357348
}
@@ -363,13 +354,8 @@ impl InteractiveTxSigningSession {
363354
witnesses: witnesses.into_iter().collect(),
364355
shared_input_signature: None,
365356
});
366-
if self.received_commitment_signed
367-
&& (self.holder_sends_tx_signatures_first || self.counterparty_sent_tx_signatures)
368-
{
369-
Ok(self.holder_tx_signatures.clone())
370-
} else {
371-
Ok(None)
372-
}
357+
358+
Ok(())
373359
}
374360

375361
pub fn remote_inputs_count(&self) -> usize {

0 commit comments

Comments
 (0)