Network discovery failure via RequestPeers about exonum HOT 5 CLOSED

@DarkEld3r another solution was proposed by @defuz and @alekseysidorov : double checking the Connect handshake logic, though it's quite complex there.
It's not hard to reproduce, when it doesn't work: A con-> B doesn't result in B con->A.

from exonum.

If node A missed node B's Connect, node A won't send its peers to B upon being requested.

This is an expected behavior and there is nothing to fix here. If A missed B's Connect message, A can't connect to B to send anything. There is some possibility that some request may be ignored because the responder is not connected to the requestor yet. However, if requestor sends any message to the responder, it's guaranteed that it already send Connect message. Thus, the connection in both directions must be established sooner or later.

@gisochre @DarkEld3r I think we need to gather more information about what actually happens and what is the state of the nodes after network partitioning. Also, perhaps it makes sense to improve the choice of time and recipient of the RequestPeers messages.

from exonum.

@defuz ONCE AGAIN (pleaze read with attention):
It's not hard to reproduce, when handshake doesn't work: A Connect-> B doesn't result in B Connect->A.

And ...logging is confusing.
(there's log Send Connect message to ... which doesn't occur AFTER message was sent, only BEFORE invoking an event on event_loop:

    fn connect(&mut self, address: &SocketAddr) {
        self.inner
            .send(InternalEvent::Invoke(Invoke::Connect(*address)))
            .log_error("Unable to connect");
    }

And ....names of network-related methods are totally confusing: in events/mod.rs, events/network.rs, node/mod.rs, node/basic.rs. A lot of similar handle_connect(ed).
Good example is just above: it should be send_invoke_connect_event instead of connect.

from exonum.

@gisochre

It's not hard to reproduce, when handshake doesn't work: A Connect-> B doesn't result in B Connect->A.

OK. This looks like misbehavior. The next steps will be to understand how this should work, and why this does not work as expected. So, how this should work:

1. Once the TCP connection is established, the client sends Connect message immediately: https://github.com/exonum/exonum-core/blob/master/exonum/src/node/basic.rs#L37
2. When the server receives Connect message, it tries to establish the backward TCP connection if it is not already open:
   https://github.com/exonum/exonum-core/blob/master/exonum/src/node/basic.rs#L75

It is easy to conclude that if A send Connect to B then B should send Connect to A or has done this before. If this does not happen, we need to understand why.

logging is confusing

names of network-related methods are totally confusing

It's great that you've noticed it. Here are the next steps you can take:

1. Create an issue about this.
2. Suggest PR.

Anyway, the logs and method names are not relevant to code misbehavior, so let's go back to the main discussion.

from exonum.

@defuz i'll start with a separate pr for logging and renaming, that'll help a lot to debug this.

from exonum.