Clean up existing and add range-based closing_signed negotiation
authorMatt Corallo <git@bluematt.me>
Tue, 20 Jul 2021 03:19:01 +0000 (03:19 +0000)
committerMatt Corallo <git@bluematt.me>
Tue, 17 Aug 2021 02:16:01 +0000 (02:16 +0000)
This adds the new range-based closing_signed negotiation specified
in https://github.com/lightningnetwork/lightning-rfc/pull/847 as
well as cleans up the existing closing_signed negotiation to unify
the new codepaths and the old ones.

Note that because the new range-based closing_signed negotiation
allows the channel fundee to ultimately select the fee out of a
range specified by the funder, which we, of course, always select
the highest allowed amount from. Thus, we've added an extra round
of closing_signed in the common case as we will not simply accept
the first fee we see, always preferring to make the funder pay as
much as they're willing to.

lightning/src/ln/chanmon_update_fail_tests.rs
lightning/src/ln/channel.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/functional_test_utils.rs
lightning/src/ln/functional_tests.rs
lightning/src/util/config.rs

index ee9a492746b4a93436b9620c78f15b3f1a6b4694..70bffeeb201bee2215377db323b66de821660d8f 100644 (file)
@@ -2597,14 +2597,16 @@ fn test_temporary_error_during_shutdown() {
        *nodes[1].chain_monitor.update_ret.lock().unwrap() = None;
        let (outpoint, latest_update) = nodes[1].chain_monitor.latest_monitor_update_id.lock().unwrap().get(&channel_id).unwrap().clone();
        nodes[1].node.channel_monitor_updated(&outpoint, latest_update);
-       let (_, closing_signed_b) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &closing_signed_b.unwrap());
-       let txn_b = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 
-       let (_, none_a) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
-       assert!(none_a.is_none());
+       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id()));
+       let (_, closing_signed_a) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
        let txn_a = nodes[0].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
 
+       nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &closing_signed_a.unwrap());
+       let (_, none_b) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
+       assert!(none_b.is_none());
+       let txn_b = nodes[1].tx_broadcaster.txn_broadcasted.lock().unwrap().split_off(0);
+
        assert_eq!(txn_a, txn_b);
        assert_eq!(txn_a.len(), 1);
        check_spends!(txn_a[0], funding_tx);
