[DRAFT] fixes and changes related to GetParachainInfo

.github/workflows/publish-check-compile.yml

@@ -48,7 +48,9 @@ jobs:
 
       - name: parity-publish update plan w/o current prdoc
         run: |
-          mv prdoc/$CURRENT_PRDOC .
+          if [ -f prdoc/$CURRENT_PRDOC ]; then
+            mv prdoc/$CURRENT_PRDOC .
+          fi
           parity-publish --color always plan --skip-check --prdoc prdoc/
 
       # The code base is not in master's state (due to commits brought by the
@@ -61,17 +63,23 @@ jobs:
 
       - name: move all prdocs except current one to unstable dir
         run: |
-          mkdir prdoc/unstable
-          mv prdoc/pr_*.prdoc prdoc/unstable
           if [ -f $CURRENT_PRDOC ]; then
+            mkdir prdoc/unstable
+            mv prdoc/pr_*.prdoc prdoc/unstable
             mv $CURRENT_PRDOC prdoc
           fi
 
       - name: parity-publish update plan just for PR's prdoc
-        run: parity-publish --color always plan --skip-check --prdoc prdoc/
+        run: |
+          if [ -f "prdoc/$CURRENT_PRDOC" ]; then
+            parity-publish --color always plan --skip-check --prdoc prdoc/
+          fi
 
       - name: parity-publish apply plan
-        run: parity-publish --color always apply --registry
+        run: |
+          if [ -f "prdoc/$CURRENT_PRDOC" ]; then
+            parity-publish --color always apply --registry
+          fi
 
       - name: parity-publish check compile
         run: |

cumulus/polkadot-omni-node/README.md

@@ -46,11 +46,11 @@ local variants are available only for a build of `polkadot-omni-node` with
 `westend-native` and `rococo-native` features respectively.
 
 <!-- TODO: https://github.com/paritytech/polkadot-sdk/issues/8747 -->
-<!-- TODO: https://github.com/paritytech/polkadot-sdk/issues/8740 -->
-Additionaly, although deprecated, the `--para-id` flag can still be used to set the JSON key named
-`para_id`. The removal of the flag will happen starting with `stable2512`. The alternative of not using
-it is to implement the `cumulus_primitives_core::GetParachainInfo` runtime API for the runtime, and
-upgrade it on-chain as well, to be compatible with nodes released starting with `stable2512`.
+Additionaly, the `--para-id` flag can be used to set the JSON key named `para_id`. This flag is used
+by nodes to determine the parachain id, and it is especially useful when the parachain id can not be
+fetched from the runtime, when the state points to a runtime that does not implement the
+`cumulus_primitives_core::GetParachainInfo` runtime API. It is recommended for runtimes to implement
+the runtime API and be upgraded on chain.
 
 Example command bellow:
 

cumulus/polkadot-omni-node/lib/src/cli.rs

