Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: format types #339

Merged
merged 1 commit into from
Nov 8, 2024
Merged

refactor: format types #339

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 12 additions & 9 deletions crates/pop-cli/src/commands/call/contract.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Args are always quoted for safety - complex types might have spaces or commas in their value entry and its best to wrap with "".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

full_message.push_str(&format!(" --args {}", args.join(", ")));
}
if self.value != DEFAULT_PAYABLE_VALUE {
full_message.push_str(&format!(" --value {}", self.value));
Expand Down Expand Up @@ -235,7 +236,7 @@ impl CallContractCommand {
.placeholder(&format!("Type required: {}", arg.type_name));

// Set default input only if the parameter type is `Option` (Not mandatory)
if arg.type_name == "Option" {
if arg.type_name.starts_with("Option<") {
input = input.default_input("");
}
contract_args.push(input.interact()?);
Expand Down Expand Up @@ -482,7 +483,7 @@ mod tests {
.expect_warning("Your call has not been executed.")
.expect_info("Gas limit: Weight { ref_time: 100, proof_size: 10 }");

let call_config = CallContractCommand {
let mut call_config = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: Some("flip".to_string()),
Expand Down Expand Up @@ -549,7 +550,7 @@ mod tests {
.expect_outro("Contract calling complete.");

// Contract deployed on Pop Network testnet, test get
let call_config = CallContractCommand {
let mut call_config = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: Some("get".to_string()),
Expand Down Expand Up @@ -696,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,2 --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(),
));

Expand Down Expand Up @@ -731,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,2 --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(),
));

Expand Down Expand Up @@ -760,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:",
Expand All @@ -780,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(),
));

Expand All @@ -804,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);
Expand All @@ -815,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(),
));

Expand Down
201 changes: 174 additions & 27 deletions crates/pop-contracts/src/utils/metadata.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
use crate::errors::Error;
use contract_extrinsics::ContractArtifacts;
use contract_transcode::ink_metadata::MessageParamSpec;
use scale_info::form::PortableForm;
use scale_info::{form::PortableForm, PortableRegistry, Type, TypeDef, TypeDefPrimitive};
use std::path::Path;

