Merge pull request #2650 from TheBlueMatt/2023-10-make-clippy-shut-up
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Thu, 12 Oct 2023 20:44:46 +0000 (20:44 +0000)
committerGitHub <noreply@github.com>
Thu, 12 Oct 2023 20:44:46 +0000 (20:44 +0000)
Make clippy shut up about `PartialOrd` and `Ord` both impl'd

lightning-invoice/src/payment.rs
lightning/src/chain/transaction.rs
lightning/src/ln/channelmanager.rs
lightning/src/ln/payment_tests.rs

index 0247913634a24f30cab3e8bb2a77ab3c49d7fdf1..89842591fdec8a89693cbb62457389e9f4a46dc9 100644 (file)
@@ -33,9 +33,10 @@ use core::time::Duration;
 /// with the same [`PaymentHash`] is never sent.
 ///
 /// If you wish to use a different payment idempotency token, see [`pay_invoice_with_id`].
-pub fn pay_invoice<C: AChannelManager>(
-       invoice: &Bolt11Invoice, retry_strategy: Retry, channelmanager: &C
+pub fn pay_invoice<C: Deref>(
+       invoice: &Bolt11Invoice, retry_strategy: Retry, channelmanager: C
 ) -> Result<PaymentId, PaymentError>
+where C::Target: AChannelManager,
 {
        let payment_id = PaymentId(invoice.payment_hash().into_inner());
        pay_invoice_with_id(invoice, payment_id, retry_strategy, channelmanager.get_cm())
@@ -52,9 +53,10 @@ pub fn pay_invoice<C: AChannelManager>(
 /// [`PaymentHash`] has never been paid before.
 ///
 /// See [`pay_invoice`] for a variant which uses the [`PaymentHash`] for the idempotency token.
-pub fn pay_invoice_with_id<C: AChannelManager>(
-       invoice: &Bolt11Invoice, payment_id: PaymentId, retry_strategy: Retry, channelmanager: &C
+pub fn pay_invoice_with_id<C: Deref>(
+       invoice: &Bolt11Invoice, payment_id: PaymentId, retry_strategy: Retry, channelmanager: C
 ) -> Result<(), PaymentError>
+where C::Target: AChannelManager,
 {
        let amt_msat = invoice.amount_milli_satoshis().ok_or(PaymentError::Invoice("amount missing"))?;
        pay_invoice_using_amount(invoice, amt_msat, payment_id, retry_strategy, channelmanager.get_cm())
@@ -69,9 +71,10 @@ pub fn pay_invoice_with_id<C: AChannelManager>(
 ///
 /// If you wish to use a different payment idempotency token, see
 /// [`pay_zero_value_invoice_with_id`].
-pub fn pay_zero_value_invoice<C: AChannelManager>(
-       invoice: &Bolt11Invoice, amount_msats: u64, retry_strategy: Retry, channelmanager: &C
+pub fn pay_zero_value_invoice<C: Deref>(
+       invoice: &Bolt11Invoice, amount_msats: u64, retry_strategy: Retry, channelmanager: C
 ) -> Result<PaymentId, PaymentError>
+where C::Target: AChannelManager,
 {
        let payment_id = PaymentId(invoice.payment_hash().into_inner());
        pay_zero_value_invoice_with_id(invoice, amount_msats, payment_id, retry_strategy,
@@ -90,10 +93,11 @@ pub fn pay_zero_value_invoice<C: AChannelManager>(
 ///
 /// See [`pay_zero_value_invoice`] for a variant which uses the [`PaymentHash`] for the
 /// idempotency token.
-pub fn pay_zero_value_invoice_with_id<C: AChannelManager>(
+pub fn pay_zero_value_invoice_with_id<C: Deref>(
        invoice: &Bolt11Invoice, amount_msats: u64, payment_id: PaymentId, retry_strategy: Retry,
-       channelmanager: &C
+       channelmanager: C
 ) -> Result<(), PaymentError>
+where C::Target: AChannelManager,
 {
        if invoice.amount_milli_satoshis().is_some() {
                Err(PaymentError::Invoice("amount unexpected"))
@@ -125,9 +129,10 @@ fn pay_invoice_using_amount<P: Deref>(
 /// Sends payment probes over all paths of a route that would be used to pay the given invoice.
 ///
 /// See [`ChannelManager::send_preflight_probes`] for more information.
-pub fn preflight_probe_invoice<C: AChannelManager>(
-       invoice: &Bolt11Invoice, channelmanager: &C, liquidity_limit_multiplier: Option<u64>,
+pub fn preflight_probe_invoice<C: Deref>(
+       invoice: &Bolt11Invoice, channelmanager: C, liquidity_limit_multiplier: Option<u64>,
 ) -> Result<Vec<(PaymentHash, PaymentId)>, ProbingError>
+where C::Target: AChannelManager,
 {
        let amount_msat = if let Some(invoice_amount_msat) = invoice.amount_milli_satoshis() {
                invoice_amount_msat
@@ -156,10 +161,11 @@ pub fn preflight_probe_invoice<C: AChannelManager>(
 /// invoice using the given amount.
 ///
 /// See [`ChannelManager::send_preflight_probes`] for more information.
-pub fn preflight_probe_zero_value_invoice<C: AChannelManager>(
-       invoice: &Bolt11Invoice, amount_msat: u64, channelmanager: &C,
+pub fn preflight_probe_zero_value_invoice<C: Deref>(
+       invoice: &Bolt11Invoice, amount_msat: u64, channelmanager: C,
        liquidity_limit_multiplier: Option<u64>,
 ) -> Result<Vec<(PaymentHash, PaymentId)>, ProbingError>
+where C::Target: AChannelManager,
 {
        if invoice.amount_milli_satoshis().is_some() {
                return Err(ProbingError::Invoice("amount unexpected"));
index 22e7ec2c2f9eb90fcfaa8f5b754c27540aa854e8..39a0a5449ac7cf1a4f5dc636363b0ad9c0245c6f 100644 (file)
@@ -75,6 +75,12 @@ impl OutPoint {
        }
 }
 
+impl core::fmt::Display for OutPoint {
+       fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+               write!(f, "{}:{}", self.txid, self.index)
+       }
+}
+
 impl_writeable!(OutPoint, { txid, index });
 
 #[cfg(test)]
index 62c6741fbdf89f25595e2c855b7dbb2fdc2e609a..1e9a3ed011def00672e46053391ac2b7d4f723ea 100644 (file)
@@ -2616,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(|_| ())
                                },
                        }
@@ -4001,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),
                                });
                        };
                }
@@ -4112,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)
                                })
                        }
@@ -4725,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() {
@@ -10748,6 +10752,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 }) => {
@@ -10792,6 +10806,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.
index 26ecbb0bd249d116cf8e54f440145c60d1796bbd..616a1d4ce00a3a429c64697100f109c144caa855 100644 (file)
@@ -1906,7 +1906,7 @@ fn do_test_intercepted_payment(test: InterceptTest) {
        // Check for unknown channel id error.
        let unknown_chan_id_err = nodes[1].node.forward_intercepted_htlc(intercept_id, &ChannelId::from_bytes([42; 32]), nodes[2].node.get_our_node_id(), expected_outbound_amount_msat).unwrap_err();
        assert_eq!(unknown_chan_id_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 {}",
                        log_bytes!([42; 32]), nodes[2].node.get_our_node_id()) });
 
        if test == InterceptTest::Fail {