Move DNSResolverMessageHandler impl to OffersMessageFlow

Author: shaavan

lightning-dns-resolver/src/lib.rs

@@ -393,7 +393,7 @@ mod test {
 		// When we get the proof back, override its contents to an offer from nodes[1]
 		let bs_offer = nodes[1].offers_handler.create_offer_builder(None).unwrap().build().unwrap();
 		nodes[0]
-			.node
+			.offers_handler
 			.testing_dnssec_proof_offer_resolution_override
 			.lock()
 			.unwrap()

lightning/src/ln/channelmanager.rs

@@ -73,7 +73,7 @@ use crate::offers::signer;
 use crate::offers::static_invoice::StaticInvoice;
 use crate::onion_message::async_payments::{AsyncPaymentsMessage, HeldHtlcAvailable, ReleaseHeldHtlc, AsyncPaymentsMessageHandler};
 use crate::onion_message::dns_resolution::HumanReadableName;
-use crate::onion_message::messenger::{Destination, MessageRouter, Responder, ResponseInstruction, MessageSendInstructions};
+use crate::onion_message::messenger::{DefaultMessageRouter, Destination, MessageRouter, MessageSendInstructions, Responder, ResponseInstruction};
 use crate::onion_message::offers::OffersMessage;
 use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
 use crate::sign::ecdsa::EcdsaChannelSigner;
@@ -87,13 +87,10 @@ use crate::util::logger::{Level, Logger, WithContext};
 use crate::util::errors::APIError;
 
 #[cfg(feature = "dnssec")]
-use crate::blinded_path::message::DNSResolverContext;
-#[cfg(feature = "dnssec")]
-use crate::onion_message::dns_resolution::{DNSResolverMessage, DNSResolverMessageHandler, DNSSECQuery, DNSSECProof, OMNameResolver};
+use crate::onion_message::dns_resolution::{DNSResolverMessage, OMNameResolver};
 
 #[cfg(not(c_bindings))]
 use {
-	crate::onion_message::messenger::DefaultMessageRouter,
 	crate::routing::router::DefaultRouter,
 	crate::routing::gossip::NetworkGraph,
 	crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters},
@@ -2397,14 +2394,6 @@ where
 	#[cfg(feature = "dnssec")]
 	pending_dns_onion_messages: Mutex<Vec<(DNSResolverMessage, MessageSendInstructions)>>,
 
-	#[cfg(feature = "_test_utils")]
-	/// In testing, it is useful be able to forge a name -> offer mapping so that we can pay an
-	/// offer generated in the test.
-	///
-	/// This allows for doing so, validating proofs as normal, but, if they pass, replacing the
-	/// offer they resolve to to the given one.
-	pub testing_dnssec_proof_offer_resolution_override: Mutex<HashMap<HumanReadableName, Offer>>,
-
 	#[cfg(test)]
 	pub(super) entropy_source: ES,
 	#[cfg(not(test))]
@@ -3278,9 +3267,6 @@ where
 			hrn_resolver: OMNameResolver::new(current_timestamp, params.best_block.height),
 			#[cfg(feature = "dnssec")]
 			pending_dns_onion_messages: Mutex::new(Vec::new()),
-
-			#[cfg(feature = "_test_utils")]
-			testing_dnssec_proof_offer_resolution_override: Mutex::new(new_hash_map()),
 		}
 	}
 
@@ -9514,6 +9500,7 @@ pub trait OffersMessageCommons {
 	/// Get pending offers messages
 	fn get_pending_offers_messages(&self) -> MutexGuard<'_, Vec<(OffersMessage, MessageSendInstructions)>>;
 
+	#[cfg(feature = "dnssec")]
 	/// Get pending DNS onion messages
 	fn get_pending_dns_onion_messages(&self) -> MutexGuard<'_, Vec<(DNSResolverMessage, MessageSendInstructions)>>;
 
@@ -9646,6 +9633,16 @@ pub trait OffersMessageCommons {
 		payer_note: Option<String>, payment_id: PaymentId,
 		human_readable_name: Option<HumanReadableName>, create_pending_payment: CPP,
 	) -> Result<(), Bolt12SemanticError>;
+
+	#[cfg(feature = "dnssec")]
+	/// Amount for payment awaiting offer
+	fn amt_msats_for_payment_awaiting_offer(&self, payment_id: PaymentId) -> Result<u64, ()>;
+
+	#[cfg(feature = "dnssec")]
+	/// Received Offer
+	fn received_offer(
+		&self, payment_id: PaymentId, retryable_invoice_request: Option<RetryableInvoiceRequest>,
+	) -> Result<(), ()>;
 }
 
 impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, MR: Deref, L: Deref> OffersMessageCommons for ChannelManager<M, T, ES, NS, SP, F, R, MR, L>
