Merge pull request #359 from str4d/356-create_spend_to_address-assumption

zcash_client_backend: Use correct output index for t-addr recipients

zcash_client_backend/src/data_api.rs

@@ -195,6 +195,12 @@ pub struct ReceivedTransaction<'a> {
 pub struct SentTransaction<'a> {
     pub tx: &'a Transaction,
     pub created: time::OffsetDateTime,
+    /// The index within the transaction that contains the recipient output.
+    ///
+    /// - If `recipient_address` is a Sapling address, this is an index into the Sapling
+    ///   outputs of the transaction.
+    /// - If `recipient_address` is a transparent address, this is an index into the
+    ///   transparent outputs of the transaction.
     pub output_index: usize,
     pub account: AccountId,
     pub recipient_address: &'a RecipientAddress,

zcash_client_backend/src/data_api/wallet.rs

@@ -229,16 +229,27 @@ where
         .build(consensus_branch_id, &prover)
         .map_err(Error::Builder)?;
 
-    // We only called add_sapling_output() once.
-    let output_index = match tx_metadata.output_index(0) {
-        Some(idx) => idx as i64,
-        None => panic!("Output 0 should exist in the transaction"),
+    let output_index = match to {
+        // Sapling outputs are shuffled, so we need to look up where the output ended up.
+        RecipientAddress::Shielded(_) => match tx_metadata.output_index(0) {
+            Some(idx) => idx,
+            None => panic!("Output 0 should exist in the transaction"),
+        },
+        RecipientAddress::Transparent(addr) => {
+            let script = addr.script();
+            tx.vout
+                .iter()
+                .enumerate()
+                .find(|(_, tx_out)| tx_out.script_pubkey == script)
+                .map(|(index, _)| index)
+                .expect("we sent to this address")
+        }
     };
 
     wallet_db.store_sent_tx(&SentTransaction {
         tx: &tx,
         created: time::OffsetDateTime::now_utc(),
-        output_index: output_index as usize,
+        output_index,
         account,
         recipient_address: to,
         value,

zcash_client_sqlite/src/wallet.rs

@@ -708,6 +708,14 @@ pub fn put_sent_note<'a, P: consensus::Parameters>(
     Ok(())
 }
 
+/// Inserts a sent note into the wallet database.
+///
+/// `output_index` is the index within the transaction that contains the recipient output:
+///
+/// - If `to` is a Sapling address, this is an index into the Sapling outputs of the
+///   transaction.
+/// - If `to` is a transparent address, this is an index into the transparent outputs of
+///   the transaction.
 pub fn insert_sent_note<'a, P: consensus::Parameters>(
     stmts: &mut DataConnStmtCache<'a, P>,
     tx_ref: i64,

zcash_client_sqlite/src/wallet/transact.rs

@@ -154,6 +154,7 @@ mod tests {
     use zcash_primitives::{
         block::BlockHash,
         consensus::BlockHeight,
+        legacy::TransparentAddress,
         note_encryption::try_sapling_output_recovery,
         prover::TxProver,
         transaction::{components::Amount, Transaction},
@@ -665,4 +666,54 @@ mod tests {
         // Neither transaction output is decryptable by the sender.
         assert!(send_and_recover_with_policy(&mut db_write, OvkPolicy::Discard).is_none());
     }
+
+    #[test]
+    fn create_to_address_succeeds_to_t_addr_zero_change() {
+        let cache_file = NamedTempFile::new().unwrap();
+        let db_cache = BlockDB(Connection::open(cache_file.path()).unwrap());
+        init_cache_database(&db_cache).unwrap();
+
+        let data_file = NamedTempFile::new().unwrap();
+        let db_data = WalletDB::for_path(data_file.path(), tests::network()).unwrap();
+        init_wallet_db(&db_data).unwrap();
+
+        // Add an account to the wallet
+        let extsk = ExtendedSpendingKey::master(&[]);
+        let extfvk = ExtendedFullViewingKey::from(&extsk);
+        init_accounts_table(&db_data, &[extfvk.clone()]).unwrap();
+
+        // Add funds to the wallet in a single note
+        let value = Amount::from_u64(51000).unwrap();
+        let (cb, _) = fake_compact_block(
+            sapling_activation_height(),
+            BlockHash([0; 32]),
+            extfvk.clone(),
+            value,
+        );
+        insert_into_cache(&db_cache, &cb);
+        let mut db_write = db_data.get_update_ops().unwrap();
+        scan_cached_blocks(&tests::network(), &db_cache, &mut db_write, None).unwrap();
+
+        // Verified balance matches total balance
+        let (_, anchor_height) = (&db_data).get_target_and_anchor_heights().unwrap().unwrap();
+        assert_eq!(get_balance(&db_data, AccountId(0)).unwrap(), value);
+        assert_eq!(
+            get_balance_at(&db_data, AccountId(0), anchor_height).unwrap(),
+            value
+        );
+
+        let to = TransparentAddress::PublicKey([7; 20]).into();
+        create_spend_to_address(
+            &mut db_write,
+            &tests::network(),
+            test_prover(),
+            AccountId(0),
+            &extsk,
+            &to,
+            Amount::from_u64(50000).unwrap(),
+            None,
+            OvkPolicy::Sender,
+        )
+        .unwrap();
+    }
 }