Skip to content

Commit

Permalink
Merge bitcoin#12151: rpc: Remove cs_main lock from blockToJSON and bl…
Browse files Browse the repository at this point in the history 
…ockheaderToJSON

b9f226b rpc: Remove cs_main lock from blockToJSON and blockHeaderToJSON (João Barbosa)
343b98c rpc: Specify chain tip instead of chain in GetDifficulty (João Barbosa)
54dc13b rpc: Fix SoftForkMajorityDesc and SoftForkDesc signatures (João Barbosa)

Pull request description:

  Motivated by bitcoin#11913 (comment), this pull makes `blockToJSON` and `blockheaderToJSON` free of `cs_main` locks.

  Locking `cs_main` was required to access `chainActive` in order to check if the block was in the chain and to retrieve the next block index.

  With the this approach, `CBlockIndex::GetAncestor()` is used in a way to check if the block belongs to the specified chain tip and, at the same time, get the next block index.

Tree-SHA512: a6720ace0182c19033bbed1a404f729d793574db8ab16e0966ffe412145611e32c30aaab02975d225df6d439d7b9ef2070e732b16137a902b0293c8cddfeb85f
  • Loading branch information
@Munkybooty
MarcoFalke authored and Munkybooty committed Aug 24, 2021
1 parent baa76aa commit 9436caf
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 49 deletions.
17 changes: 7 additions & 10 deletions src/rest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,12 @@ static bool rest_headers(HTTPRequest* req,
if (!ParseHashStr(hashStr, hash))
return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr);

