fix: chain & contract manual interaction

Author: Daanvdplas

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

@@ -591,13 +591,13 @@ pub fn find_callable_by_name(
 ) -> Result<CallItem, Error> {
 	let pallet = find_pallet_by_name(pallets, pallet_name)?;
 	if let Some(function) = pallet.functions.iter().find(|&e| e.name == function_name) {
-		return Ok(CallItem::Function(function.clone()))
+		return Ok(CallItem::Function(function.clone()));
 	}
 	if let Some(constant) = pallet.constants.iter().find(|&e| e.name == function_name) {
-		return Ok(CallItem::Constant(constant.clone()))
+		return Ok(CallItem::Constant(constant.clone()));
 	}
 	if let Some(storage) = pallet.state.iter().find(|&e| e.name == function_name) {
-		return Ok(CallItem::Storage(storage.clone()))
+		return Ok(CallItem::Storage(storage.clone()));
 	}
 	Err(Error::FunctionNotFound(format!(
 		"Could not find a function, constant or storage with the name \"{function_name}\""

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

@@ -14,7 +14,7 @@ use crate::{
 use anyhow::{Result, anyhow};
 use clap::Args;
 use pop_chains::{
-	Action, CallData, CallItem, DynamicPayload, OnlineClient, Pallet, Param, Payload,
+	Action, CallData, CallItem, DynamicPayload, Function, OnlineClient, Pallet, Param, Payload,
 	SubstrateConfig, construct_extrinsic, construct_sudo_extrinsic, decode_call_data,
 	encode_call_data, find_callable_by_name, find_pallet_by_name, raw_value_to_string,
 	render_storage_key_values, sign_and_submit_extrinsic, supported_actions, type_to_param,
@@ -272,16 +272,7 @@ impl CallChainCommand {
 					}
 
 					// Resolve dispatchable function arguments.
-					let args = if self.args.is_empty() {
-						let mut args = Vec::new();
-						for param in &function.params {
-							let input = prompt_for_param(cli, param, true)?;
-							args.push(input);
-						}
-						args
-					} else {
-						self.expand_file_arguments()?
-					};
+					let args = self.resolve_function_args(function, cli)?;
 
 					// If the chain has sudo prompt the user to confirm if they want to execute the
 					// call via sudo.
@@ -493,6 +484,36 @@ impl CallChainCommand {
 			})
 			.collect()
 	}
+
+	/// Resolves dispatchable arguments by leveraging CLI-provided values when available.
+	fn resolve_function_args(
+		&mut self,
+		function: &Function,
+		cli: &mut impl Cli,
+	) -> Result<Vec<String>> {
+		let expanded_args = self.expand_file_arguments()?;
+		if expanded_args.len() > function.params.len() {
+			return Err(anyhow!(
+				"Expected {} arguments for `{}`, but received {}. Remove the extra values or run \
+				 without `--args` to be prompted.",
+				function.params.len(),
+				function.name,
+				expanded_args.len()
+			));
+		}
+
+		let mut resolved_args = Vec::with_capacity(function.params.len());
+		for (idx, param) in function.params.iter().enumerate() {
+			if let Some(value) = expanded_args.get(idx) {
+				resolved_args.push(value.clone());
+			} else {
+				resolved_args.push(prompt_for_param(cli, param, true)?);
+			}
+		}
+
+		self.args = resolved_args.clone();
+		Ok(resolved_args)
+	}
 }
 
 /// Represents a configured dispatchable function call, including the pallet, function, arguments,
@@ -1129,6 +1150,45 @@ mod tests {
 		Ok(())
 	}
 
+	#[test]
+	fn resolve_function_args_preserves_cli_values() -> Result<()> {
+		let function = Function {
+			pallet: "System".to_string(),
+			name: "remark".to_string(),
+			params: vec![Param { name: "remark".to_string(), ..Default::default() }],
+			is_supported: true,
+			..Default::default()
+		};
+		let mut call_config =
+			CallChainCommand { args: vec!["0x11".to_string()], ..Default::default() };
+		let mut cli = MockCli::new();
+		let resolved = call_config.resolve_function_args(&function, &mut cli)?;
+		assert_eq!(resolved, vec!["0x11".to_string()]);
+		cli.verify()
+	}
+
+	#[test]
+	fn resolve_function_args_prompts_for_missing_values() -> Result<()> {
+		let function = Function {
+			pallet: "System".to_string(),
+			name: "remark".to_string(),
+			params: vec![
+				Param { name: "first".to_string(), ..Default::default() },
+				Param { name: "second".to_string(), ..Default::default() },
+			],
+			is_supported: true,
+			..Default::default()
+		};
+		let mut call_config =
+			CallChainCommand { args: vec!["0x11".to_string()], ..Default::default() };
+		let mut cli =
+			MockCli::new().expect_input("Enter the value for the parameter: second", "0x22".into());
+		let resolved = call_config.resolve_function_args(&function, &mut cli)?;
+		assert_eq!(resolved, vec!["0x11".to_string(), "0x22".to_string()]);
+		assert_eq!(call_config.args, resolved);
+		cli.verify()
+	}
+
 	#[test]
 	fn parse_pallet_name_works() -> Result<()> {
 		assert_eq!(parse_pallet_name("system").unwrap(), "System");

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

@@ -246,8 +246,8 @@ impl CallContractCommand {
 	}
 
 	fn configure_message(&mut self, message: &ContractFunction, cli: &mut impl Cli) -> Result<()> {
-		// Resolve message arguments.
-		self.args = request_contract_function_args(message, cli)?;
+		let resolved_args = request_contract_function_args(message, cli, Some(&self.args))?;
+		self.args = resolved_args;
 
 		// Resolve value.
 		if message.payable && self.value == DEFAULT_PAYABLE_VALUE {
@@ -647,7 +647,7 @@ mod tests {
 		cli::MockCli,
 		common::{urls, wallet::USE_WALLET_PROMPT},
 	};
-	use pop_contracts::{mock_build_process, new_environment};
+	use pop_contracts::{Param, mock_build_process, new_environment};
 	use std::{env, fs::write};
 	use url::Url;
 
@@ -733,6 +733,36 @@ mod tests {
 		cli.verify()
 	}
 
+	#[test]
+	fn configure_message_prompts_for_remaining_args() -> Result<()> {
+		let message = ContractFunction {
+			label: "run".into(),
+			payable: false,
+			args: vec![
+				Param { label: "first".into(), type_name: "u32".into() },
+				Param { label: "second".into(), type_name: "u32".into() },
+			],
+			docs: String::new(),
+			default: false,
+			mutates: true,
+		};
+
+		let mut command = CallContractCommand {
+			args: vec!["10".to_string()],
+			value: DEFAULT_PAYABLE_VALUE.to_string(),
+			dev_mode: true,
+			..Default::default()
+		};
+
+		let mut cli =
+			MockCli::new().expect_input("Enter the value for the parameter: second", "20".into());
+
+		command.configure_message(&message, &mut cli)?;
+
+		assert_eq!(command.args, vec!["10".to_string(), "20".to_string()]);
+		cli.verify()
+	}
+
 	// This test only covers the interactive portion of the call contract command, without actually
 	// calling the contract.
 	#[tokio::test]

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

@@ -343,8 +343,8 @@ impl UpContractCommand {
 
 		let function =
 			extract_function(self.path.clone(), &self.constructor, FunctionType::Constructor)?;
-		if self.args.is_empty() && !function.args.is_empty() {
-			self.args = request_contract_function_args(&function, &mut Cli)?;
+		if !function.args.is_empty() {
+			self.args = request_contract_function_args(&function, &mut Cli, Some(&self.args))?;
 		}
 		normalize_call_args(&mut self.args, &function);
 		// Otherwise instantiate.

crates/pop-cli/src/common/contracts.rs

@@ -114,9 +114,26 @@ pub fn has_contract_been_built(path: &Path) -> bool {
 pub fn request_contract_function_args(
 	function: &ContractFunction,
 	cli: &mut impl Cli,
+	existing_args: Option<&[String]>,
 ) -> anyhow::Result<Vec<String>> {
-	let mut user_provided_args = Vec::new();
-	for arg in &function.args {
+	let mut resolved_args = Vec::with_capacity(function.args.len());
+	let provided_args = existing_args.unwrap_or(&[]);
+
+	if provided_args.len() > function.args.len() {
+		return Err(anyhow::anyhow!(
+			"Expected {} arguments for `{}`, but received {}. Remove the extra values or run \
+			 without `--args` to be prompted.",
+			function.args.len(),
+			function.label,
+			provided_args.len()
+		));
+	}
+
+	for (idx, arg) in function.args.iter().enumerate() {
+		if let Some(value) = provided_args.get(idx) {
+			resolved_args.push(value.clone());
+			continue;
+		}
 		let mut input = cli
 			.input(format!("Enter the value for the parameter: {}", arg.label))
 			.placeholder(&format!("Type required: {}", arg.type_name));
@@ -125,9 +142,10 @@ pub fn request_contract_function_args(
 		if arg.type_name.starts_with("Option<") {
 			input = input.default_input("");
 		}
-		user_provided_args.push(input.interact()?);
+		resolved_args.push(input.interact()?);
 	}
-	Ok(user_provided_args)
+
+	Ok(resolved_args)
 }
 
 /// Normalizes contract arguments before execution.
@@ -184,13 +202,8 @@ mod tests {
 	use cliclack::spinner;
 	use duct::cmd;
 	use pop_common::{find_free_port, set_executable_permission};
-	use pop_contracts::{
-		FunctionType, Param, extract_function, is_chain_alive, run_eth_rpc_node, run_ink_node,
-	};
-	use std::{
-		env,
-		fs::{self, File},
-	};
+	use pop_contracts::{Param, is_chain_alive, run_eth_rpc_node, run_ink_node};
+	use std::fs::{self, File};
 	use url::Url;
 
 	#[test]
@@ -214,18 +227,62 @@ mod tests {
 		Ok(())
 	}
 
-	#[tokio::test]
-	async fn request_contract_function_args_works() -> anyhow::Result<()> {
-		let mut current_dir = env::current_dir().expect("Failed to get current directory");
-		current_dir.pop();
+	#[test]
+	fn request_contract_function_args_works() -> anyhow::Result<()> {
+		let function = ContractFunction {
+			label: "new".to_string(),
+			payable: false,
+			args: vec![Param { label: "init_value".into(), type_name: "bool".into() }],
+			docs: String::new(),
+			default: false,
+			mutates: true,
+		};
 		let mut cli = MockCli::new()
 			.expect_input("Enter the value for the parameter: init_value", "true".into());
-		let function = extract_function(
-			current_dir.join("pop-contracts/tests/files/testing.json"),
-			"new",
-			FunctionType::Constructor,
-		)?;
-		assert_eq!(request_contract_function_args(&function, &mut cli)?, vec!["true"]);
+		assert_eq!(request_contract_function_args(&function, &mut cli, None)?, vec!["true"]);
+		cli.verify()
+	}
+
+	#[test]
+	fn request_contract_function_args_respects_existing() -> anyhow::Result<()> {
+		let mut cli =
+			MockCli::new().expect_input("Enter the value for the parameter: number", "2".into());
+		let function = ContractFunction {
+			label: "specific_flip".to_string(),
+			payable: true,
+			args: vec![
+				Param { label: "new_value".into(), type_name: "bool".into() },
+				Param { label: "number".into(), type_name: "Option<u32>".into() },
+			],
+			docs: String::new(),
+			default: false,
+			mutates: true,
+		};
+		let existing = vec!["true".to_string()];
+		assert_eq!(
+			request_contract_function_args(&function, &mut cli, Some(&existing))?,
+			vec!["true", "2"]
+		);
+		cli.verify()
+	}
+
+	#[test]
+	fn request_contract_function_args_preserves_preprovided_args() -> anyhow::Result<()> {
+		let function = ContractFunction {
+			label: "specific_flip".into(),
+			payable: true,
+			args: vec![
+				Param { label: "new_value".into(), type_name: "bool".into() },
+				Param { label: "number".into(), type_name: "Option<u32>".into() },
+			],
+			docs: String::new(),
+			default: false,
+			mutates: true,
+		};
+
+		let mut cli = MockCli::new();
+		let existing = vec!["true".to_string(), "Some(2)".to_string()];
+		assert_eq!(request_contract_function_args(&function, &mut cli, Some(&existing))?, existing);
 		cli.verify()
 	}