Limit inbound fee updates by dust exposure instead of our estimator

[rust-lightning] / lightning / src / ln / channel.rs

diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
index 9a5fd0422228e1e46f4bdd3d27c6cb1c246575a8..a0071d5436bc7d7c65498b9b1ef8a24afb0d0306 100644 (file)
--- a/lightning/src/ln/channel.rs
+++ b/lightning/src/ln/channel.rs
@@ -750,7 +750,12 @@ impl<Signer: Sign> Channel<Signer> {
                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)));
                }
-               let upper_limit = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64  * 2;
+               // We only bound the fee updates on the upper side to prevent completely absurd feerates,
+               // always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
+               // We generally don't care too much if they set the feerate to something very high, but it
+               // could result in the channel being useless due to everything being dust.
+               let upper_limit = cmp::max(250 * 25,
+                       fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
                if feerate_per_kw as u64 > upper_limit {
                        return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
                }
@@ -3111,8 +3116,27 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Peer sent update_fee when we needed a channel_reestablish".to_owned()));
                }
                Channel::<Signer>::check_remote_fee(fee_estimator, msg.feerate_per_kw)?;
+               let feerate_over_dust_buffer = msg.feerate_per_kw > self.get_dust_buffer_feerate();
+
                self.pending_update_fee = Some((msg.feerate_per_kw, FeeUpdateState::RemoteAnnounced));
                self.update_time_counter += 1;
+               // If the feerate has increased over the previous dust buffer (note that
+               // `get_dust_buffer_feerate` considers the `pending_update_fee` status), check that we
+               // won't be pushed over our dust exposure limit by the feerate increase.
+               if feerate_over_dust_buffer {
+                       let inbound_stats = self.get_inbound_pending_htlc_stats();
+                       let outbound_stats = self.get_outbound_pending_htlc_stats();
+                       let holder_tx_dust_exposure = inbound_stats.on_holder_tx_dust_exposure_msat + outbound_stats.on_holder_tx_dust_exposure_msat;
+                       let counterparty_tx_dust_exposure = inbound_stats.on_counterparty_tx_dust_exposure_msat + outbound_stats.on_counterparty_tx_dust_exposure_msat;
+                       if holder_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() {
+                               return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our own transactions (totaling {} msat)",
+                                       msg.feerate_per_kw, holder_tx_dust_exposure)));
+                       }
+                       if counterparty_tx_dust_exposure > self.get_max_dust_htlc_exposure_msat() {
+                               return Err(ChannelError::Close(format!("Peer sent update_fee with a feerate ({}) which may over-expose us to dust-in-flight on our counterparty's transactions (totaling {} msat)",
+                                       msg.feerate_per_kw, counterparty_tx_dust_exposure)));
+                       }
+               }
                Ok(())
        }
 
@@ -3684,7 +3708,11 @@ impl<Signer: Sign> Channel<Signer> {
                // whichever is higher. This ensures that we aren't suddenly exposed to significantly
                // more dust balance if the feerate increases when we have several HTLCs pending
                // which are near the dust limit.
-               cmp::max(2530, self.feerate_per_kw * 1250 / 1000)
+               let mut feerate_per_kw = self.feerate_per_kw;
+               if let Some((feerate, _)) = self.pending_update_fee {
+                       feerate_per_kw = cmp::max(feerate_per_kw, feerate);
+               }
+               cmp::max(2530, feerate_per_kw * 1250 / 1000)
        }
 
        pub fn get_cur_holder_commitment_transaction_number(&self) -> u64 {
@@ -4462,6 +4490,7 @@ impl<Signer: Sign> Channel<Signer> {
        }
        /// Only fails in case of bad keys
        fn send_commitment_no_status_check<L: Deref>(&mut self, logger: &L) -> Result<(msgs::CommitmentSigned, ChannelMonitorUpdate), ChannelError> where L::Target: Logger {
+               log_trace!(logger, "Updating HTLC state for a newly-sent commitment_signed...");
                // We can upgrade the status of some HTLCs that are waiting on a commitment, even if we
                // fail to generate this, we still are at least at a position where upgrading their status
                // is acceptable.
@@ -4470,6 +4499,7 @@ impl<Signer: Sign> Channel<Signer> {
                                Some(InboundHTLCState::AwaitingAnnouncedRemoteRevoke(forward_info.clone()))
                        } else { None };
                        if let Some(state) = new_state {
+                               log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce {} to AwaitingAnnouncedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
                                htlc.state = state;
                        }
                }
@@ -4477,12 +4507,14 @@ impl<Signer: Sign> Channel<Signer> {
                        if let Some(fail_reason) = if let &mut OutboundHTLCState::AwaitingRemoteRevokeToRemove(ref mut fail_reason) = &mut htlc.state {
                                Some(fail_reason.take())
                        } else { None } {
+                               log_trace!(logger, " ...promoting outbound AwaitingRemoteRevokeToRemove {} to AwaitingRemovedRemoteRevoke", log_bytes!(htlc.payment_hash.0));
                                htlc.state = OutboundHTLCState::AwaitingRemovedRemoteRevoke(fail_reason);
                        }
                }
                if let Some((feerate, update_state)) = self.pending_update_fee {
                        if update_state == FeeUpdateState::AwaitingRemoteRevokeToAnnounce {
                                debug_assert!(!self.is_outbound());
+                               log_trace!(logger, " ...promoting inbound AwaitingRemoteRevokeToAnnounce fee update {} to Committed", feerate);
                                self.feerate_per_kw = feerate;
                                self.pending_update_fee = None;
                        }