From: Chris Waterson Date: Tue, 18 Jul 2023 14:47:44 +0000 (-0700) Subject: Wait to create a channel until after accepting. X-Git-Tag: v0.0.117-alpha1~59^2 X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=01847277b957ec94129141a7e7439ae539c094f1;p=rust-lightning 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. --- diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index e099b2241..418f8d545 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -343,11 +343,15 @@ pub enum Event { channel_value_satoshis: u64, /// The script which should be used in the transaction output. output_script: Script, - /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`], or a - /// random value for an inbound channel. This may be zero for objects serialized with LDK - /// versions prior to 0.0.113. + /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound + /// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if + /// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise + /// `user_channel_id` will be randomized for an inbound channel. This may be zero for objects + /// serialized with LDK versions prior to 0.0.113. /// /// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels user_channel_id: u128, }, /// Indicates that we've been offered a payment and it needs to be claimed via calling @@ -751,6 +755,13 @@ pub enum Event { }, /// Used to indicate that a previously opened channel with the given `channel_id` is in the /// process of closure. + /// + /// Note that this event is only triggered for accepted channels: if the + /// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true and the channel is + /// rejected, no `ChannelClosed` event will be sent. + /// + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels ChannelClosed { /// The `channel_id` of the channel which has been closed. Note that on-chain transactions /// resolving the channel are likely still awaiting confirmation. @@ -787,8 +798,9 @@ pub enum Event { }, /// Indicates a request to open a new channel by a peer. /// - /// To accept the request, call [`ChannelManager::accept_inbound_channel`]. To reject the - /// request, call [`ChannelManager::force_close_without_broadcasting_txn`]. + /// To accept the request, call [`ChannelManager::accept_inbound_channel`]. To reject the request, + /// call [`ChannelManager::force_close_without_broadcasting_txn`]. Note that a ['ChannelClosed`] + /// event will _not_ be triggered if the channel is rejected. /// /// The event is only triggered when a new open channel request is received and the /// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 1978467ba..fda5d7128 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -740,19 +740,6 @@ pub(super) struct ChannelContext { #[cfg(not(test))] closing_fee_limits: Option<(u64, u64)>, - /// Flag that ensures that `accept_inbound_channel` must be called before `funding_created` - /// is executed successfully. The reason for this flag is that when the - /// `UserConfig::manually_accept_inbound_channels` config flag is set to true, inbound channels - /// are required to be manually accepted by the node operator before the `msgs::AcceptChannel` - /// message is created and sent out. During the manual accept process, `accept_inbound_channel` - /// is called by `ChannelManager::accept_inbound_channel`. - /// - /// The flag counteracts that a counterparty node could theoretically send a - /// `msgs::FundingCreated` message before the node operator has manually accepted an inbound - /// channel request made by the counterparty node. That would execute `funding_created` before - /// `accept_inbound_channel`, and `funding_created` should therefore not execute successfully. - inbound_awaiting_accept: bool, - /// The hash of the block in which the funding transaction was included. funding_tx_confirmed_in: Option, funding_tx_confirmation_height: u32, @@ -5650,8 +5637,6 @@ impl OutboundV1Channel { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - inbound_awaiting_accept: false, - funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, @@ -6032,7 +6017,7 @@ impl InboundV1Channel { fee_estimator: &LowerBoundedFeeEstimator, entropy_source: &ES, signer_provider: &SP, counterparty_node_id: PublicKey, our_supported_features: &ChannelTypeFeatures, their_features: &InitFeatures, msg: &msgs::OpenChannel, user_id: u128, config: &UserConfig, - current_chain_height: u32, logger: &L, outbound_scid_alias: u64 + current_chain_height: u32, logger: &L, outbound_scid_alias: u64, is_0conf: bool, ) -> Result, ChannelError> where ES::Target: EntropySource, SP::Target: SignerProvider, @@ -6222,6 +6207,12 @@ impl InboundV1Channel { let mut secp_ctx = Secp256k1::new(); secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes()); + let minimum_depth = if is_0conf { + Some(0) + } else { + Some(cmp::max(config.channel_handshake_config.minimum_depth, 1)) + }; + let chan = Self { context: ChannelContext { user_id, @@ -6280,8 +6271,6 @@ impl InboundV1Channel { closing_fee_limits: None, target_closing_feerate_sats_per_kw: None, - inbound_awaiting_accept: true, - funding_tx_confirmed_in: None, funding_tx_confirmation_height: 0, short_channel_id: None, @@ -6299,7 +6288,7 @@ impl InboundV1Channel { holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat }, counterparty_max_accepted_htlcs: msg.max_accepted_htlcs, holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS), - minimum_depth: Some(cmp::max(config.channel_handshake_config.minimum_depth, 1)), + minimum_depth, counterparty_forwarding_info: None, @@ -6357,21 +6346,11 @@ impl InboundV1Channel { Ok(chan) } - pub fn is_awaiting_accept(&self) -> bool { - self.context.inbound_awaiting_accept - } - - /// Sets this channel to accepting 0conf, must be done before `get_accept_channel` - pub fn set_0conf(&mut self) { - assert!(self.context.inbound_awaiting_accept); - self.context.minimum_depth = Some(0); - } - /// Marks an inbound channel as accepted and generates a [`msgs::AcceptChannel`] message which /// should be sent back to the counterparty node. /// /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel - pub fn accept_inbound_channel(&mut self, user_id: u128) -> msgs::AcceptChannel { + pub fn accept_inbound_channel(&mut self) -> msgs::AcceptChannel { if self.context.is_outbound() { panic!("Tried to send accept_channel for an outbound channel?"); } @@ -6381,12 +6360,6 @@ impl InboundV1Channel { if self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { panic!("Tried to send an accept_channel for a channel that has already advanced"); } - if !self.context.inbound_awaiting_accept { - panic!("The inbound channel has already been accepted"); - } - - self.context.user_id = user_id; - self.context.inbound_awaiting_accept = false; self.generate_accept_channel_message() } @@ -6482,9 +6455,6 @@ impl InboundV1Channel { // channel. return Err((self, ChannelError::Close("Received funding_created after we got the channel!".to_owned()))); } - if self.context.inbound_awaiting_accept { - return Err((self, ChannelError::Close("FundingCreated message received before the channel was accepted".to_owned()))); - } if self.context.commitment_secrets.get_min_seen_secret() != (1 << 48) || self.context.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER || self.context.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER { @@ -7384,8 +7354,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch closing_fee_limits: None, target_closing_feerate_sats_per_kw, - inbound_awaiting_accept: false, - funding_tx_confirmed_in, funding_tx_confirmation_height, short_channel_id, @@ -7625,10 +7593,10 @@ mod tests { // Make sure A's dust limit is as we expect. let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = InboundV1Channel::::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(); + let mut node_b_chan = InboundV1Channel::::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(); // Node B --> Node A: accept channel, explicitly setting B's dust limit. - let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0); + let mut accept_channel_msg = node_b_chan.accept_inbound_channel(); accept_channel_msg.dust_limit_satoshis = 546; node_a_chan.accept_channel(&accept_channel_msg, &config.channel_handshake_limits, &channelmanager::provided_init_features(&config)).unwrap(); node_a_chan.context.holder_dust_limit_satoshis = 1560; @@ -7754,10 +7722,10 @@ mod tests { // Create Node B's channel by receiving Node A's open_channel message let open_channel_msg = node_a_chan.get_open_channel(chain_hash); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = InboundV1Channel::::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(); + let mut node_b_chan = InboundV1Channel::::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(); // Node B --> Node A: accept channel - let accept_channel_msg = node_b_chan.accept_inbound_channel(0); + let accept_channel_msg = node_b_chan.accept_inbound_channel(); node_a_chan.accept_channel(&accept_channel_msg, &config.channel_handshake_limits, &channelmanager::provided_init_features(&config)).unwrap(); // Node A --> Node B: funding created @@ -7826,12 +7794,12 @@ mod tests { // Test that `InboundV1Channel::new` creates a channel with the correct value for // `holder_max_htlc_value_in_flight_msat`, when configured with a valid percentage value, // which is set to the lower bound - 1 (2%) of the `channel_value`. - let chan_3 = InboundV1Channel::::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(); + let chan_3 = InboundV1Channel::::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(); let chan_3_value_msat = chan_3.context.channel_value_satoshis * 1000; assert_eq!(chan_3.context.holder_max_htlc_value_in_flight_msat, (chan_3_value_msat as f64 * 0.02) as u64); // Test with the upper bound - 1 of valid values (99%). - let chan_4 = InboundV1Channel::::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(); + let chan_4 = InboundV1Channel::::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(); let chan_4_value_msat = chan_4.context.channel_value_satoshis * 1000; assert_eq!(chan_4.context.holder_max_htlc_value_in_flight_msat, (chan_4_value_msat as f64 * 0.99) as u64); @@ -7850,14 +7818,14 @@ mod tests { // Test that `InboundV1Channel::new` uses the lower bound of the configurable percentage values (1%) // if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a value less than 1. - let chan_7 = InboundV1Channel::::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(); + let chan_7 = InboundV1Channel::::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(); let chan_7_value_msat = chan_7.context.channel_value_satoshis * 1000; assert_eq!(chan_7.context.holder_max_htlc_value_in_flight_msat, (chan_7_value_msat as f64 * 0.01) as u64); // Test that `InboundV1Channel::new` uses the upper bound of the configurable percentage values // (100%) if `max_inbound_htlc_value_in_flight_percent_of_channel` is set to a larger value // than 100. - let chan_8 = InboundV1Channel::::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(); + let chan_8 = InboundV1Channel::::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(); let chan_8_value_msat = chan_8.context.channel_value_satoshis * 1000; assert_eq!(chan_8.context.holder_max_htlc_value_in_flight_msat, chan_8_value_msat); } @@ -7907,7 +7875,7 @@ mod tests { inbound_node_config.channel_handshake_config.their_channel_reserve_proportional_millionths = (inbound_selected_channel_reserve_perc * 1_000_000.0) as u32; if outbound_selected_channel_reserve_perc + inbound_selected_channel_reserve_perc < 1.0 { - let chan_inbound_node = InboundV1Channel::::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(); + let chan_inbound_node = InboundV1Channel::::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(); 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); @@ -7915,7 +7883,7 @@ mod tests { assert_eq!(chan_inbound_node.context.counterparty_selected_channel_reserve_satoshis.unwrap(), expected_outbound_selected_chan_reserve); } else { // Channel Negotiations failed - let result = InboundV1Channel::::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); + let result = InboundV1Channel::::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); assert!(result.is_err()); } } @@ -7940,10 +7908,10 @@ mod tests { // Make sure A's dust limit is as we expect. let open_channel_msg = node_a_chan.get_open_channel(genesis_block(network).header.block_hash()); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); - let mut node_b_chan = InboundV1Channel::::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(); + let mut node_b_chan = InboundV1Channel::::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(); // Node B --> Node A: accept channel, explicitly setting B's dust limit. - let mut accept_channel_msg = node_b_chan.accept_inbound_channel(0); + let mut accept_channel_msg = node_b_chan.accept_inbound_channel(); accept_channel_msg.dust_limit_satoshis = 546; node_a_chan.accept_channel(&accept_channel_msg, &config.channel_handshake_limits, &channelmanager::provided_init_features(&config)).unwrap(); node_a_chan.context.holder_dust_limit_satoshis = 1560; @@ -8778,7 +8746,7 @@ mod tests { let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); let res = InboundV1Channel::::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); + &channelmanager::provided_init_features(&config), &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false); assert!(res.is_ok()); } @@ -8820,7 +8788,7 @@ mod tests { let channel_b = InboundV1Channel::::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_a, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), - &open_channel_msg, 7, &config, 0, &&logger, 42 + &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false ).unwrap(); assert_eq!(channel_a.context.channel_type, expected_channel_type); @@ -8862,7 +8830,7 @@ mod tests { let channel_b = InboundV1Channel::::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_a, &channelmanager::provided_channel_type_features(&config), &init_features_with_simple_anchors, - &open_channel_msg, 7, &config, 0, &&logger, 42 + &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false ); assert!(channel_b.is_err()); } @@ -8905,7 +8873,7 @@ mod tests { let res = InboundV1Channel::::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_a, &channelmanager::provided_channel_type_features(&config), &simple_anchors_init, - &open_channel_msg, 7, &config, 0, &&logger, 42 + &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false ); assert!(res.is_err()); @@ -8923,7 +8891,7 @@ mod tests { let channel_b = InboundV1Channel::::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_a, &channelmanager::provided_channel_type_features(&config), &channelmanager::provided_init_features(&config), - &open_channel_msg, 7, &config, 0, &&logger, 42 + &open_channel_msg, 7, &config, 0, &&logger, 42, /*is_0conf=*/false ).unwrap(); let mut accept_channel_msg = channel_b.get_accept_channel_message(); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 961f25cc6..2cc0c1484 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -660,6 +660,13 @@ pub(super) struct PeerState { /// been assigned a `channel_id`, the entry in this map is removed and one is created in /// `channel_by_id`. pub(super) inbound_v1_channel_by_id: HashMap<[u8; 32], InboundV1Channel>, + /// `temporary_channel_id` -> `InboundChannelRequest`. + /// + /// When manual channel acceptance is enabled, this holds all unaccepted inbound channels where + /// the peer is the counterparty. If the channel is accepted, then the entry in this table is + /// removed, and an InboundV1Channel is created and placed in the `inbound_v1_channel_by_id` table. If + /// the channel is rejected, then the entry is simply removed. + pub(super) inbound_channel_request_by_id: HashMap<[u8; 32], InboundChannelRequest>, /// The latest `InitFeatures` we heard from the peer. latest_features: InitFeatures, /// Messages to send to the peer - pushed to in the same lock that they are generated in (except @@ -714,17 +721,32 @@ impl PeerState { fn total_channel_count(&self) -> usize { self.channel_by_id.len() + self.outbound_v1_channel_by_id.len() + - self.inbound_v1_channel_by_id.len() + self.inbound_v1_channel_by_id.len() + + self.inbound_channel_request_by_id.len() } // Returns a bool indicating if the given `channel_id` matches a channel we have with this peer. fn has_channel(&self, channel_id: &[u8; 32]) -> bool { self.channel_by_id.contains_key(channel_id) || self.outbound_v1_channel_by_id.contains_key(channel_id) || - self.inbound_v1_channel_by_id.contains_key(channel_id) + self.inbound_v1_channel_by_id.contains_key(channel_id) || + self.inbound_channel_request_by_id.contains_key(channel_id) } } +/// A not-yet-accepted inbound (from counterparty) channel. Once +/// accepted, the parameters will be used to construct a channel. +pub(super) struct InboundChannelRequest { + /// The original OpenChannel message. + pub open_channel_msg: msgs::OpenChannel, + /// The number of ticks remaining before the request expires. + pub ticks_remaining: i32, +} + +/// The number of ticks that may elapse while we're waiting for an unaccepted inbound channel to be +/// accepted. An unaccepted channel that exceeds this limit will be abandoned. +const UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS: i32 = 2; + /// Stores a PaymentSecret and any other data we may need to validate an inbound payment is /// actually ours and not some duplicate HTLC sent to us by a node along the route. /// @@ -1423,9 +1445,15 @@ pub struct ChannelDetails { /// /// [`outbound_capacity_msat`]: ChannelDetails::outbound_capacity_msat pub unspendable_punishment_reserve: Option, - /// The `user_channel_id` passed in to create_channel, or a random value if the channel was - /// inbound. This may be zero for inbound channels serialized with LDK versions prior to - /// 0.0.113. + /// The `user_channel_id` value passed in to [`ChannelManager::create_channel`] for outbound + /// channels, or to [`ChannelManager::accept_inbound_channel`] for inbound channels if + /// [`UserConfig::manually_accept_inbound_channels`] config flag is set to true. Otherwise + /// `user_channel_id` will be randomized for an inbound channel. This may be zero for objects + /// serialized with LDK versions prior to 0.0.113. + /// + /// [`ChannelManager::create_channel`]: crate::ln::channelmanager::ChannelManager::create_channel + /// [`ChannelManager::accept_inbound_channel`]: crate::ln::channelmanager::ChannelManager::accept_inbound_channel + /// [`UserConfig::manually_accept_inbound_channels`]: crate::util::config::UserConfig::manually_accept_inbound_channels pub user_channel_id: u128, /// The currently negotiated fee rate denominated in satoshi per 1000 weight units, /// which is applied to commitment and HTLC transactions. @@ -2588,6 +2616,12 @@ where self.finish_force_close_channel(chan.context.force_shutdown(false)); // Unfunded channel has no update (None, chan.context.get_counterparty_node_id()) + } else if peer_state.inbound_channel_request_by_id.remove(channel_id).is_some() { + log_error!(self.logger, "Force-closing channel {}", log_bytes!(channel_id[..])); + // N.B. that we don't send any channel close event here: we + // don't have a user_channel_id, and we never sent any opening + // events anyway. + (None, *peer_node_id) } else { return Err(APIError::ChannelUnavailable{ err: format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(*channel_id), peer_node_id) }); } @@ -4487,6 +4521,21 @@ where peer_state.outbound_v1_channel_by_id.retain(|chan_id, chan| process_unfunded_channel_tick(chan_id, &mut chan.context, &mut chan.unfunded_context)); peer_state.inbound_v1_channel_by_id.retain(|chan_id, chan| process_unfunded_channel_tick(chan_id, &mut chan.context, &mut chan.unfunded_context)); + for (chan_id, req) in peer_state.inbound_channel_request_by_id.iter_mut() { + if { req.ticks_remaining -= 1 ; req.ticks_remaining } <= 0 { + log_error!(self.logger, "Force-closing unaccepted inbound channel {} for not accepting in a timely manner", log_bytes!(&chan_id[..])); + peer_state.pending_msg_events.push( + events::MessageSendEvent::HandleError { + node_id: counterparty_node_id, + action: msgs::ErrorAction::SendErrorMessage { + msg: msgs::ErrorMessage { channel_id: chan_id.clone(), data: "Channel force-closed".to_owned() } + }, + } + ); + } + } + peer_state.inbound_channel_request_by_id.retain(|_, req| req.ticks_remaining > 0); + if peer_state.ok_to_remove(true) { pending_peers_awaiting_removal.push(counterparty_node_id); } @@ -5266,49 +5315,61 @@ where let mut peer_state_lock = peer_state_mutex.lock().unwrap(); let peer_state = &mut *peer_state_lock; let is_only_peer_channel = peer_state.total_channel_count() == 1; - match peer_state.inbound_v1_channel_by_id.entry(temporary_channel_id.clone()) { - hash_map::Entry::Occupied(mut channel) => { - if !channel.get().is_awaiting_accept() { - return Err(APIError::APIMisuseError { err: "The channel isn't currently awaiting to be accepted.".to_owned() }); + + // Find (and remove) the channel in the unaccepted table. If it's not there, something weird is + // happening and return an error. N.B. that we create channel with an outbound SCID of zero so + // that we can delay allocating the SCID until after we're sure that the checks below will + // succeed. + let mut channel = match peer_state.inbound_channel_request_by_id.remove(temporary_channel_id) { + Some(unaccepted_channel) => { + let best_block_height = self.best_block.read().unwrap().height(); + InboundV1Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider, + counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features, + &unaccepted_channel.open_channel_msg, user_channel_id, &self.default_configuration, best_block_height, + &self.logger, /*outbound_scid_alias=*/0, accept_0conf).map_err(|e| APIError::ChannelUnavailable { err: e.to_string() }) + } + _ => Err(APIError::APIMisuseError { err: "No such channel awaiting to be accepted.".to_owned() }) + }?; + + if accept_0conf { + // This should have been correctly configured by the call to InboundV1Channel::new. + debug_assert!(channel.context.minimum_depth().unwrap() == 0); + } else if channel.context.get_channel_type().requires_zero_conf() { + let send_msg_err_event = events::MessageSendEvent::HandleError { + node_id: channel.context.get_counterparty_node_id(), + action: msgs::ErrorAction::SendErrorMessage{ + msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "No zero confirmation channels accepted".to_owned(), } } - if accept_0conf { - channel.get_mut().set_0conf(); - } else if channel.get().context.get_channel_type().requires_zero_conf() { - let send_msg_err_event = events::MessageSendEvent::HandleError { - node_id: channel.get().context.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage{ - msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "No zero confirmation channels accepted".to_owned(), } - } - }; - peer_state.pending_msg_events.push(send_msg_err_event); - let _ = remove_channel!(self, channel); - return Err(APIError::APIMisuseError { err: "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned() }); - } else { - // If this peer already has some channels, a new channel won't increase our number of peers - // with unfunded channels, so as long as we aren't over the maximum number of unfunded - // channels per-peer we can accept channels from a peer with existing ones. - if is_only_peer_channel && peers_without_funded_channels >= MAX_UNFUNDED_CHANNEL_PEERS { - let send_msg_err_event = events::MessageSendEvent::HandleError { - node_id: channel.get().context.get_counterparty_node_id(), - action: msgs::ErrorAction::SendErrorMessage{ - msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "Have too many peers with unfunded channels, not accepting new ones".to_owned(), } - } - }; - peer_state.pending_msg_events.push(send_msg_err_event); - let _ = remove_channel!(self, channel); - return Err(APIError::APIMisuseError { err: "Too many peers with unfunded channels, refusing to accept new ones".to_owned() }); + }; + peer_state.pending_msg_events.push(send_msg_err_event); + return Err(APIError::APIMisuseError { err: "Please use accept_inbound_channel_from_trusted_peer_0conf to accept channels with zero confirmations.".to_owned() }); + } else { + // If this peer already has some channels, a new channel won't increase our number of peers + // with unfunded channels, so as long as we aren't over the maximum number of unfunded + // channels per-peer we can accept channels from a peer with existing ones. + if is_only_peer_channel && peers_without_funded_channels >= MAX_UNFUNDED_CHANNEL_PEERS { + let send_msg_err_event = events::MessageSendEvent::HandleError { + node_id: channel.context.get_counterparty_node_id(), + action: msgs::ErrorAction::SendErrorMessage{ + msg: msgs::ErrorMessage { channel_id: temporary_channel_id.clone(), data: "Have too many peers with unfunded channels, not accepting new ones".to_owned(), } } - } - - peer_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel { - node_id: channel.get().context.get_counterparty_node_id(), - msg: channel.get_mut().accept_inbound_channel(user_channel_id), - }); - } - hash_map::Entry::Vacant(_) => { - return Err(APIError::ChannelUnavailable { err: format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(*temporary_channel_id), counterparty_node_id) }); + }; + peer_state.pending_msg_events.push(send_msg_err_event); + return Err(APIError::APIMisuseError { err: "Too many peers with unfunded channels, refusing to accept new ones".to_owned() }); } } + + // Now that we know we have a channel, assign an outbound SCID alias. + let outbound_scid_alias = self.create_and_insert_outbound_scid_alias(); + channel.context.set_outbound_scid_alias(outbound_scid_alias); + + peer_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel { + node_id: channel.context.get_counterparty_node_id(), + msg: channel.accept_inbound_channel(), + }); + + peer_state.inbound_v1_channel_by_id.insert(temporary_channel_id.clone(), channel); + Ok(()) } @@ -5353,7 +5414,7 @@ where num_unfunded_channels += 1; } } - num_unfunded_channels + num_unfunded_channels + peer.inbound_channel_request_by_id.len() } fn internal_open_channel(&self, counterparty_node_id: &PublicKey, msg: &msgs::OpenChannel) -> Result<(), MsgHandleErrInternal> { @@ -5365,11 +5426,6 @@ where return Err(MsgHandleErrInternal::send_err_msg_no_close("No inbound channels accepted".to_owned(), msg.temporary_channel_id.clone())); } - let mut random_bytes = [0u8; 16]; - random_bytes.copy_from_slice(&self.entropy_source.get_secure_random_bytes()[..16]); - let user_channel_id = u128::from_be_bytes(random_bytes); - let outbound_scid_alias = self.create_and_insert_outbound_scid_alias(); - // Get the number of peers with channels, but without funded ones. We don't care too much // about peers that never open a channel, so we filter by peers that have at least one // channel, and then limit the number of those with unfunded channels. @@ -5404,46 +5460,59 @@ where msg.temporary_channel_id.clone())); } + let channel_id = msg.temporary_channel_id; + let channel_exists = peer_state.has_channel(&channel_id); + if channel_exists { + return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision for the same peer!".to_owned(), msg.temporary_channel_id.clone())); + } + + // If we're doing manual acceptance checks on the channel, then defer creation until we're sure we want to accept. + if self.default_configuration.manually_accept_inbound_channels { + let mut pending_events = self.pending_events.lock().unwrap(); + pending_events.push_back((events::Event::OpenChannelRequest { + temporary_channel_id: msg.temporary_channel_id.clone(), + counterparty_node_id: counterparty_node_id.clone(), + funding_satoshis: msg.funding_satoshis, + push_msat: msg.push_msat, + channel_type: msg.channel_type.clone().unwrap(), + }, None)); + peer_state.inbound_channel_request_by_id.insert(channel_id, InboundChannelRequest { + open_channel_msg: msg.clone(), + ticks_remaining: UNACCEPTED_INBOUND_CHANNEL_AGE_LIMIT_TICKS, + }); + return Ok(()); + } + + // Otherwise create the channel right now. + let mut random_bytes = [0u8; 16]; + random_bytes.copy_from_slice(&self.entropy_source.get_secure_random_bytes()[..16]); + let user_channel_id = u128::from_be_bytes(random_bytes); let mut channel = match InboundV1Channel::new(&self.fee_estimator, &self.entropy_source, &self.signer_provider, counterparty_node_id.clone(), &self.channel_type_features(), &peer_state.latest_features, msg, user_channel_id, - &self.default_configuration, best_block_height, &self.logger, outbound_scid_alias) + &self.default_configuration, best_block_height, &self.logger, /*outbound_scid_alias=*/0, /*is_0conf=*/false) { Err(e) => { - self.outbound_scid_aliases.lock().unwrap().remove(&outbound_scid_alias); return Err(MsgHandleErrInternal::from_chan_no_close(e, msg.temporary_channel_id)); }, Ok(res) => res }; - let channel_id = channel.context.channel_id(); - let channel_exists = peer_state.has_channel(&channel_id); - if channel_exists { - self.outbound_scid_aliases.lock().unwrap().remove(&outbound_scid_alias); - return Err(MsgHandleErrInternal::send_err_msg_no_close("temporary_channel_id collision for the same peer!".to_owned(), msg.temporary_channel_id.clone())) - } else { - if !self.default_configuration.manually_accept_inbound_channels { - let channel_type = channel.context.get_channel_type(); - if channel_type.requires_zero_conf() { - return Err(MsgHandleErrInternal::send_err_msg_no_close("No zero confirmation channels accepted".to_owned(), msg.temporary_channel_id.clone())); - } - if channel_type.requires_anchors_zero_fee_htlc_tx() { - return Err(MsgHandleErrInternal::send_err_msg_no_close("No channels with anchor outputs accepted".to_owned(), msg.temporary_channel_id.clone())); - } - peer_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel { - node_id: counterparty_node_id.clone(), - msg: channel.accept_inbound_channel(user_channel_id), - }); - } else { - let mut pending_events = self.pending_events.lock().unwrap(); - pending_events.push_back((events::Event::OpenChannelRequest { - temporary_channel_id: msg.temporary_channel_id.clone(), - counterparty_node_id: counterparty_node_id.clone(), - funding_satoshis: msg.funding_satoshis, - push_msat: msg.push_msat, - channel_type: channel.context.get_channel_type().clone(), - }, None)); - } - peer_state.inbound_v1_channel_by_id.insert(channel_id, channel); + + let channel_type = channel.context.get_channel_type(); + if channel_type.requires_zero_conf() { + return Err(MsgHandleErrInternal::send_err_msg_no_close("No zero confirmation channels accepted".to_owned(), msg.temporary_channel_id.clone())); } + if channel_type.requires_anchors_zero_fee_htlc_tx() { + return Err(MsgHandleErrInternal::send_err_msg_no_close("No channels with anchor outputs accepted".to_owned(), msg.temporary_channel_id.clone())); + } + + let outbound_scid_alias = self.create_and_insert_outbound_scid_alias(); + channel.context.set_outbound_scid_alias(outbound_scid_alias); + + peer_state.pending_msg_events.push(events::MessageSendEvent::SendAcceptChannel { + node_id: counterparty_node_id.clone(), + msg: channel.accept_inbound_channel(), + }); + peer_state.inbound_v1_channel_by_id.insert(channel_id, channel); Ok(()) } @@ -7358,6 +7427,7 @@ where channel_by_id: HashMap::new(), outbound_v1_channel_by_id: HashMap::new(), inbound_v1_channel_by_id: HashMap::new(), + inbound_channel_request_by_id: HashMap::new(), latest_features: init_msg.features.clone(), pending_msg_events: Vec::new(), in_flight_monitor_updates: BTreeMap::new(), @@ -8560,6 +8630,7 @@ where channel_by_id, outbound_v1_channel_by_id: HashMap::new(), inbound_v1_channel_by_id: HashMap::new(), + inbound_channel_request_by_id: HashMap::new(), latest_features: InitFeatures::empty(), pending_msg_events: Vec::new(), in_flight_monitor_updates: BTreeMap::new(), @@ -10280,7 +10351,9 @@ mod tests { let open_channel_msg = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); assert!(!open_channel_msg.channel_type.unwrap().supports_anchors_zero_fee_htlc_tx()); - check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000); + // Since nodes[1] should not have accepted the channel, it should + // not have generated any events. + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] diff --git a/lightning/src/ln/functional_tests.rs b/lightning/src/ln/functional_tests.rs index 53aa75d82..fbd7ddddf 100644 --- a/lightning/src/ln/functional_tests.rs +++ b/lightning/src/ln/functional_tests.rs @@ -7013,7 +7013,7 @@ fn test_user_configurable_csv_delay() { open_channel.to_self_delay = 200; if let Err(error) = InboundV1Channel::new(&LowerBoundedFeeEstimator::new(&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }), &nodes[0].keys_manager, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &nodes[0].node.channel_type_features(), &nodes[1].node.init_features(), &open_channel, 0, - &low_our_to_self_config, 0, &nodes[0].logger, 42) + &low_our_to_self_config, 0, &nodes[0].logger, 42, /*is_0conf=*/false) { match error { ChannelError::Close(err) => { assert!(regex::Regex::new(r"Configured with an unreasonable our_to_self_delay \(\d+\) putting user funds at risks").unwrap().is_match(err.as_str())); }, @@ -7045,7 +7045,7 @@ fn test_user_configurable_csv_delay() { open_channel.to_self_delay = 200; if let Err(error) = InboundV1Channel::new(&LowerBoundedFeeEstimator::new(&test_utils::TestFeeEstimator { sat_per_kw: Mutex::new(253) }), &nodes[0].keys_manager, &nodes[0].keys_manager, nodes[1].node.get_our_node_id(), &nodes[0].node.channel_type_features(), &nodes[1].node.init_features(), &open_channel, 0, - &high_their_to_self_config, 0, &nodes[0].logger, 42) + &high_their_to_self_config, 0, &nodes[0].logger, 42, /*is_0conf=*/false) { match error { ChannelError::Close(err) => { assert!(regex::Regex::new(r"They wanted our payments to be delayed by a needlessly long period\. Upper limit: \d+\. Actual: \d+").unwrap().is_match(err.as_str())); }, @@ -7878,71 +7878,9 @@ fn test_manually_reject_inbound_channel_request() { } _ => panic!("Unexpected event"), } - check_closed_event!(nodes[1], 1, ClosureReason::HolderForceClosed, [nodes[0].node.get_our_node_id()], 100000); -} -#[test] -fn test_reject_funding_before_inbound_channel_accepted() { - // This tests that when `UserConfig::manually_accept_inbound_channels` is set to true, inbound - // channels must to be manually accepted through `ChannelManager::accept_inbound_channel` by - // the node operator before the counterparty sends a `FundingCreated` message. If a - // `FundingCreated` message is received before the channel is accepted, it should be rejected - // and the channel should be closed. - let mut manually_accept_conf = UserConfig::default(); - manually_accept_conf.manually_accept_inbound_channels = true; - 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, Some(manually_accept_conf.clone())]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - - nodes[0].node.create_channel(nodes[1].node.get_our_node_id(), 100000, 10001, 42, Some(manually_accept_conf)).unwrap(); - let res = get_event_msg!(nodes[0], MessageSendEvent::SendOpenChannel, nodes[1].node.get_our_node_id()); - let temp_channel_id = res.temporary_channel_id; - - nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &res); - - // Assert that `nodes[1]` has no `MessageSendEvent::SendAcceptChannel` in the `msg_events`. - assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); - - // Clear the `Event::OpenChannelRequest` event without responding to the request. - nodes[1].node.get_and_clear_pending_events(); - - // Get the `AcceptChannel` message of `nodes[1]` without calling - // `ChannelManager::accept_inbound_channel`, which generates a - // `MessageSendEvent::SendAcceptChannel` event. The message is passed to `nodes[0]` - // `handle_accept_channel`, which is required in order for `create_funding_transaction` to - // succeed when `nodes[0]` is passed to it. - let accept_chan_msg = { - let mut node_1_per_peer_lock; - let mut node_1_peer_state_lock; - let channel = get_inbound_v1_channel_ref!(&nodes[1], nodes[0], node_1_per_peer_lock, node_1_peer_state_lock, temp_channel_id); - channel.get_accept_channel_message() - }; - nodes[0].node.handle_accept_channel(&nodes[1].node.get_our_node_id(), &accept_chan_msg); - - let (temporary_channel_id, tx, _) = create_funding_transaction(&nodes[0], &nodes[1].node.get_our_node_id(), 100000, 42); - - nodes[0].node.funding_transaction_generated(&temporary_channel_id, &nodes[1].node.get_our_node_id(), tx.clone()).unwrap(); - let funding_created_msg = get_event_msg!(nodes[0], MessageSendEvent::SendFundingCreated, nodes[1].node.get_our_node_id()); - - // The `funding_created_msg` should be rejected by `nodes[1]` as it hasn't accepted the channel - nodes[1].node.handle_funding_created(&nodes[0].node.get_our_node_id(), &funding_created_msg); - - let close_msg_ev = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(close_msg_ev.len(), 1); - - let expected_err = "FundingCreated message received before the channel was accepted"; - match close_msg_ev[0] { - MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, ref node_id, } => { - assert_eq!(msg.channel_id, temp_channel_id); - assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - assert_eq!(msg.data, expected_err); - } - _ => panic!("Unexpected event"), - } - - check_closed_event!(nodes[1], 1, ClosureReason::ProcessingError { err: expected_err.to_string() } - , [nodes[0].node.get_our_node_id()], 100000); + // There should be no more events to process, as the channel was never opened. + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); } #[test] @@ -7970,10 +7908,10 @@ fn test_can_not_accept_inbound_channel_twice() { let api_res = nodes[1].node.accept_inbound_channel(&temporary_channel_id, &nodes[0].node.get_our_node_id(), 0); match api_res { Err(APIError::APIMisuseError { err }) => { - assert_eq!(err, "The channel isn't currently awaiting to be accepted."); + assert_eq!(err, "No such channel awaiting to be accepted."); }, Ok(_) => panic!("Channel shouldn't be possible to be accepted twice"), - Err(_) => panic!("Unexpected Error"), + Err(e) => panic!("Unexpected Error {:?}", e), } } _ => panic!("Unexpected event"), @@ -8001,11 +7939,11 @@ fn test_can_not_accept_unknown_inbound_channel() { let unknown_channel_id = [0; 32]; let api_res = nodes[0].node.accept_inbound_channel(&unknown_channel_id, &nodes[1].node.get_our_node_id(), 0); match api_res { - Err(APIError::ChannelUnavailable { err }) => { - assert_eq!(err, format!("Channel with id {} not found for the passed counterparty node_id {}", log_bytes!(unknown_channel_id), nodes[1].node.get_our_node_id())); + Err(APIError::APIMisuseError { err }) => { + assert_eq!(err, "No such channel awaiting to be accepted."); }, Ok(_) => panic!("It shouldn't be possible to accept an unkown channel"), - Err(_) => panic!("Unexpected Error"), + Err(e) => panic!("Unexpected Error: {:?}", e), } } diff --git a/lightning/src/ln/shutdown_tests.rs b/lightning/src/ln/shutdown_tests.rs index f023897b2..bd4df0af0 100644 --- a/lightning/src/ln/shutdown_tests.rs +++ b/lightning/src/ln/shutdown_tests.rs @@ -744,13 +744,13 @@ fn test_invalid_upfront_shutdown_script() { open_channel.shutdown_scriptpubkey = Some(Builder::new().push_int(0) .push_slice(&[0, 0]) .into_script()); - nodes[0].node.handle_open_channel(&nodes[1].node.get_our_node_id(), &open_channel); + nodes[1].node.handle_open_channel(&nodes[0].node.get_our_node_id(), &open_channel); - let events = nodes[0].node.get_and_clear_pending_msg_events(); + let events = nodes[1].node.get_and_clear_pending_msg_events(); assert_eq!(events.len(), 1); match events[0] { MessageSendEvent::HandleError { action: ErrorAction::SendErrorMessage { ref msg }, node_id } => { - assert_eq!(node_id, nodes[1].node.get_our_node_id()); + assert_eq!(node_id, nodes[0].node.get_our_node_id()); assert_eq!(msg.data, "Peer is signaling upfront_shutdown but has provided an unacceptable scriptpubkey format: Script(OP_0 OP_PUSHBYTES_2 0000)"); }, _ => panic!("Unexpected event"),