index 373bfef13435359dbb7fe53e997a245e618d1533..0ada482f3679563b98de11a861ee9a9d9caf6080 100644 (file)
@@ -453,7 +453,9 @@ pub(super) struct Channel<Signer: Sign> {
        /// Max to_local and to_remote outputs in a remote-generated commitment transaction
        counterparty_max_commitment_tx_output: Mutex<(u64, u64)>,
 
-       last_sent_closing_fee: Option<(u32, u64, Signature)>, // (feerate, fee, holder_sig)
+       last_sent_closing_fee: Option<(u64, Signature)>, // (fee, holder_sig)
+       closing_fee_limits: Option<(u64, u64)>,
+       target_closing_feerate_sats_per_kw: Option<u32>,
 
        /// If our counterparty sent us a closing_signed while we were waiting for a `ChannelMonitor`
        /// update, we need to delay processing it until later. We do that here by simply storing the
@@ -701,6 +703,8 @@ impl<Signer: Sign> Channel<Signer> {
 
                        last_sent_closing_fee: None,
                        pending_counterparty_closing_signed: None,
+                       closing_fee_limits: None,
+                       target_closing_feerate_sats_per_kw: None,
 
                        funding_tx_confirmed_in: None,
                        funding_tx_confirmation_height: 0,
@@ -961,6 +965,8 @@ impl<Signer: Sign> Channel<Signer> {
 
                        last_sent_closing_fee: None,
                        pending_counterparty_closing_signed: None,
+                       closing_fee_limits: None,
+                       target_closing_feerate_sats_per_kw: None,
 
                        funding_tx_confirmed_in: None,
                        funding_tx_confirmation_height: 0,
@@ -2989,6 +2995,7 @@ impl<Signer: Sign> Channel<Signer> {
                // will be retransmitted.
                self.last_sent_closing_fee = None;
                self.pending_counterparty_closing_signed = None;
+               self.closing_fee_limits = None;
 
                let mut inbound_drop_count = 0;
                self.pending_inbound_htlcs.retain(|htlc| {
@@ -3356,6 +3363,56 @@ impl<Signer: Sign> Channel<Signer> {
                }
        }
 
+       /// Calculates and returns our minimum and maximum closing transaction fee amounts, in whole
+       /// satoshis. The amounts remain consistent unless a peer disconnects/reconnects or we restart,
+       /// at which point they will be recalculated.
+       fn calculate_closing_fee_limits<F: Deref>(&mut self, fee_estimator: &F) -> (u64, u64)
+               where F::Target: FeeEstimator
+       {
+               if let Some((min, max)) = self.closing_fee_limits { return (min, max); }
+
+               // Propose a range from our current Background feerate to our Normal feerate plus our
+               // force_close_avoidance_max_fee_satoshis.
+               // If we fail to come to consensus, we'll have to force-close.
+               let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
+               let normal_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
+               let mut proposed_max_feerate = if self.is_outbound() { normal_feerate } else { u32::max_value() };
+
+               // The spec requires that (when the channel does not have anchors) we only send absolute
+               // channel fees no greater than the absolute channel fee on the current commitment
+               // transaction. It's unclear *which* commitment transaction this refers to, and there isn't
+               // very good reason to apply such a limit in any case. We don't bother doing so, risking
+               // some force-closure by old nodes, but we wanted to close the channel anyway.
+
+               if let Some(target_feerate) = self.target_closing_feerate_sats_per_kw {
+                       let min_feerate = if self.is_outbound() { target_feerate } else { cmp::min(self.feerate_per_kw, target_feerate) };
+                       proposed_feerate = cmp::max(proposed_feerate, min_feerate);
+                       proposed_max_feerate = cmp::max(proposed_max_feerate, min_feerate);
+               }
+
+               // Note that technically we could end up with a lower minimum fee if one sides' balance is
+               // below our dust limit, causing the output to disappear. We don't bother handling this
+               // case, however, as this should only happen if a channel is closed before any (material)
+               // payments have been made on it. This may cause slight fee overpayment and/or failure to
+               // come to consensus with our counterparty on appropriate fees, however it should be a
+               // relatively rare case. We can revisit this later, though note that in order to determine
+               // if the funders' output is dust we have to know the absolute fee we're going to use.
+               let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
+               let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000;
+               let proposed_max_total_fee_satoshis = if self.is_outbound() {
+                               // We always add force_close_avoidance_max_fee_satoshis to our normal
+                               // feerate-calculated fee, but allow the max to be overridden if we're using a
+                               // target feerate-calculated fee.
+                               cmp::max(normal_feerate as u64 * tx_weight / 1000 + self.config.force_close_avoidance_max_fee_satoshis,
+                                       proposed_max_feerate as u64 * tx_weight / 1000)
+                       } else {
+                               u64::max_value()
+                       };
+
+               self.closing_fee_limits = Some((proposed_total_fee_satoshis, proposed_max_total_fee_satoshis));
+               self.closing_fee_limits.clone().unwrap()
+       }
+
        pub fn maybe_propose_closing_signed<F: Deref, L: Deref>(&mut self, fee_estimator: &F, logger: &L)
                -> Result<(Option<msgs::ClosingSigned>, Option<Transaction>), ChannelError>
                where F::Target: FeeEstimator, L::Target: Logger
@@ -3376,27 +3433,26 @@ impl<Signer: Sign> Channel<Signer> {
                        return Ok((None, None));
                }
 
-               let mut proposed_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               if self.feerate_per_kw > proposed_feerate {
-                       proposed_feerate = self.feerate_per_kw;
-               }
+               let (our_min_fee, our_max_fee) = self.calculate_closing_fee_limits(fee_estimator);
+
                assert!(self.shutdown_scriptpubkey.is_some());
-               let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
-               let proposed_total_fee_satoshis = proposed_feerate as u64 * tx_weight / 1000;
-               log_trace!(logger, "Proposing initial closing signed for our counterparty with a feerate of {} sat/kWeight (= {} sats)",
-                       proposed_feerate, proposed_total_fee_satoshis);
+               let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(our_min_fee, false);
+               log_trace!(logger, "Proposing initial closing_signed for our counterparty with a fee range of {}-{} sat (with initial proposal {} sats)",
+                       our_min_fee, our_max_fee, total_fee_satoshis);
 
-               let (closing_tx, total_fee_satoshis) = self.build_closing_transaction(proposed_total_fee_satoshis, false);
                let sig = self.holder_signer
                        .sign_closing_transaction(&closing_tx, &self.secp_ctx)
                        .map_err(|()| ChannelError::Close("Failed to get signature for closing transaction.".to_owned()))?;
 
-               self.last_sent_closing_fee = Some((proposed_feerate, total_fee_satoshis, sig.clone()));
+               self.last_sent_closing_fee = Some((total_fee_satoshis, sig.clone()));
                Ok((Some(msgs::ClosingSigned {
                        channel_id: self.channel_id,
                        fee_satoshis: total_fee_satoshis,
                        signature: sig,
-                       fee_range: None,
+                       fee_range: Some(msgs::ClosingSignedFeeRange {
+                               min_fee_satoshis: our_min_fee,
+                               max_fee_satoshis: our_max_fee,
+                       }),
                }), None))
        }
 
@@ -3532,6 +3588,10 @@ impl<Signer: Sign> Channel<Signer> {
                        return Err(ChannelError::Close("Remote tried to send us a closing tx with > 21 million BTC fee".to_owned()));
                }
 
+               if self.is_outbound() && self.last_sent_closing_fee.is_none() {
+                       return Err(ChannelError::Close("Remote tried to send a closing_signed when we were supposed to propose the first one".to_owned()));
+               }
+
                if self.channel_state & ChannelState::MonitorUpdateFailed as u32 != 0 {
                        self.pending_counterparty_closing_signed = Some(msg.clone());
                        return Ok((None, None));
@@ -3555,78 +3615,104 @@ impl<Signer: Sign> Channel<Signer> {
                        },
                };
 
-               let closing_tx_max_weight = self.get_closing_transaction_weight(
-                       if let Some(oup) = closing_tx.output.get(0) { Some(&oup.script_pubkey) } else { None },
-                       if let Some(oup) = closing_tx.output.get(1) { Some(&oup.script_pubkey) } else { None });
-               if let Some((_, last_fee, sig)) = self.last_sent_closing_fee {
+               assert!(self.shutdown_scriptpubkey.is_some());
+               if let Some((last_fee, sig)) = self.last_sent_closing_fee {
                        if last_fee == msg.fee_satoshis {
                                self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
-                               assert!(closing_tx.get_weight() as u64 <= closing_tx_max_weight);
-                               debug_assert!(closing_tx.get_weight() as u64 >= closing_tx_max_weight - 2);
                                self.channel_state = ChannelState::ShutdownComplete as u32;
                                self.update_time_counter += 1;
                                return Ok((None, Some(closing_tx)));
                        }
                }
 
-               macro_rules! propose_new_feerate {
-                       ($new_feerate: expr) => {
-                               assert!(self.shutdown_scriptpubkey.is_some());
-                               let tx_weight = self.get_closing_transaction_weight(Some(&self.get_closing_scriptpubkey()), Some(self.counterparty_shutdown_scriptpubkey.as_ref().unwrap()));
-                               let (closing_tx, used_total_fee) = self.build_closing_transaction($new_feerate as u64 * tx_weight / 1000, false);
+               let (our_min_fee, our_max_fee) = self.calculate_closing_fee_limits(fee_estimator);
+
+               macro_rules! propose_fee {
+                       ($new_fee: expr) => {
+                               let (mut tx, used_fee) = if $new_fee == msg.fee_satoshis {
+                                       (closing_tx, $new_fee)
+                               } else {
+                                       self.build_closing_transaction($new_fee, false)
+                               };
+
                                let sig = self.holder_signer
-                                       .sign_closing_transaction(&closing_tx, &self.secp_ctx)
+                                       .sign_closing_transaction(&tx, &self.secp_ctx)
                                        .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
-                               assert!(closing_tx.get_weight() as u64 <= tx_weight);
-                               self.last_sent_closing_fee = Some(($new_feerate, used_total_fee, sig.clone()));
+
+                               let signed_tx = if $new_fee == msg.fee_satoshis {
+                                       self.channel_state = ChannelState::ShutdownComplete as u32;
+                                       self.update_time_counter += 1;
+                                       self.build_signed_closing_transaction(&mut tx, &msg.signature, &sig);
+                                       Some(tx)
+                               } else { None };
+
+                               self.last_sent_closing_fee = Some((used_fee, sig.clone()));
                                return Ok((Some(msgs::ClosingSigned {
                                        channel_id: self.channel_id,
-                                       fee_satoshis: used_total_fee,
+                                       fee_satoshis: used_fee,
                                        signature: sig,
-                                       fee_range: None,
-                               }), None))
+                                       fee_range: Some(msgs::ClosingSignedFeeRange {
+                                               min_fee_satoshis: our_min_fee,
+                                               max_fee_satoshis: our_max_fee,
+                                       }),
+                               }), signed_tx))
                        }
                }
 
-               let mut min_feerate = 253;
-               if self.is_outbound() {
-                       let max_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
-                       if (msg.fee_satoshis as u64) > max_feerate as u64 * closing_tx_max_weight / 1000 {
-                               if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
-                                       if max_feerate <= last_feerate {
-                                               return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something higher ({}) than our Normal feerate ({})", last_feerate, max_feerate)));
-                                       }
+               if let Some(msgs::ClosingSignedFeeRange { min_fee_satoshis, max_fee_satoshis }) = msg.fee_range {
+                       if msg.fee_satoshis < min_fee_satoshis || msg.fee_satoshis > max_fee_satoshis {
+                               return Err(ChannelError::Close(format!("Peer sent a bogus closing_signed - suggested fee of {} sat was not in their desired range of {} sat - {} sat", msg.fee_satoshis, min_fee_satoshis, max_fee_satoshis)));
+                       }
+                       if max_fee_satoshis < our_min_fee {
+                               return Err(ChannelError::Warn(format!("Unable to come to consensus about closing feerate, remote's max fee ({} sat) was smaller than our min fee ({} sat)", max_fee_satoshis, our_min_fee)));
+                       }
+                       if min_fee_satoshis > our_max_fee {
+                               return Err(ChannelError::Warn(format!("Unable to come to consensus about closing feerate, remote's min fee ({} sat) was greater than our max fee ({} sat)", min_fee_satoshis, our_max_fee)));
+                       }
+
+                       if !self.is_outbound() {
+                               // They have to pay, so pick the highest fee in the overlapping range.
+                               debug_assert_eq!(our_max_fee, u64::max_value()); // We should never set an upper bound
+                               propose_fee!(cmp::min(max_fee_satoshis, our_max_fee));
+                       } else {
+                               if msg.fee_satoshis < our_min_fee || msg.fee_satoshis > our_max_fee {
+                                       return Err(ChannelError::Close(format!("Peer sent a bogus closing_signed - suggested fee of {} sat was not in our desired range of {} sat - {} sat after we informed them of our range.",
+                                               msg.fee_satoshis, our_min_fee, our_max_fee)));
                                }
-                               propose_new_feerate!(max_feerate);
+                               // The proposed fee is in our acceptable range, accept it and broadcast!
+                               propose_fee!(msg.fee_satoshis);
                        }
                } else {
-                       min_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background);
-               }
-               if (msg.fee_satoshis as u64) < min_feerate as u64 * closing_tx_max_weight / 1000 {
-                       if let Some((last_feerate, _, _)) = self.last_sent_closing_fee {
-                               if min_feerate >= last_feerate {
-                                       return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wanted something lower ({}) than our Background feerate ({}).", last_feerate, min_feerate)));
+                       // Old fee style negotiation. We don't bother to enforce whether they are complying
+                       // with the "making progress" requirements, we just comply and hope for the best.
+                       if let Some((last_fee, _)) = self.last_sent_closing_fee {
+                               if msg.fee_satoshis > last_fee {
+                                       if msg.fee_satoshis < our_max_fee {
+                                               propose_fee!(msg.fee_satoshis);
+                                       } else if last_fee < our_max_fee {
+                                               propose_fee!(our_max_fee);
+                                       } else {
+                                               return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wants something ({} sat) higher than our max fee ({} sat)", msg.fee_satoshis, our_max_fee)));
+                                       }
+                               } else {
+                                       if msg.fee_satoshis > our_min_fee {
+                                               propose_fee!(msg.fee_satoshis);
+                                       } else if last_fee > our_min_fee {
+                                               propose_fee!(our_min_fee);
+                                       } else {
+                                               return Err(ChannelError::Close(format!("Unable to come to consensus about closing feerate, remote wants something ({} sat) lower than our min fee ({} sat)", msg.fee_satoshis, our_min_fee)));
+                                       }
+                               }
+                       } else {
+                               if msg.fee_satoshis < our_min_fee {
+                                       propose_fee!(our_min_fee);
+                               } else if msg.fee_satoshis > our_max_fee {
+                                       propose_fee!(our_max_fee);
+                               } else {
+                                       propose_fee!(msg.fee_satoshis);
                                }
                        }
-                       propose_new_feerate!(min_feerate);
                }
-
-               let sig = self.holder_signer
-                       .sign_closing_transaction(&closing_tx, &self.secp_ctx)
-                       .map_err(|_| ChannelError::Close("External signer refused to sign closing transaction".to_owned()))?;
-               self.build_signed_closing_transaction(&mut closing_tx, &msg.signature, &sig);
-               assert!(closing_tx.get_weight() as u64 <= closing_tx_max_weight);
-               debug_assert!(closing_tx.get_weight() as u64 >= closing_tx_max_weight - 2);
-
-               self.channel_state = ChannelState::ShutdownComplete as u32;
-               self.update_time_counter += 1;
-
-               Ok((Some(msgs::ClosingSigned {
-                       channel_id: self.channel_id,
-                       fee_satoshis: msg.fee_satoshis,
-                       signature: sig,
-                       fee_range: None,
-               }), Some(closing_tx)))
        }
 
        // Public utilities:
@@ -4669,7 +4755,8 @@ impl<Signer: Sign> Channel<Signer> {
 
        /// Begins the shutdown process, getting a message for the remote peer and returning all
        /// holding cell HTLCs for payment failure.
-       pub fn get_shutdown<K: Deref>(&mut self, keys_provider: &K, their_features: &InitFeatures) -> Result<(msgs::Shutdown, Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>), APIError>
+       pub fn get_shutdown<K: Deref>(&mut self, keys_provider: &K, their_features: &InitFeatures, target_feerate_sats_per_kw: Option<u32>)
+       -> Result<(msgs::Shutdown, Option<ChannelMonitorUpdate>, Vec<(HTLCSource, PaymentHash)>), APIError>
        where K::Target: KeysInterface<Signer = Signer> {
                for htlc in self.pending_outbound_htlcs.iter() {
                        if let OutboundHTLCState::LocalAnnounced(_) = htlc.state {
@@ -4702,6 +4789,7 @@ impl<Signer: Sign> Channel<Signer> {
                };
 
                // From here on out, we may not fail!
+               self.target_closing_feerate_sats_per_kw = target_feerate_sats_per_kw;
                if self.channel_state < ChannelState::FundingSent as u32 {
                        self.channel_state = ChannelState::ShutdownComplete as u32;
                } else {
@@ -5054,6 +5142,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                        (3, self.counterparty_selected_channel_reserve_satoshis, option),
                        (5, self.config, required),
                        (7, self.shutdown_scriptpubkey, option),
+                       (9, self.target_closing_feerate_sats_per_kw, option),
                });
 
                Ok(())
@@ -5286,12 +5375,14 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                };
 
                let mut announcement_sigs = None;
+               let mut target_closing_feerate_sats_per_kw = None;
                read_tlv_fields!(reader, {
                        (0, announcement_sigs, option),
                        (1, minimum_depth, option),
                        (3, counterparty_selected_channel_reserve_satoshis, option),
                        (5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
                        (7, shutdown_scriptpubkey, option),
+                       (9, target_closing_feerate_sats_per_kw, option),
                });
 
                let mut secp_ctx = Secp256k1::new();
@@ -5342,6 +5433,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
                        last_sent_closing_fee: None,
                        pending_counterparty_closing_signed: None,
+                       closing_fee_limits: None,
+                       target_closing_feerate_sats_per_kw,
 
                        funding_tx_confirmed_in,
                        funding_tx_confirmation_height,
index f85fe21af02c78d14e4a0ea91692330fe584aa62..c0e32bbdd47dd36c34c75050e222b850a2e1496e 100644 (file)
@@ -1284,12 +1284,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                self.list_channels_with_filter(|&(_, ref channel)| channel.is_live())
        }
 
-       /// Begins the process of closing a channel. After this call (plus some timeout), no new HTLCs
-       /// will be accepted on the given channel, and after additional timeout/the closing of all
-       /// pending HTLCs, the channel will be closed on chain.
-       ///
-       /// May generate a SendShutdown message event on success, which should be relayed.
-       pub fn close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
+       fn close_channel_internal(&self, channel_id: &[u8; 32], target_feerate_sats_per_1000_weight: Option<u32>) -> Result<(), APIError> {
                let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(&self.total_consistency_lock, &self.persistence_notifier);
 
                let counterparty_node_id;
@@ -1305,7 +1300,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                                Some(peer_state) => {
                                                        let peer_state = peer_state.lock().unwrap();
                                                        let their_features = &peer_state.latest_features;
-                                                       chan_entry.get_mut().get_shutdown(&self.keys_manager, their_features)?
+                                                       chan_entry.get_mut().get_shutdown(&self.keys_manager, their_features, target_feerate_sats_per_1000_weight)?
                                                },
                                                None => return Err(APIError::ChannelUnavailable { err: format!("Not connected to node: {}", counterparty_node_id) }),
                                        };
@@ -1350,6 +1345,50 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                Ok(())
        }
 
+       /// Begins the process of closing a channel. After this call (plus some timeout), no new HTLCs
+       /// will be accepted on the given channel, and after additional timeout/the closing of all
+       /// pending HTLCs, the channel will be closed on chain.
+       ///
+       ///  * If we are the channel initiator, we will pay between our [`Background`] and
+       ///    [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`Normal`] fee
+       ///    estimate.
+       ///  * If our counterparty is the channel initiator, we will require a channel closing
+       ///    transaction feerate of at least our [`Background`] feerate or the feerate which
+       ///    would appear on a force-closure transaction, whichever is lower. We will allow our
+       ///    counterparty to pay as much fee as they'd like, however.
+       ///
+       /// May generate a SendShutdown message event on success, which should be relayed.
+       ///
+       /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis
+       /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
+       /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
+       pub fn close_channel(&self, channel_id: &[u8; 32]) -> Result<(), APIError> {
+               self.close_channel_internal(channel_id, None)
+       }
+
+       /// Begins the process of closing a channel. After this call (plus some timeout), no new HTLCs
+       /// will be accepted on the given channel, and after additional timeout/the closing of all
+       /// pending HTLCs, the channel will be closed on chain.
+       ///
+       /// `target_feerate_sat_per_1000_weight` has different meanings depending on if we initiated
+       /// the channel being closed or not:
+       ///  * If we are the channel initiator, we will pay at least this feerate on the closing
+       ///    transaction. The upper-bound is set by
+       ///    [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`Normal`] fee
+       ///    estimate (or `target_feerate_sat_per_1000_weight`, if it is greater).
+       ///  * If our counterparty is the channel initiator, we will refuse to accept a channel closure
+       ///    transaction feerate below `target_feerate_sat_per_1000_weight` (or the feerate which
+       ///    will appear on a force-closure transaction, whichever is lower).
+       ///
+       /// May generate a SendShutdown message event on success, which should be relayed.
+       ///
+       /// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis
+       /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
+       /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
+       pub fn close_channel_with_target_feerate(&self, channel_id: &[u8; 32], target_feerate_sats_per_1000_weight: u32) -> Result<(), APIError> {
+               self.close_channel_internal(channel_id, Some(target_feerate_sats_per_1000_weight))
+       }
+
        #[inline]
        fn finish_force_close_channel(&self, shutdown_res: ShutdownResult) {
                let (monitor_update_option, mut failed_htlcs) = shutdown_res;
@@ -3367,7 +3406,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
                                        }
 
                                        if !chan_entry.get().received_shutdown() {
-                                               log_info!(self.logger, "Received a shutdown message from our couterparty for channel {}{}.",
+                                               log_info!(self.logger, "Received a shutdown message from our counterparty for channel {}{}.",
                                                        log_bytes!(msg.channel_id),
                                                        if chan_entry.get().sent_shutdown() { " after we initiated shutdown" } else { "" });
                                        }
index 6c08065bc2f13cfd487bd005d96de13637575558..565f1305963a82c563515d5d645910ba5e7fac9a 100644 (file)
@@ -755,7 +755,7 @@ macro_rules! check_closed_broadcast {
 
 pub fn close_channel<'a, 'b, 'c>(outbound_node: &Node<'a, 'b, 'c>, inbound_node: &Node<'a, 'b, 'c>, channel_id: &[u8; 32], funding_tx: Transaction, close_inbound_first: bool) -> (msgs::ChannelUpdate, msgs::ChannelUpdate, Transaction) {
        let (node_a, broadcaster_a, struct_a) = if close_inbound_first { (&inbound_node.node, &inbound_node.tx_broadcaster, inbound_node) } else { (&outbound_node.node, &outbound_node.tx_broadcaster, outbound_node) };
-       let (node_b, broadcaster_b) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster) } else { (&inbound_node.node, &inbound_node.tx_broadcaster) };
+       let (node_b, broadcaster_b, struct_b) = if close_inbound_first { (&outbound_node.node, &outbound_node.tx_broadcaster, outbound_node) } else { (&inbound_node.node, &inbound_node.tx_broadcaster, inbound_node) };
        let (tx_a, tx_b);
 
        node_a.close_channel(channel_id).unwrap();
@@ -788,29 +788,33 @@ pub fn close_channel<'a, 'b, 'c>(outbound_node: &Node<'a, 'b, 'c>, inbound_node:
        let (as_update, bs_update) = if close_inbound_first {
                assert!(node_a.get_and_clear_pending_msg_events().is_empty());
                node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap());
-               assert_eq!(broadcaster_a.txn_broadcasted.lock().unwrap().len(), 1);
-               tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
-               let (as_update, closing_signed_a) = get_closing_signed_broadcast!(node_a, node_b.get_our_node_id());
 
-               node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a.unwrap());
-               let (bs_update, none_b) = get_closing_signed_broadcast!(node_b, node_a.get_our_node_id());
-               assert!(none_b.is_none());
+               node_b.handle_closing_signed(&node_a.get_our_node_id(), &get_event_msg!(struct_a, MessageSendEvent::SendClosingSigned, node_b.get_our_node_id()));
                assert_eq!(broadcaster_b.txn_broadcasted.lock().unwrap().len(), 1);
                tx_b = broadcaster_b.txn_broadcasted.lock().unwrap().remove(0);
+               let (bs_update, closing_signed_b) = get_closing_signed_broadcast!(node_b, node_a.get_our_node_id());
+
+               node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap());
+               let (as_update, none_a) = get_closing_signed_broadcast!(node_a, node_b.get_our_node_id());
+               assert!(none_a.is_none());
+               assert_eq!(broadcaster_a.txn_broadcasted.lock().unwrap().len(), 1);
+               tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
                (as_update, bs_update)
        } else {
                let closing_signed_a = get_event_msg!(struct_a, MessageSendEvent::SendClosingSigned, node_b.get_our_node_id());
 
                node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a);
-               assert_eq!(broadcaster_b.txn_broadcasted.lock().unwrap().len(), 1);
-               tx_b = broadcaster_b.txn_broadcasted.lock().unwrap().remove(0);
-               let (bs_update, closing_signed_b) = get_closing_signed_broadcast!(node_b, node_a.get_our_node_id());
+               node_a.handle_closing_signed(&node_b.get_our_node_id(), &get_event_msg!(struct_b, MessageSendEvent::SendClosingSigned, node_a.get_our_node_id()));
 
-               node_a.handle_closing_signed(&node_b.get_our_node_id(), &closing_signed_b.unwrap());
-               let (as_update, none_a) = get_closing_signed_broadcast!(node_a, node_b.get_our_node_id());
-               assert!(none_a.is_none());
                assert_eq!(broadcaster_a.txn_broadcasted.lock().unwrap().len(), 1);
                tx_a = broadcaster_a.txn_broadcasted.lock().unwrap().remove(0);
+               let (as_update, closing_signed_a) = get_closing_signed_broadcast!(node_a, node_b.get_our_node_id());
+
+               node_b.handle_closing_signed(&node_a.get_our_node_id(), &closing_signed_a.unwrap());
+               let (bs_update, none_b) = get_closing_signed_broadcast!(node_b, node_a.get_our_node_id());
+               assert!(none_b.is_none());
+               assert_eq!(broadcaster_b.txn_broadcasted.lock().unwrap().len(), 1);
+               tx_b = broadcaster_b.txn_broadcasted.lock().unwrap().remove(0);
                (as_update, bs_update)
        };
        assert_eq!(tx_a, tx_b);
index ea1c8993067b7590d6f7e327102ff0d6f85cc9ab..59ccfbe96752aef3052153b7054432234e641bd1 100644 (file)
@@ -876,10 +876,12 @@ fn pre_funding_lock_shutdown_test() {
 
        let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
        nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed);
-       let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed.unwrap());
-       let (_, node_0_none) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
-       assert!(node_0_none.is_none());
+       let node_1_closing_signed = get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed);
+       let (_, node_0_2nd_closing_signed) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_2nd_closing_signed.unwrap());
+       let (_, node_1_none) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
+       assert!(node_1_none.is_none());
 
        assert!(nodes[0].node.list_channels().is_empty());
        assert!(nodes[1].node.list_channels().is_empty());
