Skip to content

Commit 4e4bec7

Browse files
committed
Wait to create a channel until after accepting.
Create a new table in 'peer_state' to maintain unaccepted inbound channels; i.e., a channel for which we've received an 'open_channel' message but that user code has not yet confirmed for acceptance. When user code accepts the channel (e.g. via 'accept_inbound_channel'), create the channel object and as before. Currently, the 'open_channel' message eagerly creates an InboundV1Channel object before determining if the channel should be accepted. Because this happens /before/ the channel has been assigned a user identity (which happens in the handler for OpenChannelRequest), the channel is assigned a random user identity. As part of the creation process, the channel's cryptographic material is initialized, which then uses this randomly generated value for the user's channel identity e.g. in SignerProvider::generate_channel_keys_id. By delaying the creation of the InboundV1Channel until /after/ the channel has been accepted, we ensure that we defer cryptographic initialization until we have given the user the opportunity to assign an identity to the channel.
1 parent 7e3de70 commit 4e4bec7

File tree

5 files changed

+191
-211
lines changed

5 files changed

+191
-211
lines changed

lightning/src/events/mod.rs

+7-3
Original file line numberDiff line numberDiff line change
@@ -343,11 +343,15 @@ pub enum Event {
343343
channel_value_satoshis: u64,
344344
/// The script which should be used in the transaction output.
345345
output_script: Script,
346-
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or a
347-
/// random value for an inbound channel. This may be zero for objects serialized with LDK
348-
/// versions prior to 0.0.113.
346+
/// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound
347+
/// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if
348+
/// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise
349+
/// `user_channel_id` will be randomized for an inbound channel. This may be zero for objects
350+
/// serialized with LDK versions prior to 0.0.113.
349351
///
350352
/// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel
353+
/// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel
354+
/// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels
351355
user_channel_id: u128,
352356
},
353357
/// Indicates that we've been offered a payment and it needs to be claimed via calling

lightning/src/ln/channel.rs

+22-53
Original file line numberDiff line numberDiff line change
@@ -740,19 +740,6 @@ pub(super) struct ChannelContext<Signer: ChannelSigner> {
740740
#[cfg(not(test))]
741741
closing_fee_limits: Option<(u64, u64)>,
742742

743-
/// Flag that ensures that `accept_inbound_channel` must be called before `funding_created`
744-
/// is executed successfully. The reason for this flag is that when the
745-
/// `UserConfig::manually_accept_inbound_channels` config flag is set to true, inbound channels
746-
/// are required to be manually accepted by the node operator before the `msgs::AcceptChannel`
747-
/// message is created and sent out. During the manual accept process, `accept_inbound_channel`
748-
/// is called by `ChannelManager::accept_inbound_channel`.
749-
///
750-
/// The flag counteracts that a counterparty node could theoretically send a
751-
/// `msgs::FundingCreated` message before the node operator has manually accepted an inbound
752-
/// channel request made by the counterparty node. That would execute `funding_created` before
753-
/// `accept_inbound_channel`, and `funding_created` should therefore not execute successfully.
754-
inbound_awaiting_accept: bool,
755-
756743
/// The hash of the block in which the funding transaction was included.
757744
funding_tx_confirmed_in: Option<BlockHash>,
758745
funding_tx_confirmation_height: u32,
@@ -5650,8 +5637,6 @@ impl<Signer: WriteableEcdsaChannelSigner> OutboundV1Channel<Signer> {
56505637
closing_fee_limits: None,
56515638
target_closing_feerate_sats_per_kw: None,
56525639

5653-
inbound_awaiting_accept: false,
5654-
56555640
funding_tx_confirmed_in: None,
56565641
funding_tx_confirmation_height: 0,
56575642
short_channel_id: None,
@@ -6032,7 +6017,7 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
60326017
fee_estimator: &LowerBoundedFeeEstimator<F>, entropy_source: &ES, signer_provider: &SP,
60336018
counterparty_node_id: PublicKey, our_supported_features: &ChannelTypeFeatures,
60346019
their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u128, config: &UserConfig,
6035-
current_chain_height: u32, logger: &L, outbound_scid_alias: u64
6020+
current_chain_height: u32, logger: &L, outbound_scid_alias: u64, is_0conf: bool,
60366021
) -> Result<InboundV1Channel<Signer>, ChannelError>
60376022
where ES::Target: EntropySource,
60386023
SP::Target: SignerProvider<Signer = Signer>,
@@ -6222,6 +6207,12 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
62226207
let mut secp_ctx = Secp256k1::new();
62236208
secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());
62246209

