fix review notes: update signing status, get_public_key() return, sig…

…n_withdraw_tx, removed unused fn

mm2src/coins/eth.rs

@@ -1970,16 +1970,17 @@ impl MarketCoinOps for EthCoin {
                 Ok(format!("04{}", uncompressed_without_prefix))
             },
             EthPrivKeyPolicy::Trezor => {
-                let enabled_address = self
+                let public_key = self
                     .deref()
                     .derivation_method
                     .hd_wallet()
                     .ok_or(UnexpectedDerivationMethod::ExpectedHDWallet)?
                     .get_enabled_address()
                     .await
                     .ok_or_else(|| UnexpectedDerivationMethod::InternalError("no enabled address".to_owned()))?
-                    .address();
-                Ok(display_eth_address(&enabled_address))
+                    .pubkey();
+                let uncompressed_without_prefix = hex::encode(public_key);
+                Ok(format!("04{}", uncompressed_without_prefix))
             },
             #[cfg(target_arch = "wasm32")]
             EthPrivKeyPolicy::Metamask(ref metamask_policy) => {
@@ -6005,7 +6006,3 @@ pub fn pubkey_from_extended(extended_pubkey: &Secp256k1ExtendedPublicKey) -> Pub
     pubkey_uncompressed
 }
 
-pub fn pubkey_from_xpub_str(xpub: &str) -> Result<Public, String> {
-    let extended_pubkey = Secp256k1ExtendedPublicKey::from_str(xpub).map_err(|e| ERRL!("{}", e))?;
-    Ok(pubkey_from_extended(&extended_pubkey))
-}

mm2src/coins/eth/eth_withdraw.rs

@@ -20,6 +20,8 @@ use mm2_err_handle::mm_error::MmResult;
 use mm2_err_handle::prelude::{MapToMmResult, MmError, OrMmError};
 use std::ops::Deref;
 use std::sync::Arc;
+
+#[cfg(target_arch = "wasm32")]
 use web3::types::TransactionRequest;
 
 #[async_trait]
@@ -109,33 +111,42 @@ where
     async fn sign_withdraw_tx(
         &self,
         req: &WithdrawRequest,
-        tx: UnSignedEthTx,
-        tx_to_send: Option<TransactionRequest>,
+        unsigned_tx: UnSignedEthTx,
     ) -> Result<(H256, BytesJson), MmError<WithdrawError>> {
         let coin = self.coin();
         match coin.priv_key_policy {
             EthPrivKeyPolicy::Iguana(_) | EthPrivKeyPolicy::HDWallet { .. } => {
                 let key_pair = self.get_key_pair(req)?;
-                // Todo: nonce_lock is still global for all addresses but this needs to be per address
-                let signed = tx.sign(key_pair.secret(), coin.chain_id);
+                let signed = unsigned_tx.sign(key_pair.secret(), coin.chain_id);
                 let bytes = rlp::encode(&signed);
 
                 Ok((signed.hash, BytesJson::from(bytes.to_vec())))
             },
             EthPrivKeyPolicy::Trezor => {
                 let derivation_path = self.get_withdraw_derivation_path(req).await?;
-                let signed = self.sign_tx_with_trezor(&derivation_path, &tx).await?;
+                let signed = self.sign_tx_with_trezor(&derivation_path, &unsigned_tx).await?;
                 let bytes = rlp::encode(&signed);
                 Ok((signed.hash, BytesJson::from(bytes.to_vec())))
             },
             #[cfg(target_arch = "wasm32")]
+            EthPrivKeyPolicy::Metamask(_) => MmError::err(WithdrawError::InternalError("invalid policy".to_owned())),
+        }
+    }
+
+    #[cfg(target_arch = "wasm32")]
+    async fn send_withdraw_tx(
+        &self,
+        req: &WithdrawRequest,
+        tx_to_send: TransactionRequest,
+    ) -> Result<(H256, BytesJson), MmError<WithdrawError>> {
+        let coin = self.coin();
+        match coin.priv_key_policy {
             EthPrivKeyPolicy::Metamask(_) => {
                 if !req.broadcast {
                     let error =
                         "Set 'broadcast' to generate, sign and broadcast a transaction with MetaMask".to_string();
                     return MmError::err(WithdrawError::BroadcastExpected(error));
                 }
-                let tx_to_send = tx_to_send.unwrap_or_default();
 
                 // Wait for 10 seconds for the transaction to appear on the RPC node.
                 let wait_rpc_timeout = 10_000;
@@ -149,19 +160,26 @@ where
                     .wait_for_tx_appears_on_rpc(tx_hash, wait_rpc_timeout, check_every)
                     .await?;
                 let tx_hex = signed_tx
-                    .map(|tx| BytesJson::from(rlp::encode(&tx).to_vec()))
+                    .map(|signed_tx| BytesJson::from(rlp::encode(&signed_tx).to_vec()))
                     // Return an empty `tx_hex` if the transaction is still not appeared on the RPC node.
                     .unwrap_or_default();
                 Ok((tx_hash, tx_hex))
             },
+            EthPrivKeyPolicy::Iguana(_) 
+            | EthPrivKeyPolicy::HDWallet { .. } 
+            | EthPrivKeyPolicy::Trezor 
+                => MmError::err(WithdrawError::InternalError("invalid policy".to_owned())),
         }
     }
 