@@ -949,10 +951,12 @@ fn updates_shutdown_wait() {
 
        let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
        nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed);
-       let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed.unwrap());
-       let (_, node_0_none) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
-       assert!(node_0_none.is_none());
+       let node_1_closing_signed = get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed);
+       let (_, node_0_2nd_closing_signed) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_2nd_closing_signed.unwrap());
+       let (_, node_1_none) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
+       assert!(node_1_none.is_none());
 
        assert!(nodes[0].node.list_channels().is_empty());
 
@@ -1027,10 +1031,12 @@ fn htlc_fail_async_shutdown() {
 
        assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
        nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed);
-       let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed.unwrap());
-       let (_, node_0_none) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
-       assert!(node_0_none.is_none());
+       let node_1_closing_signed = get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id());
+       nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed);
+       let (_, node_0_2nd_closing_signed) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
+       nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_2nd_closing_signed.unwrap());
+       let (_, node_1_none) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
+       assert!(node_1_none.is_none());
 
        assert!(nodes[0].node.list_channels().is_empty());
 
@@ -1125,19 +1131,21 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
        let node_0_closing_signed = get_event_msg!(nodes[0], MessageSendEvent::SendClosingSigned, nodes[1].node.get_our_node_id());
        if recv_count > 0 {
                nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_closing_signed);
