Rework WalletTest::get_confirmed_sends trait method

Closes #1544.

zcash_client_backend/src/data_api.rs

@@ -67,6 +67,7 @@ use incrementalmerkletree::{frontier::Frontier, Retention};
 use nonempty::NonEmpty;
 use secrecy::SecretVec;
 use shardtree::{error::ShardTreeError, store::ShardStore, ShardTree};
+use zcash_keys::address::Address;
 use zip32::fingerprint::SeedFingerprint;
 
 use self::{
@@ -1233,11 +1234,12 @@ pub trait WalletTest: InputSource + WalletRead {
         _protocol: ShieldedProtocol,
     ) -> Result<Vec<NoteId>, <Self as WalletRead>::Error>;
 
+    /// Returns the outputs for a transaction sent by the wallet.
     #[allow(clippy::type_complexity)]
-    fn get_confirmed_sends(
+    fn get_sent_outputs(
         &self,
         txid: &TxId,
-    ) -> Result<Vec<(u64, Option<String>, Option<String>, Option<u32>)>, <Self as WalletRead>::Error>;
+    ) -> Result<Vec<OutputOfSentTx>, <Self as WalletRead>::Error>;
 
     #[allow(clippy::type_complexity)]
     fn get_checkpoint_history(
@@ -1270,6 +1272,37 @@ pub trait WalletTest: InputSource + WalletRead {
     ) -> Result<Vec<ReceivedNote<Self::NoteRef, Note>>, <Self as InputSource>::Error>;
 }
 
+/// The output of a transaction sent by the wallet.
+///
+/// This type is opaque, and exists for use by tests defined in this crate.
+#[cfg(any(test, feature = "test-dependencies"))]
+#[derive(Clone, Debug)]
+pub struct OutputOfSentTx {
+    value: NonNegativeAmount,
+    external_recipient: Option<Address>,
+    ephemeral_address: Option<(Address, u32)>,
+}
+
+#[cfg(any(test, feature = "test-dependencies"))]
+impl OutputOfSentTx {
+    /// Constructs an output from its test-relevant parts.
+    ///
+    /// If the output is to an ephemeral address, `ephemeral_address` should contain the
+    /// address along with the `address_index` it was derived from under the BIP 32 path
+    /// `m/44'/<coin_type>'/<account>'/2/<address_index>`.
+    pub fn from_parts(
+        value: NonNegativeAmount,
+        external_recipient: Option<Address>,
+        ephemeral_address: Option<(Address, u32)>,
+    ) -> Self {
+        Self {
+            value,
+            external_recipient,
+            ephemeral_address,
+        }
+    }
+}
+
 /// The relevance of a seed to a given wallet.
 ///
 /// This is the return type for [`WalletRead::seed_relevance_to_derived_accounts`].

zcash_client_backend/src/data_api/testing/pool.rs

@@ -318,7 +318,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
     DSF: DataStoreFactory,
     <DSF as DataStoreFactory>::AccountId: std::fmt::Debug,
 {
-    use crate::data_api::GAP_LIMIT;
+    use crate::data_api::{OutputOfSentTx, GAP_LIMIT};
 
     let mut st = TestBuilder::new()
         .with_data_store_factory(ds_factory)
@@ -409,7 +409,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
         // Check that there are sent outputs with the correct values.
         let confirmed_sent: Vec<Vec<_>> = txids
             .iter()
-            .map(|sent_txid| st.wallet().get_confirmed_sends(sent_txid).unwrap())
+            .map(|sent_txid| st.wallet().get_sent_outputs(sent_txid).unwrap())
             .collect();
 
         // Verify that a status request has been generated for the second transaction of
@@ -420,21 +420,24 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
         assert!(expected_step0_change < expected_ephemeral);
         assert_eq!(confirmed_sent.len(), 2);
         assert_eq!(confirmed_sent[0].len(), 2);
+        assert_eq!(confirmed_sent[0][0].value, expected_step0_change);
+        let OutputOfSentTx {
+            value: ephemeral_v,
+            external_recipient: to_addr,
+            ephemeral_address,
+        } = confirmed_sent[0][1].clone();
+        assert_eq!(ephemeral_v, expected_ephemeral);
+        assert!(to_addr.is_some());
         assert_eq!(
-            confirmed_sent[0][0].0,
-            u64::try_from(expected_step0_change).unwrap()
+            ephemeral_address,
+            to_addr.map(|addr| (addr, expected_index)),
         );
-        let (ephemeral_v, to_addr, ephemeral_addr, index) = confirmed_sent[0][1].clone();
-        assert_eq!(ephemeral_v, u64::try_from(expected_ephemeral).unwrap());
-        assert!(to_addr.is_some());
-        assert_eq!(ephemeral_addr, to_addr);
-        assert_eq!(index, Some(expected_index));
 
         assert_eq!(confirmed_sent[1].len(), 1);
         assert_matches!(
-            confirmed_sent[1][0].clone(),
-            (sent_v, sent_to_addr, None, None)
-            if sent_v == u64::try_from(transfer_amount).unwrap() && sent_to_addr == Some(tex_addr.encode(st.network())));
+            &confirmed_sent[1][0],
+            OutputOfSentTx { value: sent_v, external_recipient: sent_to_addr, ephemeral_address: None }
+            if sent_v == &transfer_amount && sent_to_addr == &Some(tex_addr));
 
         // Check that the transaction history matches what we expect.
         let tx_history = st.wallet().get_tx_history().unwrap();
@@ -466,7 +469,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
             -ZatBalance::from(expected_ephemeral),
         );
 
