Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Extract syncing protocol from sc-network #12828

Merged
merged 47 commits into from
Mar 6, 2023

Conversation

altonen
Copy link
Contributor

@altonen altonen commented Dec 2, 2022

This PR extracts the syncing protocol from sc-network by introducing a SyncingEngine object which holds all the syncing-related code that was previously in Protocol. It's an async task that can be communicated with using SyncingService. SyncingEngine also owns ChainSync and is responsible for polling it.

It was discovered in paritytech/polkadot-sdk#556 that the current handshaking implementation is incorrect and requires quite a bit of work to get it working properly. I would rather not tinker with the support beams in this PR and would like to address the handshaking issue in a follow-up PR if it's alright.

The only new code this PR adds is the SyncingService which models the already-existing NetworkService concept, meaning it now implements the traits (such as NetworkBlock and JustificatonSyncLink) that were previously implemented by NetworkService. Other than that, this PR is just moving code away from sc-network and fixing any references other crates made to syncing-related concepts. The syncing code is in dire need of refactoring but that can be addressed in future PRs.

Resolves #8686

polkadot companion: paritytech/polkadot#6380
cumulus companion: paritytech/cumulus#1939

Add supplementary asynchronous API for the import queue which means
it can be run as an independent task and communicated with through
the `ImportQueueService`.

This commit removes removes block and justification imports from
`sc-network` and provides `ChainSync` with a handle to import queue so
it can import blocks and justifications. Polling of the import queue is
moved complete out of `sc-network` and `sc_consensus::Link` is
implemented for `ChainSyncInterfaceHandled` so the import queue
can still influence the syncing process.
Some of the tests have to be rewritten
Remove `SyncConnected`/`SyncDisconnected` events from
`NetworkEvenStream` and provide those events through
`ChainSyncInterface` instead.

Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take
`SyncEventStream` object which they listen to for incoming sync peer
events.
This interface provides a set of miscellaneous functions that other
subsystems can use to query, for example, the syncing status.
Subscribe to `NetworkStreamEvent` and poll the incoming notifications
and substream events from `SyncingEngine`.

The code needs refactoring.
This commits removes the last hard dependency of syncing from
`sc-network` meaning the protocol now lives completely outside of
`sc-network`, ignoring the hardcoded peerset entry which will be
addressed in the future.

Code needs a lot of refactoring.
They were written for the sole purpose of verifying that `NetworWorker`
continues to function while the calls are being dispatched to
`ChainSync`.
@altonen altonen requested review from bkchr, dmitry-markin and a team December 2, 2022 09:55
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 2, 2022
@altonen altonen added B3-apinoteworthy C1-low PR touches the given topic and has a low impact on builders. labels Dec 2, 2022
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 3, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 3, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 3, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 4, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 4, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 5, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
ukint-vs pushed a commit to gear-tech/substrate that referenced this pull request Apr 10, 2023
* Move import queue out of `sc-network`

Add supplementary asynchronous API for the import queue which means
it can be run as an independent task and communicated with through
the `ImportQueueService`.

This commit removes removes block and justification imports from
`sc-network` and provides `ChainSync` with a handle to import queue so
it can import blocks and justifications. Polling of the import queue is
moved complete out of `sc-network` and `sc_consensus::Link` is
implemented for `ChainSyncInterfaceHandled` so the import queue
can still influence the syncing process.

* Move stuff to SyncingEngine

* Move `ChainSync` instanation to `SyncingEngine`

Some of the tests have to be rewritten

* Move peer hashmap to `SyncingEngine`

* Let `SyncingEngine` to implement `ChainSyncInterface`

* Introduce `SyncStatusProvider`

* Move `sync_peer_(connected|disconnected)` to `SyncingEngine`

* Implement `SyncEventStream`

Remove `SyncConnected`/`SyncDisconnected` events from
`NetworkEvenStream` and provide those events through
`ChainSyncInterface` instead.

Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take
`SyncEventStream` object which they listen to for incoming sync peer
events.

* Introduce `ChainSyncInterface`

This interface provides a set of miscellaneous functions that other
subsystems can use to query, for example, the syncing status.

* Move event stream polling to `SyncingEngine`

Subscribe to `NetworkStreamEvent` and poll the incoming notifications
and substream events from `SyncingEngine`.

The code needs refactoring.

* Make `SyncingEngine` into an asynchronous runner

This commits removes the last hard dependency of syncing from
`sc-network` meaning the protocol now lives completely outside of
`sc-network`, ignoring the hardcoded peerset entry which will be
addressed in the future.

Code needs a lot of refactoring.

* Fix warnings

* Code refactoring

* Use `SyncingService` for BEEFY

* Use `SyncingService` for GRANDPA

* Remove call delegation from `NetworkService`

* Remove `ChainSyncService`

* Remove `ChainSync` service tests

They were written for the sole purpose of verifying that `NetworWorker`
continues to function while the calls are being dispatched to
`ChainSync`.

* Refactor code

* Refactor code

* Update client/finality-grandpa/src/communication/tests.rs

Co-authored-by: Anton <[email protected]>

* Fix warnings

* Apply review comments

* Fix docs

* Fix test

* cargo-fmt

* Update client/network/sync/src/engine.rs

Co-authored-by: Anton <[email protected]>

* Update client/network/sync/src/engine.rs

Co-authored-by: Anton <[email protected]>

* Add missing docs

* Refactor code

---------

