Void event type for connection limits and allow-block list behaviours.

[shamil-gadelshin]

Libp2p v0.51.3

When we use connection limits or allow-block list behaviours along with #[behaviour(out_event = "Event")] directive:

#[derive(NetworkBehaviour)]
#[behaviour(out_event = "Event")]
#[behaviour(event_process = false)]
pub(crate) struct Behavior {
..
    pub(crate) connection_limits: ConnectionLimitsBehaviour,
    pub(crate) block_list: BlockListBehaviour,
}

we need to specify the Void type for the event:

#[derive(Debug, From)]
pub(crate) enum Event {
..
    VoidEventStub(Void),
}

Otherwise, we get an error: the trait std::convert::From<void::Void> is not implemented for behavior::Event.

It's confusing for a couple of reasons:

• Void type is not exported from libp2p and must be added separately
• we need to specify an additional event stub for both behaviours.

Is there a better option to work with events for connection limits or allow-block list behaviours?

[thomaseizinger]

• Void type is not exported from libp2p and must be added separately

void is > 1.0 so I think it is safe to appear in public APIs and can easily be depended on separately.

Is there a better option to work with events for connection limits or allow-block list behaviours?

Not to my knowledge. Ideally, we would use the never (!) here but that isn't stable yet.

Does it work if you let the derive macro auto-generate the enum? You'll have two Void variants then but those are easily handled in a match using void::unreachable: https://docs.rs/void/1.0.2/void/fn.unreachable.html

[shamil-gadelshin]

void is > 1.0 so I think it is safe to appear in public APIs and can easily be depended on separately.

I see. You seem to prefer a separate third-party crate for such dependencies rather than reexported types.

Does it work if you let the derive macro auto-generate the enum?

We tend to have more control over events` names.

It works either way. Thanks for the answers.

[thomaseizinger]

void is > 1.0 so I think it is safe to appear in public APIs and can easily be depended on separately.

I see. You seem to prefer a separate third-party crate for such dependencies rather than reexported types.

Re-exporting types makes dependency upgrades easier because you can directly access the type in the correct version.

At some point though, this has to stop. We don't re-export tokio or async-std either but they still appear in our public APIs. futures, either and serde are other examples.

Latest once a crate signals stability (> 1.0), I think it is safe to stop re-exporting it and have the user declare a dependency themselves. Otherwise, you end up with lots of your dependencies all re-exporting the same version. That makes the import path redundant which I'd argue is confusing.

Simply put, once a crate is stable, the need for re-exporting goes away. It no longer solves a problem and I like to not have code around that doesn't solve any problems :)

[shamil-gadelshin]

It makes sense. Thanks for the clarification!

[nazar-pc]

Right now Void is a public API of the crate. Re-exporting void makes little sense, but Void must be IMHO. It can change from re-exported to concrete type in the future if necessary, making transition easier for users. It is far less ubiquitous than serde and the likes.

[mxinden]

My concern was never that void crate is used, but rather that it is used in one specific situation, but the type is not re-exported. It would be very low priority for me to do anything about it.

My understanding with never-say-never, given that it doesn't need the match, there is no need for downstream users to explicitly name it, and thus no need for rust-libp2p to re-export it. @thomaseizinger please correct me in case I am missing something.

[shamil-gadelshin]

I could spend some free time on an experiment with never-say-never and one of the behaviors. We could briefly touch this on the next rust-libp2p maintainers call.

[thomaseizinger]

My concern was never that void crate is used, but rather that it is used in one specific situation, but the type is not re-exported. It would be very low priority for me to do anything about it.

My understanding with never-say-never, given that it doesn't need the match, there is no need for downstream users to explicitly name it, and thus no need for rust-libp2p to re-export it. @thomaseizinger please correct me in case I am missing something.

Yes exactly, that is the hypothesis.

[thomaseizinger]

I could spend some free time on an experiment with never-say-never and one of the behaviors. We could briefly touch this on the next rust-libp2p maintainers call.

That would be great! The next one will be on the upcoming Tuesday.

[shamil-gadelshin]

Following the maintainers call: shamil-gadelshin@c971dc6 @thomaseizinger

[thomaseizinger]

I just realized how we can fix your original issue @shamil-gadelshin !

We can extend the derive-macro to always generate the following impl if out_event is specified.

impl From<#derive_prelude::Void> for #out_event {
	fn from(v: #derive_prelude::Void) -> Self {
        void::unreachable(v)
	}
}

We will also have to add Void to our derive_prelude module in libp2p-swarm. This satisfies the trait-bounds and allows you to omit all From impls for Void.

Are you open to send a PR?

[shamil-gadelshin]

@thomaseizinger Here is my attempt: shamil-gadelshin@d6bb609

The straightforward solution doesn't cover all the cases: custom_out_event_no_type_parameters test fails to compile.

[thomaseizinger]

Can you open a PR please?

[shamil-gadelshin]

Can you open a PR please?

Sure, I'll do that when I have a working version.

[shamil-gadelshin]

On the second look, it seems that a problem with the test lies deeper and originates in the lack of information for the derive_behaviour macro when the event type is specified.
When the event type is not specified (it is generated) we can add the From<Void> conversion using a generics definition provided for the Behaviour struct with the derive attribute it seems we can rely on those generics to fill the impl<..> part of the conversion, on the other hand, when we specify #[behaviour(to_swarm = "FooEvent")] we are not able to get information about generics of this type. We probably could add properties of this attribute like (generics = None | SameAsBehaviour) (#[behaviour(to_swarm = "FooEvent", no_generics)]) but I'm not sure it is worth it. @thomaseizinger Maybe we can discuss it during the next rust maintainer's call.

[thomaseizinger]

I am wondering why you put the #impl_generics block on the impl From block in shamil-gadelshin@d6bb609?

Those are the generics of the behaviour, not the event. What we can do is parse what is in to_swarm as ItemType which gives us access to all generics. We can then use those to correctly emit an impl From for the specified type.

#[derive(NetworkBehaviour)]
    #[behaviour(to_swarm = "OutEvent<O>")]
    struct Behaviour<T: 'static + Send> {
        custom: InnerBehaviour<T>,
    }

Does that make sense?