maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge #19589: rpc: Avoid useless mempool query in gettxoutproof

Browse source 
fa5979d12f rpc: Avoid useless mempool query in gettxoutproof (MarcoFalke)
fa1f7f28cb rpc: Style fixups in gettxoutproof (MarcoFalke)

Pull request description:

  `GetTransaction` implicitly and unconditionally asks the mempool global for a transaction. This is problematic for several reasons:

  * `gettxoutproof` is for on-chain txs only and asking the mempool for on-chain txs is confusing and minimally wasteful
  * Globals are confusing and make code harder to test with unit tests

  Fix both issues by passing in an optional mempool. This also helps with #19556

ACKs for top commit:
  hebasto:
    re-ACK fa5979d12f
  jnewbery:
    utACK fa5979d12f
  promag:
    Code review ACK fa5979d12f.

Tree-SHA512: 048361b82abfcc40481181bd44f70cfc9e97d5d6356549df34bbe30b9de7a0a72d2207a3ad0279b21f06293509b284d8967f58ca7e716263a22b20aa4e7f9c54
This commit is contained in:
fanquake 2020-07-28 14:27:41 +08:00
parent b62fbf9e1c fa5979d12f
commit a1da180b1b
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
4 changed files with 76 additions and 48 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

36
src/rest.cpp
View file

