Skip to content

Commit 6ab56a4

Browse files
TheBlueMattTheBlueMatt
committed
Move to Funded after funding_signed rather than on funding
Previously, channels were stored in different maps in `PeerState` based on whether the funding had been set, keeping the keys across the maps consistent (pre-funding temporary_channel_ids vs funding-outpoint-based channel_ids). However, channels are now stored in a single `channel_by_id` map, making that point moot. Instead, here, we convert the `ChannelPhase` state transition boundary to "once we have a `ChannelMonitor`", which makes more sense now, and was actually the original proposed boundary. This also requires calling `signer_maybe_unblocked` on a pre-funded outbound channel, but that nicely also lets us limit the scope of `FundingCreated` message generation, which we do in the next commit.
1 parent 2d26679 commit 6ab56a4

File tree

3 files changed

+193
-166
lines changed

3 files changed

+193
-166
lines changed

‎lightning/src/ln/channel.rs‎

Lines changed: 125 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,6 @@ pub(super) struct MonitorRestoreUpdates {
793793
pub(super) struct SignerResumeUpdates {
794794
pub commitment_update: Option<msgs::CommitmentUpdate>,
795795
pub funding_signed: Option<msgs::FundingSigned>,
796-
pub funding_created: Option<msgs::FundingCreated>,
797796
pub channel_ready: Option<msgs::ChannelReady>,
798797
}
799798

@@ -2942,99 +2941,6 @@ impl<SP: Deref> Channel<SP> where
29422941
}
29432942

29442943
// Message handlers:
2945-
2946-
/// Handles a funding_signed message from the remote end.
2947-
/// If this call is successful, broadcast the funding transaction (and not before!)
2948-
pub fn funding_signed<L: Deref>(
2949-
&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
2950-
) -> Result<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, ChannelError>
2951-
where
2952-
L::Target: Logger
2953-
{
2954-
if !self.context.is_outbound() {
2955-
return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
2956-
}
2957-
if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
2958-
return Err(ChannelError::Close("Received funding_signed in strange state!".to_owned()));
2959-
}
2960-
if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
2961-
self.context.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
2962-
self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
2963-
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
2964-
}
2965-
2966-
let funding_script = self.context.get_funding_redeemscript();
2967-
2968-
let counterparty_keys = self.context.build_remote_transaction_keys();
2969-
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
2970-
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
2971-
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
2972-
2973-
log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
2974-
&self.context.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
2975-
2976-
let holder_signer = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
2977-
let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
2978-
{
2979-
let trusted_tx = initial_commitment_tx.trust();
2980-
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
2981-
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.context.channel_value_satoshis);
2982-
// They sign our commitment transaction, allowing us to broadcast the tx if we wish.
2983-
if let Err(_) = self.context.secp_ctx.verify_ecdsa(&sighash, &msg.signature, &self.context.get_counterparty_pubkeys().funding_pubkey) {
2984-
return Err(ChannelError::Close("Invalid funding_signed signature from peer".to_owned()));
2985-
}
2986-
}
2987-
2988-
let holder_commitment_tx = HolderCommitmentTransaction::new(
2989-
initial_commitment_tx,
2990-
msg.signature,
2991-
Vec::new(),
2992-
&self.context.get_holder_pubkeys().funding_pubkey,
2993-
self.context.counterparty_funding_pubkey()
2994-
);
2995-
2996-
self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, Vec::new())
2997-
.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
2998-
2999-
3000-
let funding_redeemscript = self.context.get_funding_redeemscript();
3001-
let funding_txo = self.context.get_funding_txo().unwrap();
3002-
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
3003-
let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.context.get_holder_pubkeys().payment_point, &self.context.get_counterparty_pubkeys().payment_point, self.context.is_outbound());
3004-
let shutdown_script = self.context.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
3005-
let mut monitor_signer = signer_provider.derive_channel_signer(self.context.channel_value_satoshis, self.context.channel_keys_id);
3006-
monitor_signer.provide_channel_parameters(&self.context.channel_transaction_parameters);
3007-
let channel_monitor = ChannelMonitor::new(self.context.secp_ctx.clone(), monitor_signer,
3008-
shutdown_script, self.context.get_holder_selected_contest_delay(),
3009-
&self.context.destination_script, (funding_txo, funding_txo_script),
3010-
&self.context.channel_transaction_parameters,
3011-
funding_redeemscript.clone(), self.context.channel_value_satoshis,
3012-
obscure_factor,
3013-
holder_commitment_tx, best_block, self.context.counterparty_node_id);
3014-
channel_monitor.provide_initial_counterparty_commitment_tx(
3015-
counterparty_initial_bitcoin_tx.txid, Vec::new(),
3016-
self.context.cur_counterparty_commitment_transaction_number,
3017-
self.context.counterparty_cur_commitment_point.unwrap(),
3018-
counterparty_initial_commitment_tx.feerate_per_kw(),
3019-
counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
3020-
counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
3021-
3022-
assert!(!self.context.channel_state.is_monitor_update_in_progress()); // We have no had any monitor(s) yet to fail update!
3023-
if self.context.is_batch_funding() {
3024-
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
3025-
} else {
3026-
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
3027-
}
3028-
self.context.cur_holder_commitment_transaction_number -= 1;
3029-
self.context.cur_counterparty_commitment_transaction_number -= 1;
3030-
3031-
log_info!(logger, "Received funding_signed from peer for channel {}", &self.context.channel_id());
3032-
3033-
let need_channel_ready = self.check_get_channel_ready(0).is_some();
3034-
self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
3035-
Ok(channel_monitor)
3036-
}
3037-
30382944
/// Updates the state of the channel to indicate that all channels in the batch have received
30392945
/// funding_signed and persisted their monitors.
30402946
/// The funding transaction is consequently allowed to be broadcast, and the channel can be
@@ -4308,20 +4214,15 @@ impl<SP: Deref> Channel<SP> where
43084214
let channel_ready = if funding_signed.is_some() {
43094215
self.check_get_channel_ready(0)
43104216
} else { None };
4311-
let funding_created = if self.context.signer_pending_funding && self.context.is_outbound() {
4312-
self.context.get_funding_created_msg(logger)
4313-
} else { None };
43144217

