From cb6a99384059b0bacff56a0e1e4de498b0a610bd Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Wed, 24 Mar 2021 18:54:15 +1300
Subject: [PATCH] zcash_client_backend: Use correct output index for t-addr
 recipients

`create_spend_to_address` was originally written only for sending to
Sapling addresses. It was later amended to support sending to
transparent addresses, but the assumption about there being a Sapling
output was not removed. This was not an issue for most transactions
because there would be change, but in the case of a z->t transaction
with no change, `create_spend_to_address` would reliably panic.

This commit fixes the bug by setting the output index for transparent
recipients to 0. The `output_index` field of `SentTransaction` is also
documented to correctly reflect its dependency on the type of
`recipient_address`.
---
 zcash_client_backend/src/data_api.rs        |  6 ++++++
 zcash_client_backend/src/data_api/wallet.rs | 14 ++++++++++----
 zcash_client_sqlite/src/wallet.rs           |  8 ++++++++
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/zcash_client_backend/src/data_api.rs b/zcash_client_backend/src/data_api.rs
index f783e086a..f5188613d 100644
--- a/zcash_client_backend/src/data_api.rs
+++ b/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,
diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs
index 558c04ee4..9e6f91b02 100644
--- a/zcash_client_backend/src/data_api/wallet.rs
+++ b/zcash_client_backend/src/data_api/wallet.rs
@@ -229,10 +229,16 @@ 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 as i64,
+            None => panic!("Output 0 should exist in the transaction"),
+        },
+        // This function only spends shielded notes, so there will only ever be a single
+        // transparent output in this case (and even if there were more, we don't shuffle
+        // the transparent outputs).
+        RecipientAddress::Transparent(_) => 0,
     };
 
     wallet_db.store_sent_tx(&SentTransaction {
diff --git a/zcash_client_sqlite/src/wallet.rs b/zcash_client_sqlite/src/wallet.rs
index bf86ddef0..6a8e62e7f 100644
--- a/zcash_client_sqlite/src/wallet.rs
+++ b/zcash_client_sqlite/src/wallet.rs
@@ -715,6 +715,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,