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

share: implement full service discovery #599

Closed
Wondertan opened this issue Apr 7, 2022 · 12 comments · Fixed by #799
Closed

share: implement full service discovery #599

Wondertan opened this issue Apr 7, 2022 · 12 comments · Fixed by #799
Assignees
Labels

Comments

@Wondertan
Copy link
Member

The simplest discovery mechanism for finding full nodes can be:

  • Full nodes advertise themselves on the full node topic/namespace
  • Light clients run a component that ensures they are connected to N amount of full nodes and, if not, discover more by the namespace
@Wondertan Wondertan self-assigned this Apr 7, 2022
@Wondertan
Copy link
Member Author

Full nodes have to be prioritized through a connection manager tagging

@liamsi
Copy link
Member

liamsi commented Apr 7, 2022

Light clients run a component that ensures they are connected to N amount of full nodes

What would be N here?

@Wondertan
Copy link
Member Author

Wondertan commented Apr 10, 2022

4/8 at the start? There won't be a lot of Full nodes in foreseeable testnet, but only bridge nodes which I would count as Full ones.

What would be N here?

There should be some ratio between the number of light nodes and full nodes which we should aim to have. By this ratio we can set the amount of connections that light nodes have to establish.

cc @musalbas

@Wondertan
Copy link
Member Author

Wondertan commented Apr 10, 2022

I also see a problem that the above simple discovery doesn't account for. Every new Full Node joining the network won't be able to get enough light clients to reconstruct the block reliably, as light clients keep connections to discovered full nodes and don't change them over time.(Besides light nodes that often go offline and back)

Discovering light nodes by fulls can potentially fix this. Even though I consider this solution the best, it is not practical, assuming that most of the light nodes will be behind the NAT, and libp2p’s NAT Hole Punching is not battle-tested enough to rely on it.

Another aspect is GossipSub which keeps the number of connections per topic within some threshold. So new Full Node that connects to the bootstrappers will receive more peers from the bootstrappers once they disconnect the full node due to several connections being more than the threshold(PeX on disconnect). The problem here is that light nodes can potentially be connected to other light nodes only, while full nodes to full nodes only. So even though GossipSub can mitigate the problem by giving more light nodes to the new fulls, it also introduces an unhealthy network topology edge case which we should avoid.

The solution I see is for light nodes to introduce some refresh timeout(~1h) on which light nodes rediscover full nodes and try to connect to a different set of full nodes with some overlap with the former one. This way, new full nodes will be connected to some light nodes less than the timeout time. The downside here might be the constantly changing topology of the network, but I don't see why this can be an issue in practice, as long as peers maintain the required amount of connections to wanted node types to function correctly.

The GossipSub issue with it dropping connections can be fixed by (1)setting a higher topic connections amount threshold and (2)using a shared connection manager, which can be used to prioritize peers/connections.

cc @musalbas @liamsi

@musalbas
Copy link
Member

musalbas commented Apr 10, 2022

I think you're overthinking this, and the effects of a slightly unbalanced network topology. You should check how Bitcoin deals with this issue (I'm not sure that it does at all, by default), or any other blockchain network with full and light nodes. Let's see what happens first with the initial implementation.

Light nodes being connected to 4-8 full nodes sounds right to me.

@musalbas
Copy link
Member

Every new Full Node joining the network won't be able to get enough light clients to reconstruct the block reliably, as light clients keep connections to discovered full nodes and don't change them over time.

It doesn't actually matter if a single full node isn't connected to any light clients (i.e. there are too many honest full nodes from light clients to pick from). What matters, is that there are enough light clients connected to the full node network, and those full nodes are connected to each other. The full nodes should be able to cover the blocks by sharing the block among themselves as per #598.

@musalbas
Copy link
Member

The problem here is that light nodes can potentially be connected to other light nodes only, while full nodes to full nodes only.

Isn't it possible to simply add a check in the light client that if it is not connected to at least 5 full nodes, it will keep trying to connect to full nodes until it's connected to enough of them?

@renaynay
Copy link
Member

The problem here is that light nodes can potentially be connected to other light nodes only, while full nodes to full nodes only.

Isn't it possible to simply add a check in the light client that if it is not connected to at least 5 full nodes, it will keep trying to connect to full nodes until it's connected to enough of them?

Yes but that still requires an element of service discovery so that light nodes know the node type of the peer they're connected to. We don't need to do gymnastics and I agree this check is enough but it still needs to be implemented somehow (possibly via custom handshake @Wondertan ?)

@musalbas
Copy link
Member

But isn't it possible to make it so that only full nodes advertise themselves into a topic?

@renaynay
Copy link
Member

Full nodes advertise themselves on the full node topic/namespace

I guess we'd have to spin up a separate topic for full nodes to advertise themselves as full nodes? @Wondertan

@Wondertan
Copy link
Member Author

@musalbas, okay, swap from my message discover of light clients to the discovery of full nodes, and the problem still persists in some way. Full nodes should also discover other node types(not necessary light) for block reconstruction guarantees and connect to them to form the network you refer to. Otherwise, new full nodes won't necessarily end up connected to the Full node network as only GossipSub manages network topology shaping.

It doesn't actually matter if a single full node isn't connected to any light clients (i.e. there are too many honest full nodes from light clients to pick from). What matters, is that there are enough light clients connected to the full node network, and those full nodes are connected to each other. The full nodes should be able to cover the blocks by sharing the block among themselves as per #598.

It is possible, and this is what the issue is describing in the first place. The GossipSub reference is mostly about tracking the fact that it manages the existing connections without understanding what type of a node it is connected to. So we have to do our own topology shaper which has higher priority than GossipSub decisions, so we any node is always connected to N amount of Full nodes.

The problem here is that light nodes can potentially be connected to other light nodes only, while full nodes to full nodes only.

Isn't it possible to simply add a check in the light client that if it is not connected to at least 5 full nodes, it will keep trying to connect to full nodes until it's connected to enough of them?

@Wondertan
Copy link
Member Author

@renaynay, yes

I guess we'd have to spin up a separate topic for full nodes to advertise themselves as full nodes? @Wondertan

Note that this doesn't require any handshaking logic changes to exchange node type. This can be done solely on the PeerDiscovery DHT layer, so we can avoid this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants