Merge pull request #1169 from TheBlueMatt/2021-11-fix-update-announcements
[rust-lightning] / lightning / src / ln / channel.rs
index 64f52288c6f363e90a215a853eb20bd04d72e97e..0cdcb0c14c2b0665f45c272e244f714208e29001 100644 (file)
@@ -387,9 +387,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
@@ -406,7 +406,12 @@ pub(crate) const FUNDING_CONF_DEADLINE_BLOCKS: u32 = 2016;
 /// size 2. However, if the number of concurrent update_add_htlc is higher, this still
 /// leads to a channel force-close. Ultimately, this is an issue coming from the
 /// design of LN state machines, allowing asynchronous updates.
-const CONCURRENT_INBOUND_HTLC_FEE_BUFFER: u32 = 2;
+pub(crate) const CONCURRENT_INBOUND_HTLC_FEE_BUFFER: u32 = 2;
+
+/// When a channel is opened, we check that the funding amount is enough to pay for relevant
+/// commitment transaction fees, with at least this many HTLCs present on the commitment
+/// transaction (not counting the value of the HTLCs themselves).
+pub(crate) const MIN_AFFORDABLE_HTLC_COUNT: usize = 4;
 
 // TODO: We should refactor this to be an Inbound/OutboundChannel until initial setup handshaking
 // has been completed, and then turn into a Channel to get compiler-time enforcement of things like
@@ -476,9 +481,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)>,
@@ -511,18 +521,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)]
@@ -674,6 +696,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
@@ -712,6 +738,12 @@ impl<Signer: Sign> Channel<Signer> {
 
                let feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
 
+               let value_to_self_msat = channel_value_satoshis * 1000 - push_msat;
+               let commitment_tx_fee = Self::commit_tx_fee_msat(feerate, MIN_AFFORDABLE_HTLC_COUNT);
+               if value_to_self_msat < commitment_tx_fee {
+                       return Err(APIError::APIMisuseError{ err: format!("Funding amount ({}) can't even pay fee for initial commitment transaction fee of {}.", value_to_self_msat / 1000, commitment_tx_fee / 1000) });
+               }
+
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_provider.get_secure_random_bytes());
 
@@ -742,7 +774,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                        cur_holder_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
                        cur_counterparty_commitment_transaction_number: INITIAL_COMMITMENT_NUMBER,
-                       value_to_self_msat: channel_value_satoshis * 1000 - push_msat,
+                       value_to_self_msat,
 
                        pending_inbound_htlcs: Vec::new(),
                        pending_outbound_htlcs: Vec::new(),
@@ -781,7 +813,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,
@@ -833,8 +867,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.
@@ -850,12 +888,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.
@@ -903,9 +942,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)));
@@ -957,31 +993,31 @@ impl<Signer: Sign> Channel<Signer> {
                // we either accept their preference or the preferences match
                local_config.announced_channel = announce;
 
-               let background_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-
                let holder_selected_channel_reserve_satoshis = Channel::<Signer>::get_holder_selected_channel_reserve_satoshis(msg.funding_satoshis);
                if holder_selected_channel_reserve_satoshis < MIN_CHAN_DUST_LIMIT_SATOSHIS {
                        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)));
                }
 
                // check if the funder's amount for the initial commitment tx is sufficient
-               // for full fee payment
+               // for full fee payment plus a few HTLCs to ensure the channel will be useful.
                let funders_amount_msat = msg.funding_satoshis * 1000 - msg.push_msat;