4315-
log_trace!(logger, "Signer unblocked with {} commitment_update, {} funding_signed, {} funding_created, and {} channel_ready",
4218+
log_trace!(logger, "Signer unblocked with {} commitment_update, {} funding_signed and {} channel_ready",
43164219
if commitment_update.is_some() { "a" } else { "no" },
43174220
if funding_signed.is_some() { "a" } else { "no" },
4318-
if funding_created.is_some() { "a" } else { "no" },
43194221
if channel_ready.is_some() { "a" } else { "no" });
43204222

43214223
SignerResumeUpdates {
43224224
commitment_update,
43234225
funding_signed,
4324-
funding_created,
43254226
channel_ready,
43264227
}
43274228
}
@@ -6445,8 +6346,8 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
64456346
/// Note that channel_id changes during this call!
64466347
/// Do NOT broadcast the funding transaction until after a successful funding_signed call!
64476348
/// If an Err is returned, it is a ChannelError::Close.
6448-
pub fn get_funding_created<L: Deref>(mut self, funding_transaction: Transaction, funding_txo: OutPoint, is_batch_funding: bool, logger: &L)
6449-
-> Result<(Channel<SP>, Option<msgs::FundingCreated>), (Self, ChannelError)> where L::Target: Logger {
6349+
pub fn get_funding_created<L: Deref>(&mut self, funding_transaction: Transaction, funding_txo: OutPoint, is_batch_funding: bool, logger: &L)
6350+
-> Result<Option<msgs::FundingCreated>, (Self, ChannelError)> where L::Target: Logger {
64506351
if !self.context.is_outbound() {
64516352
panic!("Tried to create outbound funding_created message on an inbound channel!");
64526353
}
@@ -6489,11 +6390,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
64896390
}
64906391
}
64916392

