Motivated by #11913 (review), 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.
77@@ -78,20 +78,22 @@ double GetDifficulty(const CChain& chain, const CBlockIndex* blockindex)
78 return dDiff;
79 }
80
81-double GetDifficulty(const CBlockIndex* blockindex)
82+static int ComputeBlockConfirmations(const CBlockIndex* tip, const CBlockIndex* blockindex, const CBlockIndex*& next)
83 {
84- return GetDifficulty(chainActive, blockindex);
85+ next = tip->GetAncestor(blockindex->nHeight + 1);
86+ if (next && next->pprev == blockindex) {
ComputeNextBlockAndDepth to mention the next part.
std::pair<int, const CBlockIndex*>?
height + 1 and ->pprev == blockindex.
ComputeNextBlockAndDepth as per @ryanofsky suggestion.
1181@@ -1185,20 +1182,21 @@ UniValue getblockchaininfo(const JSONRPCRequest& request)
1182
1183 LOCK(cs_main);
1184
1185+ const CBlockIndex* tip = chainActive.Tip();
102@@ -101,26 +103,22 @@ UniValue blockheaderToJSON(const CBlockIndex* blockindex)
103 result.push_back(Pair("mediantime", (int64_t)blockindex->GetMedianTimePast()));
104 result.push_back(Pair("nonce", (uint64_t)blockindex->nNonce));
105 result.push_back(Pair("bits", strprintf("%08x", blockindex->nBits)));
106- result.push_back(Pair("difficulty", GetDifficulty(blockindex)));
107+ result.push_back(Pair("difficulty", GetDifficulty(chainActive.Tip(), blockindex)));
tip instead of chainActive.Tip()?
143@@ -146,12 +144,11 @@ UniValue blockToJSON(const CBlock& block, const CBlockIndex* blockindex, bool tx
144 result.push_back(Pair("mediantime", (int64_t)blockindex->GetMedianTimePast()));
145 result.push_back(Pair("nonce", (uint64_t)block.nNonce));
146 result.push_back(Pair("bits", strprintf("%08x", block.nBits)));
147- result.push_back(Pair("difficulty", GetDifficulty(blockindex)));
148+ result.push_back(Pair("difficulty", GetDifficulty(chainActive.Tip(), blockindex)));
tip instead of chainActive.Tip()?
82+static int ComputeBlockConfirmations(const CBlockIndex* tip, const CBlockIndex* blockindex, const CBlockIndex*& next)
83 {
84- return GetDifficulty(chainActive, blockindex);
85+ next = tip->GetAncestor(blockindex->nHeight + 1);
86+ if (next && next->pprev == blockindex) {
87+ return tip->nHeight - blockindex->nHeight;
+1 I believe?
85+ next = tip->GetAncestor(blockindex->nHeight + 1);
86+ if (next && next->pprev == blockindex) {
87+ return tip->nHeight - blockindex->nHeight;
88+ }
89+ next = nullptr;
90+ return blockindex == tip ? 0 : -1;
? 1 : -1. Blocks that are the chain tip have 1 confirmation.
242@@ -242,11 +243,7 @@ static bool rest_block(HTTPRequest* req,
243 }
244
245 case RF_JSON: {
246- UniValue objBlock;
247- {
248- LOCK(cs_main);
249- objBlock = blockToJSON(block, pblockindex, showTxDetails);
250- }
251+ UniValue objBlock = std::move(blockToJSON(block, pblockindex, tip, showTxDetails));
tip, pblockindex.
781@@ -785,7 +782,7 @@ UniValue getblock(const JSONRPCRequest& request)
782 return strHex;
783 }
784
785- return blockToJSON(block, pblockindex, verbosity >= 2);
786+ return blockToJSON(block, pblockindex, chainActive.Tip(), verbosity >= 2);
242@@ -242,11 +243,7 @@ static bool rest_block(HTTPRequest* req,
243 }
244
245 case RF_JSON: {
246- UniValue objBlock;
247- {
248- LOCK(cs_main);
249- objBlock = blockToJSON(block, pblockindex, showTxDetails);
250- }
251+ UniValue objBlock = std::move(blockToJSON(block, tip, pblockindex, showTxDetails));
Rebased. @MarcoFalke like @TheBlueMatt said above:
in the context of #11913 it makes sense to not lock cs_main and then unlock, then re-lock it there.
I’d say at least let’s wait for that.
242@@ -242,11 +243,7 @@ static bool rest_block(HTTPRequest* req,
243 }
244
245 case RetFormat::JSON: {
246- UniValue objBlock;
247- {
248- LOCK(cs_main);
249- objBlock = blockToJSON(block, pblockindex, showTxDetails);
250- }
251+ UniValue objBlock = blockToJSON(block, tip, pblockindex, showTxDetails);
50@@ -51,14 +51,14 @@ extern void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue&
51 /* Calculate the difficulty for a given block index,
52 * or the block index of the given chain.
53 */
54-double GetDifficulty(const CChain& chain, const CBlockIndex* blockindex)
55+double GetDifficulty(const CBlockIndex* tip, const CBlockIndex* blockindex)
commit: rpc: Specify chain tip instead of chain in GetDifficulty
I don’t think this is really necessary. Would rather just inline GetDifficulty(blockindex ? blockindex : chainActive.Tip()) at the few call sites if blockindex may be null.
I don’t actually see any instances in this file where blockindex might be nullptr.
Below there is the test
0// Verify that difficulty is 1.0 for a null chain tip.
1BOOST_AUTO_TEST_CASE(get_difficulty_for_null_tip)
2{
3 double difficulty = GetDifficulty(nullptr, nullptr);
4 RejectDifficultyMismatch(difficulty, 1.0);
5}
Do you agree changing to
0// Verify that difficulty is 1.0 for a null block index.
1BOOST_AUTO_TEST_CASE(get_difficulty_for_null_block)
2{
3 double difficulty = GetDifficulty(nullptr);
4 RejectDifficultyMismatch(difficulty, 1.0);
5}
or should change the return value?
If you don’t mind I would rather keep the existing behaviour and discuss this elsewhere.
chainActive. Given that you are updating every invocation, this seems like the right time to make the change I am proposing.
tip to GetDifficulty in most places.
9@@ -10,8 +10,7 @@ class CBlockIndex;
10 class UniValue;
11
12 /**
13- * Get the difficulty of the net wrt to the given block index, or the chain tip if
14- * not provided.
15+ * Get the difficulty of the net wrt to the given block index.
51@@ -52,10 +52,7 @@ static CUpdatedBlock latestblock;
52 */
53 double GetDifficulty(const CBlockIndex* blockindex)
54 {
55- if (blockindex == nullptr)
56- {
57- return 1.0;
58- }
59+ assert(blockindex);
chainActive.Tip() is always non null.
57@@ -58,10 +58,7 @@ static CUpdatedBlock latestblock;
58 */
59 double GetDifficulty(const CBlockIndex* blockindex)
60 {
61- if (blockindex == nullptr)
62- {
63- return 1.0;
64- }
65+ assert(blockindex);
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.