-               let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-               assert!(node_1_closing_signed.is_some());
+               let node_1_closing_signed = get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id());
+               nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed);
+               let (_, node_0_2nd_closing_signed) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
+               assert!(node_0_2nd_closing_signed.is_some());
        }
 
        nodes[0].node.peer_disconnected(&nodes[1].node.get_our_node_id(), false);
        nodes[1].node.peer_disconnected(&nodes[0].node.get_our_node_id(), false);
 
-       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
-       let node_0_2nd_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
        nodes[1].node.peer_connected(&nodes[0].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
+       let node_1_2nd_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+       nodes[0].node.peer_connected(&nodes[1].node.get_our_node_id(), &msgs::Init { features: InitFeatures::empty() });
        if recv_count == 0 {
                // If all closing_signeds weren't delivered we can just resume where we left off...
-               let node_1_2nd_reestablish = get_event_msg!(nodes[1], MessageSendEvent::SendChannelReestablish, nodes[0].node.get_our_node_id());
+               let node_0_2nd_reestablish = get_event_msg!(nodes[0], MessageSendEvent::SendChannelReestablish, nodes[1].node.get_our_node_id());
 
                nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &node_1_2nd_reestablish);
                let node_0_msgs = nodes[0].node.get_and_clear_pending_msg_events();
@@ -1169,10 +1177,12 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
                nodes[0].node.handle_shutdown(&nodes[1].node.get_our_node_id(), &InitFeatures::known(), &node_1_3rd_shutdown);
 
                nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_2nd_closing_signed);
