Merge pull request #1473 from nuttycom/wallet/enrichment_queue

Add queues to track the need for transaction enhancement and/or verification of mined status.

zcash_client_backend/CHANGELOG.md

@@ -28,6 +28,8 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
   - `chain::BlockCache` trait, behind the `sync` feature flag.
   - `WalletRead::get_spendable_transparent_outputs`
   - `DecryptedTransaction::mined_height`
+  - `TransactionDataRequest`
+  - `TransactionStatus`
 - `zcash_client_backend::fees`:
   - `EphemeralBalance`
   - `ChangeValue::shielded, is_ephemeral`
@@ -61,12 +63,14 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
   have changed as a consequence of this extraction; please see the `zip321`
   CHANGELOG for details.
 - `zcash_client_backend::data_api`:
+  - `WalletRead` has a new `transaction_data_requests` method.
   - `WalletRead` has new `get_known_ephemeral_addresses`,
     `find_account_for_ephemeral_address`, and `get_transparent_address_metadata`
     methods when the "transparent-inputs" feature is enabled.
   - `WalletWrite` has a new `reserve_next_n_ephemeral_addresses` method when
     the "transparent-inputs" feature is enabled.
-  - `WalletWrite` has new methods `import_account_hd` and `import_account_ufvk`.
+  - `WalletWrite` has new methods `import_account_hd`, `import_account_ufvk`,
+    and `set_transaction_status`.
   - `error::Error` has new `Address` and (when the "transparent-inputs" feature
     is enabled) `PaysEphemeralTransparentAddress` variants.
   - The `WalletWrite::store_sent_tx` method has been renamed to
@@ -75,9 +79,15 @@ funds to those addresses. See [ZIP 320](https://zips.z.cash/zip-0320) for detail
     `zcash_client_sqlite`) to improve transactionality of writes for multi-step
     proposals.
   - `wallet::input_selection::InputSelectorError` has a new `Address` variant.
+  - `wallet::decrypt_and_store_transaction` now takes an additional optional
+    `mined_height` argument that can be used to provide the mined height
+    returned by the light wallet server in a `RawTransaction` value directly to
+    the back end.
   - `DecryptedTransaction::new` takes an additional `mined_height` argument.
   - `SentTransaction` now stores its `outputs` and `utxos_spent` fields as
     references to slices, with a corresponding change to `SentTransaction::new`.
