Skip to content

Introduce SendOnlyMessageHandler trait #3922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2025

Conversation

tnull
Copy link
Contributor

@tnull tnull commented Jul 10, 2025

Previously, we introduced a new send_only_message_handler: BaseMessageHandler field to MessageHandler, which we expect to only ever be implemented by ChainMonitor.

However, given that more objects implement BaseMessageHandler, the API allowed to plugin different objects, such as ChannelManager, resulting in unexpected behavior.

Here, we resolve this by introducing a new super trait SendOnlyMessageHandler that is only implemented by ChainMonitor.

(cc @adi2011)

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 10, 2025

I've assigned @joostjager as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Copy link
Contributor

@joostjager joostjager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not much to comment on here.

@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

Previously, we introduced a new `send_only_message_handler:
BaseMessageHandler` field to `MessageHandler`, which we expect to only
ever be implemented by `ChainMonitor`.

However, given that more objects implement `BaseMessageHandler`, the API
allowed to plugin different objects, such as `ChannelManager`, resulting
in unexpected behavior.

Here, we resolve this by introducing a new super trait
`SendOnlyMessageHandler` that is only implemented by `ChainMonitor`.
@tnull tnull force-pushed the 2025-07-introduce-send-only-trait branch from 3da1368 to 1e99cdb Compare July 10, 2025 14:18
@tnull
Copy link
Contributor Author

tnull commented Jul 10, 2025

Force-pushed the following fixup:

> git diff-tree -U2 3da136849 1e99cdbdd
diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 8a7febe4c..da5cdf7c9 100644
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -2180,5 +2180,5 @@ pub trait OnionMessageHandler: BaseMessageHandler {
 /// A handler which can only be used to send messages.
 ///
-/// This is mostly implemented by [`ChainMonitor`].
+/// This is implemented by [`ChainMonitor`].
 ///
 /// [`ChainMonitor`]: crate::chain::chainmonitor::ChainMonitor

@tnull
Copy link
Contributor Author

tnull commented Jul 11, 2025

Given we recently decided to more often merge with 1 review for simple changes, I'm going to land this.

@tnull tnull merged commit 9863f29 into lightningdevkit:main Jul 11, 2025
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants