From fa1c3591add184ecb43eb3b149f4833f241e3858 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 1 May 2019 11:21:30 -0400 Subject: [PATCH 1/3] rpc: Use IsValidNumArgs in getblock --- src/rpc/blockchain.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 672fc6967..cd56e48ab 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -824,9 +824,7 @@ static CBlock GetBlockChecked(const CBlockIndex* pblockindex) static UniValue getblock(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() < 1 || request.params.size() > 2) - throw std::runtime_error( - RPCHelpMan{"getblock", + const RPCHelpMan help{"getblock", "\nIf verbosity is 0, returns a string that is serialized, hex-encoded data for block 'hash'.\n" "If verbosity is 1, returns an Object with information about block .\n" "If verbosity is 2, returns an Object with information about block and information about each transaction. \n", @@ -878,7 +876,11 @@ static UniValue getblock(const JSONRPCRequest& request) HelpExampleCli("getblock", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\"") + HelpExampleRpc("getblock", "\"00000000c937983704a73af28acdec37b049d214adbda81d7e2a3dd146f6ed09\"") }, - }.ToString()); + }; + + if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { + throw std::runtime_error(help.ToString()); + } LOCK(cs_main); From fab00a5cb90053e9671627af6a35e57610a44e89 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 1 May 2019 12:24:55 -0400 Subject: [PATCH 2/3] rpc: Serialize in getblock without cs_main --- src/rpc/blockchain.cpp | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index cd56e48ab..cd636a712 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -93,6 +93,7 @@ static int ComputeNextBlockAndDepth(const CBlockIndex* tip, const CBlockIndex* b UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex) { + // Serialize passed information without accessing chain state of the active chain! UniValue result(UniValue::VOBJ); result.pushKV("hash", blockindex->GetBlockHash().GetHex()); const CBlockIndex* pnext; @@ -119,6 +120,7 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails) { + // Serialize passed information without accessing chain state of the active chain! UniValue result(UniValue::VOBJ); result.pushKV("hash", blockindex->GetBlockHash().GetHex()); const CBlockIndex* pnext; @@ -882,8 +884,6 @@ static UniValue getblock(const JSONRPCRequest& request) throw std::runtime_error(help.ToString()); } - LOCK(cs_main); - uint256 hash(ParseHashV(request.params[0], "blockhash")); int verbosity = 1; @@ -894,12 +894,20 @@ static UniValue getblock(const JSONRPCRequest& request) verbosity = request.params[1].get_bool() ? 1 : 0; } - const CBlockIndex* pblockindex = LookupBlockIndex(hash); - if (!pblockindex) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); - } + CBlock block; + const CBlockIndex* pblockindex; + const CBlockIndex* tip; + { + LOCK(cs_main); + pblockindex = LookupBlockIndex(hash); + tip = chainActive.Tip(); - const CBlock block = GetBlockChecked(pblockindex); + if (!pblockindex) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); + } + + block = GetBlockChecked(pblockindex); + } if (verbosity <= 0) { @@ -909,7 +917,7 @@ static UniValue getblock(const JSONRPCRequest& request) return strHex; } - return blockToJSON(block, chainActive.Tip(), pblockindex, verbosity >= 2); + return blockToJSON(block, tip, pblockindex, verbosity >= 2); } struct CCoinsStats From faea56400d5578023133cf4d1c761cdeb0c3e3da Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 2 May 2019 14:34:09 -0400 Subject: [PATCH 3/3] rpc: Add lock annotations to block{,header}ToJSON --- src/rpc/blockchain.cpp | 4 ++++ src/rpc/blockchain.h | 12 ++++++++---- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index cd636a712..9d7d213f6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -94,6 +94,8 @@ static int ComputeNextBlockAndDepth(const CBlockIndex* tip, const CBlockIndex* b UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex) { // Serialize passed information without accessing chain state of the active chain! + AssertLockNotHeld(cs_main); // For performance reasons + UniValue result(UniValue::VOBJ); result.pushKV("hash", blockindex->GetBlockHash().GetHex()); const CBlockIndex* pnext; @@ -121,6 +123,8 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails) { // Serialize passed information without accessing chain state of the active chain! + AssertLockNotHeld(cs_main); // For performance reasons + UniValue result(UniValue::VOBJ); result.pushKV("hash", blockindex->GetBlockHash().GetHex()); const CBlockIndex* pnext; diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index 55d1de453..ff461fbcb 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -5,9 +5,13 @@ #ifndef BITCOIN_RPC_BLOCKCHAIN_H #define BITCOIN_RPC_BLOCKCHAIN_H -#include -#include #include +#include + +#include +#include + +extern RecursiveMutex cs_main; class CBlock; class CBlockIndex; @@ -28,7 +32,7 @@ double GetDifficulty(const CBlockIndex* blockindex); void RPCNotifyBlockChange(bool ibd, const CBlockIndex *); /** Block description to JSON */ -UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails = false); +UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails = false) LOCKS_EXCLUDED(cs_main); /** Mempool information to JSON */ UniValue MempoolInfoToJSON(const CTxMemPool& pool); @@ -37,7 +41,7 @@ UniValue MempoolInfoToJSON(const CTxMemPool& pool); UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose = false); /** Block header to JSON */ -UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex); +UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex) LOCKS_EXCLUDED(cs_main); /** Used by getblockstats to get feerates at different percentiles by weight */ void CalculatePercentilesByWeight(CAmount result[NUM_GETBLOCKSTATS_PERCENTILES], std::vector>& scores, int64_t total_weight);