Skip to content

Commit

Permalink
Syncing strategy refactoring (paritytech#5469)
Browse files Browse the repository at this point in the history
This is a step towards
paritytech#5333

The commits with code moving (but no other changes) and actual changes
are separated for easier review.

Essentially this results in `SyncingStrategy` trait replacing struct
(which is renamed to `PolkadotSyncingStrategy`, open for better name
suggestions). Technically it is already possible to replace
`PolkadotSyncingStrategy<B, Client>` with `Box<dyn SyncingStrategy<B>`
in syncing engine, but I decided to postpone such change until we
actually have an ability to customize it. It should also be possible to
swap `PolkadotSyncingStrategy` with just `ChainSync` that only supports
regular full sync from genesis (it also implements `SyncingStrategy`
trait).

While extracted trait still has a lot of non-generic stuff in it like
exposed knowledge of warp sync and `StrategyKey` with hardcoded set of
options, I believe this is a step in the right direction and will
address those in upcoming PRs.

With paritytech#5431 that landed
earlier warp sync configuration is more straightforward, but there are
still numerous things interleaved that will take some time to abstract
away nicely and expose in network config for developers. For now this is
an internal change even though data structures are technically public
and require major version bump.
  • Loading branch information
nazar-pc authored and mordamax committed Sep 10, 2024
1 parent dd43261 commit 3ffe84c
Show file tree
Hide file tree
Showing 5 changed files with 941 additions and 859 deletions.
11 changes: 11 additions & 0 deletions prdoc/pr_5469.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
title: Syncing strategy refactoring

doc:
- audience: Node Dev
description: |
Mostly internal changes to syncing strategies that is a step towards making them configurable/extensible in the
future. It is unlikely that external developers will need to change their code.

crates:
- name: sc-network-sync
bump: major
9 changes: 5 additions & 4 deletions substrate/client/network/sync/src/engine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ use crate::{
},
strategy::{
warp::{EncodedProof, WarpProofRequest, WarpSyncConfig},
StrategyKey, SyncingAction, SyncingConfig, SyncingStrategy,
PolkadotSyncingStrategy, StrategyKey, SyncingAction, SyncingConfig, SyncingStrategy,
},
types::{
BadPeer, ExtendedPeerInfo, OpaqueStateRequest, OpaqueStateResponse, PeerRequest, SyncEvent,
Expand Down Expand Up @@ -189,7 +189,7 @@ pub struct Peer<B: BlockT> {

pub struct SyncingEngine<B: BlockT, Client> {
/// Syncing strategy.
strategy: SyncingStrategy<B, Client>,
strategy: PolkadotSyncingStrategy<B, Client>,

/// Blockchain client.
client: Arc<Client>,
Expand Down Expand Up @@ -389,7 +389,8 @@ where
);

// Initialize syncing strategy.
let strategy = SyncingStrategy::new(syncing_config, client.clone(), warp_sync_config)?;
let strategy =
PolkadotSyncingStrategy::new(syncing_config, client.clone(), warp_sync_config)?;

let block_announce_protocol_name = block_announce_config.protocol_name().clone();
let (tx, service_rx) = tracing_unbounded("mpsc_chain_sync", 100_000);
Expand Down Expand Up @@ -697,7 +698,7 @@ where
number,
)
},
// Nothing to do, this is handled internally by `SyncingStrategy`.
// Nothing to do, this is handled internally by `PolkadotSyncingStrategy`.
SyncingAction::Finished => {},
}
}
Expand Down
Loading

0 comments on commit 3ffe84c

Please sign in to comment.