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

Companion for paritytech/polkadot#6117 #1749

Merged
merged 5 commits into from
Oct 10, 2022
Merged

Companion for paritytech/polkadot#6117 #1749

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b3d9ae
Companion for paritytech/polkadot#6117
altonen Oct 10, 2022
bf369f8
Fix code
altonen Oct 10, 2022
7ebce8b
Fix trait
altonen Oct 10, 2022
16dc9e6
Merge remote-tracking branch 'origin/master' into move-block-announce…
Oct 10, 2022
b04a64c
update lockfile for {"polkadot", "substrate"}
Oct 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/relay-chain-minimal-node/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ cumulus-relay-chain-interface = { path = "../relay-chain-interface" }
cumulus-relay-chain-rpc-interface = { path = "../relay-chain-rpc-interface" }
cumulus-primitives-core = { path = "../../primitives/core" }

array-bytes = "4.1"
lru = "0.8"
tracing = "0.1.25"
async-trait = "0.1.52"
Expand Down
72 changes: 69 additions & 3 deletions client/relay-chain-minimal-node/src/network.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,21 @@ use polkadot_service::{BlockT, NumberFor};
use polkadot_node_network_protocol::PeerId;
use sc_network::{NetworkService, SyncState};

use sc_network_common::sync::{Metrics, SyncStatus};
use sc_client_api::HeaderBackend;
use sc_network_common::{
config::{
NonDefaultSetConfig, NonReservedPeerMode, NotificationHandshake, ProtocolId, SetConfig,
},
protocol::role::Roles,
sync::{message::BlockAnnouncesHandshake, Metrics, SyncStatus},
};
use sc_network_light::light_client_requests;
use sc_network_sync::{block_request_handler, state_request_handler};
use sc_service::{error::Error, Configuration, NetworkStarter, SpawnTaskHandle};
use sp_consensus::BlockOrigin;
use sp_runtime::Justifications;

use std::sync::Arc;
use std::{iter, sync::Arc};

use crate::BlockChainRpcClient;

Expand Down Expand Up @@ -59,6 +66,16 @@ pub(crate) fn build_collator_network(
let light_client_request_protocol_config =
light_client_requests::generate_protocol_config(&protocol_id, genesis_hash, None);

let chain_sync = DummyChainSync;
let block_announce_config = chain_sync.get_block_announce_proto_config::<Block>(
protocol_id.clone(),
&None,
Roles::from(&config.role),
client.info().best_number,
client.info().best_hash,
genesis_hash,
);

let network_params = sc_network::config::Params {
role: config.role.clone(),
executor: {
Expand All @@ -68,12 +85,13 @@ pub(crate) fn build_collator_network(
}))
},
fork_id: None,
chain_sync: Box::new(DummyChainSync),
chain_sync: Box::new(chain_sync),
network_config: config.network.clone(),
chain: client.clone(),
import_queue: Box::new(DummyImportQueue),
protocol_id,
metrics_registry: config.prometheus_config.as_ref().map(|config| config.registry.clone()),
block_announce_config,
block_request_protocol_config,
state_request_protocol_config,
warp_sync_protocol_config: None,
Expand Down Expand Up @@ -116,6 +134,54 @@ pub(crate) fn build_collator_network(
/// we provide a noop implementation.
struct DummyChainSync;

impl DummyChainSync {
Copy link
Contributor

Choose a reason for hiding this comment

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

This does not need to be on the DummyChainSync no?

Copy link
Contributor Author

@altonen altonen Oct 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

It doesn't have to be but it's part of ChainSync in Substrate so I thought making it part of DummyChainSync would be closest to what we have in Substrate

Copy link
Contributor

Choose a reason for hiding this comment

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

On second thought, it might make more sense to have a separate method in substrate that exposes this. In case something changes we want the cumulus implementation to follow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the vision is to make it more transparent so that you could instantiate some syncing engine implementation that would then install the necessary notification and request protocols to, e.g., sc_network::Params so that there would be no need for calling, e.g. light_client_requests::generate_protocol_config() (except for in the syncing implementation of course) but I'm not opposed to exposing this through Substrate as a temporary measure.

The fire I'm trying to put out right now though is that Polkadot CI is blocked by this PR. How do you want to proceed with this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, I think its fine for now then, we could have a follow up.

pub fn get_block_announce_proto_config<B: BlockT>(
&self,
protocol_id: ProtocolId,
fork_id: &Option<String>,
roles: Roles,
best_number: NumberFor<B>,
best_hash: B::Hash,
genesis_hash: B::Hash,
) -> NonDefaultSetConfig {
let block_announces_protocol = {
let genesis_hash = genesis_hash.as_ref();
if let Some(ref fork_id) = fork_id {
format!(
"/{}/{}/block-announces/1",
array_bytes::bytes2hex("", genesis_hash),
fork_id
)
} else {
format!("/{}/block-announces/1", array_bytes::bytes2hex("", genesis_hash))
}
};

NonDefaultSetConfig {
notifications_protocol: block_announces_protocol.into(),
fallback_names: iter::once(
format!("/{}/block-announces/1", protocol_id.as_ref()).into(),
)
.collect(),
max_notification_size: 1024 * 1024,
handshake: Some(NotificationHandshake::new(BlockAnnouncesHandshake::<B>::build(
roles,
best_number,
best_hash,
genesis_hash,
))),
// NOTE: `set_config` will be ignored by `protocol.rs` as the block announcement
// protocol is still hardcoded into the peerset.
set_config: SetConfig {
in_peers: 0,
out_peers: 0,
reserved_nodes: Vec::new(),
non_reserved_mode: NonReservedPeerMode::Deny,
},
}
}
}

impl<B: BlockT> sc_network_common::sync::ChainSync<B> for DummyChainSync {
fn peer_info(&self, _who: &PeerId) -> Option<sc_network_common::sync::PeerInfo<B>> {
None
Expand Down