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

Cannot use nested #[derive(NetworkBehaviour)] #1679

Closed
xu-cheng opened this issue Jul 27, 2020 · 2 comments · Fixed by #1681
Closed

Cannot use nested #[derive(NetworkBehaviour)] #1679

xu-cheng opened this issue Jul 27, 2020 · 2 comments · Fixed by #1681

Comments

@xu-cheng
Copy link
Contributor

Below is an example to reproduce this bug. Here there are two structures that derive NetworkBehaviour in a nested fashion.

src/main.rs

use anyhow::Result;
use futures::{future::poll_fn, prelude::*};
use libp2p::{
    identity::Keypair,
    ping::{Ping, PingConfig, PingEvent},
    swarm::{
        NetworkBehaviourAction, NetworkBehaviourEventProcess, PollParameters, Swarm, SwarmEvent,
    },
    NetworkBehaviour, PeerId,
};
use std::{
    collections::VecDeque,
    task::{Context, Poll},
};

#[derive(Default)]
struct FooEvent;

#[derive(NetworkBehaviour)]
#[behaviour(poll_method = "poll", out_event = "FooEvent")]
struct Foo {
    ping: Ping,
    #[behaviour(ignore)]
    events: VecDeque<FooEvent>,
}

impl NetworkBehaviourEventProcess<PingEvent> for Foo {
    fn inject_event(&mut self, event: PingEvent) {
        println!("recv ping {:?}", event);
        self.events.push_back(FooEvent::default());
    }
}

impl Foo {
    fn new() -> Self {
        Self {
            ping: Ping::new(PingConfig::new().with_keep_alive(true)),
            events: VecDeque::new(),
        }
    }
    fn poll<T>(
        &mut self,
        _: &mut Context,
        _: &mut impl PollParameters,
    ) -> Poll<NetworkBehaviourAction<T, FooEvent>> {
        if let Some(event) = self.events.pop_front() {
            return Poll::Ready(NetworkBehaviourAction::GenerateEvent(event));
        }

        Poll::Pending
    }
}

#[derive(NetworkBehaviour)]
struct Bar {
    foo: Foo,
}

impl NetworkBehaviourEventProcess<FooEvent> for Bar {
    fn inject_event(&mut self, _: FooEvent) {
        println!("recv foo event");
    }
}

impl Bar {
    fn new() -> Self {
        Self { foo: Foo::new() }
    }
}

#[tokio::main]
async fn main() -> Result<()> {
    let key1 = Keypair::generate_ed25519();
    let key2 = Keypair::generate_ed25519();
    let peer1 = PeerId::from(key1.public());
    let peer2 = PeerId::from(key2.public());

    let transport1 = libp2p::build_development_transport(key1)?;
    let transport2 = libp2p::build_development_transport(key2)?;

    let mut swarm1 = Swarm::new(transport1, Bar::new(), peer1);
    let mut swarm2 = Swarm::new(transport2, Bar::new(), peer2);

    Swarm::listen_on(&mut swarm1, "/ip4/127.0.0.1/tcp/0".parse()?)?;
    let address1 = loop {
        match swarm1.next_event().await {
            SwarmEvent::NewListenAddr(address) => break address,
            _ => {}
        }
    };
    println!("swarm1 listen on {}", address1);
    Swarm::dial_addr(&mut swarm2, address1)?;

    let _ = tokio::spawn(poll_fn(move |cx| {
        loop {
            match swarm1.poll_next_unpin(cx) {
                Poll::Ready(_) => {}
                Poll::Pending => break,
            }
        }

        Poll::<()>::Pending
    }));
    let _ = tokio::spawn(poll_fn(move |cx| {
        loop {
            match swarm2.poll_next_unpin(cx) {
                Poll::Ready(_) => {}
                Poll::Pending => break,
            }
        }

        Poll::<()>::Pending
    }));

    tokio::time::delay_for(tokio::time::Duration::from_secs(5)).await;

    Ok(())
}

Cargo.toml

[package]
name = "debug-p2p"
version = "0.1.0"
authors = ["Cheng XU <rust@xuc.me>"]
edition = "2018"
publish = false

[dependencies]
libp2p = "*"
futures = "*"
tokio = { version = "*", features = ["full"] }
anyhow = "*"

Running the above example, we get the following output, which means that neither of the inject_event function is called.

swarm1 listen on /ip4/127.0.0.1/tcp/54121

If we change the swarm to use Foo directly, that is:

- let mut swarm1 = Swarm::new(transport1, Bar::new(), peer1);
- let mut swarm2 = Swarm::new(transport2, Bar::new(), peer2);
+ let mut swarm1 = Swarm::new(transport1, Foo::new(), peer1);
+ let mut swarm2 = Swarm::new(transport2, Foo::new(), peer2);

We got the normal behaviour:

swarm1 listen on /ip4/127.0.0.1/tcp/55298
recv ping PingEvent { peer: PeerId("12D3KooWBQVD7UYtqd8hT38CjSAV7FTsJbEkyUveLyNedN7WfKEM"), result: Ok(Pong) }
recv ping PingEvent { peer: PeerId("12D3KooWPmpQJwavtw8nPQFkBniMYeWdKatXm1bjoctYd55T7xgi"), result: Ok(Ping { rtt: 1.421545ms }) }
recv ping PingEvent { peer: PeerId("12D3KooWPmpQJwavtw8nPQFkBniMYeWdKatXm1bjoctYd55T7xgi"), result: Ok(Pong) }
recv ping PingEvent { peer: PeerId("12D3KooWBQVD7UYtqd8hT38CjSAV7FTsJbEkyUveLyNedN7WfKEM"), result: Ok(Ping { rtt: 1.580406ms }) }
@thomaseizinger
Copy link
Contributor

Not sure if it solves the problem, but you might be interested in the event_process = false attribute: https://docs.rs/libp2p/0.22.0/libp2p/swarm/trait.NetworkBehaviour.html#deriving-networkbehaviour

@xu-cheng
Copy link
Contributor Author

I found out what is the wrong. #1681 should fix it.

romanb pushed a commit that referenced this issue Jul 28, 2020
…1681)

* core-derive: use full qualified name when polling NetworkBehaviour

If the users define a custom poll method also named in `poll` like
`#[behaviour(poll_method = "poll")`, it will cause issues as the wrong
`poll` method being called.

So use full qualified name to avoid ambiguity.

Fixes #1679.

* Update changelog for patch release.

Co-authored-by: Max Inden <mail@max-inden.de>
Co-authored-by: Roman S. Borschel <roman@parity.io>
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 a pull request may close this issue.

2 participants