const CBlockIndex* tip = nullptr;
std::vector<const CBlockIndex *> headers;
headers.reserve(count);
{
LOCK(cs_main);
tip = chainActive.Tip();
const CBlockIndex* pindex = LookupBlockIndex(hash);
while (pindex != nullptr && chainActive.Contains(pindex)) {
headers.push_back(pindex);
Expand Down Expand Up @@ -170,11 +172,8 @@ static bool rest_headers(HTTPRequest* req,
}
case RetFormat::JSON: {
UniValue jsonHeaders(UniValue::VARR);
{
LOCK(cs_main);
for (const CBlockIndex *pindex : headers) {
jsonHeaders.push_back(blockheaderToJSON(pindex));
}
for (const CBlockIndex *pindex : headers) {
jsonHeaders.push_back(blockheaderToJSON(tip, pindex));
}
std::string strJSON = jsonHeaders.write() + "\n";
req->WriteHeader("Content-Type", "application/json");
Expand Down Expand Up @@ -202,8 +201,10 @@ static bool rest_block(HTTPRequest* req,

CBlock block;
CBlockIndex* pblockindex = nullptr;
CBlockIndex* tip = nullptr;
{
LOCK(cs_main);
tip = chainActive.Tip();
pblockindex = LookupBlockIndex(hash);
if (!pblockindex) {
return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found");
Expand Down Expand Up @@ -236,11 +237,7 @@ static bool rest_block(HTTPRequest* req,
}

case RetFormat::JSON: {
UniValue objBlock;
{
LOCK(cs_main);
objBlock = blockToJSON(block, pblockindex, showTxDetails);
}
UniValue objBlock = blockToJSON(block, tip, pblockindex, showTxDetails);
std::string strJSON = objBlock.write() + "\n";
req->WriteHeader("Content-Type", "application/json");
req->WriteReply(HTTP_OK, strJSON);
Expand Down
59 changes: 29 additions & 30 deletions src/rpc/blockchain.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,7 @@ extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue&
*/
double GetDifficulty(const CBlockIndex* blockindex)
{
if (blockindex == nullptr)
{
return 1.0;
}
assert(blockindex);

int nShift = (blockindex->nBits >> 24) & 0xff;
double dDiff =
Expand All @@ -94,15 +91,22 @@ double GetDifficulty(const CBlockIndex* blockindex)
return dDiff;
}

UniValue blockheaderToJSON(const CBlockIndex* blockindex)
static int ComputeNextBlockAndDepth(const CBlockIndex* tip, const CBlockIndex* blockindex, const CBlockIndex*& next)
{
next = tip->GetAncestor(blockindex->nHeight + 1);
if (next && next->pprev == blockindex) {
return tip->nHeight - blockindex->nHeight + 1;
}
next = nullptr;
return blockindex == tip ? 1 : -1;
}

UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex)
{
AssertLockHeld(cs_main);
UniValue result(UniValue::VOBJ);
result.pushKV("hash", blockindex->GetBlockHash().GetHex());
int confirmations = -1;
// Only report confirmations if the block is on the main chain
if (chainActive.Contains(blockindex))
confirmations = chainActive.Height() - blockindex->nHeight + 1;
const CBlockIndex* pnext;
int confirmations = ComputeNextBlockAndDepth(tip, blockindex, pnext);
result.pushKV("confirmations", confirmations);
result.pushKV("height", blockindex->nHeight);
result.pushKV("version", blockindex->nVersion);
Expand All @@ -118,7 +122,6 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex)

if (blockindex->pprev)
result.pushKV("previousblockhash", blockindex->pprev->GetBlockHash().GetHex());
CBlockIndex *pnext = chainActive.Next(blockindex);
if (pnext)
result.pushKV("nextblockhash", pnext->GetBlockHash().GetHex());

Expand All @@ -127,15 +130,12 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex)
return result;
}

UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails)
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails)
{
AssertLockHeld(cs_main);
UniValue result(UniValue::VOBJ);
result.pushKV("hash", blockindex->GetBlockHash().GetHex());
int confirmations = -1;
// Only report confirmations if the block is on the main chain
if (chainActive.Contains(blockindex))
confirmations = chainActive.Height() - blockindex->nHeight + 1;
const CBlockIndex* pnext;
int confirmations = ComputeNextBlockAndDepth(tip, blockindex, pnext);
result.pushKV("confirmations", confirmations);
result.pushKV("size", (int)::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION));
result.pushKV("height", blockindex->nHeight);
Expand Down Expand Up @@ -177,7 +177,6 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx

if (blockindex->pprev)
result.pushKV("previousblockhash", blockindex->pprev->GetBlockHash().GetHex());
CBlockIndex *pnext = chainActive.Next(blockindex);
if (pnext)
result.pushKV("nextblockhash", pnext->GetBlockHash().GetHex());

Expand Down Expand Up @@ -836,7 +835,7 @@ static UniValue getblockheader(const JSONRPCRequest& request)
return strHex;
}

return blockheaderToJSON(pblockindex);
return blockheaderToJSON(chainActive.Tip(), pblockindex);
}

static UniValue getblockheaders(const JSONRPCRequest& request)
Expand Down Expand Up @@ -920,7 +919,7 @@ static UniValue getblockheaders(const JSONRPCRequest& request)

for (; pblockindex; pblockindex = chainActive.Next(pblockindex))
{
arrHeaders.push_back(blockheaderToJSON(pblockindex));
arrHeaders.push_back(blockheaderToJSON(chainActive.Tip(), pblockindex));
if (--nCount <= 0)
break;
}
Expand Down Expand Up @@ -1107,7 +1106,7 @@ static UniValue getblock(const JSONRPCRequest& request)
return strHex;
}

return blockToJSON(block, pblockindex, verbosity >= 2);
return blockToJSON(block, chainActive.Tip(), pblockindex, verbosity >= 2);
}

static UniValue pruneblockchain(const JSONRPCRequest& request)
Expand Down Expand Up @@ -1309,7 +1308,7 @@ static UniValue verifychain(const JSONRPCRequest& request)
}