@ -67,14 +67,33 @@ static bool RESTERR(HTTPRequest* req, enum HTTPStatusCode status, std::string me
return false;
}
/**
* Get the node context.
*
* @param[in] req The HTTP request, whose status code will be set if node
* context is not found.
* @returns Pointer to the node context or nullptr if not found.
*/
static NodeContext* GetNodeContext(const util::Ref& context, HTTPRequest* req)
{
NodeContext* node = context.Has<NodeContext>() ? &context.Get<NodeContext>() : nullptr;
if (!node) {
RESTERR(req, HTTP_INTERNAL_SERVER_ERROR,
strprintf("%s:%d (%s)\n"
"Internal bug detected: Node context not found!\n"
"You may report this issue here: %s\n",
__FILE__, __LINE__, __func__, PACKAGE_BUGREPORT));
return nullptr;
}
return node;
}
/**
* Get the node context mempool.
*
* Set the HTTP error and return nullptr if node context
* mempool is not found.
*
* @param[in] req the HTTP request
* return pointer to the mempool or nullptr if no mempool found
* @param[in] req The HTTP request, whose status code will be set if node
* context mempool is not found.
* @returns Pointer to the mempool or nullptr if no mempool found.
*/
static CTxMemPool* GetMemPool(const util::Ref& context, HTTPRequest* req)
{
@ -371,10 +390,13 @@ static bool rest_tx(const util::Ref& context, HTTPRequest* req, const std::strin
g_txindex->BlockUntilSyncedToCurrentChain();
}
CTransactionRef tx;
const NodeContext* const node = GetNodeContext(context, req);
if (!node) return false;
uint256 hashBlock = uint256();
if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock))
const CTransactionRef tx = GetTransaction(/* block_index */ nullptr, node->mempool, hash, Params().GetConsensus(), hashBlock);
if (!tx) {
return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found");
}
switch (rf) {
case RetFormat::BINARY: {

35
src/rpc/rawtransaction.cpp
View file

@ -157,6 +157,8 @@ static UniValue getrawtransaction(const JSONRPCRequest& request)
},
}.Check(request);
const NodeContext& node = EnsureNodeContext(request.context);
bool in_active_chain = true;
uint256 hash = ParseHashV(request.params[0], "parameter 1");
CBlockIndex* blockindex = nullptr;
@ -188,9 +190,9 @@ static UniValue getrawtransaction(const JSONRPCRequest& request)
f_txindex_ready = g_txindex->BlockUntilSyncedToCurrentChain();
}
CTransactionRef tx;
uint256 hash_block;
if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, blockindex)) {
const CTransactionRef tx = GetTransaction(blockindex, node.mempool, hash, Params().GetConsensus(), hash_block);
if (!tx) {
std::string errmsg;
if (blockindex) {
if (!(blockindex->nStatus & BLOCK_HAVE_DATA)) {
@ -245,10 +247,11 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
for (unsigned int idx = 0; idx < txids.size(); idx++) {
const UniValue& txid = txids[idx];
uint256 hash(ParseHashV(txid, "txid"));
if (setTxids.count(hash))
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ")+txid.get_str());
setTxids.insert(hash);
oneTxid = hash;
if (setTxids.count(hash)) {
throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ") + txid.get_str());
}
setTxids.insert(hash);
oneTxid = hash;
}
CBlockIndex* pblockindex = nullptr;
@ -281,11 +284,11 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
LOCK(cs_main);
if (pblockindex == nullptr)
{
CTransactionRef tx;
if (!GetTransaction(oneTxid, tx, Params().GetConsensus(), hashBlock) || hashBlock.IsNull())
if (pblockindex == nullptr) {
const CTransactionRef tx = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, oneTxid, Params().GetConsensus(), hashBlock);
if (!tx || hashBlock.IsNull()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block");
}
pblockindex = LookupBlockIndex(hashBlock);
if (!pblockindex) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction index corrupt");
@ -293,15 +296,19 @@ static UniValue gettxoutproof(const JSONRPCRequest& request)
}
CBlock block;
if(!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus()))
if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) {
throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk");
}
unsigned int ntxFound = 0;
for (const auto& tx : block.vtx)
if (setTxids.count(tx->GetHash()))
for (const auto& tx : block.vtx) {
if (setTxids.count(tx->GetHash())) {
ntxFound++;
if (ntxFound != setTxids.size())
}
}
if (ntxFound != setTxids.size()) {
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Not all transactions found in specified or retrieved block");
}
CDataStream ssMB(SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS);
CMerkleBlock mb(block, setTxids);

38
src/validation.cpp
View file

@ -1089,45 +1089,33 @@ bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTrans
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept);
}
/**
* Return transaction in txOut, and if it was found inside a block, its hash is placed in hashBlock.
* If blockIndex is provided, the transaction is fetched from the corresponding block.
*/
bool GetTransaction(const uint256& hash, CTransactionRef& txOut, const Consensus::Params& consensusParams, uint256& hashBlock, const CBlockIndex* const block_index)
CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock)
{
LOCK(cs_main);
if (!block_index) {
CTransactionRef ptx = mempool.get(hash);
if (ptx) {
txOut = ptx;
return true;
}
if (g_txindex) {
return g_txindex->FindTx(hash, hashBlock, txOut);
}
} else {
if (block_index) {
CBlock block;
if (ReadBlockFromDisk(block, block_index, consensusParams)) {
for (const auto& tx : block.vtx) {
if (tx->GetHash() == hash) {
txOut = tx;
hashBlock = block_index->GetBlockHash();
return true;
return tx;
}
}
}
return nullptr;
}
return false;
if (mempool) {
CTransactionRef ptx = mempool->get(hash);
if (ptx) return ptx;
}
if (g_txindex) {
CTransactionRef tx;
if (g_txindex->FindTx(hash, hashBlock, tx)) return tx;
}
return nullptr;
}
//////////////////////////////////////////////////////////////////////////////
//
// CBlock and CBlockIndex

15
src/validation.h
View file

@ -164,8 +164,19 @@ bool LoadGenesisBlock(const CChainParams& chainparams);
void UnloadBlockIndex();
/** Run an instance of the script checking thread */
void ThreadScriptCheck(int worker_num);
/** Retrieve a transaction (from memory pool, or from disk, if possible) */
bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, const CBlockIndex* const blockIndex = nullptr);
/**
* Return transaction from the block at block_index.
* If block_index is not provided, fall back to mempool.
* If mempool is not provided or the tx couldn't be found in mempool, fall back to g_txindex.
*
* @param[in] block_index The block to read from disk, or nullptr
* @param[in] mempool If block_index is not provided, look in the mempool, if provided
* @param[in] hash The txid
* @param[in] consensusParams The params
* @param[out] hashBlock The hash of block_index, if the tx was found via block_index
* @returns The tx if found, otherwise nullptr
*/
CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock);
/**
* Find the best known block, and make it the tip of the block chain
*