@@ -316,7 +316,7 @@ impl<Config: CliConfig> RelayChainCli<Config> {
 		let base = FromArgMatches::from_arg_matches(&matches).unwrap_or_else(|e| e.exit());
 
 		let extension = Extensions::try_get(&*para_config.chain_spec);
-		let chain_id = extension.map(|e| e.relay_chain.clone());
+		let chain_id = extension.map(|e| e.relay_chain());
 
 		let base_path = para_config.base_path.path().join("polkadot");
 		Self { base, chain_id, base_path: Some(base_path), _phantom: Default::default() }

cumulus/polkadot-omni-node/lib/src/common/chain_spec.rs

@@ -32,59 +32,52 @@ pub struct DiskChainSpecLoader;
 
 impl LoadSpec for DiskChainSpecLoader {
 	fn load_spec(&self, path: &str) -> Result<Box<dyn ChainSpec>, String> {
-		Ok(Box::new(DeprecatedGenericChainSpec::from_json_file(path.into())?))
-	}
-}
-
-/// Generic extensions for Parachain ChainSpecs.
-#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, ChainSpecExtension)]
-pub struct Extensions {
-	/// The relay chain of the Parachain.
-	#[serde(alias = "relayChain", alias = "RelayChain")]
-	pub relay_chain: String,
-}
-
-impl Extensions {
-	/// Try to get the extension from the given `ChainSpec`.
-	pub fn try_get(chain_spec: &dyn sc_service::ChainSpec) -> Option<&Self> {
-		sc_chain_spec::get_extension(chain_spec.extensions())
+		Ok(Box::new(GenericChainSpec::from_json_file(path.into())?))
 	}
 }
 
 /// Generic extensions for Parachain ChainSpecs used for extracting the extensions from chain specs.
-/// This is also used only while `para_id` is around the corner.
-// TODO: https://github.com/paritytech/polkadot-sdk/issues/8747
-// TODO: https://github.com/paritytech/polkadot-sdk/issues/8740
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, ChainSpecExtension)]
-pub struct DeprecatedExtensions {
+pub struct Extensions {
 	/// The relay chain of the Parachain. It is kept here only for compatibility reasons until
 	/// people migrate to using the new `Extensions` struct and associated logic in the node
 	/// corresponding to pulling the parachain id from the runtime.
 	#[serde(alias = "relayChain", alias = "RelayChain")]
-	pub relay_chain: String,
+	relay_chain: String,
 	/// The id of the Parachain.
 	#[serde(alias = "paraId", alias = "ParaId")]
-	#[deprecated(
-		note = "The para_id information is not required anymore and will be removed starting with `stable2512`. Runtimes must implement a new API called `cumulus_primitives_core::GetParachainInfo` to still be compatible with node versions starting with `stable2512`."
-	)]
-	pub para_id: Option<u32>,
+	para_id: Option<u32>,
 }
 
