From bad089439668bbddae686f1b9f2584d9c3c80c47 Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 21 Sep 2023 16:17:22 -0600 Subject: [PATCH 1/5] update send_to_addresses name --- zingolib/src/lightclient.rs | 4 ++-- zingolib/src/wallet.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/zingolib/src/lightclient.rs b/zingolib/src/lightclient.rs index 480aa38e33..1a90cda924 100644 --- a/zingolib/src/lightclient.rs +++ b/zingolib/src/lightclient.rs @@ -1015,7 +1015,7 @@ impl LightClient { let prover = LocalTxProver::from_bytes(&sapling_spend, &sapling_output); self.wallet - .send_to_address( + .send_to_addresses( prover, vec![crate::wallet::Pool::Orchard, crate::wallet::Pool::Sapling], address_amount_memo_tuples, @@ -1079,7 +1079,7 @@ impl LightClient { let prover = LocalTxProver::from_bytes(&sapling_spend, &sapling_output); self.wallet - .send_to_address( + .send_to_addresses( prover, pools_to_shield.to_vec(), vec![(&addr, balance_to_shield - fee, None)], diff --git a/zingolib/src/wallet.rs b/zingolib/src/wallet.rs index 02a5dd7d1b..10a2ae290a 100644 --- a/zingolib/src/wallet.rs +++ b/zingolib/src/wallet.rs @@ -919,7 +919,7 @@ impl LightWallet { ) } - pub async fn send_to_address( + pub async fn send_to_addresses( &self, prover: P, policy: NoteSelectionPolicy, @@ -936,7 +936,7 @@ impl LightWallet { // Call the internal function match self - .send_to_address_inner(prover, policy, tos, submission_height, broadcast_fn) + .send_to_addresses_inner(prover, policy, tos, submission_height, broadcast_fn) .await { Ok((transaction_id, raw_transaction)) => { @@ -950,7 +950,7 @@ impl LightWallet { } } - async fn send_to_address_inner( + async fn send_to_addresses_inner( &self, prover: P, policy: NoteSelectionPolicy, From 4c55031346db492493ebf4537b204be45119abb1 Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 21 Sep 2023 17:25:07 -0600 Subject: [PATCH 2/5] factor builder spend addition into fn --- zingolib/src/wallet.rs | 199 ++++++++++++++++++++++------------------- 1 file changed, 109 insertions(+), 90 deletions(-) diff --git a/zingolib/src/wallet.rs b/zingolib/src/wallet.rs index 10a2ae290a..1183a1dd0f 100644 --- a/zingolib/src/wallet.rs +++ b/zingolib/src/wallet.rs @@ -950,94 +950,18 @@ impl LightWallet { } } - async fn send_to_addresses_inner( + async fn load_transaction_builder_spends( &self, - prover: P, - policy: NoteSelectionPolicy, - tos: Vec<(&str, u64, Option)>, submission_height: BlockHeight, - broadcast_fn: F, - ) -> Result<(String, Vec), String> - where - F: Fn(Box<[u8]>) -> Fut, - Fut: Future>, - { - let start_time = now(); - if tos.is_empty() { - return Err("Need at least one destination address".to_string()); - } - - if !self.wallet_capability().can_spend_from_all_pools() { - // Creating transactions in context of all possible combinations - // of wallet capabilities requires a rigorous case study - // and can have undesired effects if not implemented properly. - // - // Thus we forbid spending for wallets without complete spending capability for now - return Err("Wallet is in watch-only mode and thus it cannot spend.".to_string()); - } - - let total_value = tos.iter().map(|to| to.1).sum::(); - info!( - "0: Creating transaction sending {} zatoshis to {} addresses", - total_value, - tos.len() - ); - - // Convert address (str) to RecipientAddress and value to Amount - let recipients = tos - .iter() - .map(|to| { - let ra = match address::RecipientAddress::decode( - &self.transaction_context.config.chain, - to.0, - ) { - Some(to) => to, - None => { - let e = format!("Invalid recipient address: '{}'", to.0); - error!("{}", e); - return Err(e); - } - }; - - let value = Amount::from_u64(to.1).unwrap(); - - Ok((ra, value, to.2.clone())) - }) - .collect::)>, String>>( - )?; - - let destination_uas = recipients - .iter() - .filter_map(|recipient| match recipient.0 { - address::RecipientAddress::Shielded(_) => None, - address::RecipientAddress::Transparent(_) => None, - address::RecipientAddress::Unified(ref ua) => Some(ua.clone()), - }) - .collect::>(); - - // Select notes to cover the target value - info!("{}: Selecting notes", now() - start_time); - - let target_amount = (Amount::from_u64(total_value).unwrap() + MINIMUM_FEE).unwrap(); - - let (orchard_notes, sapling_notes, utxos, selected_value) = - self.select_notes_and_utxos(target_amount, policy).await; - if selected_value < target_amount { - let e = format!( - "Insufficient verified shielded funds. Have {} zats, need {} zats. NOTE: funds need at least {} confirmations before they can be spent. Transparent funds must be shielded before they can be spent. If you are trying to spend transparent funds, please use the shield button and try again in a few minutes.", - u64::from(selected_value), u64::from(target_amount), self.transaction_context.config - .reorg_buffer_offset + 1 - ); - error!("{}", e); - return Err(e); - } - info!("Selected notes worth {}", u64::from(selected_value)); + orchard_notes: &[SpendableOrchardNote], + sapling_notes: &[SpendableSaplingNote], + utxos: &[ReceivedTransparentOutput], + ) -> Result, String> { let txmds_readlock = self .transaction_context .transaction_metadata_set .read() .await; - let witness_trees = txmds_readlock .witness_trees .as_ref() @@ -1050,14 +974,6 @@ impl LightWallet { submission_height, Some(orchard_anchor), ); - info!( - "{}: Adding {} sapling notes, {} orchard notes, and {} utxos", - now() - start_time, - sapling_notes.len(), - orchard_notes.len(), - utxos.len() - ); - // Add all tinputs // Create a map from address -> sk for all taddrs, so we can spend from the // right address @@ -1136,6 +1052,110 @@ impl LightWallet { return Err(e); } } + drop(txmds_readlock); + Ok(builder) + } + + async fn send_to_addresses_inner( + &self, + prover: P, + policy: NoteSelectionPolicy, + tos: Vec<(&str, u64, Option)>, + submission_height: BlockHeight, + broadcast_fn: F, + ) -> Result<(String, Vec), String> + where + F: Fn(Box<[u8]>) -> Fut, + Fut: Future>, + { + let start_time = now(); + if tos.is_empty() { + return Err("Need at least one destination address".to_string()); + } + + if !self.wallet_capability().can_spend_from_all_pools() { + // Creating transactions in context of all possible combinations + // of wallet capabilities requires a rigorous case study + // and can have undesired effects if not implemented properly. + // + // Thus we forbid spending for wallets without complete spending capability for now + return Err("Wallet is in watch-only mode and thus it cannot spend.".to_string()); + } + + let total_value = tos.iter().map(|to| to.1).sum::(); + info!( + "0: Creating transaction sending {} zatoshis to {} addresses", + total_value, + tos.len() + ); + + // Convert address (str) to RecipientAddress and value to Amount + let recipients = tos + .iter() + .map(|to| { + let ra = match address::RecipientAddress::decode( + &self.transaction_context.config.chain, + to.0, + ) { + Some(to) => to, + None => { + let e = format!("Invalid recipient address: '{}'", to.0); + error!("{}", e); + return Err(e); + } + }; + + let value = Amount::from_u64(to.1).unwrap(); + + Ok((ra, value, to.2.clone())) + }) + .collect::)>, String>>( + )?; + + let destination_uas = recipients + .iter() + .filter_map(|recipient| match recipient.0 { + address::RecipientAddress::Shielded(_) => None, + address::RecipientAddress::Transparent(_) => None, + address::RecipientAddress::Unified(ref ua) => Some(ua.clone()), + }) + .collect::>(); + + // Select notes to cover the target value + info!("{}: Selecting notes", now() - start_time); + + let target_amount = (Amount::from_u64(total_value).unwrap() + MINIMUM_FEE).unwrap(); + + let (orchard_notes, sapling_notes, utxos, selected_value) = + self.select_notes_and_utxos(target_amount, policy).await; + if selected_value < target_amount { + let e = format!( + "Insufficient verified shielded funds. Have {} zats, need {} zats. NOTE: funds need at least {} confirmations before they can be spent. Transparent funds must be shielded before they can be spent. If you are trying to spend transparent funds, please use the shield button and try again in a few minutes.", + u64::from(selected_value), u64::from(target_amount), self.transaction_context.config + .reorg_buffer_offset + 1 + ); + error!("{}", e); + return Err(e); + } + info!("Selected notes worth {}", u64::from(selected_value)); + + info!( + "{}: Adding {} sapling notes, {} orchard notes, and {} utxos", + now() - start_time, + &sapling_notes.len(), + &orchard_notes.len(), + &utxos.len() + ); + + let mut builder = self + .load_transaction_builder_spends( + submission_height, + &orchard_notes, + &sapling_notes, + &utxos, + ) + .await + .expect("To populate a builder with notes."); // We'll use the first ovk to encrypt outgoing transactions let sapling_ovk = @@ -1279,7 +1299,6 @@ impl LightWallet { // Now that we've gotten this far, we need to write // so we drop the readlock - drop(txmds_readlock); // Mark notes as spent. { // Mark sapling notes as unconfirmed spent From c2f3f65945e14d4396da3a548a09c7b85607989a Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 21 Sep 2023 18:06:57 -0600 Subject: [PATCH 3/5] grab lightclient changes from zip317_implementation draft --- zingolib/src/lightclient.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/zingolib/src/lightclient.rs b/zingolib/src/lightclient.rs index 1a90cda924..e007bfebcb 100644 --- a/zingolib/src/lightclient.rs +++ b/zingolib/src/lightclient.rs @@ -1010,14 +1010,18 @@ impl LightClient { let result = { let _lock = self.sync_lock.lock().await; + // I am not clear on how long this operation may take, but it's + // clearly unnecessary in a send that doesn't include sapling + // TODO: Remove from sends that don't include Sapling let (sapling_output, sapling_spend) = self.read_sapling_params()?; - let prover = LocalTxProver::from_bytes(&sapling_spend, &sapling_output); + let sapling_prover = LocalTxProver::from_bytes(&sapling_spend, &sapling_output); self.wallet .send_to_addresses( - prover, - vec![crate::wallet::Pool::Orchard, crate::wallet::Pool::Sapling], + sapling_prover, + vec![crate::wallet::Pool::Orchard, crate::wallet::Pool::Sapling], // This policy doesn't allow + // spend from transparent. address_amount_memo_tuples, transaction_submission_height, |transaction_bytes| { @@ -1044,8 +1048,13 @@ impl LightClient { address: Option, ) -> Result { let transaction_submission_height = self.get_submission_height().await?; - let fee = u64::from(MINIMUM_FEE); - let tbal = self.wallet.tbalance(None).await.unwrap_or(0); + let fee = u64::from(MINIMUM_FEE); // TODO: This can no longer be hard coded, and must be calced + // as a fn of the transactions structure. + let tbal = self + .wallet + .tbalance(None) + .await + .expect("to receive a balance"); let sapling_bal = self .wallet .spendable_sapling_balance(None) From 898d32d4fd348a7bbaa96b464ec7cd82d7266795 Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 21 Sep 2023 18:20:02 -0600 Subject: [PATCH 4/5] correct name to sapling_prover --- zingolib/src/lightclient.rs | 4 ++-- zingolib/src/wallet.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/zingolib/src/lightclient.rs b/zingolib/src/lightclient.rs index e007bfebcb..313033b30e 100644 --- a/zingolib/src/lightclient.rs +++ b/zingolib/src/lightclient.rs @@ -1085,11 +1085,11 @@ impl LightClient { let _lock = self.sync_lock.lock().await; let (sapling_output, sapling_spend) = self.read_sapling_params()?; - let prover = LocalTxProver::from_bytes(&sapling_spend, &sapling_output); + let sapling_prover = LocalTxProver::from_bytes(&sapling_spend, &sapling_output); self.wallet .send_to_addresses( - prover, + sapling_prover, pools_to_shield.to_vec(), vec![(&addr, balance_to_shield - fee, None)], transaction_submission_height, diff --git a/zingolib/src/wallet.rs b/zingolib/src/wallet.rs index 1183a1dd0f..b39cce5e60 100644 --- a/zingolib/src/wallet.rs +++ b/zingolib/src/wallet.rs @@ -921,7 +921,7 @@ impl LightWallet { pub async fn send_to_addresses( &self, - prover: P, + sapling_prover: P, policy: NoteSelectionPolicy, tos: Vec<(&str, u64, Option)>, submission_height: BlockHeight, @@ -936,7 +936,7 @@ impl LightWallet { // Call the internal function match self - .send_to_addresses_inner(prover, policy, tos, submission_height, broadcast_fn) + .send_to_addresses_inner(sapling_prover, policy, tos, submission_height, broadcast_fn) .await { Ok((transaction_id, raw_transaction)) => { From cef98284841a0031575fe12a6eda9841c4cd3d19 Mon Sep 17 00:00:00 2001 From: zancas Date: Thu, 21 Sep 2023 18:24:43 -0600 Subject: [PATCH 5/5] correct prover name inside send_to_addresses_inner --- zingolib/src/wallet.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zingolib/src/wallet.rs b/zingolib/src/wallet.rs index b39cce5e60..a358ea9c8b 100644 --- a/zingolib/src/wallet.rs +++ b/zingolib/src/wallet.rs @@ -1058,7 +1058,7 @@ impl LightWallet { async fn send_to_addresses_inner( &self, - prover: P, + sapling_prover: P, policy: NoteSelectionPolicy, tos: Vec<(&str, u64, Option)>, submission_height: BlockHeight, @@ -1269,7 +1269,7 @@ impl LightWallet { builder.with_progress_notifier(transmitter); let (transaction, _) = match builder.build( - &prover, + &sapling_prover, &transaction::fees::fixed::FeeRule::non_standard(MINIMUM_FEE), ) { Ok(res) => res,