diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 1c8e544d8c..fb5910f140 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -32,10 +32,14 @@ and this library adheres to Rust's notion of `ShieldedProtocol` to `zcash_protocol::PoolType`. - `zcash_client_backend::input_selection::GreedyInputSelectorError` has a new variant `UnsupportedTexAddress`. -- `zcash_client_backend::wallet::Recipient` variants have changed. Instead of - wrapping protocol-address types, the `Recipient` type now wraps a - `zcash_address::ZcashAddress`. This simplifies the process of tracking the - original address to which value was sent. +- `zcash_client_backend::wallet`: + - `Recipient` variants have changed. Instead of wrapping protocol-address + types, the `External` and `InternalAccount` variants now wrap a + `zcash_address::ZcashAddress`. This simplifies the process of tracking + the original address to which value was sent. + - `WalletTransparentOutput::{from_parts, height}` now take and return the + height as an `Option`, accounting for cases where the height + is not known. - MSRV is now 1.66.0 (except for the `lightwalletd-tonic` feature flag which requires 1.70.0). diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 7d555b07f6..53e0be8247 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -191,7 +191,7 @@ impl WalletTx { pub struct WalletTransparentOutput { outpoint: OutPoint, txout: TxOut, - height: BlockHeight, + height: Option, recipient_address: TransparentAddress, } @@ -199,7 +199,7 @@ impl WalletTransparentOutput { pub fn from_parts( outpoint: OutPoint, txout: TxOut, - height: BlockHeight, + height: Option, ) -> Option { txout .recipient_address() @@ -219,7 +219,7 @@ impl WalletTransparentOutput { &self.txout } - pub fn height(&self) -> BlockHeight { + pub fn height(&self) -> Option { self.height } diff --git a/zcash_client_sqlite/CHANGELOG.md b/zcash_client_sqlite/CHANGELOG.md index feed5cea6d..b15f86a721 100644 --- a/zcash_client_sqlite/CHANGELOG.md +++ b/zcash_client_sqlite/CHANGELOG.md @@ -7,6 +7,8 @@ and this library adheres to Rust's notion of ## [Unreleased] ### Changed +- The result of the `v_tx_outputs` SQL query could now include transparent outputs + with unknown height. - MSRV is now 1.66.0. ## [0.10.3] - 2024-04-08 diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 83ba9d5e1e..9ae749e6e6 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -1208,7 +1208,7 @@ pub(crate) fn shield_transparent() { value: NonNegativeAmount::const_from_u64(10000), script_pubkey: taddr.script(), }, - h, + Some(h), ) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index 57390a724f..228ab20c97 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -1299,6 +1299,7 @@ pub(crate) fn get_wallet_summary( let zero_conf_height = (chain_tip_height + 1).saturating_sub(min_confirmations); let stable_height = chain_tip_height.saturating_sub(PRUNING_DEPTH); + // FIXME: This can select utxos with unknown height (sentinel -1); is that correct? let mut stmt_transparent_balances = tx.prepare( "SELECT u.received_by_account_id, SUM(u.value_zat) FROM utxos u @@ -2091,7 +2092,14 @@ fn to_unspent_transparent_output(row: &Row) -> Result Ok(None), + h @ 0..=0xFFFFFFFFi64 => Ok(Some(BlockHeight::from(h as u32))), + raw_value => Err(SqliteClientError::CorruptedData(format!( + "Invalid height: {}", + raw_value + ))), + }?; let outpoint = OutPoint::new(txid_bytes, index); WalletTransparentOutput::from_parts( @@ -2100,7 +2108,7 @@ fn to_unspent_transparent_output(row: &Row) -> Result Result, SqliteClientError> { + // FIXME: This can select utxos with unknown height (sentinel -1); is that correct? let mut stmt_select_utxo = conn.prepare_cached( "SELECT u.prevout_txid, u.prevout_idx, u.script, u.value_zat, u.height FROM utxos u @@ -2158,6 +2167,7 @@ pub(crate) fn get_unspent_transparent_outputs( .unwrap_or(max_height) .saturating_sub(PRUNING_DEPTH); + // FIXME: This can select utxos with unknown height (sentinel -1); is that correct? let mut stmt_utxos = conn.prepare( "SELECT u.prevout_txid, u.prevout_idx, u.script, u.value_zat, u.height @@ -2211,6 +2221,7 @@ pub(crate) fn get_transparent_balances( .unwrap_or(max_height) .saturating_sub(PRUNING_DEPTH); + // FIXME: This can select utxos with unknown height (sentinel -1); is that correct? let mut stmt_blocks = conn.prepare( "SELECT u.address, SUM(u.value_zat) FROM utxos u @@ -2453,6 +2464,7 @@ pub(crate) fn mark_transparent_utxo_spent( tx_ref: i64, outpoint: &OutPoint, ) -> Result<(), SqliteClientError> { + // FIXME: This can select utxos with unknown height (sentinel -1); is that correct? let mut stmt_mark_transparent_utxo_spent = conn.prepare_cached( "INSERT INTO transparent_received_output_spends (transparent_received_output_id, transaction_id) SELECT txo.id, :spent_in_tx @@ -2527,7 +2539,7 @@ pub(crate) fn put_legacy_transparent_utxo( output: &WalletTransparentOutput, received_by_account: AccountId, ) -> Result { - #[cfg(feature = "transparent-inputs")] + // This can insert utxos with unknown height (sentinel -1). let mut stmt_upsert_legacy_transparent_utxo = conn.prepare_cached( "INSERT INTO utxos ( prevout_txid, prevout_idx, @@ -2553,7 +2565,7 @@ pub(crate) fn put_legacy_transparent_utxo( ":address": &output.recipient_address().encode(params), ":script": &output.txout().script_pubkey.0, ":value_zat": &i64::from(Amount::from(output.txout().value)), - ":height": &u32::from(output.height()), + ":height": &output.height().map_or(-1, i64::from), ]; stmt_upsert_legacy_transparent_utxo.query_row(sql_args, |row| row.get::<_, i64>(0).map(UtxoId)) @@ -2928,7 +2940,8 @@ mod tests { // Pretend the output's transaction was mined at `height_1`. let utxo = - WalletTransparentOutput::from_parts(outpoint.clone(), txout.clone(), height_1).unwrap(); + WalletTransparentOutput::from_parts(outpoint.clone(), txout.clone(), Some(height_1)) + .unwrap(); let res0 = st.wallet_mut().put_received_transparent_utxo(&utxo); assert_matches!(res0, Ok(_)); @@ -2939,17 +2952,17 @@ mod tests { height_1, &[] ).as_deref(), - Ok([ret]) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo.outpoint(), utxo.txout(), height_1) + Ok([ret]) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo.outpoint(), utxo.txout(), Some(height_1)) ); assert_matches!( st.wallet().get_unspent_transparent_output(utxo.outpoint()), - Ok(Some(ret)) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo.outpoint(), utxo.txout(), height_1) + Ok(Some(ret)) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo.outpoint(), utxo.txout(), Some(height_1)) ); // Change the mined height of the UTXO and upsert; we should get back // the same `UtxoId`. let height_2 = BlockHeight::from_u32(34567); - let utxo2 = WalletTransparentOutput::from_parts(outpoint, txout, height_2).unwrap(); + let utxo2 = WalletTransparentOutput::from_parts(outpoint, txout, Some(height_2)).unwrap(); let res1 = st.wallet_mut().put_received_transparent_utxo(&utxo2); assert_matches!(res1, Ok(id) if id == res0.unwrap()); @@ -2964,7 +2977,7 @@ mod tests { // We can still look up the specific output, and it has the expected height. assert_matches!( st.wallet().get_unspent_transparent_output(utxo2.outpoint()), - Ok(Some(ret)) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo2.outpoint(), utxo2.txout(), height_2) + Ok(Some(ret)) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo2.outpoint(), utxo2.txout(), Some(height_2)) ); // If we include `height_2` then the output is returned. @@ -2972,7 +2985,7 @@ mod tests { st.wallet() .get_unspent_transparent_outputs(taddr, height_2, &[]) .as_deref(), - Ok([ret]) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo.outpoint(), utxo.txout(), height_2) + Ok([ret]) if (ret.outpoint(), ret.txout(), ret.height()) == (utxo.outpoint(), utxo.txout(), Some(height_2)) ); assert_matches!( @@ -3107,7 +3120,7 @@ mod tests { // Pretend the output was received in the chain tip. let height = st.wallet().chain_height().unwrap().unwrap(); - let utxo = WalletTransparentOutput::from_parts(outpoint, txout, height).unwrap(); + let utxo = WalletTransparentOutput::from_parts(outpoint, txout, Some(height)).unwrap(); st.wallet_mut() .put_received_transparent_utxo(&utxo) .unwrap(); diff --git a/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs b/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs index fc64ff26c4..954d856199 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/add_utxo_account.rs @@ -90,6 +90,7 @@ impl RusqliteMigration for Migration