@@ -9664,6 +9661,7 @@ where
 		self.pending_offers_messages.lock().expect("Mutex is locked by other thread.")
 	}
 
+	#[cfg(feature = "dnssec")]
 	fn get_pending_dns_onion_messages(&self) -> MutexGuard<'_, Vec<(DNSResolverMessage, MessageSendInstructions)>> {
 		self.pending_dns_onion_messages.lock().expect("Mutex is locked by other thread.")
 	}
@@ -9897,6 +9895,18 @@ where
 
 		self.enqueue_invoice_request(invoice_request, reply_paths)
 	}
+
+	#[cfg(feature = "dnssec")]
+	fn amt_msats_for_payment_awaiting_offer(&self, payment_id: PaymentId) -> Result<u64, ()> {
+		self.pending_outbound_payments.amt_msats_for_payment_awaiting_offer(payment_id)
+	}
+
+	#[cfg(feature = "dnssec")]
+	fn received_offer(
+		&self, payment_id: PaymentId, retryable_invoice_request: Option<RetryableInvoiceRequest>,
+	) -> Result<(), ()> {
+		self.pending_outbound_payments.received_offer(payment_id, retryable_invoice_request)
+	}
 }
 
 /// Defines the maximum number of [`OffersMessage`] including different reply paths to be sent
@@ -11448,69 +11458,6 @@ where
 	}
 }
 
-#[cfg(feature = "dnssec")]
-impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, MR: Deref, L: Deref>
-DNSResolverMessageHandler for ChannelManager<M, T, ES, NS, SP, F, R, MR, L>
-where
-	M::Target: chain::Watch<<SP::Target as SignerProvider>::EcdsaSigner>,
-	T::Target: BroadcasterInterface,
-	ES::Target: EntropySource,
-	NS::Target: NodeSigner,
-	SP::Target: SignerProvider,
-	F::Target: FeeEstimator,
-	R::Target: Router,
-	MR::Target: MessageRouter,
-	L::Target: Logger,
-{
-	fn handle_dnssec_query(
-		&self, _message: DNSSECQuery, _responder: Option<Responder>,
-	) -> Option<(DNSResolverMessage, ResponseInstruction)> {
-		None
-	}
-
-	fn handle_dnssec_proof(&self, message: DNSSECProof, context: DNSResolverContext) {
-		let offer_opt = self.hrn_resolver.handle_dnssec_proof_for_offer(message, context);
-		#[cfg_attr(not(feature = "_test_utils"), allow(unused_mut))]
-		if let Some((completed_requests, mut offer)) = offer_opt {
-			for (name, payment_id) in completed_requests {
-				#[cfg(feature = "_test_utils")]
-				if let Some(replacement_offer) = self.testing_dnssec_proof_offer_resolution_override.lock().unwrap().remove(&name) {
-					// If we have multiple pending requests we may end up over-using the override
-					// offer, but tests can deal with that.
-					offer = replacement_offer;
-				}
-				if let Ok(amt_msats) = self.pending_outbound_payments.amt_msats_for_payment_awaiting_offer(payment_id) {
-					let offer_pay_res =
-						self.pay_for_offer_intern(&offer, None, Some(amt_msats), None, payment_id, Some(name),
-							|invoice_request, nonce| {
-								let retryable_invoice_request = RetryableInvoiceRequest {
-									invoice_request: invoice_request.clone(),
-									nonce,
-								};
-								self.pending_outbound_payments
-									.received_offer(payment_id, Some(retryable_invoice_request))
-									.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)
-						});
-					if offer_pay_res.is_err() {
-						// The offer we tried to pay is the canonical current offer for the name we
-						// wanted to pay. If we can't pay it, there's no way to recover so fail the
-						// payment.
-						// Note that the PaymentFailureReason should be ignored for an
-						// AwaitingInvoice payment.
-						self.pending_outbound_payments.abandon_payment(
-							payment_id, PaymentFailureReason::RouteNotFound, &self.pending_events,
-						);
-					}
-				}
-			}
-		}
-	}
-
-	fn release_pending_messages(&self) -> Vec<(DNSResolverMessage, MessageSendInstructions)> {
-		core::mem::take(&mut self.pending_dns_onion_messages.lock().unwrap())
-	}
-}
-
 impl<M: Deref, T: Deref, ES: Deref, NS: Deref, SP: Deref, F: Deref, R: Deref, MR: Deref, L: Deref>
 NodeIdLookUp for ChannelManager<M, T, ES, NS, SP, F, R, MR, L>
 where
