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

Network sync refactoring (part 2) #11322

Merged
merged 13 commits into from
May 3, 2022

Conversation

nazar-pc
Copy link
Contributor

This is the second step towards #8686.

As #8686 (comment) suggests, this moves api.v1 proto scheme and a bunch of data structures related to sync into new sc-network-sync module, which sc-network now depends on.

This is mostly an implementation detail of sc-network, most of sc-network public API is unchanged.

I also noticed that there are some shared data structures that will be used in sc-network, sc-network-sync and upcoming sc-network-light (will be sent in a separate PR) and moved them into another new crate sc-network-common. sc-network-common is fairly minimal and can be expanded more in the future if necessary.

Overall this is mostly moving code around and really there is no changes in business logic.

I have made changes in commits such that they should make sense and compile/run (except two where I intentionally moved files in specific way). If this is merged without squashing, the history of all files should be preserved throughout all of the changes (useful for tracing origin of code changes in the future).

Reviewing individual commits is recommended.

P.S. I was unlucky to do this right when libp2p was updated 🙃

@nazar-pc
Copy link
Contributor Author

nazar-pc commented Apr 29, 2022

I see Polkadot digs a bit deeper into networking and fails CI because of that.

Would you prefer to add re-exports of sc-network-common in sc-network to fix that for now or create a companion that switches some of the things to sc-network-common already?

UPD: Went with re-exports, so nothing needs to be aware of this refactoring happening yet.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ty!

client/network/sync/Cargo.toml Outdated Show resolved Hide resolved
client/network/src/config.rs Outdated Show resolved Hide resolved
@bkchr bkchr added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels May 3, 2022
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
@bkchr
Copy link
Member

bkchr commented May 3, 2022

bot merge

@paritytech-processbot
Copy link

Waiting for commit status.

@bkchr bkchr added the D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit label May 3, 2022
@paritytech-processbot
Copy link

Merge cancelled due to error. Error: Statuses failed for 29b58a8

@nazar-pc
Copy link
Contributor Author

nazar-pc commented May 3, 2022

CI failure must be related to #11340 that was just merged

@bkchr bkchr merged commit c2fc4b3 into paritytech:master May 3, 2022
@nazar-pc nazar-pc deleted the network-sync-refactoring-part-2 branch May 3, 2022 14:14
godcodehunter pushed a commit to sensoriumxr/substrate that referenced this pull request Jun 22, 2022
* Move `api.v1.proto` schema into new crate `sc-network-sync`

* Move `sc_network::protocol::sync::state` module into `sc_network_sync::state`

* Move `sc_network::protocol::sync::blocks` module into `sc_network_sync::blocks` and some data structures from `sc_network::protocol::message` module into `sc_network_sync::message`

* Move some data structures from `sc_network::config` and `sc_network::request_responses` into new `sc-network-common` crate

* Move `sc_network::protocol::sync::warm` and `sc_network::warp_request_handler` modules into `sc_network_sync`

* Move `client/network/sync/src/lib.rs` to `client/network/sync/src/lib_old.rs` to preserve history of changes of the file in the next commit

* Move `client/network/src/protocol/sync.rs` on top of `client/network/sync/src/lib.rs` to preserve history of changes

* Move `sc_network::protocol::sync` to `sc_network_sync` with submodules, move message data structures around accordingly

* Move `sc_network::block_request_handler` to `sc_network_sync::block_request_handler`

* Move `sc_network::state_request_handler` to `sc_network_sync::state_request_handler`

* Add re-exports for compatibility reasons

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
DaviRain-Su pushed a commit to octopus-network/substrate that referenced this pull request Aug 23, 2022
* Move `api.v1.proto` schema into new crate `sc-network-sync`

* Move `sc_network::protocol::sync::state` module into `sc_network_sync::state`

* Move `sc_network::protocol::sync::blocks` module into `sc_network_sync::blocks` and some data structures from `sc_network::protocol::message` module into `sc_network_sync::message`

* Move some data structures from `sc_network::config` and `sc_network::request_responses` into new `sc-network-common` crate

* Move `sc_network::protocol::sync::warm` and `sc_network::warp_request_handler` modules into `sc_network_sync`

* Move `client/network/sync/src/lib.rs` to `client/network/sync/src/lib_old.rs` to preserve history of changes of the file in the next commit

* Move `client/network/src/protocol/sync.rs` on top of `client/network/sync/src/lib.rs` to preserve history of changes

* Move `sc_network::protocol::sync` to `sc_network_sync` with submodules, move message data structures around accordingly

* Move `sc_network::block_request_handler` to `sc_network_sync::block_request_handler`

* Move `sc_network::state_request_handler` to `sc_network_sync::state_request_handler`

* Add re-exports for compatibility reasons

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* Move `api.v1.proto` schema into new crate `sc-network-sync`

* Move `sc_network::protocol::sync::state` module into `sc_network_sync::state`

* Move `sc_network::protocol::sync::blocks` module into `sc_network_sync::blocks` and some data structures from `sc_network::protocol::message` module into `sc_network_sync::message`

* Move some data structures from `sc_network::config` and `sc_network::request_responses` into new `sc-network-common` crate

* Move `sc_network::protocol::sync::warm` and `sc_network::warp_request_handler` modules into `sc_network_sync`

* Move `client/network/sync/src/lib.rs` to `client/network/sync/src/lib_old.rs` to preserve history of changes of the file in the next commit

* Move `client/network/src/protocol/sync.rs` on top of `client/network/sync/src/lib.rs` to preserve history of changes

* Move `sc_network::protocol::sync` to `sc_network_sync` with submodules, move message data structures around accordingly

* Move `sc_network::block_request_handler` to `sc_network_sync::block_request_handler`

* Move `sc_network::state_request_handler` to `sc_network_sync::state_request_handler`

* Add re-exports for compatibility reasons

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>

Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
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. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants