From 49e609c7c7ff3ef9c3d9b6cb30b432f67a7c1bb0 Mon Sep 17 00:00:00 2001 From: Nicolas80 <nicolas.pmail@protonmail.com> Date: Sun, 30 Mar 2025 19:00:32 +0200 Subject: [PATCH] Code review: * Adapted commands identity `LinkAccount` & `ChangeOwnerKey` * Added possibility to provide * `-a` address of vault account to link * `-v` name of vault account to link * `-S` (secret_format) doesn't have a default anymore since it conflicts with using `-a` or `-v` * Added an error message if none of the params are provided --- src/commands/identity.rs | 160 ++++++++++++++++++++++++++------------- 1 file changed, 108 insertions(+), 52 deletions(-) diff --git a/src/commands/identity.rs b/src/commands/identity.rs index 4c744cd..25e8409 100644 --- a/src/commands/identity.rs +++ b/src/commands/identity.rs @@ -1,5 +1,6 @@ use crate::*; +use crate::commands::vault::retrieve_account_tree_node_for_name; use crate::{ commands::revocation::generate_revoc_doc, runtime::runtime_types::{ @@ -63,32 +64,30 @@ pub enum Subcommand { /// Display member count MemberCount, /// Link an account to the identity - LinkAccount { - /// Secret key format of account to link (seed, substrate) - #[clap(short = 'S', long, default_value = SecretFormat::Substrate)] - secret_format: SecretFormat, - /// Secret of account to link - /// most likely different from the one owning the identity - #[clap(short, long)] - secret: Option<String>, - /// Crypto scheme of account to link (sr25519, ed25519) - #[clap(short = 'c', long, required = false, default_value = CryptoScheme::Ed25519)] - crypto_scheme: CryptoScheme, - }, + LinkAccount(AccountLinkParams), /// Migrate identity to another account /// Change Owner Key - ChangeOwnerKey { - /// Secret key format of account to link (seed, substrate) - #[clap(short = 'S', long, default_value = SecretFormat::Substrate)] - secret_format: SecretFormat, - /// Secret of account to link - /// most likely different from the one owning the identity - #[clap(short, long)] - secret: Option<String>, - /// Crypto scheme of account to link (sr25519, ed25519) - #[clap(short = 'c', long, required = false, default_value = CryptoScheme::Ed25519)] - crypto_scheme: CryptoScheme, - }, + ChangeOwnerKey(AccountLinkParams), +} + +#[derive(clap::Args, Clone, Debug)] +pub struct AccountLinkParams { + /// SS58 Address of vault account to link + #[clap(short, conflicts_with_all=["vault_name","secret_format", "secret", "crypto_scheme"])] + address: Option<AccountId>, + /// Name of vault account to link + #[clap(short = 'v', conflicts_with_all=["secret_format", "secret", "crypto_scheme"])] + vault_name: Option<String>, + /// Secret key format of account to link (seed, substrate) + #[clap(short = 'S', long)] + secret_format: Option<SecretFormat>, + /// Secret of account to link + /// most likely different from the one owning the identity + #[clap(short, long)] + secret: Option<String>, + /// Crypto scheme of account to link (sr25519, ed25519) + #[clap(short = 'c', long, required = false, default_value = CryptoScheme::Ed25519)] + crypto_scheme: CryptoScheme, } /// handle identity commands @@ -156,41 +155,98 @@ pub async fn handle_command(data: Data, command: Subcommand) -> Result<(), GcliE .unwrap() ) } - Subcommand::LinkAccount { - secret_format, - secret, - crypto_scheme, - } => { - let keypair = get_keypair(secret_format, secret.as_deref(), crypto_scheme)?; - println!( - "target address:'{}' (using crypto-scheme:{})", - keypair.address(), - <&'static str>::from(crypto_scheme) - ); - let address = keypair.address(); - data = data.fetch_idty_index().await?; // idty index required for payload - link_account(&data, address, keypair).await?; + Subcommand::LinkAccount(params) => { + async fn perform_link_account(data: Data, keypair: KeyPair) -> Result<(), GcliError> { + println!("Trying to make the link"); + let address = keypair.address(); + let data = data.fetch_idty_index().await?; // idty index required for payload + link_account(&data, address, keypair).await?; + Ok(()) + } + + if let Some(address) = params.address { + let key_pair = fetch_vault_keypair_for_target_address(&data, address).await?; + perform_link_account(data, key_pair).await?; + } else if let Some(vault_name) = params.vault_name { + let account_tree_node = + retrieve_account_tree_node_for_name(data.connect_db(), &vault_name).await?; + let address = account_tree_node.borrow().account.address.0.clone(); + + let key_pair = fetch_vault_keypair_for_target_address(&data, address).await?; + perform_link_account(data, key_pair).await?; + } else if let Some(secret_format) = params.secret_format { + let keypair = get_keypair( + secret_format, + params.secret.as_deref(), + params.crypto_scheme, + )?; + println!( + "target address:'{}' (using crypto-scheme:{})", + keypair.address(), + <&'static str>::from(params.crypto_scheme) + ); + perform_link_account(data, keypair).await?; + } else { + return Err(GcliError::Input( + "One of `address`/`vault_name`/`secret_format`(and optional `secret` & `crypto_scheme`) must be provided".to_string(), + )); + } } - Subcommand::ChangeOwnerKey { - secret_format, - secret, - crypto_scheme, - } => { - let keypair = get_keypair(secret_format, secret.as_deref(), crypto_scheme)?; - println!( - "target address:'{}' (using crypto-scheme:{})", - keypair.address(), - <&'static str>::from(crypto_scheme) - ); - let address = keypair.address(); - data = data.fetch_idty_index().await?; // idty index required for payload - change_owner_key(&data, address, keypair).await?; + Subcommand::ChangeOwnerKey(params) => { + async fn perform_change_owner_key( + data: Data, + keypair: KeyPair, + ) -> Result<(), GcliError> { + println!("Trying to change owner key"); + let address = keypair.address(); + let data = data.fetch_idty_index().await?; // idty index required for payload + change_owner_key(&data, address, keypair).await?; + Ok(()) + } + + if let Some(address) = params.address { + let key_pair = fetch_vault_keypair_for_target_address(&data, address).await?; + perform_change_owner_key(data, key_pair).await?; + } else if let Some(vault_name) = params.vault_name { + let account_tree_node = + retrieve_account_tree_node_for_name(data.connect_db(), &vault_name).await?; + let address = account_tree_node.borrow().account.address.0.clone(); + + let key_pair = fetch_vault_keypair_for_target_address(&data, address).await?; + perform_change_owner_key(data, key_pair).await?; + } else if let Some(secret_format) = params.secret_format { + let keypair = get_keypair( + secret_format, + params.secret.as_deref(), + params.crypto_scheme, + )?; + println!( + "target address:'{}' (using crypto-scheme:{})", + keypair.address(), + <&'static str>::from(params.crypto_scheme) + ); + perform_change_owner_key(data, keypair).await?; + } else { + return Err(GcliError::Input( + "One of `address`/`vault_name`/`secret_format`(and optional `secret` & `crypto_scheme`) must be provided".to_string(), + )); + } } }; Ok(()) } +async fn fetch_vault_keypair_for_target_address( + data: &Data, + address: AccountId, +) -> Result<KeyPair, GcliError> { + println!("Trying to retrieve key pair for target address:'{address}'"); + commands::vault::try_fetch_key_pair(data, address) + .await? + .ok_or_else(|| GcliError::Input("target vault account not found".to_string())) +} + // ====================== // TODO derive this automatically -- GitLab