From eddac2620a5b6a28133262e815b9c8b77ea69084 Mon Sep 17 00:00:00 2001 From: Thoralf-M <46689931+Thoralf-M@users.noreply.github.com> Date: Thu, 26 Oct 2023 18:59:23 +0200 Subject: [PATCH] Improve syncing and fix deadlock (#1510) * Improve syncing * Fix potential deadlock * format * Update changelog and version * Remove more self.reads() * addresses() on AccountDetails * Logs --------- Co-authored-by: Thibault Martinez --- Cargo.lock | 2 +- bindings/nodejs/CHANGELOG.md | 7 +++ bindings/nodejs/package.json | 2 +- sdk/CHANGELOG.md | 4 +- sdk/Cargo.toml | 2 +- sdk/src/wallet/account/mod.rs | 13 +++-- sdk/src/wallet/account/operations/balance.rs | 12 +++-- .../account/operations/output_claiming.rs | 49 +++++++++++++------ .../account/operations/participation/mod.rs | 26 ++++++---- 9 files changed, 80 insertions(+), 37 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b7099d2f71..5585bea127 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1616,7 +1616,7 @@ dependencies = [ [[package]] name = "iota-sdk" -version = "1.1.1" +version = "1.1.2" dependencies = [ "anymap", "async-trait", diff --git a/bindings/nodejs/CHANGELOG.md b/bindings/nodejs/CHANGELOG.md index 67686b21a5..a9f11477c9 100644 --- a/bindings/nodejs/CHANGELOG.md +++ b/bindings/nodejs/CHANGELOG.md @@ -19,6 +19,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security --> +## 1.1.2 - 2023-10-26 + +### Fixed + +- Slow syncing with many claimable outputs; +- Potential deadlock in syncing; + ## 1.1.1 - 2023-10-11 ### Added diff --git a/bindings/nodejs/package.json b/bindings/nodejs/package.json index 778818d180..f2cdc767da 100644 --- a/bindings/nodejs/package.json +++ b/bindings/nodejs/package.json @@ -1,6 +1,6 @@ { "name": "@iota/sdk", - "version": "1.1.1", + "version": "1.1.2", "description": "Node.js binding to the IOTA SDK library", "main": "out/index.js", "types": "out/index.d.ts", diff --git a/sdk/CHANGELOG.md b/sdk/CHANGELOG.md index 902376c243..f6cd51ec84 100644 --- a/sdk/CHANGELOG.md +++ b/sdk/CHANGELOG.md @@ -19,7 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Security --> -## 1.1.2 - 2023-MM-DD +## 1.1.2 - 2023-10-26 ### Added @@ -32,6 +32,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `Account::claim_outputs()` if an input has less amount than min storage deposit; - URLs aren't truncated after the hostname anymore; - Ledger nano potentially failing to identify the correct remainder output; +- Slow syncing with many claimable outputs; +- Potential deadlock in syncing; ## 1.1.1 - 2023-10-11 diff --git a/sdk/Cargo.toml b/sdk/Cargo.toml index 4edfac9bf7..8a77cdec8e 100644 --- a/sdk/Cargo.toml +++ b/sdk/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "iota-sdk" -version = "1.1.1" +version = "1.1.2" authors = ["IOTA Stiftung"] edition = "2021" description = "The IOTA SDK provides developers with a seamless experience to develop on IOTA by providing account abstractions and clients to interact with node APIs." diff --git a/sdk/src/wallet/account/mod.rs b/sdk/src/wallet/account/mod.rs index e938d54394..e717b9207b 100644 --- a/sdk/src/wallet/account/mod.rs +++ b/sdk/src/wallet/account/mod.rs @@ -294,9 +294,7 @@ impl AccountInner { /// Returns all addresses of the account pub async fn addresses(&self) -> Result> { let account_details = self.details().await; - let mut all_addresses = account_details.public_addresses().clone(); - all_addresses.extend(account_details.internal_addresses().clone()); - Ok(all_addresses.to_vec()) + Ok(account_details.addresses()) } /// Returns all public addresses of the account @@ -445,6 +443,15 @@ impl AccountInner { } } +impl AccountDetails { + /// Returns all addresses of the account + pub(crate) fn addresses(&self) -> Vec { + let mut all_addresses = self.public_addresses().clone(); + all_addresses.extend(self.internal_addresses().clone()); + all_addresses.to_vec() + } +} + pub(crate) fn build_transaction_from_payload_and_inputs( tx_id: TransactionId, tx_payload: TransactionPayload, diff --git a/sdk/src/wallet/account/operations/balance.rs b/sdk/src/wallet/account/operations/balance.rs index 0b52770a77..2a9b18f955 100644 --- a/sdk/src/wallet/account/operations/balance.rs +++ b/sdk/src/wallet/account/operations/balance.rs @@ -63,12 +63,17 @@ where ) -> Result { let network_id = self.client().get_network_id().await?; let rent_structure = self.client().get_rent_structure().await?; + let local_time = self.client().get_time_checked().await?; let mut balance = Balance::default(); let mut total_rent_amount = 0; let mut total_native_tokens = NativeTokensBuilder::default(); #[cfg(feature = "participation")] - let voting_output = self.get_voting_output().await?; + let voting_output = account_details.get_voting_output()?; + + let account_addresses = account_details.addresses(); + + let claimable_outputs = account_details.claimable_outputs(OutputsToClaim::All, local_time)?; for address_with_unspent_outputs in addresses_with_unspent_outputs { #[cfg(feature = "participation")] @@ -163,10 +168,7 @@ where // if we have multiple unlock conditions for basic or nft outputs, then we might can't // spend the balance at the moment or in the future - let account_addresses = self.addresses().await?; - let local_time = self.client().get_time_checked().await?; - let is_claimable = - self.claimable_outputs(OutputsToClaim::All).await?.contains(output_id); + let is_claimable = claimable_outputs.contains(output_id); // For outputs that are expired or have a timelock unlock condition, but no expiration // unlock condition and we then can unlock them, then diff --git a/sdk/src/wallet/account/operations/output_claiming.rs b/sdk/src/wallet/account/operations/output_claiming.rs index 37ea891d15..f958837b2a 100644 --- a/sdk/src/wallet/account/operations/output_claiming.rs +++ b/sdk/src/wallet/account/operations/output_claiming.rs @@ -16,7 +16,7 @@ use crate::{ }, }, wallet::account::{ - operations::helpers::time::can_output_be_unlocked_now, types::Transaction, Account, OutputData, + operations::helpers::time::can_output_be_unlocked_now, types::Transaction, Account, AccountDetails, OutputData, TransactionOptions, }, }; @@ -32,31 +32,29 @@ pub enum OutputsToClaim { All, } -impl Account -where - crate::wallet::Error: From, -{ +impl AccountDetails { /// Get basic and nft outputs that have /// [`ExpirationUnlockCondition`](crate::types::block::output::unlock_condition::ExpirationUnlockCondition), /// [`StorageDepositReturnUnlockCondition`] or /// [`TimelockUnlockCondition`](crate::types::block::output::unlock_condition::TimelockUnlockCondition) and can be /// unlocked now and also get basic outputs with only an [`AddressUnlockCondition`] unlock condition, for /// additional inputs - pub async fn claimable_outputs(&self, outputs_to_claim: OutputsToClaim) -> crate::wallet::Result> { - log::debug!("[OUTPUT_CLAIMING] claimable_outputs"); - let account_details = self.details().await; - - let local_time = self.client().get_time_checked().await?; + pub(crate) fn claimable_outputs( + &self, + outputs_to_claim: OutputsToClaim, + time: u32, + ) -> crate::wallet::Result> { + log::debug!("[AccountDetails] claimable_outputs"); // Get outputs for the claim let mut output_ids_to_claim: HashSet = HashSet::new(); - for (output_id, output_data) in account_details + for (output_id, output_data) in self .unspent_outputs .iter() .filter(|(_, o)| o.output.is_basic() || o.output.is_nft()) { // Don't use outputs that are locked for other transactions - if !account_details.locked_outputs.contains(output_id) && account_details.outputs.contains_key(output_id) { + if !self.locked_outputs.contains(output_id) && self.outputs.contains_key(output_id) { if let Some(unlock_conditions) = output_data.output.unlock_conditions() { // If there is a single [UnlockCondition], then it's an // [AddressUnlockCondition] and we own it already without @@ -65,11 +63,11 @@ where && can_output_be_unlocked_now( // We use the addresses with unspent outputs, because other addresses of the // account without unspent outputs can't be related to this output - &account_details.addresses_with_unspent_outputs, + &self.addresses_with_unspent_outputs, // outputs controlled by an alias or nft are currently not considered &[], output_data, - local_time, + time, // Not relevant without alias addresses None, )? @@ -78,7 +76,7 @@ where OutputsToClaim::MicroTransactions => { if let Some(sdr) = unlock_conditions.storage_deposit_return() { // If expired, it's not a micro transaction anymore - if unlock_conditions.is_expired(local_time) { + if unlock_conditions.is_expired(time) { continue; } // Only micro transaction if not the same @@ -99,7 +97,7 @@ where } OutputsToClaim::Amount => { let mut claimable_amount = output_data.output.amount(); - if !unlock_conditions.is_expired(local_time) { + if !unlock_conditions.is_expired(time) { claimable_amount -= unlock_conditions .storage_deposit_return() .map(|s| s.amount()) @@ -123,6 +121,25 @@ where ); Ok(output_ids_to_claim.into_iter().collect()) } +} + +impl Account +where + crate::wallet::Error: From, +{ + /// Get basic and nft outputs that have + /// [`ExpirationUnlockCondition`](crate::types::block::output::unlock_condition::ExpirationUnlockCondition), + /// [`StorageDepositReturnUnlockCondition`] or + /// [`TimelockUnlockCondition`](crate::types::block::output::unlock_condition::TimelockUnlockCondition) and can be + /// unlocked now and also get basic outputs with only an [`AddressUnlockCondition`] unlock condition, for + /// additional inputs + pub async fn claimable_outputs(&self, outputs_to_claim: OutputsToClaim) -> crate::wallet::Result> { + let account_details = self.details().await; + + let local_time = self.client().get_time_checked().await?; + + account_details.claimable_outputs(outputs_to_claim, local_time) + } /// Get basic outputs that have only one unlock condition which is [AddressUnlockCondition], so they can be used as /// additional inputs diff --git a/sdk/src/wallet/account/operations/participation/mod.rs b/sdk/src/wallet/account/operations/participation/mod.rs index 4939ddf933..1637d84b65 100644 --- a/sdk/src/wallet/account/operations/participation/mod.rs +++ b/sdk/src/wallet/account/operations/participation/mod.rs @@ -26,7 +26,7 @@ use crate::{ block::output::{unlock_condition::UnlockCondition, Output, OutputId}, }, wallet::{ - account::{Account, OutputData}, + account::{Account, AccountDetails, OutputData}, task, Result, }, }; @@ -231,14 +231,7 @@ where /// /// If multiple outputs with this tag exist, the one with the largest amount will be returned. pub async fn get_voting_output(&self) -> Result> { - log::debug!("[get_voting_output]"); - Ok(self - .unspent_outputs(None) - .await? - .iter() - .filter(|output_data| is_valid_participation_output(&output_data.output)) - .max_by_key(|output_data| output_data.output.amount()) - .cloned()) + self.details().await.get_voting_output() } /// Gets client for an event. @@ -303,6 +296,21 @@ where } } +impl AccountDetails { + /// Returns the voting output ("PARTICIPATION" tag). + /// + /// If multiple outputs with this tag exist, the one with the largest amount will be returned. + pub(crate) fn get_voting_output(&self) -> Result> { + log::debug!("[get_voting_output]"); + Ok(self + .unspent_outputs + .values() + .filter(|output_data| is_valid_participation_output(&output_data.output)) + .max_by_key(|output_data| output_data.output.amount()) + .cloned()) + } +} + fn is_valid_participation_output(output: &Output) -> bool { // Only basic outputs can be participation outputs. if let Output::Basic(basic_output) = &output {