From b3f0ec5694728e7ae72f150b290366fe9f096ddb Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 28 Mar 2019 10:56:20 -0400 Subject: [PATCH 1/7] Merge #15616: rpc: Clarify decodescript RPCResult doc fa926ec24f rpc: Mention all output types in decodescript doc (MarcoFalke) fa3caa1666 rpc: decodescript use IsValidNumArgs over hardcoded check (MarcoFalke) faad33ff15 rpc: Clarify decodescript RPCResult doc (MarcoFalke) Pull request description: * Remove `"hex"` from the decodescript RPCResult doc * Add `"segwit`" to the doc Follow up to a6099ef319a73e2255dca77065600abb22c4f5f8 and 4f933b3d23010d3b03998460290faed97cd6f236 ACKs for commit fa926e: ryanofsky: utACK fa926ec24fb3d07de32bd8f67a297f9e4f0822a6. Only change since last review is listing possible output types in the help string using a new `GetAllOutputTypes` function Tree-SHA512: e6ecc563d04769942567118d50188467bf64ceb276ba6268928d469e8f06621f2ca1ae1e555d3daa6ec22a615ee259bb31c4141c19818d0f53fb6c529b18381b --- src/rpc/rawtransaction.cpp | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index e1630ba595..79dc89b6b9 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -530,33 +530,44 @@ static UniValue decoderawtransaction(const JSONRPCRequest& request) return result; } +static std::string GetAllOutputTypes() +{ + std::string ret; + for (int i = TX_NONSTANDARD; i <= TX_NULL_DATA; ++i) { + if (i != TX_NONSTANDARD) ret += ", "; + ret += GetTxnOutputType(static_cast(i)); + } + return ret; +} + static UniValue decodescript(const JSONRPCRequest& request) { - if (request.fHelp || request.params.size() != 1) - throw std::runtime_error( - RPCHelpMan{"decodescript", + const RPCHelpMan help{"decodescript", "\nDecode a hex-encoded script.\n", { {"hexstring", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hex-encoded script"}, }, RPCResult{ "{\n" - " \"asm\":\"asm\", (string) Script public key\n" - " \"hex\":\"hex\", (string) hex-encoded public key\n" - " \"type\":\"type\", (string) The output type\n" - " \"reqSigs\": n, (numeric) The required signatures\n" - " \"addresses\": [ (json array of string)\n" - " \"address\" (string) dash address\n" + " \"asm\":\"asm\", (string) Script public key\n" + " \"type\":\"type\", (string) The output type (e.g. "+GetAllOutputTypes()+")\n" + " \"reqSigs\": n, (numeric) The required signatures\n" + " \"addresses\": [ (json array of string)\n" + " \"address\" (string) dash address\n" " ,...\n" " ],\n" - " \"p2sh\",\"address\" (string) address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH).\n" + " \"p2sh\":\"str\" (string) address of P2SH script wrapping this redeem script (not returned if the script is already a P2SH).\n" "}\n" }, RPCExamples{ HelpExampleCli("decodescript", "\"hexstring\"") + HelpExampleRpc("decodescript", "\"hexstring\"") }, - }.ToString()); + }; + + if (request.fHelp || !help.IsValidNumArgs(request.params.size())) { + throw std::runtime_error(help.ToString()); + } RPCTypeCheck(request.params, {UniValue::VSTR}); @@ -568,7 +579,7 @@ static UniValue decodescript(const JSONRPCRequest& request) } else { // Empty scripts are valid } - ScriptPubKeyToUniv(script, r, false); + ScriptPubKeyToUniv(script, r, /* fIncludeHex */ false); UniValue type; From 7975686b0e1480184822b1e37c43d38186d8ca7f Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Mon, 15 Apr 2019 08:37:32 +1200 Subject: [PATCH 2/7] Merge #15557: Enhance `bumpfee` to include inputs when targeting a feerate 184f8785f wallet_bumpfee.py: add test for change key preservation (Gregory Sanders) d08becff8 add functional tests for feerate bumpfee with adding inputs (Gregory Sanders) 0ea47ba7b generalize bumpfee to add inputs when needed (Gregory Sanders) Pull request description: When targeting a feerate using `bumpfee`, call a new function that directly uses `CWallet::CreateTransaction` and coin control to get the desired result. This allows us to get a superset of previous behavior, with an arbitrary RBF bump of a transaction provided it passes the preconditional checks and spare confirmed utxos are available. Note(s): 0) The coin selection will use knapsack solver for the residual selection. 1) This functionality, just like knapsack coin selection in general, will hoover up negative-value inputs when given the chance. 2) Newly added inputs must be confirmed due to current Core policy. See error: `replacement-adds-unconfirmed` 3) Supporting this with `totalFee` is difficult since the "minimum total fee" option in `CreateTransaction` logic was (rightly)taken out in #10390 . ACKs for commit 184f87: jnewbery: utACK 184f8785f710d58d9ef82e611591c9cbff5ab89d Tree-SHA512: fb6542bdfb2c6010e328ec475cf9dcbff4eb2b1a1b27f78010214534908987a5635797196fa05edddffcbcf2987335872dc644a99261886d5cbb34a8f262ad3e --- src/wallet/coincontrol.h | 2 ++ src/wallet/wallet.cpp | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/coincontrol.h b/src/wallet/coincontrol.h index 4f7669ae98..fc87645373 100644 --- a/src/wallet/coincontrol.h +++ b/src/wallet/coincontrol.h @@ -49,6 +49,8 @@ public: bool m_avoid_partial_spends; //! Fee estimation mode to control arguments to estimateSmartFee FeeEstimateMode m_fee_mode; + //! Minimum chain depth value for coin availability + int m_min_depth{0}; //! Controls which types of coins are allowed to be used (default: ALL_COINS) CoinType nCoinType; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index c2af686996..78636d3784 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3627,7 +3627,7 @@ bool CWallet::CreateTransaction(interfaces::Chain::Lock& locked_chain, const std { CAmount nAmountAvailable{0}; std::vector vAvailableCoins; - AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control); + AvailableCoins(*locked_chain, vAvailableCoins, true, &coin_control, 1, MAX_MONEY, MAX_MONEY, 0, coin_control.m_min_depth); CoinSelectionParams coin_selection_params; // Parameters for coin selection, init with dummy coin_selection_params.use_bnb = false; // never use BnB From f17497a7d7e604249d82a49c2ec5783b659f94ca Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 19 Apr 2019 11:59:56 -0400 Subject: [PATCH 3/7] Merge #15670: refactor: combine Chain::findFirstBlockWithTime/findFirstBlockWithTimeAndHeight 765c0b364d refactor: combine Chain::findFirstBlockWithTime/findFirstBlockWithTimeAndHeight (Antoine Riard) Pull request description: As suggested in #14711, pass height to CChain::FindEarliestAtLeast to simplify Chain interface by combining findFirstBlockWithTime and findFirstBlockWithTimeAndHeight into one ACKs for commit 765c0b: jnewbery: utACK 765c0b364d41e9a251c3f88cbe203645854fd790. Nice work @ariard! ryanofsky: utACK 765c0b364d41e9a251c3f88cbe203645854fd790. Looks good, thanks for implementing the suggestion! Tree-SHA512: 63f98252a93da95f08c0b6325ea98f717aa9ae4036d17eaa6edbec68e5ddd65672d66a6af267b80c36311fffa9b415a47308e95ea7718b300b685e23d4e9e6ec --- src/chain.cpp | 7 ++++--- src/chain.h | 4 ++-- src/interfaces/chain.cpp | 18 ++-------------- src/interfaces/chain.h | 17 ++++----------- src/rpc/blockchain.cpp | 2 +- src/test/skiplist_tests.cpp | 42 ++++++++++++++++++++++++------------- src/wallet/wallet.cpp | 4 ++-- 7 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/chain.cpp b/src/chain.cpp index c64d99370c..685629f91a 100644 --- a/src/chain.cpp +++ b/src/chain.cpp @@ -59,10 +59,11 @@ const CBlockIndex *CChain::FindFork(const CBlockIndex *pindex) const { return pindex; } -CBlockIndex* CChain::FindEarliestAtLeast(int64_t nTime) const +CBlockIndex* CChain::FindEarliestAtLeast(int64_t nTime, int height) const { - std::vector::const_iterator lower = std::lower_bound(vChain.begin(), vChain.end(), nTime, - [](CBlockIndex* pBlock, const int64_t& time) -> bool { return pBlock->GetBlockTimeMax() < time; }); + std::pair blockparams = std::make_pair(nTime, height); + std::vector::const_iterator lower = std::lower_bound(vChain.begin(), vChain.end(), blockparams, + [](CBlockIndex* pBlock, const std::pair& blockparams) -> bool { return pBlock->GetBlockTimeMax() < blockparams.first || pBlock->nHeight < blockparams.second; }); return (lower == vChain.end() ? nullptr : *lower); } diff --git a/src/chain.h b/src/chain.h index 7e3c6ce68d..559183468a 100644 --- a/src/chain.h +++ b/src/chain.h @@ -430,8 +430,8 @@ public: /** Find the last common block between this chain and a block index entry. */ const CBlockIndex *FindFork(const CBlockIndex *pindex) const; - /** Find the earliest block with timestamp equal or greater than the given. */ - CBlockIndex* FindEarliestAtLeast(int64_t nTime) const; + /** Find the earliest block with timestamp equal or greater than the given time and height equal or greater than the given height. */ + CBlockIndex* FindEarliestAtLeast(int64_t nTime, int height) const; }; #endif // BITCOIN_CHAIN_H diff --git a/src/interfaces/chain.cpp b/src/interfaces/chain.cpp index 4ed1afadce..9d37c098b6 100644 --- a/src/interfaces/chain.cpp +++ b/src/interfaces/chain.cpp @@ -89,30 +89,16 @@ class LockImpl : public Chain::Lock, public UniqueLock CBlockIndex* block = ::ChainActive()[height]; return block && ((block->nStatus & BLOCK_HAVE_DATA) != 0) && block->nTx > 0; } - Optional findFirstBlockWithTime(int64_t time, uint256* hash) override + Optional findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) override { LockAnnotation lock(::cs_main); - CBlockIndex* block = ::ChainActive().FindEarliestAtLeast(time); + CBlockIndex* block = ::ChainActive().FindEarliestAtLeast(time, height); if (block) { if (hash) *hash = block->GetBlockHash(); return block->nHeight; } return nullopt; } - Optional findFirstBlockWithTimeAndHeight(int64_t time, int height) override - { - // TODO: Could update CChain::FindEarliestAtLeast() to take a height - // parameter and use it with std::lower_bound() to make this - // implementation more efficient and allow combining - // findFirstBlockWithTime and findFirstBlockWithTimeAndHeight into one - // method. - for (CBlockIndex* block = ::ChainActive()[height]; block; block = ::ChainActive().Next(block)) { - if (block->GetBlockTime() >= time) { - return block->nHeight; - } - } - return nullopt; - } Optional findPruned(int start_height, Optional stop_height) override { LockAnnotation lock(::cs_main); diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 10f1b3064d..12e0ba1c4e 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -99,21 +99,12 @@ public: //! pruned), and contains transactions. virtual bool haveBlockOnDisk(int height) = 0; - //! Return height of the first block in the chain with timestamp equal - //! or greater than the given time, or nullopt if there is no block with - //! a high enough timestamp. Also return the block hash as an optional - //! output parameter (to avoid the cost of a second lookup in case this - //! information is needed.) - virtual Optional findFirstBlockWithTime(int64_t time, uint256* hash) = 0; - //! Return height of the first block in the chain with timestamp equal //! or greater than the given time and height equal or greater than the - //! given height, or nullopt if there is no such block. - //! - //! Calling this with height 0 is equivalent to calling - //! findFirstBlockWithTime, but less efficient because it requires a - //! linear instead of a binary search. - virtual Optional findFirstBlockWithTimeAndHeight(int64_t time, int height) = 0; + //! given height, or nullopt if there is no block with a high enough + //! timestamp and height. Also return the block hash as an optional output parameter + //! (to avoid the cost of a second lookup in case this information is needed.) + virtual Optional findFirstBlockWithTimeAndHeight(int64_t time, int height, uint256* hash) = 0; //! Return height of last block in the specified range which is pruned, or //! nullopt if no block in the range is pruned. Range is inclusive. diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 6c6f07ae2e..579f00961c 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1203,7 +1203,7 @@ static UniValue pruneblockchain(const JSONRPCRequest& request) // too low to be a block time (corresponds to timestamp from Sep 2001). if (heightParam > 1000000000) { // Add a 2 hour buffer to include blocks which might have had old timestamps - CBlockIndex* pindex = ::ChainActive().FindEarliestAtLeast(heightParam - TIMESTAMP_WINDOW); + CBlockIndex* pindex = ::ChainActive().FindEarliestAtLeast(heightParam - TIMESTAMP_WINDOW, 0); if (!pindex) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find block with at least the specified timestamp."); } diff --git a/src/test/skiplist_tests.cpp b/src/test/skiplist_tests.cpp index dd46af66ea..af86ee18b0 100644 --- a/src/test/skiplist_tests.cpp +++ b/src/test/skiplist_tests.cpp @@ -135,7 +135,7 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_test) // Pick a random element in vBlocksMain. int r = InsecureRandRange(vBlocksMain.size()); int64_t test_time = vBlocksMain[r].nTime; - CBlockIndex *ret = chain.FindEarliestAtLeast(test_time); + CBlockIndex* ret = chain.FindEarliestAtLeast(test_time, 0); BOOST_CHECK(ret->nTimeMax >= test_time); BOOST_CHECK((ret->pprev==nullptr) || ret->pprev->nTimeMax < test_time); BOOST_CHECK(vBlocksMain[r].GetAncestor(ret->nHeight) == ret); @@ -157,22 +157,34 @@ BOOST_AUTO_TEST_CASE(findearliestatleast_edge_test) CChain chain; chain.SetTip(&blocks.back()); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(50)->nHeight, 0); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(100)->nHeight, 0); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(150)->nHeight, 3); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(200)->nHeight, 3); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(250)->nHeight, 6); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(300)->nHeight, 6); - BOOST_CHECK(!chain.FindEarliestAtLeast(350)); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(50, 0)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(100, 0)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(150, 0)->nHeight, 3); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(200, 0)->nHeight, 3); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(250, 0)->nHeight, 6); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(300, 0)->nHeight, 6); + BOOST_CHECK(!chain.FindEarliestAtLeast(350, 0)); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0)->nHeight, 0); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-1)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 0)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-1, 0)->nHeight, 0); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(std::numeric_limits::min())->nHeight, 0); - BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-int64_t(std::numeric_limits::max()) - 1)->nHeight, 0); - BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits::max())); - BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits::max())); - BOOST_CHECK(!chain.FindEarliestAtLeast(int64_t(std::numeric_limits::max()) + 1)); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(std::numeric_limits::min(), 0)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(-int64_t(std::numeric_limits::max()) - 1, 0)->nHeight, 0); + BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits::max(), 0)); + BOOST_CHECK(!chain.FindEarliestAtLeast(std::numeric_limits::max(), 0)); + BOOST_CHECK(!chain.FindEarliestAtLeast(int64_t(std::numeric_limits::max()) + 1, 0)); + + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, -1)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 0)->nHeight, 0); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 3)->nHeight, 3); + BOOST_CHECK_EQUAL(chain.FindEarliestAtLeast(0, 8)->nHeight, 8); + BOOST_CHECK(!chain.FindEarliestAtLeast(0, 9)); + + CBlockIndex* ret1 = chain.FindEarliestAtLeast(100, 2); + BOOST_CHECK(ret1->nTimeMax >= 100 && ret1->nHeight == 2); + BOOST_CHECK(!chain.FindEarliestAtLeast(300, 9)); + CBlockIndex* ret2 = chain.FindEarliestAtLeast(200, 4); + BOOST_CHECK(ret2->nTimeMax >= 200 && ret2->nHeight == 4); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 78636d3784..a809f69075 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2179,7 +2179,7 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r uint256 start_block; { auto locked_chain = chain().lock(); - const Optional start_height = locked_chain->findFirstBlockWithTime(startTime - TIMESTAMP_WINDOW, &start_block); + const Optional start_height = locked_chain->findFirstBlockWithTimeAndHeight(startTime - TIMESTAMP_WINDOW, 0, &start_block); const Optional tip_height = locked_chain->getHeight(); WalletLogPrintf("%s: Rescanning last %i blocks\n", __func__, tip_height && start_height ? *tip_height - *start_height + 1 : 0); } @@ -5265,7 +5265,7 @@ std::shared_ptr CWallet::CreateWalletFromFile(interfaces::Chain& chain, // unless a full rescan was requested if (gArgs.GetArg("-rescan", 0) != 2) { if (walletInstance->nTimeFirstKey) { - if (Optional first_block = locked_chain->findFirstBlockWithTimeAndHeight(walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW, rescan_height)) { + if (Optional first_block = locked_chain->findFirstBlockWithTimeAndHeight(walletInstance->nTimeFirstKey - TIMESTAMP_WINDOW, rescan_height, nullptr)) { rescan_height = *first_block; } } From baf262b02260dd61f0db57d1a60ba9217acb4e3a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 29 Apr 2019 08:48:36 -0400 Subject: [PATCH 4/7] Merge #12051: add missing debian contrib file to tarball 5d7ce74ab3 add missing debian contrib files to tarball (Peter Wagner) Pull request description: the current release is missing the debian contrib folder, add it ACKs for commit 5d7ce7: Tree-SHA512: 9d38c9ec0cc13171582c0bde57a2f69b22026a91f353e20da556cb63a4cfbba68b2465c9c62eaa98df50a65d971cc4411ffee519824b34068772ae8ddedb7d4c --- Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile.am b/Makefile.am index a1e2ce47f9..61aa4d3e61 100644 --- a/Makefile.am +++ b/Makefile.am @@ -47,9 +47,9 @@ OSX_PLIST=$(top_builddir)/share/qt/Info.plist #not installed OSX_QT_TRANSLATIONS = ar,bg,ca,cs,da,de,es,fa,fi,fr,gd,gl,he,hu,it,ja,ko,lt,lv,pl,pt,ru,sk,sl,sv,uk,zh_CN,zh_TW DIST_CONTRIB = \ + $(top_srcdir)/contrib/debian/copyright \ $(top_srcdir)/contrib/linearize/linearize-data.py \ $(top_srcdir)/contrib/linearize/linearize-hashes.py - DIST_SHARE = \ $(top_srcdir)/share/genbuild.sh \ $(top_srcdir)/share/rpcauth From 744ebbed3a0f6db743c2bd8cc83657f47210d1f2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 29 Apr 2019 09:10:34 -0400 Subject: [PATCH 5/7] Merge #15877: doc: Fix -dustrelayfee= argument docs grammar 64491cb376 doc: Fix -dustrelayfee= argument docs grammar (keepkeyjon) Pull request description: ACKs for commit 64491c: fanquake: utACK 64491cb Tree-SHA512: 562180e5bb065c71cda89555afd1cd5a54a98b058ab9006af3a6437fbbde46c7f3930b3fe98900bbb18f329057e00da81bc8290bdf6160d7eccc97d255b30e4b --- src/init.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index 8fbad41b5f..b0cccefdf9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -702,7 +702,7 @@ void SetupServerArgs() gArgs.AddArg("-platform-user=", "Set the username for the \"platform user\", a restricted user intended to be used by Dash Platform, to the specified username.", ArgsManager::ALLOW_ANY, OptionsCategory::MASTERNODE); gArgs.AddArg("-acceptnonstdtxn", strprintf("Relay and mine \"non-standard\" transactions (%sdefault: %u)", "testnet/regtest only; ", !testnetChainParams->RequireStandard()), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); - gArgs.AddArg("-dustrelayfee=", strprintf("Fee rate (in %s/kB) used to defined dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); + gArgs.AddArg("-dustrelayfee=", strprintf("Fee rate (in %s/kB) used to define dust, the value of an output such that it will cost more than its value in fees at this fee rate to spend it. (default: %s)", CURRENCY_UNIT, FormatMoney(DUST_RELAY_TX_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); gArgs.AddArg("-incrementalrelayfee=", strprintf("Fee rate (in %s/kB) used to define cost of relay, used for mempool limiting and BIP 125 replacement. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_INCREMENTAL_RELAY_FEE)), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::NODE_RELAY); gArgs.AddArg("-bytespersigop", strprintf("Equivalent bytes per sigop in transactions for relay and mining (default: %u)", DEFAULT_BYTES_PER_SIGOP), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); gArgs.AddArg("-datacarrier", strprintf("Relay and mine data carrier transactions (default: %u)", DEFAULT_ACCEPT_DATACARRIER), ArgsManager::ALLOW_ANY, OptionsCategory::NODE_RELAY); From e54d4af104ad4d1e903c7d33d7ac6446dc1b2371 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 14 Jan 2019 18:05:26 +0100 Subject: [PATCH 6/7] Merge #14982: rpc: Add getrpcinfo command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit a0ac15459a0df598e1ee1fd36a3899a129cecaeb doc: Add getrpcinfo release notes (João Barbosa) 251a91c1bf245b3674c2612149382a0f1e18dc98 qa: Add tests for getrpcinfo (João Barbosa) d0730f5ce475e5a84da7c61fe79bcd6ed24d693e rpc: Add getrpcinfo command (João Barbosa) 068a8fc05f8dbec198bdc3fe46f955d8a5255303 rpc: Track active commands (João Barbosa) bf4383277d6761cc5b7a91975752c08df829af72 rpc: Remove unused PreCommand signal (João Barbosa) Pull request description: The new `getrpcinfo` command exposes details of the RPC interface. The details can be configuration properties or runtime values/stats. This can be particular useful to coordinate concurrent functional tests (see #14958 from where this was extracted). Tree-SHA512: 7292cb6087f4c429973d991aa2b53ffa1327d5a213df7d6ba5fc69b01b2e1a411f6d1609fed9234896293317dab05f65064da48b8f2b4a998eba532591d31882 --- doc/release-notes-14982.md | 5 +++ src/rpc/server.cpp | 59 ++++++++++++++++++++++++++++++++ test/functional/interface_rpc.py | 13 ++++++- 3 files changed, 76 insertions(+), 1 deletion(-) create mode 100644 doc/release-notes-14982.md diff --git a/doc/release-notes-14982.md b/doc/release-notes-14982.md new file mode 100644 index 0000000000..3f0bf8aacd --- /dev/null +++ b/doc/release-notes-14982.md @@ -0,0 +1,5 @@ +New RPCs +-------- + +- The RPC `getrpcinfo` returns runtime details of the RPC server. At the moment + it returns the active commands and the corresponding execution time. diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index d01352a475..4c30aa469d 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -35,6 +35,35 @@ static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& req // Any commands submitted by this user will have their commands filtered based on the mapPlatformRestrictions static const std::string defaultPlatformUser = "platform-user"; +struct RPCCommandExecutionInfo +{ + std::string method; + int64_t start; +}; + +struct RPCServerInfo +{ + Mutex mutex; + std::list active_commands GUARDED_BY(mutex); +}; + +static RPCServerInfo g_rpc_server_info; + +struct RPCCommandExecution +{ + std::list::iterator it; + explicit RPCCommandExecution(const std::string& method) + { + LOCK(g_rpc_server_info.mutex); + it = g_rpc_server_info.active_commands.insert(g_rpc_server_info.active_commands.cend(), {method, GetTimeMicros()}); + } + ~RPCCommandExecution() + { + LOCK(g_rpc_server_info.mutex); + g_rpc_server_info.active_commands.erase(it); + } +}; + static struct CRPCSignals { boost::signals2::signal Started; @@ -191,11 +220,40 @@ static UniValue uptime(const JSONRPCRequest& jsonRequest) return GetTime() - GetStartupTime(); } +static UniValue getrpcinfo(const JSONRPCRequest& request) +{ + if (request.fHelp || request.params.size() > 0) { + throw std::runtime_error( + RPCHelpMan{"getrpcinfo", + "\nReturns details of the RPC server.\n", + {}, + RPCResults{}, + RPCExamples{""}, + }.ToString() + ); + } + + LOCK(g_rpc_server_info.mutex); + UniValue active_commands(UniValue::VARR); + for (const RPCCommandExecutionInfo& info : g_rpc_server_info.active_commands) { + UniValue entry(UniValue::VOBJ); + entry.pushKV("method", info.method); + entry.pushKV("duration", GetTimeMicros() - info.start); + active_commands.push_back(entry); + } + + UniValue result(UniValue::VOBJ); + result.pushKV("active_commands", active_commands); + + return result; +} + // clang-format off static const CRPCCommand vRPCCommands[] = { // category name actor (function) argNames // --------------------- ------------------------ ----------------------- ---------- /* Overall control/query calls */ + { "control", "getrpcinfo", &getrpcinfo, {} }, { "control", "help", &help, {"command","subcommand"} }, { "control", "stop", &stop, {"wait"} }, { "control", "uptime", &uptime, {} }, @@ -495,6 +553,7 @@ static bool ExecuteCommand(const CRPCCommand& command, const JSONRPCRequest& req try { + RPCCommandExecution execution(request.strMethod); // Execute, convert arguments to array if necessary if (request.params.isObject()) { return command.actor(transformNamedArguments(request, command.argNames), result, last_handler); diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index b50eb8908c..c7af0fc8d8 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -6,7 +6,7 @@ from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal +from test_framework.util import assert_equal, assert_greater_than_or_equal def expect_http_status(expected_http_status, expected_rpc_code, fcn, *args): @@ -22,6 +22,16 @@ class RPCInterfaceTest(BitcoinTestFramework): self.num_nodes = 1 self.setup_clean_chain = True + def test_getrpcinfo(self): + self.log.info("Testing getrpcinfo...") + + info = self.nodes[0].getrpcinfo() + assert_equal(len(info['active_commands']), 1) + + command = info['active_commands'][0] + assert_equal(command['method'], 'getrpcinfo') + assert_greater_than_or_equal(command['duration'], 0) + def test_batch_request(self): self.log.info("Testing basic JSON-RPC batch request...") @@ -55,6 +65,7 @@ class RPCInterfaceTest(BitcoinTestFramework): expect_http_status(500, -8, self.nodes[0].getblockhash, 42) def run_test(self): + self.test_getrpcinfo() self.test_batch_request() self.test_http_status_codes() From 8de8cf33d1d079377aaf1c447d318e755257b4d5 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 16 Jan 2019 13:04:00 +0100 Subject: [PATCH 7/7] Merge #14958: qa: Remove race between connecting and shutdown on separate connections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 4412a59bfe8228698e5b5bbe8bb21c8e8a70d357 qa: Remove race between connecting and shutdown on separate connections (João Barbosa) Pull request description: Fixes the error https://github.com/bitcoin/bitcoin/pull/14670#issuecomment-447255352 reported by @ken2812221. There is a race between RPC stop and another concurrent call in the test framework. The connection must be established and the command `waitfornewblock` running before calling `stop`. See also https://github.com/bitcoin/bitcoin/pull/14670#issuecomment-447304513. Tree-SHA512: 77feb8628d3b9c025ec0cf83565d4d6680cad4fb182fc93a65df8b573f3e799ba4c44e06d9001dd8a375ca0b1ee17f10e66c3902b6256d0ae2acbc64539185d7 --- test/functional/feature_shutdown.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/functional/feature_shutdown.py b/test/functional/feature_shutdown.py index 5bf25d81b6..15b67b55fe 100755 --- a/test/functional/feature_shutdown.py +++ b/test/functional/feature_shutdown.py @@ -5,7 +5,7 @@ """Test dashd shutdown.""" from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, get_rpc_proxy +from test_framework.util import assert_equal, get_rpc_proxy, wait_until from threading import Thread def test_long_call(node): @@ -20,8 +20,14 @@ class ShutdownTest(BitcoinTestFramework): def run_test(self): node = get_rpc_proxy(self.nodes[0].url, 1, timeout=600, coveragedir=self.nodes[0].coverage_dir) + # Force connection establishment by executing a dummy command. + node.getblockcount() Thread(target=test_long_call, args=(node,)).start() - # wait 1 second to ensure event loop waits for current connections to close + # Wait until the server is executing the above `waitfornewblock`. + wait_until(lambda: len(self.nodes[0].getrpcinfo()['active_commands']) == 2) + # Wait 1 second after requesting shutdown but not before the `stop` call + # finishes. This is to ensure event loop waits for current connections + # to close. self.stop_node(0, wait=1000) if __name__ == '__main__':