/** Implementation of IsSuperMajority with better feedback */
static UniValue SoftForkMajorityDesc(int version, CBlockIndex* pindex, const Consensus::Params& consensusParams)
static UniValue SoftForkMajorityDesc(int version, const CBlockIndex* pindex, const Consensus::Params& consensusParams)
{
UniValue rv(UniValue::VOBJ);
bool activated = false;
Expand All @@ -1329,7 +1328,7 @@ static UniValue SoftForkMajorityDesc(int version, CBlockIndex* pindex, const Con
return rv;
}

static UniValue SoftForkDesc(const std::string &name, int version, CBlockIndex* pindex, const Consensus::Params& consensusParams)
static UniValue SoftForkDesc(const std::string &name, int version, const CBlockIndex* pindex, const Consensus::Params& consensusParams)
{
UniValue rv(UniValue::VOBJ);
rv.pushKV("id", name);
Expand Down Expand Up @@ -1438,20 +1437,21 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)

std::string strChainName = gArgs.IsArgSet("-devnet") ? gArgs.GetDevNetName() : Params().NetworkIDString();

const CBlockIndex* tip = chainActive.Tip();
UniValue obj(UniValue::VOBJ);
obj.pushKV("chain", strChainName);
obj.pushKV("blocks", (int)chainActive.Height());
obj.pushKV("headers", pindexBestHeader ? pindexBestHeader->nHeight : -1);
obj.pushKV("bestblockhash", chainActive.Tip()->GetBlockHash().GetHex());
obj.pushKV("difficulty", (double)GetDifficulty(chainActive.Tip()));
obj.pushKV("mediantime", (int64_t)chainActive.Tip()->GetMedianTimePast());
obj.pushKV("verificationprogress", GuessVerificationProgress(Params().TxData(), chainActive.Tip()));
obj.pushKV("bestblockhash", tip->GetBlockHash().GetHex());
obj.pushKV("difficulty", (double)GetDifficulty(tip));
obj.pushKV("mediantime", (int64_t)tip->GetMedianTimePast());
obj.pushKV("verificationprogress", GuessVerificationProgress(Params().TxData(), tip));
obj.pushKV("initialblockdownload", IsInitialBlockDownload());
obj.pushKV("chainwork", chainActive.Tip()->nChainWork.GetHex());
obj.pushKV("chainwork", tip->nChainWork.GetHex());
obj.pushKV("size_on_disk", CalculateCurrentUsage());
obj.pushKV("pruned", fPruneMode);
if (fPruneMode) {
CBlockIndex* block = chainActive.Tip();
const CBlockIndex* block = tip;
assert(block);
while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
block = block->pprev;
Expand All @@ -1468,7 +1468,6 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
}

const Consensus::Params& consensusParams = Params().GetConsensus();
CBlockIndex* tip = chainActive.Tip();
UniValue softforks(UniValue::VARR);
UniValue bip9_softforks(UniValue::VOBJ);
// sorted by activation block
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/blockchain.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ double GetDifficulty(const CBlockIndex* blockindex);
void RPCNotifyBlockChange(bool ibd, const CBlockIndex *);

/** Block description to JSON */
UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails = false);
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails = false);

/** Mempool information to JSON */
UniValue mempoolInfoToJSON();
Expand All @@ -36,7 +36,7 @@ UniValue mempoolInfoToJSON();
UniValue mempoolToJSON(bool fVerbose = false);

/** Block header to JSON */
UniValue blockheaderToJSON(const CBlockIndex* blockindex);
UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex);

/** Used by getblockstats to get feerates at different percentiles by weight */
void CalculatePercentilesBySize(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_size);
Expand Down
7 changes: 0 additions & 7 deletions src/test/blockchain_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,4 @@ BOOST_AUTO_TEST_CASE(get_difficulty_for_very_high_target)
TestDifficulty(0x12345678, 5913134931067755359633408.0);
}

// Verify that difficulty is 1.0 for an empty chain.
BOOST_AUTO_TEST_CASE(get_difficulty_for_null_tip)
{
double difficulty = GetDifficulty(nullptr);
RejectDifficultyMismatch(difficulty, 1.0);
}

BOOST_AUTO_TEST_SUITE_END()

0 comments on commit 9436caf

Please sign in to comment.