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

[Access] Relay messages to unstaked network directly #1106

Closed
synzhu opened this issue Aug 9, 2021 · 7 comments
Closed

[Access] Relay messages to unstaked network directly #1106

synzhu opened this issue Aug 9, 2021 · 7 comments
Assignees

Comments

@synzhu
Copy link
Contributor

synzhu commented Aug 9, 2021

This issue is to address the following concern raised about allowing unstaked nodes to connect to multiple upstream staked nodes:

Staked nodes will be relaying duplicate information to the network, because on each new block proposal every staked nodes will relay it to the unstaked network. This means that an unstaked node which is connected to n upstream AN's will receive (and gossip-forward) every block proposal n times.

Currently, we are implementing our own relay engine to relay messages received on the staked network to the unstaked network:

https://github.com/onflow/flow-go/blob/master/engine/access/relay/engine.go#L107-L108

However, this calls Publish on the libp2p pubsub.Topic:

https://github.com/onflow/flow-go/blob/master/network/p2p/libp2pNode.go#L487

Which creates a new libp2p Message with the node's own ID / signature:

https://github.com/libp2p/go-libp2p-pubsub/blob/master/topic.go#L224-L234

In order to take advantage of libp2p's built-in message deduplication, we need to relay the libp2p Message unchanged.

Code pointers:

Instead of using the relay engine, what we probably need to do is to call GossipSubRouter.Publish directly to publish the message onto the unstaked network.

Here is where we process the libp2p Message: https://github.com/onflow/flow-go/blob/master/network/p2p/readSubscription.go#L58-L59

As you can see, we only pass on rawMsg.Data to the engine level, meaning we lose all the other information. Instead, we need some mechanism that intercepts the rawMsg at this level, and directly passes that to GossipSub.Publish on the unstaked network.

The benefit of this approach, is that it should reduce amount of messages on the gossip layer.

Previously, the same block proposal sent from two different staked AN's were treated as two different messages at the gossip layer, and so they would both be processed and forwarded. This means that both would have to go through the hotstuff validation logic, and only at that point does the second one that was received get discarded.

With the new approach, the same block proposal is detected as a duplicate at the gossip layer, preventing it from being further disseminated in the network and also preventing further processing up the stack.

@synzhu synzhu self-assigned this Aug 9, 2021
@vishalchangrani
Copy link
Contributor

I believe this is an improvement not only for the unstaked side of the network but also on the staked side of the network where multiple consensus nodes are publishing the same blocks.

@synzhu
Copy link
Contributor Author

synzhu commented Aug 10, 2021

So I've done some more digging, and it turns out we won’t be able to access the GossipSubRouter since it is a private field. Thus, we can't directly relay messages by calling GossipSubRouter.Publish as hoped.

However, I realized that we may be approaching this completely wrong in the first place.

First of all, on the staked access node, we're creating two entirely separate instances of Network, which in turn creates two entirely distinct libp2p Nodes

I'm guessing the reason it's implemented this way is because conceptually we are creating two "separate" networks, but in fact I'm not sure we have to explicitly separate them this way.

Here is a method that allows us to register a topic validator:

https://github.com/libp2p/go-libp2p-pubsub/blob/master/pubsub.go#L1297

The validators are run before any message is sent or gossiped-forward on the network

What we could to instead, is the following:

  • For all staked AN's, we register a validator that checks if the message originated from a node who is in the protocol state (ie a staked node). If it is, then we can allow the validator to pass.
    • What this implies is, that messages from the staked network will be gossiped-forward to the unstaked nodes. This is handled automatically by libp2p, and we don't need to do any extra work.
  • If the message did not original from a node that we recognize from the protocol state, we fail the validation. At the same time, we intercept the message (since it must be from an unstaked node) and process it as we should.
    • This implies that messages from the public / unstaked network won't be gossiped-forward to the staked network.
    • At the same time, the staked AN's themselves will receive the messages from the unstaked network, and can process them however it needs (respond to sync requests, etc.)

The reason this should work is because unstaked nodes can only bootstrap from staked AN's and other unstaked nodes. In other words, the staked AN's are the only things connecting the staked network with the public network, and the two would be completely disconnected otherwise. This is ensured by connection gating on all of the staked nodes. Therefore, all traffic between the public and private networks has to pass through them.

One thing I'm not sure about is how this would work with DHT. Ideally, we would want to ensure that it is possible for the rest of the staked nodes to not participate in the DHT, so that unstaked nodes can't find out their addresses via the DHT. However, i believe it should be okay even if they do find out about the addresses because connection gating will prevent any messages from unstaked nodes to be gossiped by staked nodes.

@vishalchangrani
Copy link
Contributor

The separation of the networks is intentional. We do not want any traffic on the staked network from nodes which are not staked.

@huitseeker
Copy link
Contributor

Yes, but we have a Connection gater, don't we @vishalchangrani ?

@synzhu
Copy link
Contributor Author

synzhu commented Aug 10, 2021

Additional notes here: (Recording things here so I remember before putting everything into notion doc)

  1. Wanted to verify that signature checking is done by libp2p before validators are called. In other words, by the time our validator is run, we know that the From field has been validated and we can take it at face value.
    • first, we check signing policy here. This ensures that if signing is required, that a signature must be present.
    • Then, signature validation is done before any of our validators are called.
  2. Here is where the From field is populated. Here is the field definition.
  3. Important: We must ensure that sign policy is never changed from the "strict" setting, for all of the nodes.
  4. Here are the acceptable types we can use to implement a validator: https://github.com/libp2p/go-libp2p-pubsub/blob/master/validation.go#L162-L170

@synzhu
Copy link
Contributor Author

synzhu commented Aug 10, 2021

Another note:

Since we would be intercepting messages from unstaked nodes at the gossip layer, they would never make it to the engines on their own. In other words, they wouldn't go through the normal processing flow of messages received on the staked network (from libp2p -> our network layer -> engine).

Instead, we will need some way of injecting that message back into the processing flow once it's been validated, whether by directly passing it to the network layer or directly passing it to an engine.

@synzhu
Copy link
Contributor Author

synzhu commented Oct 6, 2021

This is done

@synzhu synzhu closed this as completed Oct 6, 2021
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

No branches or pull requests

3 participants