smol-dot / smoldot

Lightweight client for Substrate-based chains, such as Polkadot and Kusama.
GNU General Public License v3.0
180 stars 48 forks source link

Consider merging `service.rs` and `peers.rs` together #478

Closed tomaka closed 11 months ago

tomaka commented 1 year ago

When thinking about how to update peers.rs in the context of https://github.com/smol-dot/smoldot/issues/111 and https://github.com/smol-dot/smoldot/issues/44, it seems to me that the logic peers.rs would be quite intertwined with the one in service.rs. In other words, the way the logic of the networking is split between peers.rs and service.rs isn't really appropriate anymore.

It would make sense to me to remove peers.rs entirely, and move what it is doing within service.rs.

Of course, we can't just move all the fields of peers.rs within the service, as it would become way too complex, but we can add more state machines that help the service do what it has do to.

tomaka commented 1 year ago

I think that the best course of action is to "soft-rewrite" service.rs.

service.rs is kind of tricky, because it's unclear what parts of the high-level networking behavior should be customizable by the top-level code (in full-node and light-base). In that kind of situation, an appropriate course of action is to have multiple copies of the same module for the different use cases.

Of course, these different copies would share a lot of code.


What we need from the networking service is:

One problem that service.rs currently suffers from is the fact that failing a connection attempt immediately unassigns the peer slot. Instead, the peer slot should be marked as "failed to connect" and then manually cleared and re-allocated by the API user.


When it comes to peers and addresses to decide who to connect to, it should be a separate mechanism from Kademlia, just like in Substrate.

Peers and addresses discovered through Kademlia are untrusted. In other words, if Alice sends to Bob the list of addresses of Charlie, Alice might lie and give fake addresses. Charlie should not suffer if that is the case.

It's still unclear to me how to manage these addresses. As a reminder, the algorithm used in Substrate is leaky.

tomaka commented 1 year ago

Also we can do #111 and #44 by rewriting service.rs, as all the necessary primitives are now available in collection.rs.

tomaka commented 1 year ago

Hold and manage the list of active connections. Hold and manage the list of pending connections.

So there's actually no need to differentiate active from pending connections. We obviously need to differentiate connections before their handshake vs after their handshake, but when it comes to "pending" vs "active" not only the definition is blurry but this distinction isn't useful in any way.