-               let lower_limit = background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT;
-               if funders_amount_msat < lower_limit {
-                       return Err(ChannelError::Close(format!("Insufficient funding amount ({}) for initial commitment. Must be at least {}", funders_amount_msat, lower_limit)));
+               let commitment_tx_fee = Self::commit_tx_fee_msat(msg.feerate_per_kw, MIN_AFFORDABLE_HTLC_COUNT) / 1000;
+               if funders_amount_msat / 1000 < commitment_tx_fee {
+                       return Err(ChannelError::Close(format!("Funding amount ({} sats) can't even pay fee for initial commitment transaction fee of {} sats.", funders_amount_msat / 1000, commitment_tx_fee)));
                }
 
-               let to_local_msat = msg.push_msat;
-               let to_remote_msat = funders_amount_msat - background_feerate as u64 * COMMITMENT_TX_BASE_WEIGHT;
-               if to_local_msat <= msg.channel_reserve_satoshis * 1000 && to_remote_msat <= holder_selected_channel_reserve_satoshis * 1000 {
-                       return Err(ChannelError::Close("Insufficient funding amount for initial commitment".to_owned()));
+               let to_remote_satoshis = funders_amount_msat / 1000 - commitment_tx_fee;
+               // While it's reasonable for us to not meet the channel reserve initially (if they don't
+               // want to push much to us), our counterparty should always have more than our reserve.
+               if to_remote_satoshis < holder_selected_channel_reserve_satoshis {
+                       return Err(ChannelError::Close("Insufficient funding amount for initial reserve".to_owned()));
                }
 
                let counterparty_shutdown_scriptpubkey = if their_features.supports_upfront_shutdown_script() {
@@ -1073,7 +1109,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,
@@ -1278,7 +1316,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);
                }
 
@@ -1693,12 +1731,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 {
@@ -2093,13 +2127,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
@@ -2108,17 +2144,24 @@ 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.
        // Note that num_htlcs should not include dust HTLCs.
-       fn commit_tx_fee_msat(&self, num_htlcs: usize) -> u64 {
+       fn commit_tx_fee_msat(feerate_per_kw: u32, num_htlcs: usize) -> u64 {
                // Note that we need to divide before multiplying to round properly,
                // since the lowest denomination of bitcoin on-chain is the satoshi.
-               (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * self.feerate_per_kw as u64 / 1000 * 1000
+               (COMMITMENT_TX_BASE_WEIGHT + num_htlcs as u64 * COMMITMENT_TX_WEIGHT_PER_HTLC) * feerate_per_kw as u64 / 1000 * 1000
        }
 
        // Get the fee cost in SATS of a commitment tx with a given number of HTLC outputs.
@@ -2192,12 +2235,12 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                let num_htlcs = included_htlcs + addl_htlcs;
-               let res = self.commit_tx_fee_msat(num_htlcs);
+               let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs);
                #[cfg(any(test, feature = "fuzztarget"))]
                {
                        let mut fee = res;
                        if fee_spike_buffer_htlc.is_some() {
-                               fee = self.commit_tx_fee_msat(num_htlcs - 1);
+                               fee = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs - 1);
                        }
                        let total_pending_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len()
                                + self.holding_cell_htlc_updates.len();
@@ -2270,12 +2313,12 @@ impl<Signer: Sign> Channel<Signer> {
                }
 
                let num_htlcs = included_htlcs + addl_htlcs;
-               let res = self.commit_tx_fee_msat(num_htlcs);
+               let res = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs);
                #[cfg(any(test, feature = "fuzztarget"))]
                {
                        let mut fee = res;
                        if fee_spike_buffer_htlc.is_some() {
-                               fee = self.commit_tx_fee_msat(num_htlcs - 1);
+                               fee = Self::commit_tx_fee_msat(self.feerate_per_kw, num_htlcs - 1);
                        }
                        let total_pending_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
                        let commitment_tx_info = CommitmentTxInfoCached {
@@ -2326,9 +2369,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
@@ -2389,9 +2431,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()));
                }
 
@@ -2406,7 +2446,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()));
@@ -2549,7 +2589,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())));
                        }
@@ -2581,7 +2621,7 @@ impl<Signer: Sign> Channel<Signer> {
                for (idx, (htlc, source)) in htlcs_cloned.drain(..).enumerate() {
                        if let Some(_) = htlc.transaction_output_index {
                                let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_stats.feerate_per_kw,
-                                       self.get_counterparty_selected_contest_delay().unwrap(), &htlc,
+                                       self.get_counterparty_selected_contest_delay().unwrap(), &htlc, self.opt_anchors(),
                                        &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
 
                                let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, self.opt_anchors(), &keys);
@@ -3990,7 +4030,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
                );
        }
 
@@ -4157,6 +4197,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;
        }
 
@@ -4390,8 +4431,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(),
@@ -4428,8 +4469,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(),
@@ -4712,7 +4753,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()));
                        }
@@ -4902,7 +4943,7 @@ impl<Signer: Sign> Channel<Signer> {
                                                && info.next_holder_htlc_id == self.next_holder_htlc_id
                                                && info.next_counterparty_htlc_id == self.next_counterparty_htlc_id
                                                && info.feerate == self.feerate_per_kw {
-                                                       let actual_fee = self.commit_tx_fee_msat(commitment_stats.num_nondust_htlcs);
+                                                       let actual_fee = Self::commit_tx_fee_msat(self.feerate_per_kw, commitment_stats.num_nondust_htlcs);
                                                        assert_eq!(actual_fee, info.fee);
                                                }
                                }
