Skip to content

Commit 1de0f5f

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 4fc3a17 commit 1de0f5f

File tree

3 files changed

+189
-164
lines changed

3 files changed

+189
-164
lines changed

‎lightning/src/ln/channel.rs‎

Lines changed: 127 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -542,7 +542,6 @@ pub(super) struct MonitorRestoreUpdates {
542542
pub(super) struct SignerResumeUpdates {
543543
pub commitment_update: Option<msgs::CommitmentUpdate>,
544544
pub funding_signed: Option<msgs::FundingSigned>,
545-
pub funding_created: Option<msgs::FundingCreated>,
546545
pub channel_ready: Option<msgs::ChannelReady>,
547546
}
548547

@@ -2672,99 +2671,6 @@ impl<SP: Deref> Channel<SP> where
26722671

26732672
// Message handlers:
26742673

2675-
/// Handles a funding_signed message from the remote end.
2676-
/// If this call is successful, broadcast the funding transaction (and not before!)
2677-
pub fn funding_signed<L: Deref>(
2678-
&mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
2679-
) -> Result<ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, ChannelError>
2680-
where
2681-
L::Target: Logger
2682-
{
2683-
if !self.context.is_outbound() {
2684-
return Err(ChannelError::Close("Received funding_signed for an inbound channel?".to_owned()));
2685-
}
2686-
if self.context.channel_state & !(ChannelState::MonitorUpdateInProgress as u32) != ChannelState::FundingCreated as u32 {
2687-
return Err(ChannelError::Close("Received funding_signed in strange state!".to_owned()));
2688-
}
2689-
if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
2690-
self.context.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
2691-
self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
2692-
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
2693-
}
2694-
2695-
let funding_script = self.context.get_funding_redeemscript();
2696-
2697-
let counterparty_keys = self.context.build_remote_transaction_keys();
2698-
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
2699-
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
2700-
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
2701-
2702-
log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
2703-
&self.context.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
2704-
2705-
let holder_signer = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
2706-
let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
2707-
{
2708-
let trusted_tx = initial_commitment_tx.trust();
2709-
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
2710-
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.context.channel_value_satoshis);
2711-
// They sign our commitment transaction, allowing us to broadcast the tx if we wish.
2712-
if let Err(_) = self.context.secp_ctx.verify_ecdsa(&sighash, &msg.signature, &self.context.get_counterparty_pubkeys().funding_pubkey) {
2713-
return Err(ChannelError::Close("Invalid funding_signed signature from peer".to_owned()));
2714-
}
2715-
}
2716-
2717-
let holder_commitment_tx = HolderCommitmentTransaction::new(
2718-
initial_commitment_tx,
2719-
msg.signature,
2720-
Vec::new(),
2721-
&self.context.get_holder_pubkeys().funding_pubkey,
2722-
self.context.counterparty_funding_pubkey()
2723-
);
2724-
2725-
self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, Vec::new())
2726-
.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;
2727-
2728-
2729-
let funding_redeemscript = self.context.get_funding_redeemscript();
2730-
let funding_txo = self.context.get_funding_txo().unwrap();
2731-
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
2732-
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());
2733-
let shutdown_script = self.context.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
2734-
let mut monitor_signer = signer_provider.derive_channel_signer(self.context.channel_value_satoshis, self.context.channel_keys_id);
2735-
monitor_signer.provide_channel_parameters(&self.context.channel_transaction_parameters);
2736-
let channel_monitor = ChannelMonitor::new(self.context.secp_ctx.clone(), monitor_signer,
2737-
shutdown_script, self.context.get_holder_selected_contest_delay(),
2738-
&self.context.destination_script, (funding_txo, funding_txo_script),
2739-
&self.context.channel_transaction_parameters,
2740-
funding_redeemscript.clone(), self.context.channel_value_satoshis,
2741-
obscure_factor,
2742-
holder_commitment_tx, best_block, self.context.counterparty_node_id);
2743-
2744-
channel_monitor.provide_initial_counterparty_commitment_tx(
2745-
counterparty_initial_bitcoin_tx.txid, Vec::new(),
2746-
self.context.cur_counterparty_commitment_transaction_number,
2747-
self.context.counterparty_cur_commitment_point.unwrap(),
2748-
counterparty_initial_commitment_tx.feerate_per_kw(),
2749-
counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
2750-
counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
2751-
2752-
assert_eq!(self.context.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update!
2753-
if self.context.is_batch_funding() {
2754-
self.context.channel_state = ChannelState::FundingSent as u32 | ChannelState::WaitingForBatch as u32;
2755-
} else {
2756-
self.context.channel_state = ChannelState::FundingSent as u32;
2757-
}
2758-
self.context.cur_holder_commitment_transaction_number -= 1;
2759-
self.context.cur_counterparty_commitment_transaction_number -= 1;
2760-
2761-
log_info!(logger, "Received funding_signed from peer for channel {}", &self.context.channel_id());
2762-
2763-
let need_channel_ready = self.check_get_channel_ready(0).is_some();
2764-
self.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
2765-
Ok(channel_monitor)
2766-
}
2767-
27682674
/// Updates the state of the channel to indicate that all channels in the batch have received
27692675
/// funding_signed and persisted their monitors.
27702676
/// The funding transaction is consequently allowed to be broadcast, and the channel can be
@@ -4029,20 +3935,15 @@ impl<SP: Deref> Channel<SP> where
40293935
let channel_ready = if funding_signed.is_some() {
40303936
self.check_get_channel_ready(0)
40313937
} else { None };
4032-
let funding_created = if self.context.signer_pending_funding && self.context.is_outbound() {
4033-
self.context.get_funding_created_msg(logger)
4034-
} else { None };
40353938

4036-
log_trace!(logger, "Signer unblocked with {} commitment_update, {} funding_signed, {} funding_created, and {} channel_ready",
3939+
log_trace!(logger, "Signer unblocked with {} commitment_update, {} funding_signed and {} channel_ready",
40373940
if commitment_update.is_some() { "a" } else { "no" },
40383941
if funding_signed.is_some() { "a" } else { "no" },
4039-
if funding_created.is_some() { "a" } else { "no" },
40403942
if channel_ready.is_some() { "a" } else { "no" });
40413943

40423944
SignerResumeUpdates {
40433945
commitment_update,
40443946
funding_signed,
4045-
funding_created,
40463947
channel_ready,
40473948
}
40483949
}
@@ -6134,8 +6035,8 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
61346035
/// Note that channel_id changes during this call!
61356036
/// Do NOT broadcast the funding transaction until after a successful funding_signed call!
61366037
/// If an Err is returned, it is a ChannelError::Close.
6137-
pub fn get_funding_created<L: Deref>(mut self, funding_transaction: Transaction, funding_txo: OutPoint, is_batch_funding: bool, logger: &L)
6138-
-> Result<(Channel<SP>, Option<msgs::FundingCreated>), (Self, ChannelError)> where L::Target: Logger {
6038+
pub fn get_funding_created<L: Deref>(&mut self, funding_transaction: Transaction, funding_txo: OutPoint, is_batch_funding: bool, logger: &L)
6039+
-> Result<Option<msgs::FundingCreated>, (Self, ChannelError)> where L::Target: Logger {
61396040
if !self.context.is_outbound() {
61406041
panic!("Tried to create outbound funding_created message on an inbound channel!");
61416042
}
@@ -6175,13 +6076,10 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
61756076
}
61766077
}
61776078

6178-
let channel = Channel {
6179-
context: self.context,
6180-
};
6181-
6182-
Ok((channel, funding_created))
6079+
Ok(funding_created)
61836080
}
61846081

6082+
61856083
fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) -> ChannelTypeFeatures {
61866084
// The default channel type (ie the first one we try) depends on whether the channel is
61876085
// public - if it is, we just go with `only_static_remotekey` as it's the only option
@@ -6413,6 +6311,113 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
64136311

64146312
Ok(())
64156313
}
6314+
6315+
/// Handles a funding_signed message from the remote end.
6316+
/// If this call is successful, broadcast the funding transaction (and not before!)
6317+
pub fn funding_signed<L: Deref>(
6318+
mut self, msg: &msgs::FundingSigned, best_block: BestBlock, signer_provider: &SP, logger: &L
6319+
) -> Result<(Channel<SP>, ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>), (OutboundV1Channel<SP>, ChannelError)>
6320+
where
6321+
L::Target: Logger
6322+
{
6323+
if !self.context.is_outbound() {
6324+
return Err((self, ChannelError::Close("Received funding_signed for an inbound channel?".to_owned())));
6325+
}
6326+
if self.context.channel_state & !(ChannelState::MonitorUpdateInProgress as u32) != ChannelState::FundingCreated as u32 {
6327+
return Err((self, ChannelError::Close("Received funding_signed in strange state!".to_owned())));
6328+
}
6329+
if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
6330+
self.context.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
6331+
self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
6332+
panic!("Should not have advanced channel commitment tx numbers prior to funding_created");
6333+
}
6334+
6335+
let funding_script = self.context.get_funding_redeemscript();
6336+
6337+
let counterparty_keys = self.context.build_remote_transaction_keys();
6338+
let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_counterparty_commitment_transaction_number, &counterparty_keys, false, false, logger).tx;
6339+
let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
6340+
let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
6341+
6342+
log_trace!(logger, "Initial counterparty tx for channel {} is: txid {} tx {}",
6343+
&self.context.channel_id(), counterparty_initial_bitcoin_tx.txid, encode::serialize_hex(&counterparty_initial_bitcoin_tx.transaction));
6344+
6345+
let holder_signer = self.context.build_holder_transaction_keys(self.context.cur_holder_commitment_transaction_number);
6346+
let initial_commitment_tx = self.context.build_commitment_transaction(self.context.cur_holder_commitment_transaction_number, &holder_signer, true, false, logger).tx;
6347+
{
6348+
let trusted_tx = initial_commitment_tx.trust();
6349+
let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
6350+
let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.context.channel_value_satoshis);
6351+
// They sign our commitment transaction, allowing us to broadcast the tx if we wish.
6352+
if let Err(_) = self.context.secp_ctx.verify_ecdsa(&sighash, &msg.signature, &self.context.get_counterparty_pubkeys().funding_pubkey) {
6353+
return Err((self, ChannelError::Close("Invalid funding_signed signature from peer".to_owned())));
6354+
}
6355+
}
6356+
6357+
let holder_commitment_tx = HolderCommitmentTransaction::new(
6358+
initial_commitment_tx,
6359+
msg.signature,
6360+
Vec::new(),
6361+
&self.context.get_holder_pubkeys().funding_pubkey,
6362+
self.context.counterparty_funding_pubkey()
6363+
);
6364+
6365+
let validated =
6366+
self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, Vec::new());
6367+
if validated.is_err() {
6368+
return Err((self, ChannelError::Close("Failed to validate our commitment".to_owned())));
6369+
}
6370+
6371+
let funding_redeemscript = self.context.get_funding_redeemscript();
6372+
let funding_txo = self.context.get_funding_txo().unwrap();
6373+
let funding_txo_script = funding_redeemscript.to_v0_p2wsh();
6374+
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());
6375+
let shutdown_script = self.context.shutdown_scriptpubkey.clone().map(|script| script.into_inner());
6376+
let mut monitor_signer = signer_provider.derive_channel_signer(self.context.channel_value_satoshis, self.context.channel_keys_id);
6377+
monitor_signer.provide_channel_parameters(&self.context.channel_transaction_parameters);
6378+
let channel_monitor = ChannelMonitor::new(self.context.secp_ctx.clone(), monitor_signer,
6379+
shutdown_script, self.context.get_holder_selected_contest_delay(),
6380+
&self.context.destination_script, (funding_txo, funding_txo_script),
6381+
&self.context.channel_transaction_parameters,
6382+
funding_redeemscript.clone(), self.context.channel_value_satoshis,
6383+
obscure_factor,
6384+
holder_commitment_tx, best_block, self.context.counterparty_node_id);
6385+
6386+
channel_monitor.provide_initial_counterparty_commitment_tx(
6387+
counterparty_initial_bitcoin_tx.txid, Vec::new(),
6388+
self.context.cur_counterparty_commitment_transaction_number,
6389+
self.context.counterparty_cur_commitment_point.unwrap(),
6390+
counterparty_initial_commitment_tx.feerate_per_kw(),
6391+
counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
6392+
counterparty_initial_commitment_tx.to_countersignatory_value_sat(), logger);
6393+
6394+
assert_eq!(self.context.channel_state & (ChannelState::MonitorUpdateInProgress as u32), 0); // We have no had any monitor(s) yet to fail update!
6395+
if self.context.is_batch_funding() {
6396+
self.context.channel_state = ChannelState::FundingSent as u32 | ChannelState::WaitingForBatch as u32;
6397+
} else {
6398+
self.context.channel_state = ChannelState::FundingSent as u32;
6399+
}
6400+
self.context.cur_holder_commitment_transaction_number -= 1;
6401+
self.context.cur_counterparty_commitment_transaction_number -= 1;
6402+
6403+
log_info!(logger, "Received funding_signed from peer for channel {}", &self.context.channel_id());
6404+
6405+
let mut channel = Channel { context: self.context };
6406+
6407+
let need_channel_ready = channel.check_get_channel_ready(0).is_some();
6408+
channel.monitor_updating_paused(false, false, need_channel_ready, Vec::new(), Vec::new(), Vec::new());
6409+
Ok((channel, channel_monitor))
6410+
}
6411+
6412+
/// Indicates that the signer may have some signatures for us, so we should retry if we're
6413+
/// blocked.
6414+
#[allow(unused)]
6415+
pub fn signer_maybe_unblocked<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> where L::Target: Logger {
6416+
if self.context.signer_pending_funding && self.context.is_outbound() {
6417+
log_trace!(logger, "Signer unblocked a funding_created");
6418+
self.context.get_funding_created_msg(logger)
6419+
} else { None }
6420+
}
64166421
}
64176422

