From a5f5a2ce53622b8d2e3fda659e497d47c5d164b0 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Sat, 6 May 2017 12:49:18 +0900 Subject: [PATCH 1/3] [rpc] Fix fVerbose parsing (remove excess if cases). --- src/rpc/rawtransaction.cpp | 14 +------------- test/functional/rawtransactions.py | 6 +++--- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 3aff1e9fbff..5d558d8bf99 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -141,19 +141,7 @@ UniValue getrawtransaction(const JSONRPCRequest& request) // Accept either a bool (true) or a num (>=1) to indicate verbose output. bool fVerbose = false; if (!request.params[1].isNull()) { - if (request.params[1].isNum()) { - if (request.params[1].get_int() != 0) { - fVerbose = true; - } - } - else if(request.params[1].isBool()) { - if(request.params[1].isTrue()) { - fVerbose = true; - } - } - else { - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid type provided. Verbose parameter must be a boolean."); - } + fVerbose = request.params[1].isNum() ? (request.params[1].get_int() != 0) : request.params[1].get_bool(); } CTransactionRef tx; diff --git a/test/functional/rawtransactions.py b/test/functional/rawtransactions.py index 2777cb96937..34e1e8ece62 100755 --- a/test/functional/rawtransactions.py +++ b/test/functional/rawtransactions.py @@ -188,13 +188,13 @@ class RawTransactionsTest(BitcoinTestFramework): assert_equal(self.nodes[0].getrawtransaction(txHash, True)["hex"], rawTxSigned['hex']) # 6. invalid parameters - supply txid and string "Flase" - assert_raises_rpc_error(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, "Flase") + assert_raises_rpc_error(-1,"not a boolean", self.nodes[0].getrawtransaction, txHash, "Flase") # 7. invalid parameters - supply txid and empty array - assert_raises_rpc_error(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, []) + assert_raises_rpc_error(-1,"not a boolean", self.nodes[0].getrawtransaction, txHash, []) # 8. invalid parameters - supply txid and empty dict - assert_raises_rpc_error(-3,"Invalid type", self.nodes[0].getrawtransaction, txHash, {}) + assert_raises_rpc_error(-1,"not a boolean", self.nodes[0].getrawtransaction, txHash, {}) inputs = [ {'txid' : "1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000", 'vout' : 1, 'sequence' : 1000}] outputs = { self.nodes[0].getnewaddress() : 1 } From b16795167704687d908f881dacf04d388db28cb3 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Tue, 25 Apr 2017 17:29:24 +0900 Subject: [PATCH 2/3] [rpc] Allow getrawtransaction to take optional blockhash to fetch transaction from a block directly. --- src/rpc/rawtransaction.cpp | 57 ++++++++++++++++++++++++------- src/validation.cpp | 70 ++++++++++++++++++++------------------ src/validation.h | 2 +- 3 files changed, 83 insertions(+), 46 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 5d558d8bf99..b26f10e476f 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -64,12 +64,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\" )\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" @@ -78,13 +81,15 @@ 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" + "2. verbose (bool, optional, default=false) If false, return a string, otherwise return a json object\n" + "3. \"blockhash\" (string, optional) 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" + " \"in_active_chain\": b, (bool) Whether specified block is in the active chain or not (only present with explicit \"blockhash\" 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" @@ -132,11 +137,15 @@ 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 in_active_chain = true; uint256 hash = ParseHashV(request.params[0], "parameter 1"); + CBlockIndex* blockindex = nullptr; // Accept either a bool (true) or a num (>=1) to indicate verbose output. bool fVerbose = false; @@ -144,18 +153,42 @@ UniValue getrawtransaction(const JSONRPCRequest& request) fVerbose = request.params[1].isNum() ? (request.params[1].get_int() != 0) : request.params[1].get_bool(); } + if (!request.params[2].isNull()) { + 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; + in_active_chain = chainActive.Contains(blockindex); + } + } + 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."); + uint256 hash_block; + if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, true, blockindex)) { + std::string errmsg; + if (blockindex) { + if (!(blockindex->nStatus & BLOCK_HAVE_DATA)) { + throw JSONRPCError(RPC_MISC_ERROR, "Block not available"); + } + 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."); + } - if (!fVerbose) + if (!fVerbose) { return EncodeHexTx(*tx, RPCSerializationFlags()); + } UniValue result(UniValue::VOBJ); - TxToJSON(*tx, hashBlock, result); + if (blockindex) result.push_back(Pair("in_active_chain", in_active_chain)); + TxToJSON(*tx, hash_block, result); return result; } @@ -983,7 +1016,7 @@ UniValue sendrawtransaction(const JSONRPCRequest& request) static const CRPCCommand commands[] = { // category name actor (function) argNames // --------------------- ------------------------ ----------------------- ---------- - { "rawtransactions", "getrawtransaction", &getrawtransaction, {"txid","verbose"} }, + { "rawtransactions", "getrawtransaction", &getrawtransaction, {"txid","verbose","blockhash"} }, { "rawtransactions", "createrawtransaction", &createrawtransaction, {"inputs","outputs","locktime","replaceable"} }, { "rawtransactions", "decoderawtransaction", &decoderawtransaction, {"hexstring"} }, { "rawtransactions", "decodescript", &decodescript, {"hexstring"} }, diff --git a/src/validation.cpp b/src/validation.cpp index 99ea1433f9c..16d2ff2a50d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -926,47 +926,51 @@ bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransa return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, pfMissingInputs, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee); } -/** 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) +/** + * 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, bool fAllowSlow, CBlockIndex* blockIndex) { - CBlockIndex *pindexSlow = nullptr; + CBlockIndex* pindexSlow = blockIndex; LOCK(cs_main); - 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()); - } - hashBlock = header.GetHash(); - if (txOut->GetHash() != hash) - return error("%s: txid mismatch", __func__); + if (!blockIndex) { + CTransactionRef ptx = mempool.get(hash); + if (ptx) { + txOut = ptx; return true; } - // transaction not found in index, nothing more can be done - return false; - } + 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; + } - 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]; + // transaction not found in index, nothing more can be done + return false; + } + + 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) { diff --git a/src/validation.h b/src/validation.h index 254f3e07549..ec17d0d92d6 100644 --- a/src/validation.h +++ b/src/validation.h @@ -273,7 +273,7 @@ void ThreadScriptCheck(); /** Check whether we are doing an initial block download (synchronizing from disk or network) */ bool IsInitialBlockDownload(); /** 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 = nullptr); /** Find the best known block, and make it the tip of the block chain */ bool ActivateBestChain(CValidationState& state, const CChainParams& chainparams, std::shared_ptr pblock = std::shared_ptr()); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); From 434526aba680cb73208e018a02827d51a71cfff6 Mon Sep 17 00:00:00 2001 From: Karl-Johan Alm Date: Mon, 8 May 2017 13:24:32 +0900 Subject: [PATCH 3/3] [test] Add tests for getrawtransaction with block hash. --- test/functional/rawtransactions.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/functional/rawtransactions.py b/test/functional/rawtransactions.py index 34e1e8ece62..45d2410f8ba 100755 --- a/test/functional/rawtransactions.py +++ b/test/functional/rawtransactions.py @@ -50,6 +50,30 @@ class RawTransactionsTest(BitcoinTestFramework): # This will raise an exception since there are missing inputs assert_raises_rpc_error(-25, "Missing inputs", self.nodes[2].sendrawtransaction, rawtx['hex']) + ##################################### + # getrawtransaction with block hash # + ##################################### + + # make a tx by sending then generate 2 blocks; block1 has the tx in it + 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 + gottx = self.nodes[0].getrawtransaction(tx, True, block1) + assert_equal(gottx['txid'], tx) + assert_equal(gottx['in_active_chain'], True) + # We should not have the 'in_active_chain' flag when we don't provide a block + gottx = self.nodes[0].getrawtransaction(tx, True) + assert_equal(gottx['txid'], tx) + assert 'in_active_chain' not in gottx + # We should not get the tx if we provide an unrelated block + assert_raises_rpc_error(-5, "No such transaction found", self.nodes[0].getrawtransaction, tx, True, block2) + # An invalid block hash should raise the correct errors + assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal", self.nodes[0].getrawtransaction, tx, True, True) + assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal", self.nodes[0].getrawtransaction, tx, True, "foobar") + assert_raises_rpc_error(-8, "parameter 3 must be of length 64", self.nodes[0].getrawtransaction, tx, True, "abcd1234") + assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa") + ######################### # RAW TX MULTISIG TESTS # #########################