From 295be99ce7e34a7a2132bf36ebb441cdf57df23c Mon Sep 17 00:00:00 2001 From: Francisco Gindre Date: Sun, 13 Oct 2024 21:18:59 -0300 Subject: [PATCH] Add min_confirmations to transparent account balances --- zcash_client_backend/src/data_api.rs | 8 +-- zcash_client_sqlite/src/wallet.rs | 2 +- zcash_client_sqlite/src/wallet/transparent.rs | 61 +++++++++++++++++-- zcash_primitives/CHANGELOG.md | 11 ++++ 4 files changed, 70 insertions(+), 12 deletions(-) diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs index dd3cadaa24..53376a198c 100644 --- a/zcash_client_backend/src/data_api.rs +++ b/zcash_client_backend/src/data_api.rs @@ -231,13 +231,7 @@ pub struct AccountBalance { /// The value of unspent Orchard outputs belonging to the account. orchard_balance: Balance, - /// The value of all unspent transparent outputs belonging to the account, irrespective of - /// confirmation depth. - /// - /// Unshielded balances are not subject to confirmation-depth constraints, because the only - /// possible operation on a transparent balance is to shield it, it is possible to create a - /// zero-conf transaction to perform that shielding, and the resulting shielded notes will be - /// subject to normal confirmation rules. + /// The value of all unspent transparent outputs belonging to the account. unshielded_balance: Balance, } diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 21977adad1..db8fee6850 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1530,7 +1530,7 @@ pub(crate) fn get_wallet_summary( drop(sapling_trace); #[cfg(feature = "transparent-inputs")] - transparent::add_transparent_account_balances(tx, chain_tip_height + 1, &mut account_balances)?; + transparent::add_transparent_account_balances(tx, chain_tip_height + 1, min_confirmations, &mut account_balances)?; // The approach used here for Sapling and Orchard subtree indexing was a quick hack // that has not yet been replaced. TODO: Make less hacky. diff --git a/zcash_client_sqlite/src/wallet/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index d9826bf8bd..5893210971 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -383,16 +383,64 @@ pub(crate) fn get_transparent_balances( pub(crate) fn add_transparent_account_balances( conn: &rusqlite::Connection, mempool_height: BlockHeight, + min_confirmations: u32, account_balances: &mut HashMap, ) -> Result<(), SqliteClientError> { - let mut stmt_account_balances = conn.prepare( + + let mut stmt_account_spendable_balances = conn.prepare( "SELECT u.account_id, SUM(u.value_zat) FROM transparent_received_outputs u JOIN transactions t ON t.id_tx = u.transaction_id - -- the transaction that created the output is mined or is definitely unexpired + -- the transaction that created the output is mined and with enough confirmations WHERE ( t.mined_height < :mempool_height -- tx is mined + AND :mempool_height - t.mined_height >= :min_confirmations -- has at least min_confirmations + ) + -- and the received txo is unspent + AND u.id NOT IN ( + SELECT transparent_received_output_id + FROM transparent_received_output_spends txo_spends + JOIN transactions tx + ON tx.id_tx = txo_spends.transaction_id + WHERE tx.mined_height IS NOT NULL -- the spending tx is mined + OR tx.expiry_height = 0 -- the spending tx will not expire + OR tx.expiry_height >= :mempool_height -- the spending tx is unexpired + ) + GROUP BY u.account_id", + )?; + let mut rows = stmt_account_spendable_balances + .query(named_params![ + ":mempool_height": u32::from(mempool_height), + ":min_confirmations": min_confirmations, + ])?; + + while let Some(row) = rows.next()? { + let account = AccountId(row.get(0)?); + let raw_value = row.get(1)?; + let value = NonNegativeAmount::from_nonnegative_i64(raw_value).map_err(|_| { + SqliteClientError::CorruptedData(format!("Negative UTXO value {:?}", raw_value)) + })?; + + + account_balances + .entry(account) + .or_insert(AccountBalance::ZERO) + .with_unshielded_balance_mut::<_, SqliteClientError>(|bal| { + bal.add_spendable_value(value)?; + Ok(()) + })?; + } + + let mut stmt_account_unconfirmed_balances = conn.prepare( + "SELECT u.account_id, SUM(u.value_zat) + FROM transparent_received_outputs u + JOIN transactions t + ON t.id_tx = u.transaction_id + -- the transaction that created the output is mined with not enough confirmations or is definitely unexpired + WHERE ( + (t.mined_height < :mempool_height + AND (:mempool_height - t.mined_height) < :min_confirmations) -- tx is mined but not confirmed OR t.expiry_height = 0 -- tx will not expire OR t.expiry_height >= :mempool_height ) @@ -408,8 +456,12 @@ pub(crate) fn add_transparent_account_balances( ) GROUP BY u.account_id", )?; - let mut rows = stmt_account_balances - .query(named_params![":mempool_height": u32::from(mempool_height),])?; + + let mut rows = stmt_account_unconfirmed_balances + .query(named_params![ + ":mempool_height": u32::from(mempool_height), + ":min_confirmations": min_confirmations, + ])?; while let Some(row) = rows.next()? { let account = AccountId(row.get(0)?); @@ -418,6 +470,7 @@ pub(crate) fn add_transparent_account_balances( SqliteClientError::CorruptedData(format!("Negative UTXO value {:?}", raw_value)) })?; + account_balances .entry(account) .or_insert(AccountBalance::ZERO) diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index d746e2b88d..4e243c549c 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -6,11 +6,22 @@ and this library adheres to Rust's notion of [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added +- `zcash_client_backend::AccountBalance::with_unshielded_balance_mut` +### Deprecated +- `zcash_client_backend::AccountBalance::add_unshielded_value` +- `zcash_client_backend::AccountBalance::unshielded` ## [0.19.0] - 2024-10-02 ### Changed - Migrated to `zcash_address 0.6`. +- Implemented changes to refactor AccountBalance to use Balance for transparent funds +(see issue #1411). `AccountBalance` now has an `unshielded` value that uses Balance. +The current implementation maintains retrocompatibility with the `unshielded` value +represeted with a `NonNegativeAmount`. There are values that are pending to be +implemented such as change tracking. + ### Fixed - The previous release did not bump `zcash_address` and ended up depending on