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/lib: Refactor trait usage #2182

Merged
merged 3 commits into from
Aug 9, 2021
Merged

Conversation

mxinden
Copy link
Member

@mxinden mxinden commented Aug 6, 2021

  • Removes the Swarm type alias, renaming ExpandedSwarm to Swarm.
  • Remove TInEvent, TOutEvent and THandler trait parameters on
    Swarm, instead deriving them through TBehaviour. Move derive logic
    to separate type aliases.
  • Simplify trait bounds on Swarm main impl and Stream impl.

ExpandedSwarm was introduced in #1046.

Turns Swarm into a type Swarm<...> = ExpandedSwarm<...> where ExpandedSwarm has a lot more template parameters.

Unfortunately Rust still has a lot of limitations when it comes to bounds. I got blocked in Substrate because of some combination of requirements that wasn't possible to satisfy in a struct declaration. This PR makes it possible to use the swarm in ways that weren't possible by removing the mandatory where TBehaviour: NetworkBehaviour requirement from Swarm.

//CC @kpp and @tomaka to make sure this is not breaking Substrate.

- Removes the `Swarm` type alias, renaming `ExpandedSwarm` to `Swarm`.
- Remove `TInEvent`, `TOutEvent` and `THandler` trait parameters on
`Swarm`, instead deriving them through `TBehaviour`. Move derive logic
to separate type aliases.
- Simplify trait bounds on `Swarm` main `impl` and `Stream` `impl`.
@tomaka
Copy link
Member

tomaka commented Aug 6, 2021

There wouldn't be any problem with Substrate. This was at a time where we were trying to give the users the possibility to plug their own NetworkBehaviour into Substrate, which is an idea we've abandoned.

As far as I remember, the problem is that you can write this:

struct Foo<T>;
impl<T, U> Foo<T> where T: NetworkBehaviour, T::Output: SomeTrait<U> {}

But you can't enforce the equivalent directly in the struct definition:

struct Foo<T, U> where T: NetworkBehaviour, T::Output: SomeTrait<U> {
    // would need to use U here, contrary to the first example where U isn't part of the struct definition
}

Unfortunately I can't remember the reason why we need to enforce NetworkBehaviour on the struct definition itself.

Copy link
Contributor

@thomaseizinger thomaseizinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very much in favour of this!

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 this pull request may close these issues.

3 participants