6492-
let channel = Channel {
6493-
context: self.context,
6494-
};
6495-
6496-
Ok((channel, funding_created))
6393+
Ok(funding_created)
64976394
}
64986395

64996396
fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) -> ChannelTypeFeatures {
@@ -6736,6 +6633,112 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
67366633

67376634
Ok(())
67386635
}
6636+
6637+
/// Handles a funding_signed message from the remote end.
6638+
/// If this call is successful, broadcast the funding transaction (and not before!)
6639+
pub fn funding_signed<L: Deref>(
6640+
mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
6641+
) -> Result<(Channel<SP>, ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>), (OutboundV1Channel<SP>, ChannelError)>
6642+
where
6643+
L::Target: Logger
6644+
{
6645+
if !self.context.is_outbound() {
6646+
return Err((self, ChannelError::Close("Received funding_signed for an inbound channel?".to_owned())));
6647+
}
6648+
if !matches!(self.context.channel_state, ChannelState::FundingNegotiated) {
6649+
return Err((self, ChannelError::Close("Received funding_signed in strange state!".to_owned())));
6650+
}
6651+
if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
6652+
self.context.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
6653+
self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
6654+
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
6655+
}
6656+
6657+
let funding_script = self.context.get_funding_redeemscript();
6658+
6659+
let counterparty_keys = self.context.build_remote_transaction_keys();
6660+
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
6661+
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
6662+
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
6663+
6664+
log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
6665+
&self.context.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
6666+
6667+
let holder_signer = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
6668+
let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
6669+
{
6670+
let trusted_tx = initial_commitment_tx.trust();
6671+
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
6672+
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.context.channel_value_satoshis);
6673+
// They sign our commitment transaction, allowing us to broadcast the tx if we wish.
6674+
if let Err(_) = self.context.secp_ctx.verify_ecdsa(&sighash, &msg.signature, &self.context.get_counterparty_pubkeys().funding_pubkey) {
6675+
return Err((self, ChannelError::Close("Invalid funding_signed signature from peer".to_owned())));
6676+
}
6677+
}
6678+
6679+
let holder_commitment_tx = HolderCommitmentTransaction::new(
6680+
initial_commitment_tx,
6681+
msg.signature,
6682+
Vec::new(),
6683+
&self.context.get_holder_pubkeys().funding_pubkey,
6684+
self.context.counterparty_funding_pubkey()
6685+
);
6686+
6687+
let validated =
6688+
self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, Vec::new());
6689+
if validated.is_err() {
6690+
return Err((self, ChannelError::Close("Failed to validate our commitment".to_owned())));
6691+
}
6692+
6693+
let funding_redeemscript = self.context.get_funding_redeemscript();
6694+
let funding_txo = self.context.get_funding_txo().unwrap();
6695+
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
6696+
let obscure_factor = get_commitment_transaction_number_obscure_factor(&self.context.get_holder_pubkeys().payment_point, &self.context.get_counterparty_pubkeys().payment_point, self.context.is_outbound());
6697+
let shutdown_script = self.context.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
6698+
let mut monitor_signer = signer_provider.derive_channel_signer(self.context.channel_value_satoshis, self.context.channel_keys_id);
6699+
monitor_signer.provide_channel_parameters(&self.context.channel_transaction_parameters);
6700+
let channel_monitor = ChannelMonitor::new(self.context.secp_ctx.clone(), monitor_signer,
6701+
shutdown_script, self.context.get_holder_selected_contest_delay(),
6702+
&self.context.destination_script, (funding_txo, funding_txo_script),
6703+
&self.context.channel_transaction_parameters,
6704+
funding_redeemscript.clone(), self.context.channel_value_satoshis,
6705+
obscure_factor,
6706+
holder_commitment_tx, best_block, self.context.counterparty_node_id);
6707+
channel_monitor.provide_initial_counterparty_commitment_tx(
6708+
counterparty_initial_bitcoin_tx.txid, Vec::new(),
6709+
self.context.cur_counterparty_commitment_transaction_number,
6710+
self.context.counterparty_cur_commitment_point.unwrap(),
6711+
counterparty_initial_commitment_tx.feerate_per_kw(),
6712+
counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
6713+
counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
6714+
6715+
assert!(!self.context.channel_state.is_monitor_update_in_progress()); // We have no had any monitor(s) yet to fail update!
6716+
if self.context.is_batch_funding() {
6717+
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::WAITING_FOR_BATCH);
6718+
} else {
6719+
self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new());
6720+
}
6721+
self.context.cur_holder_commitment_transaction_number -= 1;
6722+
self.context.cur_counterparty_commitment_transaction_number -= 1;
6723+
6724+
log_info!(logger, "Received funding_signed from peer for channel {}", &self.context.channel_id());
6725+
6726+
let mut channel = Channel { context: self.context };
6727+
6728+
let need_channel_ready = channel.check_get_channel_ready(0).is_some();
6729+
channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
6730+
Ok((channel, channel_monitor))
6731+
}
6732+
6733+
/// Indicates that the signer may have some signatures for us, so we should retry if we're
6734+
/// blocked.
6735+
#[allow(unused)]
6736+
pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> where L::Target: Logger {
6737+
if self.context.signer_pending_funding && self.context.is_outbound() {
6738+
log_trace!(logger, "Signer unblocked a funding_created");
6739+
self.context.get_funding_created_msg(logger)
6740+
} else { None }
6741+
}
67396742
}
67406743

