From 3e44c807dd8d28f2a7221e58c58946383676f187 Mon Sep 17 00:00:00 2001
From: /alex/ <alexander.schmidt@iota.org>
Date: Tue, 30 Apr 2024 16:29:43 +0200
Subject: [PATCH] Clean up wallet syncing code (#2108)

* const fn

* sync options

* move instead of clone

* combinators

* rm unnecessary cloning 1

* rm unnecessary cloning 2

* slices; impl Into<Option>

* refactor

* get rid of output_data

* nits

* clippy

* nit

* nest

* nit

* update python binding

* update nodejs binding

* update core binding

* rm output_data.py

* nit

* suggestion

Co-authored-by: Thoralf-M <46689931+Thoralf-M@users.noreply.github.com>

* rename local var

* fix tests and clippy

* ci-doc

* rm HashSet piping of foundry output ids

* rm TODO

* fix nodejs how-to

* remove todo

* undo rename

* rust: cleanup

* python: cleanup

* nodejs: cleanup

* rust: more cleanup

* ...

* ....

* .....

---------

Co-authored-by: Thoralf-M <46689931+Thoralf-M@users.noreply.github.com>
Co-authored-by: DaughterOfMars <alexandcoats@gmail.com>
---
 bindings/core/src/method/wallet.rs            |   4 +-
 .../how_tos/wallet/consolidate-outputs.ts     |  12 +-
 bindings/nodejs/lib/types/wallet/output.ts    |  28 ++---
 bindings/python/iota_sdk/types/output_data.py |  12 +-
 bindings/python/iota_sdk/wallet/wallet.py     |   2 +-
 cli/src/wallet_cli/mod.rs                     |   8 +-
 .../core/operations/background_syncing.rs     |   3 +-
 .../addresses/output_ids/account_foundry.rs   |  11 +-
 .../syncing/addresses/output_ids/mod.rs       |  50 ++++----
 .../operations/syncing/addresses/outputs.rs   |  33 +++---
 sdk/src/wallet/operations/syncing/mod.rs      |  93 ++++++++-------
 sdk/src/wallet/operations/syncing/options.rs  | 112 +++++++++---------
 sdk/src/wallet/operations/syncing/outputs.rs  |  59 +++++----
 sdk/src/wallet/types/address.rs               |  24 +++-
 sdk/src/wallet/types/mod.rs                   |  16 +--
 sdk/src/wallet/update.rs                      |  24 ++--
 16 files changed, 259 insertions(+), 232 deletions(-)

diff --git a/bindings/core/src/method/wallet.rs b/bindings/core/src/method/wallet.rs
index 1522beb9a8..eea0c5685d 100644
--- a/bindings/core/src/method/wallet.rs
+++ b/bindings/core/src/method/wallet.rs
@@ -139,8 +139,8 @@ pub enum WalletMethod {
     /// Expected response: [`Transaction`](crate::Response::Transaction)
     #[serde(rename_all = "camelCase")]
     GetIncomingTransaction { transaction_id: TransactionId },
-    /// Get the [`OutputData`](iota_sdk::wallet::types::OutputData) of an output stored in the wallet.
-    /// Expected response: [`OutputData`](crate::Response::OutputData)
+    /// Get the [`OutputData`](iota_sdk::wallet::types::OutputData) of an output stored
+    /// in the wallet. Expected response: [`OutputData`](crate::Response::OutputData)
     #[serde(rename_all = "camelCase")]
     GetOutput { output_id: OutputId },
     // /// Expected response: [`ParticipationEvent`](crate::Response::ParticipationEvent)
diff --git a/bindings/nodejs/examples/how_tos/wallet/consolidate-outputs.ts b/bindings/nodejs/examples/how_tos/wallet/consolidate-outputs.ts
index 9e28f71cb4..04e6b008b1 100644
--- a/bindings/nodejs/examples/how_tos/wallet/consolidate-outputs.ts
+++ b/bindings/nodejs/examples/how_tos/wallet/consolidate-outputs.ts
@@ -1,7 +1,7 @@
 // Copyright 2023 IOTA Stiftung
 // SPDX-License-Identifier: Apache-2.0
 
-import { CommonOutput, Utils, Wallet, initLogger } from '@iota/sdk';
+import { CommonOutput, Wallet, initLogger } from '@iota/sdk';
 
 // This example uses secrets in environment variables for simplicity which should not be done in production.
 require('dotenv').config({ path: '.env' });
@@ -46,11 +46,10 @@ async function run() {
         const outputs = await wallet.unspentOutputs();
         console.log('Outputs BEFORE consolidation:');
 
-        outputs.forEach(({ output, address }, i) => {
+        outputs.forEach(({ output }, i) => {
             console.log(`OUTPUT #${i}`);
             console.log(
-                '- address: %s\n- amount: %d\n- native token: %s',
-                Utils.addressToBech32(address, 'rms'),
+                '- amount: %d\n- native token: %s',
                 output.getAmount(),
                 output instanceof CommonOutput
                     ? (output as CommonOutput).getNativeToken() ?? []
@@ -80,11 +79,10 @@ async function run() {
 
         // Outputs after consolidation
         console.log('Outputs AFTER consolidation:');
-        outputs.forEach(({ output, address }, i) => {
+        outputs.forEach(({ output }, i) => {
             console.log(`OUTPUT #${i}`);
             console.log(
-                '- address: %s\n- amount: %d\n- native tokens: %s',
-                Utils.addressToBech32(address, 'rms'),
+                '- amount: %d\n- native tokens: %s',
                 output.getAmount(),
                 output instanceof CommonOutput
                     ? (output as CommonOutput).getNativeToken()
diff --git a/bindings/nodejs/lib/types/wallet/output.ts b/bindings/nodejs/lib/types/wallet/output.ts
index 7171cc57f6..f4df32792b 100644
--- a/bindings/nodejs/lib/types/wallet/output.ts
+++ b/bindings/nodejs/lib/types/wallet/output.ts
@@ -2,9 +2,8 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import { Type } from 'class-transformer';
-import { Address, AddressDiscriminator } from '../block/address';
 import { Output, OutputDiscriminator, OutputId } from '../block/output';
-import { OutputMetadataResponse } from '../models/api';
+import { OutputIdProof, OutputMetadataResponse } from '../models/api';
 
 /** Output to claim */
 export enum OutputsToClaim {
@@ -15,28 +14,23 @@ export enum OutputsToClaim {
     All = 'All',
 }
 
-/** An output with metadata */
+/** An output with additional data */
 export class OutputData {
-    /** The identifier of an Output */
-    outputId!: OutputId;
-    /** The metadata of the output */
-    metadata!: OutputMetadataResponse;
-    /** The actual Output */
+    /** The output itself */
     @Type(() => Output, {
         discriminator: OutputDiscriminator,
     })
     output!: Output;
-    /** Associated account address */
-    @Type(() => Address, {
-        discriminator: AddressDiscriminator,
-    })
-    address!: Address;
-    /** Network ID */
+    /** The metadata of the output */
+    metadata!: OutputMetadataResponse;
+    /** The output ID proof */
+    OutputIdProof!: OutputIdProof;
+    /** The corresponding output ID */
+    outputId!: OutputId;
+    /** The network ID the output belongs to */
     networkId!: string;
-    /** Remainder */
+    /** Whether the output represents a remainder amount */
     remainder!: boolean;
-    /** BIP32 path */
-    chain?: Segment[];
 }
 
 /** A Segment of the BIP32 path*/
diff --git a/bindings/python/iota_sdk/types/output_data.py b/bindings/python/iota_sdk/types/output_data.py
index ad23de6b93..8b5be17af4 100644
--- a/bindings/python/iota_sdk/types/output_data.py
+++ b/bindings/python/iota_sdk/types/output_data.py
@@ -13,19 +13,19 @@
 @json
 @dataclass
 class OutputData:
-    """Output data.
+    """An output with additional data.
 
     Attributes:
-        output_id: With the output data corresponding output ID.
-        metadata: With the output corresponding metadata.
-        output: The output object itself.
+        output: The output itself.
+        metadata: The metadata of the output.
         output_id_proof: The output ID proof.
+        output_id: The corresponding output ID.
         network_id: The network ID the output belongs to.
         remainder: Whether the output represents a remainder amount.
     """
-    output_id: OutputId
-    metadata: OutputMetadata
     output: Output
+    metadata: OutputMetadata
     output_id_proof: OutputIdProof
+    output_id: OutputId
     network_id: str
     remainder: bool
diff --git a/bindings/python/iota_sdk/wallet/wallet.py b/bindings/python/iota_sdk/wallet/wallet.py
index 6e95c64bd6..8a8b88a576 100644
--- a/bindings/python/iota_sdk/wallet/wallet.py
+++ b/bindings/python/iota_sdk/wallet/wallet.py
@@ -16,9 +16,9 @@
 from iota_sdk.types.client_options import ClientOptions
 from iota_sdk.types.filter_options import FilterOptions
 from iota_sdk.types.native_token import NativeToken
+from iota_sdk.types.output import BasicOutput, NftOutput, Output, deserialize_output
 from iota_sdk.types.output_data import OutputData
 from iota_sdk.types.output_id import OutputId
-from iota_sdk.types.output import BasicOutput, NftOutput, Output, deserialize_output
 from iota_sdk.types.output_params import OutputParams
 from iota_sdk.types.transaction_data import PreparedTransactionData, SignedTransactionData
 from iota_sdk.types.transaction_id import TransactionId
diff --git a/cli/src/wallet_cli/mod.rs b/cli/src/wallet_cli/mod.rs
index 16e7fbadd1..0deaa23576 100644
--- a/cli/src/wallet_cli/mod.rs
+++ b/cli/src/wallet_cli/mod.rs
@@ -1765,14 +1765,14 @@ pub async fn prompt_internal(
     Ok(PromptResponse::Reprompt)
 }
 
-fn print_outputs(mut outputs: Vec<OutputData>, title: &str) -> Result<(), Error> {
-    if outputs.is_empty() {
+fn print_outputs(mut outputs_data: Vec<OutputData>, title: &str) -> Result<(), Error> {
+    if outputs_data.is_empty() {
         println_log_info!("No outputs found");
     } else {
         println_log_info!("{title}");
-        outputs.sort_unstable_by_key(|o| o.output_id);
+        outputs_data.sort_unstable_by_key(|o| o.output_id);
 
-        for (i, output_data) in outputs.into_iter().enumerate() {
+        for (i, output_data) in outputs_data.into_iter().enumerate() {
             let kind_str = if output_data.output.is_implicit_account() {
                 "ImplicitAccount"
             } else {
diff --git a/sdk/src/wallet/core/operations/background_syncing.rs b/sdk/src/wallet/core/operations/background_syncing.rs
index 5d70eb14b7..e496d56151 100644
--- a/sdk/src/wallet/core/operations/background_syncing.rs
+++ b/sdk/src/wallet/core/operations/background_syncing.rs
@@ -28,11 +28,12 @@ where
     /// Start the background syncing process for the wallet, default interval is 7 seconds
     pub async fn start_background_syncing(
         &self,
-        options: Option<SyncOptions>,
+        options: impl Into<Option<SyncOptions>> + Send,
         interval: Option<Duration>,
     ) -> Result<(), WalletError> {
         log::debug!("[start_background_syncing]");
 
+        let options = options.into();
         let (tx_background_sync, mut rx_background_sync) = self.background_syncing_status.clone();
 
         // stop existing process if running
diff --git a/sdk/src/wallet/operations/syncing/addresses/output_ids/account_foundry.rs b/sdk/src/wallet/operations/syncing/addresses/output_ids/account_foundry.rs
index e67ef0339f..17d4095ee2 100644
--- a/sdk/src/wallet/operations/syncing/addresses/output_ids/account_foundry.rs
+++ b/sdk/src/wallet/operations/syncing/addresses/output_ids/account_foundry.rs
@@ -55,7 +55,7 @@ impl<S: 'static + SecretManage> Wallet<S> {
     ) -> Result<Vec<OutputId>, WalletError> {
         log::debug!("[SYNC] get_foundry_output_ids");
         // Get account outputs, so we can then get the foundry outputs with the account addresses
-        let account_outputs_with_meta = self.get_outputs(account_output_ids.to_vec()).await?;
+        let account_outputs_with_meta = self.get_outputs_request_unknown(account_output_ids).await?;
 
         let bech32_hrp = self.client().get_bech32_hrp().await?;
 
@@ -77,14 +77,9 @@ impl<S: 'static + SecretManage> Wallet<S> {
             }
         }
 
-        let mut output_ids = HashSet::new();
         let results: Vec<Result<OutputIdsResponse, WalletError>> = futures::future::try_join_all(tasks).await?;
+        let responses: Vec<OutputIdsResponse> = results.into_iter().collect::<Result<Vec<_>, _>>()?;
 
-        for res in results {
-            let foundry_output_ids = res?;
-            output_ids.extend(foundry_output_ids.items);
-        }
-
-        Ok(output_ids.into_iter().collect())
+        Ok(responses.into_iter().flat_map(|res| res.items).collect())
     }
 }
diff --git a/sdk/src/wallet/operations/syncing/addresses/output_ids/mod.rs b/sdk/src/wallet/operations/syncing/addresses/output_ids/mod.rs
index 16caf0613c..c39504c21c 100644
--- a/sdk/src/wallet/operations/syncing/addresses/output_ids/mod.rs
+++ b/sdk/src/wallet/operations/syncing/addresses/output_ids/mod.rs
@@ -20,8 +20,10 @@ use crate::{
     },
     types::block::{address::Bech32Address, output::OutputId},
     wallet::{
-        constants::PARALLEL_REQUESTS_AMOUNT, operations::syncing::SyncOptions,
-        types::address::AddressWithUnspentOutputs, Wallet, WalletError,
+        constants::PARALLEL_REQUESTS_AMOUNT,
+        operations::syncing::SyncOptions,
+        types::address::{AddressWithUnspentOutputIds, SpentOutputId},
+        Wallet, WalletError,
     },
 };
 
@@ -217,11 +219,8 @@ impl<S: 'static + SecretManage> Wallet<S> {
         let results = futures::future::try_join_all(tasks).await?;
 
         // Get all results
-        let mut output_ids = HashSet::new();
-        for res in results {
-            let found_output_ids = res?;
-            output_ids.extend(found_output_ids);
-        }
+        let output_ids = results.into_iter().collect::<Result<Vec<_>, _>>()?;
+        let output_ids: HashSet<OutputId> = HashSet::from_iter(output_ids.into_iter().flat_map(|v| v.into_iter()));
 
         Ok(output_ids.into_iter().collect())
     }
@@ -230,20 +229,20 @@ impl<S: 'static + SecretManage> Wallet<S> {
     /// return spent outputs separated
     pub(crate) async fn get_output_ids_for_addresses(
         &self,
-        addresses_with_unspent_outputs: Vec<AddressWithUnspentOutputs>,
+        addresses: &[AddressWithUnspentOutputIds],
         options: &SyncOptions,
-    ) -> Result<(Vec<AddressWithUnspentOutputs>, Vec<OutputId>), WalletError> {
+    ) -> Result<(Vec<AddressWithUnspentOutputIds>, Vec<SpentOutputId>), WalletError> {
         log::debug!("[SYNC] start get_output_ids_for_addresses");
         let address_output_ids_start_time = Instant::now();
 
-        let mut addresses_with_outputs = Vec::new();
+        let mut addresses_with_unspent_outputs = Vec::new();
         // spent outputs or account/nft/foundries that don't get synced anymore, because of other sync options
-        let mut spent_or_not_anymore_synced_outputs = Vec::new();
+        let mut spent_or_ignored_outputs = 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
+        for addresses_chunk in addresses
             .chunks(PARALLEL_REQUESTS_AMOUNT)
-            .map(|x: &[AddressWithUnspentOutputs]| x.to_vec())
+            .map(|x: &[AddressWithUnspentOutputIds]| x.to_vec())
         {
             let results: Vec<Result<_, WalletError>>;
             #[cfg(target_family = "wasm")]
@@ -276,35 +275,36 @@ impl<S: 'static + SecretManage> Wallet<S> {
                 results = futures::future::try_join_all(tasks).await?;
             }
 
-            for res in results {
-                let (mut address, output_ids): (AddressWithUnspentOutputs, Vec<OutputId>) = res?;
+            let addresses_with_new_unspent_output_ids = results.into_iter().collect::<Result<Vec<_>, _>>()?;
+
+            for (mut address, new_unspent_output_ids) in addresses_with_new_unspent_output_ids {
                 // only return addresses with outputs
-                if !output_ids.is_empty() {
+                if !new_unspent_output_ids.is_empty() {
                     // outputs we had before, but now not anymore, got spent or are account/nft/foundries that don't
                     // get synced anymore because of other sync options
-                    for output_id in address.output_ids {
-                        if !output_ids.contains(&output_id) {
-                            spent_or_not_anymore_synced_outputs.push(output_id);
+                    for output_id in address.unspent_output_ids {
+                        if !new_unspent_output_ids.contains(&output_id) {
+                            spent_or_ignored_outputs.push(output_id);
                         }
                     }
-                    address.output_ids = output_ids;
-                    addresses_with_outputs.push(address);
+                    address.unspent_output_ids = new_unspent_output_ids;
+                    addresses_with_unspent_outputs.push(address);
                 } else {
                     // outputs we had before, but now not anymore, got spent or are account/nft/foundries that don't
                     // get synced anymore because of other sync options
-                    spent_or_not_anymore_synced_outputs.extend(address.output_ids);
+                    spent_or_ignored_outputs.extend(address.unspent_output_ids);
                 }
             }
         }
 
         log::debug!(
-            "[SYNC] spent or not anymore synced account/nft/foundries outputs: {:?}",
-            spent_or_not_anymore_synced_outputs
+            "[SYNC] spent or ignored account/nft/foundries outputs: {:?}",
+            spent_or_ignored_outputs
         );
         log::debug!(
             "[SYNC] finished get_output_ids_for_addresses in {:.2?}",
             address_output_ids_start_time.elapsed()
         );
-        Ok((addresses_with_outputs, spent_or_not_anymore_synced_outputs))
+        Ok((addresses_with_unspent_outputs, spent_or_ignored_outputs))
     }
 }
diff --git a/sdk/src/wallet/operations/syncing/addresses/outputs.rs b/sdk/src/wallet/operations/syncing/addresses/outputs.rs
index 90df1a4d45..77dee0b8ac 100644
--- a/sdk/src/wallet/operations/syncing/addresses/outputs.rs
+++ b/sdk/src/wallet/operations/syncing/addresses/outputs.rs
@@ -8,47 +8,52 @@ use crate::{
     wallet::{
         constants::PARALLEL_REQUESTS_AMOUNT,
         task,
-        types::{address::AddressWithUnspentOutputs, OutputData},
+        types::address::{AddressWithUnspentOutputIds, AddressWithUnspentOutputs},
         Wallet, WalletError,
     },
 };
 
 impl<S: 'static + SecretManage> Wallet<S> {
-    /// Get outputs from addresses
+    /// Get unspent outputs from addresses
     pub(crate) async fn get_outputs_from_address_output_ids(
         &self,
-        addresses_with_unspent_outputs: Vec<AddressWithUnspentOutputs>,
-    ) -> Result<Vec<(AddressWithUnspentOutputs, Vec<OutputData>)>, WalletError> {
+        addresses_with_unspent_output_ids: &[AddressWithUnspentOutputIds],
+    ) -> Result<Vec<AddressWithUnspentOutputs>, WalletError> {
         log::debug!("[SYNC] start get_outputs_from_address_output_ids");
         let address_outputs_start_time = Instant::now();
 
+        let network_id = self.client().get_network_id().await?;
+
         let mut addresses_with_outputs = 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
+        for addresses_chunk in addresses_with_unspent_output_ids
             .chunks(PARALLEL_REQUESTS_AMOUNT)
-            .map(|x: &[AddressWithUnspentOutputs]| x.to_vec())
+            .map(|x: &[AddressWithUnspentOutputIds]| x.to_vec())
         {
             let mut tasks = Vec::new();
-            for address_with_unspent_outputs in addresses_chunk {
+            for address_with_unspent_output_ids in addresses_chunk {
                 let wallet = self.clone();
                 tasks.push(async move {
                     task::spawn(async move {
                         let unspent_outputs_with_metadata = wallet
-                            .get_outputs(address_with_unspent_outputs.output_ids.clone())
+                            .get_outputs_request_unknown(address_with_unspent_output_ids.unspent_output_ids())
                             .await?;
-                        let unspent_outputs_data = wallet
-                            .output_response_to_output_data(unspent_outputs_with_metadata)
+                        let unspent_outputs = wallet
+                            .output_response_to_output_data(unspent_outputs_with_metadata, network_id)
                             .await?;
-                        Ok((address_with_unspent_outputs, unspent_outputs_data))
+
+                        Ok(AddressWithUnspentOutputs {
+                            address_with_unspent_output_ids,
+                            unspent_outputs,
+                        })
                     })
                     .await
                 });
             }
             let results: Vec<Result<_, WalletError>> = futures::future::try_join_all(tasks).await?;
-            for res in results {
-                addresses_with_outputs.push(res?);
-            }
+            let result = results.into_iter().collect::<Result<Vec<_>, _>>()?;
+            addresses_with_outputs.extend(result.into_iter());
         }
         log::debug!(
             "[SYNC] finished get_outputs_from_address_output_ids in {:.2?}",
diff --git a/sdk/src/wallet/operations/syncing/mod.rs b/sdk/src/wallet/operations/syncing/mod.rs
index 405338d7fb..046fa54d83 100644
--- a/sdk/src/wallet/operations/syncing/mod.rs
+++ b/sdk/src/wallet/operations/syncing/mod.rs
@@ -18,13 +18,16 @@ use crate::{
     },
     wallet::{
         constants::MIN_SYNC_INTERVAL,
-        types::{address::AddressWithUnspentOutputs, Balance, OutputData},
+        types::{
+            address::{AddressWithUnspentOutputIds, AddressWithUnspentOutputs, SpentOutputId},
+            Balance, OutputData,
+        },
         Wallet, WalletError,
     },
 };
 
 impl<S: 'static + SecretManage> Wallet<S> {
-    /// Set the fallback SyncOptions for account syncing.
+    /// Set the default SyncOptions for account syncing.
     /// If storage is enabled, will persist during restarts.
     pub async fn set_default_sync_options(&self, options: SyncOptions) -> Result<(), WalletError> {
         #[cfg(feature = "storage")]
@@ -46,51 +49,55 @@ impl<S: 'static + SecretManage> Wallet<S> {
     // are found.
     async fn request_outputs_recursively(
         &self,
-        addresses_to_sync: Vec<AddressWithUnspentOutputs>,
+        addresses: &[AddressWithUnspentOutputIds],
         options: &SyncOptions,
-    ) -> Result<(Vec<AddressWithUnspentOutputs>, Vec<OutputId>, Vec<OutputData>), WalletError> {
-        // 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<Address, Address> = HashMap::new();
-        let mut addresses_with_unspent_output_ids_all = Vec::new();
-        let mut unspent_outputs_data_all = Vec::new();
-
+    ) -> Result<(Vec<AddressWithUnspentOutputIds>, Vec<SpentOutputId>, Vec<OutputData>), WalletError> {
         let bech32_hrp = self.client().get_bech32_hrp().await?;
+        let network_id = self.client().get_network_id().await?;
 
         // 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?;
+        let (addresses_with_unspent_output_ids, mut spent_or_not_synced_output_ids) =
+            self.get_output_ids_for_addresses(addresses, options).await?;
 
-        // 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)
+        // Get the corresponding unspent outputs
+        let mut addresses_with_unspent_outputs = self
+            .get_outputs_from_address_output_ids(&addresses_with_unspent_output_ids)
             .await?;
 
+        // 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<Address, Address> = HashMap::new();
+        let mut new_addresses_with_unspent_output_ids = Vec::new();
+        let mut unspent_outputs_data = Vec::new();
+
         loop {
-            // Try to discover new addresses
+            // Try to discover new addresses and collect them in `addresses_to_scan`.
             // 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 {
+            for AddressWithUnspentOutputs {
+                address_with_unspent_output_ids,
+                unspent_outputs,
+            } in addresses_with_unspent_outputs.drain(..)
+            {
+                for unspent_output in &unspent_outputs {
+                    match &unspent_output.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(),
+                                AccountAddress::from(account.account_id_non_null(&unspent_output.output_id)).into(),
+                                (*address_with_unspent_output_ids).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(),
+                                NftAddress::from(nft.nft_id_non_null(&unspent_output.output_id)).into(),
+                                (*address_with_unspent_output_ids).inner().clone(),
                             );
                         }
                         _ => {}
                     }
                 }
-                addresses_with_unspent_output_ids_all.push(address_with_unspent);
-                unspent_outputs_data_all.extend(unspent_data);
+                new_addresses_with_unspent_output_ids.push(address_with_unspent_output_ids);
+                unspent_outputs_data.extend(unspent_outputs);
             }
 
             log::debug!("[SYNC] new_addresses: {addresses_to_scan:?}");
@@ -102,7 +109,7 @@ impl<S: 'static + SecretManage> Wallet<S> {
 
             // 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
+                let address_with_unspent_output_ids = new_addresses_with_unspent_output_ids
                     .iter_mut()
                     .find(|address| address.address.inner() == &output_address)
                     // Panic: can't happen because one is a superset of the other
@@ -114,15 +121,19 @@ impl<S: 'static + SecretManage> Wallet<S> {
 
                 // Update address with new associated unspent outputs
                 address_with_unspent_output_ids
-                    .output_ids
+                    .unspent_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_with_metadata =
+                    self.get_outputs_request_unknown(&account_or_nft_output_ids).await?;
                 let account_or_nft_outputs_data = self
-                    .output_response_to_output_data(account_or_nft_outputs_with_metadata)
+                    .output_response_to_output_data(account_or_nft_outputs_with_metadata, network_id)
                     .await?;
 
-                new_unspent_outputs_data.push((address_with_unspent_output_ids.clone(), account_or_nft_outputs_data));
+                addresses_with_unspent_outputs.push(AddressWithUnspentOutputs {
+                    address_with_unspent_output_ids: address_with_unspent_output_ids.clone(),
+                    unspent_outputs: account_or_nft_outputs_data,
+                });
             }
         }
 
@@ -131,13 +142,13 @@ impl<S: 'static + SecretManage> Wallet<S> {
         // 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_all: HashSet<OutputId> =
-            HashSet::from_iter(unspent_outputs_data_all.iter().map(|o| o.output_id));
+            HashSet::from_iter(unspent_outputs_data.iter().map(|o| o.output_id));
         spent_or_not_synced_output_ids.retain(|o| !unspent_output_ids_all.contains(o));
 
         Ok((
-            addresses_with_unspent_output_ids_all,
+            new_addresses_with_unspent_output_ids,
             spent_or_not_synced_output_ids,
-            unspent_outputs_data_all,
+            unspent_outputs_data,
         ))
     }
 }
@@ -149,8 +160,8 @@ where
 {
     /// Sync the wallet by fetching new information from the nodes. Will also reissue pending transactions
     /// if necessary. A custom default can be set using set_default_sync_options.
-    pub async fn sync(&self, options: Option<SyncOptions>) -> Result<Balance, WalletError> {
-        let options = match options {
+    pub async fn sync(&self, options: impl Into<Option<SyncOptions>> + Send) -> Result<Balance, WalletError> {
+        let options = match options.into() {
             Some(opt) => opt,
             None => self.default_sync_options().await,
         };
@@ -196,17 +207,17 @@ where
     async fn sync_internal(&self, options: &SyncOptions) -> Result<(), WalletError> {
         log::debug!("[SYNC] sync_internal");
 
-        let wallet_address_with_unspent_outputs = AddressWithUnspentOutputs {
+        let wallet_address_with_unspent_outputs = AddressWithUnspentOutputIds {
             address: self.address().await,
-            output_ids: self.ledger().await.unspent_outputs().keys().copied().collect(),
+            unspent_output_ids: self.ledger().await.unspent_outputs().keys().copied().collect(),
         };
 
         let mut addresses_to_sync = vec![wallet_address_with_unspent_outputs];
 
         if options.sync_implicit_accounts {
             if let Ok(implicit_account_creation_address) = self.implicit_account_creation_address().await {
-                addresses_to_sync.push(AddressWithUnspentOutputs {
-                    output_ids: self
+                addresses_to_sync.push(AddressWithUnspentOutputIds {
+                    unspent_output_ids: self
                         .ledger()
                         .await
                         .implicit_accounts()
@@ -224,7 +235,7 @@ where
         }
 
         let (_addresses_with_unspent_outputs, spent_or_not_synced_output_ids, outputs_data) =
-            self.request_outputs_recursively(addresses_to_sync, options).await?;
+            self.request_outputs_recursively(&addresses_to_sync, options).await?;
 
         // Request possible spent outputs
         log::debug!("[SYNC] spent_or_not_synced_outputs: {spent_or_not_synced_output_ids:?}");
diff --git a/sdk/src/wallet/operations/syncing/options.rs b/sdk/src/wallet/operations/syncing/options.rs
index f2626acf09..9616249ebd 100644
--- a/sdk/src/wallet/operations/syncing/options.rs
+++ b/sdk/src/wallet/operations/syncing/options.rs
@@ -3,28 +3,21 @@
 
 use serde::{Deserialize, Serialize};
 
-const DEFAULT_FORCE_SYNCING: bool = false;
-const DEFAULT_SYNC_INCOMING_TRANSACTIONS: bool = false;
-const DEFAULT_SYNC_ONLY_MOST_BASIC_OUTPUTS: bool = false;
-const DEFAULT_SYNC_PENDING_TRANSACTIONS: bool = true;
-const DEFAULT_SYNC_NATIVE_TOKEN_FOUNDRIES: bool = false;
-const DEFAULT_SYNC_IMPLICIT_ACCOUNTS: bool = false;
-
 /// The synchronization options
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct SyncOptions {
     /// Syncing is usually skipped if it's called repeatedly in a short amount of time as there can only be new changes
     /// every slot and calling it twice "at the same time" will not return new data.
     /// When this to true, we sync anyways, even if it's called 0ms after the last sync finished.
-    #[serde(default)]
+    #[serde(default = "no")]
     pub force_syncing: bool,
     /// Try to sync transactions from incoming outputs with their inputs. Some data may not be obtained if it has been
     /// pruned.
-    #[serde(default = "default_sync_incoming_transactions")]
+    #[serde(default = "no")]
     pub sync_incoming_transactions: bool,
     /// Checks pending transactions and reissues them if necessary.
-    #[serde(default = "default_sync_pending_transactions")]
+    #[serde(default = "yes")]
     pub sync_pending_transactions: bool,
     /// Specifies what outputs should be synced for the ed25519 address from the wallet.
     #[serde(default)]
@@ -37,73 +30,53 @@ pub struct SyncOptions {
     pub nft: NftSyncOptions,
     /// Specifies if only basic outputs with an AddressUnlockCondition alone should be synced, will overwrite
     /// `wallet`, `account` and `nft` options.
-    #[serde(default = "default_sync_only_most_basic_outputs")]
+    #[serde(default = "no")]
     pub sync_only_most_basic_outputs: bool,
     /// Sync native token foundries, so their metadata can be returned in the balance.
-    #[serde(default = "default_sync_native_token_foundries")]
+    #[serde(default = "no")]
     pub sync_native_token_foundries: bool,
     /// Sync implicit accounts.
-    #[serde(default = "default_sync_implicit_accounts")]
+    #[serde(default = "no")]
     pub sync_implicit_accounts: bool,
 }
 
-fn default_force_syncing() -> bool {
-    DEFAULT_FORCE_SYNCING
-}
-
-fn default_sync_incoming_transactions() -> bool {
-    DEFAULT_SYNC_INCOMING_TRANSACTIONS
-}
-
-fn default_sync_only_most_basic_outputs() -> bool {
-    DEFAULT_SYNC_ONLY_MOST_BASIC_OUTPUTS
-}
-
-fn default_sync_pending_transactions() -> bool {
-    DEFAULT_SYNC_PENDING_TRANSACTIONS
-}
-
-fn default_sync_native_token_foundries() -> bool {
-    DEFAULT_SYNC_NATIVE_TOKEN_FOUNDRIES
-}
-
-fn default_sync_implicit_accounts() -> bool {
-    DEFAULT_SYNC_IMPLICIT_ACCOUNTS
-}
-
 impl Default for SyncOptions {
     fn default() -> Self {
         Self {
-            sync_incoming_transactions: default_sync_incoming_transactions(),
-            sync_pending_transactions: default_sync_pending_transactions(),
+            force_syncing: no(),
+            sync_incoming_transactions: no(),
+            sync_pending_transactions: yes(),
             wallet: WalletSyncOptions::default(),
             account: AccountSyncOptions::default(),
             nft: NftSyncOptions::default(),
-            sync_only_most_basic_outputs: default_sync_only_most_basic_outputs(),
-            sync_native_token_foundries: default_sync_native_token_foundries(),
-            force_syncing: default_force_syncing(),
-            sync_implicit_accounts: default_sync_implicit_accounts(),
+            sync_only_most_basic_outputs: no(),
+            sync_native_token_foundries: no(),
+            sync_implicit_accounts: no(),
         }
     }
 }
 
 /// Sync options for Ed25519 addresses from the wallet
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(default, rename_all = "camelCase")]
 pub struct WalletSyncOptions {
+    #[serde(default = "yes")]
     pub basic_outputs: bool,
+    #[serde(default = "yes")]
     pub account_outputs: bool,
+    #[serde(default = "yes")]
     pub nft_outputs: bool,
+    #[serde(default = "yes")]
     pub delegation_outputs: bool,
 }
 
 impl Default for WalletSyncOptions {
     fn default() -> Self {
         Self {
-            basic_outputs: true,
-            account_outputs: true,
-            nft_outputs: true,
-            delegation_outputs: true,
+            basic_outputs: yes(),
+            account_outputs: yes(),
+            nft_outputs: yes(),
+            delegation_outputs: yes(),
         }
     }
 }
@@ -115,13 +88,18 @@ impl WalletSyncOptions {
 }
 
 /// Sync options for addresses from account outputs
-#[derive(Debug, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(default, rename_all = "camelCase")]
 pub struct AccountSyncOptions {
+    #[serde(default = "no")]
     pub basic_outputs: bool,
+    #[serde(default = "no")]
     pub account_outputs: bool,
+    #[serde(default = "yes")]
     pub foundry_outputs: bool,
+    #[serde(default = "no")]
     pub nft_outputs: bool,
+    #[serde(default = "no")]
     pub delegation_outputs: bool,
 }
 
@@ -129,11 +107,11 @@ impl Default for AccountSyncOptions {
     // Sync only foundries
     fn default() -> Self {
         Self {
-            basic_outputs: false,
-            account_outputs: false,
-            foundry_outputs: true,
-            nft_outputs: false,
-            delegation_outputs: false,
+            basic_outputs: no(),
+            account_outputs: no(),
+            foundry_outputs: yes(),
+            nft_outputs: no(),
+            delegation_outputs: no(),
         }
     }
 }
@@ -149,17 +127,39 @@ impl AccountSyncOptions {
 }
 
 /// Sync options for addresses from NFT outputs
-#[derive(Debug, Default, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash, Serialize, Deserialize)]
 #[serde(default, rename_all = "camelCase")]
 pub struct NftSyncOptions {
+    #[serde(default = "no")]
     pub basic_outputs: bool,
+    #[serde(default = "no")]
     pub account_outputs: bool,
+    #[serde(default = "no")]
     pub nft_outputs: bool,
+    #[serde(default = "no")]
     pub delegation_outputs: bool,
 }
 
+impl Default for NftSyncOptions {
+    fn default() -> Self {
+        Self {
+            basic_outputs: no(),
+            account_outputs: no(),
+            nft_outputs: no(),
+            delegation_outputs: no(),
+        }
+    }
+}
 impl NftSyncOptions {
     pub(crate) fn all_outputs(&self) -> bool {
         self.basic_outputs && self.account_outputs && self.nft_outputs && self.delegation_outputs
     }
 }
+
+const fn yes() -> bool {
+    true
+}
+
+const fn no() -> bool {
+    false
+}
diff --git a/sdk/src/wallet/operations/syncing/outputs.rs b/sdk/src/wallet/operations/syncing/outputs.rs
index b8e62b6462..c6ab36af37 100644
--- a/sdk/src/wallet/operations/syncing/outputs.rs
+++ b/sdk/src/wallet/operations/syncing/outputs.rs
@@ -18,46 +18,53 @@ use crate::{
 };
 
 impl<S: 'static + SecretManage> Wallet<S> {
-    /// Convert OutputWithMetadataResponse to OutputData with the network_id added
+    /// Convert `OutputWithMetadataResponse` to `OutputData` with the network_id added.
     pub(crate) async fn output_response_to_output_data(
         &self,
-        outputs_with_meta: Vec<OutputWithMetadataResponse>,
+        outputs_with_metadata: Vec<OutputWithMetadataResponse>,
+        network_id: u64,
     ) -> Result<Vec<OutputData>, WalletError> {
         log::debug!("[SYNC] convert output_responses");
-        // store outputs with network_id
-        let network_id = self.client().get_network_id().await?;
+
         let wallet_ledger = self.ledger().await;
 
-        Ok(outputs_with_meta
+        Ok(outputs_with_metadata
             .into_iter()
-            .map(|output_with_meta| {
-                // check if we know the transaction that created this output and if we created it (if we store incoming
-                // transactions separated, then this check wouldn't be required)
-                let remainder = wallet_ledger
-                    .transactions
-                    .get(output_with_meta.metadata().output_id().transaction_id())
-                    .map_or(false, |tx| !tx.incoming);
-
-                OutputData {
-                    output_id: output_with_meta.metadata().output_id().to_owned(),
-                    metadata: *output_with_meta.metadata(),
-                    output: output_with_meta.output().clone(),
-                    output_id_proof: output_with_meta.output_id_proof().clone(),
-                    network_id,
-                    remainder,
-                }
-            })
+            .map(
+                |OutputWithMetadataResponse {
+                     output,
+                     output_id_proof,
+                     metadata,
+                 }| {
+                    // check if we know the transaction that created this output and if we created it (if we store
+                    // incoming transactions separated, then this check wouldn't be required)
+                    let remainder = wallet_ledger
+                        .transactions
+                        .get(metadata.output_id().transaction_id())
+                        .map_or(false, |tx| !tx.incoming);
+
+                    OutputData {
+                        output_id: metadata.output_id().to_owned(),
+                        metadata,
+                        output,
+                        output_id_proof,
+                        network_id,
+                        remainder,
+                    }
+                },
+            )
             .collect())
     }
 
     /// Gets outputs by their id, already known outputs are not requested again, but loaded from the account set as
     /// unspent, because we wouldn't get them from the node if they were spent
-    pub(crate) async fn get_outputs(
+    pub(crate) async fn get_outputs_request_unknown(
         &self,
-        output_ids: Vec<OutputId>,
+        output_ids: &[OutputId],
     ) -> Result<Vec<OutputWithMetadataResponse>, WalletError> {
         log::debug!("[SYNC] start get_outputs");
         let get_outputs_start_time = Instant::now();
+
         let mut outputs = Vec::new();
         let mut unknown_outputs = Vec::new();
         let mut unspent_outputs = Vec::new();
@@ -71,14 +78,14 @@ impl<S: 'static + SecretManage> Wallet<S> {
                         log::warn!("Removing spent output metadata for {output_id}, because it's still unspent");
                         output_data.metadata.spent = None;
                     }
-                    unspent_outputs.push((output_id, output_data.clone()));
+                    unspent_outputs.push((*output_id, output_data.clone()));
                     outputs.push(OutputWithMetadataResponse::new(
                         output_data.output.clone(),
                         output_data.output_id_proof.clone(),
                         output_data.metadata,
                     ));
                 }
-                None => unknown_outputs.push(output_id),
+                None => unknown_outputs.push(*output_id),
             }
         }
         // known output is unspent, so insert it to the unspent outputs again, because if it was an
diff --git a/sdk/src/wallet/types/address.rs b/sdk/src/wallet/types/address.rs
index 1fa0ae745f..d702bb3bde 100644
--- a/sdk/src/wallet/types/address.rs
+++ b/sdk/src/wallet/types/address.rs
@@ -4,15 +4,27 @@
 use getset::{Getters, Setters};
 use serde::{Deserialize, Serialize};
 
-use crate::types::block::{address::Bech32Address, output::OutputId};
+use crate::{
+    types::block::{address::Bech32Address, output::OutputId},
+    wallet::OutputData,
+};
+
+#[derive(Debug, Getters, Setters, Clone, Serialize, Deserialize, Eq, PartialEq, derive_more::Deref)]
+#[serde(rename_all = "camelCase")]
+#[getset(get = "pub")]
+pub(crate) struct AddressWithUnspentOutputIds {
+    #[deref]
+    pub(crate) address: Bech32Address,
+    pub(crate) unspent_output_ids: Vec<OutputId>,
+}
 
-/// An account address with unspent output_ids for unspent outputs.
 #[derive(Debug, Getters, Setters, Clone, Serialize, Deserialize, Eq, PartialEq)]
 #[serde(rename_all = "camelCase")]
 #[getset(get = "pub")]
 pub(crate) struct AddressWithUnspentOutputs {
-    /// The address.
-    pub(crate) address: Bech32Address,
-    /// Output ids
-    pub(crate) output_ids: Vec<OutputId>,
+    #[serde(flatten)]
+    pub(crate) address_with_unspent_output_ids: AddressWithUnspentOutputIds,
+    pub(crate) unspent_outputs: Vec<OutputData>,
 }
+
+pub(crate) type SpentOutputId = OutputId;
diff --git a/sdk/src/wallet/types/mod.rs b/sdk/src/wallet/types/mod.rs
index e2dac2034d..0eef099fbb 100644
--- a/sdk/src/wallet/types/mod.rs
+++ b/sdk/src/wallet/types/mod.rs
@@ -32,19 +32,21 @@ use crate::{
     wallet::WalletError,
 };
 
-/// An output with metadata
+/// An output with additional data.
 #[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct OutputData {
-    /// The output id
-    pub output_id: OutputId,
-    pub metadata: OutputMetadata,
-    /// The actual Output
+    /// The output itself.
     pub output: Output,
-    /// The output ID proof
+    /// The metadata of the output.
+    pub metadata: OutputMetadata,
+    /// The output ID proof.
     pub output_id_proof: OutputIdProof,
-    /// Network ID
+    /// The corresponding output ID.
+    pub output_id: OutputId,
+    /// The network ID the output belongs to.
     pub network_id: u64,
+    /// Whether the output represents a remainder amount.
     pub remainder: bool,
 }
 
diff --git a/sdk/src/wallet/update.rs b/sdk/src/wallet/update.rs
index bb7572c6d3..a4f65a8b3a 100644
--- a/sdk/src/wallet/update.rs
+++ b/sdk/src/wallet/update.rs
@@ -58,7 +58,7 @@ impl<S: 'static + SecretManage> Wallet<S> {
     /// Update wallet with newly synced data and emit events for outputs.
     pub(crate) async fn update_after_sync(
         &self,
-        unspent_outputs: Vec<OutputData>,
+        unspent_outputs_data: Vec<OutputData>,
         spent_or_unsynced_output_metadata_map: HashMap<OutputId, Option<OutputMetadata>>,
     ) -> Result<(), WalletError> {
         log::debug!("[SYNC] Update wallet ledger with new synced transactions");
@@ -67,13 +67,13 @@ impl<S: 'static + SecretManage> Wallet<S> {
         let mut wallet_ledger = self.ledger_mut().await;
 
         // Update spent outputs
-        for (output_id, output_metadata_response_opt) in spent_or_unsynced_output_metadata_map {
+        for (output_id, output_metadata_opt) in spent_or_unsynced_output_metadata_map {
             // If we got the output response and it's still unspent, skip it
-            if let Some(output_metadata_response) = output_metadata_response_opt {
-                if output_metadata_response.is_spent() {
+            if let Some(output_metadata) = output_metadata_opt {
+                if output_metadata.is_spent() {
                     wallet_ledger.unspent_outputs.remove(&output_id);
                     if let Some(output_data) = wallet_ledger.outputs.get_mut(&output_id) {
-                        output_data.metadata = output_metadata_response;
+                        output_data.metadata = output_metadata;
                     }
                 } else {
                     // not spent, just not synced, skip
@@ -116,20 +116,20 @@ impl<S: 'static + SecretManage> Wallet<S> {
         }
 
         // Add new synced outputs
-        for output_data in unspent_outputs {
+        for unspent_output_data in unspent_outputs_data {
             // Insert output, if it's unknown emit the NewOutputEvent
             if wallet_ledger
                 .outputs
-                .insert(output_data.output_id, output_data.clone())
+                .insert(unspent_output_data.output_id, unspent_output_data.clone())
                 .is_none()
             {
                 #[cfg(feature = "events")]
                 {
                     let transaction = wallet_ledger
                         .incoming_transactions
-                        .get(output_data.output_id.transaction_id());
+                        .get(unspent_output_data.output_id.transaction_id());
                     self.emit(WalletEvent::NewOutput(Box::new(NewOutputEvent {
-                        output: output_data.clone(),
+                        output: unspent_output_data.clone(),
                         transaction: transaction
                             .as_ref()
                             .map(|tx| SignedTransactionPayloadDto::from(&tx.payload)),
@@ -138,8 +138,10 @@ impl<S: 'static + SecretManage> Wallet<S> {
                     .await;
                 }
             };
-            if !output_data.is_spent() {
-                wallet_ledger.unspent_outputs.insert(output_data.output_id, output_data);
+            if !unspent_output_data.is_spent() {
+                wallet_ledger
+                    .unspent_outputs
+                    .insert(unspent_output_data.output_id, unspent_output_data);
             }
         }