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

swarm/src/handler/multi.rs: MultiHandler isn't used anywhere inside rust-libp2p. Maybe get rid of it? #3171

Closed
efarg opened this issue Nov 26, 2022 · 5 comments

Comments

@efarg
Copy link
Contributor

efarg commented Nov 26, 2022

Description

Currently MultiHandler isn't used anywhere inside the project. Is it possible to remove or modify it?

Motivation

error[E0271]: expected `std::vec::IntoIter<IndexedProtoName>` to be an iterator that yields `ProtocolName`, but it yields `IndexedProtoName`
   --> swarm/src/handler/multi.rs:458:21
    |
458 |     type InfoIter = std::vec::IntoIter<IndexedProtoName>;
    |                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `ProtocolName`, found struct `IndexedProtoName`

The error above is what brought thomaseizinger's attention to this issue. For further understanding, see comments: #2831 (comment) and #2966 (comment) and the replies by @thomaseizinger on the latter.

This happens due to changing the current definition of UpgradeInfoSend from this

pub trait UpgradeInfoSend: Send + 'static {
/// Equivalent to [`UpgradeInfo::Info`](upgrade::UpgradeInfo::Info).
type Info: upgrade::ProtocolName + Clone + Send + 'static;
/// Equivalent to [`UpgradeInfo::InfoIter`](upgrade::UpgradeInfo::InfoIter).
type InfoIter: Iterator<Item = Self::Info> + Send + 'static;
/// Equivalent to [`UpgradeInfo::protocol_info`](upgrade::UpgradeInfo::protocol_info).
fn protocol_info(&self) -> Self::InfoIter;
}

to this

pub trait UpgradeInfoSend: Send + 'static {
    /// Equivalent to [`UpgradeInfo::InfoIter`](upgrade::UpgradeInfo::InfoIter).
    type InfoIter: Iterator<Item = upgrade::ProtocolName> + Send + 'static;

    /// Equivalent to [`UpgradeInfo::protocol_info`](upgrade::UpgradeInfo::protocol_info).
    fn protocol_info(&self) -> Self::InfoIter;
}

Are you planning to do it yourself in a pull request?

Yes.

@thomaseizinger
Copy link
Contributor

MultiHandler has been here ever since I started working on rust-libp2p but I personally never had a use for it.

@mxinden Do you know of any users?

@mxinden
Copy link
Member

mxinden commented Nov 27, 2022

It is used in Substrate.

https://github.com/paritytech/substrate/blob/a92005a5092ebe6a636c96dcae814f9a378f5940/client/network/src/request_responses.rs#L49

I still need to read up on the above. Thanks @efarg for the detailed issue.

@thomaseizinger
Copy link
Contributor

Okay, thanks for the insight. I think I already know how to fix the issue in #2966.

@efarg Did you push all your code? Then I might just push a quick patch on top fixing MultiHandler.

@efarg
Copy link
Contributor Author

efarg commented Nov 28, 2022

@thomaseizinger yes, the code that would give the multihandler error is all here https://github.com/efarg/rust-libp2p/tree/2966

@thomaseizinger
Copy link
Contributor

I posted a patch onto the PR that fixes the multihandler issue: #2966 (comment)

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

No branches or pull requests

3 participants