diff --git a/mm2src/coins/coin_balance.rs b/mm2src/coins/coin_balance.rs index 5c5476f58a..5630c0a575 100644 --- a/mm2src/coins/coin_balance.rs +++ b/mm2src/coins/coin_balance.rs @@ -483,59 +483,6 @@ pub mod common_impl { accounts: Vec::with_capacity(accounts.len() + 1), }; - if accounts.get(&path_to_address.account_id).is_none() { - // Is seems that we couldn't find any HD account from the HD wallet storage. - drop(accounts); - info!( - "{} HD wallet hasn't been enabled before. Create default HD account", - coin.ticker() - ); - - // Create new HD account. - let mut new_account = create_new_account(coin, hd_wallet, xpub_extractor, Some(path_to_address.account_id)) - .await - .map_mm_err()?; - let scan_new_addresses = matches!( - params.scan_policy, - EnableCoinScanPolicy::ScanIfNewWallet | EnableCoinScanPolicy::Scan - ); - - let account_balance = enable_hd_account( - coin, - hd_wallet, - &mut new_account, - path_to_address.chain, - &address_scanner, - scan_new_addresses, - params.min_addresses_number.max(Some(path_to_address.address_id + 1)), - ) - .await?; - drop(new_account); - - if coin.is_trezor() { - let enabled_address = - hd_wallet - .get_enabled_address() - .await - .ok_or(EnableCoinBalanceError::NewAddressDerivingError( - NewAddressDerivingError::Internal( - "Couldn't find enabled address after it has already been enabled".to_string(), - ), - ))?; - coin.received_enabled_address_from_hw_wallet(enabled_address) - .await - .map_err(|e| { - EnableCoinBalanceError::NewAddressDerivingError(NewAddressDerivingError::Internal(format!( - "Coin rejected the enabled address derived from the hardware wallet: {}", - e - ))) - })?; - } - // Todo: The enabled address should be indicated in the response. - result.accounts.push(account_balance); - return Ok(result); - } - debug!( "{} HD accounts were found on {} coin activation", accounts.len(), @@ -563,8 +510,41 @@ pub mod common_impl { .await?; result.accounts.push(account_balance); } + let account_of_enabled_address_exists = accounts.contains_key(&path_to_address.account_id); drop(accounts); + // Make sure that the account of the enabled address is available, or create it otherwise. + if !account_of_enabled_address_exists { + // Is seems that we couldn't find the account for the enabled address in the HD wallet storage. + info!( + "account={} hasn't been enabled before, enabling it now for coin={}", + path_to_address.account_id, + coin.ticker() + ); + + // Create new HD account. + let mut new_account = create_new_account(coin, hd_wallet, xpub_extractor, Some(path_to_address.account_id)) + .await + .map_mm_err()?; + let scan_new_addresses = matches!( + params.scan_policy, + EnableCoinScanPolicy::ScanIfNewWallet | EnableCoinScanPolicy::Scan + ); + + let account_balance = enable_hd_account( + coin, + hd_wallet, + &mut new_account, + path_to_address.chain, + &address_scanner, + scan_new_addresses, + params.min_addresses_number.max(Some(path_to_address.address_id + 1)), + ) + .await?; + + result.accounts.push(account_balance); + } + if coin.is_trezor() { let enabled_address = hd_wallet @@ -585,6 +565,7 @@ pub mod common_impl { })?; } + // Todo: The enabled address should be indicated in the response. Ok(result) } diff --git a/mm2src/coins/eth/v2_activation.rs b/mm2src/coins/eth/v2_activation.rs index 6bb4683036..7dcf2294da 100644 --- a/mm2src/coins/eth/v2_activation.rs +++ b/mm2src/coins/eth/v2_activation.rs @@ -767,7 +767,7 @@ pub(crate) async fn build_address_and_priv_key_policy( let hd_wallet_storage = HDWalletCoinStorage::init_with_rmd160(ctx, ticker.to_string(), hd_wallet_rmd160) .await .mm_err(EthActivationV2Error::from)?; - let accounts = load_hd_accounts_from_storage(&hd_wallet_storage, &path_to_coin) + let accounts = load_hd_accounts_from_storage(&hd_wallet_storage, &path_to_coin, &global_hd_ctx) .await .map_mm_err()?; let gap_limit = gap_limit.unwrap_or(DEFAULT_GAP_LIMIT); @@ -805,15 +805,12 @@ pub(crate) async fn build_address_and_priv_key_policy( let hd_wallet_storage = HDWalletCoinStorage::init_with_rmd160(ctx, ticker.to_string(), hd_wallet_rmd160) .await .mm_err(EthActivationV2Error::from)?; - let accounts = load_hd_accounts_from_storage(&hd_wallet_storage, &path_to_coin) - .await - .map_mm_err()?; let gap_limit = gap_limit.unwrap_or(DEFAULT_GAP_LIMIT); let hd_wallet = EthHDWallet { hd_wallet_rmd160, hd_wallet_storage, - derivation_path: path_to_coin.clone(), - accounts: HDAccountsMutex::new(accounts), + derivation_path: path_to_coin, + accounts: HDAccountsMutex::new(Default::default()), enabled_address: *path_to_address, gap_limit, }; diff --git a/mm2src/coins/hd_wallet/coin_ops.rs b/mm2src/coins/hd_wallet/coin_ops.rs index a28fea0672..797b3ce0d9 100644 --- a/mm2src/coins/hd_wallet/coin_ops.rs +++ b/mm2src/coins/hd_wallet/coin_ops.rs @@ -218,13 +218,14 @@ pub trait HDWalletCoinOps { if new_known_addresses_number >= max_addresses_number { return MmError::err(AccountUpdatingError::AddressLimitReached { max_addresses_number }); } + let account_xpub = hd_account.extended_pubkey().to_string(bip32::Prefix::XPUB); match chain { Bip44Chain::External => hd_wallet - .update_external_addresses_number(hd_account.account_id(), new_known_addresses_number) + .update_external_addresses_number(account_xpub, new_known_addresses_number) .await .map_mm_err()?, Bip44Chain::Internal => hd_wallet - .update_internal_addresses_number(hd_account.account_id(), new_known_addresses_number) + .update_internal_addresses_number(account_xpub, new_known_addresses_number) .await .map_mm_err()?, } diff --git a/mm2src/coins/hd_wallet/mod.rs b/mm2src/coins/hd_wallet/mod.rs index 6ed0d0a9a5..d3f5e55418 100644 --- a/mm2src/coins/hd_wallet/mod.rs +++ b/mm2src/coins/hd_wallet/mod.rs @@ -1,7 +1,8 @@ use async_trait::async_trait; -use common::log::warn; -use crypto::{Bip32DerPathOps, Bip32Error, Bip44Chain, ChildNumber, DerivationPath, HDPathToAccount, HDPathToCoin, - Secp256k1ExtendedPublicKey, StandardHDPath, StandardHDPathError}; +use bip32::ExtendedPublicKey; +use common::log::{error, warn, LogOnError}; +use crypto::{Bip32DerPathOps, Bip32Error, Bip44Chain, ChildNumber, DerivationPath, GlobalHDAccountArc, + HDPathToAccount, HDPathToCoin, Secp256k1ExtendedPublicKey, StandardHDPath, StandardHDPathError}; use futures::lock::{MappedMutexGuard as AsyncMappedMutexGuard, Mutex as AsyncMutex, MutexGuard as AsyncMutexGuard}; use mm2_err_handle::prelude::*; use primitives::hash::H160; @@ -242,25 +243,77 @@ where } } -pub async fn load_hd_accounts_from_storage( +// TODO: Ideally this should be general and not specific for secp256k1 keys only. i.e. we should have this function work with different types +// of extended public and private keys (like Solana's and Sia's ed25519 keys). +// This was generic in the past, but changed here: https://github.com/KomodoPlatform/komodo-defi-framework/pull/2482/ +pub async fn load_hd_accounts_from_storage( hd_wallet_storage: &HDWalletCoinStorage, derivation_path: &HDPathToCoin, -) -> HDWalletStorageResult>> + global_hd_ctx: &GlobalHDAccountArc, +) -> HDWalletStorageResult>>> where HDAddress: HDAddressOps + Send, - ExtendedPublicKey: ExtendedPublicKeyOps, - ::Err: Display, { + let coin_der_path = derivation_path.to_derivation_path(); let accounts = hd_wallet_storage.load_all_accounts().await?; - let res: HDWalletStorageResult>> = accounts - .iter() + + // Filter accounts that when derived from the current HD wallet, they yield the correct xpub. + // If they don't yield the correct xpub, this means that account belong to another HD wallet with + // different purpose'/coin_type' fields but same account_id' field. This happens when someone changes + // the purpose'/coin_type' fields in the coins cnofig without changing the ticker. + // TODO: This is a temporary solution that fixes the issue. But we might wanna re-design how accounts + // are loaded from the database (loading only the single account the user asked for and not all accounts). + let mut bad_accounts = Vec::new(); + let accounts = accounts.into_iter().filter(|account| { + let mut account_der_path = coin_der_path.clone(); + let child_number = match ChildNumber::new(account.account_id, true) { + Ok(num) => num, + Err(e) => { + error!( + "Failed to create ChildNumber for account_id={} found in storage: {}", + account.account_id, e + ); + return false; + }, + }; + account_der_path.push(child_number); + let derived_xpub = match global_hd_ctx.derive_secp256k1_xpub(&account_der_path) { + Ok(xpub) => xpub.to_string(bip32::Prefix::XPUB), + Err(e) => { + error!( + "Failed to derive xpub for account_id={} found in storage: {}", + account.account_id, e + ); + return false; + }, + }; + if derived_xpub != account.account_xpub { + error!( + "Account with id={} has an xpub={} that doesn't match the one derived from the current HD wallet. Skipping.", + account.account_id, account.account_xpub + ); + bad_accounts.push(account.account_xpub.clone()); + return false; + } + true + }); + + let res: HDWalletStorageResult<_> = accounts .map(|account_info| { - let account = HDAccount::try_from_storage_item(derivation_path, account_info)?; + let account = HDAccount::try_from_storage_item(derivation_path, &account_info)?; Ok((account.account_id, account)) }) .collect(); + match res { - Ok(accounts) => Ok(accounts), + Ok(accounts) => { + // If there are any bad accounts, delete them from the storage. + if !bad_accounts.is_empty() { + // Ignore and log errors that occur during the deletion of bad accounts. + hd_wallet_storage.delete_accounts(bad_accounts).await.error_log(); + } + Ok(accounts) + }, Err(e) if e.get_inner().is_deserializing_err() => { warn!("Error loading HD accounts from the storage: '{}'. Clear accounts", e); hd_wallet_storage.clear_accounts().await?; @@ -394,10 +447,19 @@ where { const INIT_ACCOUNT_ID: u32 = 0; let new_account_id = match account_id { + // TODO: We should never let the user choose to create a new account with arbitrary account_id. account_ids must be instantiated + // in order and to instantiate account x+1, account x must have been already instantiated and used (i.e. has tx history). + // ref. https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#account Some(account_id) => account_id, None => { let accounts = hd_wallet.get_accounts_mut().await; + // Get's the last account ID we have. let last_account_id = accounts.iter().last().map(|(account_id, _account)| *account_id); + // This selects an available account ID, between 0..=last_id, and if all IDs are taken already, we use `last_id + 1`. + // TODO: This is inherently wrong. If account 0 is used, and account last_id is also used, then there should not exist + // any accounts with ID such that "0 < ID < last_id" and they aren't used. + // It is safe to just use `last_id + 1` but we gotta make sure that `last_id` was used and has some tx history. + // ref. https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#account last_account_id.map_or(INIT_ACCOUNT_ID, |last_id| { (INIT_ACCOUNT_ID..=last_id) .find(|id| !accounts.contains_key(id)) diff --git a/mm2src/coins/hd_wallet/storage/mock_storage.rs b/mm2src/coins/hd_wallet/storage/mock_storage.rs index 8086e58be8..f3340640d5 100644 --- a/mm2src/coins/hd_wallet/storage/mock_storage.rs +++ b/mm2src/coins/hd_wallet/storage/mock_storage.rs @@ -1,5 +1,6 @@ use crate::hd_wallet::{HDAccountStorageItem, HDWalletId, HDWalletStorageInternalOps, HDWalletStorageResult}; use async_trait::async_trait; +use crypto::XPub; use mm2_core::mm_ctx::MmArc; #[cfg(test)] use mocktopus::macros::*; @@ -19,18 +20,10 @@ impl HDWalletStorageInternalOps for HDWalletMockStorage { unimplemented!() } - async fn load_account( - &self, - _wallet_id: HDWalletId, - _account_id: u32, - ) -> HDWalletStorageResult> { - unimplemented!() - } - async fn update_external_addresses_number( &self, _wallet_id: HDWalletId, - _account_id: u32, + _account_xpub: XPub, _new_external_addresses_number: u32, ) -> HDWalletStorageResult<()> { unimplemented!() @@ -39,7 +32,7 @@ impl HDWalletStorageInternalOps for HDWalletMockStorage { async fn update_internal_addresses_number( &self, _wallet_id: HDWalletId, - _account_id: u32, + _account_xpub: XPub, _new_internal_addresses_number: u32, ) -> HDWalletStorageResult<()> { unimplemented!() @@ -53,5 +46,9 @@ impl HDWalletStorageInternalOps for HDWalletMockStorage { unimplemented!() } + async fn delete_accounts(&self, _wallet_id: HDWalletId, _account_xpubs: Vec) -> HDWalletStorageResult<()> { + unimplemented!() + } + async fn clear_accounts(&self, _wallet_id: HDWalletId) -> HDWalletStorageResult<()> { unimplemented!() } } diff --git a/mm2src/coins/hd_wallet/storage/mod.rs b/mm2src/coins/hd_wallet/storage/mod.rs index c01712bfc9..146540c124 100644 --- a/mm2src/coins/hd_wallet/storage/mod.rs +++ b/mm2src/coins/hd_wallet/storage/mod.rs @@ -33,8 +33,8 @@ type HDWalletStorageBoxed = Box; pub enum HDWalletStorageError { #[display(fmt = "HD wallet not allowed")] HDWalletUnavailable, - #[display(fmt = "HD account '{:?}':{} not found", wallet_id, account_id)] - HDAccountNotFound { wallet_id: HDWalletId, account_id: u32 }, + #[display(fmt = "HD account '{:?}':{} not found", wallet_id, account_xpub)] + HDAccountNotFound { wallet_id: HDWalletId, account_xpub: XPub }, #[display(fmt = "Error saving changes in HD wallet storage: {}", _0)] ErrorSaving(String), #[display(fmt = "Error loading from HD wallet storage: {}", _0)] @@ -98,23 +98,17 @@ pub(crate) trait HDWalletStorageInternalOps { async fn load_accounts(&self, wallet_id: HDWalletId) -> HDWalletStorageResult>; - async fn load_account( - &self, - wallet_id: HDWalletId, - account_id: u32, - ) -> HDWalletStorageResult>; - async fn update_external_addresses_number( &self, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_external_addresses_number: u32, ) -> HDWalletStorageResult<()>; async fn update_internal_addresses_number( &self, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_internal_addresses_number: u32, ) -> HDWalletStorageResult<()>; @@ -124,6 +118,8 @@ pub(crate) trait HDWalletStorageInternalOps { account: HDAccountStorageItem, ) -> HDWalletStorageResult<()>; + async fn delete_accounts(&self, wallet_id: HDWalletId, accounts_xpubs: Vec) -> HDWalletStorageResult<()>; + async fn clear_accounts(&self, wallet_id: HDWalletId) -> HDWalletStorageResult<()>; } @@ -133,39 +129,27 @@ pub trait HDWalletStorageOps { /// Getter for the HD wallet storage. fn hd_wallet_storage(&self) -> &HDWalletCoinStorage; - /// Loads all accounts from the HD wallet storage. - async fn load_all_accounts(&self) -> HDWalletStorageResult> { - let storage = self.hd_wallet_storage(); - storage.load_all_accounts().await - } - - /// Loads a specific account from the HD wallet storage. - async fn load_account(&self, account_id: u32) -> HDWalletStorageResult> { - let storage = self.hd_wallet_storage(); - storage.load_account(account_id).await - } - /// Updates the number of external addresses for a specific account. async fn update_external_addresses_number( &self, - account_id: u32, + account_xpub: XPub, new_external_addresses_number: u32, ) -> HDWalletStorageResult<()> { let storage = self.hd_wallet_storage(); storage - .update_external_addresses_number(account_id, new_external_addresses_number) + .update_external_addresses_number(account_xpub, new_external_addresses_number) .await } /// Updates the number of internal addresses for a specific account. async fn update_internal_addresses_number( &self, - account_id: u32, + account_xpub: XPub, new_internal_addresses_number: u32, ) -> HDWalletStorageResult<()> { let storage = self.hd_wallet_storage(); storage - .update_internal_addresses_number(account_id, new_internal_addresses_number) + .update_internal_addresses_number(account_xpub, new_internal_addresses_number) .await } @@ -260,30 +244,25 @@ impl HDWalletCoinStorage { self.inner.load_accounts(wallet_id).await } - async fn load_account(&self, account_id: u32) -> HDWalletStorageResult> { - let wallet_id = self.wallet_id(); - self.inner.load_account(wallet_id, account_id).await - } - async fn update_external_addresses_number( &self, - account_id: u32, + account_xpub: XPub, new_external_addresses_number: u32, ) -> HDWalletStorageResult<()> { let wallet_id = self.wallet_id(); self.inner - .update_external_addresses_number(wallet_id, account_id, new_external_addresses_number) + .update_external_addresses_number(wallet_id, account_xpub, new_external_addresses_number) .await } async fn update_internal_addresses_number( &self, - account_id: u32, + account_xpub: XPub, new_internal_addresses_number: u32, ) -> HDWalletStorageResult<()> { let wallet_id = self.wallet_id(); self.inner - .update_internal_addresses_number(wallet_id, account_id, new_internal_addresses_number) + .update_internal_addresses_number(wallet_id, account_xpub, new_internal_addresses_number) .await } @@ -292,6 +271,11 @@ impl HDWalletCoinStorage { self.inner.upload_new_account(wallet_id, account_info).await } + pub async fn delete_accounts(&self, accounts_xpubs: Vec) -> HDWalletStorageResult<()> { + let wallet_id = self.wallet_id(); + self.inner.delete_accounts(wallet_id, accounts_xpubs).await + } + pub async fn clear_accounts(&self) -> HDWalletStorageResult<()> { let wallet_id = self.wallet_id(); self.inner.clear_accounts(wallet_id).await @@ -411,6 +395,80 @@ mod tests { } async fn test_delete_accounts_impl() { + let wallet0_account0 = HDAccountStorageItem { + account_id: 0, + account_xpub: "xpub6DEHSksajpRPM59RPw7Eg6PKdU7E2ehxJWtYdrfQ6JFmMGBsrR6jA78ANCLgzKYm4s5UqQ4ydLEYPbh3TRVvn5oAZVtWfi4qJLMntpZ8uGJ".to_owned(), + external_addresses_number: 1, + internal_addresses_number: 2, + }; + let wallet0_account1 = HDAccountStorageItem { + account_id: 1, + account_xpub: "xpub6DEHSksajpRPQq2FdGT6JoieiQZUpTZ3WZn8fcuLJhFVmtCpXbuXxp5aPzaokwcLV2V9LE55Dwt8JYkpuMv7jXKwmyD28WbHYjBH2zhbW2p".to_owned(), + external_addresses_number: 1, + internal_addresses_number: 2, + }; + let wallet0_account2 = HDAccountStorageItem { + account_id: 2, + account_xpub: "xpub6EuV33a2DXxAhoJTRTnr8qnysu81AA4YHpLY6o8NiGkEJ8KADJ35T64eJsStWsmRf1xXkEANVjXFXnaUKbRtFwuSPCLfDdZwYNZToh4LBCd".to_owned(), + external_addresses_number: 3, + internal_addresses_number: 4, + }; + let wallet1_account0 = HDAccountStorageItem { + account_id: 0, + account_xpub: "xpub6CUGRUonZSQ4TWtTMmzXdrXDtypWKiKrhko4egpiMZbpiaQL2jkwSB1icqYh2cfDfVxdx4df189oLKnC5fSwqPfgyP3hooxujYzAu3fDVmz".to_owned(), + external_addresses_number: 5, + internal_addresses_number: 6, + }; + + let ctx = mm_ctx_with_custom_db(); + let device0_rmd160 = H160::from("0000000000000000000000000000000000000010"); + let device1_rmd160 = H160::from("0000000000000000000000000000000000000020"); + + let wallet0_db = HDWalletCoinStorage::init_with_rmd160(&ctx, "RICK".to_owned(), device0_rmd160) + .await + .expect("!HDWalletCoinStorage::new"); + let wallet1_db = HDWalletCoinStorage::init_with_rmd160(&ctx, "RICK".to_owned(), device1_rmd160) + .await + .expect("!HDWalletCoinStorage::new"); + + wallet0_db + .upload_new_account(wallet0_account0.clone()) + .await + .expect("!HDWalletCoinStorage::upload_new_account: RICK wallet=0 account=0"); + wallet0_db + .upload_new_account(wallet0_account1.clone()) + .await + .expect("!HDWalletCoinStorage::upload_new_account: RICK wallet=0 account=1"); + wallet0_db + .upload_new_account(wallet0_account2.clone()) + .await + .expect("!HDWalletCoinStorage::upload_new_account: RICK wallet=0 account=2"); + wallet1_db + .upload_new_account(wallet1_account0.clone()) + .await + .expect("!HDWalletCoinStorage::upload_new_account: RICK wallet=1 account=0"); + + // Delete account 0 and 2 in wallet 0. + wallet0_db + .delete_accounts(vec![ + wallet0_account0.account_xpub.clone(), + wallet0_account2.account_xpub.clone(), + ]) + .await + .expect("HDWalletCoinStorage::delete_accounts: RICK wallet=0"); + + // All accounts must be in the only one database. + // Rows in the database must differ by only `coin`, `hd_wallet_rmd160` and `account_id` values. + let all_accounts: Vec<_> = get_all_storage_items(&ctx) + .await + .into_iter() + .sorted_by(|x, y| x.external_addresses_number.cmp(&y.external_addresses_number)) + .collect(); + // Only account 1 in wallet 0 and account 0 in wallet 1 should exist at this point. + assert_eq!(all_accounts, vec![wallet0_account1, wallet1_account0]); + } + + async fn test_clear_accounts_impl() { let wallet0_account0 = HDAccountStorageItem { account_id: 0, account_xpub: "xpub6DEHSksajpRPM59RPw7Eg6PKdU7E2ehxJWtYdrfQ6JFmMGBsrR6jA78ANCLgzKYm4s5UqQ4ydLEYPbh3TRVvn5oAZVtWfi4qJLMntpZ8uGJ".to_owned(), @@ -511,10 +569,10 @@ mod tests { .await .expect("!HDWalletCoinStorage::upload_new_account: RICK wallet=0 account=1"); - db.update_internal_addresses_number(0, 5) + db.update_internal_addresses_number(account0.account_xpub.clone(), 5) .await .expect("!HDWalletCoinStorage::update_internal_addresses_number"); - db.update_external_addresses_number(1, 10) + db.update_external_addresses_number(account1.account_xpub.clone(), 10) .await .expect("!HDWalletCoinStorage::update_external_addresses_number"); @@ -547,6 +605,14 @@ mod tests { #[test] fn test_delete_accounts() { block_on(test_delete_accounts_impl()) } + #[cfg(target_arch = "wasm32")] + #[wasm_bindgen_test] + async fn test_clear_accounts() { test_clear_accounts_impl().await } + + #[cfg(not(target_arch = "wasm32"))] + #[test] + fn test_clear_accounts() { block_on(test_clear_accounts_impl()) } + #[cfg(target_arch = "wasm32")] #[wasm_bindgen_test] async fn test_update_account() { test_update_account_impl().await } diff --git a/mm2src/coins/hd_wallet/storage/sqlite_storage.rs b/mm2src/coins/hd_wallet/storage/sqlite_storage.rs index f430eac042..d372c2c9a8 100644 --- a/mm2src/coins/hd_wallet/storage/sqlite_storage.rs +++ b/mm2src/coins/hd_wallet/storage/sqlite_storage.rs @@ -4,10 +4,10 @@ use crate::hd_wallet::{HDAccountStorageItem, HDWalletId, HDWalletStorageError, H HDWalletStorageResult}; use async_trait::async_trait; use common::async_blocking; +use crypto::XPub; use db_common::owned_named_params; -use db_common::sqlite::rusqlite::{Connection, Error as SqlError, Row}; -use db_common::sqlite::{query_single_row_with_named_params, AsSqlNamedParams, OwnedSqlNamedParams, SqliteConnShared, - SqliteConnWeak}; +use db_common::sqlite::rusqlite::{named_params, Connection, Error as SqlError, Row}; +use db_common::sqlite::{AsSqlNamedParams, OwnedSqlNamedParams, SqliteConnShared, SqliteConnWeak}; use derive_more::Display; use mm2_core::mm_ctx::MmArc; use mm2_err_handle::prelude::*; @@ -25,14 +25,14 @@ const CREATE_HD_ACCOUNT_TABLE: &str = "CREATE TABLE IF NOT EXISTS hd_account ( const INSERT_ACCOUNT: &str = "INSERT INTO hd_account (coin, hd_wallet_rmd160, account_id, account_xpub, external_addresses_number, internal_addresses_number) - VALUES (:coin, :hd_wallet_rmd160, :account_id, :account_xpub, :external_addresses_number, :internal_addresses_number);"; + SELECT :coin, :hd_wallet_rmd160, :account_id, :account_xpub, :external_addresses_number, :internal_addresses_number + WHERE NOT EXISTS (SELECT 1 FROM hd_account WHERE coin=:coin AND hd_wallet_rmd160=:hd_wallet_rmd160 AND account_xpub=:account_xpub);"; const DELETE_ACCOUNTS_BY_WALLET_ID: &str = "DELETE FROM hd_account WHERE coin=:coin AND hd_wallet_rmd160=:hd_wallet_rmd160;"; -const SELECT_ACCOUNT: &str = "SELECT account_id, account_xpub, external_addresses_number, internal_addresses_number - FROM hd_account - WHERE coin=:coin AND hd_wallet_rmd160=:hd_wallet_rmd160 AND account_id=:account_id;"; +const DELETE_ACCOUNT_BY_XPUB: &str = + "DELETE FROM hd_account WHERE coin=:coin AND hd_wallet_rmd160=:hd_wallet_rmd160 AND account_xpub=:account_xpub;"; const SELECT_ACCOUNTS_BY_WALLET_ID: &str = "SELECT account_id, account_xpub, external_addresses_number, internal_addresses_number @@ -130,38 +130,16 @@ impl HDWalletStorageInternalOps for HDWalletSqliteStorage { .await } - async fn load_account( - &self, - wallet_id: HDWalletId, - account_id: u32, - ) -> HDWalletStorageResult> { - let selfi = self.clone(); - async_blocking(move || { - let conn_shared = selfi.get_shared_conn()?; - let conn = Self::lock_conn_mutex(&conn_shared)?; - - let mut params = wallet_id.to_sql_params(); - params.extend(owned_named_params! { - ":account_id": account_id, - }); - query_single_row_with_named_params(&conn, SELECT_ACCOUNT, ¶ms.as_sql_named_params(), |row: &Row<'_>| { - HDAccountStorageItem::try_from(row) - }) - .map_to_mm(HDWalletStorageError::from) - }) - .await - } - async fn update_external_addresses_number( &self, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_external_addresses_number: u32, ) -> HDWalletStorageResult<()> { self.update_addresses_number( UpdatingProperty::ExternalAddressesNumber, wallet_id, - account_id, + account_xpub, new_external_addresses_number, ) .await @@ -170,13 +148,13 @@ impl HDWalletStorageInternalOps for HDWalletSqliteStorage { async fn update_internal_addresses_number( &self, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_internal_addresses_number: u32, ) -> HDWalletStorageResult<()> { self.update_addresses_number( UpdatingProperty::InternalAddressesNumber, wallet_id, - account_id, + account_xpub, new_internal_addresses_number, ) .await @@ -200,6 +178,31 @@ impl HDWalletStorageInternalOps for HDWalletSqliteStorage { .await } + async fn delete_accounts(&self, wallet_id: HDWalletId, account_xpubs: Vec) -> HDWalletStorageResult<()> { + let selfi = self.clone(); + async_blocking(move || { + let conn_shared = selfi.get_shared_conn()?; + let mut conn = Self::lock_conn_mutex(&conn_shared)?; + let tx = conn + .transaction() + .map_to_mm(|e| HDWalletStorageError::Internal(format!("Error starting transaction: {}", e)))?; + + for account_xpub in account_xpubs { + let params = named_params! { + ":coin": wallet_id.coin.clone(), + ":hd_wallet_rmd160": wallet_id.hd_wallet_rmd160.clone(), + ":account_xpub": account_xpub, + }; + tx.execute(DELETE_ACCOUNT_BY_XPUB, params) + .map_to_mm(HDWalletStorageError::from)?; + } + tx.commit() + .map_to_mm(|e| HDWalletStorageError::Internal(format!("Error committing transaction: {}", e)))?; + Ok(()) + }) + .await + } + async fn clear_accounts(&self, wallet_id: HDWalletId) -> HDWalletStorageResult<()> { let selfi = self.clone(); async_blocking(move || { @@ -239,11 +242,11 @@ impl HDWalletSqliteStorage { &self, updating_property: UpdatingProperty, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_addresses_number: u32, ) -> HDWalletStorageResult<()> { let sql = format!( - "UPDATE hd_account SET {updating_property}=:new_value WHERE coin=:coin AND hd_wallet_rmd160=:hd_wallet_rmd160 AND account_id=:account_id;", + "UPDATE hd_account SET {updating_property}=:new_value WHERE coin=:coin AND hd_wallet_rmd160=:hd_wallet_rmd160 AND account_xpub=:account_xpub;", ); let selfi = self.clone(); @@ -253,7 +256,7 @@ impl HDWalletSqliteStorage { let mut params = owned_named_params! { ":new_value": new_addresses_number, - ":account_id": account_id, + ":account_xpub": account_xpub, }; params.extend(wallet_id.to_sql_params()); diff --git a/mm2src/coins/hd_wallet/storage/wasm_storage.rs b/mm2src/coins/hd_wallet/storage/wasm_storage.rs index 5c7b57b77d..8a5bb0ebcb 100644 --- a/mm2src/coins/hd_wallet/storage/wasm_storage.rs +++ b/mm2src/coins/hd_wallet/storage/wasm_storage.rs @@ -4,13 +4,13 @@ use crate::CoinsContext; use async_trait::async_trait; use crypto::XPub; use mm2_core::mm_ctx::MmArc; -use mm2_db::indexed_db::cursor_prelude::*; +use mm2_db::indexed_db::{cursor_prelude::*, OnUpgradeError}; use mm2_db::indexed_db::{DbIdentifier, DbInstance, DbLocked, DbTable, DbTransactionError, DbUpgrader, IndexedDb, IndexedDbBuilder, InitDbError, InitDbResult, ItemId, MultiIndex, OnUpgradeResult, SharedDb, TableSignature, WeakDb}; use mm2_err_handle::prelude::*; -const DB_VERSION: u32 = 1; +const DB_VERSION: u32 = 2; /// An index of the `HDAccountTable` table that consists of the following properties: /// * coin - coin ticker /// * hd_wallet_rmd160 - RIPEMD160(SHA256(x)) where x is a pubkey extracted from a Hardware Wallet device or passphrase. @@ -18,8 +18,8 @@ const WALLET_ID_INDEX: &str = "wallet_id"; /// A **unique** index of the `HDAccountTable` table that consists of the following properties: /// * coin - coin ticker /// * hd_wallet_rmd160 - RIPEMD160(SHA256(x)) where x is a pubkey extracted from a Hardware Wallet device or passphrase. -/// * account_id - HD account id -const WALLET_ACCOUNT_ID_INDEX: &str = "wallet_account_id"; +/// * account_xpub - HD account xpub +const WALLET_ACCOUNT_XPUB_INDEX: &str = "wallet_account_xpub"; type HDWalletDbLocked<'a> = DbLocked<'a, HDWalletDb>; @@ -96,15 +96,39 @@ struct HDAccountTable { impl TableSignature for HDAccountTable { const TABLE_NAME: &'static str = "hd_account"; - fn on_upgrade_needed(upgrader: &DbUpgrader, old_version: u32, new_version: u32) -> OnUpgradeResult<()> { - if let (0, 1) = (old_version, new_version) { - let table = upgrader.create_table(Self::TABLE_NAME)?; - table.create_multi_index(WALLET_ID_INDEX, &["coin", "hd_wallet_rmd160"], false)?; - table.create_multi_index( - WALLET_ACCOUNT_ID_INDEX, - &["coin", "hd_wallet_rmd160", "account_id"], - true, - )?; + fn on_upgrade_needed(upgrader: &DbUpgrader, mut current_version: u32, new_version: u32) -> OnUpgradeResult<()> { + /// A deprecated index that is now replaced by `WALLET_ACCOUNT_XPUB_INDEX`. + const DEPRECATED_WALLET_ACCOUNT_ID_INDEX: &str = "wallet_account_id"; + + while current_version < new_version { + match current_version { + 0 => { + let table = upgrader.create_table(Self::TABLE_NAME)?; + table.create_multi_index(WALLET_ID_INDEX, &["coin", "hd_wallet_rmd160"], false)?; + table.create_multi_index( + DEPRECATED_WALLET_ACCOUNT_ID_INDEX, + &["coin", "hd_wallet_rmd160", "account_id"], + true, + )?; + }, + 1 => { + let table = upgrader.open_table(Self::TABLE_NAME)?; + table.create_multi_index( + WALLET_ACCOUNT_XPUB_INDEX, + &["coin", "hd_wallet_rmd160", "account_xpub"], + true, + )?; + table.delete_index(DEPRECATED_WALLET_ACCOUNT_ID_INDEX)?; + }, + unsupported_version => { + return MmError::err(OnUpgradeError::UnsupportedVersion { + unsupported_version, + old_version: current_version, + new_version, + }); + }, + }; + current_version += 1; } Ok(()) @@ -190,31 +214,13 @@ impl HDWalletStorageInternalOps for HDWalletIndexedDbStorage { .collect()) } - async fn load_account( - &self, - wallet_id: HDWalletId, - account_id: u32, - ) -> HDWalletStorageResult> { - let shared_db = self.get_shared_db()?; - let locked_db = Self::lock_db_mutex(&shared_db).await?; - - let transaction = locked_db.inner.transaction().await.map_mm_err()?; - let table = transaction.table::().await.map_mm_err()?; - - let maybe_account = Self::find_account(&table, wallet_id, account_id).await?; - match maybe_account { - Some((_account_item_id, account_item)) => Ok(Some(HDAccountStorageItem::from(account_item))), - None => Ok(None), - } - } - async fn update_external_addresses_number( &self, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_external_addresses_number: u32, ) -> HDWalletStorageResult<()> { - self.update_account(wallet_id, account_id, |account| { + self.update_account(wallet_id, account_xpub, |account| { account.external_addresses_number = new_external_addresses_number; }) .await @@ -223,10 +229,10 @@ impl HDWalletStorageInternalOps for HDWalletIndexedDbStorage { async fn update_internal_addresses_number( &self, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, new_internal_addresses_number: u32, ) -> HDWalletStorageResult<()> { - self.update_account(wallet_id, account_id, |account| { + self.update_account(wallet_id, account_xpub, |account| { account.internal_addresses_number = new_internal_addresses_number; }) .await @@ -251,6 +257,26 @@ impl HDWalletStorageInternalOps for HDWalletIndexedDbStorage { .mm_err(HDWalletStorageError::from) } + async fn delete_accounts(&self, wallet_id: HDWalletId, account_xpubs: Vec) -> HDWalletStorageResult<()> { + let shared_db = self.get_shared_db()?; + let locked_db = Self::lock_db_mutex(&shared_db).await?; + + let transaction = locked_db.inner.transaction().await.map_mm_err()?; + let table = transaction.table::().await.map_mm_err()?; + + for account_xpub in account_xpubs { + let index_keys = MultiIndex::new(WALLET_ACCOUNT_XPUB_INDEX) + .with_value(wallet_id.coin.clone()) + .map_mm_err()? + .with_value(wallet_id.hd_wallet_rmd160.clone()) + .map_mm_err()? + .with_value(account_xpub) + .map_mm_err()?; + table.delete_item_by_unique_multi_index(index_keys).await.map_mm_err()?; + } + Ok(()) + } + async fn clear_accounts(&self, wallet_id: HDWalletId) -> HDWalletStorageResult<()> { let shared_db = self.get_shared_db()?; let locked_db = Self::lock_db_mutex(&shared_db).await?; @@ -282,14 +308,14 @@ impl HDWalletIndexedDbStorage { async fn find_account( table: &DbTable<'_, HDAccountTable>, wallet_id: HDWalletId, - account_id: u32, + account_xpub: XPub, ) -> HDWalletStorageResult> { - let index_keys = MultiIndex::new(WALLET_ACCOUNT_ID_INDEX) + let index_keys = MultiIndex::new(WALLET_ACCOUNT_XPUB_INDEX) .with_value(wallet_id.coin) .map_mm_err()? .with_value(wallet_id.hd_wallet_rmd160) .map_mm_err()? - .with_value(account_id) + .with_value(account_xpub) .map_mm_err()?; table .get_item_by_unique_multi_index(index_keys) @@ -297,7 +323,7 @@ impl HDWalletIndexedDbStorage { .mm_err(HDWalletStorageError::from) } - async fn update_account(&self, wallet_id: HDWalletId, account_id: u32, f: F) -> HDWalletStorageResult<()> + async fn update_account(&self, wallet_id: HDWalletId, account_xpub: XPub, f: F) -> HDWalletStorageResult<()> where F: FnOnce(&mut HDAccountTable), { @@ -307,9 +333,12 @@ impl HDWalletIndexedDbStorage { let transaction = locked_db.inner.transaction().await.map_mm_err()?; let table = transaction.table::().await.map_mm_err()?; - let (account_item_id, mut account) = Self::find_account(&table, wallet_id.clone(), account_id) + let (account_item_id, mut account) = Self::find_account(&table, wallet_id.clone(), account_xpub.clone()) .await? - .or_mm_err(|| HDWalletStorageError::HDAccountNotFound { wallet_id, account_id })?; + .or_mm_err(|| HDWalletStorageError::HDAccountNotFound { + wallet_id, + account_xpub, + })?; // Apply `f` to `account` and upload the changes to the storage. f(&mut account); diff --git a/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs b/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs index 76491bb172..809f027cfa 100644 --- a/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs +++ b/mm2src/coins/utxo/utxo_builder/utxo_coin_builder.rs @@ -209,7 +209,7 @@ pub trait UtxoFieldsWithGlobalHDBuilder: UtxoCoinBuilderCommonOps { HDWalletCoinStorage::init_with_rmd160(self.ctx(), self.ticker().to_owned(), hd_wallet_rmd160) .await .map_mm_err()?; - let accounts = load_hd_accounts_from_storage(&hd_wallet_storage, path_to_coin) + let accounts = load_hd_accounts_from_storage(&hd_wallet_storage, path_to_coin, &global_hd_ctx) .await .mm_err(UtxoCoinBuildError::from)?; let gap_limit = self.gap_limit(); @@ -314,23 +314,20 @@ pub trait UtxoFieldsWithHardwareWalletBuilder: UtxoCoinBuilderCommonOps { let hd_wallet_storage = HDWalletCoinStorage::init(self.ctx(), ticker).await.map_mm_err()?; - let accounts = load_hd_accounts_from_storage(&hd_wallet_storage, &path_to_coin) - .await - .mm_err(UtxoCoinBuildError::from)?; let gap_limit = self.gap_limit(); let hd_wallet = UtxoHDWallet { inner: HDWallet { hd_wallet_rmd160, hd_wallet_storage, derivation_path: path_to_coin, - accounts: HDAccountsMutex::new(accounts), + accounts: HDAccountsMutex::new(Default::default()), enabled_address: self.activation_params().path_to_address, gap_limit, }, address_format, }; - // TODO: Creating a dummy output script for now. We better set it to the enabled address output script. + // Note that we set this temporarily to a dummy value. We later set it correctly to the actual enabled address once we receive it from the Trezor device. let recently_spent_outpoints = AsyncMutex::new(RecentlySpentOutPoints::new(Default::default())); // Create an abortable system linked to the `MmCtx` so if the context is stopped via `MmArc::stop`, diff --git a/mm2src/coins/utxo/utxo_tests.rs b/mm2src/coins/utxo/utxo_tests.rs index 8a30c42c58..0fa600c797 100644 --- a/mm2src/coins/utxo/utxo_tests.rs +++ b/mm2src/coins/utxo/utxo_tests.rs @@ -4496,21 +4496,21 @@ fn test_account_balance_rpc() { #[test] fn test_scan_for_new_addresses() { - static mut ACCOUNT_ID: u32 = 0; + static mut ACCOUNT_XPUB: &str = ""; static mut NEW_EXTERNAL_ADDRESSES_NUMBER: u32 = 0; static mut NEW_INTERNAL_ADDRESSES_NUMBER: u32 = 0; HDWalletMockStorage::update_external_addresses_number.mock_safe( - |_, _, account_id, new_external_addresses_number| { - assert_eq!(account_id, unsafe { ACCOUNT_ID }); + |_, _, account_xpub, new_external_addresses_number| { + assert_eq!(account_xpub, unsafe { ACCOUNT_XPUB }); assert_eq!(new_external_addresses_number, unsafe { NEW_EXTERNAL_ADDRESSES_NUMBER }); MockResult::Return(Box::pin(futures::future::ok(()))) }, ); HDWalletMockStorage::update_internal_addresses_number.mock_safe( - |_, _, account_id, new_internal_addresses_number| { - assert_eq!(account_id, unsafe { ACCOUNT_ID }); + |_, _, account_xpub, new_internal_addresses_number| { + assert_eq!(account_xpub, unsafe { ACCOUNT_XPUB }); assert_eq!(new_internal_addresses_number, unsafe { NEW_INTERNAL_ADDRESSES_NUMBER }); MockResult::Return(Box::pin(futures::future::ok(()))) }, @@ -4652,7 +4652,7 @@ fn test_scan_for_new_addresses() { // Check balance of Account#0 unsafe { - ACCOUNT_ID = 0; + ACCOUNT_XPUB = "xpub6DEHSksajpRPM59RPw7Eg6PKdU7E2ehxJWtYdrfQ6JFmMGBsrR6jA78ANCLgzKYm4s5UqQ4ydLEYPbh3TRVvn5oAZVtWfi4qJLMntpZ8uGJ"; NEW_EXTERNAL_ADDRESSES_NUMBER = 4; NEW_INTERNAL_ADDRESSES_NUMBER = 4; } @@ -4677,7 +4677,7 @@ fn test_scan_for_new_addresses() { // Check balance of Account#1 unsafe { - ACCOUNT_ID = 1; + ACCOUNT_XPUB = "xpub6DEHSksajpRPQq2FdGT6JoieiQZUpTZ3WZn8fcuLJhFVmtCpXbuXxp5aPzaokwcLV2V9LE55Dwt8JYkpuMv7jXKwmyD28WbHYjBH2zhbW2p"; NEW_EXTERNAL_ADDRESSES_NUMBER = 5; NEW_INTERNAL_ADDRESSES_NUMBER = 2; } diff --git a/mm2src/crypto/src/global_hd_ctx.rs b/mm2src/crypto/src/global_hd_ctx.rs index 6496065ab4..d546f996ff 100644 --- a/mm2src/crypto/src/global_hd_ctx.rs +++ b/mm2src/crypto/src/global_hd_ctx.rs @@ -2,7 +2,7 @@ use crate::privkey::{bip39_seed_from_passphrase, key_pair_from_secret, PrivKeyError}; use crate::{mm2_internal_der_path, Bip32Error, CryptoInitError, CryptoInitResult}; -use bip32::{DerivationPath, ExtendedPrivateKey}; +use bip32::{DerivationPath, ExtendedPrivateKey, ExtendedPublicKey}; use common::drop_mutability; use keys::{KeyPair, Secret as Secp256k1Secret}; use mm2_err_handle::prelude::*; @@ -78,6 +78,16 @@ impl GlobalHDAccountCtx { pub fn derive_secp256k1_secret(&self, derivation_path: &DerivationPath) -> MmResult { derive_secp256k1_secret(self.bip39_secp_priv_key.clone(), derivation_path) } + + /// Derives an `ExtendedPublicKey` from [`HDAccountCtx::bip39_secp_priv_key`] + /// at the given `m/purpose'/coin_type'/account'` derivation path. + /// Remember, XPub are for accounts. + pub fn derive_secp256k1_xpub( + &self, + derivation_path: &DerivationPath, + ) -> MmResult, Bip32Error> { + derive_secp256k1_xpub_impl(self.bip39_secp_priv_key.clone(), derivation_path) + } } pub fn derive_secp256k1_secret( @@ -93,3 +103,16 @@ pub fn derive_secp256k1_secret( let secret = *priv_key.private_key().as_ref(); Ok(Secp256k1Secret::from(secret)) } + +fn derive_secp256k1_xpub_impl( + bip39_secp_priv_key: ExtendedPrivateKey, + derivation_path: &DerivationPath, +) -> MmResult, Bip32Error> { + let mut priv_key = bip39_secp_priv_key; + for child in derivation_path.iter() { + priv_key = priv_key.derive_child(child)?; + } + drop_mutability!(priv_key); + + Ok(priv_key.public_key()) +} diff --git a/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs b/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs index 439467c334..78cf772bf8 100644 --- a/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs +++ b/mm2src/mm2_main/tests/mm2_tests/mm2_tests_inner.rs @@ -5302,9 +5302,7 @@ fn test_sign_verify_message_segwit_with_bip84_derivation_path() { assert!(!verify_response.result.is_valid, "Cross-verification should fail"); } -///NOTE: Should not fail after [issue #2470](https://github.com/KomodoPlatform/komodo-defi-framework/issues/2470) is resolved. #[test] -#[ignore] #[cfg(not(target_arch = "wasm32"))] fn test_hd_address_conflict_across_derivation_paths() { const PASSPHRASE: &str = "tank abandon bind salon remove wisdom net size aspect direct source fossil"; @@ -5326,6 +5324,7 @@ fn test_hd_address_conflict_across_derivation_paths() { let mut conf = Mm2TestConf::seednode_with_hd_account(PASSPHRASE, &coins); let mm_hd = MarketMakerIt::start(conf.conf.clone(), conf.rpc_password.clone(), None).unwrap(); + log!("First kdf log path: {}", mm_hd.log_path.display()); let path_to_address = HDAccountAddressId { account_id: 0, @@ -5348,7 +5347,6 @@ fn test_hd_address_conflict_across_derivation_paths() { log!("Old address: {}", old_address); // Shutdown MM and restart RICK with derivation path m/84'/141' - log!("Conf log path: {}", mm_hd.log_path.display()); conf.conf["dbdir"] = mm_hd.folder.join("DB").to_str().unwrap().into(); block_on(mm_hd.stop()).unwrap(); @@ -5367,6 +5365,7 @@ fn test_hd_address_conflict_across_derivation_paths() { }); conf.conf["coins"] = json!([coin]); let mm_hd = MarketMakerIt::start(conf.conf, conf.rpc_password, None).unwrap(); + log!("Second kdf log path: {}", mm_hd.log_path.display()); // Re-enable RICK, but it will try to reuse address0 stored under old path(m/49'/141') let rick_2 = block_on(enable_utxo_v2_electrum( @@ -5384,17 +5383,11 @@ fn test_hd_address_conflict_across_derivation_paths() { log!("New address: {}", new_address); // KDF has a bug and reuses the same account (and thus the same address) for derivation paths that use different `m/purpose'/coin'` fields. - // This stems from the fact that KDF doesn't differentiate/store the "purpose" & "coin" derivation fields in the database, but it rather stores the whole xpub - // that repsresents `m/purpose'/coin'/account_id'` - // Now, when KDF queries the database for already stored accounts, it specifies the specifies `COIN=ticker` in the SQL query, and since - // we badly mutated the conf by changing the derivation path but not the coin ticker, it returns accounts belonging to the old coin ticker (old derivation path). - // This wouldn't have happened if we gave the conf with `m/84'/141'` ticker="RICK-segwit" and `m/49'/141'` ticker="RICK-legacy", but we don't do that. - assert_ne!( old_address, new_address, "Address from old derivation path(m/49'/141') should not match address from new derivation path(m/84'/141')"