X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannel.rs;h=e9017d94107f5c0e761134342669d61ebd58c70e;hb=d3ddf15357589ed10f4c844dc22822755b804306;hp=8e389d3a8167daed5a28a6c801a5a0ff4b7d53b8;hpb=efaba118da42431fb64e0ee23111b333ad3b3a87;p=rust-lightning diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8e389d3a..e9017d94 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -267,7 +267,7 @@ enum HTLCUpdateAwaitingACK { } macro_rules! define_state_flags { - ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr)),+], $extra_flags: expr) => { + ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr, $get: ident, $set: ident, $clear: ident)),+], $extra_flags: expr) => { #[doc = $flag_type_doc] #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)] struct $flag_type(u32); @@ -296,15 +296,18 @@ macro_rules! define_state_flags { #[allow(unused)] fn is_empty(&self) -> bool { self.0 == 0 } - #[allow(unused)] fn is_set(&self, flag: Self) -> bool { *self & flag == flag } + #[allow(unused)] + fn set(&mut self, flag: Self) { *self |= flag } + #[allow(unused)] + fn clear(&mut self, flag: Self) -> Self { self.0 &= !flag.0; *self } } - impl core::ops::Not for $flag_type { - type Output = Self; - fn not(self) -> Self::Output { Self(!self.0) } - } + $( + define_state_flags!($flag_type, Self::$flag, $get, $set, $clear); + )* + impl core::ops::BitOr for $flag_type { type Output = Self; fn bitor(self, rhs: Self) -> Self::Output { Self(self.0 | rhs.0) } @@ -323,8 +326,28 @@ macro_rules! define_state_flags { ($flag_type_doc: expr, $flag_type: ident, $flags: tt) => { define_state_flags!($flag_type_doc, $flag_type, $flags, 0); }; + ($flag_type: ident, $flag: expr, $get: ident, $set: ident, $clear: ident) => { + impl $flag_type { + #[allow(unused)] + fn $get(&self) -> bool { self.is_set($flag_type::new() | $flag) } + #[allow(unused)] + fn $set(&mut self) { self.set($flag_type::new() | $flag) } + #[allow(unused)] + fn $clear(&mut self) -> Self { self.clear($flag_type::new() | $flag) } + } + }; ($flag_type_doc: expr, FUNDED_STATE, $flag_type: ident, $flags: tt) => { define_state_flags!($flag_type_doc, $flag_type, $flags, FundedStateFlags::ALL.0); + + define_state_flags!($flag_type, FundedStateFlags::PEER_DISCONNECTED, + is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected); + define_state_flags!($flag_type, FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS, + is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress); + define_state_flags!($flag_type, FundedStateFlags::REMOTE_SHUTDOWN_SENT, + is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent); + define_state_flags!($flag_type, FundedStateFlags::LOCAL_SHUTDOWN_SENT, + is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent); + impl core::ops::BitOr for $flag_type { type Output = Self; fn bitor(self, rhs: FundedStateFlags) -> Self::Output { Self(self.0 | rhs.0) } @@ -371,15 +394,19 @@ define_state_flags!( "Flags that apply to all [`ChannelState`] variants in which the channel is funded.", FundedStateFlags, [ ("Indicates the remote side is considered \"disconnected\" and no updates are allowed \ - until after we've done a `channel_reestablish` dance.", PEER_DISCONNECTED, state_flags::PEER_DISCONNECTED), + until after we've done a `channel_reestablish` dance.", PEER_DISCONNECTED, state_flags::PEER_DISCONNECTED, + is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected), ("Indicates the user has told us a `ChannelMonitor` update is pending async persistence \ somewhere and we should pause sending any outbound messages until they've managed to \ - complete it.", MONITOR_UPDATE_IN_PROGRESS, state_flags::MONITOR_UPDATE_IN_PROGRESS), + complete it.", MONITOR_UPDATE_IN_PROGRESS, state_flags::MONITOR_UPDATE_IN_PROGRESS, + is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress), ("Indicates we received a `shutdown` message from the remote end. If set, they may not add \ any new HTLCs to the channel, and we are expected to respond with our own `shutdown` \ - message when possible.", REMOTE_SHUTDOWN_SENT, state_flags::REMOTE_SHUTDOWN_SENT), + message when possible.", REMOTE_SHUTDOWN_SENT, state_flags::REMOTE_SHUTDOWN_SENT, + is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent), ("Indicates we sent a `shutdown` message. At this point, we may not add any new HTLCs to \ - the channel.", LOCAL_SHUTDOWN_SENT, state_flags::LOCAL_SHUTDOWN_SENT) + the channel.", LOCAL_SHUTDOWN_SENT, state_flags::LOCAL_SHUTDOWN_SENT, + is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent) ] ); @@ -387,9 +414,9 @@ define_state_flags!( "Flags that only apply to [`ChannelState::NegotiatingFunding`].", NegotiatingFundingFlags, [ ("Indicates we have (or are prepared to) send our `open_channel`/`accept_channel` message.", - OUR_INIT_SENT, state_flags::OUR_INIT_SENT), + OUR_INIT_SENT, state_flags::OUR_INIT_SENT, is_our_init_sent, set_our_init_sent, clear_our_init_sent), ("Indicates we have received their `open_channel`/`accept_channel` message.", - THEIR_INIT_SENT, state_flags::THEIR_INIT_SENT) + THEIR_INIT_SENT, state_flags::THEIR_INIT_SENT, is_their_init_sent, set_their_init_sent, clear_their_init_sent) ] ); @@ -398,13 +425,16 @@ define_state_flags!( FUNDED_STATE, AwaitingChannelReadyFlags, [ ("Indicates they sent us a `channel_ready` message. Once both `THEIR_CHANNEL_READY` and \ `OUR_CHANNEL_READY` are set, our state moves on to `ChannelReady`.", - THEIR_CHANNEL_READY, state_flags::THEIR_CHANNEL_READY), + THEIR_CHANNEL_READY, state_flags::THEIR_CHANNEL_READY, + is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready), ("Indicates we sent them a `channel_ready` message. Once both `THEIR_CHANNEL_READY` and \ `OUR_CHANNEL_READY` are set, our state moves on to `ChannelReady`.", - OUR_CHANNEL_READY, state_flags::OUR_CHANNEL_READY), + OUR_CHANNEL_READY, state_flags::OUR_CHANNEL_READY, + is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready), ("Indicates the channel was funded in a batch and the broadcast of the funding transaction \ is being held until all channels in the batch have received `funding_signed` and have \ - their monitors persisted.", WAITING_FOR_BATCH, state_flags::WAITING_FOR_BATCH) + their monitors persisted.", WAITING_FOR_BATCH, state_flags::WAITING_FOR_BATCH, + is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch) ] ); @@ -415,10 +445,13 @@ define_state_flags!( `revoke_and_ack` message. During this period, we can't generate new `commitment_signed` \ messages as we'd be unable to determine which HTLCs they included in their `revoke_and_ack` \ implicit ACK, so instead we have to hold them away temporarily to be sent later.", - AWAITING_REMOTE_REVOKE, state_flags::AWAITING_REMOTE_REVOKE) + AWAITING_REMOTE_REVOKE, state_flags::AWAITING_REMOTE_REVOKE, + is_awaiting_remote_revoke, set_awaiting_remote_revoke, clear_awaiting_remote_revoke) ] ); +// Note that the order of this enum is implicitly defined by where each variant is placed. Take this +// into account when introducing new states and update `test_channel_state_order` accordingly. #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)] enum ChannelState { /// We are negotiating the parameters required for the channel prior to funding it. @@ -439,12 +472,12 @@ enum ChannelState { } macro_rules! impl_state_flag { - ($get: ident, $set: ident, $clear: ident, $state_flag: expr, [$($state: ident),+]) => { + ($get: ident, $set: ident, $clear: ident, [$($state: ident),+]) => { #[allow(unused)] fn $get(&self) -> bool { match self { $( - ChannelState::$state(flags) => flags.is_set($state_flag.into()), + ChannelState::$state(flags) => flags.$get(), )* _ => false, } @@ -453,7 +486,7 @@ macro_rules! impl_state_flag { fn $set(&mut self) { match self { $( - ChannelState::$state(flags) => *flags |= $state_flag, + ChannelState::$state(flags) => flags.$set(), )* _ => debug_assert!(false, "Attempted to set flag on unexpected ChannelState"), } @@ -462,17 +495,17 @@ macro_rules! impl_state_flag { fn $clear(&mut self) { match self { $( - ChannelState::$state(flags) => *flags &= !($state_flag), + ChannelState::$state(flags) => { let _ = flags.$clear(); }, )* _ => debug_assert!(false, "Attempted to clear flag on unexpected ChannelState"), } } }; - ($get: ident, $set: ident, $clear: ident, $state_flag: expr, FUNDED_STATES) => { - impl_state_flag!($get, $set, $clear, $state_flag, [AwaitingChannelReady, ChannelReady]); + ($get: ident, $set: ident, $clear: ident, FUNDED_STATES) => { + impl_state_flag!($get, $set, $clear, [AwaitingChannelReady, ChannelReady]); }; - ($get: ident, $set: ident, $clear: ident, $state_flag: expr, $state: ident) => { - impl_state_flag!($get, $set, $clear, $state_flag, [$state]); + ($get: ident, $set: ident, $clear: ident, $state: ident) => { + impl_state_flag!($get, $set, $clear, [$state]); }; } @@ -523,35 +556,27 @@ impl ChannelState { } } - fn should_force_holding_cell(&self) -> bool { + fn can_generate_new_commitment(&self) -> bool { match self { ChannelState::ChannelReady(flags) => - flags.is_set(ChannelReadyFlags::AWAITING_REMOTE_REVOKE) || - flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into()) || - flags.is_set(FundedStateFlags::PEER_DISCONNECTED.into()), + !flags.is_set(ChannelReadyFlags::AWAITING_REMOTE_REVOKE) && + !flags.is_set(FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS.into()) && + !flags.is_set(FundedStateFlags::PEER_DISCONNECTED.into()), _ => { - debug_assert!(false, "The holding cell is only valid within ChannelReady"); + debug_assert!(false, "Can only generate new commitment within ChannelReady"); false }, } } - impl_state_flag!(is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected, - FundedStateFlags::PEER_DISCONNECTED, FUNDED_STATES); - impl_state_flag!(is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress, - FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS, FUNDED_STATES); - impl_state_flag!(is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent, - FundedStateFlags::LOCAL_SHUTDOWN_SENT, FUNDED_STATES); - impl_state_flag!(is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent, - FundedStateFlags::REMOTE_SHUTDOWN_SENT, FUNDED_STATES); - impl_state_flag!(is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready, - AwaitingChannelReadyFlags::OUR_CHANNEL_READY, AwaitingChannelReady); - impl_state_flag!(is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready, - AwaitingChannelReadyFlags::THEIR_CHANNEL_READY, AwaitingChannelReady); - impl_state_flag!(is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch, - AwaitingChannelReadyFlags::WAITING_FOR_BATCH, AwaitingChannelReady); - impl_state_flag!(is_awaiting_remote_revoke, set_awaiting_remote_revoke, clear_awaiting_remote_revoke, - ChannelReadyFlags::AWAITING_REMOTE_REVOKE, ChannelReady); + impl_state_flag!(is_peer_disconnected, set_peer_disconnected, clear_peer_disconnected, FUNDED_STATES); + impl_state_flag!(is_monitor_update_in_progress, set_monitor_update_in_progress, clear_monitor_update_in_progress, FUNDED_STATES); + impl_state_flag!(is_local_shutdown_sent, set_local_shutdown_sent, clear_local_shutdown_sent, FUNDED_STATES); + impl_state_flag!(is_remote_shutdown_sent, set_remote_shutdown_sent, clear_remote_shutdown_sent, FUNDED_STATES); + impl_state_flag!(is_our_channel_ready, set_our_channel_ready, clear_our_channel_ready, AwaitingChannelReady); + impl_state_flag!(is_their_channel_ready, set_their_channel_ready, clear_their_channel_ready, AwaitingChannelReady); + impl_state_flag!(is_waiting_for_batch, set_waiting_for_batch, clear_waiting_for_batch, AwaitingChannelReady); + impl_state_flag!(is_awaiting_remote_revoke, set_awaiting_remote_revoke, clear_awaiting_remote_revoke, ChannelReady); } pub const INITIAL_COMMITMENT_NUMBER: u64 = (1 << 48) - 1; @@ -732,8 +757,8 @@ struct CommitmentStats<'a> { total_fee_sat: u64, // the total fee included in the transaction num_nondust_htlcs: usize, // the number of HTLC outputs (dust HTLCs *non*-included) htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction - local_balance_msat: u64, // local balance before fees but considering dust limits - remote_balance_msat: u64, // remote balance before fees but considering dust limits + local_balance_msat: u64, // local balance before fees *not* considering dust limits + remote_balance_msat: u64, // remote balance before fees *not* considering dust limits outbound_htlc_preimages: Vec, // preimages for successful offered HTLCs since last commitment inbound_htlc_preimages: Vec, // preimages for successful received HTLCs since last commitment } @@ -816,7 +841,7 @@ pub(super) struct ReestablishResponses { pub(crate) struct ShutdownResult { pub(crate) closure_reason: ClosureReason, /// A channel monitor update to apply. - pub(crate) monitor_update: Option<(PublicKey, OutPoint, ChannelMonitorUpdate)>, + pub(crate) monitor_update: Option<(PublicKey, OutPoint, ChannelId, ChannelMonitorUpdate)>, /// A list of dropped outbound HTLCs that can safely be failed backwards immediately. pub(crate) dropped_outbound_htlcs: Vec<(HTLCSource, PaymentHash, PublicKey, ChannelId)>, /// An unbroadcasted batch funding transaction id. The closure of this channel should be @@ -1733,13 +1758,13 @@ impl ChannelContext where SP::Target: SignerProvider { } } - let mut value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset; + let value_to_self_msat: i64 = (self.value_to_self_msat - local_htlc_total_msat) as i64 + value_to_self_msat_offset; assert!(value_to_self_msat >= 0); // Note that in case they have several just-awaiting-last-RAA fulfills in-progress (ie // AwaitingRemoteRevokeToRemove or AwaitingRemovedRemoteRevoke) we may have allowed them to // "violate" their reserve value by couting those against it. Thus, we have to convert // everything to i64 before subtracting as otherwise we can overflow. - let mut value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset; + let value_to_remote_msat: i64 = (self.channel_value_satoshis * 1000) as i64 - (self.value_to_self_msat as i64) - (remote_htlc_total_msat as i64) - value_to_self_msat_offset; assert!(value_to_remote_msat >= 0); #[cfg(debug_assertions)] @@ -1805,10 +1830,6 @@ impl ChannelContext where SP::Target: SignerProvider { htlcs_included.sort_unstable_by_key(|h| h.0.transaction_output_index.unwrap()); htlcs_included.append(&mut included_dust_htlcs); - // For the stats, trimmed-to-0 the value in msats accordingly - value_to_self_msat = if (value_to_self_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_self_msat }; - value_to_remote_msat = if (value_to_remote_msat * 1000) < broadcaster_dust_limit_satoshis as i64 { 0 } else { value_to_remote_msat }; - CommitmentStats { tx, feerate_per_kw, @@ -1842,8 +1863,6 @@ impl ChannelContext where SP::Target: SignerProvider { /// will sign and send to our counterparty. /// If an Err is returned, it is a ChannelError::Close (for get_funding_created) fn build_remote_transaction_keys(&self) -> TxCreationKeys { - //TODO: Ensure that the payment_key derived here ends up in the library users' wallet as we - //may see payments to it! let revocation_basepoint = &self.get_holder_pubkeys().revocation_basepoint; let htlc_basepoint = &self.get_holder_pubkeys().htlc_basepoint; let counterparty_pubkeys = self.get_counterparty_pubkeys(); @@ -1881,7 +1900,8 @@ impl ChannelContext where SP::Target: SignerProvider { if let Some(feerate) = outbound_feerate_update { feerate_per_kw = cmp::max(feerate_per_kw, feerate); } - cmp::max(2530, feerate_per_kw * 1250 / 1000) + let feerate_plus_quarter = feerate_per_kw.checked_mul(1250).map(|v| v / 1000); + cmp::max(2530, feerate_plus_quarter.unwrap_or(u32::max_value())) } /// Get forwarding information for the counterparty. @@ -2393,16 +2413,13 @@ impl ChannelContext where SP::Target: SignerProvider { // funding transaction, don't return a funding txo (which prevents providing the // monitor update to the user, even if we return one). // See test_duplicate_chan_id and test_pre_lockin_no_chan_closed_update for more. - let generate_monitor_update = match self.channel_state { - ChannelState::AwaitingChannelReady(_)|ChannelState::ChannelReady(_)|ChannelState::ShutdownComplete => true, - _ => false, - }; - if generate_monitor_update { + if !self.channel_state.is_pre_funded_state() { self.latest_monitor_update_id = CLOSED_CHANNEL_UPDATE_ID; - Some((self.get_counterparty_node_id(), funding_txo, ChannelMonitorUpdate { + Some((self.get_counterparty_node_id(), funding_txo, self.channel_id(), ChannelMonitorUpdate { update_id: self.latest_monitor_update_id, counterparty_node_id: Some(self.counterparty_node_id), updates: vec![ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast }], + channel_id: Some(self.channel_id()), })) } else { None } } else { None }; @@ -2562,26 +2579,24 @@ impl FailHTLCContents for msgs::OnionErrorPacket { HTLCUpdateAwaitingACK::FailHTLC { htlc_id, err_packet: self } } } -impl FailHTLCContents for (u16, [u8; 32]) { - type Message = msgs::UpdateFailMalformedHTLC; // (failure_code, sha256_of_onion) +impl FailHTLCContents for ([u8; 32], u16) { + type Message = msgs::UpdateFailMalformedHTLC; fn to_message(self, htlc_id: u64, channel_id: ChannelId) -> Self::Message { msgs::UpdateFailMalformedHTLC { htlc_id, channel_id, - failure_code: self.0, - sha256_of_onion: self.1 + sha256_of_onion: self.0, + failure_code: self.1 } } fn to_inbound_htlc_state(self) -> InboundHTLCState { - InboundHTLCState::LocalRemoved( - InboundHTLCRemovalReason::FailMalformed((self.1, self.0)) - ) + InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::FailMalformed(self)) } fn to_htlc_update_awaiting_ack(self, htlc_id: u64) -> HTLCUpdateAwaitingACK { HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, - failure_code: self.0, - sha256_of_onion: self.1 + sha256_of_onion: self.0, + failure_code: self.1 } } } @@ -2716,7 +2731,7 @@ impl Channel where where L::Target: Logger { // Assert that we'll add the HTLC claim to the holding cell in `get_update_fulfill_htlc` // (see equivalent if condition there). - assert!(self.context.channel_state.should_force_holding_cell()); + assert!(!self.context.channel_state.can_generate_new_commitment()); let mon_update_id = self.context.latest_monitor_update_id; // Forget the ChannelMonitor update let fulfill_resp = self.get_update_fulfill_htlc(htlc_id_arg, payment_preimage_arg, logger); self.context.latest_monitor_update_id = mon_update_id; @@ -2784,9 +2799,10 @@ impl Channel where updates: vec![ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage: payment_preimage_arg.clone(), }], + channel_id: Some(self.context.channel_id()), }; - if self.context.channel_state.should_force_holding_cell() { + if !self.context.channel_state.can_generate_new_commitment() { // Note that this condition is the same as the assertion in // `claim_htlc_while_disconnected_dropping_mon_update` and must match exactly - // `claim_htlc_while_disconnected_dropping_mon_update` would not work correctly if we @@ -2908,7 +2924,7 @@ impl Channel where pub fn queue_fail_malformed_htlc( &mut self, htlc_id_arg: u64, failure_code: u16, sha256_of_onion: [u8; 32], logger: &L ) -> Result<(), ChannelError> where L::Target: Logger { - self.fail_htlc(htlc_id_arg, (failure_code, sha256_of_onion), true, logger) + self.fail_htlc(htlc_id_arg, (sha256_of_onion, failure_code), true, logger) .map(|msg_opt| assert!(msg_opt.is_none(), "We forced holding cell?")) } @@ -2921,7 +2937,7 @@ impl Channel where /// return `Ok(_)` if preconditions are met. In any case, `Err`s will only be /// [`ChannelError::Ignore`]. fn fail_htlc( - &mut self, htlc_id_arg: u64, err_packet: E, mut force_holding_cell: bool, + &mut self, htlc_id_arg: u64, err_contents: E, mut force_holding_cell: bool, logger: &L ) -> Result, ChannelError> where L::Target: Logger { if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { @@ -2960,7 +2976,7 @@ impl Channel where return Ok(None); } - if self.context.channel_state.should_force_holding_cell() { + if !self.context.channel_state.can_generate_new_commitment() { debug_assert!(force_holding_cell, "!force_holding_cell is only called when emptying the holding cell, so we shouldn't end up back in it!"); force_holding_cell = true; } @@ -2988,7 +3004,7 @@ impl Channel where } } log_trace!(logger, "Placing failure for HTLC ID {} in holding cell in channel {}.", htlc_id_arg, &self.context.channel_id()); - self.context.holding_cell_htlc_updates.push(err_packet.to_htlc_update_awaiting_ack(htlc_id_arg)); + self.context.holding_cell_htlc_updates.push(err_contents.to_htlc_update_awaiting_ack(htlc_id_arg)); return Ok(None); } @@ -2996,10 +3012,10 @@ impl Channel where E::Message::name(), &self.context.channel_id()); { let htlc = &mut self.context.pending_inbound_htlcs[pending_idx]; - htlc.state = err_packet.clone().to_inbound_htlc_state(); + htlc.state = err_contents.clone().to_inbound_htlc_state(); } - Ok(Some(err_packet.to_message(htlc_id_arg, self.context.channel_id()))) + Ok(Some(err_contents.to_message(htlc_id_arg, self.context.channel_id()))) } // Message handlers: @@ -3056,12 +3072,12 @@ impl Channel where let mut check_reconnection = false; match &self.context.channel_state { ChannelState::AwaitingChannelReady(flags) => { - let flags = *flags & !FundedStateFlags::ALL; + let flags = flags.clone().clear(FundedStateFlags::ALL.into()); debug_assert!(!flags.is_set(AwaitingChannelReadyFlags::OUR_CHANNEL_READY) || !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)); - if flags & !AwaitingChannelReadyFlags::WAITING_FOR_BATCH == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY { + if flags.clone().clear(AwaitingChannelReadyFlags::WAITING_FOR_BATCH) == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY { // If we reconnected before sending our `channel_ready` they may still resend theirs. check_reconnection = true; - } else if (flags & !AwaitingChannelReadyFlags::WAITING_FOR_BATCH).is_empty() { + } else if flags.clone().clear(AwaitingChannelReadyFlags::WAITING_FOR_BATCH).is_empty() { self.context.channel_state.set_their_channel_ready(); } else if flags == AwaitingChannelReadyFlags::OUR_CHANNEL_READY { self.context.channel_state = ChannelState::ChannelReady(self.context.channel_state.with_funded_state_flags_mask().into()); @@ -3311,7 +3327,7 @@ impl Channel where Err(ChannelError::Close("Remote tried to fulfill/fail an HTLC we couldn't find".to_owned())) } - pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64), ChannelError> { + pub fn update_fulfill_htlc(&mut self, msg: &msgs::UpdateFulfillHTLC) -> Result<(HTLCSource, u64, Option), ChannelError> { if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { return Err(ChannelError::Close("Got fulfill HTLC message when channel was not in an operational state".to_owned())); } @@ -3319,7 +3335,7 @@ impl Channel where return Err(ChannelError::Close("Peer sent update_fulfill_htlc when we needed a channel_reestablish".to_owned())); } - self.mark_outbound_htlc_removed(msg.htlc_id, Some(msg.payment_preimage), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat)) + self.mark_outbound_htlc_removed(msg.htlc_id, Some(msg.payment_preimage), None).map(|htlc| (htlc.source.clone(), htlc.amount_msat, htlc.skimmed_fee_msat)) } pub fn update_fail_htlc(&mut self, msg: &msgs::UpdateFailHTLC, fail_reason: HTLCFailReason) -> Result<(), ChannelError> { @@ -3522,7 +3538,8 @@ impl Channel where htlc_outputs: htlcs_and_sigs, claimed_htlcs, nondust_htlc_sources, - }] + }], + channel_id: Some(self.context.channel_id()), }; self.context.cur_holder_commitment_transaction_number -= 1; @@ -3577,7 +3594,7 @@ impl Channel where ) -> (Option, Vec<(HTLCSource, PaymentHash)>) where F::Target: FeeEstimator, L::Target: Logger { - if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) && !self.context.channel_state.should_force_holding_cell() { + if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) && self.context.channel_state.can_generate_new_commitment() { self.free_holding_cell_htlcs(fee_estimator, logger) } else { (None, Vec::new()) } } @@ -3598,6 +3615,7 @@ impl Channel where update_id: self.context.latest_monitor_update_id + 1, // We don't increment this yet! counterparty_node_id: Some(self.context.counterparty_node_id), updates: Vec::new(), + channel_id: Some(self.context.channel_id()), }; let mut htlc_updates = Vec::new(); @@ -3612,7 +3630,7 @@ impl Channel where // the limit. In case it's less rare than I anticipate, we may want to revisit // handling this case better and maybe fulfilling some of the HTLCs while attempting // to rebalance channels. - match &htlc_update { + let fail_htlc_res = match &htlc_update { &HTLCUpdateAwaitingACK::AddHTLC { amount_msat, cltv_expiry, ref payment_hash, ref source, ref onion_routing_packet, skimmed_fee_msat, blinding_point, .. @@ -3640,6 +3658,7 @@ impl Channel where } } } + None }, &HTLCUpdateAwaitingACK::ClaimHTLC { ref payment_preimage, htlc_id, .. } => { // If an HTLC claim was previously added to the holding cell (via @@ -3653,40 +3672,33 @@ impl Channel where { monitor_update } else { unreachable!() }; update_fulfill_count += 1; monitor_update.updates.append(&mut additional_monitor_update.updates); + None }, &HTLCUpdateAwaitingACK::FailHTLC { htlc_id, ref err_packet } => { - match self.fail_htlc(htlc_id, err_packet.clone(), false, logger) { - Ok(update_fail_msg_option) => { - // If an HTLC failure was previously added to the holding cell (via - // `queue_fail_htlc`) then generating the fail message itself must - // not fail - we should never end up in a state where we double-fail - // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait - // for a full revocation before failing. - debug_assert!(update_fail_msg_option.is_some()); - update_fail_count += 1; - }, - Err(e) => { - if let ChannelError::Ignore(_) = e {} - else { - panic!("Got a non-IgnoreError action trying to fail holding cell HTLC"); - } - } - } + Some(self.fail_htlc(htlc_id, err_packet.clone(), false, logger) + .map(|fail_msg_opt| fail_msg_opt.map(|_| ()))) }, &HTLCUpdateAwaitingACK::FailMalformedHTLC { htlc_id, failure_code, sha256_of_onion } => { - match self.fail_htlc(htlc_id, (failure_code, sha256_of_onion), false, logger) { - Ok(update_fail_malformed_opt) => { - debug_assert!(update_fail_malformed_opt.is_some()); // See above comment - update_fail_count += 1; - }, - Err(e) => { - if let ChannelError::Ignore(_) = e {} - else { - panic!("Got a non-IgnoreError action trying to fail holding cell HTLC"); - } - } - } - }, + Some(self.fail_htlc(htlc_id, (sha256_of_onion, failure_code), false, logger) + .map(|fail_msg_opt| fail_msg_opt.map(|_| ()))) + } + }; + if let Some(res) = fail_htlc_res { + match res { + Ok(fail_msg_opt) => { + // If an HTLC failure was previously added to the holding cell (via + // `queue_fail_{malformed_}htlc`) then generating the fail message itself must + // not fail - we should never end up in a state where we double-fail + // an HTLC or fail-then-claim an HTLC as it indicates we didn't wait + // for a full revocation before failing. + debug_assert!(fail_msg_opt.is_some()); + update_fail_count += 1; + }, + Err(ChannelError::Ignore(_)) => {}, + Err(_) => { + panic!("Got a non-IgnoreError action trying to fail holding cell HTLC"); + }, + } } } if update_add_count == 0 && update_fulfill_count == 0 && update_fail_count == 0 && self.context.holding_cell_update_fee.is_none() { @@ -3782,6 +3794,7 @@ impl Channel where idx: self.context.cur_counterparty_commitment_transaction_number + 1, secret: msg.per_commitment_secret, }], + channel_id: Some(self.context.channel_id()), }; // Update state now that we've passed all the can-fail calls... @@ -4193,8 +4206,8 @@ impl Channel where // first received the funding_signed. let mut funding_broadcastable = if self.context.is_outbound() && - matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)) || - matches!(self.context.channel_state, ChannelState::ChannelReady(_)) + (matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(flags) if !flags.is_set(AwaitingChannelReadyFlags::WAITING_FOR_BATCH)) || + matches!(self.context.channel_state, ChannelState::ChannelReady(_))) { self.context.funding_transaction.take() } else { None }; @@ -4839,6 +4852,7 @@ impl Channel where updates: vec![ChannelMonitorUpdateStep::ShutdownScript { scriptpubkey: self.get_closing_scriptpubkey(), }], + channel_id: Some(self.context.channel_id()), }; self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); self.push_ret_blockable_mon_update(monitor_update) @@ -5204,7 +5218,7 @@ impl Channel where if !self.is_awaiting_monitor_update() { return false; } if matches!( self.context.channel_state, ChannelState::AwaitingChannelReady(flags) - if (flags & !(AwaitingChannelReadyFlags::THEIR_CHANNEL_READY | FundedStateFlags::PEER_DISCONNECTED | FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS | AwaitingChannelReadyFlags::WAITING_FOR_BATCH)).is_empty() + if flags.clone().clear(AwaitingChannelReadyFlags::THEIR_CHANNEL_READY | FundedStateFlags::PEER_DISCONNECTED | FundedStateFlags::MONITOR_UPDATE_IN_PROGRESS | AwaitingChannelReadyFlags::WAITING_FOR_BATCH).is_empty() ) { // If we're not a 0conf channel, we'll be waiting on a monitor update with only // AwaitingChannelReady set, though our peer could have sent their channel_ready. @@ -5290,14 +5304,14 @@ impl Channel where // Note that we don't include ChannelState::WaitingForBatch as we don't want to send // channel_ready until the entire batch is ready. - let need_commitment_update = if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if (f & !FundedStateFlags::ALL).is_empty()) { + let need_commitment_update = if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f.clone().clear(FundedStateFlags::ALL.into()).is_empty()) { self.context.channel_state.set_our_channel_ready(); true - } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f & !FundedStateFlags::ALL == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY) { + } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f.clone().clear(FundedStateFlags::ALL.into()) == AwaitingChannelReadyFlags::THEIR_CHANNEL_READY) { self.context.channel_state = ChannelState::ChannelReady(self.context.channel_state.with_funded_state_flags_mask().into()); self.context.update_time_counter += 1; true - } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f & !FundedStateFlags::ALL == AwaitingChannelReadyFlags::OUR_CHANNEL_READY) { + } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(f) if f.clone().clear(FundedStateFlags::ALL.into()) == AwaitingChannelReadyFlags::OUR_CHANNEL_READY) { // We got a reorg but not enough to trigger a force close, just ignore. false } else { @@ -5872,7 +5886,7 @@ impl Channel where return Err(ChannelError::Ignore("Cannot send an HTLC while disconnected from channel counterparty".to_owned())); } - let need_holding_cell = self.context.channel_state.should_force_holding_cell(); + let need_holding_cell = !self.context.channel_state.can_generate_new_commitment(); log_debug!(logger, "Pushing new outbound HTLC with hash {} for {} msat {}", payment_hash, amount_msat, if force_holding_cell { "into holding cell" } @@ -5978,7 +5992,8 @@ impl Channel where feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()), to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()), to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()), - }] + }], + channel_id: Some(self.context.channel_id()), }; self.context.channel_state.set_awaiting_remote_revoke(); monitor_update @@ -6172,6 +6187,7 @@ impl Channel where updates: vec![ChannelMonitorUpdateStep::ShutdownScript { scriptpubkey: self.get_closing_scriptpubkey(), }], + channel_id: Some(self.context.channel_id()), }; self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); self.push_ret_blockable_mon_update(monitor_update) @@ -6488,7 +6504,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { // Now that we're past error-generating stuff, update our local state: self.context.channel_state = ChannelState::FundingNegotiated; - self.context.channel_id = funding_txo.to_channel_id(); + self.context.channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); // If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100. // We can skip this if it is a zero-conf channel. @@ -6599,29 +6615,31 @@ impl OutboundV1Channel where SP::Target: SignerProvider { let keys = self.context.get_holder_pubkeys(); msgs::OpenChannel { - chain_hash, - temporary_channel_id: self.context.channel_id, - funding_satoshis: self.context.channel_value_satoshis, + common_fields: msgs::CommonOpenChannelFields { + chain_hash, + temporary_channel_id: self.context.channel_id, + funding_satoshis: self.context.channel_value_satoshis, + dust_limit_satoshis: self.context.holder_dust_limit_satoshis, + max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat, + htlc_minimum_msat: self.context.holder_htlc_minimum_msat, + commitment_feerate_sat_per_1000_weight: self.context.feerate_per_kw as u32, + to_self_delay: self.context.get_holder_selected_contest_delay(), + max_accepted_htlcs: self.context.holder_max_accepted_htlcs, + funding_pubkey: keys.funding_pubkey, + revocation_basepoint: keys.revocation_basepoint.to_public_key(), + payment_basepoint: keys.payment_point, + delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(), + htlc_basepoint: keys.htlc_basepoint.to_public_key(), + first_per_commitment_point, + channel_flags: if self.context.config.announced_channel {1} else {0}, + shutdown_scriptpubkey: Some(match &self.context.shutdown_scriptpubkey { + Some(script) => script.clone().into_inner(), + None => Builder::new().into_script(), + }), + channel_type: Some(self.context.channel_type.clone()), + }, push_msat: self.context.channel_value_satoshis * 1000 - self.context.value_to_self_msat, - dust_limit_satoshis: self.context.holder_dust_limit_satoshis, - max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat, channel_reserve_satoshis: self.context.holder_selected_channel_reserve_satoshis, - htlc_minimum_msat: self.context.holder_htlc_minimum_msat, - feerate_per_kw: self.context.feerate_per_kw as u32, - to_self_delay: self.context.get_holder_selected_contest_delay(), - max_accepted_htlcs: self.context.holder_max_accepted_htlcs, - funding_pubkey: keys.funding_pubkey, - revocation_basepoint: keys.revocation_basepoint.to_public_key(), - payment_point: keys.payment_point, - delayed_payment_basepoint: keys.delayed_payment_basepoint.to_public_key(), - htlc_basepoint: keys.htlc_basepoint.to_public_key(), - first_per_commitment_point, - channel_flags: if self.context.config.announced_channel {1} else {0}, - shutdown_scriptpubkey: Some(match &self.context.shutdown_scriptpubkey { - Some(script) => script.clone().into_inner(), - None => Builder::new().into_script(), - }), - channel_type: Some(self.context.channel_type.clone()), } } @@ -6827,7 +6845,7 @@ impl OutboundV1Channel where SP::Target: SignerProvider { &self.context.channel_transaction_parameters, funding_redeemscript.clone(), self.context.channel_value_satoshis, obscure_factor, - holder_commitment_tx, best_block, self.context.counterparty_node_id); + holder_commitment_tx, best_block, self.context.counterparty_node_id, self.context.channel_id()); channel_monitor.provide_initial_counterparty_commitment_tx( counterparty_initial_bitcoin_tx.txid, Vec::new(), self.context.cur_counterparty_commitment_transaction_number, @@ -6871,6 +6889,41 @@ pub(super) struct InboundV1Channel where SP::Target: SignerProvider { pub unfunded_context: UnfundedChannelContext, } +/// Fetches the [`ChannelTypeFeatures`] that will be used for a channel built from a given +/// [`msgs::OpenChannel`]. +pub(super) fn channel_type_from_open_channel( + msg: &msgs::OpenChannel, their_features: &InitFeatures, + our_supported_features: &ChannelTypeFeatures +) -> Result { + if let Some(channel_type) = &msg.common_fields.channel_type { + if channel_type.supports_any_optional_bits() { + return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned())); + } + + // We only support the channel types defined by the `ChannelManager` in + // `provided_channel_type_features`. The channel type must always support + // `static_remote_key`. + if !channel_type.requires_static_remote_key() { + return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned())); + } + // Make sure we support all of the features behind the channel type. + if !channel_type.is_subset(our_supported_features) { + return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned())); + } + let announced_channel = if (msg.common_fields.channel_flags & 1) == 1 { true } else { false }; + if channel_type.requires_scid_privacy() && announced_channel { + return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned())); + } + Ok(channel_type.clone()) + } else { + let channel_type = ChannelTypeFeatures::from_init(&their_features); + if channel_type != ChannelTypeFeatures::only_static_remote_key() { + return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned())); + } + Ok(channel_type) + } +} + impl InboundV1Channel where SP::Target: SignerProvider { /// Creates a new channel from a remote sides' request for one. /// Assumes chain_hash has already been checked and corresponds with what we expect! @@ -6884,47 +6937,22 @@ impl InboundV1Channel where SP::Target: SignerProvider { F::Target: FeeEstimator, L::Target: Logger, { - let logger = WithContext::from(logger, Some(counterparty_node_id), Some(msg.temporary_channel_id)); - let announced_channel = if (msg.channel_flags & 1) == 1 { true } else { false }; + let logger = WithContext::from(logger, Some(counterparty_node_id), Some(msg.common_fields.temporary_channel_id)); + let announced_channel = if (msg.common_fields.channel_flags & 1) == 1 { true } else { false }; // First check the channel type is known, failing before we do anything else if we don't // support this channel type. - let channel_type = if let Some(channel_type) = &msg.channel_type { - if channel_type.supports_any_optional_bits() { - return Err(ChannelError::Close("Channel Type field contained optional bits - this is not allowed".to_owned())); - } - - // We only support the channel types defined by the `ChannelManager` in - // `provided_channel_type_features`. The channel type must always support - // `static_remote_key`. - if !channel_type.requires_static_remote_key() { - return Err(ChannelError::Close("Channel Type was not understood - we require static remote key".to_owned())); - } - // Make sure we support all of the features behind the channel type. - if !channel_type.is_subset(our_supported_features) { - return Err(ChannelError::Close("Channel Type contains unsupported features".to_owned())); - } - if channel_type.requires_scid_privacy() && announced_channel { - return Err(ChannelError::Close("SCID Alias/Privacy Channel Type cannot be set on a public channel".to_owned())); - } - channel_type.clone() - } else { - let channel_type = ChannelTypeFeatures::from_init(&their_features); - if channel_type != ChannelTypeFeatures::only_static_remote_key() { - return Err(ChannelError::Close("Only static_remote_key is supported for non-negotiated channel types".to_owned())); - } - channel_type - }; + let channel_type = channel_type_from_open_channel(msg, their_features, our_supported_features)?; - let channel_keys_id = signer_provider.generate_channel_keys_id(true, msg.funding_satoshis, user_id); - let holder_signer = signer_provider.derive_channel_signer(msg.funding_satoshis, channel_keys_id); + let channel_keys_id = signer_provider.generate_channel_keys_id(true, msg.common_fields.funding_satoshis, user_id); + let holder_signer = signer_provider.derive_channel_signer(msg.common_fields.funding_satoshis, channel_keys_id); let pubkeys = holder_signer.pubkeys().clone(); let counterparty_pubkeys = ChannelPublicKeys { - funding_pubkey: msg.funding_pubkey, - revocation_basepoint: RevocationBasepoint::from(msg.revocation_basepoint), - payment_point: msg.payment_point, - delayed_payment_basepoint: DelayedPaymentBasepoint::from(msg.delayed_payment_basepoint), - htlc_basepoint: HtlcBasepoint::from(msg.htlc_basepoint) + funding_pubkey: msg.common_fields.funding_pubkey, + revocation_basepoint: RevocationBasepoint::from(msg.common_fields.revocation_basepoint), + payment_point: msg.common_fields.payment_basepoint, + delayed_payment_basepoint: DelayedPaymentBasepoint::from(msg.common_fields.delayed_payment_basepoint), + htlc_basepoint: HtlcBasepoint::from(msg.common_fields.htlc_basepoint) }; if config.channel_handshake_config.our_to_self_delay < BREAKDOWN_TIMEOUT { @@ -6932,59 +6960,59 @@ impl InboundV1Channel where SP::Target: SignerProvider { } // Check sanity of message fields: - if msg.funding_satoshis > config.channel_handshake_limits.max_funding_satoshis { - return Err(ChannelError::Close(format!("Per our config, funding must be at most {}. It was {}", config.channel_handshake_limits.max_funding_satoshis, msg.funding_satoshis))); + if msg.common_fields.funding_satoshis > config.channel_handshake_limits.max_funding_satoshis { + return Err(ChannelError::Close(format!("Per our config, funding must be at most {}. It was {}", config.channel_handshake_limits.max_funding_satoshis, msg.common_fields.funding_satoshis))); } - if msg.funding_satoshis >= TOTAL_BITCOIN_SUPPLY_SATOSHIS { - return Err(ChannelError::Close(format!("Funding must be smaller than the total bitcoin supply. It was {}", msg.funding_satoshis))); + if msg.common_fields.funding_satoshis >= TOTAL_BITCOIN_SUPPLY_SATOSHIS { + return Err(ChannelError::Close(format!("Funding must be smaller than the total bitcoin supply. It was {}", msg.common_fields.funding_satoshis))); } - if msg.channel_reserve_satoshis > msg.funding_satoshis { - return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must be not greater than funding_satoshis: {}", msg.channel_reserve_satoshis, msg.funding_satoshis))); + if msg.channel_reserve_satoshis > msg.common_fields.funding_satoshis { + return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must be not greater than funding_satoshis: {}", msg.channel_reserve_satoshis, msg.common_fields.funding_satoshis))); } - let full_channel_value_msat = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000; + let full_channel_value_msat = (msg.common_fields.funding_satoshis - msg.channel_reserve_satoshis) * 1000; if msg.push_msat > full_channel_value_msat { return Err(ChannelError::Close(format!("push_msat {} was larger than channel amount minus reserve ({})", msg.push_msat, full_channel_value_msat))); } - if msg.dust_limit_satoshis > msg.funding_satoshis { - return Err(ChannelError::Close(format!("dust_limit_satoshis {} was larger than funding_satoshis {}. Peer never wants payout outputs?", msg.dust_limit_satoshis, msg.funding_satoshis))); + if msg.common_fields.dust_limit_satoshis > msg.common_fields.funding_satoshis { + return Err(ChannelError::Close(format!("dust_limit_satoshis {} was larger than funding_satoshis {}. Peer never wants payout outputs?", msg.common_fields.dust_limit_satoshis, msg.common_fields.funding_satoshis))); } - if msg.htlc_minimum_msat >= full_channel_value_msat { - return Err(ChannelError::Close(format!("Minimum htlc value ({}) was larger than full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat))); + if msg.common_fields.htlc_minimum_msat >= full_channel_value_msat { + return Err(ChannelError::Close(format!("Minimum htlc value ({}) was larger than full channel value ({})", msg.common_fields.htlc_minimum_msat, full_channel_value_msat))); } - Channel::::check_remote_fee(&channel_type, fee_estimator, msg.feerate_per_kw, None, &&logger)?; + Channel::::check_remote_fee(&channel_type, fee_estimator, msg.common_fields.commitment_feerate_sat_per_1000_weight, None, &&logger)?; let max_counterparty_selected_contest_delay = u16::min(config.channel_handshake_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT); - if msg.to_self_delay > max_counterparty_selected_contest_delay { - return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_counterparty_selected_contest_delay, msg.to_self_delay))); + if msg.common_fields.to_self_delay > max_counterparty_selected_contest_delay { + return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_counterparty_selected_contest_delay, msg.common_fields.to_self_delay))); } - if msg.max_accepted_htlcs < 1 { + if msg.common_fields.max_accepted_htlcs < 1 { return Err(ChannelError::Close("0 max_accepted_htlcs makes for a useless channel".to_owned())); } - if msg.max_accepted_htlcs > MAX_HTLCS { - return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than {}", msg.max_accepted_htlcs, MAX_HTLCS))); + if msg.common_fields.max_accepted_htlcs > MAX_HTLCS { + return Err(ChannelError::Close(format!("max_accepted_htlcs was {}. It must not be larger than {}", msg.common_fields.max_accepted_htlcs, MAX_HTLCS))); } // Now check against optional parameters as set by config... - if msg.funding_satoshis < config.channel_handshake_limits.min_funding_satoshis { - return Err(ChannelError::Close(format!("Funding satoshis ({}) is less than the user specified limit ({})", msg.funding_satoshis, config.channel_handshake_limits.min_funding_satoshis))); + if msg.common_fields.funding_satoshis < config.channel_handshake_limits.min_funding_satoshis { + return Err(ChannelError::Close(format!("Funding satoshis ({}) is less than the user specified limit ({})", msg.common_fields.funding_satoshis, config.channel_handshake_limits.min_funding_satoshis))); } - if msg.htlc_minimum_msat > config.channel_handshake_limits.max_htlc_minimum_msat { - return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat, config.channel_handshake_limits.max_htlc_minimum_msat))); + if msg.common_fields.htlc_minimum_msat > config.channel_handshake_limits.max_htlc_minimum_msat { + return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.common_fields.htlc_minimum_msat, config.channel_handshake_limits.max_htlc_minimum_msat))); } - if msg.max_htlc_value_in_flight_msat < config.channel_handshake_limits.min_max_htlc_value_in_flight_msat { - return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.max_htlc_value_in_flight_msat, config.channel_handshake_limits.min_max_htlc_value_in_flight_msat))); + if msg.common_fields.max_htlc_value_in_flight_msat < config.channel_handshake_limits.min_max_htlc_value_in_flight_msat { + return Err(ChannelError::Close(format!("max_htlc_value_in_flight_msat ({}) is less than the user specified limit ({})", msg.common_fields.max_htlc_value_in_flight_msat, config.channel_handshake_limits.min_max_htlc_value_in_flight_msat))); } if msg.channel_reserve_satoshis > config.channel_handshake_limits.max_channel_reserve_satoshis { return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, config.channel_handshake_limits.max_channel_reserve_satoshis))); } - if msg.max_accepted_htlcs < config.channel_handshake_limits.min_max_accepted_htlcs { - return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, config.channel_handshake_limits.min_max_accepted_htlcs))); + if msg.common_fields.max_accepted_htlcs < config.channel_handshake_limits.min_max_accepted_htlcs { + return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.common_fields.max_accepted_htlcs, config.channel_handshake_limits.min_max_accepted_htlcs))); } - if msg.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); + if msg.common_fields.dust_limit_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is less than the implementation limit ({})", msg.common_fields.dust_limit_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS))); } - if msg.dust_limit_satoshis > MAX_CHAN_DUST_LIMIT_SATOSHIS { - return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS))); + if msg.common_fields.dust_limit_satoshis > MAX_CHAN_DUST_LIMIT_SATOSHIS { + return Err(ChannelError::Close(format!("dust_limit_satoshis ({}) is greater than the implementation limit ({})", msg.common_fields.dust_limit_satoshis, MAX_CHAN_DUST_LIMIT_SATOSHIS))); } // Convert things into internal flags and prep our state: @@ -6995,7 +7023,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { } } - let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis, config); + let holder_selected_channel_reserve_satoshis = get_holder_selected_channel_reserve_satoshis(msg.common_fields.funding_satoshis, config); if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS { // Protocol level safety check in place, although it should never happen because // of `MIN_THEIR_CHAN_RESERVE_SATOSHIS` @@ -7008,8 +7036,8 @@ impl InboundV1Channel where SP::Target: SignerProvider { log_debug!(logger, "channel_reserve_satoshis ({}) is smaller than our dust limit ({}). We can broadcast stale states without any risk, implying this channel is very insecure for our counterparty.", msg.channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS); } - if holder_selected_channel_reserve_satoshis < msg.dust_limit_satoshis { - return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.dust_limit_satoshis, holder_selected_channel_reserve_satoshis))); + if holder_selected_channel_reserve_satoshis < msg.common_fields.dust_limit_satoshis { + return Err(ChannelError::Close(format!("Dust limit ({}) too high for the channel reserve we require the remote to keep ({})", msg.common_fields.dust_limit_satoshis, holder_selected_channel_reserve_satoshis))); } // check if the funder's amount for the initial commitment tx is sufficient @@ -7019,8 +7047,8 @@ impl InboundV1Channel where SP::Target: SignerProvider { } else { 0 }; - let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat; - let commitment_tx_fee = commit_tx_fee_msat(msg.feerate_per_kw, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000; + let funders_amount_msat = msg.common_fields.funding_satoshis * 1000 - msg.push_msat; + let commitment_tx_fee = commit_tx_fee_msat(msg.common_fields.commitment_feerate_sat_per_1000_weight, MIN_AFFORDABLE_HTLC_COUNT, &channel_type) / 1000; if (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value) < commitment_tx_fee { return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", (funders_amount_msat / 1000).saturating_sub(anchor_outputs_value), commitment_tx_fee))); } @@ -7033,7 +7061,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { } let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() { - match &msg.shutdown_scriptpubkey { + match &msg.common_fields.shutdown_scriptpubkey { &Some(ref script) => { // Peer is signaling upfront_shutdown and has opt-out with a 0-length script. We don't enforce anything if script.len() == 0 { @@ -7093,8 +7121,8 @@ impl InboundV1Channel where SP::Target: SignerProvider { inbound_handshake_limits_override: None, - temporary_channel_id: Some(msg.temporary_channel_id), - channel_id: msg.temporary_channel_id, + temporary_channel_id: Some(msg.common_fields.temporary_channel_id), + channel_id: msg.common_fields.temporary_channel_id, channel_state: ChannelState::NegotiatingFunding( NegotiatingFundingFlags::OUR_INIT_SENT | NegotiatingFundingFlags::THEIR_INIT_SENT ), @@ -7133,9 +7161,9 @@ impl InboundV1Channel where SP::Target: SignerProvider { signer_pending_funding: false, #[cfg(debug_assertions)] - holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), + holder_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.common_fields.funding_satoshis * 1000 - msg.push_msat)), #[cfg(debug_assertions)] - counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.funding_satoshis * 1000 - msg.push_msat)), + counterparty_max_commitment_tx_output: Mutex::new((msg.push_msat, msg.common_fields.funding_satoshis * 1000 - msg.push_msat)), last_sent_closing_fee: None, pending_counterparty_closing_signed: None, @@ -7148,17 +7176,17 @@ impl InboundV1Channel where SP::Target: SignerProvider { short_channel_id: None, channel_creation_height: current_chain_height, - feerate_per_kw: msg.feerate_per_kw, - channel_value_satoshis: msg.funding_satoshis, - counterparty_dust_limit_satoshis: msg.dust_limit_satoshis, + feerate_per_kw: msg.common_fields.commitment_feerate_sat_per_1000_weight, + channel_value_satoshis: msg.common_fields.funding_satoshis, + counterparty_dust_limit_satoshis: msg.common_fields.dust_limit_satoshis, holder_dust_limit_satoshis: MIN_CHAN_DUST_LIMIT_SATOSHIS, - counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.max_htlc_value_in_flight_msat, msg.funding_satoshis * 1000), - holder_max_htlc_value_in_flight_msat: get_holder_max_htlc_value_in_flight_msat(msg.funding_satoshis, &config.channel_handshake_config), + counterparty_max_htlc_value_in_flight_msat: cmp::min(msg.common_fields.max_htlc_value_in_flight_msat, msg.common_fields.funding_satoshis * 1000), + holder_max_htlc_value_in_flight_msat: get_holder_max_htlc_value_in_flight_msat(msg.common_fields.funding_satoshis, &config.channel_handshake_config), counterparty_selected_channel_reserve_satoshis: Some(msg.channel_reserve_satoshis), holder_selected_channel_reserve_satoshis, - counterparty_htlc_minimum_msat: msg.htlc_minimum_msat, + counterparty_htlc_minimum_msat: msg.common_fields.htlc_minimum_msat, 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, + counterparty_max_accepted_htlcs: msg.common_fields.max_accepted_htlcs, holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, MAX_HTLCS), minimum_depth, @@ -7169,7 +7197,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { holder_selected_contest_delay: config.channel_handshake_config.our_to_self_delay, is_outbound_from_holder: false, counterparty_parameters: Some(CounterpartyChannelTransactionParameters { - selected_contest_delay: msg.to_self_delay, + selected_contest_delay: msg.common_fields.to_self_delay, pubkeys: counterparty_pubkeys, }), funding_outpoint: None, @@ -7178,7 +7206,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { funding_transaction: None, is_batch_funding: None, - counterparty_cur_commitment_point: Some(msg.first_per_commitment_point), + counterparty_cur_commitment_point: Some(msg.common_fields.first_per_commitment_point), counterparty_prev_commitment_point: None, counterparty_node_id, @@ -7359,7 +7387,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { // Now that we're past error-generating stuff, update our local state: self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); - self.context.channel_id = funding_txo.to_channel_id(); + self.context.channel_id = ChannelId::v1_from_funding_outpoint(funding_txo); self.context.cur_counterparty_commitment_transaction_number -= 1; self.context.cur_holder_commitment_transaction_number -= 1; @@ -7377,7 +7405,7 @@ impl InboundV1Channel where SP::Target: SignerProvider { &self.context.channel_transaction_parameters, funding_redeemscript.clone(), self.context.channel_value_satoshis, obscure_factor, - holder_commitment_tx, best_block, self.context.counterparty_node_id); + holder_commitment_tx, best_block, self.context.counterparty_node_id, self.context.channel_id()); channel_monitor.provide_initial_counterparty_commitment_tx( counterparty_initial_commitment_tx.trust().txid(), Vec::new(), self.context.cur_counterparty_commitment_transaction_number + 1, @@ -7480,6 +7508,8 @@ impl Writeable for Channel where SP::Target: SignerProvider { let mut channel_state = self.context.channel_state; if matches!(channel_state, ChannelState::AwaitingChannelReady(_)|ChannelState::ChannelReady(_)) { channel_state.set_peer_disconnected(); + } else { + debug_assert!(false, "Pre-funded/shutdown channels should not be written"); } channel_state.to_u32().write(writer)?; } @@ -8381,6 +8411,18 @@ mod tests { use bitcoin::address::{WitnessProgram, WitnessVersion}; use crate::prelude::*; + #[test] + fn test_channel_state_order() { + use crate::ln::channel::NegotiatingFundingFlags; + use crate::ln::channel::AwaitingChannelReadyFlags; + use crate::ln::channel::ChannelReadyFlags; + + assert!(ChannelState::NegotiatingFunding(NegotiatingFundingFlags::new()) < ChannelState::FundingNegotiated); + assert!(ChannelState::FundingNegotiated < ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new())); + assert!(ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()) < ChannelState::ChannelReady(ChannelReadyFlags::new())); + assert!(ChannelState::ChannelReady(ChannelReadyFlags::new()) < ChannelState::ShutdownComplete); + } + struct TestFeeEstimator { fee_est: u32 } @@ -8485,7 +8527,7 @@ mod tests { // same as the old fee. fee_est.fee_est = 500; let open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network)); - assert_eq!(open_channel_msg.feerate_per_kw, original_fee); + assert_eq!(open_channel_msg.common_fields.commitment_feerate_sat_per_1000_weight, original_fee); } #[test] @@ -8888,17 +8930,34 @@ mod tests { fn blinding_point_skimmed_fee_malformed_ser() { // Ensure that channel blinding points, skimmed fees, and malformed HTLCs are (de)serialized // properly. + let logger = test_utils::TestLogger::new(); let feeest = LowerBoundedFeeEstimator::new(&TestFeeEstimator{fee_est: 15000}); let secp_ctx = Secp256k1::new(); let seed = [42; 32]; let network = Network::Testnet; + let best_block = BestBlock::from_network(network); let keys_provider = test_utils::TestKeysInterface::new(&seed, network); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); let config = UserConfig::default(); let features = channelmanager::provided_init_features(&config); - let outbound_chan = OutboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &features, 10000000, 100000, 42, &config, 0, 42, None).unwrap(); - let mut chan = Channel { context: outbound_chan.context }; + let mut outbound_chan = OutboundV1Channel::<&TestKeysInterface>::new( + &feeest, &&keys_provider, &&keys_provider, node_b_node_id, &features, 10000000, 100000, 42, &config, 0, 42, None + ).unwrap(); + let inbound_chan = InboundV1Channel::<&TestKeysInterface>::new( + &feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), + &features, &outbound_chan.get_open_channel(ChainHash::using_genesis_block(network)), 7, &config, 0, &&logger, false + ).unwrap(); + outbound_chan.accept_channel(&inbound_chan.get_accept_channel_message(), &config.channel_handshake_limits, &features).unwrap(); + let tx = Transaction { version: 1, lock_time: LockTime::ZERO, input: Vec::new(), output: vec![TxOut { + value: 10000000, script_pubkey: outbound_chan.context.get_funding_redeemscript(), + }]}; + let funding_outpoint = OutPoint{ txid: tx.txid(), index: 0 }; + let funding_created = outbound_chan.get_funding_created(tx.clone(), funding_outpoint, false, &&logger).map_err(|_| ()).unwrap().unwrap(); + let mut chan = match inbound_chan.funding_created(&funding_created, best_block, &&keys_provider, &&logger) { + Ok((chan, _, _)) => chan, + Err((_, e)) => panic!("{}", e), + }; let dummy_htlc_source = HTLCSource::OutboundRoute { path: Path { @@ -9790,7 +9849,7 @@ mod tests { channel_type_features.set_zero_conf_required(); let mut open_channel_msg = node_a_chan.get_open_channel(ChainHash::using_genesis_block(network)); - open_channel_msg.channel_type = Some(channel_type_features); + open_channel_msg.common_fields.channel_type = Some(channel_type_features); let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap()); let res = InboundV1Channel::<&TestKeysInterface>::new(&feeest, &&keys_provider, &&keys_provider, node_b_node_id, &channelmanager::provided_channel_type_features(&config), @@ -9873,7 +9932,7 @@ mod tests { // Set `channel_type` to `None` to force the implicit feature negotiation. let mut open_channel_msg = channel_a.get_open_channel(ChainHash::using_genesis_block(network)); - open_channel_msg.channel_type = None; + open_channel_msg.common_fields.channel_type = None; // Since A supports both `static_remote_key` and `option_anchors`, but B only accepts // `static_remote_key`, it will fail the channel. @@ -9919,7 +9978,7 @@ mod tests { ).unwrap(); let mut open_channel_msg = channel_a.get_open_channel(ChainHash::using_genesis_block(network)); - open_channel_msg.channel_type = Some(simple_anchors_channel_type.clone()); + open_channel_msg.common_fields.channel_type = Some(simple_anchors_channel_type.clone()); let res = InboundV1Channel::<&TestKeysInterface>::new( &fee_estimator, &&keys_provider, &&keys_provider, node_id_a,