Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: fix missing cs_main lock in AppInitMain and LoadExternalBlockFile #2728

Merged
merged 3 commits into from
Jan 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bench/checkblock.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static void DeserializeAndCheckBlockTest(benchmark::State& state)
assert(rewound);

CValidationState state;
assert(CheckBlock(block, state));
assert(WITH_LOCK(cs_main, return CheckBlock(block, state); ));
}
}

Expand Down
7 changes: 5 additions & 2 deletions src/evo/specialtx_validation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "primitives/transaction.h"
#include "primitives/block.h"
#include "script/standard.h"
#include "validation.h" // needed by CheckLLMQCommitment (!TODO: remove)

/* -- Helper static functions -- */

Expand Down Expand Up @@ -473,7 +472,7 @@ bool VerifyLLMQCommitment(const llmq::CFinalCommitment& qfc, const CBlockIndex*
return true;
}

static bool CheckLLMQCommitmentTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state)
static bool CheckLLMQCommitmentTx(const CTransaction& tx, const CBlockIndex* pindexPrev, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

Expand Down Expand Up @@ -541,6 +540,8 @@ static bool CheckSpecialTxBasic(const CTransaction& tx, CValidationState& state)
// - pindexPrev=pindex->pprev: ConnectBlock-->ProcessSpecialTxsInBlock-->CheckSpecialTx
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache* view, CValidationState& state)
{
AssertLockHeld(cs_main);

if (!CheckSpecialTxBasic(tx, state)) {
// pass the state returned by the function above
return false;
Expand Down Expand Up @@ -592,6 +593,8 @@ bool CheckSpecialTxNoContext(const CTransaction& tx, CValidationState& state)

bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, const CCoinsViewCache* view, CValidationState& state, bool fJustCheck)
{
AssertLockHeld(cs_main);

// check special txes
for (const CTransactionRef& tx: block.vtx) {
if (!CheckSpecialTx(*tx, pindex->pprev, view, state)) {
Expand Down
9 changes: 5 additions & 4 deletions src/evo/specialtx_validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#define PIVX_SPECIALTX_H

#include "llmq/quorums_commitment.h"
#include "validation.h" // cs_main needed by CheckLLMQCommitment (!TODO: remove)
#include "version.h"

class CBlock;
Expand All @@ -22,18 +23,18 @@ static const unsigned int MAX_SPECIALTX_EXTRAPAYLOAD = 10000;
/** Payload validity checks (including duplicate unique properties against list at pindexPrev)*/
// Note: for +v2, if the tx is not a special tx, this method returns true.
// Note2: This function only performs extra payload related checks, it does NOT checks regular inputs and outputs.
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache* view, CValidationState& state);
bool CheckSpecialTx(const CTransaction& tx, const CBlockIndex* pindexPrev, const CCoinsViewCache* view, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

// Basic non-contextual checks for special txes
// Note: for +v2, if the tx is not a special tx, this method returns true.
bool CheckSpecialTxNoContext(const CTransaction& tx, CValidationState& state);
bool CheckSpecialTxNoContext(const CTransaction& tx, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

// Update internal tiertwo data when blocks containing special txes get connected/disconnected
bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, const CCoinsViewCache* view, CValidationState& state, bool fJustCheck);
bool ProcessSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex, const CCoinsViewCache* view, CValidationState& state, bool fJustCheck) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
bool UndoSpecialTxsInBlock(const CBlock& block, const CBlockIndex* pindex);

// Validate given LLMQ final commitment with the list at pindexQuorum
bool VerifyLLMQCommitment(const llmq::CFinalCommitment& qfc, const CBlockIndex* pindexPrev, CValidationState& state);
bool VerifyLLMQCommitment(const llmq::CFinalCommitment& qfc, const CBlockIndex* pindexPrev, CValidationState& state) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

uint256 CalcTxInputsHash(const CTransaction& tx);

Expand Down
20 changes: 9 additions & 11 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1489,6 +1489,8 @@ bool AppInitMain()
bool fReset = fReindex;
std::string strLoadError;

LOCK(cs_main);

do {
const int64_t load_block_index_start_time = GetTimeMillis();

Expand Down Expand Up @@ -1593,7 +1595,6 @@ bool AppInitMain()

if (Params().NetworkIDString() == CBaseChainParams::MAIN) {
// Prune zerocoin invalid outs if they were improperly stored in the coins database
LOCK(cs_main);
int chainHeight = chainActive.Height();
bool fZerocoinActive = chainHeight > 0 && consensus.NetworkUpgradeActive(chainHeight, Consensus::UPGRADE_ZC);

Expand All @@ -1617,16 +1618,13 @@ bool AppInitMain()

if (!is_coinsview_empty) {
uiInterface.InitMessage(_("Verifying blocks..."));
{
LOCK(cs_main);
CBlockIndex *tip = chainActive.Tip();
RPCNotifyBlockChange(true, tip);
if (tip && tip->nTime > GetAdjustedTime() + 2 * 60 * 60) {
strLoadError = _("The block database contains a block which appears to be from the future. "
"This may be due to your computer's date and time being set incorrectly. "
"Only rebuild the block database if you are sure that your computer's date and time are correct");
break;
}
CBlockIndex *tip = chainActive.Tip();
RPCNotifyBlockChange(true, tip);
if (tip && tip->nTime > GetAdjustedTime() + 2 * 60 * 60) {
strLoadError = _("The block database contains a block which appears to be from the future. "
"This may be due to your computer's date and time being set incorrectly. "
"Only rebuild the block database if you are sure that your computer's date and time are correct");
break;
}

if (!CVerifyDB().VerifyDB(pcoinsdbview.get(), gArgs.GetArg("-checklevel", DEFAULT_CHECKLEVEL),
Expand Down
12 changes: 9 additions & 3 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,10 @@ void MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlockIndex*
}

/** Check whether the last unknown block a peer advertised is not yet known. */
void ProcessBlockAvailability(NodeId nodeid)
static void ProcessBlockAvailability(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

CNodeState* state = State(nodeid);
assert(state != nullptr);

Expand All @@ -334,8 +336,10 @@ void ProcessBlockAvailability(NodeId nodeid)
}

/** Update tracking information about which blocks a peer is assumed to have. */
void UpdateBlockAvailability(NodeId nodeid, const uint256& hash)
static void UpdateBlockAvailability(NodeId nodeid, const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

CNodeState* state = State(nodeid);
assert(state != nullptr);

Expand All @@ -354,8 +358,10 @@ void UpdateBlockAvailability(NodeId nodeid, const uint256& hash)

/** Update pindexLastCommonBlock and add not-in-flight missing successors to vBlocks, until it has
* at most count entries. */
void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller)
static void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector<const CBlockIndex*>& vBlocks, NodeId& nodeStaller) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

if (count == 0)
return;

Expand Down
4 changes: 2 additions & 2 deletions src/rest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ static bool rest_block(HTTPRequest* req,
}

case RF_JSON: {
UniValue objBlock = blockToJSON(block, pblockindex, showTxDetails);
UniValue objBlock = WITH_LOCK(cs_main, return blockToJSON(block, pblockindex, showTxDetails); );
std::string strJSON = objBlock.write() + "\n";
req->WriteHeader("Content-Type", "application/json");
req->WriteReply(HTTP_OK, strJSON);
Expand Down Expand Up @@ -353,7 +353,7 @@ static bool rest_tx(HTTPRequest* req, const std::string& strURIPart)

case RF_JSON: {
UniValue objTx(UniValue::VOBJ);
TxToJSON(nullptr, *tx, hashBlock, objTx);
WITH_LOCK(cs_main, TxToJSON(nullptr, *tx, hashBlock, objTx); );
std::string strJSON = objTx.write() + "\n";
req->WriteHeader("Content-Type", "application/json");
req->WriteReply(HTTP_OK, strJSON);
Expand Down
4 changes: 3 additions & 1 deletion src/rpc/blockchain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex)
return result;
}

UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails = false)
UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

UniValue result(UniValue::VOBJ);
result.pushKV("hash", block.GetHash().GetHex());
int confirmations = -1;
Expand Down
4 changes: 3 additions & 1 deletion src/rpc/rawtransaction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ static void PayloadToJSON(const CTransaction& tx, UniValue& entry)
}

// pwallet can be nullptr. If not null, the json could include information available only to the wallet.
void TxToJSON(CWallet* const pwallet, const CTransaction& tx, const uint256 hashBlock, UniValue& entry)
void TxToJSON(CWallet* const pwallet, const CTransaction& tx, const uint256 hashBlock, UniValue& entry) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
{
AssertLockHeld(cs_main);

random-zebra marked this conversation as resolved.
Show resolved Hide resolved
// Call into TxToUniv() in bitcoin-common to decode the transaction hex.
//
// Blockchain contextual information (confirmations and blocktime) is not
Expand Down
2 changes: 1 addition & 1 deletion src/rpc/rpcevo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ static std::string SignAndSendSpecialTx(CWallet* const pwallet, CMutableTransact

CValidationState state;
CCoinsViewCache view(pcoinsTip.get());
if (!CheckSpecialTx(tx, GetChainTip(), &view, state)) {
if (!WITH_LOCK(cs_main, return CheckSpecialTx(tx, GetChainTip(), &view, state); )) {
throw JSONRPCError(RPC_MISC_ERROR, FormatStateMessage(state));
}

Expand Down
1 change: 1 addition & 0 deletions src/test/checkblock_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ BOOST_AUTO_TEST_CASE(May15)
CBlock forkingBlock;
if (read_block("Mar12Fork.dat", forkingBlock))
{
LOCK(cs_main);
CValidationState state;

// After May 15'th, big blocks are OK:
Expand Down
Loading