From 7b5148cc64cff10e6426890253b0f82098b2bc56 Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Tue, 31 Dec 2019 12:18:16 -0700 Subject: [PATCH] fix tests for enable-debug build --- src/gtest/test_mempool.cpp | 6 ++++++ src/wallet/gtest/test_wallet.cpp | 28 ++++++++++++++++++++++++++ src/wallet/gtest/test_wallet_zkeys.cpp | 7 +++++++ src/wallet/test/rpc_wallet_tests.cpp | 20 +++++++++--------- src/wallet/wallet.cpp | 5 ++++- 5 files changed, 55 insertions(+), 11 deletions(-) diff --git a/src/gtest/test_mempool.cpp b/src/gtest/test_mempool.cpp index a2daa1ff4..48e322b76 100644 --- a/src/gtest/test_mempool.cpp +++ b/src/gtest/test_mempool.cpp @@ -118,6 +118,7 @@ TEST(Mempool, OverwinterNotActiveYet) { CValidationState state1; CTransaction tx1(mtx); + LOCK(cs_main); EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs)); EXPECT_EQ(state1.GetRejectReason(), "tx-overwinter-not-active"); @@ -142,6 +143,7 @@ TEST(Mempool, SproutV3TxFailsAsExpected) { CValidationState state1; CTransaction tx1(mtx); + LOCK(cs_main); EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs)); EXPECT_EQ(state1.GetRejectReason(), "version"); } @@ -163,6 +165,7 @@ TEST(Mempool, SproutV3TxWhenOverwinterActive) { CValidationState state1; CTransaction tx1(mtx); + LOCK(cs_main); EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs)); EXPECT_EQ(state1.GetRejectReason(), "tx-overwinter-flag-not-set"); @@ -198,6 +201,7 @@ TEST(Mempool, SproutNegativeVersionTxWhenOverwinterActive) { EXPECT_EQ(tx1.nVersion, -3); CValidationState state1; + LOCK(cs_main); EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs)); EXPECT_EQ(state1.GetRejectReason(), "bad-txns-version-too-low"); } @@ -214,6 +218,7 @@ TEST(Mempool, SproutNegativeVersionTxWhenOverwinterActive) { EXPECT_EQ(tx1.nVersion, -2147483645); CValidationState state1; + LOCK(cs_main); EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs)); EXPECT_EQ(state1.GetRejectReason(), "bad-txns-version-too-low"); } @@ -246,6 +251,7 @@ TEST(Mempool, ExpiringSoonTxRejection) { CValidationState state1; CTransaction tx1(mtx); + LOCK(cs_main); EXPECT_FALSE(AcceptToMemoryPool(pool, state1, tx1, false, &missingInputs)); EXPECT_EQ(state1.GetRejectReason(), "tx-expiring-soon"); } diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index cf2a12a00..8c09ddd14 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -185,6 +185,7 @@ TEST(WalletTests, FindUnspentSproutNotes) { auto consensusParams = RegtestActivateSapling(); CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -378,6 +379,7 @@ TEST(WalletTests, SetSaplingNoteAddrsInCWalletTx) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); auto sk = GetTestMasterSaplingSpendingKey(); auto expsk = sk.expsk; @@ -459,6 +461,7 @@ TEST(WalletTests, SetInvalidSaplingNoteDataInCWalletTx) { TEST(WalletTests, CheckSproutNoteCommitmentAgainstNotePlaintext) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); auto address = sk.address(); @@ -480,6 +483,7 @@ TEST(WalletTests, CheckSproutNoteCommitmentAgainstNotePlaintext) { TEST(WalletTests, GetSproutNoteNullifier) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); auto address = sk.address(); @@ -513,6 +517,7 @@ TEST(WalletTests, FindMySaplingNotes) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); // Generate dummy Sapling address auto sk = GetTestMasterSaplingSpendingKey(); @@ -546,6 +551,7 @@ TEST(WalletTests, FindMySaplingNotes) { TEST(WalletTests, FindMySproutNotes) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); auto sk2 = libzcash::SproutSpendingKey::random(); @@ -571,6 +577,7 @@ TEST(WalletTests, FindMySproutNotes) { TEST(WalletTests, FindMySproutNotesInEncryptedWallet) { TestWallet wallet; + LOCK(wallet.cs_wallet); uint256 r {GetRandHash()}; CKeyingMaterial vMasterKey (r.begin(), r.end()); @@ -601,6 +608,7 @@ TEST(WalletTests, FindMySproutNotesInEncryptedWallet) { TEST(WalletTests, GetConflictedSproutNotes) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -635,6 +643,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); // Generate Sapling address auto sk = GetTestMasterSaplingSpendingKey(); @@ -750,6 +759,7 @@ TEST(WalletTests, GetConflictedSaplingNotes) { TEST(WalletTests, SproutNullifierIsSpent) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -792,6 +802,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); // Generate dummy Sapling address auto sk = GetTestMasterSaplingSpendingKey(); @@ -847,6 +858,7 @@ TEST(WalletTests, SaplingNullifierIsSpent) { TEST(WalletTests, NavigateFromSproutNullifierToNote) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -875,6 +887,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); // Generate dummy Sapling address auto sk = GetTestMasterSaplingSpendingKey(); @@ -965,6 +978,7 @@ TEST(WalletTests, NavigateFromSaplingNullifierToNote) { TEST(WalletTests, SpentSproutNoteIsFromMe) { CWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -996,6 +1010,7 @@ TEST(WalletTests, SpentSaplingNoteIsFromMe) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); // Generate Sapling address auto sk = GetTestMasterSaplingSpendingKey(); @@ -1193,6 +1208,7 @@ TEST(WalletTests, CachedWitnessesEmptyChain) { TEST(WalletTests, CachedWitnessesChainTip) { TestWallet wallet; + LOCK(wallet.cs_wallet); std::pair anchors1; CBlock block1; SproutMerkleTree sproutTree; @@ -1295,6 +1311,7 @@ TEST(WalletTests, CachedWitnessesChainTip) { TEST(WalletTests, CachedWitnessesDecrementFirst) { TestWallet wallet; + LOCK(wallet.cs_wallet); SproutMerkleTree sproutTree; SaplingMerkleTree saplingTree; @@ -1375,6 +1392,7 @@ TEST(WalletTests, CachedWitnessesDecrementFirst) { TEST(WalletTests, CachedWitnessesCleanIndex) { TestWallet wallet; + LOCK(wallet.cs_wallet); std::vector blocks; std::vector indices; std::vector sproutNotes; @@ -1462,6 +1480,7 @@ TEST(WalletTests, CachedWitnessesCleanIndex) { TEST(WalletTests, ClearNoteWitnessCache) { TestWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -1520,6 +1539,7 @@ TEST(WalletTests, ClearNoteWitnessCache) { TEST(WalletTests, WriteWitnessCache) { TestWallet wallet; + LOCK(wallet.cs_wallet); MockWalletDB walletdb; CBlockLocator loc; @@ -1607,6 +1627,7 @@ TEST(WalletTests, SetBestChainIgnoresTxsWithoutShieldedData) { SelectParams(CBaseChainParams::REGTEST); TestWallet wallet; + LOCK(wallet.cs_wallet); MockWalletDB walletdb; CBlockLocator loc; @@ -1686,6 +1707,7 @@ TEST(WalletTests, SetBestChainIgnoresTxsWithoutShieldedData) { TEST(WalletTests, UpdateSproutNullifierNoteMap) { TestWallet wallet; + LOCK(wallet.cs_wallet); uint256 r {GetRandHash()}; CKeyingMaterial vMasterKey (r.begin(), r.end()); @@ -1721,6 +1743,7 @@ TEST(WalletTests, UpdateSproutNullifierNoteMap) { TEST(WalletTests, UpdatedSproutNoteData) { TestWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -1770,6 +1793,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); auto m = GetTestMasterSaplingSpendingKey(); @@ -1878,6 +1902,7 @@ TEST(WalletTests, UpdatedSaplingNoteData) { TEST(WalletTests, MarkAffectedSproutTransactionsDirty) { TestWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -1911,6 +1936,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { auto consensusParams = RegtestActivateSapling(); TestWallet wallet; + LOCK(wallet.cs_wallet); // Generate Sapling address auto sk = GetTestMasterSaplingSpendingKey(); @@ -2020,6 +2046,7 @@ TEST(WalletTests, MarkAffectedSaplingTransactionsDirty) { TEST(WalletTests, SproutNoteLocking) { TestWallet wallet; + LOCK(wallet.cs_wallet); auto sk = libzcash::SproutSpendingKey::random(); wallet.AddSproutSpendingKey(sk); @@ -2053,6 +2080,7 @@ TEST(WalletTests, SproutNoteLocking) { TEST(WalletTests, SaplingNoteLocking) { TestWallet wallet; + LOCK(wallet.cs_wallet); SaplingOutPoint sop1 {uint256(), 1}; SaplingOutPoint sop2 {uint256(), 2}; diff --git a/src/wallet/gtest/test_wallet_zkeys.cpp b/src/wallet/gtest/test_wallet_zkeys.cpp index 2847f1905..2297b3a78 100644 --- a/src/wallet/gtest/test_wallet_zkeys.cpp +++ b/src/wallet/gtest/test_wallet_zkeys.cpp @@ -20,6 +20,7 @@ TEST(wallet_zkeys_tests, StoreAndLoadSaplingZkeys) { SelectParams(CBaseChainParams::MAIN); CWallet wallet; + LOCK(wallet.cs_wallet); // wallet should be empty std::set addrs; @@ -114,6 +115,7 @@ TEST(wallet_zkeys_tests, store_and_load_zkeys) { SelectParams(CBaseChainParams::MAIN); CWallet wallet; + LOCK(wallet.cs_wallet); // wallet should be empty std::set addrs; @@ -171,6 +173,7 @@ TEST(wallet_zkeys_tests, StoreAndLoadViewingKeys) { SelectParams(CBaseChainParams::MAIN); CWallet wallet; + LOCK(wallet.cs_wallet); // wallet should be empty std::set addrs; @@ -223,6 +226,7 @@ TEST(wallet_zkeys_tests, write_zkey_direct_to_db) { bool fFirstRun; CWallet wallet("wallet.dat"); + LOCK(wallet.cs_wallet); ASSERT_EQ(DB_LOAD_OK, wallet.LoadWallet(fFirstRun)); // No default CPubKey set @@ -295,6 +299,7 @@ TEST(wallet_zkeys_tests, WriteViewingKeyDirectToDB) { bool fFirstRun; CWallet wallet("wallet-vkey.dat"); + LOCK(wallet.cs_wallet); ASSERT_EQ(DB_LOAD_OK, wallet.LoadWallet(fFirstRun)); // No default CPubKey set @@ -340,6 +345,7 @@ TEST(wallet_zkeys_tests, write_cryptedzkey_direct_to_db) { bool fFirstRun; CWallet wallet("wallet_crypted.dat"); + LOCK(wallet.cs_wallet); ASSERT_EQ(DB_LOAD_OK, wallet.LoadWallet(fFirstRun)); // No default CPubKey set @@ -414,6 +420,7 @@ TEST(wallet_zkeys_tests, WriteCryptedSaplingZkeyDirectToDb) { bool fFirstRun; CWallet wallet("wallet_crypted_sapling.dat"); + LOCK(wallet.cs_wallet); ASSERT_EQ(DB_LOAD_OK, wallet.LoadWallet(fFirstRun)); // No default CPubKey set diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp index 48705f90a..28a6c969b 100644 --- a/src/wallet/test/rpc_wallet_tests.cpp +++ b/src/wallet/test/rpc_wallet_tests.cpp @@ -59,7 +59,7 @@ BOOST_FIXTURE_TEST_SUITE(rpc_wallet_tests, WalletTestingSetup) BOOST_AUTO_TEST_CASE(rpc_addmultisig) { - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); rpcfn_type addmultisig = tableRPC["addmultisigaddress"]->actor; @@ -335,7 +335,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance) { SelectParams(CBaseChainParams::TESTNET); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); BOOST_CHECK_THROW(CallRPC("z_getbalance too many args"), runtime_error); @@ -935,7 +935,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_parameters) { SelectParams(CBaseChainParams::TESTNET); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); BOOST_CHECK_THROW(CallRPC("z_sendmany"), runtime_error); BOOST_CHECK_THROW(CallRPC("z_sendmany toofewargs"), runtime_error); @@ -1060,7 +1060,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_internals) SelectParams(CBaseChainParams::TESTNET); auto consensusParams = Params().GetConsensus(); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); UniValue retValue; @@ -1267,7 +1267,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_taddr_to_sapling) { RegtestActivateSapling(); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); if (!pwalletMain->HaveHDSeed()) { pwalletMain->GenerateNewSeed(); @@ -1487,7 +1487,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_listunspent_parameters) { SelectParams(CBaseChainParams::TESTNET); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); UniValue retValue; @@ -1536,7 +1536,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_parameters) { SelectParams(CBaseChainParams::TESTNET); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase"), runtime_error); BOOST_CHECK_THROW(CallRPC("z_shieldcoinbase toofewargs"), runtime_error); @@ -1616,7 +1616,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_shieldcoinbase_internals) SelectParams(CBaseChainParams::TESTNET); auto consensusParams = Params().GetConsensus(); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); // Mutable tx containing contextual information we need to build tx // We removed the ability to create pre-Sapling Sprout proofs, so we can @@ -1671,7 +1671,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_parameters) { SelectParams(CBaseChainParams::TESTNET); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); BOOST_CHECK_THROW(CallRPC("z_mergetoaddress"), runtime_error); BOOST_CHECK_THROW(CallRPC("z_mergetoaddress toofewargs"), runtime_error); @@ -1819,7 +1819,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_mergetoaddress_internals) SelectParams(CBaseChainParams::TESTNET); auto consensusParams = Params().GetConsensus(); - LOCK(pwalletMain->cs_wallet); + LOCK2(cs_main, pwalletMain->cs_wallet); UniValue retValue; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index cfe20b9e8..fc74f0677 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -595,7 +595,8 @@ void CWallet::RunSaplingMigration(int blockHeight) { if (!Params().GetConsensus().NetworkUpgradeActive(blockHeight, Consensus::UPGRADE_SAPLING)) { return; } - LOCK(cs_wallet); + // need cs_wallet to call CommitTransaction() + LOCK2(cs_main, cs_wallet); if (!fSaplingMigrationEnabled) { return; } @@ -954,6 +955,7 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const * spends it: */ bool CWallet::IsSproutSpent(const uint256& nullifier) const { + LOCK(cs_main); pair range; range = mapTxSproutNullifiers.equal_range(nullifier); @@ -968,6 +970,7 @@ bool CWallet::IsSproutSpent(const uint256& nullifier) const { } bool CWallet::IsSaplingSpent(const uint256& nullifier) const { + LOCK(cs_main); pair range; range = mapTxSaplingNullifiers.equal_range(nullifier);