{ } } + // Although height is INTEGER NOT NULL, we use -1 as a sentinel for an unknown height. transaction.execute_batch( "CREATE TABLE utxos_new ( id_utxo INTEGER PRIMARY KEY, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/utxos_table.rs b/zcash_client_sqlite/src/wallet/init/migrations/utxos_table.rs index 083393e677..0e17f7856f 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/utxos_table.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/utxos_table.rs @@ -30,6 +30,7 @@ impl RusqliteMigration for Migration { type Error = WalletMigrationError; fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), WalletMigrationError> { + // Although height is INTEGER NOT NULL, we use -1 as a sentinel for an unknown height. transaction.execute_batch( "CREATE TABLE IF NOT EXISTS utxos ( id_utxo INTEGER PRIMARY KEY, diff --git a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_transparent_history.rs b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_transparent_history.rs index 70a9566f4c..027384fc6f 100644 --- a/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_transparent_history.rs +++ b/zcash_client_sqlite/src/wallet/init/migrations/v_transactions_transparent_history.rs @@ -34,6 +34,8 @@ impl RusqliteMigration for Migration { type Error = WalletMigrationError; fn up(&self, transaction: &rusqlite::Transaction) -> Result<(), Self::Error> { + // FIXME: This will *not* include a transaction by virtue of it involving a utxo + // that is in the utxos table with unknown height. Is that correct? transaction.execute_batch( "DROP VIEW v_transactions; CREATE VIEW v_transactions AS @@ -134,6 +136,8 @@ impl RusqliteMigration for Migration { GROUP BY notes.account_id, notes.txid;" )?; + // FIXME: This *will* include outputs that are in the utxos table with unknown height. + // Is that correct? transaction.execute_batch( "DROP VIEW v_tx_outputs; CREATE VIEW v_tx_outputs AS