diff --git a/zcash_client_backend/CHANGELOG.md b/zcash_client_backend/CHANGELOG.md index ed16fcf50..fc4806396 100644 --- a/zcash_client_backend/CHANGELOG.md +++ b/zcash_client_backend/CHANGELOG.md @@ -14,8 +14,7 @@ and this library adheres to Rust's notion of to facilitate the automatic shielding of transparent funds received by the wallet. - A `zcash_client_backend::wallet::WalletTransparentOutput` type - has been added under the `transparent-inputs` feature flag in support - of autoshielding functionality. + in support of `transparent-inputs` functionality. - A new `data_api::wallet::spend` method has been added, which is intended to supersede the `data_api::wallet::create_spend_to_address` method. This new method now constructs transactions via interpretation diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 226fa85b7..bf47b7f4b 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -10,9 +10,6 @@ use zcash_primitives::{ }, }; -#[cfg(feature = "transparent-inputs")] -use zcash_primitives::{legacy::keys::IncomingViewingKey, sapling::keys::OutgoingViewingKey}; - use crate::{ address::RecipientAddress, data_api::{ @@ -25,6 +22,9 @@ use crate::{ zip321::{Payment, TransactionRequest}, }; +#[cfg(feature = "transparent-inputs")] +use zcash_primitives::{legacy::keys::IncomingViewingKey, sapling::keys::OutgoingViewingKey}; + /// Scans a [`Transaction`] for any information that can be decrypted by the accounts in /// the wallet, and saves it to the wallet. pub fn decrypt_and_store_transaction( @@ -50,14 +50,10 @@ where .or_else(|| params.activation_height(NetworkUpgrade::Sapling)) .ok_or(Error::SaplingNotActive)?; - let sapling_outputs = decrypt_transaction(params, height, tx, &ufvks); - - if !(sapling_outputs.is_empty() && tx.transparent_bundle().iter().all(|b| b.vout.is_empty())) { - data.store_decrypted_tx(&DecryptedTransaction { - tx, - sapling_outputs: &sapling_outputs, - })?; - } + data.store_decrypted_tx(&DecryptedTransaction { + tx, + sapling_outputs: &decrypt_transaction(params, height, tx, &ufvks), + })?; Ok(()) } @@ -479,7 +475,7 @@ where let utxos = wallet_db.get_unspent_transparent_outputs(&taddr, latest_anchor)?; let total_amount = utxos .iter() - .map(|utxo| utxo.txout.value) + .map(|utxo| utxo.txout().value) .sum::>() .ok_or_else(|| E::from(Error::InvalidAmount))?; @@ -498,7 +494,7 @@ where .unwrap(); for utxo in &utxos { builder - .add_transparent_input(secret_key, utxo.outpoint.clone(), utxo.txout.clone()) + .add_transparent_input(secret_key, utxo.outpoint().clone(), utxo.txout().clone()) .map_err(Error::Builder)?; } @@ -526,6 +522,6 @@ where memo: Some(memo.clone()), }], fee_amount: fee, - utxos_spent: utxos.iter().map(|utxo| utxo.outpoint.clone()).collect(), + utxos_spent: utxos.iter().map(|utxo| utxo.outpoint().clone()).collect(), }) } diff --git a/zcash_client_backend/src/wallet.rs b/zcash_client_backend/src/wallet.rs index 5c6c6c53a..619e73ef6 100644 --- a/zcash_client_backend/src/wallet.rs +++ b/zcash_client_backend/src/wallet.rs @@ -30,15 +30,44 @@ pub struct WalletTx { pub shielded_outputs: Vec>, } +#[derive(Debug, Clone)] pub struct WalletTransparentOutput { - pub outpoint: OutPoint, - pub txout: TxOut, - pub height: BlockHeight, + outpoint: OutPoint, + txout: TxOut, + height: BlockHeight, + recipient_address: TransparentAddress, } impl WalletTransparentOutput { - pub fn address(&self) -> TransparentAddress { - self.txout.script_pubkey.address().unwrap() + pub fn from_parts( + outpoint: OutPoint, + txout: TxOut, + height: BlockHeight, + ) -> Option { + txout + .recipient_address() + .map(|recipient_address| WalletTransparentOutput { + outpoint, + txout, + height, + recipient_address, + }) + } + + pub fn outpoint(&self) -> &OutPoint { + &self.outpoint + } + + pub fn txout(&self) -> &TxOut { + &self.txout + } + + pub fn height(&self) -> BlockHeight { + self.height + } + + pub fn recipient_address(&self) -> &TransparentAddress { + &self.recipient_address } } diff --git a/zcash_client_sqlite/src/lib.rs b/zcash_client_sqlite/src/lib.rs index 21369bb95..7224902ad 100644 --- a/zcash_client_sqlite/src/lib.rs +++ b/zcash_client_sqlite/src/lib.rs @@ -538,7 +538,6 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { &mut self, d_tx: &DecryptedTransaction, ) -> Result { - let nullifiers = self.wallet_db.get_all_nullifiers()?; self.transactionally(|up| { let tx_ref = wallet::put_tx_data(up, d_tx.tx, None, None)?; @@ -579,8 +578,15 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { } } + // If any of the utxos spent in the transaction are ours, mark them as spent. + #[cfg(feature = "transparent-inputs")] + for txin in d_tx.tx.transparent_bundle().iter().flat_map(|b| b.vin.iter()) { + wallet::mark_transparent_utxo_spent(up, tx_ref, &txin.prevout)?; + } + // If we have some transparent outputs: if !d_tx.tx.transparent_bundle().iter().any(|b| b.vout.is_empty()) { + let nullifiers = self.wallet_db.get_all_nullifiers()?; // If the transaction contains shielded spends from our wallet, we will store z->t // transactions we observe in the same way they would be stored by // create_spend_to_address. @@ -590,16 +596,17 @@ impl<'a, P: consensus::Parameters> WalletWrite for DataConnStmtCache<'a, P> { .any(|input| *nf == input.nullifier) ) { for (output_index, txout) in d_tx.tx.transparent_bundle().iter().flat_map(|b| b.vout.iter()).enumerate() { - let recipient = Recipient::Transparent(txout.script_pubkey.address().unwrap()); - wallet::put_sent_output( - up, - *account_id, - tx_ref, - output_index, - &recipient, - txout.value, - None - )?; + if let Some(address) = txout.recipient_address() { + wallet::put_sent_output( + up, + *account_id, + tx_ref, + output_index, + &Recipient::Transparent(address), + txout.value, + None + )?; + } } } } diff --git a/zcash_client_sqlite/src/prepared.rs b/zcash_client_sqlite/src/prepared.rs index a605a7d67..272394c81 100644 --- a/zcash_client_sqlite/src/prepared.rs +++ b/zcash_client_sqlite/src/prepared.rs @@ -513,12 +513,12 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> { self.stmt_insert_received_transparent_utxo .query_row( named_params![ - ":address": &output.address().encode(&self.wallet_db.params), - ":prevout_txid": &output.outpoint.hash().to_vec(), - ":prevout_idx": &output.outpoint.n(), - ":script": &output.txout.script_pubkey.0, - ":value_zat": &i64::from(output.txout.value), - ":height": &u32::from(output.height), + ":address": &output.recipient_address().encode(&self.wallet_db.params), + ":prevout_txid": &output.outpoint().hash().to_vec(), + ":prevout_idx": &output.outpoint().n(), + ":script": &output.txout().script_pubkey.0, + ":value_zat": &i64::from(output.txout().value), + ":height": &u32::from(output.height()), ], |row| { let id = row.get(0)?; @@ -540,12 +540,12 @@ impl<'a, P: consensus::Parameters> DataConnStmtCache<'a, P> { self.stmt_update_received_transparent_utxo .query_row( named_params![ - ":prevout_txid": &output.outpoint.hash().to_vec(), - ":prevout_idx": &output.outpoint.n(), - ":address": &output.address().encode(&self.wallet_db.params), - ":script": &output.txout.script_pubkey.0, - ":value_zat": &i64::from(output.txout.value), - ":height": &u32::from(output.height), + ":prevout_txid": &output.outpoint().hash().to_vec(), + ":prevout_idx": &output.outpoint().n(), + ":address": &output.recipient_address().encode(&self.wallet_db.params), + ":script": &output.txout().script_pubkey.0, + ":value_zat": &i64::from(output.txout().value), + ":height": &u32::from(output.height()), ], |row| { let id = row.get(0)?; diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs index bf0e1ceef..c588218ce 100644 --- a/zcash_client_sqlite/src/wallet.rs +++ b/zcash_client_sqlite/src/wallet.rs @@ -933,7 +933,9 @@ pub(crate) fn get_unspent_transparent_outputs( let addr_str = address.encode(&wdb.params); - let rows = stmt_blocks.query_map(params![addr_str, u32::from(max_height)], |row| { + let mut utxos = Vec::::new(); + let mut rows = stmt_blocks.query(params![addr_str, u32::from(max_height)])?; + while let Some(row) = rows.next()? { let txid: Vec = row.get(0)?; let mut txid_bytes = [0u8; 32]; txid_bytes.copy_from_slice(&txid); @@ -943,21 +945,24 @@ pub(crate) fn get_unspent_transparent_outputs( let value = Amount::from_i64(row.get(3)?).unwrap(); let height: u32 = row.get(4)?; - Ok(WalletTransparentOutput { - outpoint: OutPoint::new(txid_bytes, index), - txout: TxOut { + let output = WalletTransparentOutput::from_parts( + OutPoint::new(txid_bytes, index), + TxOut { value, script_pubkey, }, - height: BlockHeight::from(height), - }) - })?; + BlockHeight::from(height), + ) + .ok_or_else(|| { + SqliteClientError::CorruptedData( + "Txout script_pubkey value did not correspond to a P2PKH or P2SH address" + .to_string(), + ) + })?; - let mut utxos = Vec::::new(); - - for utxo in rows { - utxos.push(utxo.unwrap()) + utxos.push(output); } + Ok(utxos) } @@ -1261,22 +1266,31 @@ mod tests { let uaddr = db_data.get_current_address(account_id).unwrap().unwrap(); let taddr = uaddr.transparent().unwrap(); - let mut utxo = WalletTransparentOutput { - outpoint: OutPoint::new([1u8; 32], 1), - txout: TxOut { + let utxo = WalletTransparentOutput::from_parts( + OutPoint::new([1u8; 32], 1), + TxOut { value: Amount::from_u64(100000).unwrap(), script_pubkey: taddr.script(), }, - height: BlockHeight::from_u32(12345), - }; + BlockHeight::from_u32(12345), + ) + .unwrap(); let res0 = super::put_received_transparent_utxo(&mut ops, &utxo); assert!(matches!(res0, Ok(_))); - // Change something about the UTXO and upsert; we should get back + // Change the mined height of the UTXO and upsert; we should get back // the same utxoid - utxo.height = BlockHeight::from_u32(34567); - let res1 = super::put_received_transparent_utxo(&mut ops, &utxo); + let utxo2 = WalletTransparentOutput::from_parts( + OutPoint::new([1u8; 32], 1), + TxOut { + value: Amount::from_u64(100000).unwrap(), + script_pubkey: taddr.script(), + }, + BlockHeight::from_u32(34567), + ) + .unwrap(); + let res1 = super::put_received_transparent_utxo(&mut ops, &utxo2); assert!(matches!(res1, Ok(id) if id == res0.unwrap())); assert!(matches!( @@ -1296,7 +1310,7 @@ mod tests { ), Ok(utxos) if { utxos.len() == 1 && - utxos.iter().any(|rutxo| rutxo.height == utxo.height) + utxos.iter().any(|rutxo| rutxo.height() == utxo2.height()) } )); @@ -1310,7 +1324,7 @@ mod tests { ) .unwrap(); - let res2 = super::put_received_transparent_utxo(&mut ops, &utxo); + let res2 = super::put_received_transparent_utxo(&mut ops, &utxo2); assert!(matches!(res2, Err(_))); } } diff --git a/zcash_primitives/CHANGELOG.md b/zcash_primitives/CHANGELOG.md index 144a4a26a..900ecbe45 100644 --- a/zcash_primitives/CHANGELOG.md +++ b/zcash_primitives/CHANGELOG.md @@ -39,6 +39,7 @@ and this library adheres to Rust's notion of - `JSDescription::net_value` - Added in `zcash_primitives::transaction::components::transparent` - `Bundle::value_balance` + - `TxOut::recipient_address` - Implementations of `memuse::DynamicUsage` for the following types: - `zcash_primitives::block::BlockHash` - `zcash_primitives::consensus`: @@ -73,6 +74,12 @@ and this library adheres to Rust's notion of - `try_sapling_note_decryption` and `try_sapling_compact_note_decryption` now take `&PreparedIncomingViewingKey` instead of `&SaplingIvk`. +### Removed +- `zcash_primitives::legacy::Script::address` This method was not generally + safe to use on arbitrary scripts, only on script_pubkey values. Its + functionality is now available via + `zcash_primitives::transaction::components::transparent::TxOut::recipient_address` + ## [0.7.0] - 2022-06-24 ### Changed - Bumped dependencies to `equihash 0.2`, `orchard 0.2`. diff --git a/zcash_primitives/src/legacy.rs b/zcash_primitives/src/legacy.rs index bd3ce98f7..912c6ffe3 100644 --- a/zcash_primitives/src/legacy.rs +++ b/zcash_primitives/src/legacy.rs @@ -43,7 +43,7 @@ impl Script { } /// Returns the address that this Script contains, if any. - pub fn address(&self) -> Option { + pub(crate) fn address(&self) -> Option { if self.0.len() == 25 && self.0[0..3] == [OpCode::Dup as u8, OpCode::Hash160 as u8, 0x14] && self.0[23..25] == [OpCode::EqualVerify as u8, OpCode::CheckSig as u8] diff --git a/zcash_primitives/src/transaction/components/transparent.rs b/zcash_primitives/src/transaction/components/transparent.rs index aeb888856..39e8e0f11 100644 --- a/zcash_primitives/src/transaction/components/transparent.rs +++ b/zcash_primitives/src/transaction/components/transparent.rs @@ -5,7 +5,7 @@ use byteorder::{LittleEndian, ReadBytesExt, WriteBytesExt}; use std::fmt::Debug; use std::io::{self, Read, Write}; -use crate::legacy::Script; +use crate::legacy::{Script, TransparentAddress}; use super::amount::{Amount, BalanceError}; @@ -182,6 +182,11 @@ impl TxOut { writer.write_all(&self.value.to_i64_le_bytes())?; self.script_pubkey.write(&mut writer) } + + /// Returns the address to which the TxOut was sent, if this is a valid P2SH or P2PKH output. + pub fn recipient_address(&self) -> Option { + self.script_pubkey.address() + } } #[cfg(any(test, feature = "test-dependencies"))]