-impl DeprecatedExtensions {
+impl Extensions {
 	/// Try to get the extension from the given `ChainSpec`.
 	pub fn try_get(chain_spec: &dyn sc_service::ChainSpec) -> Option<&Self> {
 		sc_chain_spec::get_extension(chain_spec.extensions())
 	}
+
+	/// Create the extensions only with the relay_chain.
+	pub fn new_with_relay_chain(relay_chain: String) -> Self {
+		Extensions { relay_chain, para_id: None }
+	}
+
+	/// Initialize extensions based on given parameters.
+	pub fn new(relay_chain: String, para_id: u32) -> Self {
+		Extensions { relay_chain, para_id: Some(para_id) }
+	}
+
+	/// Para id field getter
+	pub fn para_id(&self) -> Option<u32> {
+		self.para_id
+	}
+
+	/// Relay chain field getter
+	pub fn relay_chain(&self) -> String {
+		self.relay_chain.clone()
+	}
 }
 
 /// Generic chain spec for all polkadot-parachain runtimes
 pub type GenericChainSpec = sc_service::GenericChainSpec<Extensions>;
-/// Generic chain spec which keeps chain spec loading compatible for those who provide
-/// `para_id` extension instead of implementing the runtime API
-/// `cumulus_primitives_core::GetParachainInfo`.
-// TODO: https://github.com/paritytech/polkadot-sdk/issues/8747
-// TODO: https://github.com/paritytech/polkadot-sdk/issues/8740
-pub type DeprecatedGenericChainSpec = sc_service::GenericChainSpec<DeprecatedExtensions>;
 
 #[cfg(test)]
 mod tests {
@@ -97,21 +90,13 @@ mod tests {
 		let pascal_case = r#"{"RelayChain":"relay","ParaId":1}"#;
 		let para_id_missing = r#"{"RelayChain":"westend"}"#;
 
-		let camel_case_extension: DeprecatedExtensions = serde_json::from_str(camel_case).unwrap();
-		let snake_case_extension: DeprecatedExtensions = serde_json::from_str(snake_case).unwrap();
-		let pascal_case_extension: DeprecatedExtensions =
-			serde_json::from_str(pascal_case).unwrap();
+		let camel_case_extension: Extensions = serde_json::from_str(camel_case).unwrap();
+		let snake_case_extension: Extensions = serde_json::from_str(snake_case).unwrap();
+		let pascal_case_extension: Extensions = serde_json::from_str(pascal_case).unwrap();
 		let missing_paraid_extension: Extensions = serde_json::from_str(para_id_missing).unwrap();
-		let missing_paraid_deprecated: DeprecatedExtensions =
-			serde_json::from_str(para_id_missing).unwrap();
 		assert_eq!(camel_case_extension, snake_case_extension);
 		assert_eq!(snake_case_extension, pascal_case_extension);
 		assert_eq!(missing_paraid_extension.relay_chain, "westend".to_string());
-
-		// TODO: remove it once `para_id` is removed: https://github.com/paritytech/polkadot-sdk/issues/8740
-		assert_eq!(missing_paraid_deprecated.relay_chain, "westend".to_string());
-		#[allow(deprecated)]
-		let test = missing_paraid_deprecated.para_id.is_none();
-		assert!(test);
+		assert!(missing_paraid_extension.para_id.is_none());
 	}
 }

cumulus/polkadot-omni-node/lib/src/common/spec.rs

@@ -15,7 +15,7 @@
 // limitations under the License.
 
 use crate::{
-	chain_spec::DeprecatedExtensions,
+	chain_spec::Extensions,
 	common::{
 		command::NodeCommandRunner,
 		rpc::BuildRpcExtensions,
@@ -51,7 +51,7 @@ use sc_telemetry::{TelemetryHandle, TelemetryWorker};
 use sc_tracing::tracing::Instrument;
 use sc_transaction_pool::TransactionPoolHandle;
 use sc_transaction_pool_api::OffchainTransactionPoolFactory;
-use sp_api::ProvideRuntimeApi;
+use sp_api::{ApiExt, ProvideRuntimeApi};
 use sp_keystore::KeystorePtr;
 use sp_runtime::traits::AccountIdConversion;
 use std::{future::Future, pin::Pin, sync::Arc, time::Duration};
@@ -157,19 +157,27 @@ pub(crate) trait BaseNodeSpec {
 		parachain_config: &Configuration,
 	) -> Option<ParaId> {
 		let best_hash = client.chain_info().best_hash;
-		let para_id = if let Ok(para_id) = client.runtime_api().parachain_id(best_hash) {
-			para_id
+		let para_id = if client
+			.runtime_api()
+			.has_api::<dyn GetParachainInfo<Self::Block>>(best_hash)
+			.ok()
+			.filter(|has_api| *has_api)
+			.is_some()
+		{
+			client
+				.runtime_api()
+				.parachain_id(best_hash)
+				.inspect_err(|err| {
+					log::error!(
+								"`cumulus_primitives_core::GetParachainInfo` runtime API call errored with {}",
+								err
+							);
+				})
+				.ok()?
 		} else {
-			// TODO: remove this once `para_id` extension is removed: https://github.com/paritytech/polkadot-sdk/issues/8740
-			#[allow(deprecated)]
-			let id = ParaId::from(
-				DeprecatedExtensions::try_get(&*parachain_config.chain_spec)
-					.and_then(|ext| ext.para_id)?,
-			);
-			// TODO: https://github.com/paritytech/polkadot-sdk/issues/8747
-			// TODO: https://github.com/paritytech/polkadot-sdk/issues/8740
-			log::info!("Deprecation notice: the parachain id was provided via the chain spec. This way of providing the parachain id to the node is not recommended. The alternative is to implement the `cumulus_primitives_core::GetParachainInfo` runtime API in the runtime, and upgrade it on-chain. Starting with `stable2512` providing the parachain id via the chain spec will not be supported anymore.");
-			id
+			ParaId::from(
+				Extensions::try_get(&*parachain_config.chain_spec).and_then(|ext| ext.para_id())?,
+			)
 		};
 
 		let parachain_account =

cumulus/polkadot-parachain/src/chain_spec/asset_hubs.rs

@@ -25,7 +25,7 @@ pub fn asset_hub_westend_development_config() -> GenericChainSpec {
 	GenericChainSpec::builder(
 		asset_hub_westend_runtime::WASM_BINARY
 			.expect("WASM binary was not built, please build it!"),
-		Extensions { relay_chain: "westend".into() },
+		Extensions::new_with_relay_chain("westend".into()),
 	)
 	.with_name("Westend Asset Hub Development")
 	.with_id("asset-hub-westend-dev")
@@ -43,7 +43,7 @@ pub fn asset_hub_westend_local_config() -> GenericChainSpec {
 	GenericChainSpec::builder(
 		asset_hub_westend_runtime::WASM_BINARY
 			.expect("WASM binary was not built, please build it!"),
-		Extensions { relay_chain: "westend-local".into() },
+		Extensions::new_with_relay_chain("westend-local".into()),
 	)
 	.with_name("Westend Asset Hub Local")
 	.with_id("asset-hub-westend-local")
@@ -61,7 +61,7 @@ pub fn asset_hub_westend_config() -> GenericChainSpec {
 	GenericChainSpec::builder(
 		asset_hub_westend_runtime::WASM_BINARY
 			.expect("WASM binary was not built, please build it!"),
-		Extensions { relay_chain: "westend".into() },
+		Extensions::new_with_relay_chain("westend".into()),
 	)
 	.with_name("Westend Asset Hub")
 	.with_id("asset-hub-westend")
@@ -80,29 +80,22 @@ pub fn asset_hub_rococo_development_config() -> GenericChainSpec {
 		properties,
 		"Rococo Asset Hub Development",
 		"asset-hub-rococo-dev",
-		1000,
 	)
 }
 
 fn asset_hub_rococo_like_development_config(
 	properties: sc_chain_spec::Properties,
 	name: &str,
 	chain_id: &str,
-	para_id: u32,
 ) -> GenericChainSpec {
 	GenericChainSpec::builder(
 		asset_hub_rococo_runtime::WASM_BINARY.expect("WASM binary was not built, please build it!"),
-		Extensions { relay_chain: "rococo-dev".into() },
+		Extensions::new_with_relay_chain("rococo-dev".into()),
 	)
 	.with_name(name)
 	.with_id(chain_id)
 	.with_chain_type(ChainType::Local)
 	.with_genesis_config_preset_name(sp_genesis_builder::DEV_RUNTIME_PRESET)
-	.with_genesis_config_patch(serde_json::json!({
-		"parachainInfo": {
-			"parachainId": para_id,
-		},
-	}))
 	.with_properties(properties)
 	.build()
 }
@@ -116,29 +109,22 @@ pub fn asset_hub_rococo_local_config() -> GenericChainSpec {
 		properties,
 		"Rococo Asset Hub Local",
 		"asset-hub-rococo-local",
-		1000,
 	)
 }
 
 fn asset_hub_rococo_like_local_config(
 	properties: sc_chain_spec::Properties,
 	name: &str,
 	chain_id: &str,
-	para_id: u32,
 ) -> GenericChainSpec {
 	GenericChainSpec::builder(
 		asset_hub_rococo_runtime::WASM_BINARY.expect("WASM binary was not built, please build it!"),
-		Extensions { relay_chain: "rococo-local".into() },
+		Extensions::new_with_relay_chain("rococo-local".into()),
 	)
 	.with_name(name)
 	.with_id(chain_id)
 	.with_chain_type(ChainType::Local)
 	.with_genesis_config_preset_name(sp_genesis_builder::LOCAL_TESTNET_RUNTIME_PRESET)
-	.with_genesis_config_patch(serde_json::json!({
-		"parachainInfo": {
-			"parachainId": para_id,
-		},
-	}))
 	.with_properties(properties)
 	.build()
 }
@@ -149,7 +135,7 @@ pub fn asset_hub_rococo_genesis_config() -> GenericChainSpec {
 	properties.insert("tokenDecimals".into(), 12.into());
 	GenericChainSpec::builder(
 		asset_hub_rococo_runtime::WASM_BINARY.expect("WASM binary was not built, please build it!"),
-		Extensions { relay_chain: "rococo".into() },
+		Extensions::new_with_relay_chain("rococo".into()),
 	)
 	.with_name("Rococo Asset Hub")
 	.with_id("asset-hub-rococo")