refactor: improve test, docs and errors

crates/pop-cli/tests/parachain.rs

@@ -87,7 +87,7 @@ async fn parachain_lifecycle() -> Result<()> {
 	assert!(content.contains("\"relay_chain\": \"paseo-local\""));
 	assert!(content.contains("\"protocolId\": \"pop-protocol\""));
 
-	// Custom network to manually set the port
+	// Overwrite the config file to manually set the port to test pop call parachain.
 	let network_toml_path = temp_parachain_dir.join("network.toml");
 	fs::create_dir_all(&temp_parachain_dir)?;
 	fs::write(

crates/pop-common/src/metadata.rs

@@ -167,49 +167,59 @@ mod tests {
 			OnlineClient::<SubstrateConfig>::from_url("wss://rpc1.paseo.popnetwork.xyz").await?;
 		let metadata = api.metadata();
 		let registry = metadata.types();
-		// Extrinsic Nfts::mint to test the majority of expresions.
-		let mut extrinsic =
+
+		// Validate `Nfts::mint` extrinsic types cover most of cases.
+		let nfts_mint_extrinsic =
 			metadata.pallet_by_name("Nfts").unwrap().call_variant_by_name("mint").unwrap();
-		let mut types_formatted = Vec::new();
-		for field in &extrinsic.fields {
-			let type_info = registry.resolve(field.ty.id).unwrap();
-			types_formatted.push(format_type(&type_info, &registry));
-		}
-		assert_eq!(types_formatted.len(), 4);
-		assert_eq!(types_formatted[0], "u32"); // collection
-		assert_eq!(types_formatted[1], "u32"); // item
-		assert_eq!(types_formatted[2], "MultiAddress<AccountId32 ([u8;32]),()>: Id(AccountId32 ([u8;32])), Index(Compact<()>), Raw([u8]), Address32([u8;32]), Address20([u8;20])"); // mint_to
-		assert_eq!(types_formatted[3], "Option<MintWitness<u32,u128> { owned_item: Option<ItemId>, mint_price: Option<Balance> }>: None, Some(MintWitness<u32,u128> { owned_item: Option<ItemId>, mint_price: Option<Balance> })"); // witness_data
+		let nfts_mint_types: Vec<String> = nfts_mint_extrinsic
+			.fields
+			.iter()
+			.map(|field| {
+				let type_info = registry.resolve(field.ty.id).unwrap();
+				format_type(&type_info, registry)
+			})
+			.collect();
+		assert_eq!(nfts_mint_types.len(), 4);
+		assert_eq!(nfts_mint_types[0], "u32"); // collection
+		assert_eq!(nfts_mint_types[1], "u32"); // item
+		assert_eq!(nfts_mint_types[2], "MultiAddress<AccountId32 ([u8;32]),()>: Id(AccountId32 ([u8;32])), Index(Compact<()>), Raw([u8]), Address32([u8;32]), Address20([u8;20])"); // mint_to
+		assert_eq!(nfts_mint_types[3], "Option<MintWitness<u32,u128> { owned_item: Option<ItemId>, mint_price: Option<Balance> }>: None, Some(MintWitness<u32,u128> { owned_item: Option<ItemId>, mint_price: Option<Balance> })"); // witness_data
 
-		// Extrinsic Sytem::remark to testing sequences.
-		extrinsic = metadata
+		//  Validate `System::remark` to cover Sequences.
+		let system_remark_extrinsic = metadata
 			.pallet_by_name("System")
 			.unwrap()
 			.call_variant_by_name("remark")
 			.unwrap();
-		types_formatted.clear();
-		for field in &extrinsic.fields {
-			let type_info = registry.resolve(field.ty.id).unwrap();
-			types_formatted.push(format_type(&type_info, &registry));
-		}
-		assert_eq!(types_formatted.len(), 1);
-		assert_eq!(types_formatted[0], "[u8]"); // remark
+		let system_remark_types: Vec<String> = system_remark_extrinsic
+			.fields
+			.iter()
+			.map(|field| {
+				let type_info = registry.resolve(field.ty.id).unwrap();
+				format_type(&type_info, registry)
+			})
+			.collect();
+		assert_eq!(system_remark_types.len(), 1);
+		assert_eq!(system_remark_types[0], "[u8]"); // remark
 
-		// Extrinsic Scheduler::set_retry to test tuples.
-		extrinsic = metadata
+		// Extrinsic Scheduler::set_retry, cover tuples.
+		let scheduler_set_retry_extrinsic = metadata
 			.pallet_by_name("Scheduler")
 			.unwrap()
 			.call_variant_by_name("set_retry")
 			.unwrap();
-		types_formatted.clear();
-		for field in &extrinsic.fields {
-			let type_info = registry.resolve(field.ty.id).unwrap();
-			types_formatted.push(format_type(&type_info, &registry));
-		}
-		assert_eq!(types_formatted.len(), 3);
-		assert_eq!(types_formatted[0], "(u32,u32)"); // task
-		assert_eq!(types_formatted[1], "u8"); // retries
-		assert_eq!(types_formatted[2], "u32"); // period
+		let scheduler_set_retry_types: Vec<String> = scheduler_set_retry_extrinsic
+			.fields
+			.iter()
+			.map(|field| {
+				let type_info = registry.resolve(field.ty.id).unwrap();
+				format_type(&type_info, registry)
+			})
+			.collect();
+		assert_eq!(scheduler_set_retry_types.len(), 3);
+		assert_eq!(scheduler_set_retry_types[0], "(u32,u32)"); // task
+		assert_eq!(scheduler_set_retry_types[1], "u8"); // retries
+		assert_eq!(scheduler_set_retry_types[2], "u32"); // period
 
 		Ok(())
 	}

crates/pop-contracts/src/utils/metadata.rs

@@ -158,12 +158,13 @@ fn process_args(
 }
 
 /// Processes a list of argument values for a specified contract function,
-/// wrapping each value in `Some(...)` or replacing it with `None` if the argument is optional
+/// wrapping each value in `Some(...)` or replacing it with `None` if the argument is optional.
 ///
 /// # Arguments
-/// * `path` -  Location path of the project.
+/// * `path` -  Location path of the project or contract artifact.
 /// * `label` - Label of the contract message to retrieve.
 /// * `args` - Argument values provided by the user.
+/// * `function_type` - Specifies whether to process arguments of messages or constructors
 pub fn process_function_args<P>(
 	path: P,
 	label: &str,

crates/pop-contracts/src/utils/mod.rs

@@ -13,6 +13,10 @@ use std::{
 
 pub mod metadata;
 
+/// Retrieves the manifest path for a contract project.
+///
+/// # Arguments
+/// * `path` - An optional path to the project directory.
 pub fn get_manifest_path(path: Option<&Path>) -> Result<ManifestPath, Error> {
 	if let Some(path) = path {
 		let full_path = PathBuf::from(path.to_string_lossy().to_string() + "/Cargo.toml");
@@ -24,18 +28,29 @@ pub fn get_manifest_path(path: Option<&Path>) -> Result<ManifestPath, Error> {
 	}
 }
 
+/// Parses a balance value from a string representation.
+///
+/// # Arguments
+/// * `balance` - A string representing the balance value to parse.
 pub fn parse_balance(
 	balance: &str,
 ) -> Result<BalanceVariant<<DefaultEnvironment as Environment>::Balance>, Error> {
 	BalanceVariant::from_str(balance).map_err(|e| Error::BalanceParsing(format!("{}", e)))
 }
 
+/// Parses an account ID from its string representation.
+///
+/// # Arguments
+/// * `account` - A string representing the account ID to parse.
 pub fn parse_account(account: &str) -> Result<<DefaultConfig as Config>::AccountId, Error> {
 	<DefaultConfig as Config>::AccountId::from_str(account)
 		.map_err(|e| Error::AccountAddressParsing(format!("{}", e)))
 }
 
 /// Parse hex encoded bytes.
+///
+/// # Arguments
+/// * `input` - A string containing hex-encoded bytes.
 pub fn parse_hex_bytes(input: &str) -> Result<Bytes, Error> {
 	let bytes = decode_hex(input).map_err(|e| Error::HexParsing(format!("{}", e)))?;
 	Ok(bytes.into())
@@ -44,7 +59,6 @@ pub fn parse_hex_bytes(input: &str) -> Result<Bytes, Error> {
 /// Canonicalizes the given path to ensure consistency and resolve any symbolic links.
 ///
 /// # Arguments
-///
 /// * `target` - A reference to the `Path` to be canonicalized.
 pub fn canonicalized_path(target: &Path) -> Result<PathBuf, Error> {
 	// Canonicalize the target path to ensure consistency and resolve any symbolic links.

crates/pop-parachains/Cargo.toml

@@ -22,9 +22,9 @@ tempfile.workspace = true
 thiserror.workspace = true
 tokio.workspace = true
 url.workspace = true
-hex.workspace = true 
 
 askama.workspace = true
+hex.workspace = true 
 indexmap.workspace = true
 reqwest.workspace = true
 scale-info.workspace = true

crates/pop-parachains/src/call/metadata/action.rs

@@ -19,59 +19,67 @@ use strum_macros::{AsRefStr, Display, EnumMessage, EnumProperty, EnumString, Var
 	VariantArray,
 )]
 pub enum Action {
+	/// Transfer balance.
 	#[strum(
 		serialize = "transfer",
 		message = "transfer_allow_death",
-		detailed_message = "Transfer Balance",
+		detailed_message = "Transfer balance",
 		props(Pallet = "Balances")
 	)]
 	Transfer,
+	/// Create an asset.
 	#[strum(
 		serialize = "create",
 		message = "create",
-		detailed_message = "Create an Asset",
+		detailed_message = "Create an asset",
 		props(Pallet = "Assets")
 	)]
 	CreateAsset,
+	/// Mint an asset.
 	#[strum(
 		serialize = "mint",
 		message = "mint",
-		detailed_message = "Mint an Asset",
+		detailed_message = "Mint an asset",
 		props(Pallet = "Assets")
 	)]
 	MintAsset,
+	/// Create an NFT collection.
 	#[strum(
 		serialize = "create_nft",
 		message = "create",
-		detailed_message = "Create an NFT Collection",
+		detailed_message = "Create an NFT collection",
 		props(Pallet = "Nfts")
 	)]
 	CreateCollection,
