From 878c4b1b507b9bfc8b9210955a56ba536010f62c Mon Sep 17 00:00:00 2001
From: Jack Grigg <jack@z.cash>
Date: Thu, 20 Oct 2016 10:32:20 -0500
Subject: [PATCH] Clear witness cache when re-witnessing notes

Closes #1378
---
 src/wallet/gtest/test_wallet.cpp |  8 ++++++++
 src/wallet/wallet.cpp            | 16 +++++++++++++++-
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp
index c1bf5d34c..44eb86fd7 100644
--- a/src/wallet/gtest/test_wallet.cpp
+++ b/src/wallet/gtest/test_wallet.cpp
@@ -734,6 +734,14 @@ TEST(wallet_tests, cached_witnesses_chain_tip) {
         wallet.GetNoteWitnesses(notes, witnesses, anchor4);
         EXPECT_TRUE((bool) witnesses[0]);
         EXPECT_EQ(anchor2, anchor4);
+
+        // Incrementing with the same block again should not change the cache
+        uint256 anchor5;
+        wallet.IncrementNoteWitnesses(&index2, &block2, tree);
+        std::vector<boost::optional<ZCIncrementalWitness>> witnesses5;
+        wallet.GetNoteWitnesses(notes, witnesses5, anchor5);
+        EXPECT_EQ(witnesses, witnesses5);
+        EXPECT_EQ(anchor4, anchor5);
     }
 }
 
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 1a865a966..21bc1d512 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -699,7 +699,21 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex,
                         JSOutPoint jsoutpt {hash, i, j};
                         if (mapWallet[hash].mapNoteData.count(jsoutpt)) {
                             CNoteData* nd = &(mapWallet[hash].mapNoteData[jsoutpt]);
-                            assert(nd->witnesses.size() == 0);
+                            if (nd->witnesses.size() > 0) {
+                                // We think this can happen because we write out the
+                                // witness cache state after every block increment or
+                                // decrement, but the block index itself is written in
+                                // batches. So if the node crashes in between these two
+                                // operations, it is possible for IncrementNoteWitnesses
+                                // to be called again on previously-cached blocks. This
+                                // doesn't affect existing cached notes because of the
+                                // CNoteData::witnessHeight checks. See #1378 for details.
+                                LogPrintf("Inconsistent witness cache state found for %s\n- Cache size: %d\n- Top: %s\n- New: %s\n",
+                                          jsoutpt.ToString(), nd->witnesses.size(),
+                                          nd->witnesses.front().root().GetHex(),
+                                          tree.witness().root().GetHex());
+                                nd->witnesses.clear();
+                            }
                             nd->witnesses.push_front(tree.witness());
                             // Set height to one less than pindex so it gets incremented
                             nd->witnessHeight = pindex->nHeight - 1;