pov-recovery: try connect to validators

[ordian]

A companion to paritytech/polkadot#6081.

• Make it compile due to unrelated unmerged companions
• Test it works with the polkadot changes
• Revert the REVERTME commits
• Test it doesn't work on master

[bkchr]

@ordian is there any update? Could you already make it fail without your patch?

[ordian]

@ordian is there any update? Could you already make it fail without your patch?

I'm trying to make it work with my patch first. Investing why it doesn't.

[ordian]

I think one more thing that should be looked into is this: https://github.com/paritytech/substrate/blob/0ee03277c33b6334ddba7434e014fa637dcb6107/client/cli/src/params/network_params.rs#L236

Relay part of collators and all validators are using --reserved-only, so it should be disabled.

[skunert]

I think one more thing that should be looked into is this: https://github.com/paritytech/substrate/blob/0ee03277c33b6334ddba7434e014fa637dcb6107/client/cli/src/params/network_params.rs#L236

I suspect that Kademlia is doing the random walks, aka connecting to nodes "on its own". If we disable this, I assume the node should not to any other node, besides the bootnodes/reserved nodes. Maybe @tomaka / @mxinden could shed some light onto this. TLDR is that we don't want that certain nodes connect to other nodes in any "automatic" way and only when some behavior triggers these connection. Currently it looks like as we are always having a connection between all nodes at some point (I get that this is related to the small size of the test network, however it would be nice to not require to run a big network to test the stuff we want to test :P).

I was suspecting this too and had dht random walks already disabled for my tests. I also modified the discovery limit to 0 here.

When disabled, I do not see this message anymore, as expected.

2022-10-17 09:16:54.645 DEBUG tokio-runtime-worker sub-libp2p: [charlie] Libp2p <= Starting random Kademlia request for PeerId("1AbHqR4KoozFoA82wnYtZKyg8rg94EjsT9GSaGKATdSnZy") 

But we still see this message for some validators (not all).

2022-10-17 09:16:54.648 DEBUG tokio-runtime-worker sub-libp2p: [nutritious-drug-9580] Libp2p => Connected(PeerId("12D3KooWPKzmmE2uYgF3z13xjpbFTp63g9dZFag8pG6MgnpSLF4S"))    

[bkchr]

But we still see this message for some validators (not all).

2022-10-17 09:16:54.648 DEBUG tokio-runtime-worker sub-libp2p: [nutritious-drug-9580] Libp2p => Connected(PeerId("12D3KooWPKzmmE2uYgF3z13xjpbFTp63g9dZFag8pG6MgnpSLF4S"))    

This is just the normal connect message? Or why is it special?

I briefly scanned the Kademlia code now. I think what happens is that the authority discovery is putting/getting values from the DHT. This is done through Kademlia. When we are not connected to the peers we want to push the value too, we will connect to them: https://docs.rs/libp2p-kad/0.41.0/src/libp2p_kad/behaviour.rs.html#2345 Kademlia learns about all bootnodes and reserved nodes on startup. IDK what bootnodes zombienet is setting, but I assume all validators or something? If yes, then all collators should know these nodes at least and will probably start connections to them to at leat push/get values from the DHT via Kademlia.

[mxinden]

Maybe @tomaka / @mxinden could shed some light onto this.

Happy to help. From briefly scanning this issue I don't quite understand what the problem / goal is. Would you mind posting a summary to catch me up?

[ordian]

Maybe @tomaka / @mxinden could shed some light onto this.

Happy to help. From briefly scanning this issue I don't quite understand what the problem / goal is. Would you mind posting a summary to catch me up?

Hey 👋

In this zombienet test we're running a small relay chain with 1 full node (ferdie) and 13 validators. All validators have --reserved-only --reserved-nodes "<ferdie>". There are also 3 cumulus nodes, two of them who do not produce para-blocks have the same flags on the relay-chain side of the collator. Given the flags, collators should not be connecting to any of the validators. But somehow they do at the beginning.

[tomaka]

--reserved-only and --reserved-nodes are only relevant when it comes to syncing/transactions/grandpa (more specifically syncing, but transactions and grandpa simply blindly follow the list of peers we use for syncing).
They intentionally don't apply to Kademlia because, as you've realized, authority-discovery can't work otherwise. They also don't apply to anything higher level such as the functions used by the parachains networking.

[bkchr]

It would be nice to understand the internals and who requests the connection. However, I get that this isn't really important here as we managed to make it fail. So, let's continue with this pr.

[ordian]

--reserved-only and --reserved-nodes are only relevant when it comes to syncing/transactions/grandpa (more specifically syncing, but transactions and grandpa simply blindly follow the list of peers we use for syncing). They intentionally don't apply to Kademlia because, as you've realized, authority-discovery can't work otherwise. They also don't apply to anything higher level such as the functions used by the parachains networking.

Sure, it's fine for authority discovery to work in that case. But I didn't expect for connections to be automatically established. I'm aware of the high-level code that issues connections, e.g. collator-protocol connecting collators to the backing group. In our case, these two collators should not be issuing them because they do not produce any collation, and even if they do connect, backing group size is set to 1 at genesis and they connect to at least 5 validators.

[paritytech-processbot]

Waiting for commit status.

[paritytech-processbot]

Merge cancelled due to error. Error: Statuses failed for 1b04f18

[ordian]

jFYI: This PR is not a companion in the traditional sense since it fixes a test that doesn't actually test what it supposed and will pas anyway. So it will not block other unmerged companions.

[bkchr]

bot merge