+  - `SentTransaction` takes an additional `target_height` argument, which is used
+    to record the target height used in transaction generation.
 - `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

zcash_client_backend/proto/service.proto

@@ -34,6 +34,9 @@ message TxFilter {
 // RawTransaction contains the complete transaction data. It also optionally includes 
 // the block height in which the transaction was included, or, when returned
 // by GetMempoolStream(), the latest block height.
+//
+// FIXME: the documentation here about mempool status contradicts the documentation
+// for the `height` field. See https://github.com/zcash/librustzcash/issues/1484
 message RawTransaction {
     bytes data = 1;     // exact data returned by Zcash 'getrawtransaction'
     uint64 height = 2;  // height that the transaction was mined (or -1)

zcash_client_backend/src/data_api.rs

@@ -541,6 +541,71 @@ pub struct SpendableNotes<NoteRef> {
     orchard: Vec<ReceivedNote<NoteRef, orchard::note::Note>>,
 }
 
+/// A request for transaction data enhancement, spentness check, or discovery
+/// of spends from a given transparent address within a specific block range.
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub enum TransactionDataRequest {
+    /// Information about the chain's view of a transaction is requested.
+    ///
+    /// The caller evaluating this request on behalf of the wallet backend should respond to this
+    /// request by determining the status of the specified transaction with respect to the main
+    /// chain; if using `lightwalletd` for access to chain data, this may be obtained by
+    /// interpreting the results of the [`GetTransaction`] RPC method. It should then call
+    /// [`WalletWrite::set_transaction_status`] to provide the resulting transaction status
+    /// information to the wallet backend.
+    ///
+    /// [`GetTransaction`]: crate::proto::service::compact_tx_streamer_client::CompactTxStreamerClient::get_transaction
+    GetStatus(TxId),
+    /// Transaction enhancement (download of complete raw transaction data) is requested.
+    ///
+    /// The caller evaluating this request on behalf of the wallet backend should respond to this
+    /// request by providing complete data for the specified transaction to
+    /// [`wallet::decrypt_and_store_transaction`]; if using `lightwalletd` for access to chain
+    /// state, this may be obtained via the [`GetTransaction`] RPC method. If no data is available
+    /// for the specified transaction, this should be reported to the backend using
+    /// [`WalletWrite::set_transaction_status`]. A [`TransactionDataRequest::Enhancement`] request
+    /// subsumes any previously existing [`TransactionDataRequest::GetStatus`] request.
+    ///
+    /// [`GetTransaction`]: crate::proto::service::compact_tx_streamer_client::CompactTxStreamerClient::get_transaction
+    Enhancement(TxId),
+    /// Information about transactions that receive or spend funds belonging to the specified
+    /// transparent address is requested.
+    ///
+    /// Fully transparent transactions, and transactions that do not contain either shielded inputs
+    /// or shielded outputs belonging to the wallet, may not be discovered by the process of chain
+    /// scanning; as a consequence, the wallet must actively query to find transactions that spend
+    /// such funds. Ideally we'd be able to query by [`OutPoint`] but this is not currently
+    /// functionality that is supported by the light wallet server.
+    ///
+    /// The caller evaluating this request on behalf of the wallet backend should respond to this
+    /// request by detecting transactions involving the specified address within the provided block
+    /// range; if using `lightwalletd` for access to chain data, this may be performed using the
+    /// [`GetTaddressTxids`] RPC method. It should then call [`wallet::decrypt_and_store_transaction`]
+    /// for each transaction so detected.
+    ///
+    /// [`GetTaddressTxids`]: crate::proto::service::compact_tx_streamer_client::CompactTxStreamerClient::get_taddress_txids
+    #[cfg(feature = "transparent-inputs")]
+    SpendsFromAddress {
+        address: TransparentAddress,
+        block_range_start: BlockHeight,
+        block_range_end: Option<BlockHeight>,
+    },
+}
+
+/// Metadata about the status of a transaction obtained by inspecting the chain state.
+#[derive(Clone, Copy, Debug)]
+pub enum TransactionStatus {
+    /// The requested transaction ID was not recognized by the node.
+    TxidNotRecognized,
+    /// The requested transaction ID corresponds to a transaction that is recognized by the node,
+    /// but is in the mempool or is otherwise not mined in the main chain (but may have been mined
+    /// on a fork that was reorged away).
+    NotInMainChain,
+    /// The requested transaction ID corresponds to a transaction that has been included in the
+    /// block at the provided height.
+    Mined(BlockHeight),
+}
+
 impl<NoteRef> SpendableNotes<NoteRef> {
     /// Construct a new empty [`SpendableNotes`].
     pub fn empty() -> Self {
@@ -1039,6 +1104,20 @@ pub trait WalletRead {
         }
         Ok(None)
     }
+
+    /// Returns a vector of [`TransactionDataRequest`] values that describe information needed by
+    /// the wallet to complete its view of transaction history.
+    ///
+    /// Requests for the same transaction data may be returned repeatedly by successive data
+    /// requests. The caller of this method should consider the latest set of requests returned
+    /// by this method to be authoritative and to subsume that returned by previous calls.
+    ///
+    /// Callers should poll this method on a regular interval, not as part of ordinary chain
+    /// scanning, which already produces requests for transaction data enhancement. Note that
+    /// responding to a set of transaction data requests may result in the creation of new
+    /// transaction data requests, such as when it is necessary to fill in purely-transparent
+    /// transaction history by walking the chain backwards via transparent inputs.
+    fn transaction_data_requests(&self) -> Result<Vec<TransactionDataRequest>, Self::Error>;
 }
 
 /// The relevance of a seed to a given wallet.
@@ -1316,6 +1395,7 @@ impl<'a, AccountId> DecryptedTransaction<'a, AccountId> {
 pub struct SentTransaction<'a, AccountId> {
     tx: &'a Transaction,
     created: time::OffsetDateTime,
+    target_height: BlockHeight,
     account: AccountId,
     outputs: &'a [SentTransactionOutput<AccountId>],
     fee_amount: NonNegativeAmount,
@@ -1325,9 +1405,20 @@ pub struct SentTransaction<'a, AccountId> {
 
 impl<'a, AccountId> SentTransaction<'a, AccountId> {
     /// Constructs a new [`SentTransaction`] from its constituent parts.
+    ///
+    /// ### Parameters
+    /// - `tx`: the raw transaction data
+    /// - `created`: the system time at which the transaction was created
+    /// - `target_height`: the target height that was used in the construction of the transaction
+    /// - `account`: the account that spent funds in creation of the transaction
+    /// - `outputs`: the outputs created by the transaction, including those sent to external
+    ///   recipients which may not otherwise be recoverable
+    /// - `fee_amount`: the fee value paid by the transaction
+    /// - `utxos_spent`: the UTXOs controlled by the wallet that were spent in this transaction
     pub fn new(
         tx: &'a Transaction,
         created: time::OffsetDateTime,
+        target_height: BlockHeight,
         account: AccountId,
         outputs: &'a [SentTransactionOutput<AccountId>],
         fee_amount: NonNegativeAmount,
@@ -1336,6 +1427,7 @@ impl<'a, AccountId> SentTransaction<'a, AccountId> {
         Self {
             tx,
             created,
+            target_height,
             account,
             outputs,
             fee_amount,
@@ -1369,6 +1461,11 @@ impl<'a, AccountId> SentTransaction<'a, AccountId> {
     pub fn utxos_spent(&self) -> &[OutPoint] {
         self.utxos_spent
     }
+
+    /// Returns the block height that this transaction was created to target.
+    pub fn target_height(&self) -> BlockHeight {
+        self.target_height
+    }
 }
 
 /// An output of a transaction generated by the wallet.
@@ -1811,9 +1908,27 @@ pub trait WalletWrite: WalletRead {
     #[cfg(feature = "transparent-inputs")]
     fn reserve_next_n_ephemeral_addresses(
         &mut self,
-        account_id: Self::AccountId,
-        n: usize,
-    ) -> Result<Vec<(TransparentAddress, TransparentAddressMetadata)>, Self::Error>;
+        _account_id: Self::AccountId,
+        _n: usize,
+    ) -> Result<Vec<(TransparentAddress, TransparentAddressMetadata)>, Self::Error> {
+        // Default impl is required for feature-flagged trait methods to prevent
+        // breakage due to inadvertent activation of features by transitive dependencies
+        // of the implementing crate.
+        Ok(vec![])
+    }
+
+    /// Updates the wallet backend with respect to the status of a specific transaction, from the
+    /// perspective of the main chain.
+    ///
+    /// Fully transparent transactions, and transactions that do not contain either shielded inputs
+    /// or shielded outputs belonging to the wallet, may not be discovered by the process of chain
+    /// scanning; as a consequence, the wallet must actively query to determine whether such
+    /// transactions have been mined.
+    fn set_transaction_status(
+        &mut self,
+        _txid: TxId,
+        _status: TransactionStatus,
+    ) -> Result<(), Self::Error>;
 }
 
 /// This trait describes a capability for manipulating wallet note commitment trees.
@@ -1913,8 +2028,9 @@ pub mod testing {
         chain::{ChainState, CommitmentTreeRoot},
         scanning::ScanRange,
         AccountBirthday, BlockMetadata, DecryptedTransaction, InputSource, NullifierQuery,
-        ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, WalletCommitmentTrees,
-        WalletRead, WalletSummary, WalletWrite, SAPLING_SHARD_HEIGHT,
+        ScannedBlock, SeedRelevance, SentTransaction, SpendableNotes, TransactionDataRequest,
+        TransactionStatus, WalletCommitmentTrees, WalletRead, WalletSummary, WalletWrite,
+        SAPLING_SHARD_HEIGHT,
     };
 
     #[cfg(feature = "transparent-inputs")]
@@ -2170,6 +2286,10 @@ pub mod testing {
         ) -> Result<Option<Self::AccountId>, Self::Error> {
             Ok(None)
         }
+
+        fn transaction_data_requests(&self) -> Result<Vec<TransactionDataRequest>, Self::Error> {
+            Ok(vec![])
+        }
     }
 
     impl WalletWrite for MockWalletDb {
@@ -2259,6 +2379,14 @@ pub mod testing {
         ) -> Result<Vec<(TransparentAddress, TransparentAddressMetadata)>, Self::Error> {
             Err(())
         }
+
+        fn set_transaction_status(
+            &mut self,
+            _txid: TxId,
+            _status: TransactionStatus,
+        ) -> Result<(), Self::Error> {
+            Ok(())
+        }
     }
 
     impl WalletCommitmentTrees for MockWalletDb {

zcash_client_backend/src/data_api/wallet.rs

@@ -92,6 +92,7 @@ pub fn decrypt_and_store_transaction<ParamsT, DbT>(
     params: &ParamsT,
     data: &mut DbT,
     tx: &Transaction,
+    mined_height: Option<BlockHeight>,
 ) -> Result<(), DbT::Error>
 where
     ParamsT: consensus::Parameters,
@@ -102,11 +103,13 @@ where
 
     // Height is block height for mined transactions, and the "mempool height" (chain height + 1)
     // for mempool transactions.
-    let height = data
-        .get_tx_height(tx.txid())?
-        .or(data.chain_height()?.map(|max_height| max_height + 1))
-        .or_else(|| params.activation_height(NetworkUpgrade::Sapling))
-        .expect("Sapling activation height must be known.");
+    let height = mined_height.map(Ok).unwrap_or_else(|| {
+        Ok(data
+            .get_tx_height(tx.txid())?
+            .or(data.chain_height()?.map(|max_height| max_height + 1))
+            .or_else(|| params.activation_height(NetworkUpgrade::Sapling))
+            .expect("Sapling activation height must be known."))
+    })?;
 
     data.store_decrypted_tx(decrypt_transaction(params, height, tx, &ufvks))?;
 
@@ -665,6 +668,7 @@ where
         transactions.push(SentTransaction::new(
             tx,
             created,
+            proposal.min_target_height(),
             account_id,
             &step_result.outputs,
             step_result.fee_amount,