diff --git a/sdk/examples/wallet/logger.rs b/sdk/examples/wallet/logger.rs index 907a75120d..d058344abf 100644 --- a/sdk/examples/wallet/logger.rs +++ b/sdk/examples/wallet/logger.rs @@ -9,11 +9,7 @@ //! ``` use iota_sdk::{ - client::{ - api::GetAddressesOptions, - constants::SHIMMER_COIN_TYPE, - secret::{mnemonic::MnemonicSecretManager, SecretManager}, - }, + client::{constants::SHIMMER_COIN_TYPE, secret::mnemonic::MnemonicSecretManager}, crypto::keys::bip44::Bip44, wallet::{ClientOptions, Wallet}, }; @@ -39,9 +35,6 @@ async fn main() -> Result<(), Box> { // Restore a wallet let client_options = ClientOptions::new().with_node(&std::env::var("NODE_URL").unwrap())?; - let secret_manager = SecretManager::Mnemonic(MnemonicSecretManager::try_from_mnemonic( - std::env::var("MNEMONIC").unwrap(), - )?); let secret_manager = MnemonicSecretManager::try_from_mnemonic(std::env::var("MNEMONIC").unwrap())?; diff --git a/sdk/examples/wallet/offline_signing/1_prepare_transaction.rs b/sdk/examples/wallet/offline_signing/1_prepare_transaction.rs index 93c374fc96..fdae78ef91 100644 --- a/sdk/examples/wallet/offline_signing/1_prepare_transaction.rs +++ b/sdk/examples/wallet/offline_signing/1_prepare_transaction.rs @@ -9,10 +9,7 @@ //! ``` use iota_sdk::{ - client::{ - api::PreparedTransactionDataDto, constants::SHIMMER_COIN_TYPE, secret::SecretManager, - stronghold::StrongholdAdapter, - }, + client::{api::PreparedTransactionDataDto, constants::SHIMMER_COIN_TYPE, secret::SecretManager}, crypto::keys::bip44::Bip44, types::block::address::Bech32Address, wallet::{ClientOptions, SendParams, Wallet}, diff --git a/sdk/examples/wallet/spammer.rs b/sdk/examples/wallet/spammer.rs index 52786a6f73..70fc0aaf88 100644 --- a/sdk/examples/wallet/spammer.rs +++ b/sdk/examples/wallet/spammer.rs @@ -12,14 +12,10 @@ use iota_sdk::{ client::{ constants::SHIMMER_COIN_TYPE, request_funds_from_faucet, - secret::{mnemonic::MnemonicSecretManager, SecretManage, SecretManager}, + secret::{mnemonic::MnemonicSecretManager, SecretManager}, }, crypto::keys::bip44::Bip44, - types::block::{ - address::{Address, Bech32Address, Hrp}, - output::BasicOutput, - payload::signed_transaction::TransactionId, - }, + types::block::{address::Bech32Address, output::BasicOutput, payload::signed_transaction::TransactionId}, wallet::{ClientOptions, FilterOptions, SendParams, Wallet}, }; diff --git a/sdk/src/client/api/block_builder/transaction_builder/mod.rs b/sdk/src/client/api/block_builder/transaction_builder/mod.rs index 75bf51aba0..70b1e185d7 100644 --- a/sdk/src/client/api/block_builder/transaction_builder/mod.rs +++ b/sdk/src/client/api/block_builder/transaction_builder/mod.rs @@ -10,11 +10,11 @@ pub(crate) mod remainder; pub(crate) mod requirement; pub(crate) mod transition; -use alloc::collections::BTreeMap; +use alloc::collections::{BTreeMap, VecDeque}; +use core::borrow::Borrow; use std::collections::{HashMap, HashSet}; use crypto::keys::bip44::Bip44; -use packable::PackableExt; pub use self::{burn::Burn, error::TransactionBuilderError, requirement::Requirement}; use crate::{ @@ -40,7 +40,7 @@ use crate::{ signed_transaction::{Transaction, TransactionCapabilities, TransactionCapabilityFlag}, TaggedDataPayload, }, - protocol::{CommittableAgeRange, ProtocolParameters}, + protocol::ProtocolParameters, slot::{SlotCommitmentId, SlotIndex}, }, }; @@ -178,7 +178,7 @@ impl Client { pub struct TransactionBuilder { available_inputs: Vec, required_inputs: HashSet, - selected_inputs: Vec, + selected_inputs: OrderedInputs, bic_context_inputs: HashSet, commitment_context_input: Option, reward_context_inputs: HashSet, @@ -250,7 +250,7 @@ impl TransactionBuilder { Self { available_inputs, required_inputs: HashSet::new(), - selected_inputs: Vec::new(), + selected_inputs: Default::default(), bic_context_inputs: HashSet::new(), commitment_context_input: None, reward_context_inputs: HashSet::new(), @@ -308,7 +308,6 @@ impl TransactionBuilder { } // Gets requirements from outputs. - // TODO this may re-evaluate outputs added by inputs self.outputs_requirements(); // Gets requirements from burn. @@ -426,12 +425,6 @@ impl TransactionBuilder { } } - let inputs_data = Self::sort_input_signing_data( - self.selected_inputs, - self.latest_slot_commitment_id.slot_index(), - self.protocol_parameters.committable_age_range(), - )?; - let mut inputs: Vec = Vec::new(); let mut context_inputs = self .bic_context_inputs @@ -440,7 +433,7 @@ impl TransactionBuilder { .chain(self.commitment_context_input.map(ContextInput::from)) .collect::>(); - for (idx, input) in inputs_data.iter().enumerate() { + for (idx, input) in self.selected_inputs.iter().enumerate() { inputs.push(Input::Utxo(UtxoInput::from(*input.output_id()))); if self.reward_context_inputs.contains(input.output_id()) { context_inputs.push(RewardContextInput::new(idx as u16).unwrap().into()); @@ -471,7 +464,7 @@ impl TransactionBuilder { let data = PreparedTransactionData { transaction, - inputs_data, + inputs_data: self.selected_inputs.into_sorted_iter().collect(), remainders: self.remainders.data, mana_rewards: self.mana_rewards.into_iter().collect(), }; @@ -503,9 +496,18 @@ impl TransactionBuilder { // New input may need context inputs self.fulfill_context_inputs_requirements(&input); - self.selected_inputs.push(input); + let required_address = input + .output + .required_address( + self.latest_slot_commitment_id.slot_index(), + self.protocol_parameters.committable_age_range(), + ) + // PANIC: safe to unwrap as non basic/account/foundry/nft/delegation outputs are already filtered out. + .unwrap() + .expect("expiration unlockable outputs already filtered out"); + self.selected_inputs.insert(required_address, input); - Ok(if added_output { self.added_outputs.last() } else { None }) + Ok(added_output.then(|| self.added_outputs.last().unwrap())) } /// Sets the required inputs of an [`TransactionBuilder`]. @@ -684,99 +686,113 @@ impl TransactionBuilder { } }) } +} - // Inputs need to be sorted before signing, because the reference unlock conditions can only reference a lower index - pub(crate) fn sort_input_signing_data( - mut inputs: Vec, - commitment_slot_index: SlotIndex, - committable_age_range: CommittableAgeRange, - ) -> Result, TransactionBuilderError> { - // initially sort by output to make it deterministic - // TODO: rethink this, we only need it deterministic for tests, for the protocol it doesn't matter, also there - // might be a more efficient way to do this - inputs.sort_by_key(|i| i.output.pack_to_vec()); - // filter for ed25519 address first - let (mut sorted_inputs, account_nft_address_inputs): (Vec, Vec) = - inputs.into_iter().partition(|input_signing_data| { - let required_address = input_signing_data - .output - .required_address(commitment_slot_index, committable_age_range) - // PANIC: safe to unwrap as non basic/account/foundry/nft outputs are already filtered out. - .unwrap() - .expect("expiration unlockable outputs already filtered out"); +#[derive(Clone, Debug, Default)] +pub(crate) struct OrderedInputs { + ed25519: VecDeque, + other: BTreeMap>, + len: usize, +} - required_address.is_ed25519() - }); +impl OrderedInputs { + pub(crate) fn sorted_iter(&self) -> OrderedInputsIter<&Address, &InputSigningData> { + OrderedInputsIter { + queue: self.ed25519.iter().collect(), + other: self.other.iter().map(|(k, v)| (k, v.iter().collect())).collect(), + } + } - for input in account_nft_address_inputs { - let required_address = input - .output - .required_address(commitment_slot_index, committable_age_range)? - .expect("expiration unlockable outputs already filtered out"); + pub(crate) fn into_sorted_iter(self) -> OrderedInputsIter { + OrderedInputsIter { + queue: self.ed25519, + other: self.other, + } + } - match sorted_inputs - .iter() - .position(|input_signing_data| match required_address { - Address::Account(unlock_address) => { - if let Output::Account(account_output) = &input_signing_data.output { - *unlock_address.account_id() - == account_output.account_id_non_null(input_signing_data.output_id()) - } else { - false - } - } - Address::Nft(unlock_address) => { - if let Output::Nft(nft_output) = &input_signing_data.output { - *unlock_address.nft_id() == nft_output.nft_id_non_null(input_signing_data.output_id()) - } else { - false - } + pub(crate) fn iter(&self) -> impl Iterator + Clone { + self.ed25519.iter().chain(self.other.values().flatten()) + } + + pub(crate) fn insert(&mut self, required_address: Address, input: InputSigningData) { + if required_address.is_ed25519_backed() { + self.ed25519.push_back(input); + } else { + self.other.entry(required_address).or_default().push_back(input); + } + self.len += 1; + } + + pub(crate) fn len(&self) -> usize { + self.len + } + + pub(crate) fn is_empty(&self) -> bool { + self.len() == 0 + } +} + +#[derive(Clone, Debug)] +pub(crate) struct OrderedInputsIter + Ord + core::hash::Hash, I: Borrow> { + queue: VecDeque, + other: BTreeMap>, +} + +impl + Ord + core::hash::Hash, I: Borrow> Iterator for OrderedInputsIter { + type Item = I; + + fn next(&mut self) -> Option { + // Inputs that are unlocked by Ed25519 addresses go in the queue first + // because they do not need to reference other inputs for their unlocks. + // Each one may have additional dependents which are added to the front of + // the queue to be sorted immediately after the input they depend upon. + // Those can also have dependents which will go after them. + // This creates a tree structure with many to one relationship, which is + // flattened by this loop in insertion order. + if let Some(input) = self.queue.pop_front() { + // Add associated inputs to the front of the queue + match &input.borrow().output { + Output::Account(account_output) => { + for input in self + .other + .remove(&Address::Account(AccountAddress::new( + account_output.account_id_non_null(input.borrow().output_id()), + ))) + .into_iter() + .flatten() + .rev() + { + self.queue.push_front(input); } - _ => false, - }) { - Some(position) => { - // Insert after the output we need - sorted_inputs.insert(position + 1, input); } - None => { - // insert before address - let account_or_nft_address = match &input.output { - Output::Account(account_output) => Some(Address::Account(AccountAddress::new( - account_output.account_id_non_null(input.output_id()), - ))), - Output::Nft(nft_output) => Some(Address::Nft(NftAddress::new( - nft_output.nft_id_non_null(input.output_id()), - ))), - _ => None, - }; - - if let Some(account_or_nft_address) = account_or_nft_address { - // Check for existing outputs for this address, and insert before - match sorted_inputs.iter().position(|input_signing_data| { - let required_address = input_signing_data - .output - .required_address(commitment_slot_index, committable_age_range) - // PANIC: safe to unwrap as non basic/alias/foundry/nft outputs are already filtered - .unwrap() - .expect("expiration unlockable outputs already filtered out"); - - required_address == account_or_nft_address - }) { - Some(position) => { - // Insert before the output with this address required for unlocking - sorted_inputs.insert(position, input); - } - // just push output - None => sorted_inputs.push(input), - } - } else { - // just push basic or foundry output - sorted_inputs.push(input); + Output::Nft(nft_output) => { + for input in self + .other + .remove(&Address::Nft(NftAddress::new( + nft_output.nft_id_non_null(input.borrow().output_id()), + ))) + .into_iter() + .flatten() + .rev() + { + self.queue.push_front(input); } } + _ => (), + }; + return Some(input); + } + // When the queue is empty, just add anything that is left over to the end of the list. + if let Some(mut entry) = self.other.first_entry() { + if let Some(input) = entry.get_mut().pop_front() { + // Since the structure is a list-of-lists, we need to pop + // the inner list if it's empty. + if entry.get().is_empty() { + self.other.pop_first(); + } + return Some(input); } } - - Ok(sorted_inputs) + None } } diff --git a/sdk/src/client/api/block_builder/transaction_builder/remainder.rs b/sdk/src/client/api/block_builder/transaction_builder/remainder.rs index 7111bdfb7b..ecdd651693 100644 --- a/sdk/src/client/api/block_builder/transaction_builder/remainder.rs +++ b/sdk/src/client/api/block_builder/transaction_builder/remainder.rs @@ -30,7 +30,7 @@ impl TransactionBuilder { Ok(()) } - /// Gets the remainder address from configuration of finds one from the inputs. + /// Gets the remainder address from configuration or finds one from the inputs. pub(crate) fn get_remainder_address(&self) -> Result)>, TransactionBuilderError> { if let Some(remainder_address) = &self.remainders.address { // Search in inputs for the Bip44 chain for the remainder address, so the ledger can regenerate it @@ -50,7 +50,7 @@ impl TransactionBuilder { return Ok(Some((remainder_address.clone(), None))); } - for input in &self.selected_inputs { + for input in self.selected_inputs.iter() { let required_address = input .output .required_address( diff --git a/sdk/src/client/api/block_builder/transaction_builder/requirement/amount.rs b/sdk/src/client/api/block_builder/transaction_builder/requirement/amount.rs index ad2ae9f372..1abaf3ce74 100644 --- a/sdk/src/client/api/block_builder/transaction_builder/requirement/amount.rs +++ b/sdk/src/client/api/block_builder/transaction_builder/requirement/amount.rs @@ -91,7 +91,7 @@ impl TransactionBuilder { let mut inputs_sdr = HashMap::new(); let mut outputs_sdr = HashMap::new(); - for selected_input in &self.selected_inputs { + for selected_input in self.selected_inputs.iter() { inputs_sum += selected_input.output.amount(); if let Some(sdruc) = sdruc_not_expired(&selected_input.output, self.latest_slot_commitment_id.slot_index()) diff --git a/sdk/src/client/api/block_builder/transaction_builder/requirement/mana.rs b/sdk/src/client/api/block_builder/transaction_builder/requirement/mana.rs index 58a0b8989f..d98500d904 100644 --- a/sdk/src/client/api/block_builder/transaction_builder/requirement/mana.rs +++ b/sdk/src/client/api/block_builder/transaction_builder/requirement/mana.rs @@ -37,15 +37,9 @@ impl TransactionBuilder { let mut should_recalculate = false; if !self.selected_inputs.is_empty() && self.all_outputs().next().is_some() { - self.selected_inputs = Self::sort_input_signing_data( - std::mem::take(&mut self.selected_inputs), - self.latest_slot_commitment_id.slot_index(), - self.protocol_parameters.committable_age_range(), - )?; - let inputs = self .selected_inputs - .iter() + .sorted_iter() .map(|i| Input::Utxo(UtxoInput::from(*i.output_id()))); let outputs = self.all_outputs().cloned(); @@ -311,7 +305,7 @@ impl TransactionBuilder { .into() .unwrap_or_else(|| self.burn.as_ref().map_or(true, |b| !b.generated_mana())); - for input in &self.selected_inputs { + for input in self.selected_inputs.iter() { selected_mana += self.total_mana(input, include_generated)?; } diff --git a/sdk/src/client/api/block_builder/transaction_builder/requirement/native_tokens.rs b/sdk/src/client/api/block_builder/transaction_builder/requirement/native_tokens.rs index 5595928a10..b65efdd25c 100644 --- a/sdk/src/client/api/block_builder/transaction_builder/requirement/native_tokens.rs +++ b/sdk/src/client/api/block_builder/transaction_builder/requirement/native_tokens.rs @@ -139,7 +139,7 @@ impl TransactionBuilder { let TokenScheme::Simple(output_foundry_simple_ts) = output_foundry.token_scheme(); let mut initial_creation = true; - for input in &self.selected_inputs { + for input in self.selected_inputs.iter() { if let Output::Foundry(input_foundry) = &input.output { let token_id = output_foundry.token_id(); diff --git a/sdk/tests/client/secret_manager/address_generation.rs b/sdk/tests/client/secret_manager/address_generation.rs index 7ff9584cce..85ef64d7c6 100644 --- a/sdk/tests/client/secret_manager/address_generation.rs +++ b/sdk/tests/client/secret_manager/address_generation.rs @@ -3,26 +3,21 @@ #[cfg(feature = "stronghold")] use crypto::keys::bip39::Mnemonic; -use crypto::keys::bip44::Bip44; +#[cfg(feature = "ledger_nano")] +use iota_sdk::client::secret::ledger_nano::LedgerSecretManager; #[cfg(feature = "stronghold")] use iota_sdk::client::secret::stronghold::StrongholdSecretManager; -#[cfg(feature = "ledger_nano")] -use iota_sdk::client::secret::{ledger_nano::LedgerSecretManager, GenerateAddressOptions}; -#[cfg(feature = "events")] -use iota_sdk::wallet::events::{WalletEvent, WalletEventType}; use iota_sdk::{ client::{ - api::GetAddressesOptions, constants::{IOTA_COIN_TYPE, SHIMMER_COIN_TYPE}, secret::{mnemonic::MnemonicSecretManager, SecretManager}, ClientError, }, - types::block::address::{Hrp, ToBech32Ext}, - wallet::{ClientOptions, Wallet}, + types::block::address::ToBech32Ext, }; use pretty_assertions::assert_eq; -use crate::client::common::{setup, tear_down, DEFAULT_MNEMONIC, NODE_LOCAL}; +use crate::client::common::{setup, tear_down, DEFAULT_MNEMONIC}; #[tokio::test] async fn address_generation_mnemonic() -> Result<(), Box> { diff --git a/sdk/tests/client/secret_manager/mnemonic.rs b/sdk/tests/client/secret_manager/mnemonic.rs index 84cf4e6cbe..aad323aa43 100644 --- a/sdk/tests/client/secret_manager/mnemonic.rs +++ b/sdk/tests/client/secret_manager/mnemonic.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use iota_sdk::client::{ - api::GetAddressesOptions, constants::{SHIMMER_COIN_TYPE, SHIMMER_TESTNET_BECH32_HRP}, secret::SecretManager, ClientError, diff --git a/sdk/tests/client/secret_manager/stronghold.rs b/sdk/tests/client/secret_manager/stronghold.rs index bb087859ee..7f304d4b15 100644 --- a/sdk/tests/client/secret_manager/stronghold.rs +++ b/sdk/tests/client/secret_manager/stronghold.rs @@ -2,7 +2,6 @@ // SPDX-License-Identifier: Apache-2.0 use iota_sdk::client::{ - api::GetAddressesOptions, constants::{SHIMMER_COIN_TYPE, SHIMMER_TESTNET_BECH32_HRP}, secret::SecretManager, ClientError,