ChainSafe / forest

🌲 Rust Filecoin Node Implementation
https://forest.chainsafe.io
Apache License 2.0
618 stars 145 forks source link

fix(p2p): handle identify event and check required protocols #4407

Closed hanabi1224 closed 3 weeks ago

hanabi1224 commented 4 weeks ago

Summary of changes

(Part of https://github.com/ChainSafe/forest/issues/4391 investigation)

In go-libp2p and js-libp2p, identify is a core protocol that other protocols depend on, e.g. 1

pic := evt.(event.EvtPeerIdentificationCompleted)
// We just finished identifying the peer, that means we should know what
// protocols it speaks. Check if it speeks the Filecoin hello protocol
// before continuing.
if p, _ := h.Peerstore().FirstSupportedProtocol(pic.Peer, hello.ProtocolID); p != hello.ProtocolID {
    continue
}

It turns out in rust-libp2p, identify is not a core protocol 2 so the protocol unsupported failure is not due to invoking the protocol before identify completes.

Unlike some other libp2p implementations, rust-libp2p does not treat Identify as a core protocol. This means that other protocols cannot rely upon the existence of Identify, and need to be manually hooked up to Identify in order to make use of its capabilities.

This PR still tries to make the identify-event-handling logic more robust and bans peers that don't support required protocols earlier. It does not fix #4391 but rather proves the issue actually exists. The root cause might be that bad peers are not banned in lotus and their addresses are exchanged in the network via Kademlia

Changes introduced in this pull request:

Reference issue to close (if applicable)

Closes

Other information and links

Change checklist