Auto merge of #1444 - str4d:1394-reindex-clear-witness-caches, r=str4d

Clear note witness caches on reindexing

This PR also fixes a test that was passing arguments in the wrong order.

Closes #1394

src/init.cpp

@@ -532,6 +532,11 @@ void ThreadImport(std::vector<boost::filesystem::path> vImportFiles)
     RenameThread("bitcoin-loadblk");
     // -reindex
     if (fReindex) {
+#ifdef ENABLE_WALLET
+        if (pwalletMain) {
+            pwalletMain->ClearNoteWitnessCache();
+        }
+#endif
         CImportingNow imp;
         int nFile = 0;
         while (true) {

src/wallet/gtest/test_wallet.cpp

@@ -639,6 +639,46 @@ TEST(wallet_tests, cached_witnesses_chain_tip) {
     }
 }
 
+TEST(wallet_tests, ClearNoteWitnessCache) {
+    TestWallet wallet;
+
+    auto sk = libzcash::SpendingKey::random();
+    wallet.AddSpendingKey(sk);
+
+    auto wtx = GetValidReceive(sk, 10, true);
+    auto note = GetNote(sk, wtx, 0, 0);
+    auto nullifier = note.nullifier(sk);
+
+    mapNoteData_t noteData;
+    JSOutPoint jsoutpt {wtx.GetHash(), 0, 0};
+    JSOutPoint jsoutpt2 {wtx.GetHash(), 0, 1};
+    CNoteData nd {sk.address(), nullifier};
+    noteData[jsoutpt] = nd;
+    wtx.SetNoteData(noteData);
+
+    // Pretend we mined the tx by adding a fake witness
+    ZCIncrementalMerkleTree tree;
+    wtx.mapNoteData[jsoutpt].witnesses.push_front(tree.witness());
+
+    wallet.AddToWallet(wtx, true, NULL);
+
+    std::vector<JSOutPoint> notes {jsoutpt, jsoutpt2};
+    std::vector<boost::optional<ZCIncrementalWitness>> witnesses;
+    uint256 anchor2;
+
+    // Before clearing, we should have a witness for one note
+    wallet.GetNoteWitnesses(notes, witnesses, anchor2);
+    EXPECT_TRUE((bool) witnesses[0]);
+    EXPECT_FALSE((bool) witnesses[1]);
+
+    // After clearing, we should not have a witness for either note
+    wallet.ClearNoteWitnessCache();
+    witnesses.clear();
+    wallet.GetNoteWitnesses(notes, witnesses, anchor2);
+    EXPECT_FALSE((bool) witnesses[0]);
+    EXPECT_FALSE((bool) witnesses[1]);
+}
+
 TEST(wallet_tests, UpdatedNoteData) {
     TestWallet wallet;
 
@@ -674,12 +714,12 @@ TEST(wallet_tests, UpdatedNoteData) {
 
     // The txs should initially be different
     EXPECT_NE(wtx.mapNoteData, wtx2.mapNoteData);
-    EXPECT_NE(wtx.mapNoteData[jsoutpt].witnesses, wtx2.mapNoteData[jsoutpt].witnesses);
+    EXPECT_EQ(1, wtx.mapNoteData[jsoutpt].witnesses.size());
 
     // After updating, they should be the same
-    EXPECT_TRUE(wallet.UpdatedNoteData(wtx, wtx2));
+    EXPECT_TRUE(wallet.UpdatedNoteData(wtx2, wtx));
     EXPECT_EQ(wtx.mapNoteData, wtx2.mapNoteData);
-    EXPECT_EQ(wtx.mapNoteData[jsoutpt].witnesses, wtx2.mapNoteData[jsoutpt].witnesses);
+    EXPECT_EQ(1, wtx.mapNoteData[jsoutpt].witnesses.size());
     // TODO: The new note should get witnessed (but maybe not here) (#1350)
 }
 

src/wallet/wallet.cpp

@@ -625,6 +625,16 @@ void CWallet::AddToSpends(const uint256& wtxid)
     }
 }
 
+void CWallet::ClearNoteWitnessCache()
+{
+    LOCK(cs_wallet);
+    for (std::pair<const uint256, CWalletTx>& wtxItem : mapWallet) {
+        for (mapNoteData_t::value_type& item : wtxItem.second.mapNoteData) {
+            item.second.witnesses.clear();
+        }
+    }
+}
+
 void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex,
                                      const CBlock* pblockIn,
                                      ZCIncrementalMerkleTree tree)

src/wallet/wallet.h

@@ -598,6 +598,8 @@ public:
      */
     int64_t nWitnessCacheSize;
 
+    void ClearNoteWitnessCache();
+
 protected:
     void IncrementNoteWitnesses(const CBlockIndex* pindex,
                                 const CBlock* pblock,