Add a blockNumberBytes field in chain specs (#2512)

* Add a `blockNumberBytes` field in chain specs

* PR number

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

bin/full-node/src/run.rs

@@ -230,6 +230,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
             num_events_receivers: 2 + if relay_chain_database.is_some() { 1 } else { 0 },
             chains: iter::once(network_service::ChainConfig {
                 protocol_id: chain_spec.protocol_id().to_owned(),
+                block_number_bytes: usize::from(chain_spec.block_number_bytes()),
                 database: database.clone(),
                 has_grandpa_protocol: matches!(
                     genesis_chain_information.finality,
@@ -279,6 +280,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
                 if let Some(relay_chains_specs) = &relay_chain_spec {
                     Some(network_service::ChainConfig {
                         protocol_id: relay_chains_specs.protocol_id().to_owned(),
+                        block_number_bytes: usize::from(relay_chains_specs.block_number_bytes()),
                         database: relay_chain_database.clone().unwrap(),
                         has_grandpa_protocol: matches!(
                             relay_genesis_chain_information.as_ref().unwrap().finality,
@@ -352,6 +354,7 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
         network_events_receiver: network_events_receivers.next().unwrap(),
         network_service: (network_service.clone(), 0),
         database,
+        block_number_bytes: usize::from(chain_spec.block_number_bytes()),
         keystore,
         jaeger_service: jaeger_service.clone(),
         slot_duration_author_ratio: 43691_u16,
@@ -366,6 +369,9 @@ pub async fn run(cli_options: cli::CliOptionsRun) {
                 network_events_receiver: network_events_receivers.next().unwrap(),
                 network_service: (network_service.clone(), 1),
                 database: relay_chain_database,
+                block_number_bytes: usize::from(
+                    relay_chain_spec.as_ref().unwrap().block_number_bytes(),
+                ),
                 keystore: Arc::new(keystore::Keystore::new(rand::random())),
                 jaeger_service, // TODO: consider passing a different jaeger service with a different service name
                 slot_duration_author_ratio: 43691_u16,

bin/full-node/src/run/consensus_service.rs

@@ -57,6 +57,9 @@ pub struct Config<'a> {
     /// Database to use to read and write information about the chain.
     pub database: Arc<database_thread::DatabaseThread>,
 
+    /// Number of bytes of the block number in the networking protocol.
+    pub block_number_bytes: usize,
+
     /// Stores of key to use for all block-production-related purposes.
     pub keystore: Arc<keystore::Keystore>,
 
@@ -170,7 +173,7 @@ impl ConsensusService {
         (config.tasks_executor)({
             let mut sync = all::AllSync::new(all::Config {
                 chain_information: finalized_chain_information,
-                block_number_bytes: 4, // TODO: pass a proper value; for example load through chain spec
+                block_number_bytes: config.block_number_bytes,
                 allow_unknown_consensus_engines: false,
                 sources_capacity: 32,
                 blocks_capacity: {

bin/full-node/src/run/network_service.rs

@@ -106,6 +106,9 @@ pub struct ChainConfig {
     /// chain, so as to not introduce conflicts in the networking messages.
     pub protocol_id: String,
 
+    /// Number of bytes of the block number in the networking protocol.
+    pub block_number_bytes: usize,
+
     /// If true, the chain uses the GrandPa networking protocol.
     pub has_grandpa_protocol: bool,
 }
@@ -213,7 +216,7 @@ impl NetworkService {
                 in_slots: 25,
                 out_slots: 25,
                 protocol_id: chain.protocol_id.clone(),
-                block_number_bytes: 4, // TODO: correct value, maybe load from chain spec?
+                block_number_bytes: chain.block_number_bytes,
                 best_hash: chain.best_block.1,
                 best_number: chain.best_block.0,
                 genesis_hash: chain.genesis_block_hash,

bin/light-base/src/lib.rs

@@ -339,6 +339,7 @@ struct ChainServices<TPlat: Platform> {
     sync_service: Arc<sync_service::SyncService<TPlat>>,
     runtime_service: Arc<runtime_service::RuntimeService<TPlat>>,
     transactions_service: Arc<transactions_service::TransactionsService<TPlat>>,
+    block_number_bytes: usize,
 }
 
 impl<TPlat: Platform> Clone for ChainServices<TPlat> {
@@ -349,6 +350,7 @@ impl<TPlat: Platform> Clone for ChainServices<TPlat> {
             sync_service: self.sync_service.clone(),
             runtime_service: self.runtime_service.clone(),
             transactions_service: self.transactions_service.clone(),
+            block_number_bytes: self.block_number_bytes,
         }
     }
 }
@@ -1110,6 +1112,7 @@ async fn start_services<TPlat: Platform>(
                     chain_information.as_ref().finalized_block_header.hash(),
                 ),
                 protocol_id: chain_spec.protocol_id().to_string(),
+                block_number_bytes: usize::from(chain_spec.block_number_bytes()),
             }],
         })
         .await;
@@ -1124,6 +1127,7 @@ async fn start_services<TPlat: Platform>(
             sync_service::SyncService::new(sync_service::Config {
                 log_name: log_name.clone(),
                 chain_information: chain_information.clone(),
+                block_number_bytes: usize::from(chain_spec.block_number_bytes()),
                 tasks_executor: Box::new({
                     let new_task_tx = new_task_tx.clone();
                     move |name, fut| new_task_tx.unbounded_send((name, fut)).unwrap()
@@ -1133,7 +1137,7 @@ async fn start_services<TPlat: Platform>(
                 parachain: Some(sync_service::ConfigParachain {
                     parachain_id: chain_spec.relay_chain().unwrap().1,
                     relay_chain_sync: relay_chain.runtime_service.clone(),
-                    relay_chain_block_number_bytes: 4, // TODO: load from chain specs or something
+                    relay_chain_block_number_bytes: relay_chain.block_number_bytes,
                 }),
             })
             .await,
@@ -1165,6 +1169,7 @@ async fn start_services<TPlat: Platform>(
             sync_service::SyncService::new(sync_service::Config {
                 log_name: log_name.clone(),
                 chain_information: chain_information.clone(),
+                block_number_bytes: usize::from(chain_spec.block_number_bytes()),
                 tasks_executor: Box::new({
                     let new_task_tx = new_task_tx.clone();
                     move |name, fut| new_task_tx.unbounded_send((name, fut)).unwrap()
@@ -1220,6 +1225,7 @@ async fn start_services<TPlat: Platform>(
         runtime_service,
         sync_service,
         transactions_service,
+        block_number_bytes: usize::from(chain_spec.block_number_bytes()),
     }
 }
 

bin/light-base/src/network_service.rs

@@ -103,6 +103,9 @@ pub struct ConfigChain {
     /// chain, so as to not introduce conflicts in the networking messages.
     pub protocol_id: String,
 
+    /// Number of bytes of the block number in the networking protocol.
+    pub block_number_bytes: usize,
+
     /// If true, the chain uses the GrandPa networking protocol.
     pub has_grandpa_protocol: bool,
 }
@@ -215,7 +218,7 @@ impl<TPlat: Platform> NetworkService<TPlat> {
                     None
                 },
                 protocol_id: chain.protocol_id.clone(),
-                block_number_bytes: 4, // TODO: correct value, maybe load from chain spec?
+                block_number_bytes: chain.block_number_bytes,
                 best_hash: chain.best_block.1,
                 best_number: chain.best_block.0,
                 genesis_hash: chain.genesis_block_hash,

bin/light-base/src/sync_service.rs

@@ -56,6 +56,9 @@ pub struct Config<TPlat: Platform> {
     /// State of the finalized chain.
     pub chain_information: chain::chain_information::ValidChainInformation,
 
+    /// Number of bytes of the block number in the networking protocol.
+    pub block_number_bytes: usize,
+
     /// Closure that spawns background tasks.
     pub tasks_executor: Box<dyn FnMut(String, future::BoxFuture<'static, ()>) + Send>,
 
@@ -130,6 +133,7 @@ impl<TPlat: Platform> SyncService<TPlat> {
                 Box::pin(standalone::start_standalone_chain(
                     log_target,
                     config.chain_information,
+                    config.block_number_bytes,
                     from_foreground,
                     config.network_service.0.clone(),
                     config.network_service.1,

bin/light-base/src/sync_service/standalone.rs

@@ -39,6 +39,7 @@ use std::{
 pub(super) async fn start_standalone_chain<TPlat: Platform>(
     log_target: String,
     chain_information: chain::chain_information::ValidChainInformation,
+    block_number_bytes: usize,
     mut from_foreground: mpsc::Receiver<ToBackground>,
     network_service: Arc<network_service::NetworkService<TPlat>>,
     network_chain_index: usize,
@@ -47,7 +48,7 @@ pub(super) async fn start_standalone_chain<TPlat: Platform>(
     let mut task = Task {
         sync: all::AllSync::new(all::Config {
             chain_information,
-            block_number_bytes: 4, // TODO: pass a proper value; for example load through chain spec
+            block_number_bytes,
             allow_unknown_consensus_engines: true,
             sources_capacity: 32,
             blocks_capacity: {

bin/wasm-node/CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Added
+
+- Add an optional `blockNumberBytes` field to chain specifications indicating the number of bytes used to encode the block number of the chain. If the field is missing, the value defaults to 4. Prior to this change, the value was always hardcoded to 4. This field is at the moment specific to smoldot, and Substrate will fail to parse chain specifications containing it. ([#2512](https://github.com/paritytech/smoldot/pull/2512))
+
 ## 0.6.24 - 2022-07-14
 
 ### Added

src/chain_spec.rs

@@ -263,6 +263,11 @@ impl ChainSpec {
         }
     }
 
+    /// Returns the number of bytes that the "block number" field of various data structures uses.
+    pub fn block_number_bytes(&self) -> u8 {
+        self.client_spec.block_number_bytes.unwrap_or(4)
+    }
+
     /// Returns true if the chain is of a type for which a live network is expected.
     pub fn has_live_network(&self) -> bool {
         match &self.client_spec.chain_type {

src/chain_spec/structs.rs

@@ -50,6 +50,13 @@ pub(super) struct ClientSpec {
     pub(super) protocol_id: Option<String>,
     #[serde(default = "Default::default", skip_serializing_if = "Option::is_none")]
     pub(super) fork_id: Option<String>,
+    /// The `blockNumberBytes` field is (at the time of writing of this comment) a custom addition
+    /// to the format of smoldot chain specs compared to Substrate. It is necessary because,
+    /// contrary to Substrate, smoldot has no way to know the size of the block number field of
+    /// various data structures. If the field is missing, a value of 4 is assumed.
+    // TODO: revisit this field in the future to maybe bring compatibility with Substrate
+    #[serde(default = "Default::default", skip_serializing_if = "Option::is_none")]
+    pub(super) block_number_bytes: Option<u8>,
     pub(super) properties: Option<Box<serde_json::value::RawValue>>,
     // TODO: make use of this
     pub(super) fork_blocks: Option<Vec<(u64, HashHexString)>>,