diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index 607207685..634818248 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -39,7 +39,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. @@ -93,6 +95,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` rather than + `BlockHeight`. ### Removed - `zcash_client_backend::data_api`: @@ -101,6 +106,8 @@ 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 the + `mined_height` method instead. ## [0.12.1] - 2024-03-27 diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 1bc4dae3f..f3bca04ba 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -249,7 +249,7 @@ impl WalletTx { pub struct WalletTransparentOutput { outpoint: OutPoint, txout: TxOut, - height: BlockHeight, + mined_height: Option, recipient_address: TransparentAddress, } @@ -257,14 +257,14 @@ impl WalletTransparentOutput { pub fn from_parts( outpoint: OutPoint, txout: TxOut, - height: BlockHeight, + mined_height: Option, ) -> Option { 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 { - self.height + pub fn mined_height(&self) -> Option { + self.mined_height } pub fn recipient_address(&self) -> &TransparentAddress { diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index afb3d170b..69f8ebac8 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -290,7 +290,7 @@ impl, P: consensus::Parameters> InputSource for &self, outpoint: &OutPoint, ) -> Result, 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")] diff --git a/zcash_client_sqlite/src/testing/pool.rs b/zcash_client_sqlite/src/testing/pool.rs index 76d172749..fbd06e6a6 100644 --- a/zcash_client_sqlite/src/testing/pool.rs +++ b/zcash_client_sqlite/src/testing/pool.rs @@ -315,7 +315,9 @@ pub(crate) fn send_multi_step_proposed_transfer() { }; 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() { }, ); 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,6 +580,7 @@ pub(crate) fn send_multi_step_proposed_transfer() { &zip317::FeeRule::standard(), ) .unwrap(); + let txid = build_result.transaction().txid(); let decrypted_tx = DecryptedTransaction::::new( build_result.transaction(), vec![], @@ -585,6 +589,13 @@ pub(crate) fn send_multi_step_proposed_transfer() { ); 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 +1543,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/transparent.rs b/zcash_client_sqlite/src/wallet/transparent.rs index 000c515e2..0db0fca04 100644 --- a/zcash_client_sqlite/src/wallet/transparent.rs +++ b/zcash_client_sqlite/src/wallet/transparent.rs @@ -169,7 +169,7 @@ fn to_unspent_transparent_output(row: &Row) -> Result = 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 Result Result, SqliteClientError> { let chain_tip_height = chain_tip_height(conn)?; - // This could, in very rare circumstances, return as unspent outputs that are actually not - // spendable, if they are the outputs of deshielding transactions where the spend anchors have - // been invalidated by a rewind. There isn't a way to detect this circumstance at present, but - // it should be vanishingly rare as the vast majority of rewinds are of a single block. + // This could return as unspent outputs that are actually not spendable, if they are the + // outputs of deshielding transactions where the spend anchors have been invalidated by a + // rewind or spent in a transaction that has not been observed by this wallet. There isn't a + // 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 - -- 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 + :allow_unspendable + OR ( + ( + 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, 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( 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();