@@ -13363,9 +13310,6 @@ where
 			hrn_resolver: OMNameResolver::new(highest_seen_timestamp, best_block_height),
 			#[cfg(feature = "dnssec")]
 			pending_dns_onion_messages: Mutex::new(Vec::new()),
-
-			#[cfg(feature = "_test_utils")]
-			testing_dnssec_proof_offer_resolution_override: Mutex::new(new_hash_map()),
 		};
 
 		for (_, monitor) in args.channel_monitors.iter() {

lightning/src/ln/functional_test_utils.rs

@@ -440,7 +440,7 @@ type TestOnionMessenger<'chan_man, 'node_cfg, 'chan_mon_cfg> = OnionMessenger<
 	&'node_cfg test_utils::TestMessageRouter<'chan_mon_cfg>,
 	Arc<TestOffersMessageFlow<'chan_man, 'node_cfg, 'chan_mon_cfg>>,
 	&'chan_man TestChannelManager<'node_cfg, 'chan_mon_cfg>,
-	&'chan_man TestChannelManager<'node_cfg, 'chan_mon_cfg>,
+	Arc<TestOffersMessageFlow<'chan_man, 'node_cfg, 'chan_mon_cfg>>,
 	IgnoringMessageHandler,
 >;
 
@@ -3362,7 +3362,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(node_count: usize, cfgs: &'b Vec<NodeC
 		#[cfg(feature = "dnssec")]
 		let onion_messenger = OnionMessenger::new(
 			dedicated_entropy, cfgs[i].keys_manager, cfgs[i].logger, &chan_mgrs[i],
-			&cfgs[i].message_router, offers_handler.clone(), &chan_mgrs[i], &chan_mgrs[i],
+			&cfgs[i].message_router, offers_handler.clone(), &chan_mgrs[i], offers_handler.clone(),
 			IgnoringMessageHandler {},
 		);
 		#[cfg(not(feature = "dnssec"))]

lightning/src/offers/flow.rs

@@ -10,6 +10,7 @@
 //! Provides data structures and functions for creating and managing Offers messages,
 //! facilitating communication, and handling Bolt12 invoice payments.
 
+use crate::prelude::*;
 use alloc::vec::Vec;
 use core::ops::Deref;
 use core::time::Duration;
@@ -40,6 +41,7 @@ use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
 
 use crate::sign::EntropySource;
+use crate::sync::Mutex;
 use crate::util::logger::{Logger, WithContext};
 
 #[cfg(c_bindings)]
@@ -49,7 +51,12 @@ use {
 };
 
 #[cfg(feature = "dnssec")]
-use crate::onion_message::dns_resolution::{DNSResolverMessage, HumanReadableName};
+use {
+	crate::blinded_path::message::DNSResolverContext,
+	crate::onion_message::dns_resolution::{
+		DNSResolverMessage, DNSResolverMessageHandler, DNSSECProof, DNSSECQuery, HumanReadableName,
+	},
+};
 
 /// A trivial trait which describes any [`OffersMessageFlow`].
 ///
@@ -394,6 +401,14 @@ where
 	/// Contains functions shared between OffersMessageHandler and ChannelManager.
 	commons: OMC,
 
+	#[cfg(feature = "_test_utils")]
+	/// In testing, it is useful be able to forge a name -> offer mapping so that we can pay an
+	/// offer generated in the test.
+	///
+	/// This allows for doing so, validating proofs as normal, but, if they pass, replacing the
+	/// offer they resolve to to the given one.
+	pub testing_dnssec_proof_offer_resolution_override: Mutex<HashMap<HumanReadableName, Offer>>,
+
 	/// The Logger for use in the OffersMessageFlow and which may be used to log
 	/// information during deserialization.
 	pub logger: L,
@@ -410,7 +425,14 @@ where
 		let mut secp_ctx = Secp256k1::new();
 		secp_ctx.seeded_randomize(&entropy_source.get_secure_random_bytes());
 
-		Self { secp_ctx, commons, entropy_source, logger }
+		Self {
+			secp_ctx,
+			commons,
+			entropy_source,
+			#[cfg(feature = "_test_utils")]
+			testing_dnssec_proof_offer_resolution_override: Mutex::new(new_hash_map()),
+			logger,
+		}
 	}
 }
 
