diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -260,7 +260,6 @@ entry.dPriority = 111.0; entry.nHeight = 11; - LOCK(cs_main); fCheckpointsEnabled = false; GlobalConfig config; @@ -277,32 +276,35 @@ for (size_t i = 0; i < sizeof(blockinfo) / sizeof(*blockinfo); ++i) { // pointer for convenience. CBlock *pblock = &pblocktemplate->block; - 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()); - // Ignore the (optional) segwit commitment added by CreateNewBlock (as - // the hardcoded nonces don't account for this) - txCoinbase.vout.resize(1); - 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]); + { + 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); + 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; } - pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); - pblock->nNonce = blockinfo[i].nonce; std::shared_ptr shared_pblock = std::make_shared(*pblock); BOOST_CHECK(ProcessNewBlock(config, shared_pblock, true, nullptr)); pblock->hashPrevBlock = pblock->GetHash(); } + LOCK(cs_main); + // Just to make sure we can still make simple blocks. BOOST_CHECK( pblocktemplate = diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -64,7 +64,6 @@ // 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 @@ -245,12 +244,12 @@ const CTransaction spend_tx(mutableSpend_tx); - 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 NULLDUMMY { + LOCK(cs_main); + CValidationState state; PrecomputedTransactionData ptd_spend_tx(spend_tx); @@ -275,15 +274,17 @@ // successes. ValidateCheckInputsForAllFlags(spend_tx, SCRIPT_VERIFY_NULLDUMMY, false, false); + } - // And if we produce a block with this tx, it should be valid (LOW_S not - // enabled yet), even though there's no cache entry. - CBlock block; + // And if we produce a block with this tx, it should be valid (LOW_S 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()); - } + 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 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2850,6 +2850,7 @@ // 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; @@ -3988,6 +3989,8 @@ bool ProcessNewBlock(const Config &config, const std::shared_ptr pblock, bool fForceProcessing, bool *fNewBlock) { + AssertLockNotHeld(cs_main); + { if (fNewBlock) { *fNewBlock = false; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -461,8 +461,6 @@ } 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(); @@ -470,6 +468,8 @@ CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); CBlockIndex *newTip = chainActive.Tip(); + LOCK(cs_main); + // Verify ScanForWalletTransactions picks up transactions in both the old // and new block files. { @@ -547,8 +547,6 @@ // importwallet RPC would start the scan at the latest block with timestamp less // 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; @@ -571,6 +569,8 @@ GetScriptForRawPubKey(coinbaseKey.GetPubKey())) .vtx[0]); + LOCK(cs_main); + // Import key into wallet and call dumpwallet to create backup file. { CWallet wallet(Params()); @@ -733,11 +733,17 @@ changePos, error)); CValidationState state; BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state)); + CMutableTransaction blocktx; + { + LOCK(wallet->cs_wallet); + blocktx = + CMutableTransaction(*wallet->mapWallet.at(wtx.GetId()).tx); + } + CreateAndProcessBlock({CMutableTransaction(blocktx)}, + GetScriptForRawPubKey(coinbaseKey.GetPubKey())); LOCK(wallet->cs_wallet); auto it = wallet->mapWallet.find(wtx.GetId()); BOOST_CHECK(it != wallet->mapWallet.end()); - CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, - GetScriptForRawPubKey(coinbaseKey.GetPubKey())); it->second.SetMerkleBranch(chainActive.Tip(), 1); return it->second; } @@ -747,7 +753,6 @@ 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. @@ -778,6 +783,7 @@ 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->GetId(), coin.i)); } }