Skip to content

Commit 4c38fc3

Browse files
moliholymoliholy
authored
fix: pop call with composite types (#696)
1 parent 1308301 commit 4c38fc3

File tree

2 files changed

+88
-47
lines changed
  • crates

2 files changed

+88
-47
lines changed

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ fn format_single_tuples<T, W: Write>(value: &Value<T>, mut writer: W) -> Option<
2222
vals.len() == 1
2323
{
2424
let val = &vals[0];
25-
return match raw_value_to_string(val) {
25+
return match raw_value_to_string(val, "") {
2626
Ok(r) => match writer.write_str(&r) {
2727
Ok(_) => Some(Ok(())),
2828
Err(_) => None,
@@ -61,15 +61,22 @@ fn format_hex<T, W: Write>(value: &Value<T>, mut writer: W) -> Option<core::fmt:
6161
/// # Returns
6262
/// * `Ok(String)` - The formatted string representation of the value.
6363
/// * `Err(_)` - If the value cannot be converted to string.
64-
pub fn raw_value_to_string<T>(value: &Value<T>) -> anyhow::Result<String> {
64+
pub fn raw_value_to_string<T>(value: &Value<T>, indent: &str) -> anyhow::Result<String> {
6565
let mut result = String::new();
6666
scale_value::stringify::to_writer_custom()
6767
.compact()
6868
.pretty()
6969
.add_custom_formatter(|v, w| format_hex(v, w))
7070
.add_custom_formatter(|v, w| format_single_tuples(v, w))
7171
.write(value, &mut result)?;
72-
Ok(result)
72+
73+
// Add indentation to each line
74+
let indented = result
75+
.lines()
76+
.map(|line| format!("{indent}{line}"))
77+
.collect::<Vec<_>>()
78+
.join("\n");
79+
Ok(indented)
7380
}
7481

7582
/// Renders storage key-value pairs into a human-readable string format.
@@ -90,16 +97,17 @@ pub fn render_storage_key_values(
9097
key_value_pairs: &[(Vec<Value>, RawValue)],
9198
) -> anyhow::Result<String> {
9299
let mut result = String::new();
100+
let indent = " ";
93101
for (keys, value) in key_value_pairs {
94102
result.push_str("[\n");
95103
if !keys.is_empty() {
96104
for key in keys {
97-
result.push_str(&format!(" {},\n", raw_value_to_string(key)?));
105+
result.push_str(&raw_value_to_string(key, indent)?);
106+
result.push_str(",\n");
98107
}
99108
}
100-
let value = raw_value_to_string(value)?;
101-
result.push_str(&format!(" {}\n", value.replace('\n', "\n ")));
102-
result.push_str("]\n");
109+
result.push_str(&raw_value_to_string(value, indent)?);
110+
result.push_str("\n]\n");
103111
}
104112
Ok(result)
105113
}
@@ -807,29 +815,29 @@ mod tests {
807815
fn raw_value_to_string_works() -> Result<()> {
808816
// Test simple integer value
809817
let value = Value::u128(250).map_context(|_| 0u32);
810-
let result = raw_value_to_string(&value)?;
818+
let result = raw_value_to_string(&value, "")?;
811819
assert_eq!(result, "250");
812820

813821
// Test boolean value
814822
let value = Value::bool(true).map_context(|_| 0u32);
815-
let result = raw_value_to_string(&value)?;
823+
let result = raw_value_to_string(&value, "")?;
816824
assert_eq!(result, "true");
817825

818826
// Test string value
819827
let value = Value::string("hello").map_context(|_| 0u32);
820-
let result = raw_value_to_string(&value)?;
828+
let result = raw_value_to_string(&value, "")?;
821829
assert_eq!(result, "\"hello\"");
822830

823831
// Test single-element tuple (should unwrap) - demonstrates format_single_tuples
824832
let inner = Value::u128(42);
825833
let value = Value::unnamed_composite(vec![inner]).map_context(|_| 0u32);
826-
let result = raw_value_to_string(&value)?;
834+
let result = raw_value_to_string(&value, "")?;
827835
assert_eq!(result, "0x2a"); // 42 in hex - unwrapped from tuple
828836

829837
// Test multi-element composite - hex formatted
830838
let value =
831839
Value::unnamed_composite(vec![Value::u128(1), Value::u128(2)]).map_context(|_| 0u32);
832-
let result = raw_value_to_string(&value)?;
840+
let result = raw_value_to_string(&value, "")?;
833841
assert_eq!(result, "0x0102"); // Formatted as hex bytes
834842

835843
Ok(())

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

Lines changed: 68 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,13 @@ use url::Url;
2424
const DEFAULT_URI: &str = "//Alice";
2525
const ENCODED_CALL_DATA_MAX_LEN: usize = 500; // Maximum length of encoded call data to display.
2626

27+
fn to_tuple(args: &[String]) -> String {
28+
if args.len() < 2 {
29+
panic!("Cannot convert to tuple: too few arguments");
30+
}
31+
format!("({})", args.join(","))
32+
}
33+
2734
/// Command to construct and execute extrinsics with configurable pallets, functions, arguments, and
2835
/// signing options.
2936
#[derive(Args, Clone, Default)]
@@ -73,8 +80,6 @@ impl CallChainCommand {
7380
pub(crate) async fn execute(mut self) -> Result<()> {
7481
let mut cli = cli::Cli;
7582
cli.intro("Call a chain")?;
76-
// Check if all fields are specified via the command line.
77-
let prompt_to_repeat_call = self.requires_user_input();
7883
// Configure the chain.
7984
let chain = chain::configure(
8085
"Which chain would you like to interact with?",
@@ -138,7 +143,7 @@ impl CallChainCommand {
138143
},
139144
CallItem::Constant(constant) => {
140145
// We already have the value of a constant, so we don't need to query it
141-
cli.success(&raw_value_to_string(&constant.value)?)?;
146+
cli.success(&raw_value_to_string(&constant.value, "")?)?;
142147
},
143148
CallItem::Storage(ref storage) => {
144149
// Parse string arguments to Value types for storage query
@@ -206,7 +211,7 @@ impl CallChainCommand {
206211
},
207212
};
208213

209-
if (!prompt_to_repeat_call && self.skip_confirm) ||
214+
if self.skip_confirm ||
210215
!cli.confirm("Do you want to perform another call?")
211216
.initial_value(true)
212217
.interact()?
@@ -305,7 +310,28 @@ impl CallChainCommand {
305310
let key_param = type_to_param(&name.to_string(), registry, key_ty)
306311
.map_err(|e| anyhow!("Failed to parse storage key type: {e}"))?;
307312

308-
let mut params = self.args.clone();
313+
let is_composite = key_param.sub_params.len() > 1;
314+
let (mut params, len) =
315+
if self.args.len() == key_param.sub_params.len() && is_composite {
316+
(vec![to_tuple(self.args.as_slice())], self.args.len())
317+
} else if self.args.len() == 1 && is_composite {
318+
// Handle composite tuple string like "(A, B, C)"
319+
let arg = self.args[0]
320+
.trim()
321+
.trim_start_matches("(")
322+
.trim_start_matches("[")
323+
.trim_end_matches(")")
324+
.trim_end_matches("]")
325+
.to_string();
326+
let len = arg
327+
.split(',')
328+
.map(|s| s.trim().to_string())
329+
.collect::<Vec<_>>()
330+
.len();
331+
(self.args.clone(), len)
332+
} else {
333+
(self.args.clone(), self.args.len())
334+
};
309335
if key_param.sub_params.is_empty() && params.is_empty() {
310336
// Prompt user for the storage key
311337
let key_value = prompt_for_param(cli, &key_param, false)?;
@@ -315,7 +341,7 @@ impl CallChainCommand {
315341
storage.query_all = true;
316342
}
317343
} else {
318-
for sub_param in key_param.sub_params.iter().skip(self.args.len()) {
344+
for sub_param in key_param.sub_params.iter().skip(len) {
319345
let sub_key_value = prompt_for_param(cli, sub_param, false)?;
320346
if !sub_key_value.is_empty() {
321347
params.push(sub_key_value);
@@ -324,6 +350,9 @@ impl CallChainCommand {
324350
break;
325351
}
326352
}
353+
if is_composite && params.len() > 1 {
354+
params = vec![to_tuple(params.as_slice())];
355+
}
327356
}
328357
params
329358
} else {
@@ -449,11 +478,6 @@ impl CallChainCommand {
449478
self.use_wallet = false;
450479
}
451480

452-
// Function to check if all required fields are specified.
453-
fn requires_user_input(&self) -> bool {
454-
self.pallet.is_none() || self.function.is_none() || self.url.is_none()
455-
}
456-
457481
/// Replaces file arguments with their contents, leaving other arguments unchanged.
458482
fn expand_file_arguments(&self) -> Result<Vec<String>> {
459483
self.args
@@ -583,6 +607,9 @@ impl Call {
583607
if self.sudo {
584608
full_message.push_str(" --sudo");
585609
}
610+
if self.skip_confirm {
611+
full_message.push_str(" --skip-confirm");
612+
}
586613
full_message
587614
}
588615
}
@@ -1060,25 +1087,6 @@ mod tests {
10601087
Ok(())
10611088
}
10621089

1063-
#[test]
1064-
fn requires_user_input_works() -> Result<()> {
1065-
let mut call_config = CallChainCommand {
1066-
pallet: Some("System".to_string()),
1067-
function: Some("remark".to_string()),
1068-
args: vec!["0x11".to_string()],
1069-
url: Some(Url::parse(urls::LOCAL)?),
1070-
suri: Some(DEFAULT_URI.to_string()),
1071-
use_wallet: false,
1072-
skip_confirm: false,
1073-
call_data: None,
1074-
sudo: false,
1075-
};
1076-
assert!(!call_config.requires_user_input());
1077-
call_config.pallet = None;
1078-
assert!(call_config.requires_user_input());
1079-
Ok(())
1080-
}
1081-
10821090
#[test]
10831091
fn expand_file_arguments_works() -> Result<()> {
10841092
let mut call_config = CallChainCommand {
@@ -1152,7 +1160,7 @@ mod tests {
11521160
let value = result.unwrap();
11531161
// The value should be a primitive (block number)
11541162
assert!(matches!(value.value, ValueDef::Primitive(_)));
1155-
let formatted_value = raw_value_to_string(&value)?;
1163+
let formatted_value = raw_value_to_string(&value, "")?;
11561164
assert!(!formatted_value.is_empty(), "Formatted value should not be empty");
11571165

11581166
// Test querying a map storage item (System::Account with Alice's account)
@@ -1173,7 +1181,7 @@ mod tests {
11731181
let account_result = account_storage.query(&client, vec![account_key]).await?;
11741182
assert!(account_result.is_some(), "Alice's account should exist in test chain");
11751183
let account_value = account_result.unwrap();
1176-
let formatted_account = raw_value_to_string(&account_value)?;
1184+
let formatted_account = raw_value_to_string(&account_value, "")?;
11771185
assert!(!formatted_account.is_empty(), "Account data should not be empty");
11781186

11791187
Ok(())
@@ -1202,7 +1210,7 @@ mod tests {
12021210

12031211
// Constants have their values already decoded
12041212
let constant_value = &version_constant.value;
1205-
let formatted_value = raw_value_to_string(constant_value)?;
1213+
let formatted_value = raw_value_to_string(constant_value, "")?;
12061214
assert!(!formatted_value.is_empty(), "Constant value should not be empty");
12071215
// Version should be a composite value with spec_name, spec_version, etc.
12081216
assert!(matches!(constant_value.value, ValueDef::Composite(_)));
@@ -1215,7 +1223,7 @@ mod tests {
12151223
.expect("System::BlockHashCount constant should exist");
12161224

12171225
let block_hash_count_value = &block_hash_count_constant.value;
1218-
let formatted_block_hash_count = raw_value_to_string(block_hash_count_value)?;
1226+
let formatted_block_hash_count = raw_value_to_string(block_hash_count_value, "")?;
12191227
assert!(!formatted_block_hash_count.is_empty(), "BlockHashCount value should not be empty");
12201228
// BlockHashCount should be a primitive value (u32)
12211229
assert!(matches!(block_hash_count_value.value, ValueDef::Primitive(_)));
@@ -1228,10 +1236,35 @@ mod tests {
12281236
.expect("System::SS58Prefix constant should exist");
12291237

12301238
let ss58_prefix_value = &ss58_prefix_constant.value;
1231-
let formatted_ss58_prefix = raw_value_to_string(ss58_prefix_value)?;
1239+
let formatted_ss58_prefix = raw_value_to_string(ss58_prefix_value, "")?;
12321240
assert!(!formatted_ss58_prefix.is_empty(), "SS58Prefix value should not be empty");
12331241
assert!(matches!(ss58_prefix_value.value, ValueDef::Primitive(_)));
12341242

12351243
Ok(())
12361244
}
1245+
1246+
#[tokio::test]
1247+
async fn query_storage_with_composite_key_works() -> Result<()> {
1248+
// Spawn a test node
1249+
let node = TestNode::spawn().await?;
1250+
let node_url = node.ws_url();
1251+
1252+
// Build the command to directly execute a storage query using a composite key
1253+
let cmd = CallChainCommand {
1254+
pallet: Some("Assets".to_string()),
1255+
function: Some("Account".to_string()),
1256+
args: vec![
1257+
"10000".to_string(), // AssetId
1258+
// Alice AccountId32 (hex) in dev networks
1259+
"0xd43593c715fdd31c61141abd04a99fd6822c8558854ccde39a5684e7a56da27d".to_string(),
1260+
],
1261+
url: Some(Url::parse(node_url)?),
1262+
skip_confirm: true, // Avoid interactive confirmation at the end of execute loop
1263+
..Default::default()
1264+
};
1265+
1266+
// Execute the command end-to-end; it should parse the composite key and perform the storage
1267+
// query
1268+
cmd.execute().await
1269+
}
12371270
}

0 commit comments

Comments
 (0)