Skip to content
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

Onion Message Responder creates a circular reference #3178

Closed
TheBlueMatt opened this issue Jul 13, 2024 · 0 comments · Fixed by #3263
Closed

Onion Message Responder creates a circular reference #3178

TheBlueMatt opened this issue Jul 13, 2024 · 0 comments · Fixed by #3263

Comments

@TheBlueMatt
Copy link
Collaborator

To deal with #2882 we created a Responder, which message handlers can respond() on, then passing the response object back to the OnionMessager. Sadly, this isn't actually really workable as it creates a circular reference - the OnionMessager is typed on the message handler, and passes the messages to it to be handled. But to be able to respond async, we also expect the message handler to hold a reference to the OnionMessenger (and presumably be typed on it). While the type issue itself isn't as big a deal because we can use AOnionMessenger to avoid the type-inexpressibility, we cannot construct the two objects with a circular reference.

For the async gossip verification, we solved this by letting the gossiper take an Optional verifier, then let the user call set_verifier on the gossiper. This would mean the OnionMessenger would need to start taking Optional handlers (rather than IgnoringMessageHandler for no handler).

The other option would be to use the message handler traits, adding a PendingOnionMessage variant for a response or adding a new method to fetch responses ala release_pending_messages. The first here would be my preference.

TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 21, 2024
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the first step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `CustomMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

By unifying the response type with the outbound message queue type,
this also fixes lightningdevkit#3178.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 21, 2024
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `OffersMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

Because `ChannelManager` needs to actually control the reply path
set in individual messages, however, we have to halfway this patch,
adding a new `MessageSendInstructions` variant that allows
specifying the `reply_path` explicitly. Still, because other message
handlers are moving this way, its nice to be consistent.

By unifying the response type with the outbound message queue type,
this also fixes lightningdevkit#3178.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 21, 2024
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `AsyncPaymentsMessageHandler`. Better, we can also
drop the `c_bindings`-specific message queue variant, unifying the
APIs.

Here we also drop `PendingOnionMessage` entirely.

By unifying the response type with the outbound message queue type,
this also fixes lightningdevkit#3178.
@TheBlueMatt TheBlueMatt linked a pull request Aug 21, 2024 that will close this issue
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 21, 2024
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the first step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `CustomMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

By unifying the response type with the outbound message queue type,
this also fixes lightningdevkit#3178.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 21, 2024
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `OffersMessageHandler`. Better, we can also drop
the `c_bindings`-specific message queue variant, unifying the APIs.

Because `ChannelManager` needs to actually control the reply path
set in individual messages, however, we have to halfway this patch,
adding a new `MessageSendInstructions` variant that allows
specifying the `reply_path` explicitly. Still, because other message
handlers are moving this way, its nice to be consistent.

By unifying the response type with the outbound message queue type,
this also fixes lightningdevkit#3178.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 21, 2024
Now that the `MessageRouter` can `create_blinded_paths` forcing
callers of the `OnionMessenger` to provide it with a reply path up
front is unnecessary complexity, doubly so in message handlers.

Here we take the next step towards untangling that, moving from
`PendingOnionMessage` to `MessageSendInstructions` for the outbound
message queue in `AsyncPaymentsMessageHandler`. Better, we can also
drop the `c_bindings`-specific message queue variant, unifying the
APIs.

Here we also drop `PendingOnionMessage` entirely.

By unifying the response type with the outbound message queue type,
this also fixes lightningdevkit#3178.
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this issue Aug 22, 2024
In order to allow onion message handlers to reply asynchronously
without introducing a circular dependency graph, the message
handlers need to be able to send replies described by
`MessageSendInstructions`. This allows them to send replies via the
normal message queuing (i.e. without making a function call to
`OnionMessenger`).

Here we enable that by adding a `MessageSendInstructions::ForReply`
variant which holds `ReplyInstruction`s.

Fixes lightningdevkit#3178
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 a pull request may close this issue.

1 participant