Skip to content

Commit 521d1bc

Browse files
MarcoFalkeMunkybooty
MarcoFalke
authored and
Munkybooty
committed
Merge bitcoin#12151: rpc: Remove cs_main lock from blockToJSON and blockheaderToJSON
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
1 parent b4992b8 commit 521d1bc

File tree

4 files changed

+37
-48
lines changed

4 files changed

+37
-48
lines changed

‎src/rest.cpp‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,12 @@ static bool rest_headers(HTTPRequest* req,
131131
if (!ParseHashStr(hashStr, hash))
132132
return RESTERR(req, HTTP_BAD_REQUEST, "Invalid hash: " + hashStr);
133133

134+
const CBlockIndex* tip = nullptr;
134135
std::vector<const CBlockIndex *> headers;
135136
headers.reserve(count);
136137
{
137138
LOCK(cs_main);
139+
tip = chainActive.Tip();
138140
const CBlockIndex* pindex = LookupBlockIndex(hash);
139141
while (pindex != nullptr && chainActive.Contains(pindex)) {
140142
headers.push_back(pindex);
@@ -170,11 +172,8 @@ static bool rest_headers(HTTPRequest* req,
170172
}
171173
case RetFormat::JSON: {
172174
UniValue jsonHeaders(UniValue::VARR);
173-
{
174-
LOCK(cs_main);
175-
for (const CBlockIndex *pindex : headers) {
176-
jsonHeaders.push_back(blockheaderToJSON(pindex));
177-
}
175+
for (const CBlockIndex *pindex : headers) {
176+
jsonHeaders.push_back(blockheaderToJSON(tip, pindex));
178177
}
179178
std::string strJSON = jsonHeaders.write() + "\n";
180179
req->WriteHeader("Content-Type", "application/json");
@@ -202,8 +201,10 @@ static bool rest_block(HTTPRequest* req,
202201

203202
CBlock block;
204203
CBlockIndex* pblockindex = nullptr;
204+
CBlockIndex* tip = nullptr;
205205
{
206206
LOCK(cs_main);
207+
tip = chainActive.Tip();
207208
pblockindex = LookupBlockIndex(hash);
208209
if (!pblockindex) {
209210
return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found");
@@ -236,11 +237,7 @@ static bool rest_block(HTTPRequest* req,
236237
}
237238

238239
case RetFormat::JSON: {
239-
UniValue objBlock;
240-
{
241-
LOCK(cs_main);
242-
objBlock = blockToJSON(block, pblockindex, showTxDetails);
243-
}
240+
UniValue objBlock = blockToJSON(block, tip, pblockindex, showTxDetails);
244241
std::string strJSON = objBlock.write() + "\n";
245242
req->WriteHeader("Content-Type", "application/json");
246243
req->WriteReply(HTTP_OK, strJSON);

‎src/rpc/blockchain.cpp‎

Lines changed: 28 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,7 @@ extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue&
7171
*/
7272
double GetDifficulty(const CBlockIndex* blockindex)
7373
{
74-
if (blockindex == nullptr)
75-
{
76-
return 1.0;
77-
}
74+
assert(blockindex);
7875

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

97-
UniValue blockheaderToJSON(const CBlockIndex* blockindex)
94+
static int ComputeNextBlockAndDepth(const CBlockIndex* tip, const CBlockIndex* blockindex, const CBlockIndex*& next)
95+
{
96+
next = tip->GetAncestor(blockindex->nHeight + 1);
97+
if (next && next->pprev == blockindex) {
98+
return tip->nHeight - blockindex->nHeight + 1;
99+
}
100+
next = nullptr;
101+
return blockindex == tip ? 1 : -1;
102+
}
103+
104+
UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex)
98105
{
99-
AssertLockHeld(cs_main);
100106
UniValue result(UniValue::VOBJ);
101107
result.pushKV("hash", blockindex->GetBlockHash().GetHex());
102-
int confirmations = -1;
103-
// Only report confirmations if the block is on the main chain
104-
if (chainActive.Contains(blockindex))
105-
confirmations = chainActive.Height() - blockindex->nHeight + 1;
108+
const CBlockIndex* pnext;
109+
int confirmations = ComputeNextBlockAndDepth(tip, blockindex, pnext);
106110
result.pushKV("confirmations", confirmations);
107111
result.pushKV("height", blockindex->nHeight);
108112
result.pushKV("version", blockindex->nVersion);
@@ -118,7 +122,6 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex)
118122

119123
if (blockindex->pprev)
120124
result.pushKV("previousblockhash", blockindex->pprev->GetBlockHash().GetHex());
121-
CBlockIndex *pnext = chainActive.Next(blockindex);
122125
if (pnext)
123126
result.pushKV("nextblockhash", pnext->GetBlockHash().GetHex());
124127

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

130-
UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool txDetails)
133+
UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails)
131134
{
132-
AssertLockHeld(cs_main);
133135
UniValue result(UniValue::VOBJ);
134136
result.pushKV("hash", blockindex->GetBlockHash().GetHex());
135-
int confirmations = -1;
136-
// Only report confirmations if the block is on the main chain
137-
if (chainActive.Contains(blockindex))
138-
confirmations = chainActive.Height() - blockindex->nHeight + 1;
137+
const CBlockIndex* pnext;
138+
int confirmations = ComputeNextBlockAndDepth(tip, blockindex, pnext);
139139
result.pushKV("confirmations", confirmations);
140140
result.pushKV("size", (int)::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION));
141141
result.pushKV("height", blockindex->nHeight);
@@ -177,7 +177,6 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx
177177

178178
if (blockindex->pprev)
179179
result.pushKV("previousblockhash", blockindex->pprev->GetBlockHash().GetHex());
180-
CBlockIndex *pnext = chainActive.Next(blockindex);
181180
if (pnext)
182181
result.pushKV("nextblockhash", pnext->GetBlockHash().GetHex());
183182

@@ -836,7 +835,7 @@ static UniValue getblockheader(const JSONRPCRequest& request)
836835
return strHex;
837836
}
838837

839-
return blockheaderToJSON(pblockindex);
838+
return blockheaderToJSON(chainActive.Tip(), pblockindex);
840839
}
841840

842841
static UniValue getblockheaders(const JSONRPCRequest& request)
@@ -1107,7 +1106,7 @@ static UniValue getblock(const JSONRPCRequest& request)
11071106
return strHex;
11081107
}
11091108

1110-
return blockToJSON(block, pblockindex, verbosity >= 2);
1109+
return blockToJSON(block, chainActive.Tip(), pblockindex, verbosity >= 2);
11111110
}
11121111

11131112
static UniValue pruneblockchain(const JSONRPCRequest& request)
@@ -1309,7 +1308,7 @@ static UniValue verifychain(const JSONRPCRequest& request)
13091308
}
13101309

13111310
/** Implementation of IsSuperMajority with better feedback */
1312-
static UniValue SoftForkMajorityDesc(int version, CBlockIndex* pindex, const Consensus::Params& consensusParams)
1311+
static UniValue SoftForkMajorityDesc(int version, const CBlockIndex* pindex, const Consensus::Params& consensusParams)
13131312
{
13141313
UniValue rv(UniValue::VOBJ);
13151314
bool activated = false;
@@ -1329,7 +1328,7 @@ static UniValue SoftForkMajorityDesc(int version, CBlockIndex* pindex, const Con
13291328
return rv;
13301329
}
13311330

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

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

1440+
const CBlockIndex* tip = chainActive.Tip();
14411441
UniValue obj(UniValue::VOBJ);
14421442
obj.pushKV("chain", strChainName);
14431443
obj.pushKV("blocks", (int)chainActive.Height());
14441444
obj.pushKV("headers", pindexBestHeader ? pindexBestHeader->nHeight : -1);
1445-
obj.pushKV("bestblockhash", chainActive.Tip()->GetBlockHash().GetHex());
1446-
obj.pushKV("difficulty", (double)GetDifficulty(chainActive.Tip()));
1447-
obj.pushKV("mediantime", (int64_t)chainActive.Tip()->GetMedianTimePast());
1448-
obj.pushKV("verificationprogress", GuessVerificationProgress(Params().TxData(), chainActive.Tip()));
1445+
obj.pushKV("bestblockhash", tip->GetBlockHash().GetHex());
1446+
obj.pushKV("difficulty", (double)GetDifficulty(tip));
1447+
obj.pushKV("mediantime", (int64_t)tip->GetMedianTimePast());
1448+
obj.pushKV("verificationprogress", GuessVerificationProgress(Params().TxData(), tip));
14491449
obj.pushKV("initialblockdownload", IsInitialBlockDownload());
1450-
obj.pushKV("chainwork", chainActive.Tip()->nChainWork.GetHex());
1450+
obj.pushKV("chainwork", tip->nChainWork.GetHex());
14511451
obj.pushKV("size_on_disk", CalculateCurrentUsage());
14521452
obj.pushKV("pruned", fPruneMode);
14531453
if (fPruneMode) {
1454-
CBlockIndex* block = chainActive.Tip();
1454+
const CBlockIndex* block = tip;
14551455
assert(block);
14561456
while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA)) {
14571457
block = block->pprev;
@@ -1468,7 +1468,6 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
14681468
}
14691469

14701470
const Consensus::Params& consensusParams = Params().GetConsensus();
1471-
CBlockIndex* tip = chainActive.Tip();
14721471
UniValue softforks(UniValue::VARR);
14731472
UniValue bip9_softforks(UniValue::VOBJ);
14741473
// sorted by activation block

‎src/rpc/blockchain.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ double GetDifficulty(const CBlockIndex* blockindex);
2727
void RPCNotifyBlockChange(bool ibd, const CBlockIndex *);
2828

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

3232
/** Mempool information to JSON */
3333
UniValue mempoolInfoToJSON();
@@ -36,7 +36,7 @@ UniValue mempoolInfoToJSON();
3636
UniValue mempoolToJSON(bool fVerbose = false);
3737

3838
/** Block header to JSON */
39-
UniValue blockheaderToJSON(const CBlockIndex* blockindex);
39+
UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex);
4040

4141
/** Used by getblockstats to get feerates at different percentiles by weight */
4242
void CalculatePercentilesBySize(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector<std::pair<CAmount, int64_t>>& scores, int64_t total_size);

‎src/test/blockchain_tests.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,4 @@ BOOST_AUTO_TEST_CASE(get_difficulty_for_very_high_target)
6868
TestDifficulty(0x12345678, 5913134931067755359633408.0);
6969
}
7070

71-
// Verify that difficulty is 1.0 for an empty chain.
72-
BOOST_AUTO_TEST_CASE(get_difficulty_for_null_tip)
73-
{
74-
double difficulty = GetDifficulty(nullptr);
75-
RejectDifficultyMismatch(difficulty, 1.0);
76-
}
77-
7871
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)