From fd0d6a6f8c453c850d5c33df01849f9557bb9724 Mon Sep 17 00:00:00 2001 From: AlexD10S Date: Mon, 2 Dec 2024 16:18:50 +0100 Subject: [PATCH] refactor: improve test, docs and errors --- crates/pop-cli/tests/parachain.rs | 2 +- crates/pop-common/src/metadata.rs | 74 +++++++++++-------- crates/pop-contracts/src/utils/metadata.rs | 5 +- crates/pop-contracts/src/utils/mod.rs | 16 +++- crates/pop-parachains/Cargo.toml | 2 +- .../src/call/metadata/action.rs | 32 +++++--- .../pop-parachains/src/call/metadata/mod.rs | 4 +- .../src/call/metadata/params.rs | 1 - crates/pop-parachains/src/call/mod.rs | 7 +- crates/pop-parachains/src/errors.rs | 8 +- 10 files changed, 93 insertions(+), 58 deletions(-) diff --git a/crates/pop-cli/tests/parachain.rs b/crates/pop-cli/tests/parachain.rs index 1cd6b246..1676bdef 100644 --- a/crates/pop-cli/tests/parachain.rs +++ b/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( diff --git a/crates/pop-common/src/metadata.rs b/crates/pop-common/src/metadata.rs index 82851c8f..53ff8dff 100644 --- a/crates/pop-common/src/metadata.rs +++ b/crates/pop-common/src/metadata.rs @@ -167,49 +167,59 @@ mod tests { OnlineClient::::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, ®istry)); - } - 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: Id(AccountId32 ([u8;32])), Index(Compact<()>), Raw([u8]), Address32([u8;32]), Address20([u8;20])"); // mint_to - assert_eq!(types_formatted[3], "Option { owned_item: Option, mint_price: Option }>: None, Some(MintWitness { owned_item: Option, mint_price: Option })"); // witness_data + let nfts_mint_types: Vec = 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: Id(AccountId32 ([u8;32])), Index(Compact<()>), Raw([u8]), Address32([u8;32]), Address20([u8;20])"); // mint_to + assert_eq!(nfts_mint_types[3], "Option { owned_item: Option, mint_price: Option }>: None, Some(MintWitness { owned_item: Option, mint_price: Option })"); // 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, ®istry)); - } - assert_eq!(types_formatted.len(), 1); - assert_eq!(types_formatted[0], "[u8]"); // remark + let system_remark_types: Vec = 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, ®istry)); - } - 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 = 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(()) } diff --git a/crates/pop-contracts/src/utils/metadata.rs b/crates/pop-contracts/src/utils/metadata.rs index ad1738f6..7e538450 100644 --- a/crates/pop-contracts/src/utils/metadata.rs +++ b/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

( path: P, label: &str, diff --git a/crates/pop-contracts/src/utils/mod.rs b/crates/pop-contracts/src/utils/mod.rs index 3aa266d9..a3a99323 100644 --- a/crates/pop-contracts/src/utils/mod.rs +++ b/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 { 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 { } } +/// 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::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<::AccountId, Error> { ::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 { 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 { /// 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 { // Canonicalize the target path to ensure consistency and resolve any symbolic links. diff --git a/crates/pop-parachains/Cargo.toml b/crates/pop-parachains/Cargo.toml index b6af7d0d..64bef2d6 100644 --- a/crates/pop-parachains/Cargo.toml +++ b/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 diff --git a/crates/pop-parachains/src/call/metadata/action.rs b/crates/pop-parachains/src/call/metadata/action.rs index 63d49ee8..7ee4440d 100644 --- a/crates/pop-parachains/src/call/metadata/action.rs +++ b/crates/pop-parachains/src/call/metadata/action.rs @@ -19,34 +19,39 @@ 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", @@ -54,6 +59,7 @@ pub enum Action { props(Pallet = "Nfts") )] MintNFT, + /// Purchase on-demand coretime. #[strum( serialize = "place_order_allow_death", message = "place_order_allow_death", @@ -61,17 +67,19 @@ pub enum Action { 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() { diff --git a/crates/pop-parachains/src/call/metadata/mod.rs b/crates/pop-parachains/src/call/metadata/mod.rs index 2d6d6b0e..30902676 100644 --- a/crates/pop-parachains/src/call/metadata/mod.rs +++ b/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, diff --git a/crates/pop-parachains/src/call/metadata/params.rs b/crates/pop-parachains/src/call/metadata/params.rs index feb73039..c5b04007 100644 --- a/crates/pop-parachains/src/call/metadata/params.rs +++ b/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 { let type_info = registry.resolve(type_id).ok_or(Error::MetadataParsingError(name.clone()))?; if let Some(last_segment) = type_info.path.segments.last() { diff --git a/crates/pop-parachains/src/call/mod.rs b/crates/pop-parachains/src/call/mod.rs index 794bc0bb..d42bb5a0 100644 --- a/crates/pop-parachains/src/call/mod.rs +++ b/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, Error> { - let api = OnlineClient::::from_url(url).await?; + let api = OnlineClient::::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, tx: &DynamicPayload, ) -> Result { - 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))) } diff --git a/crates/pop-parachains/src/errors.rs b/crates/pop-parachains/src/errors.rs index 5031a934..585e9f78 100644 --- a/crates/pop-parachains/src/errors.rs +++ b/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}")]