Co-authored-by: Anton <[email protected]>
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 13, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 20, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 21, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 25, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request Apr 28, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request May 1, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
atodorov added a commit to gluwa/creditcoin that referenced this pull request May 4, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request May 22, 2023
jiguantong added a commit to darwinia-network/darwinia that referenced this pull request May 23, 2023
AurevoirXavier added a commit to darwinia-network/darwinia that referenced this pull request May 23, 2023
* Remove `account`

* Update and companions

* Companion of paritytech/cumulus#2164

* Companion of paritytech/substrate#13159

* Companion of paritytech/cumulus#1909

* Fmt

* Companion of paritytech/polkadot#6744

* Companion of paritytech/cumulus#2245

* Companion of paritytech/substrate#12828

* Companion of paritytech/cumulus#2287

* Companion of paritytech/substrate#13592

* Companion of paritytech/cumulus#2308

* Companion of paritytech/substrate#13410

* Companion of paritytech/substrate#13305

* Companion of polkadot-evm/frontier#1050

* TODO weight

* TODO weight

* Companion of polkadot-evm/frontier#1040

Signed-off-by: Xavier Lau <[email protected]>

* Remove unused dep

* Try fix dev node paritytech/substrate#12828

* Fix the frontier part (#1154)

* Fix dev node

* Fmt

* Bump moonbeam

* Bump moonbeam

* Remove unnecessary clone

* Fix tests

---------

Signed-off-by: Xavier Lau <[email protected]>
Co-authored-by: Guantong <[email protected]>
Co-authored-by: bear <[email protected]>
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/polkadot-release-analysis-v0-9-41-v0-9-42/2828/3

nathanwhit pushed a commit to gluwa/creditcoin that referenced this pull request Jun 7, 2023
- Use of `#[pallet::generate_store(pub(super) trait Store)]` will be removed soon,
  paritytech/substrate#13535

- Update return values and arguments, b/c syncing protocol was extracted from
  sc-network. See paritytech/substrate#12828

- Update call to construct_genesis_block(), see
  paritytech/substrate#13427

- Use of deprecated associated function
  `frame_support::dispatch::Weight::from_ref_time`: Will be removed soon; use
  `from_parts` instead.
ggwpez pushed a commit to ggwpez/runtimes that referenced this pull request Jul 13, 2023
* Companion for paritytech/substrate#12764

* Remove `async-trait`

* Companion for paritytech/substrate#12828

* carg fmt

* Update client/relay-chain-minimal-node/src/network.rs

Co-authored-by: Bastian Köcher <[email protected]>

* update lockfile for {"polkadot", "substrate"}

---------

Co-authored-by: Bastian Köcher <[email protected]>
Co-authored-by: parity-processbot <>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Move import queue out of `sc-network`

Add supplementary asynchronous API for the import queue which means
it can be run as an independent task and communicated with through
the `ImportQueueService`.

This commit removes removes block and justification imports from
`sc-network` and provides `ChainSync` with a handle to import queue so
it can import blocks and justifications. Polling of the import queue is
moved complete out of `sc-network` and `sc_consensus::Link` is
implemented for `ChainSyncInterfaceHandled` so the import queue
can still influence the syncing process.

* Move stuff to SyncingEngine

* Move `ChainSync` instanation to `SyncingEngine`

Some of the tests have to be rewritten

* Move peer hashmap to `SyncingEngine`

* Let `SyncingEngine` to implement `ChainSyncInterface`

* Introduce `SyncStatusProvider`

* Move `sync_peer_(connected|disconnected)` to `SyncingEngine`

* Implement `SyncEventStream`

Remove `SyncConnected`/`SyncDisconnected` events from
`NetworkEvenStream` and provide those events through
`ChainSyncInterface` instead.

Modify BEEFY/GRANDPA/transactions protocol and `NetworkGossip` to take
`SyncEventStream` object which they listen to for incoming sync peer
events.

* Introduce `ChainSyncInterface`

This interface provides a set of miscellaneous functions that other
subsystems can use to query, for example, the syncing status.

* Move event stream polling to `SyncingEngine`

Subscribe to `NetworkStreamEvent` and poll the incoming notifications
and substream events from `SyncingEngine`.

The code needs refactoring.

* Make `SyncingEngine` into an asynchronous runner

This commits removes the last hard dependency of syncing from
`sc-network` meaning the protocol now lives completely outside of
`sc-network`, ignoring the hardcoded peerset entry which will be
addressed in the future.

Code needs a lot of refactoring.

* Fix warnings

* Code refactoring

* Use `SyncingService` for BEEFY

* Use `SyncingService` for GRANDPA

* Remove call delegation from `NetworkService`

* Remove `ChainSyncService`

* Remove `ChainSync` service tests

They were written for the sole purpose of verifying that `NetworWorker`
continues to function while the calls are being dispatched to
`ChainSync`.

* Refactor code

* Refactor code

* Update client/finality-grandpa/src/communication/tests.rs

Co-authored-by: Anton <[email protected]>

* Fix warnings

* Apply review comments

* Fix docs

* Fix test

* cargo-fmt

* Update client/network/sync/src/engine.rs

Co-authored-by: Anton <[email protected]>

* Update client/network/sync/src/engine.rs

Co-authored-by: Anton <[email protected]>

* Add missing docs

* Refactor code

---------

Co-authored-by: Anton <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. T2-API This PR/Issue is related to APIs.
Projects
Status: Done
Status: done
Development

Successfully merging this pull request may close these issues.

Extracting the syncing protocol from sc-network
6 participants