From 6761f6bdf30ce11601dcc145b7b023570ff0c6b8 Mon Sep 17 00:00:00 2001 From: Alex Bean Date: Tue, 26 Nov 2024 17:13:42 +0100 Subject: [PATCH] feat: allow users to specify custom contract metadata files (#347) * chore: allow the user specify the metadata file to call a contract * test: unit test to parse metadata from a file * docs: fix docs * refactor: ensure_contract_built after user input path * fix: call contract when metadata file * fix: remove default_input in contract address * docs: rename metadata with artifact * fix: panic at has_contract_been_built * fix: clippy * refactor: keep ensure_contract_built as a CallContractCommand function * fix: ensure_contract_built * docs: improve comments * fix: feedback and include wasm file for testing --- crates/pop-cli/src/commands/call/contract.rs | 179 +++++++++++++----- crates/pop-cli/src/common/build.rs | 7 +- crates/pop-contracts/src/call.rs | 44 ++++- crates/pop-contracts/src/utils/metadata.rs | 53 ++++-- crates/pop-contracts/tests/files/testing.wasm | Bin 0 -> 3710 bytes 5 files changed, 209 insertions(+), 74 deletions(-) create mode 100644 crates/pop-contracts/tests/files/testing.wasm diff --git a/crates/pop-cli/src/commands/call/contract.rs b/crates/pop-cli/src/commands/call/contract.rs index 8db1cc63..7b0d588f 100644 --- a/crates/pop-cli/src/commands/call/contract.rs +++ b/crates/pop-cli/src/commands/call/contract.rs @@ -20,7 +20,7 @@ const DEFAULT_PAYABLE_VALUE: &str = "0"; #[derive(Args, Clone)] pub struct CallContractCommand { - /// Path to the contract build directory. + /// Path to the contract build directory or a contract artifact. #[arg(short = 'p', long)] path: Option, /// The address of the contract to call. @@ -68,8 +68,6 @@ pub struct CallContractCommand { impl CallContractCommand { /// Executes the command. pub(crate) async fn execute(mut self) -> Result<()> { - // Ensure contract is built. - self.ensure_contract_built(&mut cli::Cli).await?; // Check if message specified via command line argument. let prompt_to_repeat_call = self.message.is_none(); // Configure the call based on command line arguments/call UI. @@ -119,27 +117,33 @@ impl CallContractCommand { } /// Checks if the contract has been built; if not, builds it. + /// If the path is a contract artifact file, skips the build process async fn ensure_contract_built(&self, cli: &mut impl Cli) -> Result<()> { - // Check if build exists in the specified "Contract build directory" - if !has_contract_been_built(self.path.as_deref()) { - // Build the contract in release mode - cli.warning("NOTE: contract has not yet been built.")?; - let spinner = spinner(); - spinner.start("Building contract in RELEASE mode..."); - let result = match build_smart_contract(self.path.as_deref(), true, Verbosity::Quiet) { - Ok(result) => result, - Err(e) => { - return Err(anyhow!(format!( + // The path is expected to be set. If it is not, exit early without attempting to build the + // contract. + let Some(path) = self.path.as_deref() else { return Ok(()) }; + // Check if the path is a file or the build exists in the specified "Contract build + // directory" + if path.is_file() || has_contract_been_built(self.path.as_deref()) { + return Ok(()); + } + // Build the contract in release mode + cli.warning("NOTE: contract has not yet been built.")?; + let spinner = spinner(); + spinner.start("Building contract in RELEASE mode..."); + let result = match build_smart_contract(self.path.as_deref(), true, Verbosity::Quiet) { + Ok(result) => result, + Err(e) => { + return Err(anyhow!(format!( "🚫 An error occurred building your contract: {}\nUse `pop build` to retry with build output.", e.to_string() ))); - }, - }; - spinner.stop(format!( - "Your contract artifacts are ready. You can find them in: {}", - result.target_directory.display() - )); - } + }, + }; + spinner.stop(format!( + "Your contract artifacts are ready. You can find them in: {}", + result.target_directory.display() + )); Ok(()) } @@ -156,25 +160,21 @@ impl CallContractCommand { } // Resolve path. - let contract_path = match self.path.as_ref() { - None => { - let path = Some(PathBuf::from("./")); - if has_contract_been_built(path.as_deref()) { - self.path = path; - } else { - // Prompt for path. - let input_path: String = cli - .input("Where is your project located?") - .placeholder("./") - .default_input("./") - .interact()?; - self.path = Some(PathBuf::from(input_path)); - } + if self.path.is_none() { + let input_path: String = cli + .input("Where is your project or contract artifact located?") + .placeholder("./") + .default_input("./") + .interact()?; + self.path = Some(PathBuf::from(input_path)); + } + let contract_path = self + .path + .as_ref() + .expect("path is guaranteed to be set as input is prompted when None; qed"); - self.path.as_ref().unwrap() - }, - Some(p) => p, - }; + // Ensure contract is built. + self.ensure_contract_built(&mut cli::Cli).await?; // Parse the contract metadata provided. If there is an error, do not prompt for more. let messages = match get_messages(contract_path) { @@ -208,7 +208,6 @@ impl CallContractCommand { Ok(_) => Ok(()), Err(_) => Err("Invalid address."), }) - .default_input("5DYs7UGBm2LuX4ryvyqfksozNAW5V47tPbGiVgnjYWCZ29bt") .interact()?; self.contract = Some(contract_address); }; @@ -434,7 +433,7 @@ mod tests { use super::*; use crate::cli::MockCli; use pop_contracts::{mock_build_process, new_environment}; - use std::env; + use std::{env, fs::write}; use url::Url; #[tokio::test] @@ -508,6 +507,60 @@ mod tests { cli.verify() } + #[tokio::test] + async fn call_contract_dry_run_with_artifact_file_works() -> Result<()> { + let mut current_dir = env::current_dir().expect("Failed to get current directory"); + current_dir.pop(); + + let mut cli = MockCli::new() + .expect_intro(&"Call a contract") + .expect_warning("Your call has not been executed.") + .expect_info("Gas limit: Weight { ref_time: 100, proof_size: 10 }"); + + // From .contract file + let mut call_config = CallContractCommand { + path: Some(current_dir.join("pop-contracts/tests/files/testing.contract")), + contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()), + message: Some("flip".to_string()), + args: vec![].to_vec(), + value: "0".to_string(), + gas_limit: Some(100), + proof_size: Some(10), + url: Url::parse("wss://rpc1.paseo.popnetwork.xyz")?, + suri: "//Alice".to_string(), + dry_run: true, + execute: false, + dev_mode: false, + }; + call_config.configure(&mut cli, false).await?; + assert_eq!(call_config.display(), format!( + "pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message flip --gas 100 --proof_size 10 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --dry_run", + current_dir.join("pop-contracts/tests/files/testing.contract").display().to_string(), + )); + // Contract deployed on Pop Network testnet, test dry-run + call_config.execute_call(&mut cli, false).await?; + + // From .json file + call_config.path = Some(current_dir.join("pop-contracts/tests/files/testing.json")); + call_config.configure(&mut cli, false).await?; + assert_eq!(call_config.display(), format!( + "pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message flip --gas 100 --proof_size 10 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --dry_run", + current_dir.join("pop-contracts/tests/files/testing.json").display().to_string(), + )); + + // From .wasm file + call_config.path = Some(current_dir.join("pop-contracts/tests/files/testing.wasm")); + call_config.configure(&mut cli, false).await?; + assert_eq!(call_config.display(), format!( + "pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message flip --gas 100 --proof_size 10 --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice --dry_run", + current_dir.join("pop-contracts/tests/files/testing.wasm").display().to_string(), + )); + // Contract deployed on Pop Network testnet, test dry-run + call_config.execute_call(&mut cli, false).await?; + + cli.verify() + } + #[tokio::test] async fn call_contract_query_duplicate_call_works() -> Result<()> { let temp_dir = new_environment("testing")?; @@ -608,7 +661,7 @@ mod tests { "wss://rpc1.paseo.popnetwork.xyz".into(), ) .expect_input( - "Where is your project located?", + "Where is your project or contract artifact located?", temp_dir.path().join("testing").display().to_string(), ).expect_info(format!( "pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message get --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice", @@ -694,7 +747,7 @@ mod tests { "wss://rpc1.paseo.popnetwork.xyz".into(), ) .expect_input( - "Where is your project located?", + "Where is your project or contract artifact 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", @@ -779,7 +832,7 @@ mod tests { "wss://rpc1.paseo.popnetwork.xyz".into(), ) .expect_input( - "Where is your project located?", + "Where is your project or contract artifact 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", @@ -828,8 +881,23 @@ mod tests { #[tokio::test] async fn guide_user_to_call_contract_fails_not_build() -> Result<()> { let temp_dir = new_environment("testing")?; - let mut cli = MockCli::new(); - assert!(matches!(CallContractCommand { + let mut current_dir = env::current_dir().expect("Failed to get current directory"); + current_dir.pop(); + // Create invalid `.json`, `.contract` and `.wasm` files for testing + let invalid_contract_path = temp_dir.path().join("testing.contract"); + let invalid_json_path = temp_dir.path().join("testing.json"); + let invalid_wasm_path = temp_dir.path().join("testing.wasm"); + write(&invalid_contract_path, b"This is an invalid contract file")?; + write(&invalid_json_path, b"This is an invalid JSON file")?; + write(&invalid_wasm_path, b"This is an invalid WASM file")?; + // Mock the build process to simulate a scenario where the contract is not properly built. + mock_build_process( + temp_dir.path().join("testing"), + invalid_contract_path.clone(), + invalid_contract_path.clone(), + )?; + // Test the path is a folder with an invalid build. + let mut command = CallContractCommand { path: Some(temp_dir.path().join("testing")), contract: None, message: None, @@ -842,7 +910,26 @@ mod tests { dry_run: false, execute: false, dev_mode: false, - }.configure(&mut cli, false).await, Err(message) if message.to_string().contains("Unable to fetch contract metadata: Failed to find any contract artifacts in target directory."))); + }; + let mut cli = MockCli::new(); + assert!( + matches!(command.configure(&mut cli, false).await, Err(message) if message.to_string().contains("Unable to fetch contract metadata")) + ); + // Test the path is a file with invalid `.contract` file. + command.path = Some(invalid_contract_path); + assert!( + matches!(command.configure(&mut cli, false).await, Err(message) if message.to_string().contains("Unable to fetch contract metadata")) + ); + // Test the path is a file with invalid `.json` file. + command.path = Some(invalid_json_path); + assert!( + matches!(command.configure(&mut cli, false).await, Err(message) if message.to_string().contains("Unable to fetch contract metadata")) + ); + // Test the path is a file with invalid `.wasm` file. + command.path = Some(invalid_wasm_path); + assert!( + matches!(command.configure(&mut cli, false).await, Err(message) if message.to_string().contains("Unable to fetch contract metadata")) + ); cli.verify() } diff --git a/crates/pop-cli/src/common/build.rs b/crates/pop-cli/src/common/build.rs index 6bc7fd4d..fd8d1b52 100644 --- a/crates/pop-cli/src/common/build.rs +++ b/crates/pop-cli/src/common/build.rs @@ -14,9 +14,10 @@ pub fn has_contract_been_built(path: Option<&Path>) -> bool { Ok(manifest) => manifest, Err(_) => return false, }; - let contract_name = manifest.package().name(); - project_path.join("target/ink").exists() && - project_path.join(format!("target/ink/{}.contract", contract_name)).exists() + manifest + .package + .map(|p| project_path.join(format!("target/ink/{}.contract", p.name())).exists()) + .unwrap_or_default() } #[cfg(test)] diff --git a/crates/pop-contracts/src/call.rs b/crates/pop-contracts/src/call.rs index e45a0a9c..5d9e9aa2 100644 --- a/crates/pop-contracts/src/call.rs +++ b/crates/pop-contracts/src/call.rs @@ -11,7 +11,7 @@ use crate::{ use anyhow::Context; use contract_build::Verbosity; use contract_extrinsics::{ - BalanceVariant, CallCommandBuilder, CallExec, DisplayEvents, ErrorVariant, + BalanceVariant, CallCommandBuilder, CallExec, ContractArtifacts, DisplayEvents, ErrorVariant, ExtrinsicOptsBuilder, TokenMetadata, }; use ink_env::{DefaultEnvironment, Environment}; @@ -54,13 +54,25 @@ pub async fn set_up_call( call_opts: CallOpts, ) -> Result, Error> { let token_metadata = TokenMetadata::query::(&call_opts.url).await?; - let manifest_path = get_manifest_path(call_opts.path.as_deref())?; let signer = create_signer(&call_opts.suri)?; - let extrinsic_opts = ExtrinsicOptsBuilder::new(signer) - .manifest_path(Some(manifest_path)) - .url(call_opts.url.clone()) - .done(); + let extrinsic_opts = match &call_opts.path { + // If path is a file construct the ExtrinsicOptsBuilder from the file. + Some(path) if path.is_file() => { + let artifacts = ContractArtifacts::from_manifest_or_file(None, Some(path))?; + ExtrinsicOptsBuilder::new(signer) + .file(Some(artifacts.artifact_path())) + .url(call_opts.url.clone()) + .done() + }, + _ => { + let manifest_path = get_manifest_path(call_opts.path.as_deref())?; + ExtrinsicOptsBuilder::new(signer) + .manifest_path(Some(manifest_path)) + .url(call_opts.url.clone()) + .done() + }, + }; let value: BalanceVariant<::Balance> = parse_balance(&call_opts.value)?; @@ -203,6 +215,26 @@ mod tests { Ok(()) } + #[tokio::test] + async fn test_set_up_call_from_artifact_file() -> Result<()> { + let current_dir = env::current_dir().expect("Failed to get current directory"); + let call_opts = CallOpts { + path: Some(current_dir.join("./tests/files/testing.json")), + contract: "5CLPm1CeUvJhZ8GCDZCR7nWZ2m3XXe4X5MtAQK69zEjut36A".to_string(), + message: "get".to_string(), + args: [].to_vec(), + value: "1000".to_string(), + gas_limit: None, + proof_size: None, + url: Url::parse(CONTRACTS_NETWORK_URL)?, + suri: "//Alice".to_string(), + execute: false, + }; + let call = set_up_call(call_opts).await?; + assert_eq!(call.message(), "get"); + Ok(()) + } + #[tokio::test] async fn test_set_up_call_error_contract_not_build() -> Result<()> { let temp_dir = new_environment("testing")?; diff --git a/crates/pop-contracts/src/utils/metadata.rs b/crates/pop-contracts/src/utils/metadata.rs index f0dd2b8e..60826fab 100644 --- a/crates/pop-contracts/src/utils/metadata.rs +++ b/crates/pop-contracts/src/utils/metadata.rs @@ -42,14 +42,15 @@ pub enum FunctionType { /// Extracts a list of smart contract messages parsing the metadata file. /// /// # Arguments -/// * `path` - Location path of the project. +/// * `path` - Location path of the project or contract metadata file. pub fn get_messages(path: &Path) -> Result, Error> { - let cargo_toml_path = match path.ends_with("Cargo.toml") { - true => path.to_path_buf(), - false => path.join("Cargo.toml"), + let contract_artifacts = if path.is_dir() || path.ends_with("Cargo.toml") { + let cargo_toml_path = + if path.ends_with("Cargo.toml") { path.to_path_buf() } else { path.join("Cargo.toml") }; + ContractArtifacts::from_manifest_or_file(Some(&cargo_toml_path), None)? + } else { + ContractArtifacts::from_manifest_or_file(None, Some(&path.to_path_buf()))? }; - let contract_artifacts = - ContractArtifacts::from_manifest_or_file(Some(&cargo_toml_path), None)?; let transcoder = contract_artifacts.contract_transcoder()?; let metadata = transcoder.metadata(); Ok(metadata @@ -336,25 +337,39 @@ mod tests { fn get_messages_work() -> Result<()> { let temp_dir = new_environment("testing")?; let current_dir = env::current_dir().expect("Failed to get current directory"); + + // Helper function to avoid duplicated code + fn assert_contract_metadata_parsed(message: Vec) -> Result<()> { + assert_eq!(message.len(), 3); + assert_eq!(message[0].label, "flip"); + assert_eq!(message[0].docs, " A message that can be called on instantiated contracts. This one flips the value of the stored `bool` from `true` to `false` and vice versa."); + assert_eq!(message[1].label, "get"); + assert_eq!(message[1].docs, " Simply returns the current value of our `bool`."); + assert_eq!(message[2].label, "specific_flip"); + assert_eq!(message[2].docs, " A message for testing, flips the value of the stored `bool` with `new_value` and is payable"); + // assert parsed arguments + assert_eq!(message[2].args.len(), 2); + 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: None, Some(u32)".to_string()); + Ok(()) + } + mock_build_process( temp_dir.path().join("testing"), current_dir.join("./tests/files/testing.contract"), current_dir.join("./tests/files/testing.json"), )?; + + // Test with a directory path let message = get_messages(&temp_dir.path().join("testing"))?; - assert_eq!(message.len(), 3); - assert_eq!(message[0].label, "flip"); - assert_eq!(message[0].docs, " A message that can be called on instantiated contracts. This one flips the value of the stored `bool` from `true` to `false` and vice versa."); - assert_eq!(message[1].label, "get"); - assert_eq!(message[1].docs, " Simply returns the current value of our `bool`."); - assert_eq!(message[2].label, "specific_flip"); - assert_eq!(message[2].docs, " A message for testing, flips the value of the stored `bool` with `new_value` and is payable"); - // assert parsed arguments - assert_eq!(message[2].args.len(), 2); - 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: None, Some(u32)".to_string()); + assert_contract_metadata_parsed(message)?; + + // Test with a metadata file path + let message = get_messages(¤t_dir.join("./tests/files/testing.contract"))?; + assert_contract_metadata_parsed(message)?; + Ok(()) } diff --git a/crates/pop-contracts/tests/files/testing.wasm b/crates/pop-contracts/tests/files/testing.wasm new file mode 100644 index 0000000000000000000000000000000000000000..430888669464e770d310139f29e8af1ebf865ba4 GIT binary patch literal 3710 zcmb_fO>ZPu6}|7Hx~pC8aXC1S84uE{iiI30?T;bxhzSyYkzz9%A+uqL>>j(uxZBen zSNAv>DN1{y2#GCQ#0H5i8-4|1#S$U0VGA2J2nk5ed9T|aN+LFl<*Daa@1A?_x#!;3 zE#2WWB_isb)`1?6<-m-`V>LbyIj~r9$(_kxa-hOX?BcK*>ro>aR^8Lh?6?~34@YNp z_qY-h_BN7}!Sfd*k?`03Z2099*z2C|*VX7nJrEnM<}bRZFRJ}f-5m^{R&`w+iI{3| z(RfxpJF8!6rE~MfVxsU*8i-r0G*gWXMC-<)87qHzX|u8Tf4@rjxcoH9n)BKoc27^` zn~kID`RUn9xjXaUrN!69DQ9-iF>FHZX2 zoR>+jFisVgZwwAP!(-anj>tn1XT9p-t>%NKnKQ28l0E=GJ}*;j0$#bKS0;e_0Jce? zBG3=pKyzwU%a%rT-u4QF$cLE~1m_Y$*tietQQ7oM=S2&#HTzsOorQ5>r*N=#*0nAI zW@%k>RDTU}1F;kc(;+~lt6c+01Q;9BQP zJgAPeA85lN+6ms3h;YK42}eV!npF4877Hr2Db zo1sVqd%(S+6dZ{Xi7JaWKpB-nVI%>cE9+891aw{lswp5C9Yw7emt0J>a*ge>9z@cj zRuWVRe?gRi06;+FfnDStV+^_9(9IMFNhU!hR7ew%LA;<2+=j*qX$FO$nQk`~_=L+q zAOduHLf{NG0m>?slmMGb2c>}f|JhX3Rhh-6g7pXu2A)#HWkXXXE}J%0fQwC)UNu$n zwM}(J6awT7LDNqe+G~@s8|1L5Ch3J%Z)RTUu^yXk1foA^tGm!0l!j3SPS{I)U*7pqR3A@Pcalxt>?~of6Z>3xev%T=q~D zUvjcc{Quy}|HOVv&+6@9Ka1wd?*zH5>e(nGAUK)9ct_o%b`_XbX5B9xmfqyPQZ}u9mW9#KT5f2sSZ%Fn% zW{TMk3i+;ps?gQoUWcPBpZ`zT*BPgR(<~@br|~at$=d1nFE7;toq|O`fqOO#+PSp( zyxyi%)3U+GY#G=L+J#yWBq+iFN3VcH#*YSN?Ub`QB?f?K2SA{(00|3>VnD*09Z7|g ze%6NW(I`f;G$fxdX20fj0=74%Y$Mbyy%J7}U%_X9TE;Wxrm8s z#1kk+nYtwZ29+1i8^3f>pzVErftnbfUK9=XI1HD1_&8ufn;XBr3k&1_D)IR@*+p&s zsdj5jZwvxIw~Ji3_||S+w=L34^BKBWMM#(tK>}|Rw<9a~G=kPw{*OS3<_`fB?SBKz zw!x@Tv%GQrpVais&*&{4`m^8=Mr~}k>u`A=gv6H+{{rI2^$_oPM6!fwh2f?c)dEtJ zSoYN^M=Kl{q*S45A#$|is(!oFY$2>x7(Oz?t&yMx=qimL=!DZPo!Gse;VlgqpFk1T<3Se(<8FPCwND(&g%kyOvL!IYK=m*JP zmn?rRu%GO+T7t~3Wx7;Cb352U zVE2gGk%gcx0Q?1J5#YDif+qqbh$}I~I=K&n)}dPr8cqZo|G0B3lr{xdIV0 z)*uXc#b5}-hXn@X=lNp3FeRT=fVksU_Ni86)@J`w>pvb=^>Dp=T7C4S>JHY|4!ia7 z*?L_apA1L!%k^P>xPCG?syR+XyDr;P_-vt)C6{4}aP{8LU5f@f6>v*6N{%lIJ4w6a3!6@BhXB^KN}Idf6G` hi&oV+JUglmJKtH`THCxIVhI~`eF@)v`23K*zW{;(I0gU! literal 0 HcmV?d00001