@@ -1150,3 +1172,73 @@ where
 		Ok(())
 	}
 }
+
+#[cfg(feature = "dnssec")]
+impl<ES: Deref, OMC: Deref, L: Deref> DNSResolverMessageHandler for OffersMessageFlow<ES, OMC, L>
+where
+	ES::Target: EntropySource,
+	OMC::Target: OffersMessageCommons,
+	L::Target: Logger,
+{
+	fn handle_dnssec_query(
+		&self, _message: DNSSECQuery, _responder: Option<Responder>,
+	) -> Option<(DNSResolverMessage, ResponseInstruction)> {
+		None
+	}
+
+	fn handle_dnssec_proof(&self, message: DNSSECProof, context: DNSResolverContext) {
+		let offer_opt =
+			self.commons.get_hrn_resolver().handle_dnssec_proof_for_offer(message, context);
+		#[cfg_attr(not(feature = "_test_utils"), allow(unused_mut))]
+		if let Some((completed_requests, mut offer)) = offer_opt {
+			for (name, payment_id) in completed_requests {
+				#[cfg(feature = "_test_utils")]
+				if let Some(replacement_offer) = self
+					.testing_dnssec_proof_offer_resolution_override
+					.lock()
+					.unwrap()
+					.remove(&name)
+				{
+					// If we have multiple pending requests we may end up over-using the override
+					// offer, but tests can deal with that.
+					offer = replacement_offer;
+				}
+				if let Ok(amt_msats) = self.commons.amt_msats_for_payment_awaiting_offer(payment_id)
+				{
+					let offer_pay_res = self.commons.pay_for_offer_intern(
+						&offer,
+						None,
+						Some(amt_msats),
+						None,
+						payment_id,
+						Some(name),
+						|invoice_request, nonce| {
+							let retryable_invoice_request = RetryableInvoiceRequest {
+								invoice_request: invoice_request.clone(),
+								nonce,
+							};
+							self.commons
+								.received_offer(payment_id, Some(retryable_invoice_request))
+								.map_err(|_| Bolt12SemanticError::DuplicatePaymentId)
+						},
+					);
+					if offer_pay_res.is_err() {
+						// The offer we tried to pay is the canonical current offer for the name we
+						// wanted to pay. If we can't pay it, there's no way to recover so fail the
+						// payment.
+						// Note that the PaymentFailureReason should be ignored for an
+						// AwaitingInvoice payment.
+						self.commons.abandon_payment_with_reason(
+							payment_id,
+							PaymentFailureReason::RouteNotFound,
+						);
+					}
+				}
+			}
+		}
+	}
+
+	fn release_pending_messages(&self) -> Vec<(DNSResolverMessage, MessageSendInstructions)> {
+		core::mem::take(&mut self.commons.get_pending_dns_onion_messages())
+	}
+}