This repository has been archived by the owner on Jan 6, 2025. It is now read-only.
Drop APeerManager dependency and replace with process_msgs_callback
#73
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
tnull
force-pushed
the
2023-12-drop-apeermanager-dependency
branch
5 times, most recently
from
dc3e5f4 to
8c5d552
Compare
Previously, `LiquidityManager` (or at least its internal message queue) was holding a reference to `PeerManager`/`impl APeerManager` in order to be able to trigger message processing after new messages were enqueued. However, this had two major drawbacks: firstly, it introduced an ugly (and hard to resolve) cycle in the type definitions as `PeerManager` depends on `CustomMessageHandler`, which in this case is `LiquidityManager`, itself depending on said `PeerManager` reference. Secondly, it introduced the complex `PeerManager` LDK object to the LSPS implementation which is otherwise not that depedent on LDK's internal types. To resolve these issues, we heere replace the dependency on `PeerManager`/`impl APeerManager` with a simple generic callback that will be called every time new messages are enqueued.
tnull
force-pushed
the
2023-12-drop-apeermanager-dependency
branch
from
8c5d552 to
64570d2
Compare
This was referenced Dec 20, 2023
johncantrell97
approved these changes
Jan 2, 2024
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #72.
Previously,
LiquidityManager(or at least its internal message queue) was holding a reference toPeerManager/impl APeerManagerin order to be able to trigger message processing after new messages were enqueued.However, this had two major drawbacks: firstly, it introduced an ugly (and hard to resolve) cycle in the type definitions as
PeerManagerdepends onCustomMessageHandler, which in this case isLiquidityManager, itself depending on saidPeerManagerreference. Secondly, it introduced the complexPeerManagerLDK object to the LSPS implementation which is otherwise not that depedent on LDK's internal types.To resolve these issues, we heere replace the dependency on
PeerManager/impl APeerManagerwith a simple generic callback that will be called every time new messages are enqueued.Looking for feedback here. As a second step we could consider dropping the
MessageQueuetrait which was introduced as part of the prior refactor that moved theAPeerManagerbound from each individual message handler to theDefaultMessageQueue.