/// Describes a parameter.
#[derive(Clone, PartialEq, Eq)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Param {
/// The label of the parameter.
pub label: String,
Expand Down Expand Up @@ -51,16 +51,16 @@ pub fn get_messages(path: &Path) -> Result<Vec<ContractFunction>, Error> {
let contract_artifacts =
ContractArtifacts::from_manifest_or_file(Some(&cargo_toml_path), None)?;
let transcoder = contract_artifacts.contract_transcoder()?;
Ok(transcoder
.metadata()
let metadata = transcoder.metadata();
Ok(metadata
.spec()
.messages()
.iter()
.map(|message| ContractFunction {
label: message.label().to_string(),
mutates: message.mutates(),
payable: message.payable(),
args: process_args(message.args()),
args: process_args(message.args(), metadata.registry()),
docs: message.docs().join(" "),
default: *message.default(),
})
Expand Down Expand Up @@ -94,15 +94,15 @@ pub fn get_constructors(path: &Path) -> Result<Vec<ContractFunction>, Error> {
let contract_artifacts =
ContractArtifacts::from_manifest_or_file(Some(&cargo_toml_path), None)?;
let transcoder = contract_artifacts.contract_transcoder()?;
Ok(transcoder
.metadata()
let metadata = transcoder.metadata();
Ok(metadata
.spec()
.constructors()
.iter()
.map(|constructor| ContractFunction {
label: constructor.label().to_string(),
payable: *constructor.payable(),
args: process_args(constructor.args()),
args: process_args(constructor.args(), metadata.registry()),
docs: constructor.docs().join(" "),
default: *constructor.default(),
mutates: true,
Expand All @@ -126,17 +126,164 @@ where
}

// Parse the parameters into a vector of argument labels.
fn process_args(params: &[MessageParamSpec<PortableForm>]) -> Vec<Param> {
fn process_args(
params: &[MessageParamSpec<PortableForm>],
registry: &PortableRegistry,
) -> Vec<Param> {
let mut args: Vec<Param> = Vec::new();
for arg in params {
args.push(Param {
label: arg.label().to_string(),
type_name: arg.ty().display_name().to_string(),
});
// Resolve type from registry to provide full type representation.
let type_name =
format_type(registry.resolve(arg.ty().ty().id).expect("type not found"), registry);
args.push(Param { label: arg.label().to_string(), type_name });
}
args
}

// Formats a specified type, using the registry to output its full type representation.
fn format_type(ty: &Type<PortableForm>, registry: &PortableRegistry) -> String {
let mut name = ty
.path
.segments
.last()
.map(|s| s.to_owned())
.unwrap_or_else(|| ty.path.to_string());

if !ty.type_params.is_empty() {
let params: Vec<_> = ty
.type_params
.iter()
.filter_map(|p| registry.resolve(p.ty.unwrap().id))
.map(|t| format_type(t, registry))
.collect();
name = format!("{name}<{}>", params.join(","));
}

name = format!(
"{name}{}",
match &ty.type_def {
TypeDef::Composite(composite) => {
if composite.fields.is_empty() {
return "".to_string();
}

let mut named = false;
let fields: Vec<_> = composite
.fields
.iter()
.filter_map(|f| match f.name.as_ref() {
None => registry.resolve(f.ty.id).map(|t| format_type(t, registry)),
Some(field) => {
named = true;
f.type_name.as_ref().map(|t| format!("{field}: {t}"))
},
})
.collect();
match named {
true => format!(" {{ {} }}", fields.join(", ")),
false => format!(" ({})", fields.join(", ")),
}
},
TypeDef::Variant(variant) => {
let variants: Vec<_> = variant
.variants
.iter()
.map(|v| {
if v.fields.is_empty() {
return v.name.clone();
}

let name = v.name.as_str();
let mut named = false;
let fields: Vec<_> = v
.fields
.iter()
.filter_map(|f| match f.name.as_ref() {
None => registry.resolve(f.ty.id).map(|t| format_type(t, registry)),
Some(field) => {
named = true;
f.type_name.as_ref().map(|t| format!("{field}: {t}"))
},
})
.collect();
format!(
"{name}{}",
match named {
true => format!("{{ {} }}", fields.join(", ")),
false => format!("({})", fields.join(", ")),
}
)
})
.collect();
format!(": {}", variants.join(", "))
},
TypeDef::Sequence(sequence) => {
format!(
"[{}]",
format_type(
registry.resolve(sequence.type_param.id).expect("sequence type not found"),
registry
)
)
},
TypeDef::Array(array) => {
format!(
"[{};{}]",
format_type(
registry.resolve(array.type_param.id).expect("array type not found"),
registry
),
array.len
)
},
TypeDef::Tuple(tuple) => {
let fields: Vec<_> = tuple
.fields
.iter()
.filter_map(|p| registry.resolve(p.id))
.map(|t| format_type(t, registry))
.collect();
format!("({})", fields.join(","))
},
TypeDef::Primitive(primitive) => {
use TypeDefPrimitive::*;
match primitive {
Bool => "bool",
Char => "char",
Str => "str",
U8 => "u8",
U16 => "u16",
U32 => "u32",
U64 => "u64",
U128 => "u128",
U256 => "u256",
I8 => "i8",
I16 => "i16",
I32 => "i32",
I64 => "i64",
I128 => "i128",
I256 => "i256",
}
.to_string()
},
TypeDef::Compact(compact) => {
format!(
"Compact<{}>",
format_type(
registry.resolve(compact.type_param.id).expect("compact type not found"),
registry
)
)
},
TypeDef::BitSequence(_) => {
unimplemented!("bit sequence not currently supported")
},
}
);

name
}

/// 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
///
Expand All @@ -153,26 +300,26 @@ pub fn process_function_args<P>(
where
P: AsRef<Path>,
{
let parsed_args = args.into_iter().map(|arg| arg.replace(",", "")).collect::<Vec<_>>();
Copy link
Contributor Author

@evilrobot-01 evilrobot-01 Nov 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this breaks entering complex types, where fields are separated by commas, hence removed.

let function = match function_type {
FunctionType::Message => get_message(path, label)?,
FunctionType::Constructor => get_constructor(path, label)?,
};
if parsed_args.len() != function.args.len() {
if args.len() != function.args.len() {
return Err(Error::IncorrectArguments {
expected: function.args.len(),
provided: parsed_args.len(),
provided: args.len(),
});
}
Ok(parsed_args
Ok(args
.into_iter()
.zip(&function.args)
.map(|(arg, param)| match (param.type_name.as_str(), arg.is_empty()) {
("Option", true) => "None".to_string(), /* If the argument is Option and empty, */
// replace it with `None`
("Option", false) => format!("Some({})", arg), /* If the argument is Option and not */
// empty, wrap it in `Some(...)`
_ => arg, // If the argument is not Option, return it as is
.map(|(arg, param)| match (param.type_name.starts_with("Option<"), arg.is_empty()) {
// If the argument is Option and empty, replace it with `None`
(true, true) => "None".to_string(),
// If the argument is Option and not empty, wrap it in `Some(...)`
(true, false) => format!("Some({})", arg),
// If the argument is not Option, return it as is
_ => arg,
})
.collect::<Vec<String>>())
}
Expand Down Expand Up @@ -207,7 +354,7 @@ mod tests {
assert_eq!(message[2].args[0].label, "new_value".to_string());
assert_eq!(message[2].args[0].type_name, "bool".to_string());
assert_eq!(message[2].args[1].label, "number".to_string());
assert_eq!(message[2].args[1].type_name, "Option".to_string());
assert_eq!(message[2].args[1].type_name, "Option<u32>: None, Some(u32)".to_string());
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whilst this may be unnecessary for Options, showing a list of potential variant values for enums could be very useful imo. This could potentially be refactored into a separate field.

Ok(())
}

Expand All @@ -231,7 +378,7 @@ mod tests {
assert_eq!(message.args[0].label, "new_value".to_string());
assert_eq!(message.args[0].type_name, "bool".to_string());
assert_eq!(message.args[1].label, "number".to_string());
assert_eq!(message.args[1].type_name, "Option".to_string());
assert_eq!(message.args[1].type_name, "Option<u32>: None, Some(u32)".to_string());
Ok(())
}

Expand Down Expand Up @@ -264,7 +411,7 @@ mod tests {
assert_eq!(constructor[1].args[0].label, "init_value".to_string());
assert_eq!(constructor[1].args[0].type_name, "bool".to_string());
assert_eq!(constructor[1].args[1].label, "number".to_string());
assert_eq!(constructor[1].args[1].type_name, "Option".to_string());
assert_eq!(constructor[1].args[1].type_name, "Option<u32>: None, Some(u32)".to_string());
Ok(())
}

Expand All @@ -291,7 +438,7 @@ mod tests {
assert_eq!(constructor.args[0].label, "init_value".to_string());
assert_eq!(constructor.args[0].type_name, "bool".to_string());
assert_eq!(constructor.args[1].label, "number".to_string());
assert_eq!(constructor.args[1].type_name, "Option".to_string());
assert_eq!(constructor.args[1].type_name, "Option<u32>: None, Some(u32)".to_string());
Ok(())
}

Expand Down