From a2e5c8f894b0a5485f1c045072e125eb98a3bf96 Mon Sep 17 00:00:00 2001 From: /alex/ Date: Fri, 8 Dec 2023 11:46:06 +0100 Subject: [PATCH] Remove `address` field from `OutputData` struct (#1604) * finish impl * nit * some renaming * slight optimization * unwrap --------- Co-authored-by: DaughterOfMars Co-authored-by: Thibault Martinez --- .../consolidate_outputs.rs | 6 +- .../wallet/operations/output_consolidation.rs | 6 +- .../operations/syncing/addresses/outputs.rs | 28 ++-- sdk/src/wallet/operations/syncing/mod.rs | 155 +++++++++--------- sdk/src/wallet/operations/syncing/outputs.rs | 1 - sdk/src/wallet/types/mod.rs | 5 +- sdk/tests/wallet/events.rs | 1 - 7 files changed, 96 insertions(+), 106 deletions(-) diff --git a/sdk/examples/how_tos/accounts_and_addresses/consolidate_outputs.rs b/sdk/examples/how_tos/accounts_and_addresses/consolidate_outputs.rs index 6280965836..78db01e983 100644 --- a/sdk/examples/how_tos/accounts_and_addresses/consolidate_outputs.rs +++ b/sdk/examples/how_tos/accounts_and_addresses/consolidate_outputs.rs @@ -58,8 +58,7 @@ async fn main() -> Result<()> { .for_each(|(i, output_data)| { println!("OUTPUT #{i}"); println!( - "- address: {:?}\n- amount: {:?}\n- native tokens: {:?}", - output_data.address.clone().to_bech32_unchecked("rms"), + "- amount: {:?}\n- native tokens: {:?}", output_data.output.amount(), output_data.output.native_token() ) @@ -99,8 +98,7 @@ async fn main() -> Result<()> { .for_each(|(i, output_data)| { println!("OUTPUT #{i}"); println!( - "- address: {:?}\n- amount: {:?}\n- native tokens: {:?}", - output_data.address.clone().to_bech32_unchecked("rms"), + "- amount: {:?}\n- native tokens: {:?}", output_data.output.amount(), output_data.output.native_token() ) diff --git a/sdk/src/wallet/operations/output_consolidation.rs b/sdk/src/wallet/operations/output_consolidation.rs index 55ebe9370b..7fae58242d 100644 --- a/sdk/src/wallet/operations/output_consolidation.rs +++ b/sdk/src/wallet/operations/output_consolidation.rs @@ -136,7 +136,7 @@ where let mut outputs_to_consolidate = Vec::new(); let wallet_data = self.data().await; - let wallet_address = &wallet_data.address; + let wallet_address = wallet_data.address.clone(); for (output_id, output_data) in &wallet_data.unspent_outputs { #[cfg(feature = "participation")] @@ -149,7 +149,7 @@ where let is_locked_output = wallet_data.locked_outputs.contains(output_id); let should_consolidate_output = self - .should_consolidate_output(output_data, slot_index, wallet_address) + .should_consolidate_output(output_data, slot_index, &wallet_address) .await?; if !is_locked_output && should_consolidate_output { outputs_to_consolidate.push(output_data.clone()); @@ -256,7 +256,7 @@ where params .target_address .map(|bech32| bech32.into_inner()) - .unwrap_or_else(|| outputs_to_consolidate[0].address.clone()), + .unwrap_or_else(|| wallet_address.into_inner()), )) // TODO https://github.com/iotaledger/iota-sdk/issues/1632 // .with_native_tokens(total_native_tokens.finish()?) diff --git a/sdk/src/wallet/operations/syncing/addresses/outputs.rs b/sdk/src/wallet/operations/syncing/addresses/outputs.rs index fadc42acd4..ee4640127b 100644 --- a/sdk/src/wallet/operations/syncing/addresses/outputs.rs +++ b/sdk/src/wallet/operations/syncing/addresses/outputs.rs @@ -1,10 +1,13 @@ // Copyright 2022 IOTA Stiftung // SPDX-License-Identifier: Apache-2.0 +use std::collections::HashMap; + use instant::Instant; use crate::{ client::secret::SecretManage, + types::block::address::Address, wallet::{ constants::PARALLEL_REQUESTS_AMOUNT, task, @@ -22,12 +25,11 @@ where pub(crate) async fn get_outputs_from_address_output_ids( &self, addresses_with_unspent_outputs: Vec, - ) -> crate::wallet::Result<(Vec, Vec)> { + ) -> crate::wallet::Result<(Vec<(AddressWithUnspentOutputs, Vec)>)> { log::debug!("[SYNC] start get_outputs_from_address_output_ids"); let address_outputs_start_time = Instant::now(); let mut addresses_with_outputs = Vec::new(); - let mut outputs_data = Vec::new(); // We split the addresses into chunks so we don't get timeouts if we have thousands for addresses_chunk in &mut addresses_with_unspent_outputs @@ -35,31 +37,33 @@ where .map(|x: &[AddressWithUnspentOutputs]| x.to_vec()) { let mut tasks = Vec::new(); - for address in addresses_chunk { + for address_with_unspent_outputs in addresses_chunk { let wallet = self.clone(); tasks.push(async move { task::spawn(async move { - let output_responses = wallet.get_outputs(address.output_ids.clone()).await?; - - let outputs = wallet - .output_response_to_output_data(output_responses, &address) + let unspent_outputs_with_metadata = wallet + .get_outputs(address_with_unspent_outputs.output_ids.clone()) + .await?; + let unspent_outputs_data = wallet + .output_response_to_output_data( + unspent_outputs_with_metadata, + &address_with_unspent_outputs, + ) .await?; - crate::wallet::Result::Ok((address, outputs)) + crate::wallet::Result::Ok((address_with_unspent_outputs, unspent_outputs_data)) }) .await }); } let results = futures::future::try_join_all(tasks).await?; for res in results { - let (address, outputs): (AddressWithUnspentOutputs, Vec) = res?; - addresses_with_outputs.push(address); - outputs_data.extend(outputs); + addresses_with_outputs.push(res?); } } log::debug!( "[SYNC] finished get_outputs_from_address_output_ids in {:.2?}", address_outputs_start_time.elapsed() ); - Ok((addresses_with_outputs, outputs_data)) + Ok(addresses_with_outputs) } } diff --git a/sdk/src/wallet/operations/syncing/mod.rs b/sdk/src/wallet/operations/syncing/mod.rs index 464ce251ab..b4a0ca8379 100644 --- a/sdk/src/wallet/operations/syncing/mod.rs +++ b/sdk/src/wallet/operations/syncing/mod.rs @@ -112,11 +112,8 @@ where }, ]; - let (addresses_with_unspent_outputs, spent_or_not_synced_output_ids, outputs_data): ( - Vec, - Vec, - Vec, - ) = self.request_outputs_recursively(address_to_sync, options).await?; + let (_addresses_with_unspent_outputs, spent_or_not_synced_output_ids, outputs_data) = + self.request_outputs_recursively(address_to_sync, options).await?; // Request possible spent outputs log::debug!("[SYNC] spent_or_not_synced_outputs: {spent_or_not_synced_output_ids:?}"); @@ -171,102 +168,98 @@ where addresses_to_sync: Vec, options: &SyncOptions, ) -> crate::wallet::Result<(Vec, Vec, Vec)> { - // Cache the account and nft address with the related ed2559 address, so we can update the account address with - // the new output ids - - let mut new_account_and_nft_addresses: HashMap = HashMap::new(); - let mut spent_or_not_synced_output_ids = Vec::new(); - let mut addresses_with_unspent_outputs = Vec::new(); - let mut outputs_data = Vec::new(); + // Cache account and nft addresses with the related Ed25519 address, so we can update the account + // address with the new output ids. + let mut addresses_to_scan: HashMap = HashMap::new(); + let mut addresses_with_unspent_output_ids_all = Vec::new(); + let mut unspent_outputs_data_all = Vec::new(); let bech32_hrp = self.client().get_bech32_hrp().await?; - loop { - let new_outputs_data = if new_account_and_nft_addresses.is_empty() { - // Get outputs for the addresses and add them also the the addresses_with_unspent_outputs - let (unspent_output_ids, spent_or_not_synced_output_ids_inner) = self - .get_output_ids_for_addresses(addresses_to_sync.clone(), options) - .await?; - - spent_or_not_synced_output_ids = spent_or_not_synced_output_ids_inner; - - // Get outputs for addresses and add them also the the addresses_with_unspent_outputs - let (addresses_with_unspent_outputs_inner, outputs_data_inner) = - self.get_outputs_from_address_output_ids(unspent_output_ids).await?; - - addresses_with_unspent_outputs = addresses_with_unspent_outputs_inner; - outputs_data.extend(outputs_data_inner.clone()); - outputs_data_inner - } else { - let mut new_outputs_data = Vec::new(); - for (account_or_nft_address, output_address) in &new_account_and_nft_addresses { - let output_ids = self - .get_output_ids_for_address( - &Bech32Address::new(bech32_hrp, account_or_nft_address.clone()), - options, - ) - .await?; - - // Update address with unspent outputs - let address_with_unspent_outputs = addresses_with_unspent_outputs - .iter_mut() - .find(|address| address.address.inner() == output_address) - .ok_or_else(|| { - crate::wallet::Error::WalletAddressMismatch(output_address.clone().to_bech32(bech32_hrp)) - })?; - address_with_unspent_outputs.output_ids.extend(output_ids.clone()); - - let new_outputs_data_inner = self.get_outputs(output_ids).await?; - - let outputs_data_inner = self - .output_response_to_output_data(new_outputs_data_inner, address_with_unspent_outputs) - .await?; - - outputs_data.extend(outputs_data_inner.clone()); - new_outputs_data.extend(outputs_data_inner); - } - new_outputs_data - }; - - // Clear, so we only get new addresses - new_account_and_nft_addresses.clear(); - - // Add new account and nft addresses - for output_data in new_outputs_data { - match output_data.output { - Output::Account(account_output) => { - let account_address = - AccountAddress::from(account_output.account_id_non_null(&output_data.output_id)); + // Get the unspent and spent/not-synced output ids per address to sync + let (addresses_to_sync_with_unspent_output_ids, mut spent_or_not_synced_output_ids) = self + .get_output_ids_for_addresses(addresses_to_sync.clone(), options) + .await?; - new_account_and_nft_addresses.insert(Address::Account(account_address), output_data.address); - } - Output::Nft(nft_output) => { - let nft_address = NftAddress::from(nft_output.nft_id_non_null(&output_data.output_id)); + // Get the corresponding unspent output data + let mut new_unspent_outputs_data = self + .get_outputs_from_address_output_ids(addresses_to_sync_with_unspent_output_ids) + .await?; - new_account_and_nft_addresses.insert(Address::Nft(nft_address), output_data.address); + loop { + // Try to discover new addresses + // See https://github.com/rust-lang/rust-clippy/issues/8539 regarding this lint. + #[allow(clippy::iter_with_drain)] + for (address_with_unspent, unspent_data) in new_unspent_outputs_data.drain(..) { + for unspent_data in &unspent_data { + match &unspent_data.output { + Output::Account(account) => { + addresses_to_scan.insert( + AccountAddress::from(account.account_id_non_null(&unspent_data.output_id)).into(), + address_with_unspent.address.inner().clone(), + ); + } + Output::Nft(nft) => { + addresses_to_scan.insert( + NftAddress::from(nft.nft_id_non_null(&unspent_data.output_id)).into(), + address_with_unspent.address.inner().clone(), + ); + } + _ => {} } - _ => {} } + addresses_with_unspent_output_ids_all.push(address_with_unspent); + unspent_outputs_data_all.extend(unspent_data); } - log::debug!("[SYNC] new_account_and_nft_addresses: {new_account_and_nft_addresses:?}"); - if new_account_and_nft_addresses.is_empty() { + log::debug!("[SYNC] new_addresses: {addresses_to_scan:?}"); + + // If there are no new addresses to scan, we are finished + if addresses_to_scan.is_empty() { break; } + + // Get the unspent outputs of the new addresses + for (account_or_nft_address, output_address) in addresses_to_scan.drain() { + let address_with_unspent_output_ids = addresses_with_unspent_output_ids_all + .iter_mut() + .find(|address| address.address.inner() == &output_address) + // Panic: can't happen because one is a superset of the other + .unwrap(); + + let account_or_nft_output_ids = self + .get_output_ids_for_address(&Bech32Address::new(bech32_hrp, account_or_nft_address), options) + .await?; + + // Update address with new associated unspent outputs + address_with_unspent_output_ids + .output_ids + .extend(account_or_nft_output_ids.clone()); + + let account_or_nft_outputs_with_metadata = self.get_outputs(account_or_nft_output_ids).await?; + let account_or_nft_outputs_data = self + .output_response_to_output_data( + account_or_nft_outputs_with_metadata, + address_with_unspent_output_ids, + ) + .await?; + + new_unspent_outputs_data.push((address_with_unspent_output_ids.clone(), account_or_nft_outputs_data)); + } } // get_output_ids_for_addresses() will return recursively owned outputs not anymore, sine they will only get // synced afterwards, so we filter these unspent outputs here. Maybe the spent_or_not_synced_output_ids can be // calculated more efficient in the future, by comparing the new and old outputs only at this point. Then this // retain isn't needed anymore. - - let unspent_output_ids: HashSet = HashSet::from_iter(outputs_data.iter().map(|o| o.output_id)); - spent_or_not_synced_output_ids.retain(|o| !unspent_output_ids.contains(o)); + let unspent_output_ids_all: HashSet = + HashSet::from_iter(unspent_outputs_data_all.iter().map(|o| o.output_id)); + spent_or_not_synced_output_ids.retain(|o| !unspent_output_ids_all.contains(o)); Ok(( - addresses_with_unspent_outputs, + addresses_with_unspent_output_ids_all, spent_or_not_synced_output_ids, - outputs_data, + unspent_outputs_data_all, )) } } diff --git a/sdk/src/wallet/operations/syncing/outputs.rs b/sdk/src/wallet/operations/syncing/outputs.rs index 0e18fc8c9c..011e268891 100644 --- a/sdk/src/wallet/operations/syncing/outputs.rs +++ b/sdk/src/wallet/operations/syncing/outputs.rs @@ -61,7 +61,6 @@ where metadata: *output_with_meta.metadata(), output: output_with_meta.output().clone(), is_spent: output_with_meta.metadata().is_spent(), - address: associated_address.address.inner.clone(), network_id, remainder, chain, diff --git a/sdk/src/wallet/types/mod.rs b/sdk/src/wallet/types/mod.rs index 39c2c5721d..8c0e0a41d2 100644 --- a/sdk/src/wallet/types/mod.rs +++ b/sdk/src/wallet/types/mod.rs @@ -21,7 +21,6 @@ use crate::{ types::{ api::core::OutputWithMetadataResponse, block::{ - address::Address, output::{Output, OutputId, OutputMetadata}, payload::signed_transaction::{dto::SignedTransactionPayloadDto, SignedTransactionPayload, TransactionId}, protocol::{CommittableAgeRange, ProtocolParameters}, @@ -45,8 +44,6 @@ pub struct OutputData { pub output: Output, /// If an output is spent pub is_spent: bool, - /// Associated wallet address. - pub address: Address, /// Network ID pub network_id: u64, pub remainder: bool, @@ -67,7 +64,7 @@ impl OutputData { .required_address(slot_index.into(), committable_age_range)? .ok_or(crate::client::Error::ExpirationDeadzone)?; - let chain = if required_address == self.address { + let chain = if &required_address == wallet_data.address.inner() { self.chain } else if required_address.is_ed25519() { if wallet_data.address.inner() == &required_address { diff --git a/sdk/tests/wallet/events.rs b/sdk/tests/wallet/events.rs index 65710e679b..a7d7a52b9a 100644 --- a/sdk/tests/wallet/events.rs +++ b/sdk/tests/wallet/events.rs @@ -49,7 +49,6 @@ fn wallet_events_serde() { metadata: rand_output_metadata(), output: Output::from(rand_basic_output(1_813_620_509_061_365)), is_spent: false, - address: Address::Ed25519(Ed25519Address::new([0; Ed25519Address::LENGTH])), network_id: 42, remainder: true, chain: None,