Log when we prepare to block a channel's next `revoke_and_ack`
[rust-lightning] / lightning / src / ln / channelmanager.rs
index 5985e0c722785cc902129b1ac1b2877d838a516e..69093d7f6a35ce22cc67fc2167ebc6e0d2974d4d 100644 (file)
@@ -803,7 +803,7 @@ pub type SimpleArcChannelManager<M, T, F, L> = ChannelManager<
        Arc<DefaultRouter<
                Arc<NetworkGraph<Arc<L>>>,
                Arc<L>,
-               Arc<Mutex<ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>>>,
+               Arc<RwLock<ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>>>,
                ProbabilisticScoringFeeParameters,
                ProbabilisticScorer<Arc<NetworkGraph<Arc<L>>>, Arc<L>>,
        >>,
@@ -832,7 +832,7 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> =
                &'e DefaultRouter<
                        &'f NetworkGraph<&'g L>,
                        &'g L,
-                       &'h Mutex<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>,
+                       &'h RwLock<ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>>,
                        ProbabilisticScoringFeeParameters,
                        ProbabilisticScorer<&'f NetworkGraph<&'g L>, &'g L>
                >,
@@ -840,6 +840,9 @@ pub type SimpleRefChannelManager<'a, 'b, 'c, 'd, 'e, 'f, 'g, 'h, M, T, F, L> =
        >;
 
 /// A trivial trait which describes any [`ChannelManager`].
+///
+/// This is not exported to bindings users as general cover traits aren't useful in other
+/// languages.
 pub trait AChannelManager {
        /// A type implementing [`chain::Watch`].
        type Watch: chain::Watch<Self::Signer> + ?Sized;
@@ -1448,12 +1451,6 @@ pub struct ChannelCounterparty {
 }
 
 /// Details of a channel, as returned by [`ChannelManager::list_channels`] and [`ChannelManager::list_usable_channels`]
-///
-/// Balances of a channel are available through [`ChainMonitor::get_claimable_balances`] and
-/// [`ChannelMonitor::get_claimable_balances`], calculated with respect to the corresponding on-chain
-/// transactions.
-///
-/// [`ChainMonitor::get_claimable_balances`]: crate::chain::chainmonitor::ChainMonitor::get_claimable_balances
 #[derive(Clone, Debug, PartialEq)]
 pub struct ChannelDetails {
        /// The channel's ID (prior to funding transaction generation, this is a random 32 bytes,
@@ -1535,11 +1532,24 @@ pub struct ChannelDetails {
        ///
        /// This value will be `None` for objects serialized with LDK versions prior to 0.0.115.
        pub feerate_sat_per_1000_weight: Option<u32>,
+       /// Our total balance.  This is the amount we would get if we close the channel.
+       /// This value is not exact. Due to various in-flight changes and feerate changes, exactly this
+       /// amount is not likely to be recoverable on close.
+       ///
+       /// This does not include any pending HTLCs which are not yet fully resolved (and, thus, whose
+       /// balance is not available for inclusion in new outbound HTLCs). This further does not include
+       /// any pending outgoing HTLCs which are awaiting some other resolution to be sent.
+       /// This does not consider any on-chain fees.
+       ///
+       /// See also [`ChannelDetails::outbound_capacity_msat`]
+       pub balance_msat: u64,
        /// The available outbound capacity for sending HTLCs to the remote peer. This does not include
        /// any pending HTLCs which are not yet fully resolved (and, thus, whose balance is not
        /// available for inclusion in new outbound HTLCs). This further does not include any pending
        /// outgoing HTLCs which are awaiting some other resolution to be sent.
        ///
+       /// See also [`ChannelDetails::balance_msat`]
+       ///
        /// This value is not exact. Due to various in-flight changes, feerate changes, and our
        /// conflict-avoidance policy, exactly this amount is not likely to be spendable. However, we
        /// should be able to spend nearly this amount.
@@ -1549,8 +1559,8 @@ pub struct ChannelDetails {
        /// the current state and per-HTLC limit(s). This is intended for use when routing, allowing us
        /// to use a limit as close as possible to the HTLC limit we can currently send.
        ///
-       /// See also [`ChannelDetails::next_outbound_htlc_minimum_msat`] and
-       /// [`ChannelDetails::outbound_capacity_msat`].
+       /// See also [`ChannelDetails::next_outbound_htlc_minimum_msat`],
+       /// [`ChannelDetails::balance_msat`], and [`ChannelDetails::outbound_capacity_msat`].
        pub next_outbound_htlc_limit_msat: u64,
        /// The minimum value for sending a single HTLC to the remote peer. This is the equivalent of
        /// [`ChannelDetails::next_outbound_htlc_limit_msat`] but represents a lower-bound, rather than
@@ -1680,6 +1690,7 @@ impl ChannelDetails {
                        channel_value_satoshis: context.get_value_satoshis(),
                        feerate_sat_per_1000_weight: Some(context.get_feerate_sat_per_1000_weight()),
                        unspendable_punishment_reserve: to_self_reserve_satoshis,
+                       balance_msat: balance.balance_msat,
                        inbound_capacity_msat: balance.inbound_capacity_msat,
                        outbound_capacity_msat: balance.outbound_capacity_msat,
                        next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat,
@@ -2605,6 +2616,8 @@ where
                                        // it does not exist for this peer. Either way, we can attempt to force-close it.
                                        //
                                        // An appropriate error will be returned for non-existence of the channel if that's the case.
+                                       mem::drop(peer_state_lock);
+                                       mem::drop(per_peer_state);
                                        return self.force_close_channel_with_peer(&channel_id, counterparty_node_id, None, false).map(|_| ())
                                },
                        }
@@ -3885,7 +3898,7 @@ where
                                btree_map::Entry::Vacant(vacant) => Some(vacant.insert(Vec::new())),
                        }
                });
