lightningdevkit / rust-lightning

A highly modular Bitcoin Lightning library written in Rust. It's rust-lightning, not Rusty's Lightning!
Other
1.15k stars 363 forks source link

Async payments message encoding and prefactor #3125

Closed valentinewallace closed 3 months ago

valentinewallace commented 3 months ago

Support encoding async payments messages HeldHtlcAvailable and ReleaseHeldHtlc and add the necessary traits and some fields for handling these messages. The actual flow of the protocol is not yet supported.

Helps address #2298.

valentinewallace commented 3 months ago

Let me know if we want to hold off on this or parts of it if we want more of the follow-on work up for review first!

codecov-commenter commented 3 months ago

Codecov Report

Attention: Patch coverage is 44.00000% with 42 lines in your changes missing coverage. Please review.

Project coverage is 90.33%. Comparing base (07f3380) to head (1a3dfdb). Report is 4 commits behind head on main.

Files Patch % Lines
lightning/src/onion_message/async_payments.rs 0.00% 41 Missing :warning:
lightning/src/ln/channelmanager.rs 87.50% 0 Missing and 1 partial :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3125 +/- ## ========================================== + Coverage 89.92% 90.33% +0.41% ========================================== Files 121 121 Lines 99172 101244 +2072 Branches 99172 101244 +2072 ========================================== + Hits 89180 91461 +2281 + Misses 7391 7222 -169 + Partials 2601 2561 -40 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

valentinewallace commented 3 months ago

Because we probably don't want async payments variants in public enums OffersMessage and ParsedOnionMessageContents yet, I added a commit cfg-gating the module and the new variants.

valentinewallace commented 3 months ago

I squashed in re-adding the default implementations and docs updates, hopefully that's okay.

valentinewallace commented 3 months ago

Oops, just noticed a CI issue. Will push momentarily.

valentinewallace commented 3 months ago

Diff since @TheBlueMatt's last ack is docs tweaks and removing default method implementations, so going to land this and happy to address anything else in follow-up.

valentinewallace commented 3 months ago

Diff since Matt's ack for reference:


diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs
index ccd17f42f..ba76815af 100644
--- a/fuzz/src/onion_message.rs
+++ b/fuzz/src/onion_message.rs
@@ -12,7 +12,9 @@ use lightning::ln::msgs::{self, DecodeError, OnionMessageHandler};
 use lightning::ln::script::ShutdownScript;
 use lightning::offers::invoice::UnsignedBolt12Invoice;
 use lightning::offers::invoice_request::UnsignedInvoiceRequest;