67416744
/// A not-yet-funded inbound (from counterparty) channel using V1 channel establishment.
@@ -8364,11 +8367,12 @@ mod tests {
83648367
value: 10000000, script_pubkey: output_script.clone(),
83658368
}]};
83668369
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
8367-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
8370+
let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
83688371
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
83698372

83708373
// Node B --> Node A: funding signed
8371-
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
8374+
let res = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger);
8375+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
83728376

83738377
// Put some inbound and outbound HTLCs in A's channel.
83748378
let htlc_amount_msat = 11_092_000; // put an amount below A's effective dust limit but above B's.
@@ -8492,11 +8496,12 @@ mod tests {
84928496
value: 10000000, script_pubkey: output_script.clone(),
84938497
}]};
84948498
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
8495-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
8499+
let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
84968500
let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
84978501

84988502
// Node B --> Node A: funding signed
8499-
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
8503+
let res = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger);
8504+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
85008505

85018506
// Now disconnect the two nodes and check that the commitment point in
85028507
// Node B's channel_reestablish message is sane.
@@ -8680,11 +8685,12 @@ mod tests {
86808685
value: 10000000, script_pubkey: output_script.clone(),
86818686
}]};
86828687
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
8683-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
8688+
let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
86848689
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
86858690

86868691
// Node B --> Node A: funding signed
8687-
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
8692+
let res = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger);
8693+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
86888694

86898695
// Make sure that receiving a channel update will update the Channel as expected.
86908696
let update = ChannelUpdate {
@@ -9850,11 +9856,8 @@ mod tests {
98509856
},
98519857
]};
98529858
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
9853-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(
9854-
tx.clone(),
9855-
funding_outpoint,
9856-
true,
9857-
&&logger,
9859+
let funding_created_msg = node_a_chan.get_funding_created(
9860+
tx.clone(), funding_outpoint, true, &&logger,
98589861
).map_err(|_| ()).unwrap();
98599862
let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(
98609863
&funding_created_msg.unwrap(),
@@ -9872,12 +9875,10 @@ mod tests {
98729875

98739876
// Receive funding_signed, but the channel will be configured to hold sending channel_ready and
98749877
// broadcasting the funding transaction until the batch is ready.
9875-
let _ = node_a_chan.funding_signed(
9876-
&funding_signed_msg.unwrap(),
9877-
best_block,
9878-
&&keys_provider,
9879-
&&logger,
9880-
).unwrap();
9878+
let res = node_a_chan.funding_signed(
9879+
&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger,
9880+
);
9881+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
98819882
let node_a_updates = node_a_chan.monitor_updating_restored(
98829883
&&logger,
98839884
&&keys_provider,

0 commit comments

Comments
 (0)