-        (ephemeral_addr.unwrap(), txids)
+        (ephemeral_address.unwrap().0, txids)
     };
 
     // Each transfer should use a different ephemeral address.
@@ -476,9 +479,8 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
 
     let height = add_funds(&mut st, value);
 
-    let ephemeral_taddr = Address::decode(st.network(), &ephemeral0).expect("valid address");
     assert_matches!(
-        ephemeral_taddr,
+        ephemeral0,
         Address::Transparent(TransparentAddress::PublicKeyHash(_))
     );
 
@@ -488,7 +490,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
             account_id,
             StandardFeeRule::Zip317,
             NonZeroU32::new(1).unwrap(),
-            &ephemeral_taddr,
+            &ephemeral0,
             transfer_amount,
             None,
             None,
@@ -503,7 +505,7 @@ pub fn send_multi_step_proposed_transfer<T: ShieldedPoolTester, DSF>(
     );
     assert_matches!(
         &create_proposed_result,
-        Err(Error::PaysEphemeralTransparentAddress(address_str)) if address_str == &ephemeral0);
+        Err(Error::PaysEphemeralTransparentAddress(address_str)) if address_str == &ephemeral0.encode(st.network()));
 
     // Simulate another wallet sending to an ephemeral address with an index
     // within the current gap limit. The `PaysEphemeralTransparentAddress` error

zcash_client_sqlite/src/lib.rs

@@ -99,7 +99,10 @@ use maybe_rayon::{
 };
 
 #[cfg(any(test, feature = "test-dependencies"))]
-use zcash_client_backend::data_api::{testing::TransactionSummary, WalletTest};
+use {
+    zcash_client_backend::data_api::{testing::TransactionSummary, OutputOfSentTx, WalletTest},
+    zcash_keys::address::Address,
+};
 
 /// `maybe-rayon` doesn't provide this as a fallback, so we have to.
 #[cfg(not(feature = "multicore"))]
@@ -654,11 +657,10 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletTest for W
         Ok(note_ids)
     }
 
-    fn get_confirmed_sends(
+    fn get_sent_outputs(
         &self,
         txid: &TxId,
-    ) -> Result<Vec<(u64, Option<String>, Option<String>, Option<u32>)>, <Self as WalletRead>::Error>
-    {
+    ) -> Result<Vec<OutputOfSentTx>, <Self as WalletRead>::Error> {
         let mut stmt_sent = self
             .conn.borrow()
             .prepare(
@@ -672,12 +674,24 @@ impl<C: Borrow<rusqlite::Connection>, P: consensus::Parameters> WalletTest for W
 
         let sends = stmt_sent
             .query_map(rusqlite::params![txid.as_ref()], |row| {
-                let v: u32 = row.get(0)?;
-                let to_address: Option<String> = row.get(1)?;
-                let ephemeral_address: Option<String> = row.get(2)?;
+                let v = row.get(0)?;
+                let to_address = row
+                    .get::<_, Option<String>>(1)?
+                    .and_then(|s| Address::decode(&self.params, &s));
+                let ephemeral_address = row
+                    .get::<_, Option<String>>(2)?
+                    .and_then(|s| Address::decode(&self.params, &s));
                 let address_index: Option<u32> = row.get(3)?;
-                Ok((u64::from(v), to_address, ephemeral_address, address_index))
+                Ok((v, to_address, ephemeral_address.zip(address_index)))
             })?
+            .map(|res| {
+                let (amount, external_recipient, ephemeral_address) = res?;
+                Ok::<_, <Self as WalletRead>::Error>(OutputOfSentTx::from_parts(
+                    NonNegativeAmount::from_u64(amount)?,
+                    external_recipient,
+                    ephemeral_address,
+                ))
+            })
             .collect::<Result<_, _>>()?;
 
         Ok(sends)