threading: reduce the scope of lock in getblocktemplate
What the developer wrote
Authored by kevkevinpal
threading: reduce the scope of lock in getblocktemplate
What changed, and why it matters
This change narrows the use of a global lock in the getblocktemplate RPC, which miners use to request work. The lock is now acquired later and held for a shorter time, rather than being held across the entire request setup. This is a performance and correctness cleanup. There is no direct evidence in the commit that it fixes an exploitable security bug, but reducing lock scope can prevent subtle race conditions or RPC stalls.
Treat as a routine correctness/performance improvement. Review whether the new lock placement preserves all required atomicity between tip reading, mempool access, and long-polling in getblocktemplate. No urgent security action is indicated by the diff alone.
Security signals we found
Reduced scope of cs_main global lock in RPC handler
Lock moved away from early, broad acquisition pattern
Long-polling wait explicitly releases the lock
No explicit security framing in commit message or diff
Evidence from the diff
The commit removes the early acquisition of cs_main in getblocktemplate and instead acquires it only when needed: briefly for a block lookup in the submit/block proposal path, and then later around the long-polling and tip-reading logic. The renamed lock variable (csmain_lock -> cs_main_lock) is held for a shorter duration and explicitly released during the long-poll wait. This reduces contention on cs_main and avoids holding the main-chain lock while parsing request parameters or decoding submitted blocks.
Changed components
src/rpc/mining.cppgetblocktemplate RPCcs_main lock usagelong-polling logicInspect captured patch +5 / −3
diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index b710c605..750ca72b 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -704,8 +704,6 @@ static RPCHelpMan getblocktemplate()
NodeContext& node = EnsureAnyNodeContext(request.context);
ChainstateManager& chainman = EnsureChainman(node);
Mining& miner = EnsureMining(node);
- WAIT_LOCK(cs_main, csmain_lock);
- uint256 tip{CHECK_NONFATAL(miner.getTip()).value().hash};
std::string strMode = "template";
UniValue lpval = NullUniValue;
@@ -735,6 +733,7 @@ static RPCHelpMan getblocktemplate()
throw JSONRPCError(RPC_DESERIALIZATION_ERROR, "Block decode failed");
uint256 hash = block.GetHash();
+ LOCK(cs_main);
const CBlockIndex* pindex = chainman.m_blockman.LookupBlockIndex(hash);
if (pindex) {
if (pindex->IsValid(BLOCK_VALID_SCRIPTS))
@@ -773,6 +772,9 @@ static RPCHelpMan getblocktemplate()
static unsigned int nTransactionsUpdatedLast;
const CTxMemPool& mempool = EnsureMemPool(node);
+ WAIT_LOCK(cs_main, cs_main_lock);
+ uint256 tip{CHECK_NONFATAL(miner.getTip()).value().hash};
+
// Long Polling (BIP22)
if (!lpval.isNull()) {
/**
@@ -811,7 +813,7 @@ static RPCHelpMan getblocktemplate()
// Release lock while waiting
{
- REVERSE_LOCK(csmain_lock, cs_main);
+ REVERSE_LOCK(cs_main_lock, cs_main);
MillisecondsDouble checktxtime{std::chrono::minutes(1)};
while (IsRPCRunning()) {
// If hashWatchedChain is not a real block hash, this will
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.