64186423
/// A not-yet-funded inbound (from counterparty) channel using V1 channel establishment.
@@ -8007,11 +8012,12 @@ use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
80078012
value: 10000000, script_pubkey: output_script.clone(),
80088013
}]};
80098014
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
8010-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
8015+
let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
80118016
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
80128017

80138018
// Node B --> Node A: funding signed
8014-
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
8019+
let res = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger);
8020+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
80158021

80168022
// Put some inbound and outbound HTLCs in A's channel.
80178023
let htlc_amount_msat = 11_092_000; // put an amount below A's effective dust limit but above B's.
@@ -8134,11 +8140,12 @@ use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
81348140
value: 10000000, script_pubkey: output_script.clone(),
81358141
}]};
81368142
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
8137-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
8143+
let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
81388144
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();
81398145

81408146
// Node B --> Node A: funding signed
8141-
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
8147+
let res = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger);
8148+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
81428149

81438150
// Now disconnect the two nodes and check that the commitment point in
81448151
// Node B's channel_reestablish message is sane.
@@ -8322,11 +8329,12 @@ use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
83228329
value: 10000000, script_pubkey: output_script.clone(),
83238330
}]};
83248331
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
8325-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
8332+
let funding_created_msg = node_a_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap();
83268333
let (_, funding_signed_msg, _) = node_b_chan.funding_created(&funding_created_msg.unwrap(), best_block, &&keys_provider, &&logger).map_err(|_| ()).unwrap();
83278334

