From e18844d1f42cf60dd94ce5ac01c4810d38d23a97 Mon Sep 17 00:00:00 2001 From: Svyatoslav Nikolsky Date: Mon, 19 Jul 2021 14:45:16 +0300 Subject: [PATCH] Allow reading suri && password override from file (#1059) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * allow reading suri && password override from file * fix clippy * Update relays/bin-substrate/src/cli/mod.rs Co-authored-by: Tomasz Drwięga * Update relays/bin-substrate/src/cli/mod.rs Co-authored-by: Tomasz Drwięga Co-authored-by: Tomasz Drwięga --- bridges/relays/bin-substrate/Cargo.toml | 1 + bridges/relays/bin-substrate/src/cli/mod.rs | 126 +++++++++++++++++- .../bin-substrate/src/cli/send_message.rs | 21 +-- 3 files changed, 128 insertions(+), 20 deletions(-) diff --git a/bridges/relays/bin-substrate/Cargo.toml b/bridges/relays/bin-substrate/Cargo.toml index 6b68fac986..7b731e3cd7 100644 --- a/bridges/relays/bin-substrate/Cargo.toml +++ b/bridges/relays/bin-substrate/Cargo.toml @@ -63,3 +63,4 @@ sp-version = { git = "https://github.com/paritytech/substrate", branch = "master hex-literal = "0.3" pallet-bridge-grandpa = { path = "../../modules/grandpa" } sp-keyring = { git = "https://github.com/paritytech/substrate", branch = "master" } +tempdir = "0.3" diff --git a/bridges/relays/bin-substrate/src/cli/mod.rs b/bridges/relays/bin-substrate/src/cli/mod.rs index 49bc5dc8c8..3088b06eb0 100644 --- a/bridges/relays/bin-substrate/src/cli/mod.rs +++ b/bridges/relays/bin-substrate/src/cli/mod.rs @@ -374,24 +374,61 @@ macro_rules! declare_chain_options { } #[doc = $chain " signing params."] - #[derive(StructOpt, Debug, PartialEq, Eq)] + #[derive(StructOpt, Debug, PartialEq, Eq, Clone)] pub struct [<$chain SigningParams>] { #[doc = "The SURI of secret key to use when transactions are submitted to the " $chain " node."] #[structopt(long)] - pub [<$chain_prefix _signer>]: String, + pub [<$chain_prefix _signer>]: Option, #[doc = "The password for the SURI of secret key to use when transactions are submitted to the " $chain " node."] #[structopt(long)] pub [<$chain_prefix _signer_password>]: Option, + + #[doc = "Path to the file, that contains SURI of secret key to use when transactions are submitted to the " $chain " node. Can be overridden with " $chain_prefix "_signer option."] + #[structopt(long)] + pub [<$chain_prefix _signer_file>]: Option, + #[doc = "Path to the file, that password for the SURI of secret key to use when transactions are submitted to the " $chain " node. Can be overridden with " $chain_prefix "_signer_password option."] + #[structopt(long)] + pub [<$chain_prefix _signer_password_file>]: Option, } impl [<$chain SigningParams>] { /// Parse signing params into chain-specific KeyPair. pub fn to_keypair(&self) -> anyhow::Result { + let suri = match (self.[<$chain_prefix _signer>].as_ref(), self.[<$chain_prefix _signer_file>].as_ref()) { + (Some(suri), _) => suri.to_owned(), + (None, Some(suri_file)) => std::fs::read_to_string(suri_file) + .map_err(|err| anyhow::format_err!( + "Failed to read SURI from file {:?}: {}", + suri_file, + err, + ))?, + (None, None) => return Err(anyhow::format_err!( + "One of options must be specified: '{}' or '{}'", + stringify!([<$chain_prefix _signer>]), + stringify!([<$chain_prefix _signer_file>]), + )), + }; + + let suri_password = match ( + self.[<$chain_prefix _signer_password>].as_ref(), + self.[<$chain_prefix _signer_password_file>].as_ref(), + ) { + (Some(suri_password), _) => Some(suri_password.to_owned()), + (None, Some(suri_password_file)) => std::fs::read_to_string(suri_password_file) + .map(Some) + .map_err(|err| anyhow::format_err!( + "Failed to read SURI password from file {:?}: {}", + suri_password_file, + err, + ))?, + _ => None, + }; + use sp_core::crypto::Pair; Chain::KeyPair::from_string( - &self.[<$chain_prefix _signer>], - self.[<$chain_prefix _signer_password>].as_deref() + &suri, + suri_password.as_deref() ).map_err(|e| anyhow::format_err!("{:?}", e)) } } @@ -419,6 +456,7 @@ declare_chain_options!(Target, target); #[cfg(test)] mod tests { + use sp_core::Pair; use std::str::FromStr; use super::*; @@ -456,4 +494,84 @@ mod tests { // then assert_eq!(hex.0, hex2.0); } + + #[test] + fn reads_suri_from_file() { + const ALICE: &str = "//Alice"; + const BOB: &str = "//Bob"; + const ALICE_PASSWORD: &str = "alice_password"; + const BOB_PASSWORD: &str = "bob_password"; + + let alice = sp_core::sr25519::Pair::from_string(ALICE, Some(ALICE_PASSWORD)).unwrap(); + let bob = sp_core::sr25519::Pair::from_string(BOB, Some(BOB_PASSWORD)).unwrap(); + let bob_with_alice_password = sp_core::sr25519::Pair::from_string(BOB, Some(ALICE_PASSWORD)).unwrap(); + + let temp_dir = tempdir::TempDir::new("reads_suri_from_file").unwrap(); + let mut suri_file_path = temp_dir.path().to_path_buf(); + let mut password_file_path = temp_dir.path().to_path_buf(); + suri_file_path.push("suri"); + password_file_path.push("password"); + std::fs::write(&suri_file_path, BOB.as_bytes()).unwrap(); + std::fs::write(&password_file_path, BOB_PASSWORD.as_bytes()).unwrap(); + + // when both seed and password are read from file + assert_eq!( + TargetSigningParams { + target_signer: Some(ALICE.into()), + target_signer_password: Some(ALICE_PASSWORD.into()), + + target_signer_file: None, + target_signer_password_file: None, + } + .to_keypair::() + .map(|p| p.public()) + .map_err(drop), + Ok(alice.public()), + ); + + // when both seed and password are read from file + assert_eq!( + TargetSigningParams { + target_signer: None, + target_signer_password: None, + + target_signer_file: Some(suri_file_path.clone()), + target_signer_password_file: Some(password_file_path.clone()), + } + .to_keypair::() + .map(|p| p.public()) + .map_err(drop), + Ok(bob.public()), + ); + + // when password are is overriden by cli option + assert_eq!( + TargetSigningParams { + target_signer: None, + target_signer_password: Some(ALICE_PASSWORD.into()), + + target_signer_file: Some(suri_file_path.clone()), + target_signer_password_file: Some(password_file_path.clone()), + } + .to_keypair::() + .map(|p| p.public()) + .map_err(drop), + Ok(bob_with_alice_password.public()), + ); + + // when both seed and password are overriden by cli options + assert_eq!( + TargetSigningParams { + target_signer: Some(ALICE.into()), + target_signer_password: Some(ALICE_PASSWORD.into()), + + target_signer_file: Some(suri_file_path), + target_signer_password_file: Some(password_file_path), + } + .to_keypair::() + .map(|p| p.public()) + .map_err(drop), + Ok(alice.public()), + ); + } } diff --git a/bridges/relays/bin-substrate/src/cli/send_message.rs b/bridges/relays/bin-substrate/src/cli/send_message.rs index 496b3707a9..dca8205043 100644 --- a/bridges/relays/bin-substrate/src/cli/send_message.rs +++ b/bridges/relays/bin-substrate/src/cli/send_message.rs @@ -60,12 +60,8 @@ pub struct SendMessage { source: SourceConnectionParams, #[structopt(flatten)] source_sign: SourceSigningParams, - /// The SURI of secret key to use when transactions are submitted to the Target node. - #[structopt(long, required_if("origin", "Target"))] - target_signer: Option, - /// The password for the SURI of secret key to use when transactions are submitted to the Target node. - #[structopt(long)] - target_signer_password: Option, + #[structopt(flatten)] + target_sign: TargetSigningParams, /// Hex-encoded lane id. Defaults to `00000000`. #[structopt(long, default_value = "00000000")] lane: HexLaneId, @@ -99,8 +95,7 @@ impl SendMessage { crate::select_full_bridge!(self.bridge, { let SendMessage { source_sign, - target_signer, - target_signer_password, + target_sign, ref mut message, dispatch_fee_payment, dispatch_weight, @@ -129,12 +124,6 @@ impl SendMessage { match origin { Origins::Source => CallOrigin::SourceAccount(source_account_id), Origins::Target => { - let target_sign = TargetSigningParams { - target_signer: target_signer.clone().ok_or_else(|| { - anyhow::format_err!("The argument target_signer is not available") - })?, - target_signer_password: target_signer_password.clone(), - }; let target_sign = target_sign.to_keypair::()?; let digest = account_ownership_digest( &target_call, @@ -361,7 +350,7 @@ mod tests { } #[test] - fn target_signer_must_exist_if_origin_is_target() { + fn accepts_send_message_command_without_target_sign_options() { // given let send_message = SendMessage::from_iter_safe(vec![ "send-message", @@ -377,7 +366,7 @@ mod tests { "1234", ]); - assert!(send_message.is_err()); + assert!(send_message.is_ok()); } #[test]