Disconect `announcement_signatures` sending from `funding_locked`
[rust-lightning] / lightning / src / ln / channel.rs
index d76840236a1b04945de7703eae62906a6ddd4469..98eb984fa4c3d6b07b9ea0e34f6c627d3f9fb8ea 100644 (file)
@@ -281,6 +281,26 @@ pub(super) enum ChannelUpdateStatus {
        Disabled,
 }
 
+/// We track when we sent an `AnnouncementSignatures` to our peer in a few states, described here.
+#[derive(PartialEq)]
+pub enum AnnouncementSigsState {
+       /// We have not sent our peer an `AnnouncementSignatures` yet, or our peer disconnected since
+       /// we sent the last `AnnouncementSignatures`.
+       NotSent,
+       /// We sent an `AnnouncementSignatures` to our peer since the last time our peer disconnected.
+       /// This state never appears on disk - instead we write `NotSent`.
+       MessageSent,
+       /// We sent a `CommitmentSigned` after the last `AnnouncementSignatures` we sent. Because we
+       /// only ever have a single `CommitmentSigned` pending at once, if we sent one after sending
+       /// `AnnouncementSignatures` then we know the peer received our `AnnouncementSignatures` if
+       /// they send back a `RevokeAndACK`.
+       /// This state never appears on disk - instead we write `NotSent`.
+       Committed,
+       /// We received a `RevokeAndACK`, effectively ack-ing our `AnnouncementSignatures`, at this
+       /// point we no longer need to re-send our `AnnouncementSignatures` again on reconnect.
+       PeerReceived,
+}
+
 /// An enum indicating whether the local or remote side offered a given HTLC.
 enum HTLCInitiator {
        LocalOffered,
@@ -374,6 +394,19 @@ pub(super) struct MonitorRestoreUpdates {
        pub finalized_claimed_htlcs: Vec<HTLCSource>,
        pub funding_broadcastable: Option<Transaction>,
        pub funding_locked: Option<msgs::FundingLocked>,
+       pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
+}
+
+/// The return value of `channel_reestablish`
+pub(super) struct ReestablishResponses {
+       pub funding_locked: Option<msgs::FundingLocked>,
+       pub raa: Option<msgs::RevokeAndACK>,
+       pub commitment_update: Option<msgs::CommitmentUpdate>,
+       pub order: RAACommitmentOrder,
+       pub mon_update: Option<ChannelMonitorUpdate>,
+       pub holding_cell_failed_htlcs: Vec<(HTLCSource, PaymentHash)>,
+       pub announcement_sigs: Option<msgs::AnnouncementSignatures>,
+       pub shutdown: Option<msgs::Shutdown>,
 }
 
 /// If the majority of the channels funds are to the fundee and the initiator holds only just
@@ -387,9 +420,9 @@ pub(super) struct MonitorRestoreUpdates {
 /// the channel. Sadly, there isn't really a good number for this - if we expect to have no new
 /// HTLCs for days we may need this to suffice for feerate increases across days, but that may
 /// leave the channel less usable as we hold a bigger reserve.
-#[cfg(fuzzing)]
+#[cfg(any(fuzzing, test))]
 pub const FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE: u64 = 2;
-#[cfg(not(fuzzing))]
+#[cfg(not(any(fuzzing, test)))]
 const FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE: u64 = 2;
 
 /// If we fail to see a funding transaction confirmed on-chain within this many blocks after the
@@ -430,6 +463,19 @@ pub(super) struct Channel<Signer: Sign> {
 
        channel_id: [u8; 32],
        channel_state: u32,
+
+       // When we reach max(6 blocks, minimum_depth), we need to send an AnnouncementSigs message to
+       // our peer. However, we want to make sure they received it, or else rebroadcast it when we
+       // next connect.
+       // We do so here, see `AnnouncementSigsSent` for more details on the state(s).
+       // Note that a number of our tests were written prior to the behavior here which retransmits
+       // AnnouncementSignatures until after an RAA completes, so the behavior is short-circuited in
+       // many tests.
+       #[cfg(any(test, feature = "_test_utils"))]
+       pub(crate) announcement_sigs_state: AnnouncementSigsState,
+       #[cfg(not(any(test, feature = "_test_utils")))]
+       announcement_sigs_state: AnnouncementSigsState,
+
        secp_ctx: Secp256k1<secp256k1::All>,
        channel_value_satoshis: u64,
 
@@ -481,9 +527,14 @@ pub(super) struct Channel<Signer: Sign> {
        holding_cell_update_fee: Option<u32>,
        next_holder_htlc_id: u64,
        next_counterparty_htlc_id: u64,
-       update_time_counter: u32,
        feerate_per_kw: u32,
 
+       /// The timestamp set on our latest `channel_update` message for this channel. It is updated
+       /// when the channel is updated in ways which may impact the `channel_update` message or when a
+       /// new block is received, ensuring it's always at least moderately close to the current real
+       /// time.
+       update_time_counter: u32,
+
        #[cfg(debug_assertions)]
        /// Max to_local and to_remote outputs in a locally-generated commitment transaction
        holder_max_commitment_tx_output: Mutex<(u64, u64)>,
@@ -516,18 +567,30 @@ pub(super) struct Channel<Signer: Sign> {
        channel_creation_height: u32,
 
        counterparty_dust_limit_satoshis: u64,
+
        #[cfg(test)]
        pub(super) holder_dust_limit_satoshis: u64,
        #[cfg(not(test))]
        holder_dust_limit_satoshis: u64,
+
        #[cfg(test)]
        pub(super) counterparty_max_htlc_value_in_flight_msat: u64,
        #[cfg(not(test))]
        counterparty_max_htlc_value_in_flight_msat: u64,
-       //get_holder_max_htlc_value_in_flight_msat(): u64,
+
+       #[cfg(test)]
+       pub(super) holder_max_htlc_value_in_flight_msat: u64,
+       #[cfg(not(test))]
+       holder_max_htlc_value_in_flight_msat: u64,
+
        /// minimum channel reserve for self to maintain - set by them.
        counterparty_selected_channel_reserve_satoshis: Option<u64>,
-       // get_holder_selected_channel_reserve_satoshis(channel_value_sats: u64): u64
+
+       #[cfg(test)]
+       pub(super) holder_selected_channel_reserve_satoshis: u64,
+       #[cfg(not(test))]
+       holder_selected_channel_reserve_satoshis: u64,
+
        counterparty_htlc_minimum_msat: u64,
        holder_htlc_minimum_msat: u64,
        #[cfg(test)]
@@ -679,6 +742,10 @@ impl<Signer: Sign> Channel<Signer> {
        /// required by us.
        ///
        /// Guaranteed to return a value no larger than channel_value_satoshis
+       ///
+       /// This is used both for new channels and to figure out what reserve value we sent to peers
+       /// for channels serialized before we included our selected reserve value in the serialized
+       /// data explicitly.
        pub(crate) fn get_holder_selected_channel_reserve_satoshis(channel_value_satoshis: u64) -> u64 {
                let (q, _) = channel_value_satoshis.overflowing_div(100);
                cmp::min(channel_value_satoshis, cmp::max(q, 1000)) //TODO
@@ -742,6 +809,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                        channel_id: keys_provider.get_secure_random_bytes(),
                        channel_state: ChannelState::OurInitSent as u32,
+                       announcement_sigs_state: AnnouncementSigsState::NotSent,
                        secp_ctx,
                        channel_value_satoshis,
 
@@ -792,7 +860,9 @@ impl<Signer: Sign> Channel<Signer> {
                        counterparty_dust_limit_satoshis: 0,
                        holder_dust_limit_satoshis: MIN_CHAN_DUST_LIMIT_SATOSHIS,
                        counterparty_max_htlc_value_in_flight_msat: 0,
+                       holder_max_htlc_value_in_flight_msat: Self::get_holder_max_htlc_value_in_flight_msat(channel_value_satoshis),
                        counterparty_selected_channel_reserve_satoshis: None, // Filled in in accept_channel
+                       holder_selected_channel_reserve_satoshis,
                        counterparty_htlc_minimum_msat: 0,
                        holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        counterparty_max_accepted_htlcs: 0,
@@ -844,8 +914,12 @@ impl<Signer: Sign> Channel<Signer> {
                where F::Target: FeeEstimator
        {
                let lower_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               if feerate_per_kw < lower_limit {
-                       return Err(ChannelError::Close(format!("Peer's feerate much too low. Actual: {}. Our expected lower limit: {}", feerate_per_kw, lower_limit)));
+               // 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.
@@ -861,12 +935,13 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Creates a new channel from a remote sides' request for one.
        /// Assumes chain_hash has already been checked and corresponds with what we expect!
-       pub fn new_from_req<K: Deref, F: Deref>(
+       pub fn new_from_req<K: Deref, F: Deref, L: Deref>(
                fee_estimator: &F, keys_provider: &K, counterparty_node_id: PublicKey, their_features: &InitFeatures,
-               msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32
+               msg: &msgs::OpenChannel, user_id: u64, config: &UserConfig, current_chain_height: u32, logger: &L
        ) -> Result<Channel<Signer>, ChannelError>
                where K::Target: KeysInterface<Signer = Signer>,
-          F::Target: FeeEstimator
+                     F::Target: FeeEstimator,
+                     L::Target: Logger,
        {
                // First check the channel type is known, failing before we do anything else if we don't
                // support this channel type.
@@ -914,9 +989,6 @@ impl<Signer: Sign> Channel<Signer> {
                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.dust_limit_satoshis > msg.channel_reserve_satoshis {
-                       return Err(ChannelError::Close(format!("Bogus; channel reserve ({}) is less than dust limit ({})", msg.channel_reserve_satoshis, msg.dust_limit_satoshis)));
-               }
                let full_channel_value_msat = (msg.funding_satoshis - msg.channel_reserve_satoshis) * 1000;
                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)));
@@ -973,7 +1045,8 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close(format!("Suitable channel reserve not found. remote_channel_reserve was ({}). dust_limit_satoshis is ({}).", holder_selected_channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS)));
                }
                if msg.channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS {
-                       return Err(ChannelError::Close(format!("channel_reserve_satoshis ({}) is smaller than our dust limit ({})", msg.channel_reserve_satoshis, MIN_CHAN_DUST_LIMIT_SATOSHIS)));
+                       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)));
@@ -1033,6 +1106,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                        channel_id: msg.temporary_channel_id,
                        channel_state: (ChannelState::OurInitSent as u32) | (ChannelState::TheirInitSent as u32),
+                       announcement_sigs_state: AnnouncementSigsState::NotSent,
                        secp_ctx,
 
                        latest_monitor_update_id: 0,
@@ -1083,7 +1157,9 @@ impl<Signer: Sign> Channel<Signer> {
                        counterparty_dust_limit_satoshis: msg.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: Self::get_holder_max_htlc_value_in_flight_msat(msg.funding_satoshis),
                        counterparty_selected_channel_reserve_satoshis: Some(msg.channel_reserve_satoshis),
+                       holder_selected_channel_reserve_satoshis,
                        counterparty_htlc_minimum_msat: msg.htlc_minimum_msat,
                        holder_htlc_minimum_msat: if config.own_channel_config.our_htlc_minimum_msat == 0 { 1 } else { config.own_channel_config.our_htlc_minimum_msat },
                        counterparty_max_accepted_htlcs: msg.max_accepted_htlcs,
@@ -1288,7 +1364,7 @@ impl<Signer: Sign> Channel<Signer> {
                        };
                        debug_assert!(broadcaster_max_commitment_tx_output.0 <= value_to_self_msat as u64 || value_to_self_msat / 1000 >= self.counterparty_selected_channel_reserve_satoshis.unwrap() as i64);
                        broadcaster_max_commitment_tx_output.0 = cmp::max(broadcaster_max_commitment_tx_output.0, value_to_self_msat as u64);
-                       debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64);
+                       debug_assert!(broadcaster_max_commitment_tx_output.1 <= value_to_remote_msat as u64 || value_to_remote_msat / 1000 >= self.holder_selected_channel_reserve_satoshis as i64);
                        broadcaster_max_commitment_tx_output.1 = cmp::max(broadcaster_max_commitment_tx_output.1, value_to_remote_msat as u64);
                }
 
@@ -1703,12 +1779,8 @@ impl<Signer: Sign> Channel<Signer> {
                if msg.channel_reserve_satoshis > self.channel_value_satoshis {
                        return Err(ChannelError::Close(format!("Bogus channel_reserve_satoshis ({}). Must not be greater than ({})", msg.channel_reserve_satoshis, self.channel_value_satoshis)));
                }
-               if msg.channel_reserve_satoshis < self.holder_dust_limit_satoshis {
-                       return Err(ChannelError::Close(format!("Peer never wants payout outputs? channel_reserve_satoshis was ({}). dust_limit is ({})", msg.channel_reserve_satoshis, self.holder_dust_limit_satoshis)));
-               }
-               let remote_reserve = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis);
-               if msg.dust_limit_satoshis > remote_reserve {
-                       return Err(ChannelError::Close(format!("Dust limit ({}) is bigger than our channel reserve ({})", msg.dust_limit_satoshis, remote_reserve)));
+               if msg.dust_limit_satoshis > self.holder_selected_channel_reserve_satoshis {
+                       return Err(ChannelError::Close(format!("Dust limit ({}) is bigger than our channel reserve ({})", msg.dust_limit_satoshis, self.holder_selected_channel_reserve_satoshis)));
                }
                let full_channel_value_msat = (self.channel_value_satoshis - msg.channel_reserve_satoshis) * 1000;
                if msg.htlc_minimum_msat >= full_channel_value_msat {
@@ -1987,7 +2059,10 @@ impl<Signer: Sign> Channel<Signer> {
                Ok((channel_monitor, self.funding_transaction.as_ref().cloned().unwrap()))
        }
 
-       pub fn funding_locked<L: Deref>(&mut self, msg: &msgs::FundingLocked, logger: &L) -> Result<(), ChannelError> where L::Target: Logger {
+       /// Handles a funding_locked message from our peer. If we've already sent our funding_locked
+       /// and the channel is now usable (and public), this may generate an announcement_signatures to
+       /// reply with.
+       pub fn funding_locked<L: Deref>(&mut self, msg: &msgs::FundingLocked, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block: &BestBlock, logger: &L) -> Result<Option<msgs::AnnouncementSignatures>, ChannelError> where L::Target: Logger {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == ChannelState::PeerDisconnected as u32 {
                        self.workaround_lnd_bug_4006 = Some(msg.clone());
                        return Err(ChannelError::Ignore("Peer sent funding_locked when we needed a channel_reestablish. The peer is likely lnd, see https://github.com/lightningnetwork/lnd/issues/4006".to_owned()));
@@ -2011,7 +2086,7 @@ impl<Signer: Sign> Channel<Signer> {
                                return Err(ChannelError::Close("Peer sent a reconnect funding_locked with a different point".to_owned()));
                        }
                        // They probably disconnected/reconnected and re-sent the funding_locked, which is required
-                       return Ok(());
+                       return Ok(None);
                } else {
                        return Err(ChannelError::Close("Peer sent a funding_locked at a strange time".to_owned()));
                }
@@ -2021,7 +2096,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                log_info!(logger, "Received funding_locked from peer for channel {}", log_bytes!(self.channel_id()));
 
-               Ok(())
+               Ok(self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height()).ok())
        }
 
        /// Returns transaction if there is pending funding transaction that is yet to broadcast
@@ -2103,13 +2178,15 @@ impl<Signer: Sign> Channel<Signer> {
        /// Doesn't bother handling the
        /// if-we-removed-it-already-but-haven't-fully-resolved-they-can-still-send-an-inbound-HTLC
        /// corner case properly.
+       /// The channel reserve is subtracted from each balance.
+       /// See also [`Channel::get_balance_msat`]
        pub fn get_inbound_outbound_available_balance_msat(&self) -> (u64, u64) {
                // Note that we have to handle overflow due to the above case.
                (
                        cmp::max(self.channel_value_satoshis as i64 * 1000
                                - self.value_to_self_msat as i64
                                - self.get_inbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64
-                               - Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) as i64 * 1000,
+                               - self.holder_selected_channel_reserve_satoshis as i64 * 1000,
                        0) as u64,
                        cmp::max(self.value_to_self_msat as i64
                                - self.get_outbound_pending_htlc_stats(None).pending_htlcs_value_msat as i64
@@ -2118,9 +2195,16 @@ impl<Signer: Sign> Channel<Signer> {
                )
        }
 
+       /// Get our total balance in msat.
+       /// 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
+       }
+
        pub fn get_holder_counterparty_selected_channel_reserve_satoshis(&self) -> (u64, Option<u64>) {
-               (Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
-               self.counterparty_selected_channel_reserve_satoshis)
+               (self.holder_selected_channel_reserve_satoshis, self.counterparty_selected_channel_reserve_satoshis)
        }
 
        // Get the fee cost in MSATS of a commitment tx with a given number of HTLC outputs.
@@ -2336,9 +2420,8 @@ impl<Signer: Sign> Channel<Signer> {
                if inbound_stats.pending_htlcs + 1 > OUR_MAX_HTLCS as u32 {
                        return Err(ChannelError::Close(format!("Remote tried to push more than our max accepted HTLCs ({})", OUR_MAX_HTLCS)));
                }
-               let holder_max_htlc_value_in_flight_msat = Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis);
-               if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > holder_max_htlc_value_in_flight_msat {
-                       return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", holder_max_htlc_value_in_flight_msat)));
+               if inbound_stats.pending_htlcs_value_msat + msg.amount_msat > self.holder_max_htlc_value_in_flight_msat {
+                       return Err(ChannelError::Close(format!("Remote HTLC add would put them over our max HTLC value ({})", self.holder_max_htlc_value_in_flight_msat)));
                }
                // Check holder_selected_channel_reserve_satoshis (we're getting paid, so they have to at least meet
                // the reserve_satoshis we told them to always have as direct payment so that they lose
@@ -2399,9 +2482,7 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
                };
 
-               let chan_reserve_msat =
-                       Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
-               if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < chan_reserve_msat {
+               if pending_remote_value_msat - msg.amount_msat - remote_commit_tx_fee_msat < self.holder_selected_channel_reserve_satoshis * 1000 {
                        return Err(ChannelError::Close("Remote HTLC add would put them under remote reserve value".to_owned()));
                }
 
@@ -2416,7 +2497,7 @@ impl<Signer: Sign> Channel<Signer> {
                        // sensitive to fee spikes.
                        let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
                        let remote_fee_cost_incl_stuck_buffer_msat = 2 * self.next_remote_commit_tx_fee_msat(htlc_candidate, Some(()));
-                       if pending_remote_value_msat - msg.amount_msat - chan_reserve_msat < remote_fee_cost_incl_stuck_buffer_msat {
+                       if pending_remote_value_msat - msg.amount_msat - self.holder_selected_channel_reserve_satoshis * 1000 < remote_fee_cost_incl_stuck_buffer_msat {
                                // Note that if the pending_forward_status is not updated here, then it's because we're already failing
                                // the HTLC, i.e. its status is already set to failing.
                                log_info!(logger, "Attempting to fail HTLC due to fee spike buffer violation in channel {}. Rebalancing is required.", log_bytes!(self.channel_id()));
@@ -2559,7 +2640,7 @@ impl<Signer: Sign> Channel<Signer> {
                } else { false };
                if update_fee {
                        debug_assert!(!self.is_outbound());
-                       let counterparty_reserve_we_require_msat = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+                       let counterparty_reserve_we_require_msat = self.holder_selected_channel_reserve_satoshis * 1000;
                        if commitment_stats.remote_balance_msat < commitment_stats.total_fee_sat * 1000 + counterparty_reserve_we_require_msat {
                                return Err((None, ChannelError::Close("Funding remote cannot afford proposed new fee".to_owned())));
                        }
@@ -2900,6 +2981,10 @@ impl<Signer: Sign> Channel<Signer> {
                self.counterparty_cur_commitment_point = Some(msg.next_per_commitment_point);
                self.cur_counterparty_commitment_transaction_number -= 1;
 
+               if self.announcement_sigs_state == AnnouncementSigsState::Committed {
+                       self.announcement_sigs_state = AnnouncementSigsState::PeerReceived;
+               }
+
                log_trace!(logger, "Updating HTLCs on receipt of RAA in channel {}...", log_bytes!(self.channel_id()));
                let mut to_forward_infos = Vec::new();
                let mut revoked_htlcs = Vec::new();
@@ -3175,6 +3260,11 @@ impl<Signer: Sign> Channel<Signer> {
                        self.channel_state = ChannelState::ShutdownComplete as u32;
                        return;
                }
+
+               if self.announcement_sigs_state == AnnouncementSigsState::MessageSent || self.announcement_sigs_state == AnnouncementSigsState::Committed {
+                       self.announcement_sigs_state = AnnouncementSigsState::NotSent;
+               }
+
                // Upon reconnect we have to start the closing_signed dance over, but shutdown messages
                // will be retransmitted.
                self.last_sent_closing_fee = None;
@@ -3251,7 +3341,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// Indicates that the latest ChannelMonitor update has been committed by the client
        /// successfully and we should restore normal operation. Returns messages which should be sent
        /// to the remote side.
-       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L) -> MonitorRestoreUpdates where L::Target: Logger {
+       pub fn monitor_updating_restored<L: Deref>(&mut self, logger: &L, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32) -> MonitorRestoreUpdates where L::Target: Logger {
                assert_eq!(self.channel_state & ChannelState::MonitorUpdateFailed as u32, ChannelState::MonitorUpdateFailed as u32);
                self.channel_state &= !(ChannelState::MonitorUpdateFailed as u32);
 
@@ -3274,6 +3364,8 @@ impl<Signer: Sign> Channel<Signer> {
                        })
                } else { None };
 
+               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block_height).ok();
+
                let mut accepted_htlcs = Vec::new();
                mem::swap(&mut accepted_htlcs, &mut self.monitor_pending_forwards);
                let mut failed_htlcs = Vec::new();
@@ -3286,7 +3378,7 @@ impl<Signer: Sign> Channel<Signer> {
                        self.monitor_pending_commitment_signed = false;
                        return MonitorRestoreUpdates {
                                raa: None, commitment_update: None, order: RAACommitmentOrder::RevokeAndACKFirst,
-                               accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked
+                               accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked, announcement_sigs
                        };
                }
 
@@ -3305,7 +3397,7 @@ impl<Signer: Sign> Channel<Signer> {
                        if commitment_update.is_some() { "a" } else { "no" }, if raa.is_some() { "an" } else { "no" },
                        match order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"});
                MonitorRestoreUpdates {
-                       raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked
+                       raa, commitment_update, order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, funding_broadcastable, funding_locked, announcement_sigs
                }
        }
 
@@ -3419,7 +3511,9 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// May panic if some calls other than message-handling calls (which will all Err immediately)
        /// have been called between remove_uncommitted_htlcs_and_mark_paused and this call.
-       pub fn channel_reestablish<L: Deref>(&mut self, msg: &msgs::ChannelReestablish, logger: &L) -> Result<(Option<msgs::FundingLocked>, Option<msgs::RevokeAndACK>, Option<msgs::CommitmentUpdate>, Option<ChannelMonitorUpdate>, RAACommitmentOrder, Vec<(HTLCSource, PaymentHash)>, Option<msgs::Shutdown>), ChannelError> where L::Target: Logger {
+       pub fn channel_reestablish<L: Deref>(&mut self, msg: &msgs::ChannelReestablish, logger: &L,
+               node_pk: PublicKey, genesis_block_hash: BlockHash, best_block: &BestBlock)
+       -> Result<ReestablishResponses, ChannelError> where L::Target: Logger {
                if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 {
                        // While BOLT 2 doesn't indicate explicitly we should error this channel here, it
                        // almost certainly indicates we are going to end up out-of-sync in some way, so we
@@ -3455,7 +3549,7 @@ impl<Signer: Sign> Channel<Signer> {
                // remaining cases either succeed or ErrorMessage-fail).
                self.channel_state &= !(ChannelState::PeerDisconnected as u32);
 
-               let shutdown_msg = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 {
+               let shutdown = if self.channel_state & (ChannelState::LocalShutdownSent as u32) != 0 {
                        assert!(self.shutdown_scriptpubkey.is_some());
                        Some(msgs::Shutdown {
                                channel_id: self.channel_id,
@@ -3463,6 +3557,8 @@ impl<Signer: Sign> Channel<Signer> {
                        })
                } else { None };
 
+               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, best_block.height()).ok();
+
                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.
                        if self.channel_state & (ChannelState::OurFundingLocked as u32) == 0 ||
@@ -3471,15 +3567,27 @@ impl<Signer: Sign> Channel<Signer> {
                                        return Err(ChannelError::Close("Peer claimed they saw a revoke_and_ack but we haven't sent funding_locked yet".to_owned()));
                                }
                                // Short circuit the whole handler as there is nothing we can resend them
-                               return Ok((None, None, None, None, RAACommitmentOrder::CommitmentFirst, Vec::new(), shutdown_msg));
+                               return Ok(ReestablishResponses {
+                                       funding_locked: None,
+                                       raa: None, commitment_update: None, mon_update: None,
+                                       order: RAACommitmentOrder::CommitmentFirst,
+                                       holding_cell_failed_htlcs: Vec::new(),
+                                       shutdown, announcement_sigs,
+                               });
                        }
 
                        // We have OurFundingLocked set!
                        let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
-                       return Ok((Some(msgs::FundingLocked {
-                               channel_id: self.channel_id(),
-                               next_per_commitment_point,
-                       }), None, None, None, RAACommitmentOrder::CommitmentFirst, Vec::new(), shutdown_msg));
+                       return Ok(ReestablishResponses {
+                               funding_locked: Some(msgs::FundingLocked {
+                                       channel_id: self.channel_id(),
+                                       next_per_commitment_point,
+                               }),
+                               raa: None, commitment_update: None, mon_update: None,
+                               order: RAACommitmentOrder::CommitmentFirst,
+                               holding_cell_failed_htlcs: Vec::new(),
+                               shutdown, announcement_sigs,
+                       });
                }
 
                let required_revoke = if msg.next_remote_commitment_number + 1 == INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number {
@@ -3503,7 +3611,7 @@ impl<Signer: Sign> Channel<Signer> {
                // the corresponding revoke_and_ack back yet.
                let next_counterparty_commitment_number = INITIAL_COMMITMENT_NUMBER - self.cur_counterparty_commitment_transaction_number + if (self.channel_state & ChannelState::AwaitingRemoteRevoke as u32) != 0 { 1 } else { 0 };
 
-               let resend_funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number == 1 {
+               let funding_locked = if msg.next_local_commitment_number == 1 && INITIAL_COMMITMENT_NUMBER - self.cur_holder_commitment_transaction_number == 1 {
                        // We should never have to worry about MonitorUpdateFailed resending FundingLocked
                        let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
                        Some(msgs::FundingLocked {
@@ -3525,18 +3633,39 @@ impl<Signer: Sign> Channel<Signer> {
                                // have received some updates while we were disconnected. Free the holding cell
                                // now!
                                match self.free_holding_cell_htlcs(logger) {
-                                       Err(ChannelError::Close(msg)) => return Err(ChannelError::Close(msg)),
+                                       Err(ChannelError::Close(msg)) => Err(ChannelError::Close(msg)),
                                        Err(ChannelError::Warn(_)) | Err(ChannelError::Ignore(_)) | Err(ChannelError::CloseDelayBroadcast(_)) =>
                                                panic!("Got non-channel-failing result from free_holding_cell_htlcs"),
-                                       Ok((Some((commitment_update, monitor_update)), htlcs_to_fail)) => {
-                                               return Ok((resend_funding_locked, required_revoke, Some(commitment_update), Some(monitor_update), self.resend_order.clone(), htlcs_to_fail, shutdown_msg));
+                                       Ok((Some((commitment_update, monitor_update)), holding_cell_failed_htlcs)) => {
+                                               Ok(ReestablishResponses {
+                                                       funding_locked, shutdown, announcement_sigs,
+                                                       raa: required_revoke,
+                                                       commitment_update: Some(commitment_update),
+                                                       order: self.resend_order.clone(),
+                                                       mon_update: Some(monitor_update),
+                                                       holding_cell_failed_htlcs,
+                                               })
                                        },
-                                       Ok((None, htlcs_to_fail)) => {
-                                               return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), htlcs_to_fail, shutdown_msg));
+                                       Ok((None, holding_cell_failed_htlcs)) => {
+                                               Ok(ReestablishResponses {
+                                                       funding_locked, shutdown, announcement_sigs,
+                                                       raa: required_revoke,
+                                                       commitment_update: None,
+                                                       order: self.resend_order.clone(),
+                                                       mon_update: None,
+                                                       holding_cell_failed_htlcs,
+                                               })
                                        },
                                }
                        } else {
-                               return Ok((resend_funding_locked, required_revoke, None, None, self.resend_order.clone(), Vec::new(), shutdown_msg));
+                               Ok(ReestablishResponses {
+                                       funding_locked, shutdown, announcement_sigs,
+                                       raa: required_revoke,
+                                       commitment_update: None,
+                                       order: self.resend_order.clone(),
+                                       mon_update: None,
+                                       holding_cell_failed_htlcs: Vec::new(),
+                               })
                        }
                } else if msg.next_local_commitment_number == next_counterparty_commitment_number - 1 {
                        if required_revoke.is_some() {
@@ -3547,12 +3676,24 @@ impl<Signer: Sign> Channel<Signer> {
 
                        if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) != 0 {
                                self.monitor_pending_commitment_signed = true;
-                               return Ok((resend_funding_locked, None, None, None, self.resend_order.clone(), Vec::new(), shutdown_msg));
+                               Ok(ReestablishResponses {
+                                       funding_locked, shutdown, announcement_sigs,
+                                       commitment_update: None, raa: None, mon_update: None,
+                                       order: self.resend_order.clone(),
+                                       holding_cell_failed_htlcs: Vec::new(),
+                               })
+                       } else {
+                               Ok(ReestablishResponses {
+                                       funding_locked, shutdown, announcement_sigs,
+                                       raa: required_revoke,
+                                       commitment_update: Some(self.get_last_commitment_update(logger)),
+                                       order: self.resend_order.clone(),
+                                       mon_update: None,
+                                       holding_cell_failed_htlcs: Vec::new(),
+                               })
                        }
-
-                       return Ok((resend_funding_locked, required_revoke, Some(self.get_last_commitment_update(logger)), None, self.resend_order.clone(), Vec::new(), shutdown_msg));
                } else {
-                       return Err(ChannelError::Close("Peer attempted to reestablish channel with a very old remote commitment transaction".to_owned()));
+                       Err(ChannelError::Close("Peer attempted to reestablish channel with a very old remote commitment transaction".to_owned()))
                }
        }
 
@@ -4000,7 +4141,7 @@ impl<Signer: Sign> Channel<Signer> {
                        // channel might have been used to route very small values (either by honest users or as DoS).
                        self.channel_value_satoshis * 1000 * 9 / 10,
 
-                       Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis)
+                       self.holder_max_htlc_value_in_flight_msat
                );
        }
 
@@ -4121,7 +4262,7 @@ impl<Signer: Sign> Channel<Signer> {
        /// Allowed in any state (including after shutdown)
        pub fn is_usable(&self) -> bool {
                let mask = ChannelState::ChannelFunded as u32 | BOTH_SIDES_SHUTDOWN_MASK;
-               (self.channel_state & mask) == (ChannelState::ChannelFunded as u32)
+               (self.channel_state & mask) == (ChannelState::ChannelFunded as u32) && !self.monitor_pending_funding_locked
        }
 
        /// Returns true if this channel is currently available for use. This is a superset of
@@ -4167,6 +4308,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
 
        pub fn set_channel_update_status(&mut self, status: ChannelUpdateStatus) {
+               self.update_time_counter += 1;
                self.channel_update_status = status;
        }
 
@@ -4204,11 +4346,13 @@ impl<Signer: Sign> Channel<Signer> {
 
                if need_commitment_update {
                        if self.channel_state & (ChannelState::MonitorUpdateFailed as u32) == 0 {
-                               let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
-                               return Some(msgs::FundingLocked {
-                                       channel_id: self.channel_id,
-                                       next_per_commitment_point,
-                               });
+                               if self.channel_state & (ChannelState::PeerDisconnected as u32) == 0 {
+                                       let next_per_commitment_point = self.holder_signer.get_per_commitment_point(self.cur_holder_commitment_transaction_number, &self.secp_ctx);
+                                       return Some(msgs::FundingLocked {
+                                               channel_id: self.channel_id,
+                                               next_per_commitment_point,
+                                       });
+                               }
                        } else {
                                self.monitor_pending_funding_locked = true;
                        }
@@ -4219,8 +4363,9 @@ impl<Signer: Sign> Channel<Signer> {
        /// When a transaction is confirmed, we check whether it is or spends the funding transaction
        /// In the first case, we store the confirmation height and calculating the short channel id.
        /// In the second, we simply return an Err indicating we need to be force-closed now.
-       pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32, txdata: &TransactionData, logger: &L)
-       -> Result<Option<msgs::FundingLocked>, ClosureReason> where L::Target: Logger {
+       pub fn transactions_confirmed<L: Deref>(&mut self, block_hash: &BlockHash, height: u32,
+               txdata: &TransactionData, genesis_block_hash: BlockHash, node_pk: PublicKey, logger: &L)
+       -> Result<(Option<msgs::FundingLocked>, Option<msgs::AnnouncementSignatures>), ClosureReason> where L::Target: Logger {
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
                for &(index_in_block, tx) in txdata.iter() {
                        if let Some(funding_txo) = self.get_funding_txo() {
@@ -4267,7 +4412,8 @@ 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));
-                                               return Ok(Some(funding_locked));
+                                               let announcement_sigs = self.get_announcement_sigs(node_pk, genesis_block_hash, height).ok();
+                                               return Ok((Some(funding_locked), announcement_sigs));
                                        }
                                }
                                for inp in tx.input.iter() {
@@ -4278,7 +4424,7 @@ impl<Signer: Sign> Channel<Signer> {
                                }
                        }
                }
-               Ok(None)
+               Ok((None, None))
        }
 
        /// When a new block is connected, we check the height of the block against outbound holding
@@ -4292,8 +4438,13 @@ impl<Signer: Sign> Channel<Signer> {
        ///
        /// May return some HTLCs (and their payment_hash) which have timed out and should be failed
        /// back.
-       pub fn best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, logger: &L)
-       -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>), ClosureReason> where L::Target: Logger {
+       pub fn best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, genesis_block_hash: BlockHash, node_pk: PublicKey, logger: &L)
+       -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason> where L::Target: Logger {
+               self.do_best_block_updated(height, highest_header_time, Some((genesis_block_hash, node_pk)), logger)
+       }
+
+       fn do_best_block_updated<L: Deref>(&mut self, height: u32, highest_header_time: u32, genesis_node_pk: Option<(BlockHash, PublicKey)>, logger: &L)
+       -> Result<(Option<msgs::FundingLocked>, Vec<(HTLCSource, PaymentHash)>, Option<msgs::AnnouncementSignatures>), ClosureReason> where L::Target: Logger {
                let mut timed_out_htlcs = Vec::new();
                // This mirrors the check in ChannelManager::decode_update_add_htlc_onion, refusing to
                // forward an HTLC when our counterparty should almost certainly just fail it for expiring
@@ -4314,8 +4465,11 @@ impl<Signer: Sign> Channel<Signer> {
                self.update_time_counter = cmp::max(self.update_time_counter, highest_header_time);
 
                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()
+                       } 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));
+                       return Ok((Some(funding_locked), timed_out_htlcs, announcement_sigs));
                }
 
                let non_shutdown_state = self.channel_state & (!MULTI_STATE_FLAGS);
@@ -4347,7 +4501,10 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ClosureReason::FundingTimedOut);
                }
 
-               Ok((None, timed_out_htlcs))
+               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()
+               } else { None };
+               Ok((None, timed_out_htlcs, announcement_sigs))
        }
 
        /// Indicates the funding transaction is no longer confirmed in the main chain. This may
@@ -4362,10 +4519,11 @@ impl<Signer: Sign> Channel<Signer> {
                        // larger. If we don't know that time has moved forward, we can just set it to the last
                        // time we saw and it will be ignored.
                        let best_time = self.update_time_counter;
-                       match self.best_block_updated(reorg_height, best_time, logger) {
-                               Ok((funding_locked, timed_out_htlcs)) => {
+                       match self.do_best_block_updated(reorg_height, best_time, None, logger) {
+                               Ok((funding_locked, timed_out_htlcs, announcement_sigs)) => {
                                        assert!(funding_locked.is_none(), "We can't generate a funding with 0 confirmations?");
                                        assert!(timed_out_htlcs.is_empty(), "We can't have accepted HTLCs with a timeout before our funding confirmation?");
+                                       assert!(announcement_sigs.is_none(), "We can't generate an announcement_sigs with 0 confirmations?");
                                        Ok(())
                                },
                                Err(e) => Err(e)
@@ -4400,8 +4558,8 @@ impl<Signer: Sign> Channel<Signer> {
                        funding_satoshis: self.channel_value_satoshis,
                        push_msat: self.channel_value_satoshis * 1000 - self.value_to_self_msat,
                        dust_limit_satoshis: self.holder_dust_limit_satoshis,
-                       max_htlc_value_in_flight_msat: Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
-                       channel_reserve_satoshis: Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
+                       max_htlc_value_in_flight_msat: self.holder_max_htlc_value_in_flight_msat,
+                       channel_reserve_satoshis: self.holder_selected_channel_reserve_satoshis,
                        htlc_minimum_msat: self.holder_htlc_minimum_msat,
                        feerate_per_kw: self.feerate_per_kw as u32,
                        to_self_delay: self.get_holder_selected_contest_delay(),
@@ -4438,8 +4596,8 @@ impl<Signer: Sign> Channel<Signer> {
                msgs::AcceptChannel {
                        temporary_channel_id: self.channel_id,
                        dust_limit_satoshis: self.holder_dust_limit_satoshis,
-                       max_htlc_value_in_flight_msat: Channel::<Signer>::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis),
-                       channel_reserve_satoshis: Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis),
+                       max_htlc_value_in_flight_msat: self.holder_max_htlc_value_in_flight_msat,
+                       channel_reserve_satoshis: self.holder_selected_channel_reserve_satoshis,
                        htlc_minimum_msat: self.holder_htlc_minimum_msat,
                        minimum_depth: self.minimum_depth.unwrap(),
                        to_self_delay: self.get_holder_selected_contest_delay(),
@@ -4513,25 +4671,24 @@ impl<Signer: Sign> Channel<Signer> {
                })
        }
 
-       /// Gets an UnsignedChannelAnnouncement, as well as a signature covering it using our
-       /// bitcoin_key, if available, 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.
+       /// 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.
+       ///
        /// 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.
-       pub fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<(msgs::UnsignedChannelAnnouncement, Signature), ChannelError> {
+       fn get_channel_announcement(&self, node_id: PublicKey, chain_hash: BlockHash) -> Result<msgs::UnsignedChannelAnnouncement, ChannelError> {
                if !self.config.announced_channel {
                        return Err(ChannelError::Ignore("Channel is not available for public announcements".to_owned()));
                }
-               if self.channel_state & (ChannelState::ChannelFunded as u32) == 0 {
-                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement until the channel funding has been locked".to_owned()));
-               }
-               if (self.channel_state & (ChannelState::LocalShutdownSent as u32 | ChannelState::ShutdownComplete as u32)) != 0 {
-                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement once the channel is closing".to_owned()));
+               if !self.is_usable() {
+                       return Err(ChannelError::Ignore("Cannot get a ChannelAnnouncement if the channel is not currently usable".to_owned()));
                }
 
                let were_node_one = node_id.serialize()[..] < self.counterparty_node_id.serialize()[..];
@@ -4547,19 +4704,47 @@ impl<Signer: Sign> Channel<Signer> {
                        excess_data: Vec::new(),
                };
 
-               let sig = self.holder_signer.sign_channel_announcement(&msg, &self.secp_ctx)
+               Ok(msg)
+       }
+
+       fn get_announcement_sigs(&mut self, node_pk: PublicKey, genesis_block_hash: BlockHash, best_block_height: u32) -> Result<msgs::AnnouncementSignatures, ChannelError> {
+               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()));
+               }
+
+               if !self.is_usable() {
+                       return Err(ChannelError::Ignore("Channel not yet available for use".to_owned()));
+               }
+
+               if self.channel_state & ChannelState::PeerDisconnected as u32 != 0 {
+                       return Err(ChannelError::Ignore("Peer currently disconnected".to_owned()));
+               }
+
+               if self.announcement_sigs_state != AnnouncementSigsState::NotSent {
+                       return Err(ChannelError::Ignore("Announcement signatures already sent".to_owned()));
+               }
+
+               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()))?;
+               self.announcement_sigs_state = AnnouncementSigsState::MessageSent;
 
-               Ok((msg, sig))
+               Ok(msgs::AnnouncementSignatures {
+                       channel_id: self.channel_id(),
+                       short_channel_id: self.get_short_channel_id().unwrap(),
+                       node_signature: our_node_sig,
+                       bitcoin_signature: our_bitcoin_sig,
+               })
        }
 
        /// Signs the given channel announcement, returning a ChannelError::Ignore if no keys are
        /// available.
-       fn sign_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, msghash: secp256k1::Message, announcement: msgs::UnsignedChannelAnnouncement, our_bitcoin_sig: Signature) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+       fn sign_channel_announcement(&self, our_node_id: PublicKey, announcement: msgs::UnsignedChannelAnnouncement) -> Result<msgs::ChannelAnnouncement, ChannelError> {
                if let Some((their_node_sig, their_bitcoin_sig)) = self.announcement_sigs {
                        let were_node_one = announcement.node_id_1 == our_node_id;
 
-                       let our_node_sig = self.secp_ctx.sign(&msghash, our_node_secret);
+                       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()))?;
                        Ok(msgs::ChannelAnnouncement {
                                node_signature_1: if were_node_one { our_node_sig } else { their_node_sig },
                                node_signature_2: if were_node_one { their_node_sig } else { our_node_sig },
@@ -4575,8 +4760,8 @@ impl<Signer: Sign> Channel<Signer> {
        /// Processes an incoming announcement_signatures message, providing a fully-signed
        /// channel_announcement message which we can broadcast and storing our counterparty's
        /// signatures for later reconstruction/rebroadcast of the channel_announcement.
-       pub fn announcement_signatures(&mut self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash, msg: &msgs::AnnouncementSignatures) -> Result<msgs::ChannelAnnouncement, ChannelError> {
-               let (announcement, our_bitcoin_sig) = self.get_channel_announcement(our_node_id.clone(), chain_hash)?;
+       pub fn announcement_signatures(&mut self, our_node_id: PublicKey, chain_hash: BlockHash, best_block_height: u32, msg: &msgs::AnnouncementSignatures) -> Result<msgs::ChannelAnnouncement, ChannelError> {
+               let announcement = self.get_channel_announcement(our_node_id.clone(), chain_hash)?;
 
                let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
 
@@ -4592,19 +4777,25 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                self.announcement_sigs = Some((msg.node_signature, msg.bitcoin_signature));
+               if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height {
+                       return Err(ChannelError::Ignore(
+                               "Got announcement_signatures prior to the required six confirmations - we may not have received a block yet that our peer has".to_owned()));
+               }
 
-               self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig)
+               self.sign_channel_announcement(our_node_id, announcement)
        }
 
        /// Gets a signed channel_announcement for this channel, if we previously received an
        /// announcement_signatures from our counterparty.
-       pub fn get_signed_channel_announcement(&self, our_node_secret: &SecretKey, our_node_id: PublicKey, chain_hash: BlockHash) -> Option<msgs::ChannelAnnouncement> {
-               let (announcement, our_bitcoin_sig) = match self.get_channel_announcement(our_node_id.clone(), chain_hash) {
+       pub fn get_signed_channel_announcement(&self, our_node_id: PublicKey, chain_hash: BlockHash, best_block_height: u32) -> Option<msgs::ChannelAnnouncement> {
+               if self.funding_tx_confirmation_height == 0 || self.funding_tx_confirmation_height + 5 > best_block_height {
+                       return None;
+               }
+               let announcement = match self.get_channel_announcement(our_node_id.clone(), chain_hash) {
                        Ok(res) => res,
                        Err(_) => return None,
                };
-               let msghash = hash_to_message!(&Sha256d::hash(&announcement.encode()[..])[..]);
-               match self.sign_channel_announcement(our_node_secret, our_node_id, msghash, announcement, our_bitcoin_sig) {
+               match self.sign_channel_announcement(our_node_id, announcement) {
                        Ok(res) => Some(res),
                        Err(_) => None,
                }
@@ -4722,7 +4913,7 @@ impl<Signer: Sign> Channel<Signer> {
                        // Check that we won't violate the remote channel reserve by adding this HTLC.
                        let htlc_candidate = HTLCCandidate::new(amount_msat, HTLCInitiator::LocalOffered);
                        let counterparty_commit_tx_fee_msat = self.next_remote_commit_tx_fee_msat(htlc_candidate, None);
-                       let holder_selected_chan_reserve_msat = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis) * 1000;
+                       let holder_selected_chan_reserve_msat = self.holder_selected_channel_reserve_satoshis * 1000;
                        if commitment_stats.remote_balance_msat < counterparty_commit_tx_fee_msat + holder_selected_chan_reserve_msat {
                                return Err(ChannelError::Ignore("Cannot send value that would put counterparty balance under holder-announced channel reserve value".to_owned()));
                        }
@@ -4879,6 +5070,10 @@ impl<Signer: Sign> Channel<Signer> {
                        Err(e) => return Err(e),
                };
 
+               if self.announcement_sigs_state == AnnouncementSigsState::MessageSent {
+                       self.announcement_sigs_state = AnnouncementSigsState::Committed;
+               }
+
                self.latest_monitor_update_id += 1;
                let monitor_update = ChannelMonitorUpdate {
                        update_id: self.latest_monitor_update_id,
@@ -5138,6 +5333,29 @@ impl Readable for ChannelUpdateStatus {
        }
 }
 
+impl Writeable for AnnouncementSigsState {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               // We only care about writing out the current state as if we had just disconnected, at
+               // which point we always set anything but AnnouncementSigsReceived to NotSent.
+               match self {
+                       AnnouncementSigsState::NotSent => 0u8.write(writer),
+                       AnnouncementSigsState::MessageSent => 0u8.write(writer),
+                       AnnouncementSigsState::Committed => 0u8.write(writer),
+                       AnnouncementSigsState::PeerReceived => 1u8.write(writer),
+               }
+       }
+}
+
+impl Readable for AnnouncementSigsState {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
+               Ok(match <u8 as Readable>::read(reader)? {
+                       0 => AnnouncementSigsState::NotSent,
+                       1 => AnnouncementSigsState::PeerReceived,
+                       _ => return Err(DecodeError::InvalidValue),
+               })
+       }
+}
+
 impl<Signer: Sign> Writeable for Channel<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
                // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been
@@ -5368,6 +5586,15 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                let chan_type = if self.channel_type != ChannelTypeFeatures::only_static_remote_key() {
                        Some(&self.channel_type) } else { None };
 
+               // The same logic applies for `holder_selected_channel_reserve_satoshis` and
+               // `holder_max_htlc_value_in_flight_msat` values other than the defaults.
+               let serialized_holder_selected_reserve =
+                       if self.holder_selected_channel_reserve_satoshis != Self::get_holder_selected_channel_reserve_satoshis(self.channel_value_satoshis)
+                       { Some(self.holder_selected_channel_reserve_satoshis) } else { None };
+               let serialized_holder_htlc_max_in_flight =
+                       if self.holder_max_htlc_value_in_flight_msat != Self::get_holder_max_htlc_value_in_flight_msat(self.channel_value_satoshis)
+                       { Some(self.holder_max_htlc_value_in_flight_msat) } else { None };
+
                write_tlv_fields!(writer, {
                        (0, self.announcement_sigs, option),
                        // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a
@@ -5379,11 +5606,14 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                        (1, self.minimum_depth, option),
                        (2, chan_type, option),
                        (3, self.counterparty_selected_channel_reserve_satoshis, option),
+                       (4, serialized_holder_selected_reserve, option),
                        (5, self.config, required),
+                       (6, serialized_holder_htlc_max_in_flight, option),
                        (7, self.shutdown_scriptpubkey, option),
                        (9, self.target_closing_feerate_sats_per_kw, option),
                        (11, self.monitor_pending_finalized_fulfills, vec_type),
                        (13, self.channel_creation_height, required),
+                       (15, self.announcement_sigs_state, required),
                });
 
                Ok(())
@@ -5619,20 +5849,28 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                let mut announcement_sigs = None;
                let mut target_closing_feerate_sats_per_kw = None;
                let mut monitor_pending_finalized_fulfills = Some(Vec::new());
+               let mut holder_selected_channel_reserve_satoshis = Some(Self::get_holder_selected_channel_reserve_satoshis(channel_value_satoshis));
+               let mut holder_max_htlc_value_in_flight_msat = Some(Self::get_holder_max_htlc_value_in_flight_msat(channel_value_satoshis));
                // Prior to supporting channel type negotiation, all of our channels were static_remotekey
                // only, so we default to that if none was written.
                let mut channel_type = Some(ChannelTypeFeatures::only_static_remote_key());
                let mut channel_creation_height = Some(serialized_height);
+               // If we read an old Channel, for simplicity we just treat it as "we never sent an
+               // AnnouncementSignatures" which implies we'll re-send it on reconnect, but that's fine.
+               let mut announcement_sigs_state = Some(AnnouncementSigsState::NotSent);
                read_tlv_fields!(reader, {
                        (0, announcement_sigs, option),
                        (1, minimum_depth, option),
                        (2, channel_type, option),
                        (3, counterparty_selected_channel_reserve_satoshis, option),
+                       (4, holder_selected_channel_reserve_satoshis, option),
                        (5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
+                       (6, holder_max_htlc_value_in_flight_msat, option),
                        (7, shutdown_scriptpubkey, option),
                        (9, target_closing_feerate_sats_per_kw, option),
                        (11, monitor_pending_finalized_fulfills, vec_type),
                        (13, channel_creation_height, option),
+                       (15, announcement_sigs_state, option),
                });
 
                let chan_features = channel_type.as_ref().unwrap();
@@ -5656,6 +5894,7 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        config: config.unwrap(),
                        channel_id,
                        channel_state,
+                       announcement_sigs_state: announcement_sigs_state.unwrap(),
                        secp_ctx,
                        channel_value_satoshis,
 
@@ -5707,7 +5946,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        counterparty_dust_limit_satoshis,
                        holder_dust_limit_satoshis,
                        counterparty_max_htlc_value_in_flight_msat,
+                       holder_max_htlc_value_in_flight_msat: holder_max_htlc_value_in_flight_msat.unwrap(),
                        counterparty_selected_channel_reserve_satoshis,
+                       holder_selected_channel_reserve_satoshis: holder_selected_channel_reserve_satoshis.unwrap(),
                        counterparty_htlc_minimum_msat,
                        holder_htlc_minimum_msat,
                        counterparty_max_accepted_htlcs,
@@ -5768,7 +6009,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, KeysInterface, BaseSign};
+       use chain::keysinterface::{InMemorySigner, KeyMaterial, KeysInterface, BaseSign};
        use chain::transaction::OutPoint;
        use util::config::UserConfig;
        use util::enforcing_trait_impls::EnforcingSigner;
@@ -5809,6 +6050,7 @@ mod tests {
                type Signer = InMemorySigner;
 
                fn get_node_secret(&self) -> SecretKey { panic!(); }
+               fn get_inbound_payment_key_material(&self) -> KeyMaterial { panic!(); }
                fn get_destination_script(&self) -> Script {
                        let secp_ctx = Secp256k1::signing_only();
                        let channel_monitor_claim_key = SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap();
@@ -5891,6 +6133,7 @@ mod tests {
                let seed = [42; 32];
                let network = Network::Testnet;
                let keys_provider = test_utils::TestKeysInterface::new(&seed, network);
+               let logger = test_utils::TestLogger::new();
 
                // Go through the flow of opening a channel between two nodes, making sure
                // they have different dust limits.
@@ -5904,7 +6147,7 @@ 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).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();
 
                // Node B --> Node A: accept channel, explicitly setting B's dust limit.
                let mut accept_channel_msg = node_b_chan.get_accept_channel();
@@ -6022,7 +6265,7 @@ mod tests {
                // Create Node B's channel by receiving Node A's open_channel message
                let open_channel_msg = node_a_chan.get_open_channel(chain_hash);
                let node_b_node_id = PublicKey::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[7; 32]).unwrap());
-               let mut node_b_chan = Channel::<EnforcingSigner>::new_from_req(&&feeest, &&keys_provider, node_b_node_id, &InitFeatures::known(), &open_channel_msg, 7, &config, 0).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
                let accept_channel_msg = node_b_chan.get_accept_channel();
@@ -6124,6 +6367,7 @@ mod tests {
 
                let mut signer = InMemorySigner::new(
                        &secp_ctx,
+                       SecretKey::from_slice(&hex::decode("4242424242424242424242424242424242424242424242424242424242424242").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("30ff4956bbdd3222d44cc5e8a1261dab1e07957bdac5ae88fe3261ef321f3749").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("0fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff").unwrap()[..]).unwrap(),
                        SecretKey::from_slice(&hex::decode("1111111111111111111111111111111111111111111111111111111111111111").unwrap()[..]).unwrap(),