6210+
let minimum_depth = if is_0conf {
6211+
Some(0)
6212+
} else {
6213+
Some(cmp::max(config.channel_handshake_config.minimum_depth, 1))
6214+
};
6215+
62256216
let chan = Self {
62266217
context: ChannelContext {
62276218
user_id,
@@ -6280,8 +6271,6 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
62806271
closing_fee_limits: None,
62816272
target_closing_feerate_sats_per_kw: None,
62826273

6283-
inbound_awaiting_accept: true,
6284-
62856274
funding_tx_confirmed_in: None,
62866275
funding_tx_confirmation_height: 0,
62876276
short_channel_id: None,
@@ -6299,7 +6288,7 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
62996288
holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat },
63006289
counterparty_max_accepted_htlcs: msg.max_accepted_htlcs,
63016290
holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS),
6302-
minimum_depth: Some(cmp::max(config.channel_handshake_config.minimum_depth, 1)),
6291+
minimum_depth,
63036292

63046293
counterparty_forwarding_info: None,
63056294

@@ -6357,16 +6346,6 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
63576346
Ok(chan)
63586347
}
63596348

6360-
pub fn is_awaiting_accept(&self) -> bool {
6361-
self.context.inbound_awaiting_accept
6362-
}
6363-
6364-
/// Sets this channel to accepting 0conf, must be done before `get_accept_channel`
6365-
pub fn set_0conf(&mut self) {
6366-
assert!(self.context.inbound_awaiting_accept);
6367-
self.context.minimum_depth = Some(0);
6368-
}
6369-
63706349
/// Marks an inbound channel as accepted and generates a [`msgs::AcceptChannel`] message which
63716350
/// should be sent back to the counterparty node.
63726351
///
@@ -6381,13 +6360,8 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
63816360
if self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
63826361
panic!("Tried to send an accept_channel for a channel that has already advanced");
63836362
}
6384-
if !self.context.inbound_awaiting_accept {
6385-
panic!("The inbound channel has already been accepted");
6386-
}
63876363

63886364
self.context.user_id = user_id;
6389-
self.context.inbound_awaiting_accept = false;
6390-
63916365
self.generate_accept_channel_message()
63926366
}
63936367

@@ -6482,9 +6456,6 @@ impl<Signer: WriteableEcdsaChannelSigner> InboundV1Channel<Signer> {
64826456
// channel.
64836457
return Err((self, ChannelError::Close("Received funding_created after we got the channel!".to_owned())));
64846458
}
6485-
if self.context.inbound_awaiting_accept {
6486-
return Err((self, ChannelError::Close("FundingCreated message received before the channel was accepted".to_owned())));
6487-
}
64886459
if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
64896460
self.context.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
64906461
self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
@@ -7384,8 +7355,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
73847355
closing_fee_limits: None,
73857356
target_closing_feerate_sats_per_kw,
73867357