-use lightning::onion_message::async_payments::{AsyncPaymentsMessage, AsyncPaymentsMessageHandler};
+use lightning::onion_message::async_payments::{
+   AsyncPaymentsMessage, AsyncPaymentsMessageHandler, HeldHtlcAvailable, ReleaseHeldHtlc,
+};
 use lightning::onion_message::messenger::{
    CustomOnionMessageHandler, Destination, MessageRouter, OnionMessagePath, OnionMessenger,
    PendingOnionMessage, Responder, ResponseInstruction,
@@ -110,7 +112,14 @@ impl OffersMessageHandler for TestOffersMessageHandler {

 struct TestAsyncPaymentsMessageHandler {}

-impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {}
+impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {
+   fn held_htlc_available(
+       &self, _message: HeldHtlcAvailable, _responder: Option<Responder>,
+   ) -> ResponseInstruction<ReleaseHeldHtlc> {
+       ResponseInstruction::NoResponse
+   }
+   fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {}
+}

 #[derive(Debug)]
 struct TestCustomMessage {}
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index a11748bc0..fe3320a13 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -66,7 +66,6 @@ use crate::offers::invoice_request::{DerivedPayerId, InvoiceRequestBuilder};
 use crate::offers::offer::{Offer, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
 use crate::offers::refund::{Refund, RefundBuilder};
-use crate::onion_message::async_payments::AsyncPaymentsMessage;
 use crate::onion_message::messenger::{new_pending_onion_message, Destination, MessageRouter, PendingOnionMessage, Responder, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::sign::{EntropySource, NodeSigner, Recipient, SignerProvider};
@@ -1848,8 +1847,6 @@ where
 //
 // `pending_offers_messages`
 //
-// `pending_async_payments_messages`
-//
 // `total_consistency_lock`
 //  |
 //  |__`forward_htlcs`
@@ -2102,8 +2099,6 @@ where
    needs_persist_flag: AtomicBool,

    pending_offers_messages: Mutex<Vec<PendingOnionMessage<OffersMessage>>>,
-   #[allow(unused)]
-   pending_async_payments_messages: Mutex<Vec<PendingOnionMessage<AsyncPaymentsMessage>>>,

    /// Tracks the message events that are to be broadcasted when we are connected to some peer.
    pending_broadcast_messages: Mutex<Vec<MessageSendEvent>>,
@@ -2898,7 +2893,6 @@ where
            funding_batch_states: Mutex::new(BTreeMap::new()),

            pending_offers_messages: Mutex::new(Vec::new()),
-           pending_async_payments_messages: Mutex::new(Vec::new()),
            pending_broadcast_messages: Mutex::new(Vec::new()),

            last_days_feerates: Mutex::new(VecDeque::new()),
@@ -12087,7 +12081,6 @@ where
            funding_batch_states: Mutex::new(BTreeMap::new()),

            pending_offers_messages: Mutex::new(Vec::new()),
-           pending_async_payments_messages: Mutex::new(Vec::new()),

            pending_broadcast_messages: Mutex::new(Vec::new()),

diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs
index 4dfcd0e77..9a026d709 100644
--- a/lightning/src/ln/peer_handler.rs
+++ b/lightning/src/ln/peer_handler.rs
@@ -28,7 +28,7 @@ use crate::util::ser::{VecWriter, Writeable, Writer};
 use crate::ln::peer_channel_encryptor::{PeerChannelEncryptor, NextNoiseStep, MessageBuf, MSG_BUF_ALLOC_SIZE};
 use crate::ln::wire;
 use crate::ln::wire::{Encode, Type};
-use crate::onion_message::async_payments::AsyncPaymentsMessageHandler;
+use crate::onion_message::async_payments::{AsyncPaymentsMessageHandler, HeldHtlcAvailable, ReleaseHeldHtlc};
 use crate::onion_message::messenger::{CustomOnionMessageHandler, PendingOnionMessage, Responder, ResponseInstruction};
 use crate::onion_message::offers::{OffersMessage, OffersMessageHandler};
 use crate::onion_message::packet::OnionMessageContents;
@@ -149,7 +149,14 @@ impl OffersMessageHandler for IgnoringMessageHandler {
        ResponseInstruction::NoResponse
    }
 }
-impl AsyncPaymentsMessageHandler for IgnoringMessageHandler {}
+impl AsyncPaymentsMessageHandler for IgnoringMessageHandler {
+   fn held_htlc_available(
+       &self, _message: HeldHtlcAvailable, _responder: Option<Responder>,
+   ) -> ResponseInstruction<ReleaseHeldHtlc> {
+       ResponseInstruction::NoResponse
+   }
+   fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {}
+}
 impl CustomOnionMessageHandler for IgnoringMessageHandler {
    type CustomMessage = Infallible;
    fn handle_custom_message(&self, _message: Self::CustomMessage, _responder: Option<Responder>) -> ResponseInstruction<Self::CustomMessage> {
diff --git a/lightning/src/onion_message/async_payments.rs b/lightning/src/onion_message/async_payments.rs
index c7688ec2a..f5953c636 100644
--- a/lightning/src/onion_message/async_payments.rs
+++ b/lightning/src/onion_message/async_payments.rs
@@ -25,21 +25,17 @@ const RELEASE_HELD_HTLC_TLV_TYPE: u64 = 74;
 ///
 /// [`OnionMessage`]: crate::ln::msgs::OnionMessage
 pub trait AsyncPaymentsMessageHandler {
-   /// Handles a [`HeldHtlcAvailable`] message. The [`ReleaseHeldHtlc`] response, if any, is enqueued
-   /// to be sent by [`OnionMessenger`].
-   ///
-   /// [`OnionMessenger`]: crate::onion_message::messenger::OnionMessenger
+   /// Handle a [`HeldHtlcAvailable`] message. A [`ReleaseHeldHtlc`] should be returned to release
+   /// the held funds.
    fn held_htlc_available(
-       &self, _message: HeldHtlcAvailable, _responder: Option<Responder>,
-   ) -> ResponseInstruction<ReleaseHeldHtlc> {
-       ResponseInstruction::NoResponse
-   }
+       &self, message: HeldHtlcAvailable, responder: Option<Responder>,
+   ) -> ResponseInstruction<ReleaseHeldHtlc>;

-   /// Handles a [`ReleaseHeldHtlc`] message. If authentication of the message succeeds, an HTLC will
-   /// be released to the corresponding payee.
-   fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {}
+   /// Handle a [`ReleaseHeldHtlc`] message. If authentication of the message succeeds, an HTLC
+   /// should be released to the corresponding payee.
+   fn release_held_htlc(&self, message: ReleaseHeldHtlc);

-   /// Releases any [`AsyncPaymentsMessage`]s that need to be sent.
+   /// Release any [`AsyncPaymentsMessage`]s that need to be sent.
    ///
    /// Typically, this is used for messages initiating an async payment flow rather than in response
    /// to another message.
@@ -48,7 +44,7 @@ pub trait AsyncPaymentsMessageHandler {
        vec![]
    }

-   /// Releases any [`AsyncPaymentsMessage`]s that need to be sent.
+   /// Release any [`AsyncPaymentsMessage`]s that need to be sent.
    ///
    /// Typically, this is used for messages initiating a payment flow rather than in response to
    /// another message.
@@ -89,7 +85,8 @@ pub struct HeldHtlcAvailable {
 /// Releases the HTLC corresponding to an inbound [`HeldHtlcAvailable`] message.
 #[derive(Clone, Debug)]
 pub struct ReleaseHeldHtlc {
-   /// Used to release the HTLC held upstream.
+   /// Used to release the HTLC held upstream if it matches the corresponding
+   /// [`HeldHtlcAvailable::payment_release_secret`].
    pub payment_release_secret: [u8; 32],
 }

diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs
index 07a1d1773..9a2dc36f8 100644
--- a/lightning/src/onion_message/functional_tests.rs
+++ b/lightning/src/onion_message/functional_tests.rs
@@ -19,7 +19,7 @@ use crate::routing::test_utils::{add_channel, add_or_update_node};
 use crate::sign::{NodeSigner, Recipient};
 use crate::util::ser::{FixedLengthReader, LengthReadable, Writeable, Writer};
 use crate::util::test_utils;
-use super::async_payments::AsyncPaymentsMessageHandler;
+use super::async_payments::{AsyncPaymentsMessageHandler, HeldHtlcAvailable, ReleaseHeldHtlc};
 use super::messenger::{CustomOnionMessageHandler, DefaultMessageRouter, Destination, OnionMessagePath, OnionMessenger, PendingOnionMessage, Responder, ResponseInstruction, SendError, SendSuccess};
 use super::offers::{OffersMessage, OffersMessageHandler};
 use super::packet::{OnionMessageContents, Packet};
@@ -83,7 +83,14 @@ impl OffersMessageHandler for TestOffersMessageHandler {

 struct TestAsyncPaymentsMessageHandler {}

-impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {}
+impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler {
+   fn held_htlc_available(
+       &self, _message: HeldHtlcAvailable, _responder: Option<Responder>,
+   ) -> ResponseInstruction<ReleaseHeldHtlc> {
+       ResponseInstruction::NoResponse
+   }
+   fn release_held_htlc(&self, _message: ReleaseHeldHtlc) {}
+}

 #[derive(Clone, Debug, PartialEq)]
 enum TestCustomMessage {