diff --git a/mm2src/coins/eth.rs b/mm2src/coins/eth.rs index 47722ccab2..3ec8f38774 100644 --- a/mm2src/coins/eth.rs +++ b/mm2src/coins/eth.rs @@ -892,10 +892,15 @@ pub struct EthCoinImpl { /// are initiated concurrently from the same address. address_nonce_locks: Arc>>>>, erc20_tokens_infos: Arc>>, - /// Stores information about NFTs owned by the user. Each entry in the HashMap is uniquely identified by a composite key - /// consisting of the token address and token ID, separated by a comma. This field is essential for tracking the NFT assets - /// information (chain & contract type, amount etc.), where ownership and amount, in ERC1155 case, might change over time. - pub nfts_infos: Arc>>, + /// Stores information about NFTs owned by the user, per wallet address. + /// + /// Each outer key is a wallet address (either the single address for non-HD wallets, or each derived address for HD wallets). + /// + /// The value is a map from a composite NFT key (`"token_address,token_id"`) to `NftInfo`, + /// tracking all NFT assets (including contract type, chain, and amount; for ERC1155, amount may change over time). + /// + /// This structure enables tracking, associating, and operating on NFTs at the per-address level. + pub nfts_by_address: Arc>>>, /// Config provided gas limits for swap and send transactions pub(crate) gas_limit: EthGasLimit, /// Config provided gas limits v2 for swap v2 transactions @@ -4744,6 +4749,16 @@ impl EthCoin { self.get_token_balance_for_address(my_address, token_address).await } + /// Returns a map from address (as string) to the NFTs owned by that address, + /// suitable for API responses. + pub async fn nfts_by_display_address(&self) -> HashMap> { + let nfts_by_address = self.nfts_by_address.lock().await; + nfts_by_address + .iter() + .map(|(address, nfts)| (address.display_address(), nfts.clone())) + .collect() + } + async fn erc1155_balance(&self, token_addr: Address, token_id: &str) -> MmResult { let wallet_amount_uint = match self.coin_type { EthCoinType::Eth | EthCoinType::Nft { .. } => { @@ -6671,7 +6686,7 @@ pub async fn eth_coin_from_conf_and_request( logs_block_range: conf["logs_block_range"].as_u64().unwrap_or(DEFAULT_LOGS_BLOCK_RANGE), address_nonce_locks, erc20_tokens_infos: Default::default(), - nfts_infos: Default::default(), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system, @@ -7576,7 +7591,7 @@ impl EthCoin { logs_block_range: self.logs_block_range, address_nonce_locks: Arc::clone(&self.address_nonce_locks), erc20_tokens_infos: Arc::clone(&self.erc20_tokens_infos), - nfts_infos: Arc::clone(&self.nfts_infos), + nfts_by_address: Arc::clone(&self.nfts_by_address), gas_limit: EthGasLimit::default(), gas_limit_v2: EthGasLimitV2::default(), abortable_system: self.abortable_system.create_subsystem().unwrap(), diff --git a/mm2src/coins/eth/for_tests.rs b/mm2src/coins/eth/for_tests.rs index 2a830993f5..add1f9f654 100644 --- a/mm2src/coins/eth/for_tests.rs +++ b/mm2src/coins/eth/for_tests.rs @@ -75,7 +75,7 @@ pub(crate) fn eth_coin_from_keypair( address_nonce_locks: Arc::new(AsyncMutex::new(new_nonce_lock())), max_eth_tx_type: None, erc20_tokens_infos: Default::default(), - nfts_infos: Arc::new(Default::default()), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system: AbortableQueue::default(), diff --git a/mm2src/coins/eth/v2_activation.rs b/mm2src/coins/eth/v2_activation.rs index 6bb4683036..853767814b 100644 --- a/mm2src/coins/eth/v2_activation.rs +++ b/mm2src/coins/eth/v2_activation.rs @@ -491,7 +491,7 @@ impl EthCoin { logs_block_range: self.logs_block_range, address_nonce_locks: self.address_nonce_locks.clone(), erc20_tokens_infos: Default::default(), - nfts_infos: Default::default(), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system, @@ -549,6 +549,10 @@ impl EthCoin { let nft_infos = get_nfts_for_activation(&chain, &my_address, original_url, proxy_sign) .await .map_mm_err()?; + let mut nfts_by_address = HashMap::new(); + nfts_by_address.insert(my_address, nft_infos); + drop_mutability!(nfts_by_address); + let coin_type = EthCoinType::Nft { platform: self.ticker.clone(), }; @@ -580,7 +584,7 @@ impl EthCoin { logs_block_range: self.logs_block_range, address_nonce_locks: self.address_nonce_locks.clone(), erc20_tokens_infos: Default::default(), - nfts_infos: Arc::new(AsyncMutex::new(nft_infos)), + nfts_by_address: Arc::new(AsyncMutex::new(nfts_by_address)), gas_limit, gas_limit_v2, abortable_system, @@ -720,7 +724,7 @@ pub async fn eth_coin_from_conf_and_request_v2( logs_block_range: conf["logs_block_range"].as_u64().unwrap_or(DEFAULT_LOGS_BLOCK_RANGE), address_nonce_locks, erc20_tokens_infos: Default::default(), - nfts_infos: Default::default(), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system, diff --git a/mm2src/coins/nft.rs b/mm2src/coins/nft.rs index 9c7ad4fa09..6c293d0b19 100644 --- a/mm2src/coins/nft.rs +++ b/mm2src/coins/nft.rs @@ -332,7 +332,8 @@ pub async fn update_nft(ctx: MmArc, req: UpdateNftReq) -> MmResult<(), UpdateNft }); } update_nft_list(&storage, scanned_block + 1, &my_address_str, &wrapper).await?; - update_nft_global_in_coins_ctx(&ctx, &storage, *chain).await?; + // Todo: Add a parameter to UpdateNftReq to either fetch it for one specific address or all addresses in the HD wallet. + update_nft_global_in_coins_ctx(&ctx, &storage, *chain, my_address).await?; update_transfers_with_empty_meta(&storage, &wrapper).await?; update_spam(&storage, *chain, &req.url_antispam).await.map_mm_err()?; update_phishing(&storage, chain, &req.url_antispam).await.map_mm_err()?; @@ -340,11 +341,17 @@ pub async fn update_nft(ctx: MmArc, req: UpdateNftReq) -> MmResult<(), UpdateNft Ok(()) } -/// Updates the global NFT information in the coins context. +/// Updates the global NFT information for a specific address in the coins context. /// -/// This function uses the up-to-date NFT list for a given chain and updates the -/// corresponding global NFT information in the coins context. -async fn update_nft_global_in_coins_ctx(ctx: &MmArc, storage: &T, chain: Chain) -> MmResult<(), UpdateNftError> +/// This function uses the up-to-date NFT list for a given chain for a given address, +/// and updates the corresponding address's NFT information in the global NFT `nfts_by_address` map +/// within the coins context. +async fn update_nft_global_in_coins_ctx( + ctx: &MmArc, + storage: &T, + chain: Chain, + address: Address, +) -> MmResult<(), UpdateNftError> where T: NftListStorageOps + NftTransferHistoryStorageOps, { @@ -357,29 +364,33 @@ where .. }) = coins.get_mut(ticker) { + // Todo: We should be storing/fetching NFT lists per address, by having a separate table for each address. let nft_list = storage .get_nft_list(vec![chain], true, 1, None, None) .await .map_mm_err()?; - update_nft_infos(nft_global, nft_list.nfts).await; + update_nft_infos(nft_global, address, nft_list.nfts).await; return Ok(()); } - // if global NFT is None in CoinsContext, then it's just not activated + // If the global NFT is None in CoinsContext, then it's just not activated. Ok(()) } -/// Updates the global NFT information with the latest NFT list. +/// Updates the global NFT information with the latest NFT list for a specific address. /// -/// This function replaces the existing NFT information (`nfts_infos`) in the global NFT with the new data provided by `nft_list`. -/// The `nft_list` must be current, accurately reflecting the NFTs presently owned by the user. -/// This includes accounting for any changes such as NFTs that have been transferred away, so user is not owner anymore, -/// or changes in the amounts of ERC1155 tokens. -/// Ensuring the data's accuracy is vital for maintaining a correct representation of ownership in the global NFT. +/// This function replaces the existing NFT information (`nfts_infos`) for the given address in the +/// global NFT `nfts_by_address` map with the new data provided by `nft_list`. +/// The `nft_list` must be current, accurately reflecting the NFTs presently owned by that address. +/// This includes accounting for any changes such as NFTs that have been transferred away (so the +/// address is no longer the owner), or changes in the amounts of ERC1155 tokens. +/// Ensuring the data's accuracy is vital for maintaining a correct representation of ownership for +/// the address. /// /// # Warning -/// Using an outdated `nft_list` for this operation may result in incorrect NFT information persisting in the global NFT, -/// potentially leading to inconsistencies with the actual state of NFT ownership. -async fn update_nft_infos(nft_global: &mut EthCoin, nft_list: Vec) { +/// Using an outdated `nft_list` for this operation may result in incorrect NFT information +/// persisting for this address, potentially leading to inconsistencies with the actual state of NFT +/// ownership. +async fn update_nft_infos(nft_global: &mut EthCoin, address: Address, nft_list: Vec) { let new_nft_infos: HashMap = nft_list .into_iter() .map(|nft| { @@ -395,9 +406,11 @@ async fn update_nft_infos(nft_global: &mut EthCoin, nft_list: Vec) { }) .collect(); - let mut global_nft_infos = nft_global.nfts_infos.lock().await; - // we can do this as some `global_nft_infos` keys may not present in `new_nft_infos`, so we will have to remove them anyway - *global_nft_infos = new_nft_infos; + // Overwrite the NFT map for this address entirely. + // If the new map is missing NFTs (e.g. transferred out), those keys will be removed; + // this keeps the stored state accurate and prunes stale entries without needing a diff. + let mut nfts_by_address = nft_global.nfts_by_address.lock().await; + nfts_by_address.insert(address, new_nft_infos); } /// `update_spam` function updates spam contracts info in NFT list and NFT transfers. diff --git a/mm2src/coins_activation/src/erc20_token_activation.rs b/mm2src/coins_activation/src/erc20_token_activation.rs index e719ad6404..7c6a7a18a8 100644 --- a/mm2src/coins_activation/src/erc20_token_activation.rs +++ b/mm2src/coins_activation/src/erc20_token_activation.rs @@ -30,7 +30,11 @@ pub struct Erc20InitResult { #[derive(Debug, Serialize)] pub struct NftInitResult { - nfts: HashMap, + /// # API Breaking Change + /// `nfts` was changed from `HashMap` to + /// `HashMap>` to support HD wallets. + /// It was renamed to `nfts_by_address` to reflect the change. + nfts_by_address: HashMap>, platform_coin: String, } @@ -198,9 +202,9 @@ impl TokenActivationOps for EthCoin { .await .map_mm_err()?, }; - let nfts = nft_global.nfts_infos.lock().await.clone(); + let nfts_by_address = nft_global.nfts_by_display_address().await; let init_result = EthTokenInitResult::Nft(NftInitResult { - nfts, + nfts_by_address, platform_coin: platform_coin.ticker().to_owned(), }); Ok((nft_global, init_result)) diff --git a/mm2src/coins_activation/src/eth_with_token_activation.rs b/mm2src/coins_activation/src/eth_with_token_activation.rs index d3ac19b0cb..1309914141 100644 --- a/mm2src/coins_activation/src/eth_with_token_activation.rs +++ b/mm2src/coins_activation/src/eth_with_token_activation.rs @@ -210,7 +210,12 @@ pub struct IguanaEthWithTokensActivationResult { current_block: u64, eth_addresses_infos: HashMap>, erc20_addresses_infos: HashMap>, - nfts_infos: HashMap, + /// # API Breaking Change + /// `nfts_infos` was changed from `HashMap` to + /// `HashMap>` to support HD wallets. + /// It was renamed to `nfts_by_address` to reflect the change. + /// Todo: create NftBalances and use CoinAddressInfo instead of HashMap + nfts_by_address: HashMap>, } /// Activation result for activating an EVM-based coin along with its associated tokens (ERC20 and NFTs) for HD wallets. @@ -219,8 +224,12 @@ pub struct HDEthWithTokensActivationResult { current_block: u64, ticker: String, wallet_balance: CoinBalanceReport, - // Todo: Move to wallet_balance when implementing HDWallet for NFTs - nfts_infos: HashMap, + /// # API Breaking Change + /// `nfts_infos` was changed from `HashMap` to + /// `HashMap>` to support HD wallets. + /// It was renamed to `nfts_by_address` to reflect the change. + // Todo: Include in a struct similar to `CoinBalanceMap` instead but has NFT infos. + nfts_by_address: HashMap>, } /// Represents the result of activating an Ethereum-based coin along with its associated tokens (ERC20 and NFTs). @@ -345,8 +354,8 @@ impl PlatformCoinWithTokensActivationOps for EthCoin { .await .map_err(EthActivationV2Error::InternalError)?; - let nfts_map = if let Some(MmCoinEnum::EthCoin(nft_global)) = nft_global { - nft_global.nfts_infos.lock().await.clone() + let nfts_by_address = if let Some(MmCoinEnum::EthCoin(nft_global)) = nft_global { + nft_global.nfts_by_display_address().await } else { Default::default() }; @@ -378,7 +387,7 @@ impl PlatformCoinWithTokensActivationOps for EthCoin { current_block, eth_addresses_infos: HashMap::from([(my_address.display_address(), eth_address_info)]), erc20_addresses_infos: HashMap::from([(my_address.display_address(), erc20_address_info)]), - nfts_infos: nfts_map, + nfts_by_address, }, )); } @@ -403,7 +412,7 @@ impl PlatformCoinWithTokensActivationOps for EthCoin { current_block, eth_addresses_infos: HashMap::from([(my_address.display_address(), eth_address_info)]), erc20_addresses_infos: HashMap::from([(my_address.display_address(), erc20_address_info)]), - nfts_infos: nfts_map, + nfts_by_address, }, )) }, @@ -448,7 +457,7 @@ impl PlatformCoinWithTokensActivationOps for EthCoin { current_block, ticker: self.ticker().to_string(), wallet_balance, - nfts_infos: nfts_map, + nfts_by_address, })) }, } diff --git a/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs b/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs index 81fe6d4a10..1b31defc95 100644 --- a/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs +++ b/mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs @@ -41,6 +41,7 @@ use mm2_test_helpers::for_tests::{eth_sepolia_conf, sepolia_erc20_dev_conf}; use mm2_test_helpers::structs::{Bip44Chain, EnableCoinBalanceMap, EthWithTokensActivationResult, HDAccountAddressId, TokenInfo}; use serde_json::Value as Json; +use std::collections::HashMap; #[cfg(any(feature = "sepolia-maker-swap-v2-tests", feature = "sepolia-taker-swap-v2-tests"))] use std::str::FromStr; use std::thread; @@ -257,9 +258,11 @@ fn geth_erc1155_balance(wallet_addr: Address, token_id: U256) -> U256 { } pub(crate) async fn fill_erc1155_info(eth_coin: &EthCoin, token_address: Address, token_id: u32, amount: u32) { - let nft_infos_lock = eth_coin.nfts_infos.clone(); - let mut nft_infos = nft_infos_lock.lock().await; + let nfts_by_address_lock = eth_coin.nfts_by_address.clone(); + let mut nfts_by_address = nfts_by_address_lock.lock().await; + // Todo: support HD wallet for `fill_erc1155_info` + let my_address = eth_coin.derivation_method().single_addr_or_err().await.unwrap(); let erc1155_nft_info = NftInfo { token_address, token_id: BigUint::from(token_id), @@ -269,13 +272,18 @@ pub(crate) async fn fill_erc1155_info(eth_coin: &EthCoin, token_address: Address }; let erc1155_address_str = token_address.addr_to_string(); let erc1155_key = format!("{},{}", erc1155_address_str, token_id); - nft_infos.insert(erc1155_key, erc1155_nft_info); + nfts_by_address + .entry(my_address) + .or_insert_with(HashMap::new) + .insert(erc1155_key, erc1155_nft_info); } pub(crate) async fn fill_erc721_info(eth_coin: &EthCoin, token_address: Address, token_id: u32) { - let nft_infos_lock = eth_coin.nfts_infos.clone(); - let mut nft_infos = nft_infos_lock.lock().await; + let nfts_by_address_lock = eth_coin.nfts_by_address.clone(); + let mut nfts_by_address = nfts_by_address_lock.lock().await; + // Todo: support HD wallet for `fill_erc721_info` + let my_address = eth_coin.derivation_method().single_addr_or_err().await.unwrap(); let erc721_nft_info = NftInfo { token_address, token_id: BigUint::from(token_id), @@ -285,7 +293,10 @@ pub(crate) async fn fill_erc721_info(eth_coin: &EthCoin, token_address: Address, }; let erc721_address_str = token_address.addr_to_string(); let erc721_key = format!("{},{}", erc721_address_str, token_id); - nft_infos.insert(erc721_key, erc721_nft_info); + nfts_by_address + .entry(my_address) + .or_insert_with(HashMap::new) + .insert(erc721_key, erc721_nft_info); } /// Creates ETH protocol coin supplied with 100 ETH diff --git a/mm2src/mm2_test_helpers/src/structs.rs b/mm2src/mm2_test_helpers/src/structs.rs index fac660af8d..6af5a5abf9 100644 --- a/mm2src/mm2_test_helpers/src/structs.rs +++ b/mm2src/mm2_test_helpers/src/structs.rs @@ -902,7 +902,7 @@ pub struct IguanaEthWithTokensActivationResult { pub current_block: u64, pub eth_addresses_infos: HashMap>, pub erc20_addresses_infos: HashMap>, - pub nfts_infos: Json, + pub nfts_by_address: Json, } #[derive(Debug, Deserialize)] @@ -911,7 +911,7 @@ pub struct HDEthWithTokensActivationResult { pub current_block: u64, pub ticker: String, pub wallet_balance: EnableCoinBalanceMap, - pub nfts_infos: Json, + pub nfts_by_address: Json, } #[derive(Debug, Deserialize)]