-
Notifications
You must be signed in to change notification settings - Fork 995
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
feat: Add Transport::box_multiplexed
#3313
Conversation
I am planning to move the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very much in favor of this change.
If I read @thomaseizinger comment correctly, he doesn't mind proceeding here.
Can you add a changelog entry?
I would include it in this pull request as one is the reason for the other. Not a strong opinion. |
…eat/box-multiplexed
Happy to if you consider it urgent. It doesn't move in the direction I'd want to because it adds another function to An alternative solution would be to create a That would avoid deprecating something down the line that we only introduced recently. |
This pull request has merge conflicts. Could you please resolve them @elenaf9? 🙏 |
I don't consider this urgent. What would be your preference? I am assuming we could not define this next to |
Introducing a |
Closed because stale. |
Description
Transports like QUIC or WebRTC, that natively multiplex their connections, currently require some extra mapping so that they can be boxed into the
Boxed<(PeerId, StreamMuxerBox)>
type required by the swarm.This PR adds a
box_multiplexed
convenience function to theTransport
trait for boxing multiplexed transports.Such a function already existed for the
Multiplexed
type that is constructed when a non-multiplexed transport is upgrade with a muxer (e.g. when using tcp+yamux).box_multiplexed
is essentially the same, with some minor changes to also allow boxingOrTransport
s (when both inner transports are multiplexed).Notes
Opening as a draft to hear what folks think about it.
I know there have been discussions (#2829, #3179) in general about finding better abstractions for transports and the interface for using them in
libp2p-swarm
.The new function won't solve this, but rather is an intermediary solution until we agree on a new API. Given that designing and implementing such a new API may take some time, I think its worth having this function to ease the usage of transports like QUIC and WebRTC in the meantime.
Links to any relevant issues
Open Questions
If we decide to add this function, I would deprecate
Multiplexed::boxed
in favor ofTransport::box_multiplexed
in a follow up PR. Any objections?Change checklist
I have added tests that prove my fix is effective or that my feature works