Network peers discovery

[jsamol]

Hi! Does smoldot discover peers in the network beyond the boot nodes? I quickly tried logging the network_service state (discovered_nodes and peers_list) in various places in light-base/src/sync_service.rs to see what peers the client had discovered, but all I can see are only the boot nodes the chain had been configured with through the specs, even though I know there are more nodes connected to this network.

Is this expected or it's just me not logging correct things?

[tomaka]

Smoldot uses the Kademlia protocol to discover new nodes. It periodically asks peers to share their peers.

The branch here is reached when we discover a new node.
The branch here is reached if there was some problem in the discovery problem.

[jsamol]

Ah, this is what I've been missing, thanks!

Ok, so other peers do get discovered, they are just rejected due to unsupported addresses. What I don't understand now is why it happens in the first place - all fail to get their multiaddr transformed to an address due to UnknownCombination, but the peers' addresses look fine to me:

/ip4/193.39.195.54/tcp/40170/p2p/12D3KooWCsSWkMNGbE7Y2yVHfjPMoWJCASBGPrdsZBqLtAn5y8Xb
/ip4/10.138.120.205/tcp/30333/ws/p2p/12D3KooWGyqP8bc1y3apnzbmfnWDnboVuwqXWTeMFjiWsECfGBWJ
/ip6/2402:1f00:8006:6d00::/tcp/30344/ws/p2p/12D3KooWSaAsySyJpVUiEjufTH8ahPbwStRuexWEM8DbQHMzUtcP

After reviewing the platform::address_parse::multiaddr_to_address code:
https://github.com/smol-dot/smoldot/blob/5bb68b3851f4f9e5a9a51fdb9413b95824d7c1ff/light-base/src/platform/address_parse.rs#L41-L156
and running some tests, I noticed that the error comes from the fact that none of the patterns could have been matched because none of the patterns expects the p2p protocol at the end. Is this the intended behaviour?

[tomaka]

The multiaddresses are not supposed to have /p2p/ at the end. The PeerId is conveyed separately through the Kademlia message and isn't part of the multiaddress.

The only place where a multiaddress should ever have /p2p/ is for things like CLI parameters, and it should be seen more as a multiaddress and peer ID combined into one string.

[jsamol]

The PeerId is conveyed separately through the Kademlia message and isn't part of the multiaddress.

That's not entirely what I observe. The addresses provided in the Kademlia response:
https://github.com/smol-dot/smoldot/blob/5bb68b3851f4f9e5a9a51fdb9413b95824d7c1ff/light-base/src/network_service.rs#L2196-L2208
resolve to Multiaddr ending with the p2p protocol. Afair, the p2p protocol is considered to be a valid part of a multiaddress, at least according to the libp2p definition.

[tomaka]

Afair, the p2p protocol is considered to be a valid part of a multiaddress, at least according to the libp2p definition

The definition doesn't say which multiaddresses are valid or not. It doesn't forbid non-sensical multiaddresses such as /ws/ws/ws/tcp. In our case, multiaddresses that end with /p2p/ are not valid.
If such a multiaddress is sent by a Substrate/Polkadot node (they changed their libp2p implementation not so long ago), then it is a bug in Substrate/Polkadot.