-               let (_, node_1_closing_signed) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
-               nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed.unwrap());
-               let (_, node_0_none) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
-               assert!(node_0_none.is_none());
+               let node_1_closing_signed = get_event_msg!(nodes[1], MessageSendEvent::SendClosingSigned, nodes[0].node.get_our_node_id());
+               nodes[0].node.handle_closing_signed(&nodes[1].node.get_our_node_id(), &node_1_closing_signed);
+               let (_, node_0_2nd_closing_signed) = get_closing_signed_broadcast!(nodes[0].node, nodes[1].node.get_our_node_id());
+               nodes[1].node.handle_closing_signed(&nodes[0].node.get_our_node_id(), &node_0_2nd_closing_signed.unwrap());
+               let (_, node_1_none) = get_closing_signed_broadcast!(nodes[1].node, nodes[0].node.get_our_node_id());
+               assert!(node_1_none.is_none());
        } else {
                // If one node, however, received + responded with an identical closing_signed we end
                // up erroring and node[0] will try to broadcast its own latest commitment transaction.
@@ -1181,15 +1191,15 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
                // closing_signed and avoiding broadcasting local commitment txn for some timeout to
                // give our counterparty enough time to (potentially) broadcast a cooperative closing
                // transaction.
-               assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+               assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
 
-               nodes[1].node.handle_channel_reestablish(&nodes[0].node.get_our_node_id(), &node_0_2nd_reestablish);
-               let msg_events = nodes[1].node.get_and_clear_pending_msg_events();
+               nodes[0].node.handle_channel_reestablish(&nodes[1].node.get_our_node_id(), &node_1_2nd_reestablish);
+               let msg_events = nodes[0].node.get_and_clear_pending_msg_events();
                assert_eq!(msg_events.len(), 1);
                if let MessageSendEvent::HandleError { ref action, .. } = msg_events[0] {
                        match action {
                                &ErrorAction::SendErrorMessage { ref msg } => {
-                                       nodes[0].node.handle_error(&nodes[1].node.get_our_node_id(), &msg);
+                                       nodes[1].node.handle_error(&nodes[0].node.get_our_node_id(), &msg);
                                        assert_eq!(msg.channel_id, chan_1.2);
                                },
                                _ => panic!("Unexpected event!"),
@@ -1197,10 +1207,10 @@ fn do_test_shutdown_rebroadcast(recv_count: u8) {
                } else { panic!("Needed SendErrorMessage close"); }
 
                // get_closing_signed_broadcast usually eats the BroadcastChannelUpdate for us and
-               // checks it, but in this case nodes[0] didn't ever get a chance to receive a
+               // checks it, but in this case nodes[1] didn't ever get a chance to receive a
                // closing_signed so we do it ourselves
-               check_closed_broadcast!(nodes[0], false);
-               check_added_monitors!(nodes[0], 1);
+               check_closed_broadcast!(nodes[1], false);
+               check_added_monitors!(nodes[1], 1);
        }
 
        assert!(nodes[0].node.list_channels().is_empty());
index 3348b7b254e6f001290904add735105da0f429fe..fb4f6fce737fb1eb47c8cb95abcc8ae69d5ef3b4 100644 (file)
@@ -223,6 +223,29 @@ pub struct ChannelConfig {
        ///
        /// Default value: 5_000_000 msat.
        pub max_dust_htlc_exposure_msat: u64,
+       /// The additional fee we're willing to pay to avoid waiting for the counterparty's
+       /// `to_self_delay` to reclaim funds.
+       ///
+       /// When we close a channel cooperatively with our counterparty, we negotiate a fee for the
+       /// closing transaction which both sides find acceptable, ultimately paid by the channel
+       /// funder/initiator.
+       ///
+       /// When we are the funder, because we have to pay the channel closing fee, we bound the
+       /// acceptable fee by our [`Background`] and [`Normal`] fees, with the upper bound increased by
+       /// this value. Because the on-chain fee we'd pay to force-close the channel is kept near our
+       /// [`Normal`] feerate during normal operation, this value represents the additional fee we're
+       /// willing to pay in order to avoid waiting for our counterparty's to_self_delay to reclaim our
+       /// funds.
+       ///
+       /// When we are not the funder, we require the closing transaction fee pay at least our
+       /// [`Background`] fee estimate, but allow our counterparty to pay as much fee as they like.
+       /// Thus, this value is ignored when we are not the funder.
+       ///
+       /// Default value: 1000 satoshis.
+       ///
+       /// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
+       /// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
+       pub force_close_avoidance_max_fee_satoshis: u64,
 }
 
 impl Default for ChannelConfig {
@@ -235,6 +258,7 @@ impl Default for ChannelConfig {
                        announced_channel: false,
                        commit_upfront_shutdown_pubkey: true,
                        max_dust_htlc_exposure_msat: 5_000_000,
+                       force_close_avoidance_max_fee_satoshis: 1000,
                }
        }
 }
@@ -243,6 +267,7 @@ impl_writeable_tlv_based!(ChannelConfig, {
        (0, forwarding_fee_proportional_millionths, required),
        (1, max_dust_htlc_exposure_msat, (default_value, 5_000_000)),
        (2, cltv_expiry_delta, required),
+       (3, force_close_avoidance_max_fee_satoshis, (default_value, 1000)),
        (4, announced_channel, required),
        (6, commit_upfront_shutdown_pubkey, required),
        (8, forwarding_fee_base_msat, required),