X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Fln%2Fchannelmanager.rs;h=69093d7f6a35ce22cc67fc2167ebc6e0d2974d4d;hb=82b532c54db938c64f9bc9fb96c8f572337052be;hp=d27f1be08cddcb75f3e3ca561f3dac86dccdab2a;hpb=620244dc2ec3153a61e009b80a8c59cf41514482;p=rust-lightning

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index d27f1be0..69093d7f 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -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;
@@ -2613,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(|_| ())
 				},
 			}
@@ -3549,19 +3554,10 @@ where
 	/// wait until you receive either a [`Event::PaymentFailed`] or [`Event::PaymentSent`] event to
 	/// determine the ultimate status of a payment.
 	///
-	/// # Requested Invoices
-	///
-	/// In the case of paying a [`Bolt12Invoice`], abandoning the payment prior to receiving the
-	/// invoice will result in an [`Event::InvoiceRequestFailed`] and prevent any attempts at paying
-	/// it once received. The other events may only be generated once the invoice has been received.
-	///
 	/// # Restart Behavior
 	///
 	/// If an [`Event::PaymentFailed`] is generated and we restart without first persisting the
-	/// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated; likewise for
-	/// [`Event::InvoiceRequestFailed`].
-	///
-	/// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice
+	/// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated.
 	pub fn abandon_payment(&self, payment_id: PaymentId) {
 		let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self);
 		self.pending_outbound_payments.abandon_payment(payment_id, PaymentFailureReason::UserAbandoned, &self.pending_events);
@@ -3902,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,
@@ -4007,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),
 				});
 			};
 		}
@@ -4118,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)
 				})
 			}
@@ -4731,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() {
@@ -6389,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));
@@ -9974,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());
@@ -10754,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 }) => {
@@ -10798,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.