7387-
inbound_awaiting_accept: false,
7388-
73897358
funding_tx_confirmed_in,
73907359
funding_tx_confirmation_height,
73917360
short_channel_id,
@@ -7625,7 +7594,7 @@ mod tests {
76257594
// Make sure A's dust limit is as we expect.
76267595
let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
76277596
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
7628-
let mut node_b_chan = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
7597+
let mut node_b_chan = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
76297598

76307599
// Node B --> Node A: accept channel, explicitly setting B's dust limit.
76317600
let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0);
@@ -7754,7 +7723,7 @@ mod tests {
77547723
// Create Node B's channel by receiving Node A's open_channel message
77557724
let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
77567725
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
7757-
let mut node_b_chan = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
7726+
let mut node_b_chan = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
77587727

77597728
// Node B --> Node A: accept channel
77607729
let accept_channel_msg = node_b_chan.accept_inbound_channel(0);
@@ -7826,12 +7795,12 @@ mod tests {
78267795
// Test that `InboundV1Channel::new` creates a channel with the correct value for
78277796
// `holder_max_htlc_value_in_flight_msat`, when configured with a valid percentage value,
78287797
// which is set to the lower bound - 1 (2%) of the `channel_value`.
7829-
let chan_3 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_2_percent), &channelmanager::provided_init_features(&config_2_percent), &chan_1_open_channel_msg, 7, &config_2_percent, 0, &&logger, 42).unwrap();
7798+
let chan_3 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_2_percent), &channelmanager::provided_init_features(&config_2_percent), &chan_1_open_channel_msg, 7, &config_2_percent, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
78307799
let chan_3_value_msat = chan_3.context.channel_value_satoshis * 1000;
78317800
assert_eq!(chan_3.context.holder_max_htlc_value_in_flight_msat, (chan_3_value_msat as f64 * 0.02) as u64);
78327801

78337802
// Test with the upper bound - 1 of valid values (99%).
7834-
let chan_4 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_99_percent), &channelmanager::provided_init_features(&config_99_percent), &chan_1_open_channel_msg, 7, &config_99_percent, 0, &&logger, 42).unwrap();
7803+
let chan_4 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_99_percent), &channelmanager::provided_init_features(&config_99_percent), &chan_1_open_channel_msg, 7, &config_99_percent, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
78357804
let chan_4_value_msat = chan_4.context.channel_value_satoshis * 1000;
78367805
assert_eq!(chan_4.context.holder_max_htlc_value_in_flight_msat, (chan_4_value_msat as f64 * 0.99) as u64);
78377806

@@ -7850,14 +7819,14 @@ mod tests {
78507819

78517820
// Test that `InboundV1Channel::new` uses the lower bound of the configurable percentage values (1%)
78527821
// if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a value less than 1.
7853-
let chan_7 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_0_percent), &channelmanager::provided_init_features(&config_0_percent), &chan_1_open_channel_msg, 7, &config_0_percent, 0, &&logger, 42).unwrap();
7822+
let chan_7 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_0_percent), &channelmanager::provided_init_features(&config_0_percent), &chan_1_open_channel_msg, 7, &config_0_percent, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
78547823
let chan_7_value_msat = chan_7.context.channel_value_satoshis * 1000;
78557824
assert_eq!(chan_7.context.holder_max_htlc_value_in_flight_msat, (chan_7_value_msat as f64 * 0.01) as u64);
78567825

