Merge pull request #1462 from nuttycom/1434-store_decrypted_tx

zcash_client_sqlite: Store received UTXOs in `store_decrypted_tx`

zcash_client_backend/CHANGELOG.md

@@ -26,7 +26,8 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
 ### Added
 - `zcash_client_backend::data_api`:
   - `chain::BlockCache` trait, behind the `sync` feature flag.
-  - `WalletRead::get_spendable_transparent_outputs`.
+  - `WalletRead::get_spendable_transparent_outputs`
+  - `DecryptedTransaction::mined_height`
 - `zcash_client_backend::fees`:
   - `EphemeralBalance`
   - `ChangeValue::shielded, is_ephemeral`
@@ -39,7 +40,9 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
   - `testing` module
 - `zcash_client_backend::sync` module, behind the `sync` feature flag.
 - `zcash_client_backend::tor` module, behind the `tor` feature flag.
-- `zcash_client_backend::wallet::Recipient::map_ephemeral_transparent_outpoint`
+- `zcash_client_backend::wallet`:
+    - `Recipient::map_ephemeral_transparent_outpoint`
+    - `WalletTransparentOutput::mined_height`
 
 ### Changed
 - MSRV is now 1.70.0.
@@ -67,6 +70,7 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
   - `error::Error` has new `Address` and (when the "transparent-inputs" feature
     is enabled) `PaysEphemeralTransparentAddress` variants.
   - `wallet::input_selection::InputSelectorError` has a new `Address` variant.
+  - `DecryptedTransaction::new` takes an additional `mined_height` argument.
 - `zcash_client_backend::data_api::fees`
   - When the "transparent-inputs" feature is enabled, `ChangeValue` can also
     represent an ephemeral transparent output in a proposal. Accordingly, the
@@ -94,6 +98,9 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
   tracking the original address to which value was sent. There is also a new
   `EphemeralTransparent` variant, and an additional generic parameter for the
   type of metadata associated with an ephemeral transparent outpoint.
+- `zcash_client_backend::wallet::WalletTransparentOutput::from_parts`
+  now takes its height argument as `Option<BlockHeight>` rather than
+  `BlockHeight`.
 
 ### Removed
 - `zcash_client_backend::data_api`:
@@ -102,12 +109,14 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
     `WalletRead::get_spendable_transparent_outputs` instead.
 - `zcash_client_backend::fees::ChangeValue::new`. Use `ChangeValue::shielded`
   or `ChangeValue::ephemeral_transparent` instead.
+- `zcash_client_backend::wallet::WalletTransparentOutput::height`
+  (use `WalletTransparentOutput::mined_height` instead).
 
 ## [0.12.1] - 2024-03-27
 
 ### Fixed
 - This release fixes a problem in note selection when sending to a transparent
-  recipient, whereby available funds were being incorrectly excluded from 
+  recipient, whereby available funds were being incorrectly excluded from
   input selection.
 
 ## [0.12.0] - 2024-03-25
@@ -177,11 +186,11 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
     - `get_transaction` now returns `Result<Option<Transaction>, _>` rather
       than returning an `Err` if the `txid` parameter does not correspond to
       a transaction in the database.
-  - `WalletWrite::create_account` now takes its `AccountBirthday` argument by 
+  - `WalletWrite::create_account` now takes its `AccountBirthday` argument by
     reference.
   - Changes to the `InputSource` trait:
     - `select_spendable_notes` now takes its `target_value` argument as a
-      `NonNegativeAmount`. Also, it now returns a `SpendableNotes` data 
+      `NonNegativeAmount`. Also, it now returns a `SpendableNotes` data
       structure instead of a vector.
   - Fields of `DecryptedTransaction` are now private. Use `DecryptedTransaction::new`
     and the newly provided accessors instead.

zcash_client_backend/src/data_api.rs

