Disable Block request and State sync handler and make domain sync service always force_synced to enable transaction propagation

[vedhavyas]

This PR brings two changes to the Domain networking

• Disables BlockRequest and StateRequest handlers so that domain nodes do not make such requests to peers and as result the receiving peer will not ban them for making such requests multiple times.
• Makes the domain sync service to be force_synced since domain do not sync from the peer nodes but rather derives and imports blocks from Consensus chain.

Domain nodes did not propagate transactions to other peers since the node's sync service is returns Pending sync state. With domain sync_service set to force_synced, the transactions are being propagated. I have not looked too deep into substrate code to bypass this with force_synced and would appreciate if anyone know a better way to achieve the same with out force_synced on Domains.

cc: @jfrank-summit

Code contributor checklist:

• I have read, understood and followed contributing guide

[nazar-pc]

force_synced is Subspace fork-specific feature, it doesn't exist in upstream Substrate and IIRC shouldn't impact transaction propagation. It only impacts which peers will node use as a target for sync mechanism.

[vedhavyas]

IIRC shouldn't impact transaction propagation. It only impacts which peers will node use as a target for sync mechanism.

hmm.. Per my testing that was not the case. Transactions were not propagated when the Sync status was Pending. I would have to check the logic for transaction propagation on substrate

[NingLin-P]

Okay, I also met the tx propagate issue before in the test, and it is also workaround by setting force_synced to true, but was forgot to update it in the production code... 🤦‍♂️ that explains why the test_domain_tx_propagate can pass while it doesn't work in production
https://github.com/subspace/subspace/blob/0d72071195941ab5026d3df1ba168f8a530db7ae/domains/test/service/src/lib.rs#L97-L100

[vedhavyas]

My intuition was correct where node does not propagate transaction if the sync_service does indeed return if the node is major sync
https://github.com/paritytech/polkadot-sdk/blob/master/substrate/client/network/transactions/src/lib.rs#L449
https://github.com/paritytech/polkadot-sdk/blob/master/substrate/client/network/transactions/src/lib.rs#L503

[vedhavyas]

@NingLin-P message your node received from its peer Inactive block announce substream also due to the pending sync status. After INACTIVITY_EVICT_THRESHOLD has passed, the peer node assumes both your node and the peer node stalled since there is no syncing activity and sync _state is still pending. This behavior is due to the fact the peer node assumes both are stalled and tried to recover by disconnecting with your peer and tries to find a new peer with synced status

https://github.com/paritytech/polkadot-sdk/blob/9643a3adf8c2d2bf2a8e496e06702013d547cd55/substrate/client/network/sync/src/engine.rs#L720

[NingLin-P]

The changes look good to me overall.

Previously, I used force_synced = true to workaround the issue in the test was mainly due to its name suggested, but now I have taken a closer look at our polkadot-sdk fork and still can't find the relation between force_synced and is_major_syncing.

[nazar-pc]

Makes sense then