78577826
// Test that `InboundV1Channel::new` uses the upper bound of the configurable percentage values
78587827
// (100%) if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a larger value
78597828
// than 100.
7860-
let chan_8 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_101_percent), &channelmanager::provided_init_features(&config_101_percent), &chan_1_open_channel_msg, 7, &config_101_percent, 0, &&logger, 42).unwrap();
7829+
let chan_8 = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&config_101_percent), &channelmanager::provided_init_features(&config_101_percent), &chan_1_open_channel_msg, 7, &config_101_percent, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
78617830
let chan_8_value_msat = chan_8.context.channel_value_satoshis * 1000;
78627831
assert_eq!(chan_8.context.holder_max_htlc_value_in_flight_msat, chan_8_value_msat);
78637832
}
@@ -7907,15 +7876,15 @@ mod tests {
79077876
inbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (inbound_selected_channel_reserve_perc * 1_000_000.0) as u32;
79087877

79097878
if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 {
7910-
let chan_inbound_node = InboundV1Channel::<EnforcingSigner>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42).unwrap();
7879+
let chan_inbound_node = InboundV1Channel::<EnforcingSigner>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
79117880

79127881
let expected_inbound_selected_chan_reserve = cmp::max(MIN_THEIR_CHAN_RESERVE_SATOSHIS, (chan.context.channel_value_satoshis as f64 * inbound_selected_channel_reserve_perc) as u64);
79137882

79147883
assert_eq!(chan_inbound_node.context.holder_selected_channel_reserve_satoshis, expected_inbound_selected_chan_reserve);
79157884
assert_eq!(chan_inbound_node.context.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve);
79167885
} else {
79177886
// Channel Negotiations failed
7918-
let result = InboundV1Channel::<EnforcingSigner>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42);
7887+
let result = InboundV1Channel::<EnforcingSigner>::new(&&fee_est, &&keys_provider, &&keys_provider, inbound_node_id, &channelmanager::provided_channel_type_features(&inbound_node_config), &channelmanager::provided_init_features(&outbound_node_config), &chan_open_channel_msg, 7, &inbound_node_config, 0, &&logger, 42, /*is_0conf=*/false);
79197888
assert!(result.is_err());
79207889
}
79217890
}
@@ -7940,7 +7909,7 @@ mod tests {
79407909
// Make sure A's dust limit is as we expect.
79417910
let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash());
79427911
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
7943-
let mut node_b_chan = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42).unwrap();
7912+
let mut node_b_chan = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false).unwrap();
79447913

79457914
// Node B --> Node A: accept channel, explicitly setting B's dust limit.
79467915
let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0);
@@ -8778,7 +8747,7 @@ mod tests {
87788747
let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
87798748
let res = InboundV1Channel::<EnforcingSigner>::new(&feeest, &&keys_provider, &&keys_provider,
87808749
node_b_node_id, &channelmanager::provided_channel_type_features(&config),
8781-
&channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42);
8750+
&channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false);
87828751
assert!(res.is_ok());
87838752
}
87848753

@@ -8820,7 +8789,7 @@ mod tests {
88208789
let channel_b = InboundV1Channel::<EnforcingSigner>::new(
88218790
&fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
88228791
&channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config),
8823-
&open_channel_msg, 7, &config, 0, &&logger, 42
8792+
&open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false
88248793
).unwrap();
88258794

88268795
assert_eq!(channel_a.context.channel_type, expected_channel_type);
@@ -8862,7 +8831,7 @@ mod tests {
88628831
let channel_b = InboundV1Channel::<EnforcingSigner>::new(
88638832
&fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
88648833
&channelmanager::provided_channel_type_features(&config), &init_features_with_simple_anchors,
8865-
&open_channel_msg, 7, &config, 0, &&logger, 42
8834+
&open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false
88668835
);
88678836
assert!(channel_b.is_err());
88688837
}
@@ -8905,7 +8874,7 @@ mod tests {
89058874
let res = InboundV1Channel::<EnforcingSigner>::new(
89068875
&fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
89078876
&channelmanager::provided_channel_type_features(&config), &simple_anchors_init,
8908-
&open_channel_msg, 7, &config, 0, &&logger, 42
8877+
&open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false
89098878
);
89108879
assert!(res.is_err());
89118880

@@ -8923,7 +8892,7 @@ mod tests {
89238892
let channel_b = InboundV1Channel::<EnforcingSigner>::new(
89248893
&fee_estimator, &&keys_provider, &&keys_provider, node_id_a,
89258894
&channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config),
8926-
&open_channel_msg, 7, &config, 0, &&logger, 42
8895+
&open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false
89278896
).unwrap();
89288897

89298898
let mut accept_channel_msg = channel_b.get_accept_channel_message();

0 commit comments

Comments
 (0)