refactor(beacon-node): simplify libp2p init - #5774
Conversation
Performance Report鉁旓笍 no performance regression detected Full benchmark results
|
|
After looking at different behavior with and without But there seems to be two places this is used In case of libp2p
In case of discv5
Also note that the internal option lodestar/packages/cli/src/cmds/beacon/handler.ts Lines 175 to 179 in 549aa03 There is a comment in the code which I am not sure it is accurate since we dial lodestar/packages/beacon-node/src/network/peers/discover.ts Lines 132 to 136 in 549aa03 When the flag was initially added it seems like the purpose was limited to "append parsed bootnode ENRs to bootMultiaddrs" which as far as I understand are the nodes we dial over libp2p. Based on my understanding, the core functionality of The second behavior "connect on start" was added in this PR #3672 and it seems like this is required as otherwise starting two nodes using dev scripts won't connect to each other, this will dial the enrs directly through libp2p as if they were discovered by discv5. If my understanding is correct I think it makes sense to update the CLI description to indicate that it specifically means that Lodestar should dial bootnodes through/over libp2p as peers, perhaps lodestar/packages/cli/src/options/beaconNodeOptions/network.ts Lines 271 to 273 in 549aa03 |
|
Yes, It seems that there are some duplicate |
It's adding ENRs to our discv5 kad table, which will be used when we perform a discv5 lookup, to discover more peers. |
|
馃帀 This PR is included in v1.10.0 馃帀 |
Motivation
Looking at libp2p initialization, I noticed that the
connectToDiscv5Bootnodesfeature was bugged, and no longer functioned.Upon fixing, I noticed an opportunity to simplify the whole thing.
Description
connectToDiscv5BootnodescreateNodeJsLibp2pfunctionssrc/network/nodejsdirectory tosrc/network/libp2p