-               for (channel_idx, &(temporary_channel_id, counterparty_node_id)) in temporary_channels.iter().enumerate() {
+               for &(temporary_channel_id, counterparty_node_id) in temporary_channels.iter() {
                        result = result.and_then(|_| self.funding_transaction_generated_intern(
                                temporary_channel_id,
                                counterparty_node_id,
@@ -3990,7 +4003,7 @@ where
                for channel_id in channel_ids {
                        if !peer_state.has_channel(channel_id) {
                                return Err(APIError::ChannelUnavailable {
-                                       err: format!("Channel with ID {} was not found for the passed counterparty_node_id {}", channel_id, counterparty_node_id),
+                                       err: format!("Channel with id {} not found for the passed counterparty node_id {}", channel_id, counterparty_node_id),
                                });
                        };
                }
@@ -4101,7 +4114,7 @@ where
                                                next_hop_channel_id, next_node_id)
                                }),
                                None => return Err(APIError::ChannelUnavailable {
-                                       err: format!("Channel with id {} not found for the passed counterparty node_id {}.",
+                                       err: format!("Channel with id {} not found for the passed counterparty node_id {}",
                                                next_hop_channel_id, next_node_id)
                                })
                        }
@@ -4714,8 +4727,10 @@ where
                if !chan.context.is_outbound() { return NotifyOption::SkipPersistNoEvents; }
                // If the feerate has decreased by less than half, don't bother
                if new_feerate <= chan.context.get_feerate_sat_per_1000_weight() && new_feerate * 2 > chan.context.get_feerate_sat_per_1000_weight() {
-                       log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {}.",
+                       if new_feerate != chan.context.get_feerate_sat_per_1000_weight() {
+                               log_trace!(self.logger, "Channel {} does not qualify for a feerate change from {} to {}.",
                                chan_id, chan.context.get_feerate_sat_per_1000_weight(), new_feerate);
+                       }
                        return NotifyOption::SkipPersistNoEvents;
                }
                if !chan.context.is_live() {
@@ -6372,6 +6387,9 @@ where
                                        if let ChannelPhase::Funded(chan) = chan_phase_entry.get_mut() {
                                                let res = try_chan_phase_entry!(self, chan.update_fulfill_htlc(&msg), chan_phase_entry);
                                                if let HTLCSource::PreviousHopData(prev_hop) = &res.0 {
+                                                       log_trace!(self.logger,
+                                                               "Holding the next revoke_and_ack from {} until the preimage is durably persisted in the inbound edge's ChannelMonitor",
+                                                               msg.channel_id);
                                                        peer_state.actions_blocking_raa_monitor_updates.entry(msg.channel_id)
                                                                .or_insert_with(Vec::new)
                                                                .push(RAAMonitorUpdateBlockingAction::from_prev_hop_data(&prev_hop));
@@ -6967,8 +6985,7 @@ where
        fn maybe_generate_initial_closing_signed(&self) -> bool {
                let mut handle_errors: Vec<(PublicKey, Result<(), _>)> = Vec::new();
                let mut has_update = false;
-               let mut shutdown_result = None;
-               let mut unbroadcasted_batch_funding_txid = None;
+               let mut shutdown_results = Vec::new();
                {
                        let per_peer_state = self.per_peer_state.read().unwrap();
 
@@ -6979,7 +6996,7 @@ where
                                peer_state.channel_by_id.retain(|channel_id, phase| {
                                        match phase {
                                                ChannelPhase::Funded(chan) => {
-                                                       unbroadcasted_batch_funding_txid = chan.context.unbroadcasted_batch_funding_txid();
+                                                       let unbroadcasted_batch_funding_txid = chan.context.unbroadcasted_batch_funding_txid();
                                                        match chan.maybe_propose_closing_signed(&self.fee_estimator, &self.logger) {
                                                                Ok((msg_opt, tx_opt)) => {
                                                                        if let Some(msg) = msg_opt {
@@ -7002,7 +7019,7 @@ where
                                                                                log_info!(self.logger, "Broadcasting {}", log_tx!(tx));
                                                                                self.tx_broadcaster.broadcast_transactions(&[&tx]);
                                                                                update_maps_on_chan_removal!(self, &chan.context);
-                                                                               shutdown_result = Some((None, Vec::new(), unbroadcasted_batch_funding_txid));
+                                                                               shutdown_results.push((None, Vec::new(), unbroadcasted_batch_funding_txid));
                                                                                false
                                                                        } else { true }
                                                                },
@@ -7024,7 +7041,7 @@ where
                        let _ = handle_error!(self, err, counterparty_node_id);
                }
 
-               if let Some(shutdown_result) = shutdown_result {
+               for shutdown_result in shutdown_results.drain(..) {
                        self.finish_close_channel(shutdown_result);
                }
 
@@ -8410,7 +8427,7 @@ impl Writeable for ChannelDetails {
                        (10, self.channel_value_satoshis, required),
                        (12, self.unspendable_punishment_reserve, option),
                        (14, user_channel_id_low, required),
-                       (16, self.next_outbound_htlc_limit_msat, required),  // Forwards compatibility for removed balance_msat field.
+                       (16, self.balance_msat, required),
                        (18, self.outbound_capacity_msat, required),
                        (19, self.next_outbound_htlc_limit_msat, required),
                        (20, self.inbound_capacity_msat, required),
@@ -8446,7 +8463,7 @@ impl Readable for ChannelDetails {
                        (10, channel_value_satoshis, required),
                        (12, unspendable_punishment_reserve, option),
                        (14, user_channel_id_low, required),
-                       (16, _balance_msat, option),  // Backwards compatibility for removed balance_msat field.
+                       (16, balance_msat, required),
                        (18, outbound_capacity_msat, required),
                        // Note that by the time we get past the required read above, outbound_capacity_msat will be
                        // filled in, so we can safely unwrap it here.
@@ -8472,8 +8489,6 @@ impl Readable for ChannelDetails {
                let user_channel_id = user_channel_id_low as u128 +
                        ((user_channel_id_high_opt.unwrap_or(0 as u64) as u128) << 64);
 
-               let _balance_msat: Option<u64> = _balance_msat;
-
                Ok(Self {
                        inbound_scid_alias,
                        channel_id: channel_id.0.unwrap(),
@@ -8486,6 +8501,7 @@ impl Readable for ChannelDetails {
                        channel_value_satoshis: channel_value_satoshis.0.unwrap(),
                        unspendable_punishment_reserve,
                        user_channel_id,
+                       balance_msat: balance_msat.0.unwrap(),
                        outbound_capacity_msat: outbound_capacity_msat.0.unwrap(),
                        next_outbound_htlc_limit_msat: next_outbound_htlc_limit_msat.0.unwrap(),
                        next_outbound_htlc_minimum_msat: next_outbound_htlc_minimum_msat.0.unwrap(),
@@ -9959,6 +9975,9 @@ where
                                                                Some((blocked_node_id, blocked_channel_outpoint, blocking_action)), ..
                                                } = action {
                                                        if let Some(blocked_peer_state) = per_peer_state.get(&blocked_node_id) {
+                                                               log_trace!(args.logger,
+                                                                       "Holding the next revoke_and_ack from {} until the preimage is durably persisted in the inbound edge's ChannelMonitor",
+                                                                       blocked_channel_outpoint.to_channel_id());
                                                                blocked_peer_state.lock().unwrap().actions_blocking_raa_monitor_updates
                                                                        .entry(blocked_channel_outpoint.to_channel_id())
                                                                        .or_insert_with(Vec::new).push(blocking_action.clone());
@@ -10739,6 +10758,16 @@ mod tests {
                check_api_error_message(expected_message, res_err)
        }
 
+       fn check_channel_unavailable_error<T>(res_err: Result<T, APIError>, expected_channel_id: ChannelId, peer_node_id: PublicKey) {
+               let expected_message = format!("Channel with id {} not found for the passed counterparty node_id {}", expected_channel_id, peer_node_id);
+               check_api_error_message(expected_message, res_err)
+       }
+
+       fn check_api_misuse_error<T>(res_err: Result<T, APIError>) {
+               let expected_message = "No such channel awaiting to be accepted.".to_string();
+               check_api_error_message(expected_message, res_err)
+       }
+
        fn check_api_error_message<T>(expected_err_message: String, res_err: Result<T, APIError>) {
                match res_err {
                        Err(APIError::APIMisuseError { err }) => {
@@ -10783,6 +10812,36 @@ mod tests {
                check_unkown_peer_error(nodes[0].node.update_channel_config(&unkown_public_key, &[channel_id], &ChannelConfig::default()), unkown_public_key);
        }
 
+       #[test]
+       fn test_api_calls_with_unavailable_channel() {
+               // Tests that our API functions that expects a `counterparty_node_id` and a `channel_id`
+               // as input, behaves as expected if the `counterparty_node_id` is a known peer in the
+               // `ChannelManager::per_peer_state` map, but the peer state doesn't contain a channel with
+               // the given `channel_id`.
+               let chanmon_cfg = create_chanmon_cfgs(2);
+               let node_cfg = create_node_cfgs(2, &chanmon_cfg);
+               let node_chanmgr = create_node_chanmgrs(2, &node_cfg, &[None, None]);
+               let nodes = create_network(2, &node_cfg, &node_chanmgr);
+
+               let counterparty_node_id = nodes[1].node.get_our_node_id();
+
+               // Dummy values
+               let channel_id = ChannelId::from_bytes([4; 32]);
+
+               // Test the API functions.
+               check_api_misuse_error(nodes[0].node.accept_inbound_channel(&channel_id, &counterparty_node_id, 42));
+
+               check_channel_unavailable_error(nodes[0].node.close_channel(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id);
+
+               check_channel_unavailable_error(nodes[0].node.force_close_broadcasting_latest_txn(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id);
+
+               check_channel_unavailable_error(nodes[0].node.force_close_without_broadcasting_txn(&channel_id, &counterparty_node_id), channel_id, counterparty_node_id);
+
+               check_channel_unavailable_error(nodes[0].node.forward_intercepted_htlc(InterceptId([0; 32]), &channel_id, counterparty_node_id, 1_000_000), channel_id, counterparty_node_id);
+
+               check_channel_unavailable_error(nodes[0].node.update_channel_config(&counterparty_node_id, &[channel_id], &ChannelConfig::default()), channel_id, counterparty_node_id);
+       }
+
        #[test]
        fn test_connection_limiting() {
                // Test that we limit un-channel'd peers and un-funded channels properly.