Skip to content

Commit 33fcec0

Browse files
authored
fix(call-contract): avoid redundant deploy and execute prompts (#981)
* fix(call-contract): avoid redundant deploy and execute prompts * fix(fork): include polkadot.js and papi links in detached output * fix: satisfy rustfmt in fork command * Revert "fix: satisfy rustfmt in fork command" This reverts commit d189b05. * Revert "fix(fork): include polkadot.js and papi links in detached output" This reverts commit c7d29b3.
1 parent 545a477 commit 33fcec0

1 file changed

Lines changed: 92 additions & 8 deletions

File tree

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

Lines changed: 92 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ impl CallContractCommand {
241241
Ok(())
242242
}
243243

244+
fn should_confirm_contract_deployment(&self) -> bool {
245+
self.contract.is_none() && !self.deployed && !self.skip_confirm
246+
}
247+
244248
/// Checks whether building the contract is required
245249
fn is_contract_build_required(&self) -> bool {
246250
let project_path = get_project_path(self.path.clone(), self.path_pos.clone());
@@ -295,14 +299,20 @@ impl CallContractCommand {
295299
}
296300

297301
// Finally prompt for confirmation.
298-
let is_call_confirmed = if message.mutates && !self.skip_confirm && !self.use_wallet {
299-
cli.confirm("Do you want to execute the call? (Selecting 'No' will perform a dry run)")
300-
.initial_value(true)
301-
.interact()?
302+
if message.mutates {
303+
if !self.skip_confirm && !self.use_wallet && !self.execute {
304+
self.execute = cli
305+
.confirm(
306+
"Do you want to execute the call? (Selecting 'No' will perform a dry run)",
307+
)
308+
.initial_value(true)
309+
.interact()?;
310+
} else {
311+
self.execute = true;
312+
}
302313
} else {
303-
true
304-
};
305-
self.execute = is_call_confirmed && message.mutates;
314+
self.execute = false;
315+
}
306316
Ok(())
307317
}
308318

@@ -338,7 +348,9 @@ impl CallContractCommand {
338348
// Ensure contract is built and check if deployed.
339349
if self.is_contract_build_required() {
340350
self.ensure_contract_built(cli).await?;
341-
self.confirm_contract_deployment(cli)?;
351+
if self.should_confirm_contract_deployment() {
352+
self.confirm_contract_deployment(cli)?;
353+
}
342354
}
343355

344356
// Parse the contract metadata provided. If there is an error, do not prompt for more.
@@ -622,6 +634,7 @@ impl CallContractCommand {
622634
self.gas_limit = None;
623635
self.proof_size = None;
624636
self.use_wallet = false;
637+
self.execute = false;
625638
}
626639
}
627640

@@ -748,6 +761,31 @@ mod tests {
748761
cli.verify()
749762
}
750763

764+
#[test]
765+
fn configure_message_does_not_prompt_execute_when_execute_flag_set() -> Result<()> {
766+
let message = ContractFunction {
767+
label: "run".into(),
768+
payable: false,
769+
args: vec![],
770+
docs: String::new(),
771+
default: false,
772+
mutates: true,
773+
};
774+
775+
let mut command = CallContractCommand {
776+
execute: true,
777+
suri: Some("//Alice".to_string()),
778+
use_wallet: false,
779+
skip_confirm: false,
780+
..Default::default()
781+
};
782+
783+
let mut cli = MockCli::new();
784+
command.configure_message(&message, &mut cli)?;
785+
assert!(command.execute);
786+
cli.verify()
787+
}
788+
751789
// This test only covers the interactive portion of the call contract command, without actually
752790
// calling the contract.
753791
#[tokio::test]
@@ -1059,6 +1097,30 @@ mod tests {
10591097
cli.verify()
10601098
}
10611099

1100+
#[test]
1101+
fn should_confirm_contract_deployment_works() {
1102+
let command = CallContractCommand { contract: None, deployed: false, ..Default::default() };
1103+
assert!(command.should_confirm_contract_deployment());
1104+
1105+
let command = CallContractCommand {
1106+
contract: Some("0x48550a4bb374727186c55365b7c9c0a1a31bdafe".to_string()),
1107+
deployed: false,
1108+
..Default::default()
1109+
};
1110+
assert!(!command.should_confirm_contract_deployment());
1111+
1112+
let command = CallContractCommand { contract: None, deployed: true, ..Default::default() };
1113+
assert!(!command.should_confirm_contract_deployment());
1114+
1115+
let command = CallContractCommand {
1116+
contract: None,
1117+
deployed: false,
1118+
skip_confirm: true,
1119+
..Default::default()
1120+
};
1121+
assert!(!command.should_confirm_contract_deployment());
1122+
}
1123+
10621124
#[tokio::test]
10631125
#[allow(deprecated)]
10641126
async fn is_contract_build_required_works() -> Result<()> {
@@ -1290,4 +1352,26 @@ mod tests {
12901352
assert!(result.is_err(), "execute should fail when node is unavailable");
12911353
cli.verify()
12921354
}
1355+
1356+
#[test]
1357+
fn reset_for_new_call_resets_execute_flag() {
1358+
let mut command = CallContractCommand {
1359+
message: Some("get".into()),
1360+
value: "10".into(),
1361+
gas_limit: Some(1),
1362+
proof_size: Some(2),
1363+
use_wallet: true,
1364+
execute: true,
1365+
..Default::default()
1366+
};
1367+
1368+
command.reset_for_new_call();
1369+
1370+
assert_eq!(command.message, None);
1371+
assert_eq!(command.value, DEFAULT_PAYABLE_VALUE);
1372+
assert_eq!(command.gas_limit, None);
1373+
assert_eq!(command.proof_size, None);
1374+
assert!(!command.use_wallet);
1375+
assert!(!command.execute);
1376+
}
12931377
}

0 commit comments

Comments
 (0)