Fix local docker compose runs #2285
Conversation
This adds support for multiple bootstrap peers. We use this in the local docker compose set up to ensure all nodes have at least one other node to talk to at startup, which they can use to find their own external address via autonat. This also means we can remove the incorrect code where we always trust our observed external address once a single peer reports one. Instead, the external address candidates are passed to autonat and confirmed later. The configuration change is backwards compatible, because we support either a single entry or a list of bootstrap peers now.
JamesHinshelwood
requested review from
bzawisto,
DrZoltanFazekas and
shawn-zil
|
Bencher Report| Branch | fix-bootstrapping |
| Testbed | self-hosted |
Click to view all benchmark results
| Benchmark | Latency | Benchmark Result milliseconds (ms) (Result Δ%) | Upper Boundary milliseconds (ms) (Limit %) |
|---|---|---|---|
| full-blocks-erc20-transfers/full-blocks-erc20-transfers | 📈 view plot 🚷 view threshold | 1,195.70 (-9.79%) | 1,796.51 (66.56%) |
| full-blocks-evm-transfers/full-blocks-evm-transfers | 📈 view plot 🚷 view threshold | 387.80 (-30.43%) | 763.80 (50.77%) |
| full-blocks-zil-transfers/full-blocks-zil-transfers | 📈 view plot 🚷 view threshold | 4,055.90 (-0.89%) | 5,239.12 (77.42%) |
| process-empty/process-empty | 📈 view plot 🚷 view threshold | 9.72 (+10.02%) | 10.74 (90.53%) |
|
@JamesHinshelwood i think one side-effect of this is that, while local docker is able to run (because each node is now connected to the 2 bootstraps) syncing is now only syncing against 2 nodes, not 4 - when I spin up a new node. |
| @@ -254,20 +254,6 @@ impl P2pNode { | |||
| SwarmEvent::NewListenAddr { address, .. } => { | |||
| info!(%address, "P2P swarm listening on"); | |||
| } | |||
| SwarmEvent::Behaviour(BehaviourEvent::Identify(identify::Event::Received { peer_id, info: identify::Info{ listen_addrs, observed_addr, protocols, .. }, .. })) => { | |||
There was a problem hiding this comment.
I believe that this is actually needed - https://docs.rs/libp2p-kad/latest/libp2p_kad/#important-discrepancies
Peer Discovery with Identify In other libp2p implementations, the Identify protocol might be seen as a core protocol. Rust-libp2p tries to stay as generic as possible, and does not make this assumption. This means that the Identify protocol must be manually hooked up to Kademlia through calls to Behaviour::add_address. If you choose not to use the Identify protocol, and do not provide an alternative peer discovery mechanism, a Kademlia node will not discover nodes beyond the network’s boot nodes. Without the Identify protocol, existing nodes in the kademlia network cannot obtain the listen addresses of nodes querying them, and thus will not be able to add them to their routing table.
This might explain why I'm only syncing against 2 nodes in docker, now.
This adds support for multiple bootstrap peers. We use this in the local docker compose set up to ensure all nodes have at least one other node to talk to at startup, which they can use to find their own external address via autonat.
This also means we can remove the incorrect code where we always trust our observed external address once a single peer reports one. Instead, the external address candidates are passed to autonat and confirmed later.
The configuration change is backwards compatible, because we support either a single entry or a list of bootstrap peers now.
Fixes #2114.