Add mappings from Orchard receivers to IVKs to the wallet

We add the Orchard change address mapping on account creation, as we
only ever generate a single change address internally. External Orchard
receivers are added to the map when `z_getaddressforaccount` is called.

This commit also fixes two bugs in the handling of Orchard internal IVKs.
When adding a full viewing key to the Orchard (Rust) wallet, we need to
derive both the internal and external IVKs (and map them to the external
FVK so we effectively never expose the internal FVK). We were only
deriving the external IVK, meaning that trying to add the Orchard change
address to the wallet should have failed. However, in a separate bug the
C++ internal IVK derivation method was calling the Rust external IVK
derivation function, meaning that we were effectively using the external
Orchard address at index 0 _as_ the change Orchard address.

src/gtest/test_keystore.cpp

@@ -578,6 +578,35 @@ TEST(KeystoreTests, StoreAndRetrieveUFVK) {
     EXPECT_FALSE(ufvkmeta.value().second.has_value());
 }
 
+TEST(KeystoreTests, StoreAndRetrieveUFVKByOrchard) {
+    SelectParams(CBaseChainParams::TESTNET);
+    CBasicKeyStore keyStore;
+
+    auto seed = MnemonicSeed::Random(SLIP44_TESTNET_TYPE);
+    auto usk = ZcashdUnifiedSpendingKey::ForAccount(seed, SLIP44_TESTNET_TYPE, 0);
+    EXPECT_TRUE(usk.has_value());
+
+    auto ufvk = usk.value().ToFullViewingKey();
+    auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), ufvk);
+    auto ufvkid = zufvk.GetKeyID();
+    EXPECT_FALSE(keyStore.GetUnifiedFullViewingKey(ufvkid).has_value());
+
+    EXPECT_TRUE(keyStore.AddUnifiedFullViewingKey(zufvk));
+    EXPECT_EQ(keyStore.GetUnifiedFullViewingKey(ufvkid).value(), zufvk);
+
+    auto addrPair = std::get<std::pair<UnifiedAddress, diversifier_index_t>>(zufvk.FindAddress(diversifier_index_t(0), {ReceiverType::Orchard}));
+    EXPECT_TRUE(addrPair.first.GetOrchardReceiver().has_value());
+    auto orchardReceiver = addrPair.first.GetOrchardReceiver().value();
+
+    // We don't store Orchard addresses in CBasicKeyStore (the addr -> ivk
+    // mapping is stored in the Rust wallet), but we still detect this because
+    // we trial-decrypt diversifiers (which also means we learn the index).
+    auto ufvkmetaUnadded = keyStore.GetUFVKMetadataForReceiver(orchardReceiver);
+    EXPECT_TRUE(ufvkmetaUnadded.has_value());
+    EXPECT_EQ(ufvkmetaUnadded.value().first, ufvkid);
+    EXPECT_EQ(ufvkmetaUnadded.value().second.value(), addrPair.second);
+}
+
 TEST(KeystoreTests, AddTransparentReceiverForUnifiedAddress) {
     SelectParams(CBaseChainParams::TESTNET);
     CBasicKeyStore keyStore;

src/rust/src/wallet.rs

@@ -73,8 +73,12 @@ impl KeyStore {
     }
 
     pub fn add_full_viewing_key(&mut self, fvk: FullViewingKey) {
-        let ivk = IncomingViewingKey::from(&fvk);
-        self.viewing_keys.insert(ivk, fvk);
+        // When we add a full viewing key, we need to add both the internal and external
+        // incoming viewing keys.
+        let external_ivk = IncomingViewingKey::from(&fvk);
+        let internal_ivk = IncomingViewingKey::from(&fvk.derive_internal());
+        self.viewing_keys.insert(external_ivk, fvk.clone());
+        self.viewing_keys.insert(internal_ivk, fvk);
     }
 
     pub fn add_spending_key(&mut self, sk: SpendingKey) {

src/wallet/gtest/test_wallet.cpp

@@ -44,6 +44,10 @@ class TestWallet : public CWallet {
 public:
     TestWallet(const CChainParams& params) : CWallet(params) { }
 
+    OrchardWallet& GetOrchardWallet() {
+        return orchardWallet;
+    }
+
     bool EncryptKeys(CKeyingMaterial& vMasterKeyIn) {
         return CCryptoKeyStore::EncryptKeys(vMasterKeyIn);
     }
@@ -2245,3 +2249,43 @@ TEST(WalletTests, GenerateUnifiedAddress) {
     // Revert to default
     RegtestDeactivateSapling();
 }
+
+TEST(WalletTests, GenerateUnifiedSpendingKeyAddsOrchardAddresses) {
+    (void) RegtestActivateSapling();
+    TestWallet wallet(Params());
+    wallet.GenerateNewSeed();
+
+    // Create an account.
+    auto ufvkpair = wallet.GenerateNewUnifiedSpendingKey();
+    auto ufvk = ufvkpair.first;
+    auto account = ufvkpair.second;
+    auto fvk = ufvk.GetOrchardKey();
+    EXPECT_TRUE(fvk.has_value());
+
+    // At this point the Orchard wallet should contain the change address, but
+    // no other addresses. We detect this by trying to look up their IVKs.
+    auto changeAddr = fvk->ToInternalIncomingViewingKey().Address(libzcash::diversifier_index_t{0});
+    auto internalIvk = wallet.GetOrchardWallet().GetIncomingViewingKeyForAddress(changeAddr);
+    EXPECT_TRUE(internalIvk.has_value());
+    EXPECT_EQ(internalIvk.value(), fvk->ToInternalIncomingViewingKey());
+    auto externalAddr = fvk->ToIncomingViewingKey().Address(libzcash::diversifier_index_t{0});
+    auto externalIvk = wallet.GetOrchardWallet().GetIncomingViewingKeyForAddress(externalAddr);
+    EXPECT_FALSE(externalIvk.has_value());
+
+    // Generate an address.
+    auto uaResult = wallet.GenerateUnifiedAddress(account, {ReceiverType::Orchard});
+    auto ua = std::get_if<std::pair<libzcash::UnifiedAddress, libzcash::diversifier_index_t>>(&uaResult);
+    EXPECT_NE(ua, nullptr);
+
+    auto uaOrchardReceiver = ua->first.GetOrchardReceiver();
+    EXPECT_TRUE(uaOrchardReceiver.has_value());
+    EXPECT_EQ(uaOrchardReceiver.value(), externalAddr);
+
+    // Now we can look up the external IVK.
+    externalIvk = wallet.GetOrchardWallet().GetIncomingViewingKeyForAddress(externalAddr);
+    EXPECT_TRUE(externalIvk.has_value());
+    EXPECT_EQ(externalIvk.value(), fvk->ToIncomingViewingKey());
+
+    // Revert to default
+    RegtestDeactivateSapling();
+}

src/wallet/wallet.cpp

@@ -268,6 +268,25 @@ bool CWallet::AddOrchardFullViewingKey(const libzcash::OrchardFullViewingKey &fv
     return true; // TODO ORCHARD: persist fvk
 }
 
+// Add Orchard payment address -> incoming viewing key map entry
+bool CWallet::AddOrchardRawAddress(
+    const libzcash::OrchardIncomingViewingKey &ivk,
+    const libzcash::OrchardRawAddress &addr)
+{
+    AssertLockHeld(cs_wallet); // orchardWallet
+    if (!orchardWallet.AddRawAddress(addr, ivk)) {
+        // We should never add an Orchard raw address for which we don't know
+        // the corresponding FVK.
+        return false;
+    };
+
+    if (!fFileBacked) {
+        return true;
+    }
+
+    return true; // TODO ORCHARD: ensure mapping will be recreated on wallet load
+}
+
 // Loads a payment address -> incoming viewing key map entry
 // to the in-memory wallet's keystore.
 bool CWallet::LoadOrchardRawAddress(
@@ -577,7 +596,17 @@ std::optional<libzcash::ZcashdUnifiedSpendingKey>
         };
 
         // Add Orchard spending key to the wallet
-        orchardWallet.AddSpendingKey(usk.value().GetOrchardKey());
+        auto orchardSk = usk.value().GetOrchardKey();
+        orchardWallet.AddSpendingKey(orchardSk);
+
+        // Associate the Orchard default change address with its IVK. We do this
+        // here because there is only ever a single Orchard change receiver, and
+        // it is never exposed to the user. External Orchard receivers are added
+        // when the user calls z_getaddressforaccount.
+        auto orchardInternalFvk = orchardSk.ToFullViewingKey().ToInternalIncomingViewingKey();
+        if (!AddOrchardRawAddress(orchardInternalFvk, orchardInternalFvk.Address(0))) {
+            throw std::runtime_error("CWallet::GenerateUnifiedSpendingKeyForAccount(): Failed to add Sapling change address to the wallet.");
+        };
 
         auto zufvk = ZcashdUnifiedFullViewingKey::FromUnifiedFullViewingKey(Params(), ufvk);
         if (!CCryptoKeyStore::AddUnifiedFullViewingKey(zufvk)) {
@@ -767,6 +796,17 @@ WalletUAGenerationResult CWallet::GenerateUnifiedAddress(
             AddSaplingPaymentAddress(dfvk.value().ToIncomingViewingKey(), saplingAddress.value());
         }
 
+        // If the address has an Orchard component, add an association between
+        // that address and the Orchard IVK corresponding to the ufvk
+        auto hasOrchard = receiverTypes.find(ReceiverType::Orchard) != receiverTypes.end();
+        if (hasOrchard) {
+            auto fvk = ufvk.value().GetOrchardKey();
+            auto orchardReceiver = address.first.GetOrchardReceiver();
+            assert (fvk.has_value() && orchardReceiver.has_value());
+
+            AddOrchardRawAddress(fvk.value().ToIncomingViewingKey(), orchardReceiver.value());
+        }
+
         // Save the metadata for the generated address so that we can re-derive
         // it in the future.
         ZcashdUnifiedAddressMetadata addrmeta(ufvkid, address.second, receiverTypes);

src/wallet/wallet.h

@@ -1463,7 +1463,15 @@ public:
     bool AddOrchardZKey(const libzcash::OrchardSpendingKey &sk);
     bool AddOrchardFullViewingKey(const libzcash::OrchardFullViewingKey &fvk);
     /**
-     * Adds an address/ivk mapping to the in-memory wallet. Returns `true`
+     * Adds an address/ivk mapping to the in-memory wallet. Returns `false` if
+     * the mapping could not be persisted, or the IVK does not correspond to an
+     * FVK known by the wallet.
+     */
+    bool AddOrchardRawAddress(
+        const libzcash::OrchardIncomingViewingKey &ivk,
+        const libzcash::OrchardRawAddress &addr);
+    /**
+     * Loads an address/ivk mapping to the in-memory wallet. Returns `true`
      * if the provided IVK corresponds to an FVK known by the wallet.
      */
     bool LoadOrchardRawAddress(

src/zcash/address/orchard.cpp

@@ -24,7 +24,7 @@ OrchardIncomingViewingKey OrchardFullViewingKey::ToIncomingViewingKey() const {
 }
 
 OrchardIncomingViewingKey OrchardFullViewingKey::ToInternalIncomingViewingKey() const {
-    return OrchardIncomingViewingKey(orchard_full_viewing_key_to_incoming_viewing_key(inner.get()));
+    return OrchardIncomingViewingKey(orchard_full_viewing_key_to_internal_incoming_viewing_key(inner.get()));
 }
 
 OrchardSpendingKey OrchardSpendingKey::ForAccount(