batteries: Allow wrapping the MessageRouter in ChannelManagerConstructor - #183
Draft
Synesso wants to merge 1 commit into
Draft
batteries: Allow wrapping the MessageRouter in ChannelManagerConstructor#183Synesso wants to merge 1 commit into
Synesso wants to merge 1 commit into
Conversation
ChannelManagerConstructor hardcodes DefaultMessageRouter for onion message path finding, with no way to customize it. Nodes that restrict outbound connections cannot act on Event::ConnectionNeeded, so onion messages (e.g. BOLT 12 invoice requests) to unconnected destinations buffer forever. Such deployments route onion messages through a trusted forwarding peer by prefixing the path with a connected node, which requires substituting the MessageRouter handed to the OnionMessenger and ChannelManager. Add a MessageRouterWrapper hook mirroring the existing RouterWrapper pattern: a nullable constructor parameter whose find_path and create_blinded_paths receive the DefaultMessageRouter the constructor would otherwise have used, so implementations can delegate any case they don't want to customize. Passing null preserves existing behavior exactly. Amp-Thread-ID: https://ampcode.com/threads/T-019fac05-3d6f-75ca-9581-2c17d12954db Co-authored-by: Amp <amp@ampcode.com>
Author
|
The CI failures here are pre-existing (upstream |
This file contains hidden or 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
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.
ChannelManagerConstructorhardcodesDefaultMessageRouterfor onion message path finding, with no way to customize it. This mirrors the existingRouterWrapperhook (which covers the paymentRouter) with aMessageRouterWrapperfor the onionMessageRouter.Motivation
DefaultMessageRouter::find_pathonly returns paths whose first hop is a directly connected peer. Nodes that restrict outbound connections (e.g. for egress-policy reasons) cannot act onEvent::ConnectionNeeded, so onion messages to unconnected destinations — BOLT 12 invoice requests in particular — buffer forever. Such deployments instead route onion messages through a trusted forwarding peer by prefixing the path with a connected node, which requires substituting theMessageRouterhanded to theOnionMessenger(andChannelManager).Today the only options are vendoring a modified copy of
ChannelManagerConstructoror reconstructingChannelManager/OnionMessenger/PeerManager/BackgroundProcessorby hand: themsg_routerfield is private, concretely typedDefaultMessageRouter, and theOnionMessengerbuilt inchain_sync_completedis not exposed. (We are currently carrying such a vendored copy.)Design
Follows
RouterWrapperas closely as possible:MessageRouterWrapperinterface withfind_pathandcreate_blinded_paths, each receiving theDefaultMessageRouterthe constructor would otherwise have used, so implementations can delegate for any case they don't want to customize.@Nullable MessageRouterWrapperparameter, immediately afterrouter_wrapper.nullpreserves existing behavior exactly.msg_routerfield becomesMessageRouter(it was only ever consumed viaas_MessageRouter()).Like the original
RouterWrapperaddition, this changes the public constructor signatures. Happy to add overloads preserving the old signatures instead if that's preferred.