From baa2a0cb645d18e17af3a35566a0a94aa0310790 Mon Sep 17 00:00:00 2001 From: shamardy Date: Fri, 25 Apr 2025 02:14:30 +0300 Subject: [PATCH 1/3] nft/evm: add per-HD-address NFT map field to EthCoinImpl (preparation for multi-address NFT support) --- mm2src/coins/eth.rs | 10 ++++++++++ mm2src/coins/eth/for_tests.rs | 1 + mm2src/coins/eth/v2_activation.rs | 3 +++ 3 files changed, 14 insertions(+) diff --git a/mm2src/coins/eth.rs b/mm2src/coins/eth.rs index 7a82841687..87b5e9cc55 100644 --- a/mm2src/coins/eth.rs +++ b/mm2src/coins/eth.rs @@ -852,6 +852,14 @@ pub struct EthCoinImpl { /// 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 NFT info *per HD wallet address* (for HD mode), mapping address to NFT key map. + /// + // Todo: Remove `nfts_infos` in favour of `nfts_by_address` + /// - For single-address coins, this will be empty for now. + /// - For HD coins, this maps each derived address to its NFT info map. + /// + /// This allows per-address NFT operations and tracking. + 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 @@ -6499,6 +6507,7 @@ pub async fn eth_coin_from_conf_and_request( address_nonce_locks, erc20_tokens_infos: Default::default(), nfts_infos: Default::default(), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system, @@ -7321,6 +7330,7 @@ impl EthCoin { 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: Default::default(), 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 781748832c..0db8d37ad8 100644 --- a/mm2src/coins/eth/for_tests.rs +++ b/mm2src/coins/eth/for_tests.rs @@ -76,6 +76,7 @@ pub(crate) fn eth_coin_from_keypair( 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 4e04bfebed..35c5d2ea9e 100644 --- a/mm2src/coins/eth/v2_activation.rs +++ b/mm2src/coins/eth/v2_activation.rs @@ -473,6 +473,7 @@ impl EthCoin { 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, @@ -560,6 +561,7 @@ impl EthCoin { address_nonce_locks: self.address_nonce_locks.clone(), erc20_tokens_infos: Default::default(), nfts_infos: Arc::new(AsyncMutex::new(nft_infos)), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system, @@ -694,6 +696,7 @@ pub async fn eth_coin_from_conf_and_request_v2( address_nonce_locks, erc20_tokens_infos: Default::default(), nfts_infos: Default::default(), + nfts_by_address: Default::default(), gas_limit, gas_limit_v2, abortable_system, From 1076d8c5321e2e2d41891614c68d44ca0b3a2ff5 Mon Sep 17 00:00:00 2001 From: shamardy Date: Fri, 25 Apr 2025 04:25:55 +0300 Subject: [PATCH 2/3] nft/eth: Remove nfts_infos field, unify and document per-address NFT map (nfts_by_address) --- mm2src/coins/eth.rs | 29 ++++++----- mm2src/coins/eth/for_tests.rs | 1 - mm2src/coins/eth/v2_activation.rs | 9 ++-- mm2src/coins/nft.rs | 51 ++++++++++++------- .../src/erc20_token_activation.rs | 10 ++-- .../src/eth_with_token_activation.rs | 25 ++++++--- .../tests/docker_tests/eth_docker_tests.rs | 23 ++++++--- 7 files changed, 95 insertions(+), 53 deletions(-) diff --git a/mm2src/coins/eth.rs b/mm2src/coins/eth.rs index 87b5e9cc55..c2cde6a1e6 100644 --- a/mm2src/coins/eth.rs +++ b/mm2src/coins/eth.rs @@ -848,17 +848,14 @@ 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 NFT info *per HD wallet address* (for HD mode), mapping address to NFT key map. + /// Stores information about NFTs owned by the user, per wallet address. /// - // Todo: Remove `nfts_infos` in favour of `nfts_by_address` - /// - For single-address coins, this will be empty for now. - /// - For HD coins, this maps each derived address to its NFT info map. + /// Each outer key is a wallet address (either the single address for non-HD wallets, or each derived address for HD wallets). /// - /// This allows per-address NFT operations and tracking. + /// 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, @@ -4525,6 +4522,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 { .. } => { @@ -6506,7 +6513,6 @@ 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, @@ -7329,8 +7335,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: Default::default(), + 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 0db8d37ad8..eb5a71301f 100644 --- a/mm2src/coins/eth/for_tests.rs +++ b/mm2src/coins/eth/for_tests.rs @@ -75,7 +75,6 @@ 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, diff --git a/mm2src/coins/eth/v2_activation.rs b/mm2src/coins/eth/v2_activation.rs index 35c5d2ea9e..19eb68b1eb 100644 --- a/mm2src/coins/eth/v2_activation.rs +++ b/mm2src/coins/eth/v2_activation.rs @@ -472,7 +472,6 @@ 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, @@ -529,6 +528,10 @@ impl EthCoin { }; let nft_infos = get_nfts_for_activation(&chain, &my_address, original_url, proxy_sign).await?; + 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(), }; @@ -560,8 +563,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: Default::default(), + nfts_by_address: Arc::new(AsyncMutex::new(nfts_by_address)), gas_limit, gas_limit_v2, abortable_system, @@ -695,7 +697,6 @@ 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, diff --git a/mm2src/coins/nft.rs b/mm2src/coins/nft.rs index ebd4c83146..6c5b077ed7 100644 --- a/mm2src/coins/nft.rs +++ b/mm2src/coins/nft.rs @@ -306,7 +306,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?; update_phishing(&storage, chain, &req.url_antispam).await?; @@ -314,11 +315,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, { @@ -331,26 +338,30 @@ 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?; - 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| { @@ -366,9 +377,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 c73672d9e8..65d1b7478a 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, } @@ -190,9 +194,9 @@ impl TokenActivationOps for EthCoin { platform_coin.initialize_global_nft(url, *komodo_proxy).await? }, }; - 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 d8c4a0f49e..210640621a 100644 --- a/mm2src/coins_activation/src/eth_with_token_activation.rs +++ b/mm2src/coins_activation/src/eth_with_token_activation.rs @@ -203,7 +203,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. @@ -212,8 +217,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). @@ -336,8 +345,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() }; @@ -369,7 +378,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, }, )); } @@ -394,7 +403,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, }, )) }, @@ -431,7 +440,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 840e30279a..bae3609683 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; @@ -256,9 +257,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), @@ -268,13 +271,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), @@ -284,7 +292,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 From e33800d198e5bb8ff68d5d1ac8f2b2b2f8d3cfd7 Mon Sep 17 00:00:00 2001 From: shamardy Date: Fri, 25 Apr 2025 05:04:21 +0300 Subject: [PATCH 3/3] test: update NFT field names in test structs to nfts_by_address (fixes docker tests) --- mm2src/mm2_test_helpers/src/structs.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mm2src/mm2_test_helpers/src/structs.rs b/mm2src/mm2_test_helpers/src/structs.rs index b5d3be28f5..b5c8f4b26e 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)]