From 135d6507c70d73b97e389b41c79e89e0522d8b91 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Wed, 20 Oct 2021 16:32:26 +0200 Subject: [PATCH 1/9] Merge bitcoin/bitcoin#23314: build: explicitly disable libsecp256k1 openssl based tests d7524546abf1fa5be8e6317ee50585e966ae6b4c build: explicitly disable libsecp256k1 openssl based tests (fanquake) Pull request description: These tests are failing when run against OpenSSL 3, and have been removed upstream, bitcoin-core/secp256k1#983, so disabled them for now to avoid `make check` failures. Note that this will also remove warning output from our build, due to the use of deprecated OpenSSL API functions. See bitcoin#23048. ACKs for top commit: MarcoFalke: cr ACK d7524546abf1fa5be8e6317ee50585e966ae6b4c elichai: Code review ACK d7524546abf1fa5be8e6317ee50585e966ae6b4c Tree-SHA512: a3805b4123ec49aaf21378066d86be382d11a022a7530682c2d8c0e756e785f32f18bea6fe73b1eca73f70bc3aee9166c391a9bf6adc0e450ebb0ce0bcf5a45e --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ca19fa8b6f30..eae2bd3f9dcb 100644 --- a/configure.ac +++ b/configure.ac @@ -1913,7 +1913,7 @@ CPPFLAGS_TEMP="$CPPFLAGS" unset CPPFLAGS CPPFLAGS="$CPPFLAGS_TEMP" -ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --disable-module-ecdh" +ac_configure_args="${ac_configure_args} --disable-shared --with-pic --enable-benchmark=no --enable-module-recovery --disable-module-ecdh --disable-openssl-tests" AC_CONFIG_SUBDIRS([src/dashbls src/secp256k1]) AC_OUTPUT From 4e8f6afb93070c1aa784cdcca74d88232d51e0e1 Mon Sep 17 00:00:00 2001 From: fanquake Date: Sat, 16 Oct 2021 08:18:05 +0800 Subject: [PATCH 2/9] Merge bitcoin/bitcoin#23268: p2p: Use absolute FQDN for DNS seed domains ca2c313aa291ae44adc1b7148ed49125bdc77bf4 Use absolute FQDN for DNS seed domains (Prayank) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/23193 by using absolute FQDN for domains used by DNS seeds. It improves security and should not break anything based on my research and testing. Few things about absolute FQDN are shared in https://superuser.com/questions/1467958/why-does-putting-a-dot-after-the-url-remove-login-information Master branch: ``` DNS seed x9.dnsseed.bitcoin.dashjr.org. responded with IP 127.0.0.1 ``` PR branch: ``` DNS seed x9.dnsseed.bitcoin.dashjr.org. responded with IP 159.89.108.149 ``` Reviewers can follow the steps mentioned in Issue to test: https://github.com/bitcoin/bitcoin/issues/23193#issuecomment-937717145 ACKs for top commit: practicalswift: cr ACK ca2c313aa291ae44adc1b7148ed49125bdc77bf4 laanwj: code review ACK ca2c313aa291ae44adc1b7148ed49125bdc77bf4 promag: Code review ACK ca2c313aa291ae44adc1b7148ed49125bdc77bf4. Tree-SHA512: 9818227332282a78c45b4470c2fc80bf899ed78aed76644ebf014e0fff1b139402ea023acdea162363a478b6f6613dbf1da57e214d2240ea0f04310473f57cca --- src/chainparams.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 66ead16b3011..9edc72a93f4c 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -250,7 +250,7 @@ class CMainParams : public CChainParams { // This is fine at runtime as we'll fall back to using them as an addrfetch if they don't support the // service bits we want, but we should get them updated to support all service bits wanted by any // release ASAP to avoid it where possible. - vSeeds.emplace_back("dnsseed.dash.org"); + vSeeds.emplace_back("dnsseed.dash.org."); // Dash addresses start with 'X' base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,76); @@ -443,7 +443,7 @@ class CTestNetParams : public CChainParams { vSeeds.clear(); // nodes with support for servicebits filtering should be at the top - vSeeds.emplace_back("testnet-seed.dashdot.io"); // Just a static list of stable node(s), only supports x9 + vSeeds.emplace_back("testnet-seed.dashdot.io."); // Just a static list of stable node(s), only supports x9 // Testnet Dash addresses start with 'y' base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,140); @@ -617,7 +617,7 @@ class CDevNetParams : public CChainParams { vFixedSeeds.clear(); vSeeds.clear(); - //vSeeds.push_back(CDNSSeedData("dashevo.org", "devnet-seed.dashevo.org")); + //vSeeds.push_back(CDNSSeedData("dashevo.org.", "devnet-seed.dashevo.org.")); // Testnet Dash addresses start with 'y' base58Prefixes[PUBKEY_ADDRESS] = std::vector(1,140); From 6f46f589cef41ca642f99ae63f1d2305ddd9dcb8 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Tue, 19 Oct 2021 15:47:28 +0200 Subject: [PATCH 3/9] Merge bitcoin/bitcoin#22918: rpc: Add level 3 verbosity to getblock RPC call (#21245 modified) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 5c34507ecbbdc29c086276d1c62835b461823507 core_write: Rename calculate_fee to have_undo for clarity (fyquah) 8edf6204a87057a451160d1e61e79d8be112e81f release-notes: Add release note about getblock verbosity level 3. (fyquah) 459104b2aae6eeaadfa5a7e47944f1a34780dacd rest: Add test for prevout fields in getblock (fyquah) 4330af6f72172848f5971a052a8f325ed50eb576 rpc: Add test for level 3 verbosity getblock rpc call. (fyquah) 51dbc167e98daab317baa80cf80bfda337672dab rpc: Add level 3 verbosity to getblock RPC call. (fyquah) 3cc95345ca49b87e8caca9a0e6418c63ae1e463a rpc: Replace boolean argument for tx details with enum class. (fyquah) Pull request description: Author of #21245 expressed [time issues](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-902332088) in the original PR. Given that #21245 has received a lot of review*, I have decided to open this new pull request with [modifications required to get ACK from luke-jr ](https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-905150806) and a few nits of mine. ### Original PR description > Display the prevout in transaction inputs when calling getblock level 3 verbosity. This PR affects the existing `/rest/block` API by adding a `prevout` fields to tx inputs. This is mentioned in the change to the release notes. > > I added some functional tests that > > * checks that the RPC call still works when TxUndo can't be found > > * Doesn't display the "value" or "scriptPubKey" of the previous output when at a lower verbosity level > > > This "completes" the issue #18771 ### Possible improvements * https://github.com/kiminuo/bitcoin/commit/b0bf4f255f86aeaddce68889087c22f9068f4d97 - I can include even this commit to this PR if deemed useful or I can leave it for a follow-up PR. See https://github.com/bitcoin/bitcoin/pull/21245#issuecomment-894853784 for more context. ### Examples Examples of the `getblock` output with various verbose levels. Note that `000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5` contains only 2 transactions. #### Verbose level 0 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 0 ``` ##### Verbose level 1 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 1 ``` ##### Verbose level 2 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 2 ``` ##### Verbose level 3 ```bash ./bitcoin-cli -testnet getblock 000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5 3 ``` #### REST ```bash curl -H "content-type:text/plain;" http://127.0.0.1:18332/rest/block/000000000000001f682b188971cc1a121546be4e9d5baf22934fdc7f538288d5.json ``` * ... and my everyday obsessive checking of my email inbox whether the PR moves forward. Edit laanwj: Removed at symbol from message, and large example output to prevent it from all ending up in the commit message. ACKs for top commit: 0xB10C: ACK 5c34507ecbbdc29c086276d1c62835b461823507 meshcollider: utACK 5c34507ecbbdc29c086276d1c62835b461823507 theStack: ACK 5c34507ecbbdc29c086276d1c62835b461823507 👘 promag: Concept ACK 5c34507ecbbdc29c086276d1c62835b461823507 Tree-SHA512: bbff120d8fd76e617b723b102b0c606e0d8eb27f21c631d5f4cdab0892137c4bc7c65b1df144993405f942c91be47a26e80480102af55bff22621c19f518aea3 --- doc/release-notes-22918.md | 11 ++++++ src/bench/rpc_blockchain.cpp | 4 +- src/core_io.h | 11 +++++- src/core_write.cpp | 31 ++++++++++++--- src/rest.cpp | 8 ++-- src/rpc/blockchain.cpp | 57 ++++++++++++++++---------- src/rpc/blockchain.h | 3 +- src/rpc/rawtransaction.cpp | 2 +- test/functional/interface_rest.py | 9 +++++ test/functional/rpc_blockchain.py | 66 +++++++++++++++++++++++++------ 10 files changed, 153 insertions(+), 49 deletions(-) create mode 100644 doc/release-notes-22918.md diff --git a/doc/release-notes-22918.md b/doc/release-notes-22918.md new file mode 100644 index 000000000000..5220513cd890 --- /dev/null +++ b/doc/release-notes-22918.md @@ -0,0 +1,11 @@ +## Updated RPCs + +- The `getblock` RPC command now supports verbose level 3 containing transaction inputs + `prevout` information. The existing `/rest/block/` REST endpoint is modified to contain + this information too. Every `vin` field will contain an additional `prevout` subfield + describing the spent output. `prevout` contains the following keys: + - `generated` - true if the spent coins was a coinbase. + - `height` + - `value` + - `scriptPubKey` + diff --git a/src/bench/rpc_blockchain.cpp b/src/bench/rpc_blockchain.cpp index a95397897a65..8585679dcf2e 100644 --- a/src/bench/rpc_blockchain.cpp +++ b/src/bench/rpc_blockchain.cpp @@ -45,7 +45,7 @@ static void BlockToJsonVerbose(benchmark::Bench& bench) TestBlockAndIndex data; const LLMQContext& llmq_ctx = *data.testing_setup->m_node.llmq_ctx; bench.run([&] { - auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, /*verbose*/ true); + auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, TxVerbosity::SHOW_DETAILS_AND_PREVOUT); ankerl::nanobench::doNotOptimizeAway(univalue); }); } @@ -56,7 +56,7 @@ static void BlockToJsonVerboseWrite(benchmark::Bench& bench) { TestBlockAndIndex data; const LLMQContext& llmq_ctx = *data.testing_setup->m_node.llmq_ctx; - auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, /*verbose*/ true); + auto univalue = blockToJSON(data.testing_setup->m_node.chainman->m_blockman, data.block, &data.blockindex, &data.blockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, TxVerbosity::SHOW_DETAILS_AND_PREVOUT); bench.run([&] { auto str = univalue.write(); ankerl::nanobench::doNotOptimizeAway(str); diff --git a/src/core_io.h b/src/core_io.h index c564e4e49cff..91c4da4064ac 100644 --- a/src/core_io.h +++ b/src/core_io.h @@ -22,6 +22,15 @@ class CTxUndo; struct CSpentIndexTxInfo; +/** + * Verbose level for block's transaction + */ +enum class TxVerbosity { + SHOW_TXID, //!< Only TXID for each block's transaction + SHOW_DETAILS, //!< Include TXID, inputs, outputs, and other common block's transaction information + SHOW_DETAILS_AND_PREVOUT //!< The same as previous option with information about prevouts if available +}; + // core_read.cpp CScript ParseScript(const std::string& s); std::string ScriptToAsmStr(const CScript& script, const bool fAttemptSighashDecode = false); @@ -47,6 +56,6 @@ std::string EncodeHexTx(const CTransaction& tx); std::string SighashToStr(unsigned char sighash_type); void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include_hex, bool include_address = true); void ScriptToUniv(const CScript& script, UniValue& out); -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr, const CSpentIndexTxInfo* ptxSpentInfo = nullptr); +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex = true, int serialize_flags = 0, const CTxUndo* txundo = nullptr, TxVerbosity verbosity = TxVerbosity::SHOW_DETAILS, const CSpentIndexTxInfo* ptxSpentInfo = nullptr); #endif // BITCOIN_CORE_IO_H diff --git a/src/core_write.cpp b/src/core_write.cpp index 493b83f3ac60..19c646fa6ec6 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -173,7 +173,7 @@ void ScriptPubKeyToUniv(const CScript& scriptPubKey, UniValue& out, bool include out.pushKV("type", GetTxnOutputType(type)); } -void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo, const CSpentIndexTxInfo* ptxSpentInfo) +void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, bool include_hex, int serialize_flags, const CTxUndo* txundo, TxVerbosity verbosity, const CSpentIndexTxInfo* ptxSpentInfo) { uint256 txid = tx.GetHash(); entry.pushKV("txid", txid.GetHex()); @@ -188,7 +188,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, // If available, use Undo data to calculate the fee. Note that txundo == nullptr // for coinbase transactions and for transactions where undo data is unavailable. - const bool calculate_fee = txundo != nullptr; + const bool have_undo = txundo != nullptr; CAmount amt_total_in = 0; CAmount amt_total_out = 0; @@ -221,9 +221,28 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, } } } - if (calculate_fee) { - const CTxOut& prev_txout = txundo->vprevout[i].out; + if (have_undo) { + const Coin& prev_coin = txundo->vprevout[i]; + const CTxOut& prev_txout = prev_coin.out; + amt_total_in += prev_txout.nValue; + switch (verbosity) { + case TxVerbosity::SHOW_TXID: + case TxVerbosity::SHOW_DETAILS: + break; + + case TxVerbosity::SHOW_DETAILS_AND_PREVOUT: + UniValue o_script_pub_key(UniValue::VOBJ); + ScriptPubKeyToUniv(prev_txout.scriptPubKey, o_script_pub_key, /* includeHex */ true, /*include_address=*/true); + + UniValue p(UniValue::VOBJ); + p.pushKV("generated", bool(prev_coin.fCoinBase)); + p.pushKV("height", uint64_t(prev_coin.nHeight)); + p.pushKV("value", ValueFromAmount(prev_txout.nValue)); + p.pushKV("scriptPubKey", o_script_pub_key); + in.pushKV("prevout", p); + break; + } } in.pushKV("sequence", (int64_t)txin.nSequence); vin.push_back(in); @@ -257,7 +276,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, } vout.push_back(out); - if (calculate_fee) { + if (have_undo) { amt_total_out += txout.nValue; } } @@ -306,7 +325,7 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, } } - if (calculate_fee) { + if (have_undo) { CAmount fee = amt_total_in - amt_total_out; if (tx.IsPlatformTransfer()) { auto payload = GetTxPayload(tx); diff --git a/src/rest.cpp b/src/rest.cpp index f2b53fcdbdfe..ef02f75ab00d 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -289,7 +289,7 @@ static bool rest_headers(const CoreContext& context, static bool rest_block(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart, - bool showTxDetails) + TxVerbosity tx_verbosity) { if (!CheckWarmup(req)) return false; @@ -344,7 +344,7 @@ static bool rest_block(const CoreContext& context, const LLMQContext* llmq_ctx = GetLLMQContext(context, req); if (!llmq_ctx) return false; - UniValue objBlock = blockToJSON(chainman.m_blockman, block, tip, pblockindex, *llmq_ctx->clhandler, *llmq_ctx->isman, showTxDetails); + UniValue objBlock = blockToJSON(chainman.m_blockman, block, tip, pblockindex, *llmq_ctx->clhandler, *llmq_ctx->isman, tx_verbosity); std::string strJSON = objBlock.write() + "\n"; req->WriteHeader("Content-Type", "application/json"); req->WriteReply(HTTP_OK, strJSON); @@ -359,12 +359,12 @@ static bool rest_block(const CoreContext& context, static bool rest_block_extended(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart) { - return rest_block(context, req, strURIPart, true); + return rest_block(context, req, strURIPart, TxVerbosity::SHOW_DETAILS_AND_PREVOUT); } static bool rest_block_notxdetails(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart) { - return rest_block(context, req, strURIPart, false); + return rest_block(context, req, strURIPart, TxVerbosity::SHOW_TXID); } static bool rest_filter_header(const CoreContext& context, HTTPRequest* req, const std::string& strURIPart) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9c5986911fd1..e9ce8ce4c1c6 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -155,31 +155,36 @@ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex return result; } -UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, const llmq::CChainLocksHandler& clhandler, const llmq::CInstantSendManager& isman, bool txDetails) +UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, const llmq::CChainLocksHandler& clhandler, const llmq::CInstantSendManager& isman, TxVerbosity verbosity) { UniValue result = blockheaderToJSON(tip, blockindex, clhandler); result.pushKV("size", (int)::GetSerializeSize(block, PROTOCOL_VERSION)); UniValue txs(UniValue::VARR); - if (txDetails) { - CBlockUndo blockUndo; - const bool have_undo{WITH_LOCK(::cs_main, return !blockman.IsBlockPruned(blockindex) && UndoReadFromDisk(blockUndo, blockindex))}; - for (size_t i = 0; i < block.vtx.size(); ++i) { - const CTransactionRef& tx = block.vtx.at(i); - // coinbase transaction (i == 0) doesn't have undo data - const CTxUndo* txundo = (have_undo && i) ? &blockUndo.vtxundo.at(i - 1) : nullptr; - UniValue objTx(UniValue::VOBJ); - TxToUniv(*tx, uint256(), objTx, true, 0, txundo); - bool fLocked = isman.IsLocked(tx->GetHash()); - objTx.pushKV("instantlock", fLocked || result["chainlock"].get_bool()); - objTx.pushKV("instantlock_internal", fLocked); - txs.push_back(objTx); - } - } else { - for (const CTransactionRef& tx : block.vtx) { - txs.push_back(tx->GetHash().GetHex()); - } + switch (verbosity) { + case TxVerbosity::SHOW_TXID: + for (const CTransactionRef& tx : block.vtx) { + txs.push_back(tx->GetHash().GetHex()); + } + break; + case TxVerbosity::SHOW_DETAILS: + case TxVerbosity::SHOW_DETAILS_AND_PREVOUT: + CBlockUndo blockUndo; + const bool have_undo{WITH_LOCK(::cs_main, return !blockman.IsBlockPruned(blockindex) && UndoReadFromDisk(blockUndo, blockindex))}; + + for (size_t i = 0; i < block.vtx.size(); ++i) { + const CTransactionRef& tx = block.vtx.at(i); + // coinbase transaction (i.e. i == 0) doesn't have undo data + const CTxUndo* txundo = (have_undo && i > 0) ? &blockUndo.vtxundo.at(i - 1) : nullptr; + UniValue objTx(UniValue::VOBJ); + TxToUniv(*tx, uint256(), objTx, true, 0, txundo, verbosity); + bool fLocked = isman.IsLocked(tx->GetHash()); + objTx.pushKV("instantlock", fLocked || result["chainlock"].get_bool()); + objTx.pushKV("instantlock_internal", fLocked); + txs.push_back(objTx); + } } + result.pushKV("tx", txs); if (!block.vtx[0]->vExtraPayload.empty()) { if (const auto opt_cbTx = GetTxPayload(block.vtx[0]->vExtraPayload)) { @@ -872,7 +877,8 @@ static RPCHelpMan getblock() return RPCHelpMan{"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", + "If verbosity is 2, returns an Object with information about block and information about each transaction.\n" + "If verbosity is 3, returns an Object with information about block and information about each transaction, including prevout information for inputs (only for unpruned blocks in the current best chain).\n", { {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"}, {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a json object, and 2 for json object with transaction data"}, @@ -968,7 +974,16 @@ static RPCHelpMan getblock() } const LLMQContext& llmq_ctx = EnsureLLMQContext(node); - return blockToJSON(chainman.m_blockman, block, tip, pblockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, verbosity >= 2); + TxVerbosity tx_verbosity; + if (verbosity == 1) { + tx_verbosity = TxVerbosity::SHOW_TXID; + } else if (verbosity == 2) { + tx_verbosity = TxVerbosity::SHOW_DETAILS; + } else { + tx_verbosity = TxVerbosity::SHOW_DETAILS_AND_PREVOUT; + } + + return blockToJSON(chainman.m_blockman, block, tip, pblockindex, *llmq_ctx.clhandler, *llmq_ctx.isman, tx_verbosity); }, }; } diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h index 13b9eac8575e..18dfa7dc05f8 100644 --- a/src/rpc/blockchain.h +++ b/src/rpc/blockchain.h @@ -6,6 +6,7 @@ #define BITCOIN_RPC_BLOCKCHAIN_H #include +#include #include #include #include @@ -40,7 +41,7 @@ double GetDifficulty(const CBlockIndex* blockindex); void RPCNotifyBlockChange(const CBlockIndex*); /** Block description to JSON */ -UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, const llmq::CChainLocksHandler& clhandler, const llmq::CInstantSendManager& isman, bool txDetails = false) LOCKS_EXCLUDED(cs_main); +UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, const llmq::CChainLocksHandler& clhandler, const llmq::CInstantSendManager& isman, TxVerbosity verbosity) LOCKS_EXCLUDED(cs_main); /** Block header to JSON */ UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex, const llmq::CChainLocksHandler& clhandler) LOCKS_EXCLUDED(cs_main); diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d6ec322952fc..424d117407cc 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -96,7 +96,7 @@ void TxToJSON(const CTransaction& tx, const uint256 hashBlock, const CTxMemPool txSpentInfoPtr = &txSpentInfo; } - TxToUniv(tx, uint256(), entry, true, 0, /* txundo = */ nullptr, txSpentInfoPtr); + TxToUniv(tx, uint256(), entry, true, 0, /* txundo = */ nullptr, TxVerbosity::SHOW_DETAILS, txSpentInfoPtr); bool chainLock = false; if (!hashBlock.IsNull()) { diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 1b69e45018e8..2e36aa217484 100755 --- a/test/functional/interface_rest.py +++ b/test/functional/interface_rest.py @@ -337,6 +337,15 @@ def run_test(self): if 'coinbase' not in tx['vin'][0]} assert_equal(non_coinbase_txs, set(txs)) + # Verify that the non-coinbase tx has "prevout" key set + for tx_obj in json_obj["tx"]: + for vin in tx_obj["vin"]: + if "coinbase" not in vin: + assert "prevout" in vin + assert_equal(vin["prevout"]["generated"], False) + else: + assert "prevout" not in vin + # Check the same but without tx details json_obj = self.test_rest_request(f"/block/notxdetails/{newblockhash[0]}") for tx in txs: diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 219d93fa1d14..4d5007121b57 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -485,17 +485,55 @@ def _test_getblock(self): miniwallet.send_self_transfer(fee_rate=fee_per_kb, from_node=node) blockhash = self.generate(node, 1)[0] - self.log.info("Test getblock with verbosity 1 doesn't include fee") - block = node.getblock(blockhash, 1) - assert 'fee' not in block['tx'][1] - - self.log.info('Test getblock with verbosity 2 includes expected fee') - block = node.getblock(blockhash, 2) - tx = block['tx'][1] - assert 'fee' in tx - assert_equal(tx['fee'], tx['size'] * fee_per_byte) - - self.log.info("Test getblock with verbosity 2 still works with pruned Undo data") + def assert_fee_not_in_block(verbosity): + block = node.getblock(blockhash, verbosity) + assert 'fee' not in block['tx'][1] + + def assert_fee_in_block(verbosity): + block = node.getblock(blockhash, verbosity) + tx = block['tx'][1] + assert 'fee' in tx + assert_equal(tx['fee'], tx['size'] * fee_per_byte) + + def assert_vin_contains_prevout(verbosity): + block = node.getblock(blockhash, verbosity) + tx = block["tx"][1] + total_vin = Decimal("0.00000000") + total_vout = Decimal("0.00000000") + for vin in tx["vin"]: + assert "prevout" in vin + assert_equal(set(vin["prevout"].keys()), set(("value", "height", "generated", "scriptPubKey"))) + assert_equal(vin["prevout"]["generated"], True) + total_vin += vin["prevout"]["value"] + for vout in tx["vout"]: + total_vout += vout["value"] + assert_equal(total_vin, total_vout + tx["fee"]) + + def assert_vin_does_not_contain_prevout(verbosity): + block = node.getblock(blockhash, verbosity) + tx = block["tx"][1] + if isinstance(tx, str): + # In verbosity level 1, only the transaction hashes are written + pass + else: + for vin in tx["vin"]: + assert "prevout" not in vin + + self.log.info("Test that getblock with verbosity 1 doesn't include fee") + assert_fee_not_in_block(1) + + self.log.info('Test that getblock with verbosity 2 and 3 includes expected fee') + assert_fee_in_block(2) + assert_fee_in_block(3) + + self.log.info("Test that getblock with verbosity 1 and 2 does not include prevout") + assert_vin_does_not_contain_prevout(1) + assert_vin_does_not_contain_prevout(2) + + self.log.info("Test that getblock with verbosity 3 includes prevout") + assert_vin_contains_prevout(3) + + self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data") datadir = get_datadir_path(self.options.tmpdir, 0) self.log.info("Test that getblock with invalid verbosity type returns proper error message") @@ -509,8 +547,10 @@ def move_block_file(old, new): # Move instead of deleting so we can restore chain state afterwards move_block_file('rev00000.dat', 'rev_wrong') - block = node.getblock(blockhash, 2) - assert 'fee' not in block['tx'][1] + assert_fee_not_in_block(2) + assert_fee_not_in_block(3) + assert_vin_does_not_contain_prevout(2) + assert_vin_does_not_contain_prevout(3) # Restore chain state move_block_file('rev_wrong', 'rev00000.dat') From bf28e85492a2752b1374643c332ace57abb54e4e Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 21 Oct 2021 10:06:00 +0200 Subject: [PATCH 4/9] Merge bitcoin/bitcoin#23287: test: get and decode tx with a single `gettransaction` RPC call 130ee481082d2612d452d7d69131ade935b225b5 test: get and decode tx with a single `gettransaction` RPC call (Sebastian Falbesoner) Pull request description: Rather than subsequently calling `gettransaction` and `decoderawtransaction` to get the decoded information for a specific tx-id, we can simply use the verbose version of `gettransaction`, which returns this in a 'decoded' key. I.e. ``` node.decoderawtransaction(node.gettransaction(txid)['hex']) ``` can simply be replaced by: ``` node.gettransaction(txid=txid, verbose=True)['decoded'] ``` Rationale: shorter code, shorter test logs, less RPC calls. ACKs for top commit: stratospher: tested ACK 130ee48 amadeuszpawlik: tACK 130ee481082d2612d452d7d69131ade935b225b5 lsilva01: Tested ACK 130ee48 on Ubuntu 20.04. shaavan: ACK 130ee481082d2612d452d7d69131ade935b225b5 Tree-SHA512: cf0bd26e1e21b8022fb8062857906e0706f0ee32d3277f985c461e2519405afe445ab005f5f763fb268c7b4d6e48b2d47eda7af8621b3bce67cece8dfc9bc153 --- test/functional/wallet_basic.py | 2 +- test/functional/wallet_create_tx.py | 4 ++-- test/functional/wallet_hd.py | 2 +- test/functional/wallet_importdescriptors.py | 4 +--- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 8d393a6f0886..12b4e83b7cf0 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -685,7 +685,7 @@ def run_test(self): self.generate(self.nodes[0], 1, sync_fun=self.no_op) destination = self.nodes[1].getnewaddress() txid = self.nodes[0].sendtoaddress(destination, 0.123) - tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded'] output_addresses = [vout['scriptPubKey']['address'] for vout in tx["vout"]] assert len(output_addresses) > 1 for address in output_addresses: diff --git a/test/functional/wallet_create_tx.py b/test/functional/wallet_create_tx.py index bd880bb40126..65c43cf7a74e 100755 --- a/test/functional/wallet_create_tx.py +++ b/test/functional/wallet_create_tx.py @@ -26,13 +26,13 @@ def test_anti_fee_sniping(self): self.bump_mocktime(8 * 60 * 60 + 1, update_schedulers=False) assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 200) txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) - tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded'] assert_equal(tx['locktime'], 0) self.log.info('Check that anti-fee-sniping is enabled when we mine a recent block') self.generate(self.nodes[0], 1) txid = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1) - tx = self.nodes[0].decoderawtransaction(self.nodes[0].gettransaction(txid)['hex']) + tx = self.nodes[0].gettransaction(txid=txid, verbose=True)['decoded'] assert 0 < tx['locktime'] <= 201 def test_tx_size_too_large(self): diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py index 586ad4a8c7cf..253d0f9c5911 100755 --- a/test/functional/wallet_hd.py +++ b/test/functional/wallet_hd.py @@ -133,7 +133,7 @@ def run_test(self): # send a tx and make sure its using the internal chain for the changeoutput txid = self.nodes[1].sendtoaddress(self.nodes[0].getnewaddress(), 1) - outs = self.nodes[1].decoderawtransaction(self.nodes[1].gettransaction(txid)['hex'])['vout'] + outs = self.nodes[1].gettransaction(txid=txid, verbose=True)['decoded']['vout'] keypath = "" for out in outs: if out['value'] != 1: diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index afbfd91797a1..1e4f9db7184d 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -560,9 +560,7 @@ def run_test(self): w0.sendtoaddress(addr, 10) self.generate(self.nodes[0], 6) # It is standard and would relay. - txid = multi_priv_big.sendtoaddress(w0.getnewaddress(), 10, "", "", - True) - + txid = multi_priv_big.sendtoaddress(w0.getnewaddress(), 10, "", "", True) self.log.info("Amending multisig with new private keys") self.nodes[1].createwallet(wallet_name="wmulti_priv3", descriptors=True) From 5c226da1648ca58541d49770d68486e89e2024c8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 22 Oct 2021 09:02:00 +0200 Subject: [PATCH 5/9] Merge bitcoin/bitcoin#23323: doc: Add note on deleting past-EOL release branches BACKPORT NOTE: we do not remove past-EOL branches. fa38d98aa98bcf34b5b59ff894bbb5da67355b29 doc: Add note on deleting past-EOL release branches (MarcoFalke) Pull request description: This is being done for years now, but wasn't documented. Some reasons to do it: * Backports to those branches are unlikely to be tested both on CI (since it is often fragile and broken for stale branches) and by users (since those users likely don't exist). If a user exists, they are better off backporting any fixes they need from the last still-supported branch and test them on their own infrastructure. * Community support of those branches is still possible, though this will need to be done in another project to relieve the burden on this project. * All release tags will remain, so no historic code is lost. ACKs for top commit: hebasto: ACK fa38d98aa98bcf34b5b59ff894bbb5da67355b29 fanquake: ACK fa38d98aa98bcf34b5b59ff894bbb5da67355b29 - I think this is fine as-is. Tree-SHA512: caa714af541a6902925c89cc6a896b125f61bd77e901c5d384d84b34def2ee654bdae9f3e995001154c29672f60d2b689d0ff92d345666564fd5aa321a5b3fe7 --- doc/release-process.md | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/release-process.md b/doc/release-process.md index f895b924517f..bccfb38c1ddc 100644 --- a/doc/release-process.md +++ b/doc/release-process.md @@ -6,6 +6,7 @@ Release Process Before every minor and major release: +* [ ] Review ["Needs backport" labels](https://github.com/dashpay/dash/labels?q=backport). * [ ] Update [bips.md](bips.md) to account for changes since the last release. * [ ] Update DIPs with any changes introduced by this release (see [this pull request](https://github.com/dashpay/dips/pull/142) for an example) * [ ] Update version in `configure.ac` (don't forget to set `CLIENT_VERSION_IS_RELEASE` to `true`) From cf6e96949f01eeceb55872736475d947a2fa8b2f Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Thu, 7 Oct 2021 13:21:59 +0200 Subject: [PATCH 6/9] Merge bitcoin/bitcoin#22539: Re-include RBF replacement txs in fee estimation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3b613722f6b895d7b268b3f878fddfc888381226 Add release notes for fee est with replacement txs (Antoine Poinsot) 45564065627ada5dfadff13bc32bc672a4edf152 qa: test fee estimation with replacement transactions (Antoine Poinsot) 053415b297b8665f2d2c4dce7c2c54bcc5298ef4 qa: split run_test into smaller parts (Antoine Poinsot) 06c5ce9714f7090bfb494309980f375975b7a00e Re-include RBF replacement txs in fee estimation (Antoine Poinsot) Pull request description: This effectively reverts #9519. RBF is now largely in use on the network (signaled for by around 20% of all transactions on average) and replacement logic is implemented in most end-user wallets. The rate of replaced transactions is also expected to rise as fee-bumping techniques are being developed for pre-signed transaction ("L2") protocols. ACKs for top commit: prayank23: reACK https://github.com/bitcoin/bitcoin/pull/22539/commits/3b613722f6b895d7b268b3f878fddfc888381226 Zero-1729: re-ACK 3b613722f6b895d7b268b3f878fddfc888381226 benthecarman: reACK 3b613722f6b895d7b268b3f878fddfc888381226 glozow: ACK 3b613722f6b895d7b268b3f878fddfc888381226 theStack: re-ACK 3b613722f6b895d7b268b3f878fddfc888381226 🍪 Tree-SHA512: a6146d15c80ff4ba9249314b0ef953a66a15673e61b8f98979642814f1b169b5695e330e3ee069fa9a7e4d1f8aa10e1dcb7f9aa79181cea5a4c4dbcaf5483023 Revert "Merge bitcoin/bitcoin#22539: Re-include RBF replacement txs in fee estimation" This reverts commit 7f5fa3592fb7764a92313cd94de0e8447edfa621. qa: split run_test into smaller parts Let's not have run_test get into a giant function as we add more tests Signed-off-by: Antoine Poinsot --- test/functional/feature_fee_estimation.py | 68 +++++++++++++---------- 1 file changed, 40 insertions(+), 28 deletions(-) diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py index c5d427369021..0c7212035da6 100755 --- a/test/functional/feature_fee_estimation.py +++ b/test/functional/feature_fee_estimation.py @@ -215,20 +215,16 @@ def transact_and_mine(self, numblocks, mining_node): newmem.append(utx) self.memutxo = newmem - def run_test(self): - self.log.info("This test is time consuming, please be patient") - self.log.info("Splitting inputs so we can generate tx's") - - # Start node0 - self.start_node(0) + def initial_split(self, node): + """Split two coinbase UTxOs into many small coins""" self.txouts = [] self.txouts2 = [] # Split a coinbase into two transaction puzzle outputs - split_inputs(self.nodes[0], self.nodes[0].listunspent(0), self.txouts, True) + split_inputs(node, node.listunspent(0), self.txouts, True) # Mine - while len(self.nodes[0].getrawmempool()) > 0: - self.generate(self.nodes[0], 1, sync_fun=self.no_op) + while len(node.getrawmempool()) > 0: + self.generate(node, 1, sync_fun=self.no_op) # Repeatedly split those 2 outputs, doubling twice for each rep # Use txouts to monitor the available utxo, since these won't be tracked in wallet @@ -236,27 +232,19 @@ def run_test(self): while reps < 5: # Double txouts to txouts2 while len(self.txouts) > 0: - split_inputs(self.nodes[0], self.txouts, self.txouts2) - while len(self.nodes[0].getrawmempool()) > 0: - self.generate(self.nodes[0], 1, sync_fun=self.no_op) + split_inputs(node, self.txouts, self.txouts2) + while len(node.getrawmempool()) > 0: + self.generate(node, 1, sync_fun=self.no_op) # Double txouts2 to txouts while len(self.txouts2) > 0: - split_inputs(self.nodes[0], self.txouts2, self.txouts) - while len(self.nodes[0].getrawmempool()) > 0: - self.generate(self.nodes[0], 1, sync_fun=self.no_op) + split_inputs(node, self.txouts2, self.txouts) + while len(node.getrawmempool()) > 0: + self.generate(node, 1, sync_fun=self.no_op) reps += 1 - self.log.info("Finished splitting") - - # Now we can connect the other nodes, didn't want to connect them earlier - # so the estimates would not be affected by the splitting transactions - self.start_node(1) - self.start_node(2) - self.connect_nodes(1, 0) - self.connect_nodes(0, 2) - self.connect_nodes(2, 1) - - self.sync_all() + def sanity_check_estimates_range(self): + """Populate estimation buckets, assert estimates are in a sane range and + are strictly increasing as the target decreases.""" self.fees_per_kb = [] self.memutxo = [] self.confutxo = self.txouts # Start with the set of confirmed txouts after splitting @@ -282,13 +270,37 @@ def run_test(self): self.log.info("Final estimates after emptying mempools") check_estimates(self.nodes[1], self.fees_per_kb) - # check that the effective feerate is greater than or equal to the mempoolminfee even for high mempoolminfee - self.log.info("Test fee rate estimation after restarting node with high MempoolMinFee") + def test_feerate_mempoolminfee(self): high_val = 3*self.nodes[1].estimatesmartfee(1)['feerate'] self.restart_node(1, extra_args=[f'-minrelaytxfee={high_val}']) check_estimates(self.nodes[1], self.fees_per_kb) self.stop_node(1, expected_stderr="Warning: -minrelaytxfee is set very high! The wallet will avoid paying less than the minimum relay fee.") + def run_test(self): + self.log.info("This test is time consuming, please be patient") + self.log.info("Splitting inputs so we can generate tx's") + + # Split two coinbases into many small utxos + self.start_node(0) + self.initial_split(self.nodes[0]) + self.log.info("Finished splitting") + + # Now we can connect the other nodes, didn't want to connect them earlier + # so the estimates would not be affected by the splitting transactions + self.start_node(1) + self.start_node(2) + self.connect_nodes(1, 0) + self.connect_nodes(0, 2) + self.connect_nodes(2, 1) + self.sync_all() + + self.log.info("Testing estimates with single transactions.") + self.sanity_check_estimates_range() + + # check that the effective feerate is greater than or equal to the mempoolminfee even for high mempoolminfee + self.log.info("Test fee rate estimation after restarting node with high MempoolMinFee") + self.test_feerate_mempoolminfee() + self.log.info("Testing that fee estimation is disabled in blocksonly.") self.restart_node(0, ["-blocksonly"]) assert_raises_rpc_error(-32603, "Fee estimation disabled", From 910f4c0f50b514b81b2dd1e168ec9f6fa6b7520d Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Thu, 14 Oct 2021 18:05:54 +0200 Subject: [PATCH 7/9] Merge bitcoin/bitcoin#23093: Add ability to flush keypool and always flush when upgrading non-HD to HD 6531599f422524fbbcc43816121e7536cf79d66c test: Add check that newkeypool flushes change addresses too (Samuel Dobson) 84fa19c77a2c8d0d01add2daf18b42af07c17710 Add release notes for keypool flush changes (Samuel Dobson) f9603ee4e05d7f0bd7d81f5cf24168c1aec8e5b0 Add test for flushing keypool with newkeypool (Samuel Dobson) 6f6f7bb36c492fa76aeda6513be58ca822ea1968 Make legacy wallet upgrades from non-HD to HD always flush the keypool (Samuel Dobson) 2434b1078147e71b09c4c1bf0b7ce3f6729a7713 Fix outdated keypool size default (Samuel Dobson) 22cc797ca5c1e70a4afb8e43f6917b4c9fe74e20 Add newkeypool RPC to flush the keypool (Samuel Dobson) Pull request description: This PR makes two main changes: 1) Adds a new RPC `newkeypool` which will entirely flush and refill the keypool. 2) When upgradewallet is called on old, non-HD wallets upgrading them to HD, we now always flush the keypool and generate a new one, to immediately start using the HD generated keys. This PR is motivated by a number of users with old, pre-compressed-key wallets upgrading them and being confused about why they still can't generate p2sh-segwit or bech32 addresses -- this is due to uncompressed keys remaining in the keypool post-upgrade and being illegal in these newer address formats. There is currently no easy way to flush the keypool other than to call `getnewaddress` a hundred/thousand times or an ugly hack of using a `sethdseed` call. ACKs for top commit: laanwj: re-ACK 6531599f422524fbbcc43816121e7536cf79d66c meshcollider: Added new commit 6531599f422524fbbcc43816121e7536cf79d66c to avoid invalidating previous ACKs. instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/23093/commits/6531599f422524fbbcc43816121e7536cf79d66c Tree-SHA512: 50c79c5d42dd27ab0ecdbfdc4071fdaa1b2dbb2f9195ed325b007106ff19226419ce57fe5b1539c0c24101b12f5e034bbcfb7bbb0451b766cb1071295383d774 --- doc/release-notes-23093.md | 8 ++++++++ src/wallet/rpcwallet.cpp | 30 +++++++++++++++++++++++++++- test/functional/wallet_keypool_hd.py | 14 +++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 doc/release-notes-23093.md diff --git a/doc/release-notes-23093.md b/doc/release-notes-23093.md new file mode 100644 index 000000000000..ce75de56d263 --- /dev/null +++ b/doc/release-notes-23093.md @@ -0,0 +1,8 @@ +Notable changes +=============== + +Updated RPCs +------------ + +- a new RPC `newkeypool` has been added, which will flush (entirely +clear and refill) the keypool. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 5aa51e8ca6db..9309f8dbf5b3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1747,7 +1747,7 @@ static RPCHelpMan keypoolrefill() "\nFills the keypool."+ HELP_REQUIRING_PASSPHRASE, { - {"newsize", RPCArg::Type::NUM, RPCArg::Default{int{DEFAULT_KEYPOOL_SIZE}}, "The new keypool size"}, + {"newsize", RPCArg::Type::NUM, RPCArg::DefaultHint{strprintf("%u, or as set by -keypool", DEFAULT_KEYPOOL_SIZE)}, "The new keypool size"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ @@ -1786,6 +1786,33 @@ static RPCHelpMan keypoolrefill() } +static RPCHelpMan newkeypool() +{ + return RPCHelpMan{"newkeypool", + "\nEntirely clears and refills the keypool."+ + HELP_REQUIRING_PASSPHRASE, + {}, + RPCResult{RPCResult::Type::NONE, "", ""}, + RPCExamples{ + HelpExampleCli("newkeypool", "") + + HelpExampleRpc("newkeypool", "") + }, + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ + std::shared_ptr const pwallet = GetWalletForJSONRPCRequest(request); + if (!pwallet) return NullUniValue; + + LOCK(pwallet->cs_wallet); + + LegacyScriptPubKeyMan& spk_man = EnsureLegacyScriptPubKeyMan(*pwallet, true); + spk_man.NewKeyPool(); + + return NullUniValue; +}, + }; +} + + static RPCHelpMan walletpassphrase() { return RPCHelpMan{"walletpassphrase", @@ -4642,6 +4669,7 @@ static const CRPCCommand commands[] = { "wallet", &listwallets, }, { "wallet", &loadwallet, }, { "wallet", &lockunspent, }, + { "wallet", &newkeypool, }, { "wallet", &removeprunedfunds, }, { "wallet", &rescanblockchain, }, { "wallet", &send, }, diff --git a/test/functional/wallet_keypool_hd.py b/test/functional/wallet_keypool_hd.py index 7ef2e1640103..b15857c31519 100755 --- a/test/functional/wallet_keypool_hd.py +++ b/test/functional/wallet_keypool_hd.py @@ -161,6 +161,20 @@ def run_test(self): assert_equal(wi['keypoolsize_hd_internal'], 100) assert_equal(wi['keypoolsize'], 100) + if not self.options.descriptors: + # Check that newkeypool entirely flushes the keypool + start_keypath = nodes[0].getaddressinfo(nodes[0].getnewaddress())['hdkeypath'] + start_change_keypath = nodes[0].getaddressinfo(nodes[0].getrawchangeaddress())['hdkeypath'] + # flush keypool and get new addresses + nodes[0].newkeypool() + end_keypath = nodes[0].getaddressinfo(nodes[0].getnewaddress())['hdkeypath'] + end_change_keypath = nodes[0].getaddressinfo(nodes[0].getrawchangeaddress())['hdkeypath'] + # The new keypath index should be 100 more than the old one + new_index = int(start_keypath.rsplit('/', 1)[1]) + 100 + new_change_index = int(start_change_keypath.rsplit('/', 1)[1]) + 100 + assert_equal(end_keypath, "m/44'/1'/0'/0/" + str(new_index)) + assert_equal(end_change_keypath, "m/44'/1'/0'/1/" + str(new_change_index)) + # create a blank wallet nodes[0].createwallet(wallet_name='w2', blank=True, disable_private_keys=True) w2 = nodes[0].get_wallet_rpc('w2') From 6df2fae0b4a0ced7e2f181a661bcf67236a242f4 Mon Sep 17 00:00:00 2001 From: Andrew Chow Date: Mon, 20 Dec 2021 14:50:54 -0500 Subject: [PATCH 8/9] Merge bitcoin/bitcoin#23341: RPC: Better safety with newkeypool command and wallet backups a2a92317ad4ab88cfca234f68337a7cd37897f10 rpc: Add warning to user about newkeypool command (Samuel Dobson) Pull request description: This PR prevents `newkeypool` from being run on non-HD wallets, because this would require a new backup every time, so it isn't very safe. David Harding also suggested [here](https://github.com/bitcoin/bitcoin/pull/23093#issuecomment-945350003) that the RPC help text should include a warning to the users about the interaction between newkeypool. ACKs for top commit: achow101: ACK a2a92317ad4ab88cfca234f68337a7cd37897f10 Tree-SHA512: 0aa497900f1d179764bce13ffce0bb081ba2ca354492bf2e04b21d0212e960b3ed13a386fbf65602b6b50461f4056a0285752ef707d312da28e82449cd8ea048 --- src/wallet/rpcwallet.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 9309f8dbf5b3..20efe856bf5d 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1789,7 +1789,12 @@ static RPCHelpMan keypoolrefill() static RPCHelpMan newkeypool() { return RPCHelpMan{"newkeypool", - "\nEntirely clears and refills the keypool."+ + "\nEntirely clears and refills the keypool.\n" + "WARNING: On non-HD wallets, this will require a new backup immediately, to include the new keys.\n" + "When restoring a backup of an HD wallet created before the newkeypool command is run, funds received to\n" + "new addresses may not appear automatically. They have not been lost, but the wallet may not find them.\n" + "This can be fixed by running the newkeypool command on the backup and then rescanning, so the wallet\n" + "re-generates the required keys." + HELP_REQUIRING_PASSPHRASE, {}, RPCResult{RPCResult::Type::NONE, "", ""}, From ea107ac58074e27a58e1585fa395e10017637ab0 Mon Sep 17 00:00:00 2001 From: "W. J. van der Laan" Date: Tue, 4 Jan 2022 15:07:19 +0100 Subject: [PATCH 9/9] Merge bitcoin/bitcoin#23320: rpc: Add RPC help for getblock verbosity level 3 059f88b6a97b7a3ae1f033885e40ac01f91e6d60 Add RPC help for getblock verbosity level 3 (Kiminuo) 1bdd5f63229ebf28c24a8656f486ed8a6c8b3787 Address review comments from #22918 (Kiminuo) Pull request description: This is a follow-up PR to #22918 which addresses review comments (first commit). The second commit adds missing RPC help for verbosity level 3. ACKs for top commit: pg156: ACK https://github.com/bitcoin/bitcoin/pull/23320/commits/059f88b6a97b7a3ae1f033885e40ac01f91e6d60 laanwj: re-ACK 059f88b6a97b7a3ae1f033885e40ac01f91e6d60 Tree-SHA512: f27d53ac34b93a304ef5668701ed2b5c986a926bc8ad0df4de89695fc9e1df26acb008611451319ea897658acd9c56c6a0555d60359960c9cd28238ebefa2d50 --- doc/release-notes-22918.md | 2 +- src/core_write.cpp | 27 +++++++++++---------------- src/rpc/blockchain.cpp | 34 +++++++++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 18 deletions(-) diff --git a/doc/release-notes-22918.md b/doc/release-notes-22918.md index 5220513cd890..8f5a7c37ce07 100644 --- a/doc/release-notes-22918.md +++ b/doc/release-notes-22918.md @@ -1,6 +1,6 @@ ## Updated RPCs -- The `getblock` RPC command now supports verbose level 3 containing transaction inputs +- The `getblock` RPC command now supports verbosity level 3 containing transaction inputs `prevout` information. The existing `/rest/block/` REST endpoint is modified to contain this information too. Every `vin` field will contain an additional `prevout` subfield describing the spent output. `prevout` contains the following keys: diff --git a/src/core_write.cpp b/src/core_write.cpp index 19c646fa6ec6..b3ac7313d6bf 100644 --- a/src/core_write.cpp +++ b/src/core_write.cpp @@ -226,22 +226,17 @@ void TxToUniv(const CTransaction& tx, const uint256& hashBlock, UniValue& entry, const CTxOut& prev_txout = prev_coin.out; amt_total_in += prev_txout.nValue; - switch (verbosity) { - case TxVerbosity::SHOW_TXID: - case TxVerbosity::SHOW_DETAILS: - break; - - case TxVerbosity::SHOW_DETAILS_AND_PREVOUT: - UniValue o_script_pub_key(UniValue::VOBJ); - ScriptPubKeyToUniv(prev_txout.scriptPubKey, o_script_pub_key, /* includeHex */ true, /*include_address=*/true); - - UniValue p(UniValue::VOBJ); - p.pushKV("generated", bool(prev_coin.fCoinBase)); - p.pushKV("height", uint64_t(prev_coin.nHeight)); - p.pushKV("value", ValueFromAmount(prev_txout.nValue)); - p.pushKV("scriptPubKey", o_script_pub_key); - in.pushKV("prevout", p); - break; + + if (verbosity == TxVerbosity::SHOW_DETAILS_AND_PREVOUT) { + UniValue o_script_pub_key(UniValue::VOBJ); + ScriptPubKeyToUniv(prev_txout.scriptPubKey, o_script_pub_key, /*include_hex=*/ true, /*include_address=*/true); + + UniValue p(UniValue::VOBJ); + p.pushKV("generated", bool(prev_coin.fCoinBase)); + p.pushKV("height", uint64_t(prev_coin.nHeight)); + p.pushKV("value", ValueFromAmount(prev_txout.nValue)); + p.pushKV("scriptPubKey", o_script_pub_key); + in.pushKV("prevout", p); } } in.pushKV("sequence", (int64_t)txin.nSequence); diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index e9ce8ce4c1c6..8bee62c8489b 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -183,6 +183,7 @@ UniValue blockToJSON(BlockManager& blockman, const CBlock& block, const CBlockIn objTx.pushKV("instantlock_internal", fLocked); txs.push_back(objTx); } + break; } result.pushKV("tx", txs); @@ -881,7 +882,7 @@ static RPCHelpMan getblock() "If verbosity is 3, returns an Object with information about block and information about each transaction, including prevout information for inputs (only for unpruned blocks in the current best chain).\n", { {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "The block hash"}, - {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a json object, and 2 for json object with transaction data"}, + {"verbosity|verbose", RPCArg::Type::NUM, RPCArg::Default{1}, "0 for hex-encoded data, 1 for a JSON object, 2 for JSON object with transaction data, and 3 for JSON object with transaction data including prevout information for inputs"}, }, { RPCResult{"for verbosity = 0", @@ -929,6 +930,37 @@ static RPCHelpMan getblock() }}, }}, }}, + RPCResult{"for verbosity = 3", + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::ELISION, "", "Same output as verbosity = 2"}, + {RPCResult::Type::ARR, "tx", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::ARR, "vin", "", + { + {RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::ELISION, "", "The same output as verbosity = 2"}, + {RPCResult::Type::OBJ, "prevout", "(Only if undo information is available)", + { + {RPCResult::Type::BOOL, "generated", "Coinbase or not"}, + {RPCResult::Type::NUM, "height", "The height of the prevout"}, + {RPCResult::Type::NUM, "value", "The value in " + CURRENCY_UNIT}, + {RPCResult::Type::OBJ, "scriptPubKey", "", + { + {RPCResult::Type::STR, "asm", "The asm"}, + {RPCResult::Type::STR, "hex", "The hex"}, + {RPCResult::Type::STR, "address", /*optional=*/ true, "The Dash address (only if a well-defined address exists)"}, + {RPCResult::Type::STR, "type", "The type, eg 'pubkeyhash'"}, + }}, + }}, + }}, + }}, + }}, + }}, + }}, }, RPCExamples{ HelpExampleCli("getblock", "\"00000000000fd08c2fb661d2fcb0d49abb3a91e5f27082ce64feed3b4dede2e2\"")