From 8eb04b2f1f1f061bcffbb9057b068511c556ced3 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Tue, 21 Feb 2023 14:39:40 +0700 Subject: [PATCH 01/13] Merge #14092: tests: Dry run bench_bitcoin as part "make check" to allow for quick identification of assertion/sanitizer failures in benchmarking code Follow-up this earlier merged backport - removed unused variable RUN_BENCH --- ci/test/00_setup_env_native_tsan.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh index 87ac6100ab..cef8acfd5a 100755 --- a/ci/test/00_setup_env_native_tsan.sh +++ b/ci/test/00_setup_env_native_tsan.sh @@ -9,7 +9,6 @@ export LC_ALL=C.UTF-8 export PACKAGES="clang llvm python3-zmq qtbase5-dev qttools5-dev-tools libevent-dev bsdmainutils libboost-filesystem-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libzmq3-dev libqrencode-dev" export DEP_OPTS="NO_UPNP=1 DEBUG=1" export TEST_RUNNER_EXTRA="--extended --exclude feature_pruning,feature_dbcrash,wallet_multiwallet.py" # Temporarily suppress ASan heap-use-after-free (see issue #14163) -export RUN_BENCH=true export GOAL="install" export BITCOIN_CONFIG="--enable-zmq --enable-reduce-exports --enable-crash-hooks --with-sanitizers=thread" export CPPFLAGS="-DDEBUG_LOCKORDER -DENABLE_DASH_DEBUG -DARENA_DEBUG" From 2cae37806be8f626912909d4483e85d791cfc67c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 24 Sep 2018 14:54:10 -0400 Subject: [PATCH 02/13] Merge #13424: Consistently validate txid / blockhash length and encoding in rpc calls 5eb20f81d9 Consistently use ParseHashV to validate hash inputs in rpc (Ben Woosley) Pull request description: ParseHashV validates the length and encoding of the string and throws an informative RPC error on failure, which is as good or better than these alternative calls. Note I switched ParseHashV to check string length first, because IsHex tests that the length is even, and an error like: "must be of length 64 (not 63, for X)" is much more informative than "must be hexadecimal string (not X)" in that case. Split from #13420 Tree-SHA512: f0786b41c0d7793ff76e4b2bb35547873070bbf7561d510029e8edb93f59176277efcd4d183b3185532ea69fc0bbbf3dbe9e19362e8017007ae9d51266cd78ae --- src/rpc/blockchain.cpp | 42 +++++++------------ src/rpc/evo.cpp | 4 +- src/rpc/governance.cpp | 22 +++++----- src/rpc/masternode.cpp | 2 +- src/rpc/mining.cpp | 4 +- src/rpc/quorums.cpp | 20 ++++----- src/rpc/rawtransaction.cpp | 7 +--- src/rpc/util.cpp | 12 ++---- src/wallet/rpcdump.cpp | 3 +- src/wallet/rpcwallet.cpp | 16 +++---- .../mining_prioritisetransaction.py | 2 +- test/functional/p2p_compactblocks.py | 2 +- test/functional/p2p_sendheaders.py | 2 +- test/functional/rpc_blockchain.py | 8 +++- test/functional/rpc_rawtransaction.py | 12 +++--- test/functional/rpc_txoutproof.py | 8 +++- test/functional/wallet_basic.py | 8 +++- test/functional/wallet_listsinceblock.py | 4 +- 18 files changed, 86 insertions(+), 92 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index d00e3773c7..108f8aa11f 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -374,7 +374,7 @@ static UniValue waitforblock(const JSONRPCRequest& request) }.Check(request); int timeout = 0; - uint256 hash = uint256S(request.params[0].get_str()); + uint256 hash(ParseHashV(request.params[0], "blockhash")); if (!request.params[1].isNull()) timeout = request.params[1].get_int(); @@ -634,7 +634,7 @@ static UniValue getmempoolancestors(const JSONRPCRequest& request) if (!request.params[1].isNull()) fVerbose = request.params[1].get_bool(); - uint256 hash = ParseHashV(request.params[0], "parameter 1"); + uint256 hash(ParseHashV(request.params[0], "parameter 1")); const CTxMemPool& mempool = EnsureMemPool(request.context); LOCK(mempool.cs); @@ -698,7 +698,7 @@ static UniValue getmempooldescendants(const JSONRPCRequest& request) if (!request.params[1].isNull()) fVerbose = request.params[1].get_bool(); - uint256 hash = ParseHashV(request.params[0], "parameter 1"); + uint256 hash(ParseHashV(request.params[0], "parameter 1")); const CTxMemPool& mempool = EnsureMemPool(request.context); LOCK(mempool.cs); @@ -749,7 +749,7 @@ static UniValue getmempoolentry(const JSONRPCRequest& request) }, }.Check(request); - uint256 hash = ParseHashV(request.params[0], "parameter 1"); + uint256 hash(ParseHashV(request.params[0], "parameter 1")); const CTxMemPool& mempool = EnsureMemPool(request.context); LOCK(mempool.cs); @@ -862,8 +862,7 @@ static UniValue getblockheader(const JSONRPCRequest& request) }, }.Check(request); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "hash")); bool fVerbose = true; if (!request.params[1].isNull()) @@ -936,8 +935,7 @@ static UniValue getblockheaders(const JSONRPCRequest& request) }, }.Check(request); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "blockhash")); const CBlockIndex* pblockindex; const CBlockIndex* tip; @@ -1048,8 +1046,7 @@ static UniValue getmerkleblocks(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "Filter is not within size constraints"); } - std::string strHash = request.params[1].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[1], "blockhash")); const CBlockIndex* pblockindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pblockindex) { @@ -1154,8 +1151,7 @@ static UniValue getblock(const JSONRPCRequest& request) }, }.Check(request); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "blockhash")); int verbosity = 1; if (!request.params[1].isNull()) { @@ -1356,8 +1352,7 @@ static UniValue gettxout(const JSONRPCRequest& request) UniValue ret(UniValue::VOBJ); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "txid")); int n = request.params[1].get_int(); COutPoint out(hash, n); bool fMempool = true; @@ -1805,8 +1800,7 @@ static UniValue preciousblock(const JSONRPCRequest& request) }, }.Check(request); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "blockhash")); CBlockIndex* pblockindex; { @@ -1841,8 +1835,7 @@ static UniValue invalidateblock(const JSONRPCRequest& request) }, }.Check(request); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "blockhash")); CValidationState state; CBlockIndex* pblockindex; @@ -1881,8 +1874,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request) }, }.Check(request); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "blockhash")); { LOCK(cs_main); @@ -1937,7 +1929,7 @@ static UniValue getchaintxstats(const JSONRPCRequest& request) LOCK(cs_main); pindex = ::ChainActive().Tip(); } else { - uint256 hash = uint256S(request.params[1].get_str()); + uint256 hash(ParseHashV(request.params[1], "blockhash")); LOCK(cs_main); pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pindex) { @@ -2114,8 +2106,7 @@ static UniValue getblockstats(const JSONRPCRequest& request) pindex = ::ChainActive()[height]; } else { - const uint256 hash = ParseHashV(request.params[0], "parameter 1"); - + const uint256 hash(ParseHashV(request.params[0], "hash_or_height")); pindex = g_chainman.m_blockman.LookupBlockIndex(hash); if (!pindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); @@ -2307,8 +2298,7 @@ static UniValue getspecialtxes(const JSONRPCRequest& request) LOCK(cs_main); - std::string strHash = request.params[0].get_str(); - uint256 hash(uint256S(strHash)); + uint256 hash(ParseHashV(request.params[0], "blockhash")); int nTxType = -1; if (!request.params[1].isNull()) { @@ -2637,7 +2627,7 @@ static UniValue getblockfilter(const JSONRPCRequest& request) }, }.Check(request); - uint256 block_hash = ParseHashV(request.params[0], "blockhash"); + uint256 block_hash(ParseHashV(request.params[0], "blockhash")); std::string filtertype_name = "basic"; if (!request.params[1].isNull()) { filtertype_name = request.params[1].get_str(); diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index 05c70a4e79..90df3dff89 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -640,7 +640,7 @@ static UniValue protx_register_common_wrapper(const JSONRPCRequest& request, paramIdx++; } else { - uint256 collateralHash = ParseHashV(request.params[paramIdx], "collateralHash"); + uint256 collateralHash(ParseHashV(request.params[paramIdx], "collateralHash")); int32_t collateralIndex = ParseInt32V(request.params[paramIdx + 1], "collateralIndex"); if (collateralHash.IsNull() || collateralIndex < 0) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("invalid hash or index: %s-%d", collateralHash.ToString(), collateralIndex)); @@ -1405,7 +1405,7 @@ static UniValue protx_info(const JSONRPCRequest& request) g_txindex->BlockUntilSyncedToCurrentChain(); } - uint256 proTxHash = ParseHashV(request.params[0], "proTxHash"); + uint256 proTxHash(ParseHashV(request.params[0], "proTxHash")); auto mnList = deterministicMNManager->GetListAtChainTip(); auto dmn = mnList.GetMN(proTxHash); if (!dmn) { diff --git a/src/rpc/governance.cpp b/src/rpc/governance.cpp index e79eb3251b..79eb21a4a3 100644 --- a/src/rpc/governance.cpp +++ b/src/rpc/governance.cpp @@ -213,7 +213,7 @@ static UniValue gobject_prepare(const JSONRPCRequest& request) COutPoint outpoint; outpoint.SetNull(); if (!request.params[5].isNull() && !request.params[6].isNull()) { - uint256 collateralHash = ParseHashV(request.params[5], "outputHash"); + uint256 collateralHash(ParseHashV(request.params[5], "outputHash")); int32_t collateralIndex = ParseInt32V(request.params[6], "outputIndex"); if (collateralHash.IsNull() || collateralIndex < 0) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, strprintf("invalid hash or index: %s-%d", collateralHash.ToString(), collateralIndex)); @@ -427,9 +427,7 @@ static UniValue gobject_vote_conf(const JSONRPCRequest& request) { gobject_vote_conf_help(request); - uint256 hash; - - hash = ParseHashV(request.params[0], "Object hash"); + uint256 hash(ParseHashV(request.params[0], "Object hash")); std::string strVoteSignal = request.params[1].get_str(); std::string strVoteOutcome = request.params[2].get_str(); @@ -605,7 +603,7 @@ static UniValue gobject_vote_many(const JSONRPCRequest& request) std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - uint256 hash = ParseHashV(request.params[0], "Object hash"); + uint256 hash(ParseHashV(request.params[0], "Object hash")); std::string strVoteSignal = request.params[1].get_str(); std::string strVoteOutcome = request.params[2].get_str(); @@ -664,7 +662,7 @@ static UniValue gobject_vote_alias(const JSONRPCRequest& request) std::shared_ptr const wallet = GetWalletForJSONRPCRequest(request); if (!wallet) return NullUniValue; - uint256 hash = ParseHashV(request.params[0], "Object hash"); + uint256 hash(ParseHashV(request.params[0], "Object hash")); std::string strVoteSignal = request.params[1].get_str(); std::string strVoteOutcome = request.params[2].get_str(); @@ -682,7 +680,7 @@ static UniValue gobject_vote_alias(const JSONRPCRequest& request) EnsureWalletIsUnlocked(wallet.get()); - uint256 proTxHash = ParseHashV(request.params[3], "protx-hash"); + uint256 proTxHash(ParseHashV(request.params[3], "protx-hash")); auto dmn = deterministicMNManager->GetListAtChainTip().GetValidMN(proTxHash); if (!dmn) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid or unknown proTxHash"); @@ -850,7 +848,7 @@ static UniValue gobject_get(const JSONRPCRequest& request) gobject_get_help(request); // COLLECT VARIABLES FROM OUR USER - uint256 hash = ParseHashV(request.params[0], "GovObj hash"); + uint256 hash(ParseHashV(request.params[0], "GovObj hash")); if (g_txindex) { g_txindex->BlockUntilSyncedToCurrentChain(); @@ -943,11 +941,11 @@ static UniValue gobject_getcurrentvotes(const JSONRPCRequest& request) // COLLECT PARAMETERS FROM USER - uint256 hash = ParseHashV(request.params[0], "Governance hash"); + uint256 hash(ParseHashV(request.params[0], "Governance hash")); COutPoint mnCollateralOutpoint; if (!request.params[1].isNull() && !request.params[2].isNull()) { - uint256 txid = ParseHashV(request.params[1], "Masternode Collateral hash"); + uint256 txid(ParseHashV(request.params[1], "Masternode Collateral hash")); std::string strVout = request.params[2].get_str(); mnCollateralOutpoint = COutPoint(txid, LocaleIndependentAtoi(strVout)); } @@ -1076,11 +1074,11 @@ static UniValue voteraw(const JSONRPCRequest& request) RPCExamples{""} }.Check(request); - uint256 hashMnCollateralTx = ParseHashV(request.params[0], "mn collateral tx hash"); + uint256 hashMnCollateralTx(ParseHashV(request.params[0], "mn collateral tx hash")); int nMnCollateralTxIndex = request.params[1].get_int(); COutPoint outpoint = COutPoint(hashMnCollateralTx, nMnCollateralTxIndex); - uint256 hashGovObj = ParseHashV(request.params[2], "Governance hash"); + uint256 hashGovObj(ParseHashV(request.params[2], "Governance hash")); std::string strVoteSignal = request.params[3].get_str(); std::string strVoteOutcome = request.params[4].get_str(); diff --git a/src/rpc/masternode.cpp b/src/rpc/masternode.cpp index 82af40b21c..8751753a43 100644 --- a/src/rpc/masternode.cpp +++ b/src/rpc/masternode.cpp @@ -420,7 +420,7 @@ static UniValue masternode_payments(const JSONRPCRequest& request) pindex = ::ChainActive().Tip(); } else { LOCK(cs_main); - uint256 blockHash = ParseHashV(request.params[0], "blockhash"); + uint256 blockHash(ParseHashV(request.params[0], "blockhash")); pindex = g_chainman.m_blockman.LookupBlockIndex(blockHash); if (pindex == nullptr) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index c65a4512c2..7d04413ac5 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -476,7 +476,7 @@ static UniValue prioritisetransaction(const JSONRPCRequest& request) LOCK(cs_main); - uint256 hash = ParseHashV(request.params[0].get_str(), "txid"); + uint256 hash(ParseHashV(request.params[0].get_str(), "txid")); CAmount nAmount = request.params[1].get_int64(); EnsureMemPool(request.context).PrioritiseTransaction(hash, nAmount); @@ -726,7 +726,7 @@ static UniValue getblocktemplate(const JSONRPCRequest& request) // Format: std::string lpstr = lpval.get_str(); - hashWatchedChain.SetHex(lpstr.substr(0, 64)); + hashWatchedChain = ParseHashV(lpstr.substr(0, 64), "longpollid"); nTransactionsUpdatedLastLP = LocaleIndependentAtoi(lpstr.substr(64)); } else diff --git a/src/rpc/quorums.cpp b/src/rpc/quorums.cpp index 2dfeb42143..28bd9efa87 100644 --- a/src/rpc/quorums.cpp +++ b/src/rpc/quorums.cpp @@ -240,7 +240,7 @@ static UniValue quorum_info(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid LLMQ type"); } - uint256 quorumHash = ParseHashV(request.params[1], "quorumHash"); + uint256 quorumHash(ParseHashV(request.params[1], "quorumHash")); bool includeSkShare = false; if (!request.params[2].isNull()) { includeSkShare = ParseBoolV(request.params[2], "includeSkShare"); @@ -381,7 +381,7 @@ static UniValue quorum_memberof(const JSONRPCRequest& request) { quorum_memberof_help(request); - uint256 protxHash = ParseHashV(request.params[0], "proTxHash"); + uint256 protxHash(ParseHashV(request.params[0], "proTxHash")); int scanQuorumsCount = -1; if (!request.params[1].isNull()) { scanQuorumsCount = ParseInt32V(request.params[1], "scanQuorumsCount"); @@ -530,8 +530,8 @@ static UniValue quorum_sigs_cmd(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid LLMQ type"); } - uint256 id = ParseHashV(request.params[1], "id"); - uint256 msgHash = ParseHashV(request.params[2], "msgHash"); + uint256 id(ParseHashV(request.params[1], "id")); + uint256 msgHash(ParseHashV(request.params[2], "msgHash")); if (cmd == "quorumsign") { uint256 quorumHash; @@ -590,7 +590,7 @@ static UniValue quorum_sigs_cmd(const JSONRPCRequest& request) return llmq_ctx.sigman->VerifyRecoveredSig(llmqType, *llmq_ctx.qman, signHeight, id, msgHash, sig, 0) || llmq_ctx.sigman->VerifyRecoveredSig(llmqType, *llmq_ctx.qman, signHeight, id, msgHash, sig, signOffset); } else { - uint256 quorumHash = ParseHashV(request.params[4], "quorumHash"); + uint256 quorumHash(ParseHashV(request.params[4], "quorumHash")); llmq::CQuorumCPtr quorum = llmq_ctx.qman->GetQuorum(llmqType, quorumHash); if (!quorum) { @@ -642,7 +642,7 @@ static UniValue quorum_selectquorum(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_PARAMETER, "invalid LLMQ type"); } - uint256 id = ParseHashV(request.params[1], "id"); + uint256 id(ParseHashV(request.params[1], "id")); UniValue ret(UniValue::VOBJ); @@ -723,7 +723,7 @@ static UniValue quorum_getdata(const JSONRPCRequest& request) NodeId nodeId = ParseInt64V(request.params[0], "nodeId"); Consensus::LLMQType llmqType = static_cast(ParseInt32V(request.params[1], "llmqType")); - uint256 quorumHash = ParseHashV(request.params[2], "quorumHash"); + uint256 quorumHash(ParseHashV(request.params[2], "quorumHash")); uint16_t nDataMask = static_cast(ParseInt32V(request.params[3], "dataMask")); uint256 proTxHash; @@ -871,7 +871,7 @@ static UniValue verifychainlock(const JSONRPCRequest& request) { verifychainlock_help(request); - const uint256 nBlockHash = ParseHashV(request.params[0], "blockHash"); + const uint256 nBlockHash(ParseHashV(request.params[0], "blockHash")); CBLSSignature chainLockSig; if (!chainLockSig.SetHexStr(request.params[1].get_str())) { @@ -915,8 +915,8 @@ static UniValue verifyislock(const JSONRPCRequest& request) { verifyislock_help(request); - uint256 id = ParseHashV(request.params[0], "id"); - uint256 txid = ParseHashV(request.params[1], "txid"); + uint256 id(ParseHashV(request.params[0], "id")); + uint256 txid(ParseHashV(request.params[1], "txid")); CBLSSignature sig; if (!sig.SetHexStr(request.params[2].get_str())) { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 22e6002c87..cd1891138c 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -297,10 +297,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) UniValue txids = request.params[0].get_array(); for (unsigned int idx = 0; idx < txids.size(); idx++) { const UniValue& txid = txids[idx]; - if (txid.get_str().length() != 64 || !IsHex(txid.get_str())) { - throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid txid ")+txid.get_str()); - } - uint256 hash(uint256S(txid.get_str())); + uint256 hash(ParseHashV(txid, "txid")); if (setTxids.count(hash)) { throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ")+txid.get_str()); } @@ -312,7 +309,7 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) uint256 hashBlock; if (!request.params[1].isNull()) { LOCK(cs_main); - hashBlock = uint256S(request.params[1].get_str()); + hashBlock = ParseHashV(request.params[1], "blockhash"); pblockindex = g_chainman.m_blockman.LookupBlockIndex(hashBlock); if (!pblockindex) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 3c21a8c6ae..6b8b48a26a 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -88,16 +88,12 @@ CAmount AmountFromValue(const UniValue& value) uint256 ParseHashV(const UniValue& v, std::string strName) { - std::string strHex; - if (v.isStr()) - strHex = v.get_str(); + std::string strHex{v.get_str()}; + if (64 != strHex.length()) + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d, for '%s')", strName, 64, strHex.length(), strHex)); if (!IsHex(strHex)) // Note: IsHex("") is false throw JSONRPCError(RPC_INVALID_PARAMETER, strName+" must be hexadecimal string (not '"+strHex+"')"); - if (64 != strHex.length()) - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("%s must be of length %d (not %d)", strName, 64, strHex.length())); - uint256 result; - result.SetHex(strHex); - return result; + return uint256S(strHex); } uint256 ParseHashO(const UniValue& o, std::string strKey) { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 3be31a41ad..8926e9135b 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -358,8 +358,7 @@ UniValue removeprunedfunds(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); - uint256 hash; - hash.SetHex(request.params[0].get_str()); + uint256 hash(ParseHashV(request.params[0], "txid")); std::vector vHash; vHash.push_back(hash); std::vector vHashOut; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b174178ae4..b9171d10f0 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1582,7 +1582,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) uint256 blockId; if (!request.params[0].isNull() && !request.params[0].get_str().empty()) { - blockId.SetHex(request.params[0].get_str()); + blockId = ParseHashV(request.params[0], "blockhash"); height = pwallet->chain().findFork(blockId, &altheight); if (!height) { @@ -1707,8 +1707,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); - uint256 hash; - hash.SetHex(request.params[0].get_str()); + uint256 hash(ParseHashV(request.params[0], "txid")); isminefilter filter = ISMINE_SPENDABLE; @@ -1772,8 +1771,7 @@ static UniValue abandontransaction(const JSONRPCRequest& request) LOCK(pwallet->cs_wallet); - uint256 hash; - hash.SetHex(request.params[0].get_str()); + uint256 hash(ParseHashV(request.params[0], "txid")); if (!pwallet->mapWallet.count(hash)) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id"); @@ -2176,17 +2174,13 @@ static UniValue lockunspent(const JSONRPCRequest& request) {"vout", UniValueType(UniValue::VNUM)}, }); - const std::string& txid = find_value(o, "txid").get_str(); - if (!IsHex(txid)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, expected hex txid"); - } - + const uint256 txid(ParseHashO(o, "txid")); const int nOutput = find_value(o, "vout").get_int(); if (nOutput < 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid parameter, vout cannot be negative"); } - const COutPoint outpt(uint256S(txid), nOutput); + const COutPoint outpt(txid, nOutput); const auto it = pwallet->mapWallet.find(outpt.hash); if (it == pwallet->mapWallet.end()) { diff --git a/test/functional/mining_prioritisetransaction.py b/test/functional/mining_prioritisetransaction.py index 98f7922e54..9af7acdc20 100755 --- a/test/functional/mining_prioritisetransaction.py +++ b/test/functional/mining_prioritisetransaction.py @@ -30,7 +30,7 @@ class PrioritiseTransactionTest(BitcoinTestFramework): assert_raises_rpc_error(-1, "prioritisetransaction", self.nodes[0].prioritisetransaction, '', 0, 0) # Test `prioritisetransaction` invalid `txid` - assert_raises_rpc_error(-8, "txid must be hexadecimal string", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0) + assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].prioritisetransaction, txid='foo', fee_delta=0) assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000')", self.nodes[0].prioritisetransaction, txid='Zd1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000', fee_delta=0) txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000' diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py index 85ce4e138b..cf4cb9135a 100755 --- a/test/functional/p2p_compactblocks.py +++ b/test/functional/p2p_compactblocks.py @@ -279,7 +279,7 @@ class CompactBlocksTest(BitcoinTestFramework): block_hash = int(node.generate(1)[0], 16) # Store the raw block in our internal format. - block = FromHex(CBlock(), node.getblock("%02x" % block_hash, False)) + block = FromHex(CBlock(), node.getblock("%064x" % block_hash, False)) for tx in block.vtx: tx.calc_sha256() block.rehash() diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index dc42f638aa..c9f709580a 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -394,7 +394,7 @@ class SendHeadersTest(BitcoinTestFramework): block_time += 9 - fork_point = self.nodes[0].getblock("%02x" % new_block_hashes[0])["previousblockhash"] + fork_point = self.nodes[0].getblock("%064x" % new_block_hashes[0])["previousblockhash"] fork_point = int(fork_point, 16) # Use getblocks/getdata diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index d20670c51d..0220069cb3 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -142,7 +142,9 @@ class BlockchainTest(BitcoinTestFramework): # Test `getchaintxstats` invalid `blockhash` assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getchaintxstats, blockhash=0) - assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0') + assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 1, for '0')", self.nodes[0].getchaintxstats, blockhash='0') + assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getchaintxstats, blockhash='ZZZ0000000000000000000000000000000000000000000000000000000000000') + assert_raises_rpc_error(-5, "Block not found", self.nodes[0].getchaintxstats, blockhash='0000000000000000000000000000000000000000000000000000000000000000') blockhash = self.nodes[0].getblockhash(200) self.nodes[0].invalidateblock(blockhash) assert_raises_rpc_error(-8, "Block is not in main chain", self.nodes[0].getchaintxstats, blockhash=blockhash) @@ -244,7 +246,9 @@ class BlockchainTest(BitcoinTestFramework): def _test_getblockheader(self): node = self.nodes[0] - assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "nonsense") + assert_raises_rpc_error(-8, "hash must be of length 64 (not 8, for 'nonsense')", node.getblockheader, "nonsense") + assert_raises_rpc_error(-8, "hash must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", node.getblockheader, "ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844") + assert_raises_rpc_error(-5, "Block not found", node.getblockheader, "0cf7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844") besthash = node.getbestblockhash() secondbesthash = node.getblockhash(199) diff --git a/test/functional/rpc_rawtransaction.py b/test/functional/rpc_rawtransaction.py index 46b9461fb4..35c0d94ae7 100755 --- a/test/functional/rpc_rawtransaction.py +++ b/test/functional/rpc_rawtransaction.py @@ -90,8 +90,9 @@ class RawTransactionsTest(BitcoinTestFramework): txid = '1d1d4e24ed99057e84c3f80fd8fbec79ed9e1acee37da269356ecea000000000' assert_raises_rpc_error(-3, "Expected type array", self.nodes[0].createrawtransaction, 'foo', {}) assert_raises_rpc_error(-1, "JSON value is not an object as expected", self.nodes[0].createrawtransaction, ['foo'], {}) - assert_raises_rpc_error(-8, "txid must be hexadecimal string", self.nodes[0].createrawtransaction, [{}], {}) - assert_raises_rpc_error(-8, "txid must be hexadecimal string", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {}) + assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].createrawtransaction, [{}], {}) + assert_raises_rpc_error(-8, "txid must be of length 64 (not 3, for 'foo')", self.nodes[0].createrawtransaction, [{'txid': 'foo'}], {}) + assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844')", self.nodes[0].createrawtransaction, [{'txid': 'ZZZ7bb8b1697ea987f3b223ba7819250cae33efacb068d23dc24859824a77844'}], {}) assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid}], {}) assert_raises_rpc_error(-8, "Invalid parameter, missing vout key", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': 'foo'}], {}) assert_raises_rpc_error(-8, "Invalid parameter, vout cannot be negative", self.nodes[0].createrawtransaction, [{'txid': txid, 'vout': -1}], {}) @@ -171,9 +172,10 @@ class RawTransactionsTest(BitcoinTestFramework): # 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(-1, "JSON value is not a string as expected", self.nodes[0].getrawtransaction, tx, True, True) + assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 6, for 'foobar')", self.nodes[0].getrawtransaction, tx, True, "foobar") + assert_raises_rpc_error(-8, "parameter 3 must be of length 64 (not 8, for 'abcd1234')", self.nodes[0].getrawtransaction, tx, True, "abcd1234") + assert_raises_rpc_error(-8, "parameter 3 must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].getrawtransaction, tx, True, "ZZZ0000000000000000000000000000000000000000000000000000000000000") assert_raises_rpc_error(-5, "Block hash not found", self.nodes[0].getrawtransaction, tx, True, "0000000000000000000000000000000000000000000000000000000000000000") # Undo the blocks and check in_active_chain self.nodes[0].invalidateblock(block1) diff --git a/test/functional/rpc_txoutproof.py b/test/functional/rpc_txoutproof.py index b75966b85c..65d5b5eda8 100755 --- a/test/functional/rpc_txoutproof.py +++ b/test/functional/rpc_txoutproof.py @@ -69,13 +69,19 @@ class MerkleBlockTest(BitcoinTestFramework): txid_spent = txin_spent["txid"] txid_unspent = txid1 if txin_spent["txid"] != txid1 else txid2 + # Invalid txids + assert_raises_rpc_error(-8, "txid must be of length 64 (not 32, for '00000000000000000000000000000000')", self.nodes[2].gettxoutproof, ["00000000000000000000000000000000"], blockhash) + assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[2].gettxoutproof, ["ZZZ0000000000000000000000000000000000000000000000000000000000000"], blockhash) + # Invalid blockhashes + assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 32, for '00000000000000000000000000000000')", self.nodes[2].gettxoutproof, [txid_spent], "00000000000000000000000000000000") + assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", self.nodes[2].gettxoutproof, [txid_spent], "ZZZ0000000000000000000000000000000000000000000000000000000000000") # We can't find the block from a fully-spent tx # Doesn't apply to Dash Core - we have txindex always on # assert_raises_rpc_error(-5, "Transaction not yet in block", self.nodes[2].gettxoutproof, [txid_spent]) # We can get the proof if we specify the block assert_equal(self.nodes[2].verifytxoutproof(self.nodes[2].gettxoutproof([txid_spent], blockhash)), [txid_spent]) # We can't get the proof if we specify a non-existent block - assert_raises_rpc_error(-5, "Block not found", self.nodes[2].gettxoutproof, [txid_spent], "00000000000000000000000000000000") + assert_raises_rpc_error(-5, "Block not found", self.nodes[2].gettxoutproof, [txid_spent], "0000000000000000000000000000000000000000000000000000000000000000") # We can get the proof if the transaction is unspent assert_equal(self.nodes[2].verifytxoutproof(self.nodes[2].gettxoutproof([txid_unspent])), [txid_unspent]) # We can get the proof if we provide a list of transactions and one of them is unspent. The ordering of the list should not matter. diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index d43bb1312d..d419bfb946 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -121,9 +121,15 @@ class WalletTest(BitcoinTestFramework): assert_equal([unspent_0], self.nodes[2].listlockunspent()) self.nodes[2].lockunspent(True, [unspent_0]) assert_equal(len(self.nodes[2].listlockunspent()), 0) - assert_raises_rpc_error(-8, "Invalid parameter, unknown transaction", + assert_raises_rpc_error(-8, "txid must be of length 64 (not 34, for '0000000000000000000000000000000000')", self.nodes[2].lockunspent, False, [{"txid": "0000000000000000000000000000000000", "vout": 0}]) + assert_raises_rpc_error(-8, "txid must be hexadecimal string (not 'ZZZ0000000000000000000000000000000000000000000000000000000000000')", + self.nodes[2].lockunspent, False, + [{"txid": "ZZZ0000000000000000000000000000000000000000000000000000000000000", "vout": 0}]) + assert_raises_rpc_error(-8, "Invalid parameter, unknown transaction", + self.nodes[2].lockunspent, False, + [{"txid": "0000000000000000000000000000000000000000000000000000000000000000", "vout": 0}]) assert_raises_rpc_error(-8, "Invalid parameter, vout index out of bounds", self.nodes[2].lockunspent, False, [{"txid": unspent_0["txid"], "vout": 999}]) diff --git a/test/functional/wallet_listsinceblock.py b/test/functional/wallet_listsinceblock.py index 3aa85bad68..7715d5f5f7 100755 --- a/test/functional/wallet_listsinceblock.py +++ b/test/functional/wallet_listsinceblock.py @@ -66,8 +66,10 @@ class ListSinceBlockTest(BitcoinTestFramework): "42759cde25462784395a337460bde75f58e73d3f08bd31fdc3507cbac856a2c4") assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, "0000000000000000000000000000000000000000000000000000000000000000") - assert_raises_rpc_error(-5, "Block not found", self.nodes[0].listsinceblock, + assert_raises_rpc_error(-8, "blockhash must be of length 64 (not 11, for 'invalid-hex')", self.nodes[0].listsinceblock, "invalid-hex") + assert_raises_rpc_error(-8, "blockhash must be hexadecimal string (not 'Z000000000000000000000000000000000000000000000000000000000000000')", self.nodes[0].listsinceblock, + "Z000000000000000000000000000000000000000000000000000000000000000") def test_reorg(self): ''' From 175503304873e89fd05039d472f7da2f9b0284c4 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Wed, 22 Feb 2023 02:53:06 +0700 Subject: [PATCH 03/13] fix: follow up bitcoin#10637: returned nBytes should be signed --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 1694cab113..068b9bb6a3 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3404,7 +3404,7 @@ bool CWallet::CreateTransaction(const std::vector& vecSend, CTransac FeeCalculation feeCalc; CFeeRate discard_rate = coin_control.m_discard_feerate ? *coin_control.m_discard_feerate : GetDiscardRate(*this); - unsigned int nBytes{0}; + int nBytes{0}; { std::vector vecCoins; LOCK(cs_wallet); From b804c7d7fe2c54ca0a0dd00d0166f0eca15dee78 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 30 Nov 2018 10:49:42 -0500 Subject: [PATCH 04/13] Merge #14380: fix assert crash when specified change output spend size is unknown 0fb2e69815 CreateTransaction: Assume minimum p2sh-p2wpkh spend size for unknown change (Gregory Sanders) b06483c96a Remove stale comment in CalculateMaximumSignedInputSize (Gregory Sanders) Pull request description: This is triggered anytime a fundraw type call(psbt or legacy) is used with a change output address that the wallet doesn't know how to sign for. This regression was added in 6a34ff5335786615771ca423134a484b04831c4e since BnB coin selection actually cares about this. The fix is to assume the smallest typical spend, a P2SH-P2WPKH, which is calculated using a "prototype" dummy signature flow. Future work could generalize this infrastructure to get estimated sizes of inputs for a variety of types. I also removed a comment which I believe is stale and misleading. Tree-SHA512: c7e2be189e524f81a7aa4454ad9370cefba715e3781f1e462c8bab77e4d27540191419029e3ebda11e3744c0703271e479dcd560d05e4d470048d9633e34da16 --- src/wallet/test/wallet_tests.cpp | 48 ++++++++++++++++++++++++++++++++ src/wallet/wallet.cpp | 2 -- test/functional/rpc_psbt.py | 4 +++ 3 files changed, 52 insertions(+), 2 deletions(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index c1a94d3836..5e7c88fd48 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -1326,4 +1326,52 @@ BOOST_FIXTURE_TEST_CASE(CreateWalletFromFile, TestChain100Setup) TestUnloadWallet(std::move(wallet)); } +// Explicit calculation which is used to test the wallet constant +// We get the same virtual size due to rounding(weight/4) for both use_max_sig values +static size_t CalculateNestedKeyhashInputSize(bool use_max_sig) +{ + // Generate ephemeral valid pubkey + CKey key; + key.MakeNewKey(true); + CPubKey pubkey = key.GetPubKey(); + + // Generate pubkey hash + uint160 key_hash(Hash160(pubkey.begin(), pubkey.end())); + + // Create inner-script to enter into keystore. Key hash can't be 0... + CScript inner_script = CScript() << OP_0 << std::vector(key_hash.begin(), key_hash.end()); + + // Create outer P2SH script for the output + CScript script_pubkey = GetScriptForRawPubKey(pubkey); + + NodeContext node; + node.fee_estimator = std::make_unique(); + node.mempool = std::make_unique(node.fee_estimator.get()); + auto chain = interfaces::MakeChain(node); + CWallet wallet(chain.get(), "", CreateDummyWalletDatabase()); + AddKey(wallet, key); + auto spk_man = wallet.GetLegacyScriptPubKeyMan(); + spk_man->AddCScript(inner_script); + + // Fill in dummy signatures for fee calculation. + SignatureData sig_data; + + if (!ProduceSignature(*spk_man, use_max_sig ? DUMMY_MAXIMUM_SIGNATURE_CREATOR : DUMMY_SIGNATURE_CREATOR, script_pubkey, sig_data)) { + // We're hand-feeding it correct arguments; shouldn't happen + assert(false); + } + + CTxIn tx_in; + UpdateInput(tx_in, sig_data); + return ::GetSerializeSize(tx_in, PROTOCOL_VERSION); +} + +static constexpr size_t DUMMY_NESTED_P2PKH_INPUT_SIZE = 113; +BOOST_FIXTURE_TEST_CASE(dummy_input_size_test, TestChain100Setup) +{ + std::cerr << "CDEF " << CalculateNestedKeyhashInputSize(false) << std::endl; + BOOST_CHECK_EQUAL(CalculateNestedKeyhashInputSize(false), DUMMY_NESTED_P2PKH_INPUT_SIZE); + BOOST_CHECK_EQUAL(CalculateNestedKeyhashInputSize(true), DUMMY_NESTED_P2PKH_INPUT_SIZE + 1); +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 068b9bb6a3..f4aa3e22f8 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1786,8 +1786,6 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* wallet, CMutableTransaction txn; txn.vin.push_back(CTxIn(COutPoint())); if (!wallet->DummySignInput(txn.vin[0], txout, use_max_sig)) { - // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) - // implies that we can sign for every input. return -1; } return ::GetSerializeSize(txn.vin[0], PROTOCOL_VERSION); diff --git a/test/functional/rpc_psbt.py b/test/functional/rpc_psbt.py index 1b01f41b60..fa93c1e3a4 100755 --- a/test/functional/rpc_psbt.py +++ b/test/functional/rpc_psbt.py @@ -158,6 +158,10 @@ class PSBTTest(BitcoinTestFramework): self.nodes[0].generate(6) self.sync_all() + # Make sure change address wallet does not have P2SH innerscript access to results in success + # when attempting BnB coin selection + block_height = self.nodes[0].getblockcount() + self.nodes[0].walletcreatefundedpsbt([], [{self.nodes[2].getnewaddress():self.nodes[0].listunspent()[0]["amount"]+1}], block_height+2, {"changeAddress":self.nodes[1].getnewaddress()}, False) # Regression test for 14473 (mishandling of already-signed witness transaction): unspent = self.nodes[0].listunspent()[0] From 27b5d685627e0e27d9b7fc01105b9d25c0548712 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 9 Apr 2019 11:17:25 -0400 Subject: [PATCH 05/13] Merge #15693: travis: Switch to ubuntu keyserver to avoid timeouts fa2056af1c travis: Properly cache and error on timeout (MarcoFalke) fa36a333ee travis: Switch to ubuntu keyserver to avoid timeouts (MarcoFalke) Pull request description: The other keyserver is consistently timing out on travis: https://travis-ci.org/bitcoin/bitcoin/jobs/512689710#L405 Attempt to fix it by using a different server. Also: * fixes #15372 * fixes #15738 ACKs for commit fa2056: ryanofsky: utACK fa2056af1c71aded3a821a07ec4de71c4be0bca3. All good changes (changing keyserver, getting rid of keyserver while loop, clarifying travis error, moving travis documentation to code comment). Tree-SHA512: ac8436616ecfee0ed579114e19f03c53ceb688fbcd95a60cffe8f15b4e569772a6ba673f353bbd789e79fe27fc5626c77fab4086768844dd51e0c6c108b52fb2 --- .travis.yml | 17 ++++++++++++++++- ci/lint/06_script.sh | 2 +- doc/README.md | 1 - doc/travis-ci.md | 42 ------------------------------------------ 4 files changed, 17 insertions(+), 45 deletions(-) delete mode 100644 doc/travis-ci.md diff --git a/.travis.yml b/.travis.yml index a2ed8639d6..d2551a1d4a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,21 @@ # - sudo/dist/group are set so as to get Blue Box VMs, necessary for [loopback] # IPv6 support +# The test build matrix (stage: test) is constructed to test a wide range of +# configurations, rather than a single pass/fail. This helps to catch build +# failures and logic errors that present on platforms other than the ones the +# author has tested. +# +# Some builders use the dependency-generator in `./depends`, rather than using +# apt-get to install build dependencies. This guarantees that the tester is +# using the same versions as Gitian, so the build results are nearly identical +# to what would be found in a final release. +# +# In order to avoid rebuilding all dependencies for each build, the binaries +# are cached and re-used when possible. Changes in the dependency-generator +# will trigger cache-invalidation and rebuilds as necessary. +# + dist: xenial os: linux @@ -177,7 +192,7 @@ before_script: # Build docker image only for develop branch of the main repo - if [ "$TRAVIS_REPO_SLUG" != "dashpay/dash" -o "$TRAVIS_BRANCH" != "develop" -o "$TRAVIS_PULL_REQUEST" != "false" ]; then export DOCKER_BUILD="false"; echo DOCKER_BUILD=$DOCKER_BUILD; fi # TODO: Check keys and signed commits - #- if [ "$TRAVIS_REPO_SLUG" = "dashpay/dash" -a "$TRAVIS_PULL_REQUEST" = "false" ]; then while read LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys; fi + #- if [ "$TRAVIS_REPO_SLUG" = "dashpay/dash" -a "$TRAVIS_PULL_REQUEST" = "false" ]; then travis_retry gpg --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys $( ./contrib/verify-commits/trusted-sha512-root-commit - while read -r LINE; do travis_retry gpg --keyserver hkp://subset.pool.sks-keyservers.net --recv-keys $LINE; done < contrib/verify-commits/trusted-keys && + travis_retry gpg --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys $( Date: Wed, 28 Aug 2019 16:02:31 -0400 Subject: [PATCH 06/13] Merge #14879: qt: Add warning messages to the debug window 593ba696fb32da558091ac02ad87c4893db4ce97 Add warning messages to the debug window (Hennadii Stepanov) Pull request description: Fix: #11016 This PR adds warning messages to the debug window in `-disablewallet` mode. ![screenshot from 2018-12-06 01-01-27](https://user-images.githubusercontent.com/32963518/49550070-413c1c80-f8f3-11e8-9865-efb49ea8da45.png) ACKs for top commit: jonasschnelli: utACK 593ba696fb32da558091ac02ad87c4893db4ce97 promag: ACK 593ba696fb32da558091ac02ad87c4893db4ce97, agree with @Sjors https://github.com/bitcoin/bitcoin/pull/14879#pullrequestreview-196433092 above. ryanofsky: utACK 593ba696fb32da558091ac02ad87c4893db4ce97 Tree-SHA512: a8ca78529bb16813ba7bfaf5ccd4349189979f08e78ea857746a6fb00fd9d7ed98d8f06f384830acba21dac57070060af23f6be8249398feb32a6efff1333de8 14879 followup: move label_alerts css styling into css files and align it with the style of labelAlerts on OverviewPage Co-authored-by: UdjinM6 --- src/qt/forms/debugwindow.ui | 16 ++++++++++++++++ src/qt/overviewpage.cpp | 5 ++--- src/qt/res/css/general.css | 7 +++++++ src/qt/res/css/traditional.css | 11 +++++++++++ src/qt/rpcconsole.cpp | 17 +++++++++++++++++ src/qt/rpcconsole.h | 3 +++ 6 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui index 9d99bff998..ecbec6794b 100644 --- a/src/qt/forms/debugwindow.ui +++ b/src/qt/forms/debugwindow.ui @@ -14,6 +14,22 @@ Tools window + + + + false + + + true + + + 3 + + + Qt::TextSelectableByMouse + + + diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index 70ca03a1c3..6fc68a0be1 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -251,9 +251,8 @@ void OverviewPage::updateWatchOnlyLabels(bool showWatchOnly) void OverviewPage::setClientModel(ClientModel *model) { this->clientModel = model; - if(model) - { - // Show warning if this is a prerelease version + if (model) { + // Show warning, for example if this is a prerelease version connect(model, &ClientModel::alertsChanged, this, &OverviewPage::updateAlerts); updateAlerts(model->getStatusBarWarnings()); } diff --git a/src/qt/res/css/general.css b/src/qt/res/css/general.css index e104e92ae7..9c62c7651a 100644 --- a/src/qt/res/css/general.css +++ b/src/qt/res/css/general.css @@ -1586,6 +1586,13 @@ QWidget#RPCConsole QLabel#peerHeading, QWidget#RPCConsole QLabel#banHeading { } +QWidget#RPCConsole QLabel#label_alerts { + background-color: #c79304; + color: #222222; + border-radius: 5px; + padding: 5px; +} + QWidget#RPCConsole QLabel#labelNetwork, QWidget#RPCConsole QLabel#label_10, QWidget#RPCConsole QLabel#labelMempoolTitle { /* Margin between Network and Block Chain headers */ diff --git a/src/qt/res/css/traditional.css b/src/qt/res/css/traditional.css index cdafe03274..cd15a30d03 100644 --- a/src/qt/res/css/traditional.css +++ b/src/qt/res/css/traditional.css @@ -250,6 +250,17 @@ QWidget .QFrame#frame_2 QListView { /* Transaction List */ margin-right: 10px; } +/****************************************************** +RPCConsole +******************************************************/ + +QWidget#RPCConsole QLabel#label_alerts { + background-color: #c79304; + color: #222222; + border-radius: 5px; + padding: 5px; +} + /****************************************************** SendCoinsDialog ******************************************************/ diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 4e973d3c00..6afafb8476 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -583,6 +583,17 @@ bool RPCConsole::eventFilter(QObject* obj, QEvent *event) void RPCConsole::setClientModel(ClientModel *model, int bestblock_height, int64_t bestblock_date, uint256 bestblock_hash, double verification_progress) { clientModel = model; + + bool wallet_enabled{false}; +#ifdef ENABLE_WALLET + wallet_enabled = WalletModel::isWalletEnabled(); +#endif // ENABLE_WALLET + if (model && !wallet_enabled) { + // Show warning, for example if this is a prerelease version + connect(model, &ClientModel::alertsChanged, this, &RPCConsole::updateAlerts); + updateAlerts(model->getStatusBarWarnings()); + } + ui->trafficGraph->setClientModel(model); if (model && clientModel->getPeerTableModel() && clientModel->getBanTableModel()) { // Keep up to date with client @@ -1441,3 +1452,9 @@ QKeySequence RPCConsole::tabShortcut(TabTypes tab_type) const assert(false); } + +void RPCConsole::updateAlerts(const QString& warnings) +{ + this->ui->label_alerts->setVisible(!warnings.isEmpty()); + this->ui->label_alerts->setText(warnings); +} diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h index 76456236db..a3019d528d 100644 --- a/src/qt/rpcconsole.h +++ b/src/qt/rpcconsole.h @@ -192,6 +192,9 @@ private: /** Update UI with latest network info from model. */ void updateNetworkState(); + +private Q_SLOTS: + void updateAlerts(const QString& warnings); }; #endif // BITCOIN_QT_RPCCONSOLE_H From 0c05d0b59de69c373daa10b6fa639f994649ed22 Mon Sep 17 00:00:00 2001 From: fanquake Date: Thu, 29 Aug 2019 11:40:10 +0800 Subject: [PATCH 07/13] Merge #16556: Fix systemd service file configuration directory setup f3b57f4a1c17aadbf02d408e980490c88838c6ba Unrecommend making config file owned by bitcoin (setpill) 870d4152dfc3d990e336723562948835c2dbd646 Set ProtectHome in systemd service file (setpill) 639a416e3758b3005b860b198f0ec7bdd80a7f0c Chgrp config dir to bitcoin in systemd service (setpill) aded0528f0e1e3735ce8dd26fd9e546150b73187 Improve clarity of systemd service file comments (setpill) Pull request description: Rationale: ran into a bug with the systemd service file, fixed it locally and figured I might as well contribute my fix. Also fixed some unrelated confusing phrasing in the comments of the same file, after discussion in IRC. ACKs for top commit: sipsorcery: tACK f3b57f4a1c17aadbf02d408e980490c88838c6ba (nothing changed since previous tACK). ryanofsky: utACK f3b57f4a1c17aadbf02d408e980490c88838c6ba. Only change since last review is removing ConfigurationDirectoryMode churn in early commits Tree-SHA512: 2188345878925b9e8a5c2c3df8dfba443720e2252a164db54a8e1d8007846721497b2d98c56f1d9b60a9a9ed4fdb1156c7b02c699616b220a9b614671617d32a --- contrib/init/dashd.service | 12 ++++++++++-- doc/init.md | 10 +++++----- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/contrib/init/dashd.service b/contrib/init/dashd.service index 8bdb2ae207..5dfd7c0c4d 100644 --- a/contrib/init/dashd.service +++ b/contrib/init/dashd.service @@ -5,8 +5,9 @@ # See "man systemd.service" for details. # Note that almost all daemon options could be specified in -# /etc/dash/dash.conf, except for those explicitly specified as arguments -# in ExecStart= +# /etc/dash/dash.conf, but keep in mind those explicitly +# specified as arguments in ExecStart= will override those in the +# config file. [Unit] Description=Dash daemon @@ -18,6 +19,10 @@ ExecStart=/usr/bin/dashd -daemon \ -conf=/etc/dash/dash.conf \ -datadir=/var/lib/dashd +# Make sure the config directory is readable by the service user +PermissionsStartOnly=true +ExecStartPre=/bin/chgrp dashcore /etc/dash + # Process management #################### @@ -54,6 +59,9 @@ PrivateTmp=true # Mount /usr, /boot/ and /etc read-only for the process. ProtectSystem=full +# Deny access to /home, /root and /run/user +ProtectHome=true + # Disallow the process and all of its children to gain # new privileges through execve(). NoNewPrivileges=true diff --git a/doc/init.md b/doc/init.md index 4b4c5da565..f5d2891912 100644 --- a/doc/init.md +++ b/doc/init.md @@ -59,11 +59,11 @@ Data directory: `/var/lib/dashd` PID file: `/var/run/dashd/dashd.pid` (OpenRC and Upstart) or `/run/dashd/dashd.pid` (systemd) Lock file: `/var/lock/subsys/dashd` (CentOS) -The configuration file, PID directory (if applicable) and data directory -should all be owned by the dashcore user and group. It is advised for security -reasons to make the configuration file and data directory only readable by the -dashcore user and group. Access to dash-cli and other dashd rpc clients -can then be controlled by group membership. +The PID directory (if applicable) and data directory should both be owned by the +dashcore user and group. It is advised for security reasons to make the +configuration file and data directory only readable by the dashcore user and +group. Access to dash-cli and other dashd rpc clients can then be +controlled by group membership. NOTE: When using the systemd .service file, the creation of the aforementioned directories and the setting of their permissions is automatically handled by From 4c72e6966dc650e646949faeb98d0a52c8862f7c Mon Sep 17 00:00:00 2001 From: MeshCollider Date: Mon, 2 Sep 2019 23:29:42 +1200 Subject: [PATCH 08/13] Merge #16185: gettransaction: add an argument to decode the transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 9965940e35c445ccded55510348af228ff22f0e9 doc: Add release note for the new gettransaction argument (darosior) b8b3f0435a2837d3897e9e232ef6ca839ce74eb8 tests: Add a new functional test for gettransaction (darosior) 7f3bb247a811582d1aa4805d8e601c19808dc7ba gettransaction: add an argument to decode the transaction (darosior) Pull request description: This PR adds a new parameter to the `gettransaction` call : `decode`. If set to `true`, it will add a new `decoded` field to the response. This mimics the behavior of `getrawtransaction`'s `verbose` argument to avoid using 2 calls if we want to decode a wallet transaction (`gettransaction` then `decoderawtransaction`). Fix #16181 . ACKs for top commit: meshcollider: re-utACK 9965940e35c445ccded55510348af228ff22f0e9 Tree-SHA512: bcb6b4bd252b3488d6afc77659c499c2ad99fd58661eb24b6a2e17014c74f22e47fde70e00fedb4f4754915786622ad02483b2cf2c4dea0ab0eb4ac8276dbeee --- doc/release-notes-16185.md | 3 +++ src/rpc/client.cpp | 1 + src/wallet/rpcwallet.cpp | 15 ++++++++++++++- test/functional/wallet_basic.py | 5 +++++ 4 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 doc/release-notes-16185.md diff --git a/doc/release-notes-16185.md b/doc/release-notes-16185.md new file mode 100644 index 0000000000..eeeb951e5b --- /dev/null +++ b/doc/release-notes-16185.md @@ -0,0 +1,3 @@ +RPC changes +----------- +The `gettransaction` RPC now accepts a third (boolean) argument `decode`. If set to `true`, a new `decoded` field will be added to the response containing the decoded transaction. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 4fb93be805..424d75f462 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -105,6 +105,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getchaintxstats", 0, "nblocks" }, { "getmerkleblocks", 2, "count" }, { "gettransaction", 1, "include_watchonly" }, + { "gettransaction", 2, "decode" }, { "getrawtransaction", 1, "verbose" }, { "createrawtransaction", 0, "inputs" }, { "createrawtransaction", 1, "outputs" }, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index b9171d10f0..ca3e76a25c 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1655,6 +1655,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"}, + {"decode", RPCArg::Type::BOOL, /* default */ "false", "Whether to add a field with the decoded transaction"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", Cat(Cat>( @@ -1688,6 +1689,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, + {RPCResult::Type::OBJ, "decoded", /*optional=*/true, "The decoded transaction", + { + {RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."}, + }}, }), }, RPCExamples{ @@ -1715,6 +1720,8 @@ static UniValue gettransaction(const JSONRPCRequest& request) filter |= ISMINE_WATCH_ONLY; } + bool decode_tx = request.params[2].isNull() ? false : request.params[2].get_bool(); + UniValue entry(UniValue::VOBJ); auto it = pwallet->mapWallet.find(hash); if (it == pwallet->mapWallet.end()) { @@ -1740,6 +1747,12 @@ static UniValue gettransaction(const JSONRPCRequest& request) std::string strHex = EncodeHexTx(*wtx.tx); entry.pushKV("hex", strHex); + if (decode_tx) { + UniValue decoded(UniValue::VOBJ); + TxToUniv(*wtx.tx, uint256(), decoded, false); + entry.pushKV("decoded", decoded); + } + return entry; } @@ -4093,7 +4106,7 @@ static const CRPCCommand commands[] = { "wallet", "getrawchangeaddress", &getrawchangeaddress, {} }, { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf","addlocked"} }, { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf","addlocked"} }, - { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly"} }, + { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","decode"} }, { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, { "wallet", "getbalances", &getbalances, {} }, { "wallet", "getwalletinfo", &getwalletinfo, {} }, diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index d419bfb946..7e9f0ab531 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -496,6 +496,11 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].setlabel(change, 'foobar') assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False) + # Test "decoded" field value in gettransaction response + self.log.info("Testing gettransaction decoding...") + tx = self.nodes[0].gettransaction(txid=txid, decode=True) + assert_equal(tx["decoded"], self.nodes[0].decoderawtransaction(tx["hex"])) + if __name__ == '__main__': WalletTest().main() From 221c91a6713dd26b8374e51a8f610429e045d8e9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 9 Sep 2019 13:54:54 +0300 Subject: [PATCH 09/13] Merge #16806: doc: Add issue templates for bug and feature request fabca7756d6908ad581f3a699f1be6ecc9f62e03 doc: Add issue templates for bug and feature request (MarcoFalke) Pull request description: Fixes #16627 Can be tested via https://github.com/MarcoFalke/bitcoin/issues ACKs for top commit: jb55: ACK fabca7756d6908ad581f3a699f1be6ecc9f62e03 fanquake: ACK fabca7756d6908ad581f3a699f1be6ecc9f62e03 Tree-SHA512: 1ebe58f9c0110a9332adf1d80001cd9ed6fe60208e387c93b8564dc66821f753e34b23cb6f4cae45168024862ee884913976e132820b7a4759fa6391b0d1127c --- .github/ISSUE_TEMPLATE/bug_report.md | 41 +++++++++++++++++++++++ .github/ISSUE_TEMPLATE/feature_request.md | 20 +++++++++++ 2 files changed, 61 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/bug_report.md create mode 100644 .github/ISSUE_TEMPLATE/feature_request.md diff --git a/.github/ISSUE_TEMPLATE/bug_report.md b/.github/ISSUE_TEMPLATE/bug_report.md new file mode 100644 index 0000000000..09464471af --- /dev/null +++ b/.github/ISSUE_TEMPLATE/bug_report.md @@ -0,0 +1,41 @@ +--- +name: Bug report +about: Create a report to help us improve +title: '' +labels: Bug +assignees: '' + +--- + + + + + +**Expected behavior** + + + +**Actual behavior** + + + +**To reproduce** + + + +**System information** + + + + + + + + + diff --git a/.github/ISSUE_TEMPLATE/feature_request.md b/.github/ISSUE_TEMPLATE/feature_request.md new file mode 100644 index 0000000000..2d5685185e --- /dev/null +++ b/.github/ISSUE_TEMPLATE/feature_request.md @@ -0,0 +1,20 @@ +--- +name: Feature request +about: Suggest an idea for this project +title: '' +labels: Feature +assignees: '' + +--- + +**Is your feature request related to a problem? Please describe.** + + +**Describe the solution you'd like** + + +**Describe alternatives you've considered** + + +**Additional context** + From af24beb9072211bb89539918061e7d53c31df4d4 Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 10 Sep 2019 10:16:07 +1000 Subject: [PATCH 10/13] Merge #16489: log: harmonize bitcoind logging e90478f43e7bf9726ba033fde4a2776f9d5a9af4 log: harmonize bitcoind server logging (Jon Atack) Pull request description: Harmonize the user-facing output of the `bitcoind -daemon`, `bitcoin-cli help stop`, `bitcoin-cli stop`, and `bitcoind -version` commands to be consistent with each other as well as with the "Bitcoin Core is probably already running" messages, e.g. `git grep 'probably already running.")'`. Before: ``` $ bitcoind -regtest -daemon Bitcoin Core daemon starting $ bitcoind -regtest -daemon Error: Bitcoin Core is probably already running. $ bitcoind -regtest -version Bitcoin Core Daemon version v0.18.99.0-e653eeff76-dirty $ bitcoin-cli -regtest help stop stop Stop Bitcoin server. $ bitcoin-cli -regtest stop Bitcoin server stopping ``` these five commands output: "Bitcoin Core daemon" "Bitcoin Core" "Bitcoin Core Daemon" "Bitcoin server" "Bitcoin server" After this commit, they are all "Bitcoin Core". ``` $ bitcoind -regtest -daemon Bitcoin Core starting $ bitcoind -regtest -daemon Error: Bitcoin Core is probably already running. $ bitcoind -regtest -version Bitcoin Core version v0.18.99.0-e90478f43e-dirty $ bitcoin-cli -regtest help stop stop Request a graceful shutdown of Bitcoin Core. $ bitcoin-cli -regtest stop Bitcoin Core stopping ``` ACKs for top commit: practicalswift: ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4 (read code which looks good) practicalswift: ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4 -- diff looks correct fjahr: utACK e90478f michaelfolkson: ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4. Tested command outputs and as described. ariard: Tested ACK e90478f fanquake: ACK e90478f43e7bf9726ba033fde4a2776f9d5a9af4 Tree-SHA512: 9ee584d260b5c224463318a51c2856a7c0e463be039fea072e5d5bab8898f0043b3930cf887a47aafd0f3447adb551b5e47a4e98ebdefc6cdb8e77edde0347b0 --- src/bitcoind.cpp | 8 ++++---- src/rpc/server.cpp | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index c85f1353c4..25c54be437 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2009-2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Copyright (c) 2014-2022 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -66,12 +66,12 @@ static bool AppInit(int argc, char* argv[]) // Process help and version before taking care about datadir if (HelpRequested(args) || args.IsArgSet("-version")) { - std::string strUsage = PACKAGE_NAME " Daemon version " + FormatFullVersion() + "\n"; + std::string strUsage = PACKAGE_NAME " version " + FormatFullVersion() + "\n"; if (args.IsArgSet("-version")) { strUsage += FormatParagraph(LicenseInfo()) + "\n"; } else { - strUsage += "\nUsage: dashd [options] Start " PACKAGE_NAME " Daemon\n"; + strUsage += "\nUsage: dashd [options] Start " PACKAGE_NAME "\n"; strUsage += "\n" + args.GetHelpMessage(); } @@ -131,7 +131,7 @@ static bool AppInit(int argc, char* argv[]) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdeprecated-declarations" #endif - tfm::format(std::cout, PACKAGE_NAME " daemon starting\n"); + tfm::format(std::cout, PACKAGE_NAME " starting\n"); // Daemonize if (daemon(1, 0)) { // don't chdir (1), do close FDs (0) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 9a0b03bdd7..a6f1c603ed 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -182,7 +182,7 @@ static RPCHelpMan stop() // Also accept the hidden 'wait' integer argument (milliseconds) // For instance, 'stop 1000' makes the call wait 1 second before returning // to the client (intended for testing) - "\nStop Dash Core server.", + "\nRequest a graceful shutdown of " PACKAGE_NAME ".", { {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", "", {}, /* hidden */ true}, }, From 0b30e0f8fe835eeb8602448540a787a6b948b172 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 5 Aug 2019 08:01:21 -0400 Subject: [PATCH 11/13] Merge #16197: net: Use mockable time for tx download /** * Dash specific comment * * Seems as event on_getdata works in our P2PInterface. * But somehow it's never called for test 'test_in_flight_max', * it may be due to bug in net_processing. * Due to that, part of functional tests is disabled */ fab365835639a3da03f8ad9a58a0db6c6c4c2314 [qa] Test that getdata requests work as expected (Suhas Daftuar) fa883ab35ad2d4328e35b1e855d0833740a6b910 net: Use mockable time for tx download (MarcoFalke) Pull request description: Two commits: * First commit changes to mockable time for tx download (refactoring, should only have an effect on regtest) * Second commit adds a test that uses mocktime to test tx download ACKs for top commit: laanwj: code review ACK 16197/commits/fab365835639a3da03f8ad9a58a0db6c6c4c2314 jamesob: ACK https://github.com/bitcoin/bitcoin/pull/16197/commits/fab365835639a3da03f8ad9a58a0db6c6c4c2314 Tree-SHA512: 3a64a3e283ec4bab1f6e506404b11f0a564a5b61d2a7508ae738a61f035e57220484c66e0ae47d847fe9f7e3ff5cc834909d7b34a9bbcea6abe01f8742806908 Co-authored-by: UdjinM6 --- test/functional/p2p_tx_download.py | 176 +++++++++++++++++++++++++++++ test/functional/test_runner.py | 1 + 2 files changed, 177 insertions(+) create mode 100755 test/functional/p2p_tx_download.py diff --git a/test/functional/p2p_tx_download.py b/test/functional/p2p_tx_download.py new file mode 100755 index 0000000000..b0e1e799f6 --- /dev/null +++ b/test/functional/p2p_tx_download.py @@ -0,0 +1,176 @@ +#!/usr/bin/env python3 +# Copyright (c) 2019 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +""" +Test transaction download behavior +""" + +from test_framework.messages import ( + CInv, + CTransaction, + FromHex, + MSG_TX, + MSG_TYPE_MASK, + msg_inv, + msg_notfound, +) +from test_framework.mininode import ( + P2PInterface, + mininode_lock, +) +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import ( + assert_equal, + wait_until, +) +from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE + + +class TestP2PConn(P2PInterface): + def __init__(self): + super().__init__() + self.tx_getdata_count = 0 + + def on_getdata(self, message): + for i in message.inv: + if i.type & MSG_TYPE_MASK == MSG_TX: + self.tx_getdata_count += 1 + + +# Constants from net_processing +GETDATA_TX_INTERVAL = 60 # seconds +MAX_GETDATA_RANDOM_DELAY = 2 # seconds +INBOUND_PEER_TX_DELAY = 2 # seconds +MAX_GETDATA_IN_FLIGHT = 100 +TX_EXPIRY_INTERVAL = GETDATA_TX_INTERVAL * 10 + +# Python test constants +NUM_INBOUND = 10 +MAX_GETDATA_INBOUND_WAIT = GETDATA_TX_INTERVAL + MAX_GETDATA_RANDOM_DELAY + INBOUND_PEER_TX_DELAY + + +class TxDownloadTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = False + self.num_nodes = 2 + + def test_tx_requests(self): + self.log.info("Test that we request transactions from all our peers, eventually") + + txid = 0xdeadbeef + + self.log.info("Announce the txid from each incoming peer to node 0") + msg = msg_inv([CInv(t=1, h=txid)]) + for p in self.nodes[0].p2ps: + p.send_message(msg) + p.sync_with_ping() + + outstanding_peer_index = [i for i in range(len(self.nodes[0].p2ps))] + + def getdata_found(peer_index): + p = self.nodes[0].p2ps[peer_index] + with mininode_lock: + return p.last_message.get("getdata") and p.last_message["getdata"].inv[-1].hash == txid + + while outstanding_peer_index: + self.bump_mocktime(MAX_GETDATA_INBOUND_WAIT) + wait_until(lambda: any(getdata_found(i) for i in outstanding_peer_index)) + for i in outstanding_peer_index: + if getdata_found(i): + outstanding_peer_index.remove(i) + + self.log.info("All outstanding peers received a getdata") + + def test_inv_block(self): + self.log.info("Generate a transaction on node 0") + tx = self.nodes[0].createrawtransaction( + inputs=[{ # coinbase + "txid": self.nodes[0].getblock(self.nodes[0].getblockhash(1))['tx'][0], + "vout": 0 + }], + outputs={ADDRESS_BCRT1_UNSPENDABLE: 500 - 0.00025}, + ) + tx = self.nodes[0].signrawtransactionwithkey( + hexstring=tx, + privkeys=[self.nodes[0].get_deterministic_priv_key().key], + )['hex'] + ctx = FromHex(CTransaction(), tx) + txid = int(ctx.rehash(), 16) + + self.log.info( + "Announce the transaction to all nodes from all {} incoming peers, but never send it".format(NUM_INBOUND)) + msg = msg_inv([CInv(t=1, h=txid)]) + for p in self.peers: + p.send_message(msg) + p.sync_with_ping() + self.bump_mocktime(1) + + self.log.info("Put the tx in node 0's mempool") + self.nodes[0].sendrawtransaction(tx) + + # Since node 1 is connected outbound to an honest peer (node 0), it + # should get the tx within a timeout. (Assuming that node 0 + # announced the tx within the timeout) + # The timeout is the sum of + # * the worst case until the tx is first requested from an inbound + # peer, plus + # * the first time it is re-requested from the outbound peer, plus + # * 2 seconds to avoid races + timeout = 2 + (MAX_GETDATA_RANDOM_DELAY + INBOUND_PEER_TX_DELAY) + ( + GETDATA_TX_INTERVAL + MAX_GETDATA_RANDOM_DELAY) + self.log.info("Tx should be received at node 1 after {} seconds".format(timeout)) + self.sync_mempools(timeout=timeout) + + def test_in_flight_max(self): + self.log.info("Test that we don't request more than {} transactions from any peer, every {} minutes".format( + MAX_GETDATA_IN_FLIGHT, TX_EXPIRY_INTERVAL / 60)) + txids = [i for i in range(MAX_GETDATA_IN_FLIGHT + 2)] + + p = self.nodes[0].p2ps[0] + + with mininode_lock: + p.tx_getdata_count = 0 + + p.send_message(msg_inv([CInv(t=1, h=i) for i in txids])) + + def wait_for_tx_getdata(target): + self.bump_mocktime(1) + return p.tx_getdata_count >= target + + wait_until(lambda: wait_for_tx_getdata(MAX_GETDATA_IN_FLIGHT), lock=mininode_lock) + + with mininode_lock: + assert_equal(p.tx_getdata_count, MAX_GETDATA_IN_FLIGHT) + + self.log.info("Now check that if we send a NOTFOUND for a transaction, we'll get one more request") + p.send_message(msg_notfound(vec=[CInv(t=1, h=txids[0])])) + wait_until(lambda: wait_for_tx_getdata(MAX_GETDATA_IN_FLIGHT + 1), timeout=10, lock=mininode_lock) + with mininode_lock: + assert_equal(p.tx_getdata_count, MAX_GETDATA_IN_FLIGHT + 1) + + WAIT_TIME = TX_EXPIRY_INTERVAL // 2 + TX_EXPIRY_INTERVAL + self.log.info("if we wait about {} minutes, we should eventually get more requests".format(WAIT_TIME / 60)) + self.bump_mocktime(WAIT_TIME) + wait_until(lambda: wait_for_tx_getdata(MAX_GETDATA_IN_FLIGHT + 2)) + + def run_test(self): + # Setup the p2p connections + self.peers = [] + for node in self.nodes: + for i in range(NUM_INBOUND): + self.peers.append(node.add_p2p_connection(TestP2PConn())) + + self.log.info("Nodes are setup with {} incoming connections each".format(NUM_INBOUND)) + + # Test the in-flight max first, because we want no transactions in + # flight ahead of this test. + self.test_in_flight_max() + + self.test_inv_block() + + self.test_tx_requests() + + +if __name__ == '__main__': + TxDownloadTest().main() diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 28b9a02be0..312d4c1d24 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -103,6 +103,7 @@ BASE_SCRIPTS = [ 'p2p_timeouts.py', 'feature_bip68_sequence.py', 'mempool_updatefromblock.py', + 'p2p_tx_download.py', 'wallet_dump.py', 'wallet_listtransactions.py', 'feature_multikeysporks.py', From 234d47294444364f37ebfe8ba1b5bdca037a774d Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Sat, 14 Sep 2019 21:42:34 +1200 Subject: [PATCH 12/13] Merge #16866: wallet: Rename 'decode' argument in gettransaction method to 'verbose' 7dee8f48088c75ab0e51be60679505f8ce570919 [wallet] Rename 'decode' argument in gettransaction method to 'verbose' (John Newbery) Pull request description: This makes the RPC method consistent with other RPC methods that have a 'verbose' option. Change the name of the return object from 'decoded' to details. Update help text. ACKs for top commit: promag: ACK 7dee8f48088c75ab0e51be60679505f8ce570919. meshcollider: Code review ACK 7dee8f48088c75ab0e51be60679505f8ce570919 0xB10C: ACK 7dee8f48088c75ab0e51be60679505f8ce570919: reviewed code Tree-SHA512: a3a62265c8e6e914591f3b3b9f9dd4f42240dc8dab9cbac6ed8d8b8319b6cc847db2ad1689d5440c162e0698f31e39fc6b868ed918b2f62879d61b9865cae66b --- doc/release-notes-16185.md | 2 +- src/rpc/client.cpp | 2 +- src/wallet/rpcwallet.cpp | 16 ++++++++-------- test/functional/wallet_basic.py | 8 ++++---- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/doc/release-notes-16185.md b/doc/release-notes-16185.md index eeeb951e5b..14e4004f72 100644 --- a/doc/release-notes-16185.md +++ b/doc/release-notes-16185.md @@ -1,3 +1,3 @@ RPC changes ----------- -The `gettransaction` RPC now accepts a third (boolean) argument `decode`. If set to `true`, a new `decoded` field will be added to the response containing the decoded transaction. +The `gettransaction` RPC now accepts a third (boolean) argument `verbose`. If set to `true`, a new `details` field will be added to the response containing additional transaction details. diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index 424d75f462..bb1afe616a 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -105,7 +105,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "getchaintxstats", 0, "nblocks" }, { "getmerkleblocks", 2, "count" }, { "gettransaction", 1, "include_watchonly" }, - { "gettransaction", 2, "decode" }, + { "gettransaction", 2, "verbose" }, { "getrawtransaction", 1, "verbose" }, { "createrawtransaction", 0, "inputs" }, { "createrawtransaction", 1, "outputs" }, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index ca3e76a25c..15531540c3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1655,7 +1655,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"}, - {"decode", RPCArg::Type::BOOL, /* default */ "false", "Whether to add a field with the decoded transaction"}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", "Whether to add a field with additional transaction details"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", Cat(Cat>( @@ -1689,7 +1689,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, - {RPCResult::Type::OBJ, "decoded", /*optional=*/true, "The decoded transaction", + {RPCResult::Type::OBJ, "details", /*optional=*/true, "Additional transaction details", { {RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."}, }}, @@ -1720,7 +1720,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) filter |= ISMINE_WATCH_ONLY; } - bool decode_tx = request.params[2].isNull() ? false : request.params[2].get_bool(); + bool verbose = request.params[2].isNull() ? false : request.params[2].get_bool(); UniValue entry(UniValue::VOBJ); auto it = pwallet->mapWallet.find(hash); @@ -1747,10 +1747,10 @@ static UniValue gettransaction(const JSONRPCRequest& request) std::string strHex = EncodeHexTx(*wtx.tx); entry.pushKV("hex", strHex); - if (decode_tx) { - UniValue decoded(UniValue::VOBJ); - TxToUniv(*wtx.tx, uint256(), decoded, false); - entry.pushKV("decoded", decoded); + if (verbose) { + UniValue details(UniValue::VOBJ); + TxToUniv(*wtx.tx, uint256(), details, false); + entry.pushKV("details", details); } return entry; @@ -4106,7 +4106,7 @@ static const CRPCCommand commands[] = { "wallet", "getrawchangeaddress", &getrawchangeaddress, {} }, { "wallet", "getreceivedbyaddress", &getreceivedbyaddress, {"address","minconf","addlocked"} }, { "wallet", "getreceivedbylabel", &getreceivedbylabel, {"label","minconf","addlocked"} }, - { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","decode"} }, + { "wallet", "gettransaction", &gettransaction, {"txid","include_watchonly","verbose"} }, { "wallet", "getunconfirmedbalance", &getunconfirmedbalance, {} }, { "wallet", "getbalances", &getbalances, {} }, { "wallet", "getwalletinfo", &getwalletinfo, {} }, diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 7e9f0ab531..36ee0309b6 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -496,10 +496,10 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].setlabel(change, 'foobar') assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False) - # Test "decoded" field value in gettransaction response - self.log.info("Testing gettransaction decoding...") - tx = self.nodes[0].gettransaction(txid=txid, decode=True) - assert_equal(tx["decoded"], self.nodes[0].decoderawtransaction(tx["hex"])) + # Test "verbose" field value in gettransaction response + self.log.info("Testing verbose gettransaction...") + tx = self.nodes[0].gettransaction(txid=txid, verbose=True) + assert_equal(tx["details"], self.nodes[0].decoderawtransaction(tx["hex"])) if __name__ == '__main__': From e4fcb170cf57f4f7523fbedb71e6386e2ec56b14 Mon Sep 17 00:00:00 2001 From: Samuel Dobson Date: Mon, 16 Sep 2019 09:44:06 +1200 Subject: [PATCH 13/13] Merge #16873: rpc: fix regression in gettransaction 1b41c2c8a126ef4be183e1d800a17d85cab8837b test: improve gettransaction test coverage (Jon Atack) 0f34f54888f680bfbe7a29ac278636d7178a99bb rpc: fix regression in gettransaction (Jon Atack) Pull request description: Closes #16872. PR #16866 renamed the `decode` argument in gettransaction to `verbose` to make it more consistent with other RPC calls like getrawtransaction. However, it inadvertently overloaded the "details" field when `verbose` is passed. The result is that the original "details" field is no longer returned correctly, which seems to be a breaking API change. This PR: - takes the simplest path to restoring the "details" field by renaming the decoded one back to "decoded" while leaving the `verbose` argument for API consistency, which was the main intent of #16866, - addresses [this comment](https://github.com/bitcoin/bitcoin/pull/16185#discussion_r320740413) by mentioning in the RPC help that the new decoded field is equivalent to decoderawtransaction, and - updates the help, functional test, and release note. Reviewers, to test this manually, build and run `bitcoin-cli help gettransaction` and `bitcoin-cli gettransaction false true`, and verify that the command returns both `details` and `decoded` fields. ACKs for top commit: jnewbery: tACK 1b41c2c8a126ef4be183e1d800a17d85cab8837b Tree-SHA512: 287edd5db7ed58fe8b548975aba58628bd45ed708b28f40174f10a35a455d89f796fbf27430aa881fc376f47aabda8803f74d4d100683bd86577a02279091cf3 --- doc/release-notes-16185.md | 5 ++++- src/wallet/rpcwallet.cpp | 12 ++++++------ test/functional/wallet_basic.py | 33 ++++++++++++++++++++++++++++++--- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/doc/release-notes-16185.md b/doc/release-notes-16185.md index 14e4004f72..2567ebea40 100644 --- a/doc/release-notes-16185.md +++ b/doc/release-notes-16185.md @@ -1,3 +1,6 @@ RPC changes ----------- -The `gettransaction` RPC now accepts a third (boolean) argument `verbose`. If set to `true`, a new `details` field will be added to the response containing additional transaction details. +The `gettransaction` RPC now accepts a third (boolean) argument `verbose`. If +set to `true`, a new `decoded` field will be added to the response containing +the decoded transaction. This field is equivalent to RPC `decoderawtransaction`, +or RPC `getrawtransaction` when `verbose` is passed. diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 15531540c3..4bd5a4cc74 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1655,7 +1655,7 @@ static UniValue gettransaction(const JSONRPCRequest& request) { {"txid", RPCArg::Type::STR, RPCArg::Optional::NO, "The transaction id"}, {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Whether to include watch-only addresses in balance calculation and details[]"}, - {"verbose", RPCArg::Type::BOOL, /* default */ "false", "Whether to add a field with additional transaction details"}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", "Whether to include a `decoded` field containing the decoded transaction (equivalent to RPC decoderawtransaction)"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", Cat(Cat>( @@ -1689,9 +1689,9 @@ static UniValue gettransaction(const JSONRPCRequest& request) }}, }}, {RPCResult::Type::STR_HEX, "hex", "Raw data for transaction"}, - {RPCResult::Type::OBJ, "details", /*optional=*/true, "Additional transaction details", + {RPCResult::Type::OBJ, "decoded", /*optional=*/true, "the decoded transaction (only present when `verbose` is passed), equivalent to the", { - {RPCResult::Type::ELISION, "", "Equivalent to the RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."}, + {RPCResult::Type::ELISION, "", "RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed."}, }}, }), }, @@ -1748,9 +1748,9 @@ static UniValue gettransaction(const JSONRPCRequest& request) entry.pushKV("hex", strHex); if (verbose) { - UniValue details(UniValue::VOBJ); - TxToUniv(*wtx.tx, uint256(), details, false); - entry.pushKV("details", details); + UniValue decoded(UniValue::VOBJ); + TxToUniv(*wtx.tx, uint256(), decoded, false); + entry.pushKV("decoded", decoded); } return entry; diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 36ee0309b6..3ee5eabbda 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -496,10 +496,37 @@ class WalletTest(BitcoinTestFramework): self.nodes[0].setlabel(change, 'foobar') assert_equal(self.nodes[0].getaddressinfo(change)['ischange'], False) - # Test "verbose" field value in gettransaction response - self.log.info("Testing verbose gettransaction...") + # Test gettransaction response with different arguments. + self.log.info("Testing gettransaction response with different arguments...") + self.nodes[0].setlabel(change, 'baz') + baz = self.nodes[0].listtransactions(label="baz", count=1)[0] + expected_receive_vout = {"label": "baz", + "address": baz["address"], + "amount": baz["amount"], + "category": baz["category"], + "vout": baz["vout"]} + expected_fields = frozenset({'amount', 'chainlock', 'confirmations', 'details', 'fee', + 'instantlock', 'instantlock_internal', + 'hex', 'timereceived', 'time', 'trusted', 'txid', 'walletconflicts'}) + + verbose_field = "decoded" + expected_verbose_fields = expected_fields | {verbose_field} + + self.log.debug("Testing gettransaction response without verbose") + tx = self.nodes[0].gettransaction(txid=txid) + assert_equal(set([*tx]), expected_fields) + assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) + + self.log.debug("Testing gettransaction response with verbose set to False") + tx = self.nodes[0].gettransaction(txid=txid, verbose=False) + assert_equal(set([*tx]), expected_fields) + assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) + + self.log.debug("Testing gettransaction response with verbose set to True") tx = self.nodes[0].gettransaction(txid=txid, verbose=True) - assert_equal(tx["details"], self.nodes[0].decoderawtransaction(tx["hex"])) + assert_equal(set([*tx]), expected_verbose_fields) + assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) + assert_equal(tx[verbose_field], self.nodes[0].decoderawtransaction(tx["hex"])) if __name__ == '__main__':