Merge pull request #1349 from TheBlueMatt/2022-03-listen-send-sync
[rust-lightning] / lightning / src / ln / channel.rs
index f385bf256bf0bb9248df26e9c697983caabbc47b..dd9fcbea9a8fa2bf05c21c9cf34c04b5e8698660 100644 (file)
@@ -39,14 +39,14 @@ use util::events::ClosureReason;
 use util::ser::{Readable, ReadableArgs, Writeable, Writer, VecWriter};
 use util::logger::Logger;
 use util::errors::APIError;
-use util::config::{UserConfig,ChannelConfig};
+use util::config::{UserConfig, ChannelConfig, ChannelHandshakeLimits};
 use util::scid_utils::scid_from_parts;
 
 use io;
 use prelude::*;
 use core::{cmp,mem,fmt};
 use core::ops::Deref;
-#[cfg(any(test, feature = "fuzztarget", debug_assertions))]
+#[cfg(any(test, fuzzing, debug_assertions))]
 use sync::Mutex;
 use bitcoin::hashes::hex::ToHex;
 
@@ -484,6 +484,8 @@ pub(super) struct Channel<Signer: Sign> {
        #[cfg(not(any(test, feature = "_test_utils")))]
        config: ChannelConfig,
 
+       inbound_handshake_limits_override: Option<ChannelHandshakeLimits>,
+
        user_id: u64,
 
        channel_id: [u8; 32],
@@ -582,6 +584,19 @@ pub(super) struct Channel<Signer: Sign> {
        #[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<BlockHash>,
        funding_tx_confirmation_height: u32,
@@ -655,9 +670,9 @@ pub(super) struct Channel<Signer: Sign> {
        // `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will
        // be, by comparing the cached values to the fee of the tranaction generated by
        // `build_commitment_transaction`.
-       #[cfg(any(test, feature = "fuzztarget"))]
+       #[cfg(any(test, fuzzing))]
        next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
-       #[cfg(any(test, feature = "fuzztarget"))]
+       #[cfg(any(test, fuzzing))]
        next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
 
        /// lnd has a long-standing bug where, upon reconnection, if the channel is not yet confirmed
@@ -669,7 +684,7 @@ pub(super) struct Channel<Signer: Sign> {
        /// See-also <https://github.com/lightningnetwork/lnd/issues/4006>
        pub workaround_lnd_bug_4006: Option<msgs::FundingLocked>,
 
-       #[cfg(any(test, feature = "fuzztarget"))]
+       #[cfg(any(test, fuzzing))]
        // When we receive an HTLC fulfill on an outbound path, we may immediately fulfill the
        // corresponding HTLC on the inbound path. If, then, the outbound path channel is
        // disconnected and reconnected (before we've exchange commitment_signed and revoke_and_ack
@@ -682,7 +697,7 @@ pub(super) struct Channel<Signer: Sign> {
        channel_type: ChannelTypeFeatures,
 }
 
-#[cfg(any(test, feature = "fuzztarget"))]
+#[cfg(any(test, fuzzing))]
 struct CommitmentTxInfoCached {
        fee: u64,
        total_pending_htlcs: usize,
@@ -835,6 +850,7 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(Channel {
                        user_id,
                        config: config.channel_options.clone(),
+                       inbound_handshake_limits_override: Some(config.peer_channel_config_limits.clone()),
 
                        channel_id: keys_provider.get_secure_random_bytes(),
                        channel_state: ChannelState::OurInitSent as u32,
@@ -880,6 +896,8 @@ impl<Signer: Sign> Channel<Signer> {
                        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,
@@ -922,14 +940,14 @@ impl<Signer: Sign> Channel<Signer> {
 
                        announcement_sigs: None,
 
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
 
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        historical_inbound_htlc_fulfills: HashSet::new(),
 
                        // We currently only actually support one channel type, so don't retry with new types
@@ -942,14 +960,6 @@ impl<Signer: Sign> Channel<Signer> {
        fn check_remote_fee<F: Deref>(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError>
                where F::Target: FeeEstimator
        {
-               let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
-               // occasional issues with feerate disagreements between an initiator that wants a feerate
-               // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
-               // sat/kw before the comparison here.
-               if feerate_per_kw + 250 < lower_limit {
-                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
-               }
                // We only bound the fee updates on the upper side to prevent completely absurd feerates,
                // always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
                // We generally don't care too much if they set the feerate to something very high, but it
@@ -959,6 +969,14 @@ impl<Signer: Sign> Channel<Signer> {
                if feerate_per_kw as u64 > upper_limit {
                        return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
                }
+               let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+               // Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
+               // occasional issues with feerate disagreements between an initiator that wants a feerate
+               // of 1.1 sat/vbyte and a receiver that wants 1.1 rounded up to 2. Thus, we always add 250
+               // sat/kw before the comparison here.
+               if feerate_per_kw + 250 < lower_limit {
+                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {} (- 250)", feerate_per_kw, lower_limit)));
+               }
                Ok(())
        }
 
@@ -1134,6 +1152,7 @@ impl<Signer: Sign> Channel<Signer> {
                let chan = Channel {
                        user_id,
                        config: local_config,
+                       inbound_handshake_limits_override: None,
 
                        channel_id: msg.temporary_channel_id,
                        channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
@@ -1178,6 +1197,8 @@ impl<Signer: Sign> Channel<Signer> {
                        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,
@@ -1224,14 +1245,14 @@ impl<Signer: Sign> Channel<Signer> {
 
                        announcement_sigs: None,
 
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
 
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        historical_inbound_htlc_fulfills: HashSet::new(),
 
                        channel_type,
@@ -1629,7 +1650,7 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                if pending_idx == core::usize::MAX {
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        // If we failed to find an HTLC to fulfill, make sure it was previously fulfilled and
                        // this is simply a duplicate claim, not previously failed and we lost funds.
                        debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
@@ -1655,7 +1676,7 @@ impl<Signer: Sign> Channel<Signer> {
                                                if htlc_id_arg == htlc_id {
                                                        // Make sure we don't leave latest_monitor_update_id incremented here:
                                                        self.latest_monitor_update_id -= 1;
-                                                       #[cfg(any(test, feature = "fuzztarget"))]
+                                                       #[cfg(any(test, fuzzing))]
                                                        debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
                                                        return UpdateFulfillFetch::DuplicateClaim {};
                                                }
@@ -1676,11 +1697,11 @@ impl<Signer: Sign> Channel<Signer> {
                        self.holding_cell_htlc_updates.push(HTLCUpdateAwaitingACK::ClaimHTLC {
                                payment_preimage: payment_preimage_arg, htlc_id: htlc_id_arg,
                        });
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
                        return UpdateFulfillFetch::NewClaim { monitor_update, htlc_value_msat, msg: None };
                }
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                self.historical_inbound_htlc_fulfills.insert(htlc_id_arg);
 
                {
@@ -1761,7 +1782,7 @@ impl<Signer: Sign> Channel<Signer> {
                        }
                }
                if pending_idx == core::usize::MAX {
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        // If we failed to find an HTLC to fail, make sure it was previously fulfilled and this
                        // is simply a duplicate fail, not previously failed and we failed-back too early.
                        debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
@@ -1774,7 +1795,7 @@ impl<Signer: Sign> Channel<Signer> {
                                match pending_update {
                                        &HTLCUpdateAwaitingACK::ClaimHTLC { htlc_id, .. } => {
                                                if htlc_id_arg == htlc_id {
-                                                       #[cfg(any(test, feature = "fuzztarget"))]
+                                                       #[cfg(any(test, fuzzing))]
                                                        debug_assert!(self.historical_inbound_htlc_fulfills.contains(&htlc_id_arg));
                                                        return Ok(None);
                                                }
@@ -1811,7 +1832,9 @@ impl<Signer: Sign> Channel<Signer> {
 
        // Message handlers:
 
-       pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, config: &UserConfig, their_features: &InitFeatures) -> Result<(), ChannelError> {
+       pub fn accept_channel(&mut self, msg: &msgs::AcceptChannel, default_limits: &ChannelHandshakeLimits, their_features: &InitFeatures) -> Result<(), ChannelError> {
+               let peer_limits = if let Some(ref limits) = self.inbound_handshake_limits_override { limits } else { default_limits };
+
                // Check sanity of message fields:
                if !self.is_outbound() {
                        return Err(ChannelError::Close("Got an accept_channel message from an inbound peer".to_owned()));
@@ -1832,7 +1855,7 @@ impl<Signer: Sign> Channel<Signer> {
                if msg.htlc_minimum_msat >= full_channel_value_msat {
                        return Err(ChannelError::Close(format!("Minimum htlc value ({}) is full channel value ({})", msg.htlc_minimum_msat, full_channel_value_msat)));
                }
-               let max_delay_acceptable = u16::min(config.peer_channel_config_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT);
+               let max_delay_acceptable = u16::min(peer_limits.their_to_self_delay, MAX_LOCAL_BREAKDOWN_TIMEOUT);
                if msg.to_self_delay > max_delay_acceptable {
                        return Err(ChannelError::Close(format!("They wanted our payments to be delayed by a needlessly long period. Upper limit: {}. Actual: {}", max_delay_acceptable, msg.to_self_delay)));
                }
@@ -1844,17 +1867,17 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                // Now check against optional parameters as set by config...
-               if msg.htlc_minimum_msat > config.peer_channel_config_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.peer_channel_config_limits.max_htlc_minimum_msat)));
+               if msg.htlc_minimum_msat > peer_limits.max_htlc_minimum_msat {
+                       return Err(ChannelError::Close(format!("htlc_minimum_msat ({}) is higher than the user specified limit ({})", msg.htlc_minimum_msat, peer_limits.max_htlc_minimum_msat)));
                }
-               if msg.max_htlc_value_in_flight_msat < config.peer_channel_config_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.peer_channel_config_limits.min_max_htlc_value_in_flight_msat)));
+               if msg.max_htlc_value_in_flight_msat < peer_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, peer_limits.min_max_htlc_value_in_flight_msat)));
                }
-               if msg.channel_reserve_satoshis > config.peer_channel_config_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.peer_channel_config_limits.max_channel_reserve_satoshis)));
+               if msg.channel_reserve_satoshis > peer_limits.max_channel_reserve_satoshis {
+                       return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is higher than the user specified limit ({})", msg.channel_reserve_satoshis, peer_limits.max_channel_reserve_satoshis)));
                }
-               if msg.max_accepted_htlcs < config.peer_channel_config_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.peer_channel_config_limits.min_max_accepted_htlcs)));
+               if msg.max_accepted_htlcs < peer_limits.min_max_accepted_htlcs {
+                       return Err(ChannelError::Close(format!("max_accepted_htlcs ({}) is less than the user specified limit ({})", msg.max_accepted_htlcs, peer_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)));
@@ -1862,8 +1885,8 @@ impl<Signer: Sign> Channel<Signer> {
                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.minimum_depth > config.peer_channel_config_limits.max_minimum_depth {
-                       return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", config.peer_channel_config_limits.max_minimum_depth, msg.minimum_depth)));
+               if msg.minimum_depth > peer_limits.max_minimum_depth {
+                       return Err(ChannelError::Close(format!("We consider the minimum depth to be unreasonably large. Expected minimum: ({}). Actual: ({})", peer_limits.max_minimum_depth, msg.minimum_depth)));
                }
                if msg.minimum_depth == 0 {
                        // Note that if this changes we should update the serialization minimum version to
@@ -1872,6 +1895,16 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Minimum confirmation depth must be at least 1".to_owned()));
                }
 
+               if let Some(ty) = &msg.channel_type {
+                       if *ty != self.channel_type {
+                               return Err(ChannelError::Close("Channel Type in accept_channel didn't match the one sent in open_channel.".to_owned()));
+                       }
+               } else if their_features.supports_channel_type() {
+                       // Assume they've accepted the channel type as they said they understand it.
+               } else {
+                       self.channel_type = ChannelTypeFeatures::from_counterparty_init(&their_features)
+               }
+
                let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
                        match &msg.shutdown_scriptpubkey {
                                &OptionalField::Present(ref script) => {
@@ -1916,6 +1949,7 @@ impl<Signer: Sign> Channel<Signer> {
                self.counterparty_shutdown_scriptpubkey = counterparty_shutdown_scriptpubkey;
 
                self.channel_state = ChannelState::OurInitSent as u32 | ChannelState::TheirInitSent as u32;
+               self.inbound_handshake_limits_override = None; // We're done enforcing limits on our peer's handshake now.
 
                Ok(())
        }
@@ -1966,6 +2000,9 @@ impl<Signer: Sign> Channel<Signer> {
                        // channel.
                        return Err(ChannelError::Close("Received funding_created after we got the channel!".to_owned()));
                }
+               if self.inbound_awaiting_accept {
+                       return Err(ChannelError::Close("FundingCreated message received before the channel was accepted".to_owned()));
+               }
                if self.commitment_secrets.get_min_seen_secret() != (1 << 48) ||
                                self.cur_counterparty_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER ||
                                self.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
@@ -2142,16 +2179,16 @@ impl<Signer: Sign> Channel<Signer> {
 
                log_info!(logger, "Received funding_locked from peer for channel {}", log_bytes!(self.channel_id()));
 
-               Ok(self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height()).ok())
+               Ok(self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height(), logger))
        }
 
        /// Returns transaction if there is pending funding transaction that is yet to broadcast
        pub fn unbroadcasted_funding(&self) -> Option<Transaction> {
-                if self.channel_state & (ChannelState::FundingCreated as u32) != 0 {
-                        self.funding_transaction.clone()
-                } else {
-                        None
-                }
+               if self.channel_state & (ChannelState::FundingCreated as u32) != 0 {
+                       self.funding_transaction.clone()
+               } else {
+                       None
+               }
        }
 
        /// Returns a HTLCStats about inbound pending htlcs
@@ -2245,8 +2282,15 @@ impl<Signer: Sign> Channel<Signer> {
        /// This is the amount that would go to us if we close the channel, ignoring any on-chain fees.
        /// See also [`Channel::get_inbound_outbound_available_balance_msat`]
        pub fn get_balance_msat(&self) -> u64 {
-               self.value_to_self_msat
-                       - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat
+               // Include our local balance, plus any inbound HTLCs we know the preimage for, minus any
+               // HTLCs sent or which will be sent after commitment signed's are exchanged.
+               let mut balance_msat = self.value_to_self_msat;
+               for ref htlc in self.pending_inbound_htlcs.iter() {
+                       if let InboundHTLCState::LocalRemoved(InboundHTLCRemovalReason::Fulfill(_)) = htlc.state {
+                               balance_msat += htlc.amount_msat;
+                       }
+               }
+               balance_msat - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat
        }
 
        pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option<u64>) {
@@ -2333,7 +2377,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                let num_htlcs = included_htlcs + addl_htlcs;
                let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs, self.opt_anchors());
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                {
                        let mut fee = res;
                        if fee_spike_buffer_htlc.is_some() {
@@ -2411,7 +2455,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                let num_htlcs = included_htlcs + addl_htlcs;
                let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs, self.opt_anchors());
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                {
                        let mut fee = res;
                        if fee_spike_buffer_htlc.is_some() {
@@ -2694,7 +2738,7 @@ impl<Signer: Sign> Channel<Signer> {
                                return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee".to_owned())));
                        }
                }
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                {
                        if self.is_outbound() {
                                let projected_commit_tx_info = self.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
@@ -3001,7 +3045,7 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Received an unexpected revoke_and_ack".to_owned()));
                }
 
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                {
                        *self.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
                        *self.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
@@ -3416,7 +3460,7 @@ impl<Signer: Sign> Channel<Signer> {
                        })
                } else { None };
 
-               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block_height).ok();
+               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block_height, logger);
 
                let mut accepted_htlcs = Vec::new();
                mem::swap(&mut accepted_htlcs, &mut self.monitor_pending_forwards);
@@ -3609,7 +3653,7 @@ impl<Signer: Sign> Channel<Signer> {
                        })
                } else { None };
 
-               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height()).ok();
+               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height(), logger);
 
                if self.channel_state & (ChannelState::FundingSent as u32) == ChannelState::FundingSent as u32 {
                        // If we're waiting on a monitor update, we shouldn't re-send any funding_locked's.
@@ -4432,9 +4476,9 @@ impl<Signer: Sign> Channel<Signer> {
                                                                // If we generated the funding transaction and it doesn't match what it
                                                                // should, the client is really broken and we should just panic and
                                                                // tell them off. That said, because hash collisions happen with high
-                                                               // probability in fuzztarget mode, if we're fuzzing we just close the
+                                                               // probability in fuzzing mode, if we're fuzzing we just close the
                                                                // channel and move on.
-                                                               #[cfg(not(feature = "fuzztarget"))]
+                                                               #[cfg(not(fuzzing))]
                                                                panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
                                                        }
                                                        self.update_time_counter += 1;
@@ -4446,7 +4490,7 @@ impl<Signer: Sign> Channel<Signer> {
                                                                        if input.witness.is_empty() {
                                                                                // We generated a malleable funding transaction, implying we've
                                                                                // just exposed ourselves to funds loss to our counterparty.
-                                                                               #[cfg(not(feature = "fuzztarget"))]
+                                                                               #[cfg(not(fuzzing))]
                                                                                panic!("Client called ChannelManager::funding_transaction_generated with bogus transaction!");
                                                                        }
                                                                }
@@ -4464,7 +4508,7 @@ impl<Signer: Sign> Channel<Signer> {
                                        // may have already happened for this block).
                                        if let Some(funding_locked) = self.check_get_funding_locked(height) {
                                                log_info!(logger, "Sending a funding_locked to our peer for channel {}", log_bytes!(self.channel_id));
-                                               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok();
+                                               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger);
                                                return Ok((Some(funding_locked), announcement_sigs));
                                        }
                                }
@@ -4518,7 +4562,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                if let Some(funding_locked) = self.check_get_funding_locked(height) {
                        let announcement_sigs = if let Some((genesis_block_hash, node_pk)) = genesis_node_pk {
-                               self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok()
+                               self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger)
                        } else { None };
                        log_info!(logger, "Sending a funding_locked to our peer for channel {}", log_bytes!(self.channel_id));
                        return Ok((Some(funding_locked), timed_out_htlcs, announcement_sigs));
@@ -4554,7 +4598,7 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                let announcement_sigs = if let Some((genesis_block_hash, node_pk)) = genesis_node_pk {
-                       self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok()
+                       self.get_announcement_sigs(node_pk, genesis_block_hash, height, logger)
                } else { None };
                Ok((None, timed_out_htlcs, announcement_sigs))
        }
@@ -4631,7 +4675,15 @@ impl<Signer: Sign> Channel<Signer> {
                }
        }
 
-       pub fn get_accept_channel(&self) -> msgs::AcceptChannel {
+       pub fn inbound_is_awaiting_accept(&self) -> bool {
+               self.inbound_awaiting_accept
+       }
+
+       /// 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) -> msgs::AcceptChannel {
                if self.is_outbound() {
                        panic!("Tried to send accept_channel for an outbound channel?");
                }
@@ -4641,7 +4693,21 @@ impl<Signer: Sign> Channel<Signer> {
                if self.cur_holder_commitment_transaction_number != INITIAL_COMMITMENT_NUMBER {
                        panic!("Tried to send an accept_channel for a channel that has already advanced");
                }
+               if !self.inbound_awaiting_accept {
+                       panic!("The inbound channel has already been accepted");
+               }
 
+               self.inbound_awaiting_accept = false;
+
+               self.generate_accept_channel_message()
+       }
+
+       /// This function is used to explicitly generate a [`msgs::AcceptChannel`] message for an
+       /// inbound channel. If the intention is to accept an inbound channel, use
+       /// [`Channel::accept_inbound_channel`] instead.
+       ///
+       /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
+       fn generate_accept_channel_message(&self) -> msgs::AcceptChannel {
                let first_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
                let keys = self.get_holder_pubkeys();
 
@@ -4664,9 +4730,19 @@ impl<Signer: Sign> Channel<Signer> {
                                Some(script) => script.clone().into_inner(),
                                None => Builder::new().into_script(),
                        }),
+                       channel_type: Some(self.channel_type.clone()),
                }
        }
 
+       /// Enables the possibility for tests to extract a [`msgs::AcceptChannel`] message for an
+       /// inbound channel without accepting it.
+       ///
+       /// [`msgs::AcceptChannel`]: crate::ln::msgs::AcceptChannel
+       #[cfg(test)]
+       pub fn get_accept_channel_message(&self) -> msgs::AcceptChannel {
+               self.generate_accept_channel_message()
+       }
+
        /// If an Err is returned, it is a ChannelError::Close (for get_outbound_funding_created)
        fn get_outbound_funding_created_signature<L: Deref>(&mut self, logger: &L) -> Result<Signature, ChannelError> where L::Target: Logger {
                let counterparty_keys = self.build_remote_transaction_keys()?;
@@ -4725,15 +4801,12 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Gets an UnsignedChannelAnnouncement for this channel. The channel must be publicly
        /// announceable and available for use (have exchanged FundingLocked messages in both
-       /// directions). Should be used for both loose and in response to an AnnouncementSignatures
-       /// message from the remote peer.
+       /// directions). Should be used for both broadcasted announcements and in response to an
+       /// AnnouncementSignatures message from the remote peer.
        ///
        /// Will only fail if we're not in a state where channel_announcement may be sent (including
        /// closing).
        ///
-       /// Note that the "channel must be funded" requirement is stricter than BOLT 7 requires - see
-       /// https://github.com/lightningnetwork/lightning-rfc/issues/468
-       ///
        /// This will only return ChannelError::Ignore upon failure.
        fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<msgs::UnsignedChannelAnnouncement, ChannelError> {
                if !self.config.announced_channel {
@@ -4759,29 +4832,43 @@ impl<Signer: Sign> Channel<Signer> {
                Ok(msg)
        }
 
-       fn get_announcement_sigs(&mut self, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32) -> Result<msgs::AnnouncementSignatures, ChannelError> {
+       fn get_announcement_sigs<L: Deref>(&mut self, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32, logger: &L)
+       -> Option<msgs::AnnouncementSignatures> where L::Target: Logger {
                if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height {
-                       return Err(ChannelError::Ignore("Funding not yet fully confirmed".to_owned()));
+                       return None;
                }
 
                if !self.is_usable() {
-                       return Err(ChannelError::Ignore("Channel not yet available for use".to_owned()));
+                       return None;
                }
 
                if self.channel_state & ChannelState::PeerDisconnected as u32 != 0 {
-                       return Err(ChannelError::Ignore("Peer currently disconnected".to_owned()));
+                       log_trace!(logger, "Cannot create an announcement_signatures as our peer is disconnected");
+                       return None;
                }
 
                if self.announcement_sigs_state != AnnouncementSigsState::NotSent {
-                       return Err(ChannelError::Ignore("Announcement signatures already sent".to_owned()));
+                       return None;
                }
 
-               let announcement = self.get_channel_announcement(node_pk, genesis_block_hash)?;
-               let (our_node_sig, our_bitcoin_sig) = self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx)
-                       .map_err(|_| ChannelError::Ignore("Signer rejected channel_announcement".to_owned()))?;
+               log_trace!(logger, "Creating an announcement_signatures message for channel {}", log_bytes!(self.channel_id()));
+               let announcement = match self.get_channel_announcement(node_pk, genesis_block_hash) {
+                       Ok(a) => a,
+                       Err(_) => {
+                               log_trace!(logger, "Cannot create an announcement_signatures as channel is not public.");
+                               return None;
+                       }
+               };
+               let (our_node_sig, our_bitcoin_sig) = match self.holder_signer.sign_channel_announcement(&announcement, &self.secp_ctx) {
+                       Err(_) => {
+                               log_error!(logger, "Signer rejected channel_announcement signing. Channel will not be announced!");
+                               return None;
+                       },
+                       Ok(v) => v
+               };
                self.announcement_sigs_state = AnnouncementSigsState::MessageSent;
 
-               Ok(msgs::AnnouncementSignatures {
+               Some(msgs::AnnouncementSignatures {
                        channel_id: self.channel_id(),
                        short_channel_id: self.get_short_channel_id().unwrap(),
                        node_signature: our_node_sig,
@@ -4861,9 +4948,9 @@ impl<Signer: Sign> Channel<Signer> {
                // Prior to static_remotekey, my_current_per_commitment_point was critical to claiming
                // current to_remote balances. However, it no longer has any use, and thus is now simply
                // set to a dummy (but valid, as required by the spec) public key.
-               // fuzztarget mode marks a subset of pubkeys as invalid so that we can hit "invalid pubkey"
+               // fuzzing mode marks a subset of pubkeys as invalid so that we can hit "invalid pubkey"
                // branches, but we unwrap it below, so we arbitrarily select a dummy pubkey which is both
-               // valid, and valid in fuzztarget mode's arbitrary validity criteria:
+               // valid, and valid in fuzzing mode's arbitrary validity criteria:
                let mut pk = [2; 33]; pk[1] = 0xff;
                let dummy_pubkey = PublicKey::from_slice(&pk).unwrap();
                let data_loss_protect = if self.cur_counterparty_commitment_transaction_number + 1 < INITIAL_COMMITMENT_NUMBER {
@@ -5149,7 +5236,7 @@ impl<Signer: Sign> Channel<Signer> {
                let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
                let (signature, htlc_signatures);
 
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                {
                        if !self.is_outbound() {
                                let projected_commit_tx_info = self.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
@@ -5635,9 +5722,9 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
                self.channel_update_status.write(writer)?;
 
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                (self.historical_inbound_htlc_fulfills.len() as u64).write(writer)?;
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                for htlc in self.historical_inbound_htlc_fulfills.iter() {
                        htlc.write(writer)?;
                }
@@ -5899,9 +5986,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 
                let channel_update_status = Readable::read(reader)?;
 
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                let mut historical_inbound_htlc_fulfills = HashSet::new();
-               #[cfg(any(test, feature = "fuzztarget"))]
+               #[cfg(any(test, fuzzing))]
                {
                        let htlc_fulfills_len: u64 = Readable::read(reader)?;
                        for _ in 0..htlc_fulfills_len {
@@ -5988,6 +6075,11 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        user_id,
 
                        config: config.unwrap(),
+
+                       // Note that we don't care about serializing handshake limits as we only ever serialize
+                       // channel data after the handshake has completed.
+                       inbound_handshake_limits_override: None,
+
                        channel_id,
                        channel_state,
                        announcement_sigs_state: announcement_sigs_state.unwrap(),
@@ -6034,6 +6126,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        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,
@@ -6068,14 +6162,14 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
 
                        announcement_sigs,
 
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 
                        workaround_lnd_bug_4006: None,
 
-                       #[cfg(any(test, feature = "fuzztarget"))]
+                       #[cfg(any(test, fuzzing))]
                        historical_inbound_htlc_fulfills,
 
                        channel_type: channel_type.unwrap(),
@@ -6105,7 +6199,7 @@ mod tests {
        use ln::chan_utils::{ChannelPublicKeys, HolderCommitmentTransaction, CounterpartyChannelTransactionParameters, htlc_success_tx_weight, htlc_timeout_tx_weight};
        use chain::BestBlock;
        use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
-       use chain::keysinterface::{InMemorySigner, KeyMaterial, KeysInterface, BaseSign};
+       use chain::keysinterface::{InMemorySigner, Recipient, KeyMaterial, KeysInterface, BaseSign};
        use chain::transaction::OutPoint;
        use util::config::UserConfig;
        use util::enforcing_trait_impls::EnforcingSigner;
@@ -6140,13 +6234,20 @@ mod tests {
                        "MAX_FUNDING_SATOSHIS is greater than all satoshis in existence");
        }
 
+       #[test]
+       fn test_no_fee_check_overflow() {
+               // Previously, calling `check_remote_fee` with a fee of 0xffffffff would overflow in
+               // arithmetic, causing a panic with debug assertions enabled.
+               assert!(Channel::<InMemorySigner>::check_remote_fee(&&TestFeeEstimator { fee_est: 42 }, u32::max_value()).is_err());
+       }
+
        struct Keys {
                signer: InMemorySigner,
        }
        impl KeysInterface for Keys {
                type Signer = InMemorySigner;
 
-               fn get_node_secret(&self) -> SecretKey { panic!(); }
+               fn get_node_secret(&self, _recipient: Recipient) -> Result<SecretKey, ()> { panic!(); }
                fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); }
                fn get_destination_script(&self) -> Script {
                        let secp_ctx = Secp256k1::signing_only();
@@ -6166,7 +6267,7 @@ mod tests {
                }
                fn get_secure_random_bytes(&self) -> [u8; 32] { [0; 32] }
                fn read_chan_signer(&self, _data: &[u8]) -> Result<Self::Signer, DecodeError> { panic!(); }
-               fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5]) -> Result<RecoverableSignature, ()> { panic!(); }
+               fn sign_invoice(&self, _hrp_bytes: &[u8], _invoice_data: &[u5], _recipient: Recipient) -> Result<RecoverableSignature, ()> { panic!(); }
        }
 
        fn public_from_secret_hex(secp_ctx: &Secp256k1<All>, hex: &str) -> PublicKey {
@@ -6244,12 +6345,12 @@ 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 node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
+               let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
 
                // Node B --> Node A: accept channel, explicitly setting B's dust limit.
-               let mut accept_channel_msg = node_b_chan.get_accept_channel();
+               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, &InitFeatures::known()).unwrap();
+               node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap();
                node_a_chan.holder_dust_limit_satoshis = 1560;
 
                // Put some inbound and outbound HTLCs in A's channel.
@@ -6365,8 +6466,8 @@ mod tests {
                let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0, &&logger).unwrap();
 
                // Node B --> Node A: accept channel
-               let accept_channel_msg = node_b_chan.get_accept_channel();
-               node_a_chan.accept_channel(&accept_channel_msg, &config, &InitFeatures::known()).unwrap();
+               let accept_channel_msg = node_b_chan.accept_inbound_channel();
+               node_a_chan.accept_channel(&accept_channel_msg, &config.peer_channel_config_limits, &InitFeatures::known()).unwrap();
 
                // Node A --> Node B: funding created
                let output_script = node_a_chan.get_funding_redeemscript();