diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index b1a2032ea8..f968869bf5 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -205,7 +205,6 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) entry.nFee = 11; entry.nHeight = 11; - LOCK(cs_main); fCheckpointsEnabled = false; // Simple block creation, nothing special yet: @@ -218,27 +217,32 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) for (unsigned int i = 0; i < sizeof(blockinfo)/sizeof(*blockinfo); ++i) { CBlock *pblock = &pblocktemplate->block; // pointer for convenience - pblock->nVersion = 1; - pblock->nTime = chainActive.Tip()->GetMedianTimePast()+1; - CMutableTransaction txCoinbase(*pblock->vtx[0]); - txCoinbase.nVersion = 1; - txCoinbase.vin[0].scriptSig = CScript(); - txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce); - txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); - txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this) - txCoinbase.vout[0].scriptPubKey = CScript(); - pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); - if (txFirst.size() == 0) - baseheight = chainActive.Height(); - if (txFirst.size() < 4) - txFirst.push_back(pblock->vtx[0]); - pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); - pblock->nNonce = blockinfo[i].nonce; + { + LOCK(cs_main); + pblock->nVersion = 1; + pblock->nTime = chainActive.Tip()->GetMedianTimePast()+1; + CMutableTransaction txCoinbase(*pblock->vtx[0]); + txCoinbase.nVersion = 1; + txCoinbase.vin[0].scriptSig = CScript(); + txCoinbase.vin[0].scriptSig.push_back(blockinfo[i].extranonce); + txCoinbase.vin[0].scriptSig.push_back(chainActive.Height()); + txCoinbase.vout.resize(1); // Ignore the (optional) segwit commitment added by CreateNewBlock (as the hardcoded nonces don't account for this) + txCoinbase.vout[0].scriptPubKey = CScript(); + pblock->vtx[0] = MakeTransactionRef(std::move(txCoinbase)); + if (txFirst.size() == 0) + baseheight = chainActive.Height(); + if (txFirst.size() < 4) + txFirst.push_back(pblock->vtx[0]); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); + pblock->nNonce = blockinfo[i].nonce; + } std::shared_ptr shared_pblock = std::make_shared(*pblock); BOOST_CHECK(ProcessNewBlock(chainparams, shared_pblock, true, nullptr)); pblock->hashPrevBlock = pblock->GetHash(); } + LOCK(cs_main); + // Just to make sure we can still make simple blocks BOOST_CHECK(pblocktemplate = AssemblerForTest(chainparams).CreateNewBlock(scriptPubKey)); diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp index fe8cb6126e..10acabe0be 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -66,7 +66,6 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup) // Test 1: block with both of those transactions should be rejected. block = CreateAndProcessBlock(spends, scriptPubKey); - LOCK(cs_main); BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash()); // Test 2: ... and should be rejected if spend1 is in the memory pool @@ -190,12 +189,12 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) spend_tx.vin[0].scriptSig << vchSig; } - LOCK(cs_main); - // Test that invalidity under a set of flags doesn't preclude validity // under other (eg consensus) flags. // spend_tx is invalid according to DERSIG { + LOCK(cs_main); + CValidationState state; PrecomputedTransactionData ptd_spend_tx(spend_tx); @@ -213,16 +212,18 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup) // test later that block validation works fine in the absence of cached // successes. ValidateCheckInputsForAllFlags(spend_tx, SCRIPT_VERIFY_DERSIG | SCRIPT_VERIFY_LOW_S | SCRIPT_VERIFY_STRICTENC, false); - - // And if we produce a block with this tx, it should be valid (DERSIG not - // enabled yet), even though there's no cache entry. - CBlock block; - - block = CreateAndProcessBlock({spend_tx}, p2pk_scriptPubKey); - BOOST_CHECK(chainActive.Tip()->GetBlockHash() == block.GetHash()); - BOOST_CHECK(pcoinsTip->GetBestBlock() == block.GetHash()); } + // And if we produce a block with this tx, it should be valid (DERSIG not + // enabled yet), even though there's no cache entry. + CBlock block; + + block = CreateAndProcessBlock({spend_tx}, p2pk_scriptPubKey); + BOOST_CHECK(chainActive.Tip()->GetBlockHash() == block.GetHash()); + BOOST_CHECK(pcoinsTip->GetBestBlock() == block.GetHash()); + + LOCK(cs_main); + // Test P2SH: construct a transaction that is valid without P2SH, and // then test validity with P2SH. { diff --git a/src/validation.cpp b/src/validation.cpp index 75c40b22fc..71a51c499c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2559,6 +2559,7 @@ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& // far from a guarantee. Things in the P2P/RPC will often end up calling // us in the middle of ProcessNewBlock - do not assume pblock is set // sanely for performance or correctness! + AssertLockNotHeld(cs_main); CBlockIndex *pindexMostWork = nullptr; CBlockIndex *pindexNewTip = nullptr; @@ -3383,6 +3384,8 @@ bool CChainState::AcceptBlock(const std::shared_ptr& pblock, CVali bool ProcessNewBlock(const CChainParams& chainparams, const std::shared_ptr pblock, bool fForceProcessing, bool *fNewBlock) { + AssertLockNotHeld(cs_main); + { CBlockIndex *pindex = nullptr; if (fNewBlock) *fNewBlock = false; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 80e31a1ce0..8e9362c649 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -370,8 +370,6 @@ static void AddKey(CWallet& wallet, const CKey& key) BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) { - LOCK(cs_main); - // Cap last block file size, and mine new block in a new block file. CBlockIndex* const nullBlock = nullptr; CBlockIndex* oldTip = chainActive.Tip(); @@ -379,6 +377,8 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CBlockIndex* newTip = chainActive.Tip(); + LOCK(cs_main); + // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { @@ -447,8 +447,6 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup) // than or equal to key birthday. BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) { - LOCK(cs_main); - // Create two blocks with same timestamp to verify that importwallet rescan // will pick up both blocks, not just the first. const int64_t BLOCK_TIME = chainActive.Tip()->GetBlockTimeMax() + 5; @@ -462,6 +460,8 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup) SetMockTime(KEY_TIME); coinbaseTxns.emplace_back(*CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())).vtx[0]); + LOCK(cs_main); + // Import key into wallet and call dumpwallet to create backup file. { CWallet wallet; @@ -627,10 +627,15 @@ public: BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy)); CValidationState state; BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + CMutableTransaction blocktx; + { + LOCK(wallet->cs_wallet); + blocktx = CMutableTransaction(*wallet->mapWallet.at(wtx.GetHash()).tx); + } + CreateAndProcessBlock({CMutableTransaction(blocktx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(wtx.GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; } @@ -641,7 +646,6 @@ public: BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) { std::string coinbaseAddress = coinbaseKey.GetPubKey().GetID().ToString(); - LOCK2(cs_main, wallet->cs_wallet); // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey // address. @@ -669,6 +673,7 @@ BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) BOOST_CHECK_EQUAL(available.size(), 2); for (const auto& group : list) { for (const auto& coin : group.second) { + LOCK(wallet->cs_wallet); wallet->LockCoin(COutPoint(coin.tx->GetHash(), coin.i)); } }