search

filecoin-project / venus

Filecoin Full Node Implementation in Go
https://venus.filecoin.io
Other
2.06k stars 463 forks source link

Enable block pubsub topic once caught-up to "trusted" peers and distrust blocks received. #3268

Closed frrist closed 5 years ago

frrist commented 5 years ago

Description

Currently we consider the initial bootstrapping of a chain to be done after the syncer has synchronously processed the chain up to the head reported by the first peer to respond to a hello message. Right now this peer is one of the bootstrap nodes held in the nodes config. For now we may consider heads published by these nodes to be "trusted". (using "" since this issue will be a precursor to #2674 and we are moving away from the idea of "trusted peer").

The syncers HandleNewTipSet method has a boolean parameter trusted that when set to false, will cause the syncer to reject tipsets exceeding the finality limit: https://github.com/filecoin-project/go-filecoin/blob/e71515a4ca441d70bb78664bf3dc74b14872297a/chain/syncer.go#L341-L344 Currently trusted is set to true everywhere, this issue aims to lay to foundation for allowing it to be set to false.

When a filecoin node joins the network after the chain has become very long (relative to its current head) it will take a long time for the node to process the chain up to the head it receives from a "trusted node". Because of this we will need to sync with a trusted node until our height is within ~finality of theirs. After this we should then enable the block pubsub topic and treat all blocks we receive on it as untrusted.

Acceptance criteria

ZenGround0 commented 5 years ago

Some opinions on a quick path forward. I'm still new to thinking about secure bootstrapping so take with a grain of salt. This is just one route, it might contradict ones you already have in mind which might turn out better.

The key and kind of subtle point I want to make is that syncing is inherently content based, not peer based (potentially many peers are talked to but only one chain is synced per invocation), so we shouldn't be altering the behavior of syncing invocations based on how much we trust the peer that told us about a head. Instead we should be altering the the behavior based on whether the chain is too long to rely on untrusted nodes. If none of our trusted bootstrappers have the risky chain then we don't fetch it.

I think the trusted boolean passed to HandleNewTipSet should be called requiresTrust and set to ci.Height - headHeight > UntrustedChainLimit. Alternatively noTrust and ci.Height - headHeight <= UntrustedChainLimit.

Then we can pipe requiresTrust to the graph sync fetcher's requestPeerFinder. When requiresTrust is high we only ask trusted nodes for help fetching. We would have to do a little edge case handling to ignore the incoming peer if it is untrusted.

We can set the trust bit on the chain info before adding to the peer tracker in the hello callback. By default we'll just pattern match on bootstrap addresses. Users could then readily configure their own trusted addresses too.

ZenGround0 commented 5 years ago

After writing this I'm realizing the ideal thing is to actually check in with trusted peers about whether they have a certain head on their chain. Then if they do you actually should ask everyone, including untrusted, for it. My reading of @frrist's original comment has this benefit in the case where a node hears about a long chain from a trusted node while my above proposal does not.

On the other hand my above proposal has the benefit that if we hear about a long chain from an untrusted peer but a trusted node vouches that this head is good then the node still fetches, whereas it would not according to my reading of the original comment.

To get both benefits for all heads regardless of who told us about them we would need another little protocol like hello for checking in with trusted nodes about the "okness" of certain headers.

Zooming out a little bit all of my comments have been focused on second order effects that probably don't matter much in the simplified case where the bootstrappers can handle everyones' requests and graphsync queries run to completion. So please don't worry too much about the past two comments.

anorth commented 5 years ago

I am trying to get away from the idea of trusted peers, though it's a useful interim. I don't think we should

only ask trusted nodes for help fetching

I agree about the content-based nature. What we want in the medium term is to transfer an idea of trust from a bootstrapper peer to the head it provides in hello, and then avoid plumbing an idea of trust any deeper. Later we can replace trust in a bootstrap peer with trust in a wide consensus of the network.

The only immediate problem I see is how to get a new Hello response from a trusted peer in case that initial chain fetch took so long that it's way behind again. The wiring of this right now is indirected through libp2p callbacks, but the node start routine knows the bootstrap peers. I think we just need a hook to re-trigger hello to some of them when the helloCallback discovers this state.

anorth commented 5 years ago

Made redundant by new spec. cc @ZenGround0