@@ -1263,6 +1263,7 @@ impl<A> ScannedBlock<A> {
 /// The purpose of this struct is to permit atomic updates of the
 /// wallet database when transactions are successfully decrypted.
 pub struct DecryptedTransaction<'a, AccountId> {
+    mined_height: Option<BlockHeight>,
     tx: &'a Transaction,
     sapling_outputs: Vec<DecryptedOutput<sapling::Note, AccountId>>,
     #[cfg(feature = "orchard")]
@@ -1272,6 +1273,7 @@ pub struct DecryptedTransaction<'a, AccountId> {
 impl<'a, AccountId> DecryptedTransaction<'a, AccountId> {
     /// Constructs a new [`DecryptedTransaction`] from its constituent parts.
     pub fn new(
+        mined_height: Option<BlockHeight>,
         tx: &'a Transaction,
         sapling_outputs: Vec<DecryptedOutput<sapling::Note, AccountId>>,
         #[cfg(feature = "orchard")] orchard_outputs: Vec<
@@ -1279,6 +1281,7 @@ impl<'a, AccountId> DecryptedTransaction<'a, AccountId> {
         >,
     ) -> Self {
         Self {
+            mined_height,
             tx,
             sapling_outputs,
             #[cfg(feature = "orchard")]
@@ -1286,6 +1289,10 @@ impl<'a, AccountId> DecryptedTransaction<'a, AccountId> {
         }
     }
 
+    /// Returns the height at which the transaction was mined, if known.
+    pub fn mined_height(&self) -> Option<BlockHeight> {
+        self.mined_height
+    }
     /// Returns the raw transaction data.
     pub fn tx(&self) -> &Transaction {
         self.tx

zcash_client_backend/src/decrypt.rs

@@ -215,6 +215,7 @@ pub fn decrypt_transaction<'a, P: consensus::Parameters, AccountId: Copy>(
         .collect();
 
     DecryptedTransaction::new(
+        Some(height),
         tx,
         sapling_outputs,
         #[cfg(feature = "orchard")]

zcash_client_backend/src/wallet.rs

@@ -249,7 +249,7 @@ impl<AccountId> WalletTx<AccountId> {
 pub struct WalletTransparentOutput {
     outpoint: OutPoint,
     txout: TxOut,
-    height: BlockHeight,
+    mined_height: Option<BlockHeight>,
     recipient_address: TransparentAddress,
 }
 
@@ -257,14 +257,14 @@ impl WalletTransparentOutput {
     pub fn from_parts(
         outpoint: OutPoint,
         txout: TxOut,
-        height: BlockHeight,
+        mined_height: Option<BlockHeight>,
     ) -> Option<WalletTransparentOutput> {
         txout
             .recipient_address()
             .map(|recipient_address| WalletTransparentOutput {
                 outpoint,
                 txout,
-                height,
+                mined_height,
                 recipient_address,
             })
     }
@@ -277,8 +277,8 @@ impl WalletTransparentOutput {
         &self.txout
     }
 
-    pub fn height(&self) -> BlockHeight {
+    pub fn mined_height(&self) -> Option<BlockHeight> {
-        self.height
+        self.mined_height
     }
 
     pub fn recipient_address(&self) -> &TransparentAddress {

zcash_client_sqlite/src/lib.rs

@@ -124,6 +124,9 @@ use wallet::{
     SubtreeScanProgress,
 };
 
+#[cfg(feature = "transparent-inputs")]
+use wallet::transparent::{find_account_for_transparent_address, put_transparent_output};
+
 #[cfg(test)]
 mod testing;
 
@@ -290,7 +293,7 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> InputSource for
         &self,
         outpoint: &OutPoint,
     ) -> Result<Option<WalletTransparentOutput>, Self::Error> {
-        wallet::transparent::get_unspent_transparent_output(self.conn.borrow(), outpoint)
+        wallet::transparent::get_wallet_transparent_output(self.conn.borrow(), outpoint, false)
     }
 
     #[cfg(feature = "transparent-inputs")]
@@ -1376,6 +1379,24 @@ impl<P: consensus::Parameters> WalletWrite for WalletDb<rusqlite::Connection, P>
                         #[cfg(feature = "transparent-inputs")]
                         wallet::transparent::ephemeral::mark_ephemeral_address_as_seen(wdb, &address, tx_ref)?;
 
+                        // If the output belongs to the wallet, add it to `transparent_received_outputs`.
+                        #[cfg(feature = "transparent-inputs")]
+                        if let Some(account_id) = find_account_for_transparent_address(
+                            wdb.conn.0,
+                            &wdb.params,
+                            &address
+                        )? {
+                            put_transparent_output(
+                                wdb.conn.0,
+                                &wdb.params,
+                                &OutPoint::new(d_tx.tx().txid().into(), u32::try_from(output_index).unwrap()),
+                                txout,
+                                d_tx.mined_height(),
+                                &address,
+                                account_id
+                            )?;
+                        }
+
                         // If a transaction we observe contains spends from our wallet, we will
                         // store its transparent outputs in the same way they would be stored by
                         // create_spend_to_address.

zcash_client_sqlite/src/testing/pool.rs

@@ -315,7 +315,9 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
     };
     use zcash_protocol::value::ZatBalance;
 
-    use crate::wallet::{sapling::tests::test_prover, GAP_LIMIT};
+    use crate::wallet::{
+        sapling::tests::test_prover, transparent::get_wallet_transparent_output, GAP_LIMIT,
+    };
 
     let mut st = TestBuilder::new()
         .with_block_cache()
@@ -553,8 +555,9 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
         },
     );
     let (colliding_addr, _) = &known_addrs[10];
+    let utxo_value = (value - zip317::MINIMUM_FEE).unwrap();
     assert_matches!(
-        builder.add_transparent_output(colliding_addr, (value - zip317::MINIMUM_FEE).unwrap()),
+        builder.add_transparent_output(colliding_addr, utxo_value),
         Ok(_)
     );
     let sk = account
@@ -577,7 +580,9 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
             &zip317::FeeRule::standard(),
         )
         .unwrap();
+    let txid = build_result.transaction().txid();
     let decrypted_tx = DecryptedTransaction::<AccountId>::new(
+        None,
         build_result.transaction(),
         vec![],
         #[cfg(feature = "orchard")]
@@ -585,6 +590,13 @@ pub(crate) fn send_multi_step_proposed_transfer<T: ShieldedPoolTester>() {
     );
     st.wallet_mut().store_decrypted_tx(decrypted_tx).unwrap();
 
+    // We call get_wallet_transparent_output with `allow_unspendable = true` to verify
+    // storage because the decrypted transaction has not yet been mined.
+    let utxo =
+        get_wallet_transparent_output(&st.db_data.conn, &OutPoint::new(txid.into(), 0), true)
+            .unwrap();
+    assert_matches!(utxo, Some(v) if v.value() == utxo_value);
+
     // That should have advanced the start of the gap to index 11.
     let new_known_addrs = st
         .wallet()
@@ -1532,7 +1544,7 @@ pub(crate) fn shield_transparent<T: ShieldedPoolTester>() {
             value: NonNegativeAmount::const_from_u64(10000),
             script_pubkey: taddr.script(),
         },
-        h,
+        Some(h),
     )
     .unwrap();
 

zcash_client_sqlite/src/wallet/transparent.rs

@@ -169,7 +169,7 @@ fn to_unspent_transparent_output(row: &Row) -> Result<WalletTransparentOutput, S
     let value = NonNegativeAmount::from_nonnegative_i64(raw_value).map_err(|_| {
         SqliteClientError::CorruptedData(format!("Invalid UTXO value: {}", raw_value))
     })?;
-    let height: u32 = row.get("received_height")?;
+    let height: Option<u32> = row.get("received_height")?;
 
     let outpoint = OutPoint::new(txid_bytes, index);
     WalletTransparentOutput::from_parts(
@@ -178,7 +178,7 @@ fn to_unspent_transparent_output(row: &Row) -> Result<WalletTransparentOutput, S
             value,
             script_pubkey,
         },
-        BlockHeight::from(height),
+        height.map(BlockHeight::from),
     )
     .ok_or_else(|| {
         SqliteClientError::CorruptedData(
@@ -188,16 +188,18 @@ fn to_unspent_transparent_output(row: &Row) -> Result<WalletTransparentOutput, S
 }
 
 /// Select an output to fund a new transaction that is targeting at least `chain_tip_height + 1`.
-pub(crate) fn get_unspent_transparent_output(
+pub(crate) fn get_wallet_transparent_output(
     conn: &rusqlite::Connection,
     outpoint: &OutPoint,
+    allow_unspendable: bool,
 ) -> Result<Option<WalletTransparentOutput>, SqliteClientError> {
     let chain_tip_height = chain_tip_height(conn)?;
 
-    // This could, in very rare circumstances, return as unspent outputs that are actually not
+    // This could return as unspent outputs that are actually not spendable, if they are the
-    // spendable, if they are the outputs of deshielding transactions where the spend anchors have
+    // outputs of deshielding transactions where the spend anchors have been invalidated by a
-    // been invalidated by a rewind. There isn't a way to detect this circumstance at present, but
+    // rewind or spent in a transaction that has not been observed by this wallet. There isn't a
-    // it should be vanishingly rare as the vast majority of rewinds are of a single block.
+    // way to detect the circumstance related to anchor invalidation at present, but it should be
+    // vanishingly rare as the vast majority of rewinds are of a single block.
     let mut stmt_select_utxo = conn.prepare_cached(
         "SELECT t.txid, u.output_index, u.script,
                 u.value_zat, t.mined_height AS received_height
@@ -207,20 +209,26 @@ pub(crate) fn get_unspent_transparent_output(
          AND u.output_index = :output_index
          -- the transaction that created the output is mined or is definitely unexpired
          AND (
-            t.mined_height IS NOT NULL -- tx is mined
+             :allow_unspendable
-            -- TODO: uncomment the following two lines in order to enable zero-conf spends
+             OR (
-            -- OR t.expiry_height = 0 -- tx will not expire
+                 (
-            -- OR t.expiry_height >= :mempool_height -- tx has not yet expired
+                    t.mined_height IS NOT NULL -- tx is mined
+                    -- TODO: uncomment the following two lines in order to enable zero-conf spends
+                    -- OR t.expiry_height = 0 -- tx will not expire
+                    -- OR t.expiry_height >= :mempool_height -- tx has not yet expired
+                 )
+                 -- and the output is unspent
+                 AND u.id NOT IN (
+                    SELECT txo_spends.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 has not yet expired
+                 )
+             )
          )
-         -- and the output is unspent
+         ",
-         AND u.id NOT IN (
-            SELECT txo_spends.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 has not yet expired
-         )",
     )?;
 
     let result: Result<Option<WalletTransparentOutput>, SqliteClientError> = stmt_select_utxo
@@ -229,6 +237,7 @@ pub(crate) fn get_unspent_transparent_output(
                 ":txid": outpoint.hash(),
                 ":output_index": outpoint.n(),
                 ":mempool_height": chain_tip_height.map(|h| u32::from(h) + 1),
+                ":allow_unspendable": allow_unspendable
             ],
             to_unspent_transparent_output,
         )?
@@ -456,7 +465,7 @@ pub(crate) fn put_received_transparent_utxo<P: consensus::Parameters>(
             params,
             output.outpoint(),
             output.txout(),
-            Some(output.height()),
+            output.mined_height(),
             address,
             receiving_account,
         )
@@ -695,7 +704,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(_));
 
@@ -706,18 +716,18 @@ mod tests {
                 height_1,
                 0
             ).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.mined_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.mined_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 = birthday + 34567;
         st.wallet_mut().update_chain_tip(height_2).unwrap();
-        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());
 
@@ -732,7 +742,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.mined_height()) == (utxo2.outpoint(), utxo2.txout(), Some(height_2))
         );
 
         // If we include `height_2` then the output is returned.
@@ -740,7 +750,7 @@ mod tests {
             st.wallet()
                 .get_spendable_transparent_outputs(taddr, height_2, 0)
                 .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.mined_height()) == (utxo.outpoint(), utxo.txout(), Some(height_2))
         );
 
         assert_matches!(
@@ -842,7 +852,8 @@ 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::fake(), txout, height).unwrap();
+        let utxo =
+            WalletTransparentOutput::from_parts(OutPoint::fake(), txout, Some(height)).unwrap();
         st.wallet_mut()
             .put_received_transparent_utxo(&utxo)
             .unwrap();

zcash_primitives/src/legacy.rs

@@ -1,6 +1,7 @@
 //! Support for legacy transparent addresses and scripts.
 
 use byteorder::{ReadBytesExt, WriteBytesExt};
+use zcash_address::TryFromRawAddress;
 
 use std::fmt;
 use std::io::{self, Read, Write};
@@ -407,6 +408,22 @@ impl TransparentAddress {
     }
 }
 
+impl TryFromRawAddress for TransparentAddress {
+    type Error = ();
+
+    fn try_from_raw_transparent_p2pkh(
+        data: [u8; 20],
+    ) -> Result<Self, zcash_address::ConversionError<Self::Error>> {
+        Ok(TransparentAddress::PublicKeyHash(data))
+    }
+
+    fn try_from_raw_transparent_p2sh(
+        data: [u8; 20],
+    ) -> Result<Self, zcash_address::ConversionError<Self::Error>> {
+        Ok(TransparentAddress::ScriptHash(data))
+    }
+}
+
 #[cfg(any(test, feature = "test-dependencies"))]
 pub mod testing {
     use proptest::prelude::{any, prop_compose};