+
     async fn build(self) -> WithdrawResult {
         let coin = self.coin();
         let ticker = coin.deref().ticker.clone();
         let req = self.request().clone();
 
+        self.on_generating_transaction()?;
+
         let to_addr = coin
             .address_from_str(&req.to)
             .map_to_mm(WithdrawError::InvalidAddress)?;
@@ -216,39 +234,45 @@ where
             wei_amount -= total_fee;
         };
 
-        let _nonce_lock = coin.nonce_lock.lock().await;
-        let (nonce, _) = get_addr_nonce(my_address, coin.web3_instances.clone())
-            .compat()
-            .timeout_secs(30.)
-            .await?
-            .map_to_mm(WithdrawError::Transport)?;
-
-        let tx = UnSignedEthTx {
-            nonce,
-            value: eth_value,
-            action: Action::Call(call_addr),
-            data: data.clone(),
-            gas,
-            gas_price,
-        };
-
-        let tx_to_send = if cfg!(target_arch = "wasm32") {
-            Some(TransactionRequest {
-                from: my_address,
-                to: Some(to_addr),
-                gas: Some(gas),
-                gas_price: Some(gas_price),
-                value: Some(eth_value),
-                data: Some(data.into()),
-                nonce: None,
-                ..TransactionRequest::default()
-            })
-        } else {
-            None
+        let (tx_hash, tx_hex) = match coin.priv_key_policy {
+            EthPrivKeyPolicy::Iguana(_) 
+            | EthPrivKeyPolicy::HDWallet { .. } 
+            | EthPrivKeyPolicy::Trezor => {
+                // Todo: nonce_lock is still global for all addresses but this needs to be per address
+                let _nonce_lock = coin.nonce_lock.lock().await;
+                let (nonce, _) = get_addr_nonce(my_address, coin.web3_instances.clone())
+                    .compat()
+                    .timeout_secs(30.)
+                    .await?
+                    .map_to_mm(WithdrawError::Transport)?;
+
+                let unsigned_tx = UnSignedEthTx {
+                    nonce,
+                    value: eth_value,
+                    action: Action::Call(call_addr),
+                    data: data.clone(),
+                    gas,
+                    gas_price,
+                };
+                self.sign_withdraw_tx(&req, unsigned_tx).await?
+            },
+            #[cfg(target_arch = "wasm32")]
+            EthPrivKeyPolicy::Metamask(_) => {
+                let tx_to_send = TransactionRequest {
+                    from: my_address,
+                    to: Some(to_addr),
+                    gas: Some(gas),
+                    gas_price: Some(gas_price),
+                    value: Some(eth_value),
+                    data: Some(data.into()),
+                    nonce: None,
+                    ..TransactionRequest::default()
+                };
+                self.send_withdraw_tx(&req, tx_to_send).await?
+            },
         };
 
-        let (tx_hash, tx_hex) = self.sign_withdraw_tx(&req, tx, tx_to_send).await?;
-
+        self.on_finishing()?;
         let tx_hash_bytes = BytesJson::from(tx_hash.0.to_vec());
         let tx_hash_str = format!("{:02x}", tx_hash_bytes);
 

mm2src/coins/rpc_command/init_withdraw.rs

@@ -59,7 +59,7 @@ pub async fn withdraw_status(
         .or_mm_err(|| WithdrawStatusError::NoSuchTask(req.task_id))
 }
 
-#[derive(Clone, Serialize)]
+#[derive(Clone, Debug, Serialize)]
 pub enum WithdrawInProgressStatus {
     Preparing,
     GeneratingTransaction,

mm2src/coins/utxo.rs

@@ -1858,63 +1858,6 @@ fn parse_hex_encoded_u32(hex_encoded: &str) -> Result<u32, MmError<String>> {
     Ok(u32::from_be_bytes(be_bytes))
 }
 
-/*
-#[cfg(not(target_arch = "wasm32"))]
-pub mod for_tests {
-    use crate::rpc_command::init_withdraw::{init_withdraw, withdraw_status, WithdrawStatusRequest};
-    use crate::{TransactionDetails, WithdrawError, WithdrawFrom, WithdrawRequest};
-    use common::executor::Timer;
-    use common::{now_ms, wait_until_ms};
-    use mm2_core::mm_ctx::MmArc;
-    use mm2_err_handle::prelude::MmResult;
-    use mm2_number::BigDecimal;
-    use rpc_task::RpcTaskStatus;
-    use std::str::FromStr;
-
-    /// Helper to call init_withdraw and wait for completion
-    pub async fn test_withdraw_init_loop(
-        ctx: MmArc,
-        ticker: &str,
-        to: &str,
-        amount: &str,
-        from_derivation_path: &str,
-    ) -> MmResult<TransactionDetails, WithdrawError> {
-        let withdraw_req = WithdrawRequest {
-            amount: BigDecimal::from_str(amount).unwrap(),
-            from: Some(WithdrawFrom::DerivationPath {
-                derivation_path: from_derivation_path.to_owned(),
-            }),
-            to: to.to_owned(),
-            coin: ticker.to_owned(),
-            max: false,
-            fee: None,
-            memo: None,
-        };
-        let init = init_withdraw(ctx.clone(), withdraw_req).await.unwrap();
-        let timeout = wait_until_ms(150000);
-        loop {
-            if now_ms() > timeout {
-                panic!("{} init_withdraw timed out", ticker);
-            }
-            let status = withdraw_status(ctx.clone(), WithdrawStatusRequest {
-                task_id: init.task_id,
-                forget_if_finished: true,
-            })
-            .await;
-            if let Ok(status) = status {
-                match status {
-                    RpcTaskStatus::Ok(tx_details) => break Ok(tx_details),
-                    RpcTaskStatus::Error(e) => break Err(e),
-                    _ => Timer::sleep(1.).await,
-                }
-            } else {
-                panic!("{} could not get withdraw_status", ticker)
-            }
-        }
-    }
-}
-*/
-
 #[test]
 fn test_parse_hex_encoded_u32() {
     assert_eq!(parse_hex_encoded_u32("0x892f2085"), Ok(2301567109));

mm2src/crypto/src/hw_rpc_task.rs

@@ -18,7 +18,7 @@ pub type HwRpcTaskUserActionRequest = RpcTaskUserActionRequest<HwRpcTaskUserActi
 
 /// When it comes to interacting with a HW device, this is a common awaiting RPC status.
 /// The status says to the user that he should pass a Trezor PIN to continue the pending RPC task.
-#[derive(Clone, Serialize, Deserialize)]
+#[derive(Clone, Debug, Serialize, Deserialize)]
 pub enum HwRpcTaskAwaitingStatus {
     EnterTrezorPin,
     EnterTrezorPassphrase,