Introduce OffersMessageFlow #3412
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shaavan
force-pushed
the
offersmessageflow
branch
from
0972b33 to
e038951
Compare
jkczyz
reviewed
Nov 19, 2024
| return Err(Bolt12SemanticError::UnsupportedChain); | ||
| } | ||
|
|
||
| let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); |
There was a problem hiding this comment.
@TheBlueMatt Do you know why we need the PersistenceNotifierGuard here? This code sends an invoice for a refund (i.e., an inbound payment). IIUC, we don't persist pending_offers_messages, so it seems we don't need it.
shaavan
force-pushed
the
offersmessageflow
branch
from
e038951 to
e1c3863
Compare
shaavan
force-pushed
the
offersmessageflow
branch
from
e1c3863 to
d5c244a
Compare
shaavan
force-pushed
the
offersmessageflow
branch
from
d5c244a to
c07e7f6
Compare
|
Updated from pr3412.04 to pr3412.05 (diff): Addressed @jkczyz's (offline) comments. Changes:
|
shaavan
force-pushed
the
offersmessageflow
branch
2 times, most recently
from
4911f9f to
9d0c511
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3412 +/- ##
==========================================
+ Coverage 89.72% 89.78% +0.05%
==========================================
Files 130 131 +1
Lines 107614 108190 +576
Branches 107614 108190 +576
==========================================
+ Hits 96559 97137 +578
+ Misses 8660 8659 -1
+ Partials 2395 2394 -1 ☔ View full report in Codecov by Sentry. |
shaavan
commented
Dec 9, 2024
shaavan
force-pushed
the
offersmessageflow
branch
from
9d0c511 to
b3c54ee
Compare
|
Updated from pr3412.06 to pr3412.07 (diff): Changes:
|
To decouple offers and onion message-related code from `ChannelManager`, this commit introduces the `message_received` function in `Offer/OnionMessageHandler`. Currently, the function focuses on handling the retry logic for `InvoiceRequest` messages. Moving this responsibility ensures a cleaner separation of concerns and sets the foundation for managing offers/onion messages directly within the appropriate handler.
Since `ChannelMessageHandler`'s `message_received` function was solely used for handling invoice request retries. This function has been removed from `ChannelMessageHandler`, and the relevant code has been migrated to `OffersMessageHandler`'s `message_received`. This ensures invoice request retries are now handled in the appropriate context.
This commit temporarily removes support for async BOLT12 message handling to enable a smoother transition in the upcoming refactor. The current implementation of async handling is abrupt, as it requires delving into the synchronous case and generating an event mid-flow based on the `manual_handling` flag. This approach adds unnecessary complexity and coupling. A later commit will introduce a new struct, `OffersMessageFlow`, designed to handle and create offer messages. This new struct will support async handling in a more structured way by allowing users to implement a parameterized trait for asynchronous message handling. Removing the existing async support now ensures a cleaner and more seamless migration of offer-related code from `ChannelManager` to `OffersMessageFlow`.
This commit introduces a new struct, `OffersMessageFlow`, to extract all offers message-related code out of `ChannelManager`. By moving this logic into a dedicated struct, it creates a foundation for separating responsibilities and sets up a base for further code restructuring in subsequent commits.
A new trait, `OffersMessageCommons`, is introduced to encapsulate functions that are commonly used by both BOLT12-related functionality and other parts of `ChannelManager`. This enables a clean transition of BOLT12 code to `OffersMessageFlow` by moving shared functions into the new trait, ensuring they remain accessible to both `ChannelManager` and the refactored BOLT12 code.
This commit introduces the `OffersMessageHandler` implementation for `OffersMessageFlow`, enabling direct access to offer-specific functionality through `OffersMessageFlow`. With `OffersMessageFlow` now serving as the source of `OffersMessageHandler` implementation, the implementation in `ChannelManager` is no longer needed and has been safely removed.
- This commit introduces a new struct, `AnOffersMessageFlow`, which generically implements `OffersMessageFlow`. - In subsequent commits, this struct will be utilized for documentation purposes.
- The _persistence_guard here was not serving any purpose, and hence can be removed in this refactoring.
shaavan
force-pushed
the
offersmessageflow
branch
from
b3c54ee to
3ba8ba5
Compare
1. This allow removing an extra function from commons, simplifying the flow trait.
shaavan
force-pushed
the
offersmessageflow
branch
from
3ba8ba5 to
189b2aa
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a new data structure to separate the functions and fields related to Offers Message from
ChannelManager.The change ensures a clear separation of responsibilities for BOLT12 messages, improving modularity and maintainability.
TODO: