[rpc] Allow fetching tx directly from specified block in getrawtransaction #10275

Open
wants to merge 7 commits into
from
View
@@ -61,12 +61,15 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, UniValue& entry)
UniValue getrawtransaction(const JSONRPCRequest& request)
{
- if (request.fHelp || request.params.size() < 1 || request.params.size() > 2)
+ if (request.fHelp || request.params.size() < 1 || request.params.size() > 3)
throw std::runtime_error(
- "getrawtransaction \"txid\" ( verbose )\n"
+ "getrawtransaction \"txid\" ( verbose \"blockhash|height\" )\n"
"\nNOTE: By default this function only works for mempool transactions. If the -txindex option is\n"
- "enabled, it also works for blockchain transactions.\n"
+ "enabled, it also works for blockchain transactions. If the block which contains the transaction\n"
+ "is known, its hash can be provided even for nodes without -txindex. Note that if a blockhash is\n"
+ "provided, only that block will be searched and if the transaction is in the mempool or other\n"
+ "blocks, or if this node does not have the given block available, the transaction will not be found.\n"
"DEPRECATED: for now, it also works for transactions with unspent outputs.\n"
"\nReturn the raw transaction data.\n"
@@ -76,12 +79,14 @@ UniValue getrawtransaction(const JSONRPCRequest& request)
"\nArguments:\n"
"1. \"txid\" (string, required) The transaction id\n"
"2. verbose (bool, optional, default=false) If false, return a string, otherwise return a json object\n"
+ "3. \"blockhash|height\" (string or numeric, optional) The block hash or block height of the block in which to look for the transaction\n"
"\nResult (if verbose is not set or set to false):\n"
"\"data\" (string) The serialized, hex-encoded data for 'txid'\n"
"\nResult (if verbose is set to true):\n"
"{\n"
+ " \"inActiveChain\": b, (bool) Whether specified block is in the active chain or not (only present with explicit \"blockhash|height\" argument)\n"
" \"hex\" : \"data\", (string) The serialized, hex-encoded data for 'txid'\n"
" \"txid\" : \"id\", (string) The transaction id (same as provided)\n"
" \"hash\" : \"id\", (string) The transaction hash (differs from txid for witness transactions)\n"
@@ -129,36 +134,56 @@ UniValue getrawtransaction(const JSONRPCRequest& request)
+ HelpExampleCli("getrawtransaction", "\"mytxid\"")
+ HelpExampleCli("getrawtransaction", "\"mytxid\" true")
+ HelpExampleRpc("getrawtransaction", "\"mytxid\", true")
+ + HelpExampleCli("getrawtransaction", "\"mytxid\" false \"myblockhash\"")
+ + HelpExampleCli("getrawtransaction", "\"mytxid\" true \"myblockhash\"")
);
LOCK(cs_main);
+ bool inActiveChain = true;
uint256 hash = ParseHashV(request.params[0], "parameter 1");
+ CBlockIndex* blockIndex = NULL;
// Accept either a bool (true) or a num (>=1) to indicate verbose output.
bool fVerbose = false;
if (request.params.size() > 1) {
if (request.params[1].isNum()) {
- if (request.params[1].get_int() != 0) {
- fVerbose = true;
- }
+ fVerbose = (request.params[1].get_int() != 0);
+ } else if (!request.params[1].isNull()) {
+ fVerbose = (request.params[1].get_bool());
}
- else if(request.params[1].isBool()) {
- if(request.params[1].isTrue()) {
- fVerbose = true;
+ }
+
+ if (!request.params[2].isNull()) {
+ if (request.params[2].isNum()) {
+ blockIndex = chainActive[request.params[2].get_int()];
+ inActiveChain = true;
+ } else {
+ uint256 blockHash = ParseHashV(request.params[2], "parameter 3");
+ if (!blockHash.IsNull()) {
+ BlockMap::iterator it = mapBlockIndex.find(blockHash);
+ if (it == mapBlockIndex.end()) {
+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block hash not found");
+ }
+ blockIndex = it->second;
+ inActiveChain = (chainActive[blockIndex->nHeight] == blockIndex);
}
}
- else {
- throw JSONRPCError(RPC_TYPE_ERROR, "Invalid type provided. Verbose parameter must be a boolean.");
- }
}
CTransactionRef tx;
uint256 hashBlock;
- if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock, true))
- throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string(fTxIndex ? "No such mempool or blockchain transaction"
- : "No such mempool transaction. Use -txindex to enable blockchain transaction queries") +
- ". Use gettransaction for wallet transactions.");
+ if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock, true, blockIndex)) {
+ std::string errmsg;
+ if (blockIndex) {
+ errmsg = "No such transaction found in the provided block";
+ } else {
+ errmsg = fTxIndex
+ ? "No such mempool or blockchain transaction"
+ : "No such mempool transaction. Use -txindex to enable blockchain transaction queries";
+ }
+ throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, errmsg + ". Use gettransaction for wallet transactions.");
+ }
std::string strHex = EncodeHexTx(*tx, RPCSerializationFlags());
@@ -167,6 +192,7 @@ UniValue getrawtransaction(const JSONRPCRequest& request)
UniValue result(UniValue::VOBJ);
result.push_back(Pair("hex", strHex));
+ if (blockIndex) result.push_back(Pair("inActiveChain", inActiveChain));
TxToJSON(*tx, hashBlock, result);
return result;
}
@@ -900,7 +926,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
static const CRPCCommand commands[] =
{ // category name actor (function) okSafeMode
// --------------------- ------------------------ ----------------------- ----------
- { "rawtransactions", "getrawtransaction", &getrawtransaction, true, {"txid","verbose"} },
+ { "rawtransactions", "getrawtransaction", &getrawtransaction, true, {"txid","verbose","blockhash"} },
{ "rawtransactions", "createrawtransaction", &createrawtransaction, true, {"inputs","outputs","locktime","replaceable"} },
{ "rawtransactions", "decoderawtransaction", &decoderawtransaction, true, {"hexstring"} },
{ "rawtransactions", "decodescript", &decodescript, true, {"hexstring"} },
View
@@ -896,43 +896,44 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa
}
/** Return transaction in txOut, and if it was found inside a block, its hash is placed in hashBlock */
-bool GetTransaction(const uint256 &hash, CTransactionRef &txOut, const Consensus::Params& consensusParams, uint256 &hashBlock, bool fAllowSlow)
+bool GetTransaction(const uint256 &hash, CTransactionRef &txOut, const Consensus::Params& consensusParams, uint256 &hashBlock, bool fAllowSlow, CBlockIndex* blockIndex)
{
- CBlockIndex *pindexSlow = NULL;
+ CBlockIndex* pindexSlow = blockIndex;
LOCK(cs_main);
- CTransactionRef ptx = mempool.get(hash);
- if (ptx)
- {
- txOut = ptx;
- return true;
- }
+ if (!blockIndex) {
@jtimon

jtimon Jul 17, 2017

Member

Perhaps the diff can be less disruptive by moving everything inside if (!blockIndex) {...} to a new static function defined right above instead of indenting all of it?

@kallewoof

kallewoof Jul 18, 2017

Contributor

I'm not a huge fan of changing code just to make diffs smaller, and as I mention above, you can put ?w=1 to get diff without indentation changes.

+ CTransactionRef ptx = mempool.get(hash);
+ if (ptx) {
+ txOut = ptx;
+ return true;
+ }
- if (fTxIndex) {
- CDiskTxPos postx;
- if (pblocktree->ReadTxIndex(hash, postx)) {
- CAutoFile file(OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION);
- if (file.IsNull())
- return error("%s: OpenBlockFile failed", __func__);
- CBlockHeader header;
- try {
- file >> header;
- fseek(file.Get(), postx.nTxOffset, SEEK_CUR);
- file >> txOut;
- } catch (const std::exception& e) {
- return error("%s: Deserialize or I/O error - %s", __func__, e.what());
+ if (fTxIndex) {
+ CDiskTxPos postx;
+ if (pblocktree->ReadTxIndex(hash, postx)) {
+ CAutoFile file(OpenBlockFile(postx, true), SER_DISK, CLIENT_VERSION);
+ if (file.IsNull())
+ return error("%s: OpenBlockFile failed", __func__);
+ CBlockHeader header;
+ try {
+ file >> header;
+ fseek(file.Get(), postx.nTxOffset, SEEK_CUR);
+ file >> txOut;
+ } catch (const std::exception& e) {
+ return error("%s: Deserialize or I/O error - %s", __func__, e.what());
+ }
+ hashBlock = header.GetHash();
+ if (txOut->GetHash() != hash)
+ return error("%s: txid mismatch", __func__);
+ return true;
}
- hashBlock = header.GetHash();
- if (txOut->GetHash() != hash)
- return error("%s: txid mismatch", __func__);
- return true;
}
- }
- if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it
- const Coin& coin = AccessByTxid(*pcoinsTip, hash);
- if (!coin.IsSpent()) pindexSlow = chainActive[coin.nHeight];
+ if (fAllowSlow) { // use coin database to locate block that contains transaction, and scan it
+ const Coin& coin = AccessByTxid(*pcoinsTip, hash);
+ if (!coin.IsSpent()) pindexSlow = chainActive[coin.nHeight];
+ }
}
if (pindexSlow) {
View
@@ -277,7 +277,7 @@ bool IsInitialBlockDownload();
*/
std::string GetWarnings(const std::string& strFor);
/** Retrieve a transaction (from memory pool, or from disk, if possible) */
-bool GetTransaction(const uint256 &hash, CTransactionRef &tx, const Consensus::Params& params, uint256 &hashBlock, bool fAllowSlow = false);
+bool GetTransaction(const uint256 &hash, CTransactionRef &tx, const Consensus::Params& params, uint256 &hashBlock, bool fAllowSlow = false, CBlockIndex* blockIndex = NULL);
/** Find the best known block, and make it the tip of the block chain */
bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr<const CBlock> pblock = std::shared_ptr<const CBlock>());
CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams);
@@ -52,6 +52,31 @@ def run_test(self):
# This will raise an exception since there are missing inputs
assert_raises_jsonrpc(-25, "Missing inputs", self.nodes[2].sendrawtransaction, rawtx['hex'])
+ ############################
+ # getrawtx with block hash #
+ ############################
+
+ # make a tx by sending then generate 2 blocks; block1 has the tx in it,
+ # presumably
+ tx = self.nodes[2].sendtoaddress(self.nodes[1].getnewaddress(), 1)
+ block1, block2 = self.nodes[2].generate(2)
+ self.sync_all()
+ # We should be able to get the raw transaction by providing the correct block
+ assert_equal(self.nodes[0].getrawtransaction(tx, True, block1)['txid'], tx)
+ # We should not get the tx if we provide an unrelated block
+ assert_raises_jsonrpc(-5, "No such", self.nodes[0].getrawtransaction, tx, True, block2)
+
+ ##############################
+ # getrawtx with block height #
+ ##############################
+
+ # Get block count
+ blocks = self.nodes[2].getblockcount()
+ # We should be able to get the raw transaction by providing the correct block height (count-1)
+ assert_equal(self.nodes[0].getrawtransaction(tx, True, blocks-1)['txid'], tx)
+ # We should not get the tx if we provide an unrelated block
+ assert_raises_jsonrpc(-5, "No such", self.nodes[0].getrawtransaction, tx, True, blocks)
+
#########################
# RAW TX MULTISIG TESTS #
#########################
@@ -143,13 +168,13 @@ def run_test(self):
assert_equal(self.nodes[0].getrawtransaction(txHash, True)["hex"], rawTxSigned['hex'])
# 6. invalid parameters - supply txid and string "Flase"
- assert_raises_jsonrpc(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, "Flase")
+ assert_raises_jsonrpc(-1,"not a boolean", self.nodes[0].getrawtransaction, txHash, "Flase")
# 7. invalid parameters - supply txid and empty array
- assert_raises_jsonrpc(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, [])
+ assert_raises_jsonrpc(-1,"not a boolean", self.nodes[0].getrawtransaction, txHash, [])
# 8. invalid parameters - supply txid and empty dict
- assert_raises_jsonrpc(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, {})
+ assert_raises_jsonrpc(-1,"not a boolean", self.nodes[0].getrawtransaction, txHash, {})
inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 1000}]
outputs = { self.nodes[0].getnewaddress() : 1 }