Skip to content

Commit

Permalink
chore: support specify contract path input with or without -p flag (#361
Browse files Browse the repository at this point in the history 
)

* pop -p arg

* pop build & pop up completed

* re-starting from scratch

* Cleaner version of first proposal

* cargo +nightly fmt

* pop build completed

* pop up contract implementation

* pop call contract implemented

* Fixed some of the issues pointed in the reviews

* cargo +nightly fmt

* Review corrections implemented

* cargo fmt

* Fixing CI errors

* License
  • Loading branch information
@ndkazu
ndkazu authored Jan 7, 2025
1 parent ca1cd92 commit a91ccfd
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 37 deletions.
23 changes: 17 additions & 6 deletions crates/pop-cli/src/commands/build/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
// SPDX-License-Identifier: GPL-3.0

use crate::cli::{self, Cli};
use crate::{
cli::{self, Cli},
common::builds::get_project_path,
};
use clap::{Args, Subcommand};
#[cfg(feature = "contract")]
use contract::BuildContract;
Expand All @@ -23,9 +26,12 @@ pub(crate) mod spec;
pub(crate) struct BuildArgs {
#[command(subcommand)]
pub command: Option<Command>,
/// Directory path for your project [default: current directory]
/// Directory path with flag for your project [default: current directory]
#[arg(long)]
pub(crate) path: Option<PathBuf>,
/// Directory path without flag for your project [default: current directory]
#[arg(value_name = "PATH", index = 1, conflicts_with = "path")]
pub(crate) path_pos: Option<PathBuf>,
/// The package to be built.
#[arg(short = 'p', long)]
pub(crate) package: Option<String>,
Expand All @@ -50,25 +56,29 @@ impl Command {
/// Executes the command.
pub(crate) fn execute(args: BuildArgs) -> anyhow::Result<&'static str> {
// If only contract feature enabled, build as contract
let project_path = get_project_path(args.path.clone(), args.path_pos.clone());

#[cfg(feature = "contract")]
if pop_contracts::is_supported(args.path.as_deref())? {
if pop_contracts::is_supported(project_path.as_deref().map(|v| v))? {

Check warning on line 62 in crates/pop-cli/src/commands/build/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary map of the identity function 

warning: unnecessary map of the identity function
  --> crates/pop-cli/src/commands/build/mod.rs:62:57
   |
62 |         if pop_contracts::is_supported(project_path.as_deref().map(|v| v))? {
   |                                                               ^^^^^^^^^^^ help: remove the call to `map`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
   = note: `#[warn(clippy::map_identity)]` on by default
// All commands originating from root command are valid
let release = match args.profile {
Some(profile) => profile.into(),
None => args.release,
};
BuildContract { path: args.path, release }.execute()?;
BuildContract { path: project_path, release }.execute()?;
return Ok("contract");
}

// If only parachain feature enabled, build as parachain
#[cfg(feature = "parachain")]
if pop_parachains::is_supported(args.path.as_deref())? {
if pop_parachains::is_supported(project_path.as_deref().map(|v| v))? {

Check warning on line 74 in crates/pop-cli/src/commands/build/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary map of the identity function 

warning: unnecessary map of the identity function
  --> crates/pop-cli/src/commands/build/mod.rs:74:58
   |
74 |         if pop_parachains::is_supported(project_path.as_deref().map(|v| v))? {
   |                                                                ^^^^^^^^^^^ help: remove the call to `map`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
let profile = match args.profile {
Some(profile) => profile,
None => args.release.into(),
};
let temp_path = PathBuf::from("./");
BuildParachain {
path: args.path.unwrap_or_else(|| PathBuf::from("./")),
path: project_path.unwrap_or_else(|| temp_path).to_path_buf(),

Check warning on line 81 in crates/pop-cli/src/commands/build/mod.rs

View workflow job for this annotation

GitHub Actions / clippy

unnecessary closure used to substitute value for `Option::None` 

warning: unnecessary closure used to substitute value for `Option::None`
  --> crates/pop-cli/src/commands/build/mod.rs:81:11
   |
81 |                 path: project_path.unwrap_or_else(|| temp_path).to_path_buf(),
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
   = note: `#[warn(clippy::unnecessary_lazy_evaluations)]` on by default
help: use `unwrap_or` instead
   |
81 |                 path: project_path.unwrap_or(temp_path).to_path_buf(),
   |                                    ~~~~~~~~~~~~~~~~~~~~
package: args.package,
profile,
}
Expand Down Expand Up @@ -140,6 +150,7 @@ mod tests {
BuildArgs {
command: None,
path: Some(project_path.clone()),
path_pos: Some(project_path.clone()),
package: package.clone(),
release,
profile: Some(profile.clone()),
Expand Down
63 changes: 39 additions & 24 deletions crates/pop-cli/src/commands/call/contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::{
cli::{self, traits::*},
common::{
builds::get_project_path,
contracts::has_contract_been_built,
wallet::{prompt_to_use_wallet, request_signature},
},
Expand All @@ -17,7 +18,7 @@ use pop_contracts::{
parse_account, set_up_call, CallExec, CallOpts, DefaultEnvironment, Verbosity,
};
use sp_weights::Weight;
use std::path::{Path, PathBuf};
use std::path::PathBuf;

const DEFAULT_URL: &str = "ws://localhost:9944/";
const DEFAULT_URI: &str = "//Alice";
Expand All @@ -28,6 +29,9 @@ pub struct CallContractCommand {
/// Path to the contract build directory or a contract artifact.
#[arg(short, long)]
path: Option<PathBuf>,
/// Directory path without flag for your project [default: current directory]
#[arg(value_name = "PATH", index = 1, conflicts_with = "path")]
pub(crate) path_pos: Option<PathBuf>,
/// The address of the contract to call.
#[arg(short, long, env = "CONTRACT")]
contract: Option<String>,
Expand Down Expand Up @@ -109,9 +113,13 @@ impl CallContractCommand {

fn display(&self) -> String {
let mut full_message = "pop call contract".to_string();

if let Some(path) = &self.path {
full_message.push_str(&format!(" --path {}", path.display()));
}
if let Some(path_pos) = &self.path_pos {
full_message.push_str(&format!(" --path {}", path_pos.display()));
}
if let Some(contract) = &self.contract {
full_message.push_str(&format!(" --contract {}", contract));
}
Expand Down Expand Up @@ -148,11 +156,12 @@ impl CallContractCommand {

/// If the contract has not been built, build it in release mode.
async fn ensure_contract_built(&self, cli: &mut impl Cli) -> Result<()> {
let project_path = get_project_path(self.path.clone(), self.path_pos.clone());
// 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) {
let result = match build_smart_contract(project_path.as_deref(), true, Verbosity::Quiet) {
Ok(result) => result,
Err(e) => {
return Err(anyhow!(format!(
Expand Down Expand Up @@ -182,14 +191,18 @@ impl CallContractCommand {

/// Checks whether building the contract is required
fn is_contract_build_required(&self) -> bool {
self.path
let project_path = get_project_path(self.path.clone(), self.path_pos.clone());

project_path
.as_ref()
.map(|p| p.is_dir() && !has_contract_been_built(Some(p)))
.unwrap_or_default()
}

/// Configure the call based on command line arguments/call UI.
async fn configure(&mut self, cli: &mut impl Cli, repeat: bool) -> Result<()> {
let mut project_path = get_project_path(self.path.clone(), self.path_pos.clone());

// Show intro on first run.
if !repeat {
cli.intro("Call a contract")?;
Expand All @@ -201,16 +214,15 @@ impl CallContractCommand {
}

// Resolve path.
if self.path.is_none() {
if project_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));
project_path = Some(PathBuf::from(input_path));
}
let contract_path = self
.path
let contract_path = project_path
.as_ref()
.expect("path is guaranteed to be set as input as prompted when None; qed");

Expand Down Expand Up @@ -357,15 +369,18 @@ impl CallContractCommand {
cli: &mut impl Cli,
prompt_to_repeat_call: bool,
) -> Result<()> {
let project_path = get_project_path(self.path.clone(), self.path_pos.clone());

let message = match &self.message {
Some(message) => message.to_string(),
None => {
return Err(anyhow!("Please specify the message to call."));
},
};
// Disable wallet signing and display warning if the call is read-only.
let path = PathBuf::from("./");
let message_metadata =
get_message(self.path.as_deref().unwrap_or_else(|| Path::new("./")), &message)?;
get_message(project_path.as_deref().unwrap_or_else(|| &path), &message)?;
if !message_metadata.mutates && self.use_wallet {
cli.warning("NOTE: Signing is not required for this read-only call. The '--use-wallet' flag will be ignored.")?;
self.use_wallet = false;
Expand All @@ -378,7 +393,7 @@ impl CallContractCommand {
},
};
let call_exec = match set_up_call(CallOpts {
path: self.path.clone(),
path: project_path,
contract,
message,
args: self.args.clone(),
Expand Down Expand Up @@ -564,6 +579,7 @@ mod tests {
// Contract deployed on Pop Network testnet, test get
CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: Some("get".to_string()),
args: vec![].to_vec(),
Expand Down Expand Up @@ -600,6 +616,7 @@ mod tests {

let mut call_config = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: Some("flip".to_string()),
args: vec![].to_vec(),
Expand Down Expand Up @@ -637,6 +654,7 @@ mod tests {
// From .contract file
let mut call_config = CallContractCommand {
path: Some(current_dir.join("pop-contracts/tests/files/testing.contract")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: Some("flip".to_string()),
args: vec![].to_vec(),
Expand Down Expand Up @@ -723,6 +741,7 @@ mod tests {
// Contract deployed on Pop Network testnet, test get
let mut call_config = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: Some("get".to_string()),
args: vec![].to_vec(),
Expand Down Expand Up @@ -770,10 +789,6 @@ mod tests {
Some(items),
1, // "get" message
)
.expect_input(
"Where is your project or contract artifact located?",
temp_dir.path().join("testing").display().to_string(),
)
.expect_input(
"Where is your contract deployed?",
"wss://rpc1.paseo.popnetwork.xyz".into(),
Expand All @@ -789,6 +804,7 @@ mod tests {

let mut call_config = CallContractCommand {
path: None,
path_pos: Some(temp_dir.path().join("testing")),
contract: None,
message: None,
args: vec![].to_vec(),
Expand Down Expand Up @@ -818,7 +834,7 @@ mod tests {
assert!(!call_config.dry_run);
assert_eq!(call_config.display(), format!(
"pop call contract --path {} --contract 15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm --message get --url wss://rpc1.paseo.popnetwork.xyz/ --suri //Alice",
temp_dir.path().join("testing").display().to_string(),
temp_dir.path().join("testing").display().to_string()
));

cli.verify()
Expand All @@ -844,10 +860,6 @@ mod tests {
];
// The inputs are processed in reverse order.
let mut cli = MockCli::new()
.expect_input(
"Where is your project or contract artifact located?",
temp_dir.path().join("testing").display().to_string(),
)
.expect_input(
"Where is your contract deployed?",
"wss://rpc1.paseo.popnetwork.xyz".into(),
Expand Down Expand Up @@ -876,6 +888,7 @@ mod tests {

let mut call_config = CallContractCommand {
path: None,
path_pos: Some(temp_dir.path().join("testing")),
contract: None,
message: None,
args: vec![].to_vec(),
Expand Down Expand Up @@ -908,7 +921,7 @@ mod tests {
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/ --use-wallet --execute",
temp_dir.path().join("testing").display().to_string(),
temp_dir.path().join("testing").display().to_string()
));

cli.verify()
Expand Down Expand Up @@ -941,10 +954,6 @@ mod tests {
Some(items),
2, // "specific_flip" message
)
.expect_input(
"Where is your project or contract artifact located?",
temp_dir.path().join("testing").display().to_string(),
)
.expect_input(
"Where is your contract deployed?",
"wss://rpc1.paseo.popnetwork.xyz".into(),
Expand All @@ -964,6 +973,7 @@ mod tests {

let mut call_config = CallContractCommand {
path: None,
path_pos: Some(temp_dir.path().join("testing")),
contract: None,
message: None,
args: vec![].to_vec(),
Expand Down Expand Up @@ -996,7 +1006,7 @@ mod tests {
assert!(call_config.dev_mode);
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",
temp_dir.path().join("testing").display().to_string(),
temp_dir.path().join("testing").display().to_string()
));

cli.verify()
Expand All @@ -1023,6 +1033,7 @@ mod tests {
// Test the path is a folder with an invalid build.
let mut command = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: None,
message: None,
args: vec![].to_vec(),
Expand Down Expand Up @@ -1073,6 +1084,7 @@ mod tests {
assert!(matches!(
CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: None,
args: vec![].to_vec(),
Expand All @@ -1092,6 +1104,7 @@ mod tests {
assert!(matches!(
CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: None,
message: Some("get".to_string()),
args: vec![].to_vec(),
Expand All @@ -1116,6 +1129,7 @@ mod tests {
let temp_dir = new_environment("testing")?;
let call_config = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: None,
args: vec![].to_vec(),
Expand Down Expand Up @@ -1147,6 +1161,7 @@ mod tests {
let temp_dir = new_environment("testing")?;
let call_config = CallContractCommand {
path: Some(temp_dir.path().join("testing")),
path_pos: None,
contract: Some("15XausWjFLBBFLDXUSBRfSfZk25warm4wZRV4ZxhZbfvjrJm".to_string()),
message: None,
args: vec![].to_vec(),
Expand Down
Loading

0 comments on commit a91ccfd

Please sign in to comment.