Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 19 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
301e8b4
zcash_client_sqlite: Add an internal newtype for transaction primary …
nuttycom Jul 29, 2024
1057ddb
zcash_client_sqlite: Add a table to track transaction status and enri…
nuttycom Jul 29, 2024
1820788
zcash_client_backend: Add `block_height` argument to `decrypt_and_sto…
nuttycom Aug 1, 2024
7fbc656
zcash_client_sqlite: Ensure that max_observed_unspent height is set c…
nuttycom Aug 1, 2024
3da29d2
zcash_client_backend: Add transaction data requests.
nuttycom Aug 1, 2024
69828bc
zcash_client_sqlite: Add `target_height` column to `transactions` table.
nuttycom Aug 1, 2024
a3109cf
zcash_client_sqlite: Add queue for transparent spend detection by add…
nuttycom Aug 1, 2024
52f7a77
Apply suggestions from code review
nuttycom Aug 7, 2024
366a3e3
Apply suggestions from code review
nuttycom Aug 9, 2024
dada980
zcash_client_sqlite: Add testing for transaction data request generat…
nuttycom Aug 9, 2024
d96bc11
Add a comment explaining why it is safe to unconditionally delete the
daira Aug 9, 2024
8752ad7
zcash_client_backend: Add `target_height` to `SentTransaction`
nuttycom Aug 9, 2024
171cc3d
Address review comments in `transaction_data_requests`
nuttycom Aug 9, 2024
239f7ff
Improve the accuracy of a comment in `set_transaction_status`.
daira Aug 9, 2024
56e4233
Minor simplifications.
daira Aug 9, 2024
1023ce7
Replace the `put_tx_meta` workaround in `send_multi_step_proposed_tra…
daira Aug 9, 2024
0bfa3d3
Merge remote-tracking branch 'daira/1485-improve-multi-step-test' int…
nuttycom Aug 9, 2024
5a1645a
zcash_client_sqlite: Make transaction retrieval depend upon presence …
nuttycom Aug 9, 2024
b47c7bf
zcash_client_sqlite: Improve the internal documentation of `set_trans…
nuttycom Aug 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion zcash_client_backend/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions zcash_client_backend/proto/service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
138 changes: 133 additions & 5 deletions zcash_client_backend/src/data_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
nuttycom marked this conversation as resolved.
Show resolved Hide resolved
/// 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
nuttycom marked this conversation as resolved.
Show resolved Hide resolved
/// 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.
daira marked this conversation as resolved.
Show resolved Hide resolved
Mined(BlockHeight),
}

impl<NoteRef> SpendableNotes<NoteRef> {
/// Construct a new empty [`SpendableNotes`].
pub fn empty() -> Self {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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,
str4d marked this conversation as resolved.
Show resolved Hide resolved
account: AccountId,
outputs: &'a [SentTransactionOutput<AccountId>],
fee_amount: NonNegativeAmount,
Expand All @@ -1336,6 +1427,7 @@ impl<'a, AccountId> SentTransaction<'a, AccountId> {
Self {
tx,
created,
target_height,
account,
outputs,
fee_amount,
Expand Down Expand Up @@ -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 {
str4d marked this conversation as resolved.
Show resolved Hide resolved
self.target_height
}
}

/// An output of a transaction generated by the wallet.
Expand Down Expand Up @@ -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.
daira marked this conversation as resolved.
Show resolved Hide resolved
Ok(vec![])
}
daira marked this conversation as resolved.
Show resolved Hide resolved

/// 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
nuttycom marked this conversation as resolved.
Show resolved Hide resolved
/// 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.
Expand Down Expand Up @@ -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")]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down
14 changes: 9 additions & 5 deletions zcash_client_backend/src/data_api/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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."))
})?;
nuttycom marked this conversation as resolved.
Show resolved Hide resolved

data.store_decrypted_tx(decrypt_transaction(params, height, tx, &ufvks))?;

Expand Down Expand Up @@ -665,6 +668,7 @@ where
transactions.push(SentTransaction::new(
tx,
created,
proposal.min_target_height(),
account_id,
&step_result.outputs,
step_result.fee_amount,
Expand Down
3 changes: 3 additions & 0 deletions zcash_client_backend/src/proto/service.rs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading