Parameterize the OnionMessageContents in message handler traits
#3261
Conversation
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
Our onion message handlers generally have one or more methods which return either a `ResponseInstruction` or a `PendingOnionMessage` parameterized with the expected message type (enum) of the message handler. This is generally fine, there's not much reason for a message handler of one type to return a response of a different type, but there's also not much reason to restrict it from doing so, either. Sadly, that restriction is also impossible to represent in our bindings - our bindings concretize all structs, enums, and traits into a single concrete instance with generics set to our concrete trait instances (which hold a jump table). This prevents us from having multiple instances of `ResponseInstruction` or `PendingOnionMessage` structs for different message types. Instead, we simply add an associated type to our onion message handlers, allowing them to return any type they want (or for the bindings to make them dynamic on the internal jump table but static at compile-time).
|
Hmm, actually this isn't sufficient for bindings on its own, we'll still fail because we have trait implementations in-crate that return types other than a generic one...instead I think we'll need to |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3261 +/- ##
=======================================
Coverage 89.80% 89.81%
=======================================
Files 125 125
Lines 102867 102867
Branches 102867 102867
=======================================
+ Hits 92381 92387 +6
- Misses 7765 7766 +1
+ Partials 2721 2714 -7 ☔ View full report in Codecov by Sentry. |
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.
Our onion message handlers generally have one or more methods which return either a
ResponseInstructionor aPendingOnionMessageparameterized with the expected message type (enum) of the message handler. This is generally fine, there's not much reason for a message handler of one type to return a response of a different type, but there's also not much reason to restrict it from doing so, either.Sadly, that restriction is also impossible to represent in our bindings - our bindings concretize all structs, enums, and traits into a single concrete instance with generics set to our concrete trait instances (which hold a jump table). This prevents us from having multiple instances of
ResponseInstructionorPendingOnionMessagestructs for different message types.Instead, we simply add an associated type to our onion message handlers, allowing them to return any type they want (or for the bindings to make them dynamic on the internal jump table but static at compile-time).