diff --git a/src/init.cpp b/src/init.cpp index dcb317b7d..ee0b27613 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -537,11 +537,6 @@ void ThreadImport(std::vector vImportFiles) RenameThread("zcash-loadblk"); // -reindex if (fReindex) { -#ifdef ENABLE_WALLET - if (pwalletMain) { - pwalletMain->ClearNoteWitnessCache(); - } -#endif CImportingNow imp; int nFile = 0; while (true) { @@ -1379,7 +1374,10 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler) CBlockIndex *pindexRescan = chainActive.Tip(); if (GetBoolArg("-rescan", false)) + { + pwalletMain->ClearNoteWitnessCache(); pindexRescan = chainActive.Genesis(); + } else { CWalletDB walletdb(strWalletFile); diff --git a/src/primitives/block.h b/src/primitives/block.h index 6180fb2ae..6b3f13a86 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -200,6 +200,10 @@ struct CBlockLocator { return vHave.empty(); } + + friend bool operator==(const CBlockLocator& a, const CBlockLocator& b) { + return (a.vHave == b.vHave); + } }; #endif // BITCOIN_PRIMITIVES_BLOCK_H diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index d30944954..0530bdb53 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -5,6 +5,7 @@ #include "base58.h" #include "chainparams.h" #include "main.h" +#include "primitives/block.h" #include "random.h" #include "wallet/wallet.h" #include "zcash/JoinSplit.hpp" @@ -29,9 +30,11 @@ public: MOCK_METHOD2(WriteTx, bool(uint256 hash, const CWalletTx& wtx)); MOCK_METHOD1(WriteWitnessCacheSize, bool(int64_t nWitnessCacheSize)); + MOCK_METHOD1(WriteBestBlock, bool(const CBlockLocator& loc)); }; -template void CWallet::WriteWitnessCache(MockWalletDB& walletdb); +template void CWallet::SetBestChainINTERNAL( + MockWalletDB& walletdb, const CBlockLocator& loc); class TestWallet : public CWallet { public: @@ -53,8 +56,8 @@ public: void DecrementNoteWitnesses(const CBlockIndex* pindex) { CWallet::DecrementNoteWitnesses(pindex); } - void WriteWitnessCache(MockWalletDB& walletdb) { - CWallet::WriteWitnessCache(walletdb); + void SetBestChain(MockWalletDB& walletdb, const CBlockLocator& loc) { + CWallet::SetBestChainINTERNAL(walletdb, loc); } bool UpdatedNoteData(const CWalletTx& wtxIn, CWalletTx& wtx) { return CWallet::UpdatedNoteData(wtxIn, wtx); @@ -918,6 +921,7 @@ TEST(wallet_tests, ClearNoteWitnessCache) { TEST(wallet_tests, WriteWitnessCache) { TestWallet wallet; MockWalletDB walletdb; + CBlockLocator loc; auto sk = libzcash::SpendingKey::random(); wallet.AddSpendingKey(sk); @@ -928,7 +932,7 @@ TEST(wallet_tests, WriteWitnessCache) { // TxnBegin fails EXPECT_CALL(walletdb, TxnBegin()) .WillOnce(Return(false)); - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); EXPECT_CALL(walletdb, TxnBegin()) .WillRepeatedly(Return(true)); @@ -937,14 +941,14 @@ TEST(wallet_tests, WriteWitnessCache) { .WillOnce(Return(false)); EXPECT_CALL(walletdb, TxnAbort()) .Times(1); - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); // WriteTx throws EXPECT_CALL(walletdb, WriteTx(wtx.GetHash(), wtx)) .WillOnce(ThrowLogicError()); EXPECT_CALL(walletdb, TxnAbort()) .Times(1); - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); EXPECT_CALL(walletdb, WriteTx(wtx.GetHash(), wtx)) .WillRepeatedly(Return(true)); @@ -953,26 +957,42 @@ TEST(wallet_tests, WriteWitnessCache) { .WillOnce(Return(false)); EXPECT_CALL(walletdb, TxnAbort()) .Times(1); - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); // WriteWitnessCacheSize throws EXPECT_CALL(walletdb, WriteWitnessCacheSize(0)) .WillOnce(ThrowLogicError()); EXPECT_CALL(walletdb, TxnAbort()) .Times(1); - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); EXPECT_CALL(walletdb, WriteWitnessCacheSize(0)) .WillRepeatedly(Return(true)); + // WriteBestBlock fails + EXPECT_CALL(walletdb, WriteBestBlock(loc)) + .WillOnce(Return(false)); + EXPECT_CALL(walletdb, TxnAbort()) + .Times(1); + wallet.SetBestChain(walletdb, loc); + + // WriteBestBlock throws + EXPECT_CALL(walletdb, WriteBestBlock(loc)) + .WillOnce(ThrowLogicError()); + EXPECT_CALL(walletdb, TxnAbort()) + .Times(1); + wallet.SetBestChain(walletdb, loc); + EXPECT_CALL(walletdb, WriteBestBlock(loc)) + .WillRepeatedly(Return(true)); + // TxCommit fails EXPECT_CALL(walletdb, TxnCommit()) .WillOnce(Return(false)); - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); EXPECT_CALL(walletdb, TxnCommit()) .WillRepeatedly(Return(true)); // Everything succeeds - wallet.WriteWitnessCache(walletdb); + wallet.SetBestChain(walletdb, loc); } TEST(wallet_tests, UpdateNullifierNoteMap) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 51ed76f3a..c9dac701d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -380,7 +380,7 @@ void CWallet::ChainTip(const CBlockIndex *pindex, const CBlock *pblock, void CWallet::SetBestChain(const CBlockLocator& loc) { CWalletDB walletdb(strWalletFile); - walletdb.WriteBestBlock(loc); + SetBestChainINTERNAL(walletdb, loc); } bool CWallet::SetMinVersion(enum WalletFeature nVersion, CWalletDB* pwalletdbIn, bool fExplicit) @@ -741,10 +741,9 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, } } - if (fFileBacked) { - CWalletDB walletdb(strWalletFile); - WriteWitnessCache(walletdb); - } + // For performance reasons, we write out the witness cache in + // CWallet::SetBestChain() (which also ensures that overall consistency + // of the wallet.dat is maintained). } } @@ -779,10 +778,10 @@ void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) } // TODO: If nWitnessCache is zero, we need to regenerate the caches (#1302) assert(nWitnessCacheSize > 0); - if (fFileBacked) { - CWalletDB walletdb(strWalletFile); - WriteWitnessCache(walletdb); - } + + // For performance reasons, we write out the witness cache in + // CWallet::SetBestChain() (which also ensures that overall consistency + // of the wallet.dat is maintained). } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 85f960eb1..cc0197f14 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -636,35 +636,40 @@ protected: void DecrementNoteWitnesses(const CBlockIndex* pindex); template - void WriteWitnessCache(WalletDB& walletdb) { + void SetBestChainINTERNAL(WalletDB& walletdb, const CBlockLocator& loc) { if (!walletdb.TxnBegin()) { // This needs to be done atomically, so don't do it at all - LogPrintf("WriteWitnessCache(): Couldn't start atomic write\n"); + LogPrintf("SetBestChain(): Couldn't start atomic write\n"); return; } try { for (std::pair& wtxItem : mapWallet) { if (!walletdb.WriteTx(wtxItem.first, wtxItem.second)) { - LogPrintf("WriteWitnessCache(): Failed to write CWalletTx, aborting atomic write\n"); + LogPrintf("SetBestChain(): Failed to write CWalletTx, aborting atomic write\n"); walletdb.TxnAbort(); return; } } if (!walletdb.WriteWitnessCacheSize(nWitnessCacheSize)) { - LogPrintf("WriteWitnessCache(): Failed to write nWitnessCacheSize, aborting atomic write\n"); + LogPrintf("SetBestChain(): Failed to write nWitnessCacheSize, aborting atomic write\n"); + walletdb.TxnAbort(); + return; + } + if (!walletdb.WriteBestBlock(loc)) { + LogPrintf("SetBestChain(): Failed to write best block, aborting atomic write\n"); walletdb.TxnAbort(); return; } } catch (const std::exception &exc) { // Unexpected failure - LogPrintf("WriteWitnessCache(): Unexpected error during atomic write:\n"); + LogPrintf("SetBestChain(): Unexpected error during atomic write:\n"); LogPrintf("%s\n", exc.what()); walletdb.TxnAbort(); return; } if (!walletdb.TxnCommit()) { // Couldn't commit all to db, but in-memory state is fine - LogPrintf("WriteWitnessCache(): Couldn't commit atomic write\n"); + LogPrintf("SetBestChain(): Couldn't commit atomic write\n"); return; } }