83288335
// Node B --> Node A: funding signed
8329-
let _ = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger).unwrap();
8336+
let res = node_a_chan.funding_signed(&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger);
8337+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
83308338

83318339
// Make sure that receiving a channel update will update the Channel as expected.
83328340
let update = ChannelUpdate {
@@ -9392,11 +9400,8 @@ use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
93929400
},
93939401
]};
93949402
let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 };
9395-
let (mut node_a_chan, funding_created_msg) = node_a_chan.get_funding_created(
9396-
tx.clone(),
9397-
funding_outpoint,
9398-
true,
9399-
&&logger,
9403+
let funding_created_msg = node_a_chan.get_funding_created(
9404+
tx.clone(), funding_outpoint, true, &&logger,
94009405
).map_err(|_| ()).unwrap();
94019406
let (mut node_b_chan, funding_signed_msg, _) = node_b_chan.funding_created(
94029407
&funding_created_msg.unwrap(),
@@ -9414,12 +9419,10 @@ use crate::ln::channelmanager::{self, HTLCSource, PaymentId};
94149419

94159420
// Receive funding_signed, but the channel will be configured to hold sending channel_ready and
94169421
// broadcasting the funding transaction until the batch is ready.
9417-
let _ = node_a_chan.funding_signed(
9418-
&funding_signed_msg.unwrap(),
9419-
best_block,
9420-
&&keys_provider,
9421-
&&logger,
9422-
).unwrap();
9422+
let res = node_a_chan.funding_signed(
9423+
&funding_signed_msg.unwrap(), best_block, &&keys_provider, &&logger,
9424+
);
9425+
let (mut node_a_chan, _) = if let Ok(res) = res { res } else { panic!(); };
94239426
let node_a_updates = node_a_chan.monitor_updating_restored(
94249427
&&logger,
94259428
&&keys_provider,

0 commit comments

Comments
 (0)