Problem (Fix #1621): Can't deposit into other's new staking address

Solution:
- Remove the restriction from client network ops code
- Add check and double confirmation in client-cli

CHANGELOG.md

@@ -10,6 +10,17 @@
 ### Improvements
 ### Bug Fixes
 
+*May 20, 2020*
+
+This release contains a hotfix for client-cli deposit transaction issue.
+
+## V0.5.3
+
+### Bug Fixes
+
+- *client-cli* [1652](https://github.com/crypto-com/chain/pull/1625): Add staking address check and double confirmation
+  in client-cli for deposit transaction
+
 *May 7, 2020*
 
 This release contains a hotfix for client-cli sync issue.

client-cli/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "client-cli"
-version = "0.5.2"
+version = "0.5.3"
 authors = ["Devashish Dixit <devashish@crypto.com>"]
 edition = "2018"
 build = "build.rs"

client-cli/src/command/transaction_command.rs

@@ -618,6 +618,55 @@ fn new_unbond_transaction<N: NetworkOpsClient>(
         .create_unbond_stake_transaction(name, enckey, address, value, attributes, true)
 }
 
+/// Check the staking address exists:
+/// - belong to our own wallet
+/// - exists on the chain
+/// - not jailed
+fn check_staking_address_for_deposit<T: WalletClient, N: NetworkOpsClient>(
+    wallet_client: &T,
+    network_ops: &N,
+    name: &str,
+    enckey: &SecKey,
+    address: &StakedStateAddress,
+) -> Result<()> {
+    // if the to_address belongs to current wallet, we do not check the state
+    let staking_addresses = wallet_client.staking_addresses(name, enckey)?;
+    if !staking_addresses.contains(address) {
+        let staking = network_ops.get_staked_state(name, address, true).err_kind(ErrorKind::ValidationError,|| "Address not found in the current wallet and is not yet initialized on the blockchain")?;
+        if staking.is_jailed() {
+            return Err(Error::new(
+                ErrorKind::ValidationError,
+                "staking address is jailed",
+            ));
+        }
+    }
+    Ok(())
+}
+
+fn double_confirm_staking_address<T: WalletClient, N: NetworkOpsClient>(
+    wallet_client: &T,
+    network_ops: &N,
+    name: &str,
+    enckey: &SecKey,
+    address: &StakedStateAddress,
+) -> Result<()> {
+    if let Err(err) =
+        check_staking_address_for_deposit(wallet_client, network_ops, name, enckey, address)
+    {
+        // double confirmation
+        ask(&format!("{}\nAre you sure to proceed? [yN]", err));
+        match yesno(false).chain(|| (ErrorKind::IoError, "Unable to read yes/no"))? {
+            None => return Err(ErrorKind::InvalidInput.into()),
+            Some(value) => {
+                if !value {
+                    return Err(Error::new(ErrorKind::InvalidInput, "User canceled"));
+                }
+            }
+        }
+    }
+    Ok(())
+}
+
 fn new_deposit_transaction<T: WalletClient, N: NetworkOpsClient>(
     wallet_client: &T,
     network_ops_client: &N,
@@ -627,6 +676,7 @@ fn new_deposit_transaction<T: WalletClient, N: NetworkOpsClient>(
     let attributes = StakedStateOpAttributes::new(get_network_id());
     let inputs = ask_inputs()?;
     let to_address = ask_staking_address()?;
+    double_confirm_staking_address(wallet_client, network_ops_client, name, enckey, &to_address)?;
     if !wallet_client.has_unspent_transactions(name, enckey, &inputs)? {
         return Err(Error::new(
             ErrorKind::InvalidInput,
@@ -646,7 +696,6 @@ fn new_deposit_transaction<T: WalletClient, N: NetworkOpsClient>(
         transactions,
         to_address,
         attributes,
-        true,
     )
 }
 
@@ -657,6 +706,13 @@ fn new_deposit_amount_transaction<T: WalletClient, N: NetworkOpsClient>(
     enckey: &SecKey,
 ) -> Result<()> {
     let to_staking_address = ask_staking_address()?;
+    double_confirm_staking_address(
+        wallet_client,
+        network_ops_client,
+        name,
+        enckey,
+        &to_staking_address,
+    )?;
     let attr = StakedStateOpAttributes::new(get_network_id());
     let amount = ask_cro()?;
     let fee = network_ops_client.calculate_deposit_fee()?;
@@ -704,7 +760,6 @@ fn new_deposit_amount_transaction<T: WalletClient, N: NetworkOpsClient>(
         transactions,
         to_staking_address,
         attr,
-        true,
     )?;
     let tx_id = transaction.tx_id();
     success(&format!(

client-network/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "client-network"
-version = "0.5.0"
+version = "0.5.3"
 authors = ["Devashish Dixit <devashish@crypto.com>"]
 edition = "2018"
 

client-network/src/network_ops.rs

@@ -12,7 +12,7 @@ use chain_core::tx::data::attribute::TxAttributes;
 use chain_core::tx::data::input::TxoPointer;
 use chain_core::tx::data::output::TxOut;
 use chain_core::tx::TxAux;
-use client_common::{Result, SecKey};
+use client_common::{ErrorKind, Result, ResultExt, SecKey};
 use client_core::types::TransactionPending;
 
 /// Interface for performing network operations on Crypto.com Chain
@@ -91,5 +91,16 @@ pub trait NetworkOpsClient: Send + Sync {
         name: &str,
         address: &StakedStateAddress,
         verify: bool,
-    ) -> Result<StakedState>;
+    ) -> Result<StakedState> {
+        self.get_staking(name, address, verify)?
+            .err_kind(ErrorKind::InvalidInput, || "staking not found")
+    }
+
+    /// Returns staked stake corresponding to given address
+    fn get_staking(
+        &self,
+        name: &str,
+        address: &StakedStateAddress,
+        verify: bool,
+    ) -> Result<Option<StakedState>>;
 }

client-network/src/network_ops/default_network_ops_client.rs

@@ -139,18 +139,14 @@ where
         attributes: StakedStateOpAttributes,
         verify_staking: bool,
     ) -> Result<(TxAux, TransactionPending)> {
-        // if the to_address belongs to current wallet, we do not check the state
-        let staking_addresses = self.wallet_client.staking_addresses(name, enckey)?;
-        if !staking_addresses.contains(&to_address) {
-            let staked_state = self.get_staked_state(name, &to_address, verify_staking)?;
-            verify_unjailed(&staked_state).map_err(|e| {
+        if let Some(staking) = self.get_staking(name, &to_address, verify_staking)? {
+            verify_unjailed(&staking).map_err(|e| {
                 Error::new(
                     ErrorKind::ValidationError,
                     format!("Failed to validate staking account: {}", e),
                 )
             })?;
         }
-
         let inputs = transactions
             .iter()
             .map(|(input, _)| input.clone())
@@ -460,13 +456,12 @@ where
         )))
     }
 
-    #[inline]
-    fn get_staked_state(
+    fn get_staking(
         &self,
         name: &str,
         address: &StakedStateAddress,
         verify: bool,
-    ) -> Result<StakedState> {
+    ) -> Result<Option<StakedState>> {
         let mstaking = if verify {
             let sync_state = self.wallet_client.get_sync_state(name)?;
             let rsp = self.client.query(
@@ -517,7 +512,7 @@ where
                     format!("Cannot deserialize staked state for address: {}", address)
                 })?
         };
-        mstaking.err_kind(ErrorKind::InvalidInput, || "staking not found")
+        Ok(mstaking)
     }
 }