+	/// Mint an NFT.
 	#[strum(
 		serialize = "mint_nft",
 		message = "mint",
 		detailed_message = "Mint an NFT",
 		props(Pallet = "Nfts")
 	)]
 	MintNFT,
+	/// Purchase on-demand coretime.
 	#[strum(
 		serialize = "place_order_allow_death",
 		message = "place_order_allow_death",
 		detailed_message = "Purchase on-demand coretime",
 		props(Pallet = "OnDemand")
 	)]
 	PurchaseOnDemandCoretime,
+	/// Reserve a parachain ID.
 	#[strum(
 		serialize = "reserve",
 		message = "reserve",
-		detailed_message = "Reserve para id",
+		detailed_message = "Reserve a parachain ID",
 		props(Pallet = "Registrar")
 	)]
 	Reserve,
+	/// Register a parachain ID with genesis state and code.
 	#[strum(
 		serialize = "register",
 		message = "register",
-		detailed_message = "Register para id with genesis state and code",
+		detailed_message = "Register a parachain ID with genesis state and code",
 		props(Pallet = "Registrar")
 	)]
 	Register,
@@ -122,14 +130,14 @@ mod tests {
 	#[test]
 	fn action_descriptions_are_correct() {
 		let descriptions = HashMap::from([
-			(Action::CreateAsset, "Create an Asset"),
-			(Action::MintAsset, "Mint an Asset"),
-			(Action::CreateCollection, "Create an NFT Collection"),
+			(Action::CreateAsset, "Create an asset"),
+			(Action::MintAsset, "Mint an asset"),
+			(Action::CreateCollection, "Create an NFT collection"),
 			(Action::MintNFT, "Mint an NFT"),
 			(Action::PurchaseOnDemandCoretime, "Purchase on-demand coretime"),
-			(Action::Transfer, "Transfer Balance"),
-			(Action::Register, "Register para id with genesis state and code"),
-			(Action::Reserve, "Reserve para id"),
+			(Action::Transfer, "Transfer balance"),
+			(Action::Register, "Register a parachain ID with genesis state and code"),
+			(Action::Reserve, "Reserve a parachain ID"),
 		]);
 
 		for action in Action::VARIANTS.iter() {

crates/pop-parachains/src/call/metadata/mod.rs

@@ -9,8 +9,8 @@ use subxt::{dynamic::Value, Metadata, OnlineClient, SubstrateConfig};
 pub mod action;
 pub mod params;
 
-#[derive(Clone, PartialEq, Eq)]
 /// Represents a pallet in the blockchain, including its extrinsics.
+#[derive(Clone, PartialEq, Eq)]
 pub struct Pallet {
 	/// The name of the pallet.
 	pub name: String,
@@ -26,8 +26,8 @@ impl Display for Pallet {
 	}
 }
 
-#[derive(Clone, PartialEq, Eq, Debug)]
 /// Represents an extrinsic in a pallet.
+#[derive(Clone, PartialEq, Eq, Debug)]
 pub struct Extrinsic {
 	/// The name of the extrinsic.
 	pub name: String,

crates/pop-parachains/src/call/metadata/params.rs

@@ -43,7 +43,6 @@ pub fn field_to_param(
 /// * `name`: The name of the parameter.
 /// * `registry`: A reference to the `PortableRegistry` to resolve type dependencies.
 /// * `type_id`: The ID of the type to be converted.
-/// * `type_name`: An optional descriptive name for the type.
 fn type_to_param(name: String, registry: &PortableRegistry, type_id: u32) -> Result<Param, Error> {
 	let type_info = registry.resolve(type_id).ok_or(Error::MetadataParsingError(name.clone()))?;
 	if let Some(last_segment) = type_info.path.segments.last() {

crates/pop-parachains/src/call/mod.rs

@@ -15,7 +15,9 @@ pub mod metadata;
 /// # Arguments
 /// * `url` - Endpoint of the node.
 pub async fn set_up_api(url: &str) -> Result<OnlineClient<SubstrateConfig>, Error> {
-	let api = OnlineClient::<SubstrateConfig>::from_url(url).await?;
+	let api = OnlineClient::<SubstrateConfig>::from_url(url)
+		.await
+		.map_err(|e| Error::ApiConnectionFailure(e.to_string()))?;
 	Ok(api)
 }
 
@@ -66,7 +68,8 @@ pub fn encode_call_data(
 	api: &OnlineClient<SubstrateConfig>,
 	tx: &DynamicPayload,
 ) -> Result<String, Error> {
-	let call_data = tx.encode_call_data(&api.metadata())?;
+	let call_data =
+		tx.encode_call_data(&api.metadata()).map_err(|_| Error::CallDataEncodingError)?;
 	Ok(format!("0x{}", hex::encode(call_data)))
 }
 

crates/pop-parachains/src/errors.rs

@@ -9,6 +9,10 @@ pub enum Error {
 	Aborted,
 	#[error("Anyhow error: {0}")]
 	AnyhowError(#[from] anyhow::Error),
+	#[error("Failed to establish a connection to the API: {0}")]
+	ApiConnectionFailure(String),
+	#[error("Failed to encode call data for the extrinsic.")]
+	CallDataEncodingError,
 	#[error("{0}")]
 	CommonError(#[from] pop_common::Error),
 	#[error("Configuration error: {0}")]
@@ -47,10 +51,6 @@ pub enum Error {
 	RustfmtError(std::io::Error),
 	#[error("Template error: {0}")]
 	SourcingError(#[from] pop_common::sourcing::Error),
-	#[error("{0}")]
-	SubxtError(#[from] subxt::Error),
-	#[error("{0}")]
-	SubxtExternalError(#[from] subxt::ext::subxt_core::Error),
 	#[error("Toml error: {0}")]
 	TomlError(#[from] toml_edit::de::Error),
 	#[error("Unsupported command: {0}")]