From 88343366ca04085904ee0ac41d5a10a86d933a35 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 23 Jul 2024 18:24:43 -0500 Subject: [PATCH] Replace use of OffersContext::Unknown with None Now that ChannelManager uses a known OffersContext when creating blinded paths, OffersContext::Unknown is no longer needed. Remove it and update OffersMessageHandler to us an Option, which is more idiomatic for signifying whether a message was delivered with or without an OffersContext. --- fuzz/src/onion_message.rs | 3 ++- lightning/src/blinded_path/message.rs | 12 +++------ lightning/src/events/mod.rs | 8 +++--- lightning/src/ln/channelmanager.rs | 26 +++++++++++-------- lightning/src/ln/offers_tests.rs | 8 +++--- lightning/src/ln/peer_handler.rs | 2 +- .../src/onion_message/functional_tests.rs | 2 +- lightning/src/onion_message/messenger.rs | 6 ++--- lightning/src/onion_message/offers.rs | 4 ++- 9 files changed, 36 insertions(+), 35 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 05ee7526f..490b7d72a 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -106,7 +106,8 @@ struct TestOffersMessageHandler {} impl OffersMessageHandler for TestOffersMessageHandler { fn handle_message( - &self, _message: OffersMessage, _context: OffersContext, _responder: Option, + &self, _message: OffersMessage, _context: Option, + _responder: Option, ) -> ResponseInstruction { ResponseInstruction::NoResponse } diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 2ff799d0e..47444eb90 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -108,11 +108,6 @@ pub enum MessageContext { /// [`OffersMessage`]: crate::onion_message::offers::OffersMessage #[derive(Clone, Debug, Eq, PartialEq)] pub enum OffersContext { - /// Represents an unknown BOLT12 message context. - /// - /// This variant is used when a message is sent without using a [`BlindedPath`] or over one - /// created prior to LDK version 0.0.124. - Unknown {}, /// Context used by a [`BlindedPath`] within an [`Offer`]. /// /// This variant is intended to be received when handling an [`InvoiceRequest`]. @@ -172,15 +167,14 @@ impl_writeable_tlv_based_enum!(MessageContext, ); impl_writeable_tlv_based_enum!(OffersContext, - (0, Unknown) => {}, - (1, InvoiceRequest) => { + (0, InvoiceRequest) => { (0, nonce, required), }, - (2, OutboundPayment) => { + (1, OutboundPayment) => { (0, payment_id, required), (1, nonce, required), }, - (3, InboundPayment) => { + (2, InboundPayment) => { (0, payment_hash, required), }, ); diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 9aa449efb..570d581a0 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -810,7 +810,7 @@ pub enum Event { /// The context of the [`BlindedPath`] used to send the invoice. /// /// [`BlindedPath`]: crate::blinded_path::BlindedPath - context: OffersContext, + context: Option, /// A responder for replying with an [`InvoiceError`] if needed. /// /// `None` if the invoice wasn't sent with a reply path. @@ -1658,7 +1658,7 @@ impl Writeable for Event { write_tlv_fields!(writer, { (0, payment_id, required), (2, invoice, required), - (4, context, required), + (4, context, option), (6, responder, option), }); }, @@ -2113,13 +2113,13 @@ impl MaybeReadable for Event { _init_and_read_len_prefixed_tlv_fields!(reader, { (0, payment_id, required), (2, invoice, required), - (4, context, required), + (4, context, option), (6, responder, option), }); Ok(Some(Event::InvoiceReceived { payment_id: payment_id.0.unwrap(), invoice: invoice.0.unwrap(), - context: context.0.unwrap(), + context, responder, })) }; diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c316dc2b4..eabc09bc0 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4204,7 +4204,7 @@ where /// /// [timer tick]: Self::timer_tick_occurred pub fn send_payment_for_bolt12_invoice( - &self, invoice: &Bolt12Invoice, context: &OffersContext, + &self, invoice: &Bolt12Invoice, context: Option<&OffersContext>, ) -> Result<(), Bolt12PaymentError> { match self.verify_bolt12_invoice(invoice, context) { Ok(payment_id) => self.send_payment_for_verified_bolt12_invoice(invoice, payment_id), @@ -4213,17 +4213,17 @@ where } fn verify_bolt12_invoice( - &self, invoice: &Bolt12Invoice, context: &OffersContext, + &self, invoice: &Bolt12Invoice, context: Option<&OffersContext>, ) -> Result { let secp_ctx = &self.secp_ctx; let expanded_key = &self.inbound_payment_key; match context { - OffersContext::Unknown {} if invoice.is_for_refund_without_paths() => { + None if invoice.is_for_refund_without_paths() => { invoice.verify_using_metadata(expanded_key, secp_ctx) }, - OffersContext::OutboundPayment { payment_id, nonce } => { - invoice.verify_using_payer_data(*payment_id, *nonce, expanded_key, secp_ctx) + Some(&OffersContext::OutboundPayment { payment_id, nonce }) => { + invoice.verify_using_payer_data(payment_id, nonce, expanded_key, secp_ctx) }, _ => Err(()), } @@ -10712,13 +10712,17 @@ where R::Target: Router, L::Target: Logger, { - fn handle_message(&self, message: OffersMessage, context: OffersContext, responder: Option) -> ResponseInstruction { + fn handle_message( + &self, message: OffersMessage, context: Option, responder: Option, + ) -> ResponseInstruction { let secp_ctx = &self.secp_ctx; let expanded_key = &self.inbound_payment_key; let abandon_if_payment = |context| { match context { - OffersContext::OutboundPayment { payment_id, .. } => self.abandon_payment(payment_id), + Some(OffersContext::OutboundPayment { payment_id, .. }) => { + self.abandon_payment(payment_id) + }, _ => {}, } }; @@ -10731,8 +10735,8 @@ where }; let nonce = match context { - OffersContext::Unknown {} if invoice_request.metadata().is_some() => None, - OffersContext::InvoiceRequest { nonce } => Some(nonce), + None if invoice_request.metadata().is_some() => None, + Some(OffersContext::InvoiceRequest { nonce }) => Some(nonce), _ => return ResponseInstruction::NoResponse, }; @@ -10827,7 +10831,7 @@ where } }, OffersMessage::Invoice(invoice) => { - let payment_id = match self.verify_bolt12_invoice(&invoice, &context) { + let payment_id = match self.verify_bolt12_invoice(&invoice, context.as_ref()) { Ok(payment_id) => payment_id, Err(()) => return ResponseInstruction::NoResponse, }; @@ -10888,7 +10892,7 @@ where }, OffersMessage::InvoiceError(invoice_error) => { let payment_hash = match context { - OffersContext::InboundPayment { payment_hash } => Some(payment_hash), + Some(OffersContext::InboundPayment { payment_hash }) => Some(payment_hash), _ => None, }; let logger = WithContext::from(&self.logger, None, None, payment_hash); diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 627fc8126..a7fc92f52 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -1099,9 +1099,9 @@ fn pays_bolt12_invoice_asynchronously() { assert_eq!(path.introduction_node, IntroductionNode::NodeId(alice_id)); } - assert!(bob.node.send_payment_for_bolt12_invoice(&invoice, &context).is_ok()); + assert!(bob.node.send_payment_for_bolt12_invoice(&invoice, context.as_ref()).is_ok()); assert_eq!( - bob.node.send_payment_for_bolt12_invoice(&invoice, &context), + bob.node.send_payment_for_bolt12_invoice(&invoice, context.as_ref()), Err(Bolt12PaymentError::DuplicateInvoice), ); @@ -1112,7 +1112,7 @@ fn pays_bolt12_invoice_asynchronously() { expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); assert_eq!( - bob.node.send_payment_for_bolt12_invoice(&invoice, &context), + bob.node.send_payment_for_bolt12_invoice(&invoice, context.as_ref()), Err(Bolt12PaymentError::DuplicateInvoice), ); @@ -1121,7 +1121,7 @@ fn pays_bolt12_invoice_asynchronously() { } assert_eq!( - bob.node.send_payment_for_bolt12_invoice(&invoice, &context), + bob.node.send_payment_for_bolt12_invoice(&invoice, context.as_ref()), Err(Bolt12PaymentError::UnexpectedInvoice), ); } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 803a89b42..3be4d287a 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -144,7 +144,7 @@ impl OnionMessageHandler for IgnoringMessageHandler { } impl OffersMessageHandler for IgnoringMessageHandler { - fn handle_message(&self, _message: OffersMessage, _context: OffersContext, _responder: Option) -> ResponseInstruction { + fn handle_message(&self, _message: OffersMessage, _context: Option, _responder: Option) -> ResponseInstruction { ResponseInstruction::NoResponse } } diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index 16e62bf33..ad6fe7d99 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -76,7 +76,7 @@ impl Drop for MessengerNode { struct TestOffersMessageHandler {} impl OffersMessageHandler for TestOffersMessageHandler { - fn handle_message(&self, _message: OffersMessage, _context: OffersContext, _responder: Option) -> ResponseInstruction { + fn handle_message(&self, _message: OffersMessage, _context: Option, _responder: Option) -> ResponseInstruction { ResponseInstruction::NoResponse } } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index 7c7cd2610..b14210db4 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -16,7 +16,7 @@ use bitcoin::hashes::sha256::Hash as Sha256; use bitcoin::secp256k1::{self, PublicKey, Scalar, Secp256k1, SecretKey}; use crate::blinded_path::{BlindedPath, IntroductionNode, NextMessageHop, NodeIdLookUp}; -use crate::blinded_path::message::{advance_path_by_one, ForwardNode, ForwardTlvs, MessageContext, OffersContext, ReceiveTlvs}; +use crate::blinded_path::message::{advance_path_by_one, ForwardNode, ForwardTlvs, MessageContext, ReceiveTlvs}; use crate::blinded_path::utils; use crate::events::{Event, EventHandler, EventsProvider, ReplayEvent}; use crate::sign::{EntropySource, NodeSigner, Recipient}; @@ -1514,8 +1514,8 @@ where match message { ParsedOnionMessageContents::Offers(msg) => { let context = match context { - None => OffersContext::Unknown {}, - Some(MessageContext::Offers(context)) => context, + None => None, + Some(MessageContext::Offers(context)) => Some(context), Some(MessageContext::Custom(_)) => { debug_assert!(false, "Shouldn't have triggered this case."); return diff --git a/lightning/src/onion_message/offers.rs b/lightning/src/onion_message/offers.rs index a8f43c2d2..6884ca77e 100644 --- a/lightning/src/onion_message/offers.rs +++ b/lightning/src/onion_message/offers.rs @@ -45,7 +45,9 @@ pub trait OffersMessageHandler { /// The returned [`OffersMessage`], if any, is enqueued to be sent by [`OnionMessenger`]. /// /// [`OnionMessenger`]: crate::onion_message::messenger::OnionMessenger - fn handle_message(&self, message: OffersMessage, context: OffersContext, responder: Option) -> ResponseInstruction; + fn handle_message( + &self, message: OffersMessage, context: Option, responder: Option, + ) -> ResponseInstruction; /// Releases any [`OffersMessage`]s that need to be sent. /// -- 2.39.5