fix: parse user inputs for Option arguments (#332)

* fix: automatically add some or none to Option argument

* test: refactor and tests

* refactor: improve code and comments

* fix: renaming and clean code

* chore: option params not mandatory

* fix: parse user inputs for Option arguments in constructor (#335)

* fix: automatically add some or none to Option argument

* fix: tests

* refactor: process_function_args

* test: update tests accordingly last changes

* fix: issue with delimiter

* test: fix unit test

* refactor: renaming and fix comments

* refactor: format types (#339)

Shows the full type representation, making it easier to see the entry format of parameter values.

* fix: logo doesn't show in README

---------

Co-authored-by: Frank Bell <60948618+evilrobot-01@users.noreply.github.com>
Co-authored-by: Alejandro Martinez Andres <11448715+al3mart@users.noreply.github.com>

README.md

@@ -1,6 +1,6 @@
 # Pop CLI
 
-<img src="https://github.com/r0gue-io/pop-cli/blob/main/.icons/logo.jpeg"></img>
+<img src=".icons/logo.jpeg"></img>
 
 <div align="center">
 

crates/pop-cli/src/commands/call/contract.rs

@@ -30,7 +30,7 @@ pub struct CallContractCommand {
 	#[clap(long, short)]
 	message: Option<String>,
 	/// The constructor arguments, encoded as strings.
-	#[clap(long, num_args = 0..)]
+	#[clap(long, num_args = 0..,)]
 	args: Vec<String>,
 	/// The value to be transferred as part of the call.
 	#[clap(name = "value", short = 'v', long, default_value = DEFAULT_PAYABLE_VALUE)]
@@ -96,7 +96,8 @@ impl CallContractCommand {
 			full_message.push_str(&format!(" --message {}", message));
 		}
 		if !self.args.is_empty() {
-			full_message.push_str(&format!(" --args {}", self.args.join(" ")));
+			let args: Vec<_> = self.args.iter().map(|a| format!("\"{a}\"")).collect();
+			full_message.push_str(&format!(" --args {}", args.join(", ")));
 		}
 		if self.value != DEFAULT_PAYABLE_VALUE {
 			full_message.push_str(&format!(" --value {}", self.value));
@@ -176,7 +177,7 @@ impl CallContractCommand {
 		};
 
 		// Parse the contract metadata provided. If there is an error, do not prompt for more.
-		let messages = match get_messages(&contract_path) {
+		let messages = match get_messages(contract_path) {
 			Ok(messages) => messages,
 			Err(e) => {
 				return Err(anyhow!(format!(
@@ -230,11 +231,15 @@ impl CallContractCommand {
 		// Resolve message arguments.
 		let mut contract_args = Vec::new();
 		for arg in &message.args {
-			contract_args.push(
-				cli.input(format!("Enter the value for the parameter: {}", arg.label))
-					.placeholder(&format!("Type required: {}", &arg.type_name))
-					.interact()?,
-			);
+			let mut input = cli
+				.input(format!("Enter the value for the parameter: {}", arg.label))
+				.placeholder(&format!("Type required: {}", arg.type_name));
+
+			// Set default input only if the parameter type is `Option` (Not mandatory)
+			if arg.type_name.starts_with("Option<") {
+				input = input.default_input("");
+			}
+			contract_args.push(input.interact()?);
 		}
 		self.args = contract_args;
 
@@ -671,6 +676,7 @@ mod tests {
 			.expect_input("Enter the proof size limit:", "".into()) // Only if call
 			.expect_input("Enter the gas limit:", "".into()) // Only if call
 			.expect_input("Value to transfer to the call:", "50".into()) // Only if payable
+			.expect_input("Enter the value for the parameter: number", "2".into()) // Args for specific_flip
 			.expect_input("Enter the value for the parameter: new_value", "true".into()) // Args for specific_flip
 			.expect_select::<PathBuf>(
 				"Select the message to call:",
@@ -691,7 +697,7 @@ mod tests {
 				"Where is your project located?",
 				temp_dir.path().join("testing").display().to_string(),
 			).expect_info(format!(
-				"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args true --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
+				"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args \"true\", \"2\" --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
 				temp_dir.path().join("testing").display().to_string(),
 			));
 
@@ -715,8 +721,9 @@ mod tests {
 			Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string())
 		);
 		assert_eq!(call_config.message, Some("specific_flip".to_string()));
-		assert_eq!(call_config.args.len(), 1);
+		assert_eq!(call_config.args.len(), 2);
 		assert_eq!(call_config.args[0], "true".to_string());
+		assert_eq!(call_config.args[1], "2".to_string());
 		assert_eq!(call_config.value, "50".to_string());
 		assert_eq!(call_config.gas_limit, None);
 		assert_eq!(call_config.proof_size, None);
@@ -725,7 +732,7 @@ mod tests {
 		assert!(call_config.execute);
 		assert!(!call_config.dry_run);
 		assert_eq!(call_config.display(), format!(
-			"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args true --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
+			"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args \"true\", \"2\" --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
 			temp_dir.path().join("testing").display().to_string(),
 		));
 
@@ -754,6 +761,7 @@ mod tests {
 		let mut cli = MockCli::new()
 			.expect_input("Signer calling the contract:", "//Alice".into())
 			.expect_input("Value to transfer to the call:", "50".into()) // Only if payable
+			.expect_input("Enter the value for the parameter: number", "2".into()) // Args for specific_flip
 			.expect_input("Enter the value for the parameter: new_value", "true".into()) // Args for specific_flip
 			.expect_select::<PathBuf>(
 				"Select the message to call:",
@@ -774,7 +782,7 @@ mod tests {
 				"Where is your project located?",
 				temp_dir.path().join("testing").display().to_string(),
 			).expect_info(format!(
-				"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args true --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
+				"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args \"true\", \"2\" --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
 				temp_dir.path().join("testing").display().to_string(),
 			));
 
@@ -798,8 +806,9 @@ mod tests {
 			Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string())
 		);
 		assert_eq!(call_config.message, Some("specific_flip".to_string()));
-		assert_eq!(call_config.args.len(), 1);
+		assert_eq!(call_config.args.len(), 2);
 		assert_eq!(call_config.args[0], "true".to_string());
+		assert_eq!(call_config.args[1], "2".to_string());
 		assert_eq!(call_config.value, "50".to_string());
 		assert_eq!(call_config.gas_limit, None);
 		assert_eq!(call_config.proof_size, None);
@@ -809,7 +818,7 @@ mod tests {
 		assert!(!call_config.dry_run);
 		assert!(call_config.dev_mode);
 		assert_eq!(call_config.display(), format!(
-			"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args true --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
+			"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message specific_flip --args \"true\", \"2\" --value 50 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --execute",
 			temp_dir.path().join("testing").display().to_string(),
 		));
 

crates/pop-cli/src/commands/up/contract.rs

@@ -35,7 +35,7 @@ pub struct UpContractCommand {
 	#[clap(name = "constructor", long, default_value = "new")]
 	constructor: String,
 	/// The constructor arguments, encoded as strings.
-	#[clap(long, num_args = 0..)]
+	#[clap(long, num_args = 0..,)]
 	args: Vec<String>,
 	/// Transfers an initial balance to the instantiated contract.
 	#[clap(name = "value", long, default_value = "0")]
@@ -321,7 +321,7 @@ mod tests {
 		let command = UpContractCommand {
 			path: None,
 			constructor: "new".to_string(),
-			args: vec!["false".to_string()].to_vec(),
+			args: vec![],
 			value: "0".to_string(),
 			gas_limit: None,
 			proof_size: None,

crates/pop-contracts/src/call/mod.rs → crates/pop-contracts/src/call.rs

@@ -4,6 +4,7 @@ use crate::{
 	errors::Error,
 	utils::{
 		helpers::{get_manifest_path, parse_account, parse_balance},
+		metadata::{process_function_args, FunctionType},
 		signer::create_signer,
 	},
 };
@@ -20,8 +21,6 @@ use subxt::{Config, PolkadotConfig as DefaultConfig};
 use subxt_signer::sr25519::Keypair;
 use url::Url;
 
-pub mod metadata;
-
 /// Attributes for the `call` command.
 pub struct CallOpts {
 	/// Path to the contract build directory.
@@ -53,7 +52,7 @@ pub struct CallOpts {
 /// * `call_opts` - options for the `call` command.
 pub async fn set_up_call(
 	call_opts: CallOpts,
-) -> anyhow::Result<CallExec<DefaultConfig, DefaultEnvironment, Keypair>> {
+) -> Result<CallExec<DefaultConfig, DefaultEnvironment, Keypair>, Error> {
 	let token_metadata = TokenMetadata::query::<DefaultConfig>(&call_opts.url).await?;
 	let manifest_path = get_manifest_path(call_opts.path.as_deref())?;
 	let signer = create_signer(&call_opts.suri)?;
@@ -67,10 +66,17 @@ pub async fn set_up_call(
 		parse_balance(&call_opts.value)?;
 
 	let contract: <DefaultConfig as Config>::AccountId = parse_account(&call_opts.contract)?;
+	// Process the argument values input by the user.
+	let args = process_function_args(
+		call_opts.path.unwrap_or_else(|| PathBuf::from("./")),
+		&call_opts.message,
+		call_opts.args,
+		FunctionType::Message,
+	)?;
 
 	let call_exec: CallExec<DefaultConfig, DefaultEnvironment, Keypair> =
 		CallCommandBuilder::new(contract.clone(), &call_opts.message, extrinsic_opts)
-			.args(call_opts.args.clone())
+			.args(args)
 			.value(value.denominate_balance(&token_metadata)?)
 			.gas_limit(call_opts.gas_limit)
 			.proof_size(call_opts.proof_size)
@@ -212,11 +218,9 @@ mod tests {
 			suri: "//Alice".to_string(),
 			execute: false,
 		};
-		let call = set_up_call(call_opts).await;
-		assert!(call.is_err());
-		let error = call.err().unwrap();
-		assert_eq!(error.root_cause().to_string(), "Failed to find any contract artifacts in target directory. \nRun `cargo contract build --release` to generate the artifacts.");
-
+		assert!(
+			matches!(set_up_call(call_opts).await, Err(Error::AnyhowError(message)) if message.root_cause().to_string() == "Failed to find any contract artifacts in target directory. \nRun `cargo contract build --release` to generate the artifacts.")
+		);
 		Ok(())
 	}
 	#[tokio::test]
@@ -233,11 +237,9 @@ mod tests {
 			suri: "//Alice".to_string(),
 			execute: false,
 		};
-		let call = set_up_call(call_opts).await;
-		assert!(call.is_err());
-		let error = call.err().unwrap();
-		assert_eq!(error.root_cause().to_string(), "No 'ink' dependency found");
-
+		assert!(
+			matches!(set_up_call(call_opts).await, Err(Error::AnyhowError(message)) if message.root_cause().to_string() == "No 'ink' dependency found")
+		);
 		Ok(())
 	}
 

crates/pop-contracts/src/errors.rs

@@ -28,6 +28,10 @@ pub enum Error {
 	InstallContractsNode(String),
 	#[error("{0}")]
 	InstantiateContractError(String),
+	#[error("Invalid constructor name: {0}")]
+	InvalidConstructorName(String),
+	#[error("Invalid message name: {0}")]
+	InvalidMessageName(String),
 	#[error("Invalid name: {0}")]
 	InvalidName(String),
 	#[error("IO error: {0}")]
@@ -36,6 +40,8 @@ pub enum Error {
 	KeyPairCreation(String),
 	#[error("Failed to get manifest path: {0}")]
 	ManifestPath(String),
+	#[error("Argument {0} is required")]
+	MissingArgument(String),
 	#[error("Failed to create new contract project: {0}")]
 	NewContract(String),
 	#[error("ParseError error: {0}")]
@@ -44,12 +50,14 @@ pub enum Error {
 	ParseSecretURI(String),
 	#[error("The `Repository` property is missing from the template variant")]
 	RepositoryMissing,
+	#[error("Sourcing error {0}")]
+	SourcingError(SourcingError),
 	#[error("Failed to execute test command: {0}")]
 	TestCommand(String),
 	#[error("Unsupported platform: {os}")]
 	UnsupportedPlatform { os: &'static str },
 	#[error("{0}")]
 	UploadContractError(String),
-	#[error("Sourcing error {0}")]
-	SourcingError(SourcingError),
+	#[error("Incorrect number of arguments provided. Expecting {expected}, {provided} provided")]
+	IncorrectArguments { expected: usize, provided: usize },
 }

crates/pop-contracts/src/lib.rs

@@ -14,9 +14,7 @@ mod utils;
 
 pub use build::{build_smart_contract, is_supported, Verbosity};
 pub use call::{
-	call_smart_contract, dry_run_call, dry_run_gas_estimate_call,
-	metadata::{get_messages, Message},
-	set_up_call, CallOpts,
+	call_smart_contract, dry_run_call, dry_run_gas_estimate_call, set_up_call, CallOpts,
 };
 pub use new::{create_smart_contract, is_valid_contract_name};
 pub use node::{contracts_node_generator, is_chain_alive, run_contracts_node};
@@ -27,4 +25,8 @@ pub use up::{
 	dry_run_gas_estimate_instantiate, dry_run_upload, instantiate_smart_contract,
 	set_up_deployment, set_up_upload, upload_smart_contract, UpOpts,
 };
-pub use utils::{helpers::parse_account, signer::parse_hex_bytes};
+pub use utils::{
+	helpers::parse_account,
+	metadata::{get_messages, ContractFunction},
+	signer::parse_hex_bytes,
+};