@@ -4927,7 +4968,7 @@ impl<Signer: Sign> Channel<Signer> {
 
                        for (ref htlc_sig, ref htlc) in htlc_signatures.iter().zip(htlcs) {
                                log_trace!(logger, "Signed remote HTLC tx {} with redeemscript {} with pubkey {} -> {} in channel {}",
-                                       encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.get_holder_selected_contest_delay(), htlc, &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
+                                       encode::serialize_hex(&chan_utils::build_htlc_transaction(&counterparty_commitment_txid, commitment_stats.feerate_per_kw, self.get_holder_selected_contest_delay(), htlc, self.opt_anchors(), &counterparty_keys.broadcaster_delayed_payment_key, &counterparty_keys.revocation_key)),
                                        encode::serialize_hex(&chan_utils::get_htlc_redeemscript(&htlc, self.opt_anchors(), &counterparty_keys)),
                                        log_bytes!(counterparty_keys.broadcaster_htlc_key.serialize()),
                                        log_bytes!(htlc_sig.serialize_compact()[..]), log_bytes!(self.channel_id()));
@@ -5358,6 +5399,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
@@ -5369,7 +5419,9 @@ 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),
@@ -5609,6 +5661,8 @@ 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());
@@ -5618,7 +5672,9 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<(&'a K, u32)> for Channel<Signer>
                        (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),
@@ -5697,7 +5753,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,
@@ -5881,6 +5939,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.
@@ -5894,7 +5953,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();
@@ -5932,13 +5991,13 @@ mod tests {
                // the dust limit check.
                let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered);
                let local_commit_tx_fee = node_a_chan.next_local_commit_tx_fee_msat(htlc_candidate, None);
-               let local_commit_fee_0_htlcs = node_a_chan.commit_tx_fee_msat(0);
+               let local_commit_fee_0_htlcs = Channel::<EnforcingSigner>::commit_tx_fee_msat(node_a_chan.feerate_per_kw, 0);
                assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs);
 
                // Finally, make sure that when Node A calculates the remote's commitment transaction fees, all
                // of the HTLCs are seen to be above the dust limit.
                node_a_chan.channel_transaction_parameters.is_outbound_from_holder = false;
-               let remote_commit_fee_3_htlcs = node_a_chan.commit_tx_fee_msat(3);
+               let remote_commit_fee_3_htlcs = Channel::<EnforcingSigner>::commit_tx_fee_msat(node_a_chan.feerate_per_kw, 3);
                let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered);
                let remote_commit_tx_fee = node_a_chan.next_remote_commit_tx_fee_msat(htlc_candidate, None);
                assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs);
@@ -5960,8 +6019,8 @@ mod tests {
                let config = UserConfig::default();
                let mut chan = Channel::<EnforcingSigner>::new_outbound(&&fee_est, &&keys_provider, node_id, &InitFeatures::known(), 10000000, 100000, 42, &config, 0).unwrap();
 
-               let commitment_tx_fee_0_htlcs = chan.commit_tx_fee_msat(0);
-               let commitment_tx_fee_1_htlc = chan.commit_tx_fee_msat(1);
+               let commitment_tx_fee_0_htlcs = Channel::<EnforcingSigner>::commit_tx_fee_msat(chan.feerate_per_kw, 0);
+               let commitment_tx_fee_1_htlc = Channel::<EnforcingSigner>::commit_tx_fee_msat(chan.feerate_per_kw, 1);
 
                // If HTLC_SUCCESS_TX_WEIGHT and HTLC_TIMEOUT_TX_WEIGHT were swapped: then this HTLC would be
                // counted as dust when it shouldn't be.
@@ -6012,7 +6071,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();
@@ -6223,10 +6282,10 @@ mod tests {
                                        let remote_signature = Signature::from_der(&hex::decode($counterparty_htlc_sig_hex).unwrap()[..]).unwrap();
 
                                        let ref htlc = htlcs[$htlc_idx];
+                                       let opt_anchors = false;
                                        let htlc_tx = chan_utils::build_htlc_transaction(&unsigned_tx.txid, chan.feerate_per_kw,
                                                chan.get_counterparty_selected_contest_delay().unwrap(),
-                                               &htlc, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
-                                       let opt_anchors = false;
+                                               &htlc, opt_anchors, &keys.broadcaster_delayed_payment_key, &keys.revocation_key);
                                        let htlc_redeemscript = chan_utils::get_htlc_redeemscript(&htlc, opt_anchors, &keys);
                                        let htlc_sighash = Message::from_slice(&bip143::SigHashCache::new(&htlc_tx).signature_hash(0, &htlc_redeemscript, htlc.amount_msat / 1000, SigHashType::All)[..]).unwrap();
                                        secp_ctx.verify(&htlc_sighash, &remote_signature, &keys.countersignatory_htlc_key).unwrap();