diff --git a/doc/release-notes-18202.md b/doc/release-notes-18202.md new file mode 100644 index 0000000000000..f2bd870ac44ec --- /dev/null +++ b/doc/release-notes-18202.md @@ -0,0 +1,8 @@ +Low-level RPC Changes +--------------------- + +- To make RPC `sendtoaddress` more consistent with `sendmany` the following error + `sendtoaddress` codes were changed from `-4` to `-6`: + - Insufficient funds + - Fee estimation failed + - Transaction has too long of a mempool chain diff --git a/doc/release-notes-19202.md b/doc/release-notes-19202.md new file mode 100644 index 0000000000000..2eecc9e21ad9e --- /dev/null +++ b/doc/release-notes-19202.md @@ -0,0 +1,6 @@ +Updated settings +---------------- + +- The `-debug=db` logging category, which was deprecated in v0.18 and replaced by + `-debug=walletdb` to distinguish it from `coindb`, has been removed. (#6033) + diff --git a/doc/release-notes-19501.md b/doc/release-notes-19501.md new file mode 100644 index 0000000000000..960de655e097c --- /dev/null +++ b/doc/release-notes-19501.md @@ -0,0 +1,10 @@ + +New settings +------------ + +Wallet +------ + +- The `sendtoaddress` and `sendmany` RPCs accept an optional `verbose=True` + argument to also return the fee reason about the sent tx. (#6033) + diff --git a/doc/release-notes-19725.md b/doc/release-notes-19725.md new file mode 100644 index 0000000000000..1754c723fe7ab --- /dev/null +++ b/doc/release-notes-19725.md @@ -0,0 +1,10 @@ + +Updated RPCs +------------ + +- The `getpeerinfo` RPC no longer returns the `addnode` field by default. This + field will be fully removed in the next major release. It can be accessed + with the configuration option `-deprecatedrpc=getpeerinfo_addnode`. However, + it is recommended to instead use the `connection_type` field (it will return + `manual` when addnode is true). (#6033) + diff --git a/src/coinjoin/util.cpp b/src/coinjoin/util.cpp index 1fecbf8b165af..50a8e6727d94b 100644 --- a/src/coinjoin/util.cpp +++ b/src/coinjoin/util.cpp @@ -274,7 +274,8 @@ bool CTransactionBuilder::Commit(bilingual_str& strResult) CTransactionRef tx; { LOCK2(pwallet->cs_wallet, cs_main); - if (!pwallet->CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, strResult, coinControl)) { + FeeCalculation fee_calc_out; + if (!pwallet->CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, strResult, coinControl, fee_calc_out)) { return false; } } diff --git a/src/logging.cpp b/src/logging.cpp index 3f70d0c22ab87..32a3add84ad97 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -98,15 +98,7 @@ void BCLog::Logger::EnableCategory(BCLog::LogFlags flag) bool BCLog::Logger::EnableCategory(const std::string& str) { BCLog::LogFlags flag; - if (!GetLogCategory(flag, str)) { - if (str == "db") { - // DEPRECATION: Added in 0.20, should start returning an error in 0.21 - LogPrintf("Warning: logging category 'db' is deprecated, use 'walletdb' instead\n"); - EnableCategory(BCLog::WALLETDB); - return true; - } - return false; - } + if (!GetLogCategory(flag, str)) return false; EnableCategory(flag); return true; } diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index b6b05331cdf0d..5b0a49592213c 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -44,6 +44,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendtoaddress", 6, "use_cj" }, { "sendtoaddress", 7, "conf_target" }, { "sendtoaddress", 9, "avoid_reuse" }, + { "sendtoaddress", 10, "verbose"}, { "settxfee", 0, "amount" }, { "sethdseed", 0, "newkeypool" }, { "getreceivedbyaddress", 1, "minconf" }, @@ -89,6 +90,7 @@ static const CRPCConvertParam vRPCConvertParams[] = { "sendmany", 6, "use_is" }, { "sendmany", 7, "use_cj" }, { "sendmany", 8, "conf_target" }, + { "sendmany", 10, "verbose" }, { "deriveaddresses", 1, "range" }, { "scantxoutset", 1, "scanobjects" }, { "addmultisigaddress", 0, "nrequired" }, diff --git a/src/rpc/evo.cpp b/src/rpc/evo.cpp index ce8b4e844e86e..d5fa516875640 100644 --- a/src/rpc/evo.cpp +++ b/src/rpc/evo.cpp @@ -270,7 +270,8 @@ static void FundSpecialTx(CWallet* pwallet, CMutableTransaction& tx, const Speci int nChangePos = -1; bilingual_str strFailReason; - if (!pwallet->CreateTransaction(vecSend, newTx, nFee, nChangePos, strFailReason, coinControl, false, tx.vExtraPayload.size())) { + FeeCalculation fee_calc_out; + if (!pwallet->CreateTransaction(vecSend, newTx, nFee, nChangePos, strFailReason, coinControl, fee_calc_out, false, tx.vExtraPayload.size())) { throw JSONRPCError(RPC_INTERNAL_ERROR, strFailReason.original); } diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 2dd68d3dbad39..e9f6d46f48bb8 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -145,7 +145,8 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::NUM, "version", "The peer version, such as 70001"}, {RPCResult::Type::STR, "subver", "The string version"}, {RPCResult::Type::BOOL, "inbound", "Inbound (true) or Outbound (false)"}, - {RPCResult::Type::BOOL, "addnode", "Whether connection was due to addnode/-connect or if it was an automatic/inbound connection"}, + {RPCResult::Type::BOOL, "addnode", "Whether connection was due to addnode/-connect or if it was an automatic/inbound connection\n" + "(DEPRECATED, returned only if the config option -deprecatedrpc=getpeerinfo_addnode is passed)"}, {RPCResult::Type::BOOL, "masternode", "Whether connection was due to masternode connection attempt"}, {RPCResult::Type::NUM, "banscore", "The ban score (DEPRECATED, returned only if config option -deprecatedrpc=banscore is passed)"}, {RPCResult::Type::NUM, "startingheight", "The starting height (block) of the peer"}, @@ -158,7 +159,8 @@ static RPCHelpMan getpeerinfo() {RPCResult::Type::BOOL, "addr_relay_enabled", "Whether we participate in address relay with this peer"}, {RPCResult::Type::NUM, "addr_processed", "The total number of addresses processed, excluding those dropped due to rate limiting"}, {RPCResult::Type::NUM, "addr_rate_limited", "The total number of addresses dropped due to rate limiting"}, - {RPCResult::Type::BOOL, "whitelisted", "Whether the peer is whitelisted"}, + {RPCResult::Type::BOOL, "whitelisted", /* optional */ true, "Whether the peer is whitelisted with default permissions\n" + "(DEPRECATED, returned only if config option -deprecatedrpc=whitelisted is passed)"}, {RPCResult::Type::ARR, "permissions", "Any special permissions that have been granted to this peer", { {RPCResult::Type::STR, "permission_type", Join(NET_PERMISSIONS_DOC, ",\n") + ".\n"}, @@ -242,7 +244,10 @@ static RPCHelpMan getpeerinfo() // their ver message. obj.pushKV("subver", stats.cleanSubVer); obj.pushKV("inbound", stats.fInbound); - obj.pushKV("addnode", stats.m_manual_connection); + if (IsDeprecatedRPCEnabled("getpeerinfo_addnode")) { + // addnode is deprecated in v21 for removal in v22 + obj.pushKV("addnode", stats.m_manual_connection); + } obj.pushKV("masternode", stats.m_masternode_connection); if (fStateStats) { if (IsDeprecatedRPCEnabled("banscore")) { @@ -262,7 +267,10 @@ static RPCHelpMan getpeerinfo() obj.pushKV("addr_processed", statestats.m_addr_processed); obj.pushKV("addr_rate_limited", statestats.m_addr_rate_limited); } - obj.pushKV("whitelisted", stats.m_legacyWhitelisted); + if (IsDeprecatedRPCEnabled("whitelisted")) { + // whitelisted is deprecated in v0.21 for removal in v0.22 + obj.pushKV("whitelisted", stats.m_legacyWhitelisted); + } UniValue permissions(UniValue::VARR); for (const auto& permission : NetPermissions::ToStrings(stats.m_permissionFlags)) { permissions.push_back(permission); diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 3df562d1ad9ff..c1e11ab09fa23 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -271,8 +271,9 @@ class WalletImpl : public Wallet LOCK(m_wallet->cs_wallet); ReserveDestination m_dest(m_wallet.get()); CTransactionRef tx; + FeeCalculation fee_calc_out; if (!m_wallet->CreateTransaction(recipients, tx, fee, change_pos, - fail_reason, coin_control, sign)) { + fail_reason, coin_control, fee_calc_out, sign)) { return {}; } return tx; diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 98da9d41bc156..4c8135baa4106 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -352,40 +352,62 @@ static RPCHelpMan setlabel() }; } +void ParseRecipients(const UniValue& address_amounts, const UniValue& subtract_fee_outputs, std::vector &recipients) { + std::set destinations; + int i = 0; + for (const std::string& address: address_amounts.getKeys()) { + CTxDestination dest = DecodeDestination(address); + if (!IsValidDestination(dest)) { + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + address); + } -static CTransactionRef SendMoney(CWallet* const pwallet, const CTxDestination& address, CAmount nValue, bool fSubtractFeeFromAmount, const CCoinControl& coin_control, mapValue_t mapValue) -{ - CAmount curBalance = pwallet->GetBalance(0, coin_control.m_avoid_address_reuse).m_mine_trusted; + if (destinations.count(dest)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + address); + } + destinations.insert(dest); + + CScript script_pub_key = GetScriptForDestination(dest); + CAmount amount = AmountFromValue(address_amounts[i++]); + + bool subtract_fee = false; + for (unsigned int idx = 0; idx < subtract_fee_outputs.size(); idx++) { + const UniValue& addr = subtract_fee_outputs[idx]; + if (addr.get_str() == address) { + subtract_fee = true; + } + } - // Check amount - if (nValue <= 0) - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid amount"); + CRecipient recipient = {script_pub_key, amount, subtract_fee}; + recipients.push_back(recipient); + } +} - if (nValue > curBalance) - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds"); +UniValue SendMoney(CWallet* const pwallet, const CCoinControl &coin_control, std::vector &recipients, mapValue_t map_value, bool verbose) +{ + EnsureWalletIsUnlocked(pwallet); if (coin_control.IsUsingCoinJoin()) { - mapValue["DS"] = "1"; + map_value["DS"] = "1"; } - // Parse Dash address - CScript scriptPubKey = GetScriptForDestination(address); - - // Create and send the transaction + // Send CAmount nFeeRequired = 0; - bilingual_str error; - std::vector vecSend; int nChangePosRet = -1; - CRecipient recipient = {scriptPubKey, nValue, fSubtractFeeFromAmount}; - vecSend.push_back(recipient); + bilingual_str error; CTransactionRef tx; - if (!pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, error, coin_control)) { - if (!fSubtractFeeFromAmount && nValue + nFeeRequired > curBalance) - error = strprintf(Untranslated("Error: This transaction requires a transaction fee of at least %s"), FormatMoney(nFeeRequired)); - throw JSONRPCError(RPC_WALLET_ERROR, error.original); + FeeCalculation fee_calc_out; + bool fCreated = pwallet->CreateTransaction(recipients, tx, nFeeRequired, nChangePosRet, error, coin_control, fee_calc_out, !pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)); + if (!fCreated) { + throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); + } + pwallet->CommitTransaction(tx, std::move(map_value), {} /* orderForm */); + if (verbose) { + UniValue entry(UniValue::VOBJ); + entry.pushKV("txid", tx->GetHash().GetHex()); + entry.pushKV("fee_reason", StringForFeeReason(fee_calc_out.reason)); + return entry; } - pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */); - return tx; + return tx->GetHash().GetHex(); } static RPCHelpMan sendtoaddress() @@ -397,12 +419,12 @@ static RPCHelpMan sendtoaddress() {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Dash address to send to."}, {"amount", RPCArg::Type::AMOUNT, RPCArg::Optional::NO, "The amount in " + CURRENCY_UNIT + " to send. eg 0.1"}, {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment used to store what the transaction is for.\n" - " This is not part of the transaction, just kept in your wallet."}, + "This is not part of the transaction, just kept in your wallet."}, {"comment_to", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment to store the name of the person or organization\n" - " to which you're sending the transaction. This is not part of the \n" - " transaction, just kept in your wallet."}, + "to which you're sending the transaction. This is not part of the \n" + "transaction, just kept in your wallet."}, {"subtractfeefromamount", RPCArg::Type::BOOL, /* default */ "false", "The fee will be deducted from the amount being sent.\n" - " The recipient will receive less amount of Dash than you enter in the amount field."}, + "The recipient will receive less amount of Dash than you enter in the amount field."}, {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"}, {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"}, {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, @@ -410,9 +432,19 @@ static RPCHelpMan sendtoaddress() " \"" + FeeModes("\"\n\"") + "\""}, {"avoid_reuse", RPCArg::Type::BOOL, /* default */ "true", "(only available if avoid_reuse wallet flag is set) Avoid spending from dirty addresses; addresses are considered\n" " dirty if they have previously been used in a transaction."}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra information about the transaction."}, }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id." + { + RPCResult{"if verbose is not set or set to false", + RPCResult::Type::STR_HEX, "txid", "The transaction id." + }, + RPCResult{"if verbose is set to true", + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, + {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + }, + }, }, RPCExamples{ HelpExampleCli("sendtoaddress", "\"" + EXAMPLE_ADDRESS[0] + "\" 0.1") @@ -434,16 +466,6 @@ static RPCHelpMan sendtoaddress() LOCK(pwallet->cs_wallet); - CTxDestination dest = DecodeDestination(request.params[0].get_str()); - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid address"); - } - - // Amount - CAmount nAmount = AmountFromValue(request.params[1]); - if (nAmount <= 0) - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); - // Wallet comments mapValue_t mapValue; if (!request.params[2].isNull() && !request.params[2].get_str().empty()) @@ -471,8 +493,19 @@ static RPCHelpMan sendtoaddress() EnsureWalletIsUnlocked(pwallet); - CTransactionRef tx = SendMoney(pwallet, dest, nAmount, fSubtractFeeFromAmount, coin_control, std::move(mapValue)); - return tx->GetHash().GetHex(); + UniValue address_amounts(UniValue::VOBJ); + const std::string address = request.params[0].get_str(); + address_amounts.pushKV(address, request.params[1]); + UniValue subtractFeeFromAmount(UniValue::VARR); + if (fSubtractFeeFromAmount) { + subtractFeeFromAmount.push_back(address); + } + + std::vector recipients; + ParseRecipients(address_amounts, subtractFeeFromAmount, recipients); + bool verbose = request.params[10].isNull() ? false: request.params[10].get_bool(); + + return SendMoney(pwallet, coin_control, recipients, mapValue, verbose); }, }; } @@ -890,9 +923,9 @@ static RPCHelpMan sendmany() {"addlocked", RPCArg::Type::BOOL, RPCArg::Optional::OMITTED_NAMED_ARG, "Ignored dummy value"}, {"comment", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, "A comment"}, {"subtractfeefrom", RPCArg::Type::ARR, RPCArg::Optional::OMITTED_NAMED_ARG, "The addresses.\n" - " The fee will be equally deducted from the amount of each selected address.\n" - " Those recipients will receive less Dash than you enter in their corresponding amount field.\n" - " If no addresses are specified here, the sender pays the fee.", + "The fee will be equally deducted from the amount of each selected address.\n" + "Those recipients will receive less Dash than you enter in their corresponding amount field.\n" + "If no addresses are specified here, the sender pays the fee.", { {"address", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Subtract fee from this address"}, }, @@ -902,11 +935,22 @@ static RPCHelpMan sendmany() {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks), or fee rate (for " + CURRENCY_UNIT + "/kB or " + CURRENCY_ATOM + "/B estimate modes)"}, {"estimate_mode", RPCArg::Type::STR, /* default */ "unset", std::string() + "The fee estimate mode, must be one of (case insensitive):\n" " \"" + FeeModes("\"\n\"") + "\""}, + {"verbose", RPCArg::Type::BOOL, /* default */ "false", "If true, return extra infomration about the transaction."}, + }, + { + RPCResult{"if verbose is not set or set to false", + RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" + "the number of addresses." + }, + RPCResult{"if verbose is set to true", + RPCResult::Type::OBJ, "", "", + { + {RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" + "the number of addresses."}, + {RPCResult::Type::STR, "fee reason", "The transaction fee reason."} + }, + }, }, - RPCResult{ - RPCResult::Type::STR_HEX, "txid", "The transaction id for the send. Only 1 transaction is created regardless of\n" - "the number of addresses." - }, RPCExamples{ "\nSend two amounts to two different addresses:\n" + HelpExampleCli("sendmany", "\"\" \"{\\\"" + EXAMPLE_ADDRESS[0] + "\\\":0.01,\\\"" + EXAMPLE_ADDRESS[1] + "\\\":0.02}\"") + @@ -935,9 +979,9 @@ static RPCHelpMan sendmany() if (!request.params[4].isNull() && !request.params[4].get_str().empty()) mapValue["comment"] = request.params[4].get_str(); - UniValue subtractFeeFrom(UniValue::VARR); + UniValue subtractFeeFromAmount(UniValue::VARR); if (!request.params[5].isNull()) - subtractFeeFrom = request.params[5].get_array(); + subtractFeeFromAmount = request.params[5].get_array(); // request.params[6] ("use_is") is deprecated and not used here @@ -949,53 +993,11 @@ static RPCHelpMan sendmany() SetFeeEstimateMode(pwallet, coin_control, request.params[9], request.params[8]); - if (coin_control.IsUsingCoinJoin()) { - mapValue["DS"] = "1"; - } - - std::set destinations; - std::vector vecSend; - - std::vector keys = sendTo.getKeys(); - for (const std::string& name_ : keys) { - CTxDestination dest = DecodeDestination(name_); - if (!IsValidDestination(dest)) { - throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, std::string("Invalid Dash address: ") + name_); - } - - if (destinations.count(dest)) { - throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated address: ") + name_); - } - destinations.insert(dest); - - CScript scriptPubKey = GetScriptForDestination(dest); - CAmount nAmount = AmountFromValue(sendTo[name_]); - if (nAmount <= 0) - throw JSONRPCError(RPC_TYPE_ERROR, "Invalid amount for send"); + std::vector recipients; + ParseRecipients(sendTo, subtractFeeFromAmount, recipients); + bool verbose = request.params[10].isNull() ? false : request.params[10].get_bool(); - bool fSubtractFeeFromAmount = false; - for (unsigned int idx = 0; idx < subtractFeeFrom.size(); idx++) { - const UniValue& addr = subtractFeeFrom[idx]; - if (addr.get_str() == name_) - fSubtractFeeFromAmount = true; - } - - CRecipient recipient = {scriptPubKey, nAmount, fSubtractFeeFromAmount}; - vecSend.push_back(recipient); - } - - EnsureWalletIsUnlocked(pwallet); - - // Send - CAmount nFeeRequired = 0; - int nChangePosRet = -1; - bilingual_str error; - CTransactionRef tx; - bool fCreated = pwallet->CreateTransaction(vecSend, tx, nFeeRequired, nChangePosRet, error, coin_control); - if (!fCreated) - throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, error.original); - pwallet->CommitTransaction(tx, std::move(mapValue), {} /* orderForm */); - return tx->GetHash().GetHex(); + return SendMoney(pwallet, coin_control, recipients, std::move(mapValue), verbose); }, }; } @@ -1580,7 +1582,7 @@ static RPCHelpMan listsinceblock() {"target_confirmations", RPCArg::Type::NUM, /* default */ "1", "Return the nth block hash from the main chain. e.g. 1 would mean the best block hash. Note: this is not used as a filter, but only affects [lastblock] in the return value"}, {"include_watchonly", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Include transactions to watch-only addresses (see 'importaddress')"}, {"include_removed", RPCArg::Type::BOOL, /* default */ "true", "Show transactions that were removed due to a reorg in the \"removed\" array\n" - " (not guaranteed to work on pruned nodes)"}, + "(not guaranteed to work on pruned nodes)"}, }, RPCResult{ RPCResult::Type::OBJ, "", "", @@ -3189,7 +3191,7 @@ static RPCHelpMan listunspent() }, }, {"include_unsafe", RPCArg::Type::BOOL, /* default */ "true", "Include outputs that are not safe to spend\n" - " See description of \"safe\" attribute below."}, + "See description of \"safe\" attribute below."}, {"query_options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "JSON with query options", { {"minimumAmount", RPCArg::Type::AMOUNT, /* default */ "0", "Minimum value of each UTXO in " + CURRENCY_UNIT + ""}, @@ -3197,8 +3199,8 @@ static RPCHelpMan listunspent() {"maximumCount", RPCArg::Type::NUM, /* default */ "unlimited", "Maximum number of UTXOs"}, {"minimumSumAmount", RPCArg::Type::AMOUNT, /* default */ "unlimited", "Minimum sum value of all UTXOs in " + CURRENCY_UNIT + ""}, {"coinType", RPCArg::Type::NUM, /* default */ "0", "Filter coinTypes as follows:\n" - " 0=ALL_COINS, 1=ONLY_FULLY_MIXED, 2=ONLY_READY_TO_MIX, 3=ONLY_NONDENOMINATED,\n" - " 4=ONLY_MASTERNODE_COLLATERAL, 5=ONLY_COINJOIN_COLLATERAL" }, + "0=ALL_COINS, 1=ONLY_FULLY_MIXED, 2=ONLY_READY_TO_MIX, 3=ONLY_NONDENOMINATED,\n" + "4=ONLY_MASTERNODE_COLLATERAL, 5=ONLY_COINJOIN_COLLATERAL" }, }, "query_options"}, }, @@ -3220,7 +3222,7 @@ static RPCHelpMan listunspent() {RPCResult::Type::STR, "desc", "(only when solvable) A descriptor for spending this output"}, {RPCResult::Type::BOOL, "reused", /* optional*/ true, "(only present if avoid_reuse is set) Whether this output is reused/dirty (sent to an address that was previously spent from)"}, {RPCResult::Type::BOOL, "safe", "Whether this output is considered safe to spend. Unconfirmed transactions" - " from outside keys and unconfirmed replacement transactions are considered unsafe\n" + "from outside keys and unconfirmed replacement transactions are considered unsafe\n" "and are not eligible for spending by fundrawtransaction and sendtoaddress."}, {RPCResult::Type::NUM, "coinjoin_rounds", "The number of CoinJoin rounds"}, }}, @@ -3536,9 +3538,9 @@ static RPCHelpMan fundrawtransaction() {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set: makes wallet determine the fee", "Set a specific fee rate in " + CURRENCY_UNIT + "/kB"}, {"subtractFeeFromOutputs", RPCArg::Type::ARR, /* default */ "empty array", "The integers.\n" - " The fee will be equally deducted from the amount of each specified output.\n" - " Those recipients will receive less Dash than you enter in their corresponding amount field.\n" - " If no outputs are specified here, the sender pays the fee.", + "The fee will be equally deducted from the amount of each specified output.\n" + "Those recipients will receive less Dash than you enter in their corresponding amount field.\n" + "If no outputs are specified here, the sender pays the fee.", { {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, @@ -4175,7 +4177,7 @@ static RPCHelpMan send() "\nEXPERIMENTAL warning: this call may be changed in future releases.\n" "\nSend a transaction.\n", { - {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "A JSON array with outputs (key-value pairs), where none of the keys are duplicated.\n" + {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs (key-value pairs), where none of the keys are duplicated.\n" "That is, each address can only appear once and there can only be one 'data' object.\n" "For convenience, a dictionary, which holds the key-value pairs directly, is also accepted.", { @@ -4513,7 +4515,7 @@ static RPCHelpMan walletcreatefundedpsbt() {"outputs", RPCArg::Type::ARR, RPCArg::Optional::NO, "The outputs (key-value pairs), where none of the keys are duplicated.\n" "That is, each address can only appear once and there can only be one 'data' object.\n" "For compatibility reasons, a dictionary, which holds the key-value pairs directly, is also\n" - " accepted as second parameter.", + "accepted as second parameter.", { {"", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED, "", { @@ -4537,9 +4539,9 @@ static RPCHelpMan walletcreatefundedpsbt() {"lockUnspents", RPCArg::Type::BOOL, /* default */ "false", "Lock selected unspent outputs"}, {"feeRate", RPCArg::Type::AMOUNT, /* default */ "not set: makes wallet determine the fee", "Set a specific fee rate in " + CURRENCY_UNIT + "/kB"}, {"subtractFeeFromOutputs", RPCArg::Type::ARR, /* default */ "empty array", "The outputs to subtract the fee from.\n" - " The fee will be equally deducted from the amount of each specified output.\n" - " Those recipients will receive less Dash than you enter in their corresponding amount field.\n" - " If no outputs are specified here, the sender pays the fee.", + "The fee will be equally deducted from the amount of each specified output.\n" + "Those recipients will receive less Dash than you enter in their corresponding amount field.\n" + "If no outputs are specified here, the sender pays the fee.", { {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, }, @@ -4750,8 +4752,8 @@ static const CRPCCommand commands[] = { "wallet", "removeprunedfunds", &removeprunedfunds, {"txid"} }, { "wallet", "rescanblockchain", &rescanblockchain, {"start_height", "stop_height"} }, { "wallet", "send", &send, {"outputs","conf_target","estimate_mode","options"} }, - { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","addlocked","comment","subtractfeefrom","use_is","use_cj","conf_target","estimate_mode"} }, - { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","use_is","use_cj","conf_target","estimate_mode", "avoid_reuse"} }, + { "wallet", "sendmany", &sendmany, {"dummy","amounts","minconf","addlocked","comment","subtractfeefrom","use_is","use_cj","conf_target","estimate_mode","verbose"} }, + { "wallet", "sendtoaddress", &sendtoaddress, {"address","amount","comment","comment_to","subtractfeefromamount","use_is","use_cj","conf_target","estimate_mode", "avoid_reuse","verbose"} }, { "wallet", "sethdseed", &sethdseed, {"newkeypool","seed"} }, { "wallet", "setcoinjoinrounds", &setcoinjoinrounds, {"rounds"} }, { "wallet", "setcoinjoinamount", &setcoinjoinamount, {"amount"} }, diff --git a/src/wallet/test/coinjoin_tests.cpp b/src/wallet/test/coinjoin_tests.cpp index e53dc78eb8974..94d3f4d54cb71 100644 --- a/src/wallet/test/coinjoin_tests.cpp +++ b/src/wallet/test/coinjoin_tests.cpp @@ -185,7 +185,8 @@ class CTransactionBuilderTestSetup : public TestChain100Setup } for (CAmount nAmount : vecAmounts) { CTransactionRef tx; - BOOST_CHECK(wallet->CreateTransaction({{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, nFeeRet, nChangePosRet, strError, coinControl)); + FeeCalculation fee_calc_out; + BOOST_CHECK(wallet->CreateTransaction({{GetScriptForDestination(tallyItem.txdest), nAmount, false}}, tx, nFeeRet, nChangePosRet, strError, coinControl, fee_calc_out)); { LOCK2(wallet->cs_wallet, cs_main); wallet->CommitTransaction(tx, {}, {}); diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 0115d90503c14..d27d72b2eec7c 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -602,7 +602,8 @@ class ListCoinsTestingSetup : public TestChain100Setup int changePos = -1; bilingual_str error; CCoinControl dummy; - BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy)); + FeeCalculation fee_calc_out; + BOOST_CHECK(wallet->CreateTransaction({recipient}, tx, fee, changePos, error, dummy, fee_calc_out)); wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; { @@ -756,7 +757,8 @@ class CreateTransactionTestSetup : public TestChain100Setup int nChangePos = nChangePosRequest; bilingual_str strError; - bool fCreationSucceeded = wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePos, strError, coinControl); + FeeCalculation fee_calc_out; + bool fCreationSucceeded = wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePos, strError, coinControl, fee_calc_out); bool fHitMaxTries = strError.original == strExceededMaxTries; // This should never happen. if (fHitMaxTries) { @@ -808,7 +810,8 @@ class CreateTransactionTestSetup : public TestChain100Setup int nChangePosRet = -1; bilingual_str strError; CCoinControl coinControl; - BOOST_CHECK(wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePosRet, strError, coinControl)); + FeeCalculation fee_calc_out; + BOOST_CHECK(wallet->CreateTransaction(GetRecipients(vecEntries), tx, nFeeRet, nChangePosRet, strError, coinControl, fee_calc_out)); wallet->CommitTransaction(tx, {}, {}); CMutableTransaction blocktx; { @@ -1147,10 +1150,11 @@ BOOST_FIXTURE_TEST_CASE(select_coins_grouped_by_addresses, ListCoinsTestingSetup int changePos = -1; bilingual_str error; CCoinControl dummy; + FeeCalculation fee_calc_out; BOOST_CHECK(wallet->CreateTransaction({CRecipient{GetScriptForRawPubKey({}), 2 * COIN, true /* subtract fee */}}, - tx1, fee, changePos, error, dummy)); + tx1, fee, changePos, error, dummy, fee_calc_out)); BOOST_CHECK(wallet->CreateTransaction({CRecipient{GetScriptForRawPubKey({}), 1 * COIN, true /* subtract fee */}}, - tx2, fee, changePos, error, dummy)); + tx2, fee, changePos, error, dummy, fee_calc_out)); wallet->CommitTransaction(tx1, {}, {}); BOOST_CHECK_EQUAL(wallet->GetAvailableBalance(), 0); CreateAndProcessBlock({CMutableTransaction(*tx2)}, GetScriptForRawPubKey({})); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 880c2993d3bdc..44a9335e69542 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3090,7 +3090,8 @@ bool CWallet::FundTransaction(CMutableTransaction& tx, CAmount& nFeeRet, int& nC LOCK(cs_wallet); CTransactionRef tx_new; - if (!CreateTransaction(vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, false, tx.vExtraPayload.size())) { + FeeCalculation fee_calc_out; + if (!CreateTransaction(vecSend, tx_new, nFeeRet, nChangePosInOut, error, coinControl, fee_calc_out, false, tx.vExtraPayload.size())) { return false; } @@ -3397,7 +3398,8 @@ bool CWallet::GetBudgetSystemCollateralTX(CTransactionRef& tx, uint256 hash, CAm if (!outpoint.IsNull()) { coinControl.Select(outpoint); } - bool success = CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, error, coinControl); + FeeCalculation fee_calc_out; + bool success = CreateTransaction(vecSend, tx, nFeeRet, nChangePosRet, error, coinControl, fee_calc_out); if(!success){ WalletLogPrintf("CWallet::GetBudgetSystemCollateralTX -- Error: %s\n", error.original); return false; @@ -3413,6 +3415,7 @@ bool CWallet::CreateTransactionInternal( int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, + FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize) { @@ -3771,6 +3774,7 @@ bool CWallet::CreateTransactionInternal( // Before we return success, we assume any change key will be used to prevent // accidental re-use. reservedest.KeepDestination(); + fee_calc_out = feeCalc; WalletLogPrintf("Fee Calculation: Fee:%d Bytes:%u Tgt:%d (requested %d) Reason:\"%s\" Decay %.5f: Estimation: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out) Fail: (%g - %g) %.2f%% %.1f/(%.1f %d mem %.1f out)\n", nFeeRet, nBytes, feeCalc.returnedTarget, feeCalc.desiredTarget, StringForFeeReason(feeCalc.reason), feeCalc.est.decay, @@ -3790,12 +3794,13 @@ bool CWallet::CreateTransaction( int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, + FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize) { int nChangePosIn = nChangePosInOut; Assert(!tx); // tx is an out-param. TODO change the return type from bool to tx (or nullptr) - bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, sign, nExtraPayloadSize); + bool res = CreateTransactionInternal(vecSend, tx, nFeeRet, nChangePosInOut, error, coin_control, fee_calc_out, sign, nExtraPayloadSize); // try with avoidpartialspends unless it's enabled already if (res && nFeeRet > 0 /* 0 means non-functional fee rate estimation */ && m_max_aps_fee > -1 && !coin_control.m_avoid_partial_spends) { CCoinControl tmp_cc = coin_control; @@ -3811,7 +3816,7 @@ bool CWallet::CreateTransaction( CTransactionRef tx2; int nChangePosInOut2 = nChangePosIn; bilingual_str error2; // fired and forgotten; if an error occurs, we discard the results - if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, sign, nExtraPayloadSize)) { + if (CreateTransactionInternal(vecSend, tx2, nFeeRet2, nChangePosInOut2, error2, tmp_cc, fee_calc_out, sign, nExtraPayloadSize)) { // if fee of this alternative one is within the range of the max fee, we use this one const bool use_aps = nFeeRet2 <= nFeeRet + m_max_aps_fee; WalletLogPrintf("Fee non-grouped = %lld, grouped = %lld, using %s\n", nFeeRet, nFeeRet2, use_aps ? "grouped" : "non-grouped"); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7cb32f4c0a736..ec45be2c44ada 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -842,7 +842,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati // ScriptPubKeyMan::GetID. In many cases it will be the hash of an internal structure std::map> m_spk_managers; - bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign, int nExtraPayloadSize); + bool CreateTransactionInternal(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign, int nExtraPayloadSize); public: /** @@ -1133,7 +1133,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * selected by SelectCoins(); Also create the change output, when needed * @note passing nChangePosInOut as -1 will result in setting a random position */ - bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, bool sign = true, int nExtraPayloadSize = 0); + bool CreateTransaction(const std::vector& vecSend, CTransactionRef& tx, CAmount& nFeeRet, int& nChangePosInOut, bilingual_str& error, const CCoinControl& coin_control, FeeCalculation& fee_calc_out, bool sign = true, int nExtraPayloadSize = 0); /** * Submit the transaction to the node's mempool and then relay to peers. * Should be called after CreateTransaction unless you want to abort diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index c8ef7d70a2adb..e6d91e438b617 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -7,6 +7,7 @@ import os from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE + from test_framework.test_framework import DashTestFramework from test_framework.util import ( assert_equal, @@ -95,6 +96,9 @@ def run_test(self): # directory content should equal the generated transaction hashes txids_rpc = list(map(lambda t: notify_outputname(self.wallet, t['txid']), self.nodes[1].listtransactions("*", block_count))) assert_equal(sorted(txids_rpc), sorted(os.listdir(self.walletnotify_dir))) + for tx_file in os.listdir(self.walletnotify_dir): + os.remove(os.path.join(self.walletnotify_dir, tx_file)) + self.log.info("test -chainlocknotify") @@ -141,5 +145,6 @@ def run_test(self): # TODO: add test for `-alertnotify` large fork notifications + if __name__ == '__main__': NotificationsTest().main() diff --git a/test/functional/p2p_blocksonly.py b/test/functional/p2p_blocksonly.py index d23d2c990bc1b..2703fe4e2df72 100755 --- a/test/functional/p2p_blocksonly.py +++ b/test/functional/p2p_blocksonly.py @@ -43,7 +43,7 @@ def blocksonly_mode_tests(self): assert_equal(self.nodes[0].getmempoolinfo()['size'], 1) self.log.info("Restarting node 0 with relay permission and blocksonly") - self.restart_node(0, ["-persistmempool=0", "-whitelist=relay@127.0.0.1", "-blocksonly"]) + self.restart_node(0, ["-persistmempool=0", "-whitelist=relay@127.0.0.1", "-blocksonly", '-deprecatedrpc=whitelisted']) assert_equal(self.nodes[0].getrawmempool(), []) first_peer = self.nodes[0].add_p2p_connection(P2PInterface()) second_peer = self.nodes[0].add_p2p_connection(P2PInterface()) diff --git a/test/functional/p2p_disconnect_ban.py b/test/functional/p2p_disconnect_ban.py index 7c784cd064315..eb9314cd82840 100755 --- a/test/functional/p2p_disconnect_ban.py +++ b/test/functional/p2p_disconnect_ban.py @@ -17,8 +17,11 @@ def set_test_params(self): def run_test(self): self.log.info("Connect nodes both way") + # By default, the test framework sets up an addnode connection from + # node 1 --> node0. By connecting node0 --> node 1, we're left with + # the two nodes being connected both ways. + # Topology will look like: node0 <--> node1 self.connect_nodes(0, 1) - self.connect_nodes(1, 0) self.log.info("Test setban and listbanned RPCs") diff --git a/test/functional/p2p_permissions.py b/test/functional/p2p_permissions.py index 38c18bbbd7155..1401c642088a0 100755 --- a/test/functional/p2p_permissions.py +++ b/test/functional/p2p_permissions.py @@ -39,6 +39,13 @@ def run_test(self): ["relay", "noban", "mempool", "download"], True) + self.checkpermission( + # check without deprecatedrpc=whitelisted + ["-whitelist=127.0.0.1"], + # Make sure the default values in the command line documentation match the ones here + ["relay", "noban", "mempool", "download"], + None) + self.checkpermission( # no permission (even with forcerelay) ["-whitelist=@127.0.0.1", "-whitelistforcerelay=1"], @@ -77,6 +84,12 @@ def run_test(self): ["noban", "mempool", "download"], False) + self.checkpermission( + # check without deprecatedrpc=whitelisted + ["-whitelist=noban,mempool@127.0.0.1", "-whitelistrelay"], + ["noban", "mempool", "download"], + None) + self.checkpermission( # legacy whitelistforcerelay should be ignored ["-whitelist=noban,mempool@127.0.0.1", "-whitelistforcerelay"], @@ -159,10 +172,15 @@ def in_mempool(): ) def checkpermission(self, args, expectedPermissions, whitelisted): + if whitelisted is not None: + args = [*args, '-deprecatedrpc=whitelisted'] self.restart_node(1, args) self.connect_nodes(0, 1) peerinfo = self.nodes[1].getpeerinfo()[0] - assert_equal(peerinfo['whitelisted'], whitelisted) + if whitelisted is None: + assert 'whitelisted' not in peerinfo + else: + assert_equal(peerinfo['whitelisted'], whitelisted) assert_equal(len(expectedPermissions), len(peerinfo['permissions'])) for p in expectedPermissions: if not p in peerinfo['permissions']: diff --git a/test/functional/rpc_getpeerinfo_banscore_deprecation.py b/test/functional/rpc_getpeerinfo_deprecation.py similarity index 51% rename from test/functional/rpc_getpeerinfo_banscore_deprecation.py rename to test/functional/rpc_getpeerinfo_deprecation.py index b830248e1e23f..340a66e12f389 100755 --- a/test/functional/rpc_getpeerinfo_banscore_deprecation.py +++ b/test/functional/rpc_getpeerinfo_deprecation.py @@ -2,23 +2,37 @@ # Copyright (c) 2020 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 deprecation of getpeerinfo RPC banscore field.""" +"""Test deprecation of getpeerinfo RPC fields.""" from test_framework.test_framework import BitcoinTestFramework -class GetpeerinfoBanscoreDeprecationTest(BitcoinTestFramework): +class GetpeerinfoDeprecationTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 self.extra_args = [[], ["-deprecatedrpc=banscore"]] def run_test(self): + self.test_banscore_deprecation() + self.test_addnode_deprecation() + + def test_banscore_deprecation(self): self.log.info("Test getpeerinfo by default no longer returns a banscore field") assert "banscore" not in self.nodes[0].getpeerinfo()[0].keys() self.log.info("Test getpeerinfo returns banscore with -deprecatedrpc=banscore") assert "banscore" in self.nodes[1].getpeerinfo()[0].keys() + def test_addnode_deprecation(self): + self.restart_node(1, ["-deprecatedrpc=getpeerinfo_addnode"]) + self.connect_nodes(0, 1) + + self.log.info("Test getpeerinfo by default no longer returns an addnode field") + assert "addnode" not in self.nodes[0].getpeerinfo()[0].keys() + + self.log.info("Test getpeerinfo returns addnode with -deprecatedrpc=addnode") + assert "addnode" in self.nodes[1].getpeerinfo()[0].keys() + if __name__ == "__main__": - GetpeerinfoBanscoreDeprecationTest().main() + GetpeerinfoDeprecationTest().main() diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 5e71bcb204267..ca77cc429d478 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -54,9 +54,11 @@ def run_test(self): # Especially the exchange of messages like getheaders and friends causes test failures here self.nodes[0].ping() self.wait_until(lambda: all(['pingtime' in n for n in self.nodes[0].getpeerinfo()])) - self.log.info('Connect nodes both way') + # By default, the test framework sets up an addnode connection from + # node 1 --> node0. By connecting node0 --> node 1, we're left with + # the two nodes being connected both ways. + # Topology will look like: node0 <--> node1 self.connect_nodes(0, 1) - self.connect_nodes(1, 0) self.sync_all() self.test_connection_count() @@ -75,6 +77,41 @@ def test_connection_count(self): # during network setup assert_equal(self.nodes[0].getconnectioncount(), 3) + def test_getpeerinfo(self): + self.log.info("Test getpeerinfo") + # Create a few getpeerinfo last_block/last_transaction values. + self.wallet.send_self_transfer(from_node=self.nodes[0]) # Make a transaction so we can see it in the getpeerinfo results + self.nodes[1].generate(1) + self.sync_all() + time_now = self.mocktime + peer_info = [x.getpeerinfo() for x in self.nodes] + # Verify last_block and last_transaction keys/values. + for node, peer, field in product(range(self.num_nodes - self.mn_count), range(2), ['last_block', 'last_transaction']): + assert field in peer_info[node][peer].keys() + if peer_info[node][peer][field] != 0: + assert_approx(peer_info[node][peer][field], time_now, vspan=60) + # check both sides of bidirectional connection between nodes + # the address bound to on one side will be the source address for the other node + assert_equal(peer_info[0][0]['addrbind'], peer_info[1][0]['addr']) + assert_equal(peer_info[1][0]['addrbind'], peer_info[0][0]['addr']) + # check the `servicesnames` field + for info in peer_info: + assert_net_servicesnames(int(info[0]["services"], 0x10), info[0]["servicesnames"]) + + # Check dynamically generated networks list in getpeerinfo help output. + assert "(ipv4, ipv6, onion, i2p, not_publicly_routable)" in self.nodes[0].help("getpeerinfo") + # This part is slightly different comparing to the Bitcoin implementation. This is expected because we create connections on network setup a bit differently too. + # We also create more connection during the test itself to test mn specific stats + assert_equal(peer_info[0][0]['connection_type'], 'inbound') + assert_equal(peer_info[0][1]['connection_type'], 'inbound') + assert_equal(peer_info[0][2]['connection_type'], 'manual') + + assert_equal(peer_info[1][0]['connection_type'], 'manual') + assert_equal(peer_info[1][1]['connection_type'], 'inbound') + + assert_equal(peer_info[2][0]['connection_type'], 'manual') + + def test_getnettotals(self): self.log.info("Test getnettotals") # Test getnettotals and getpeerinfo by doing a ping. The bytes @@ -113,15 +150,13 @@ def test_getnetworkinfo(self): with self.nodes[0].assert_debug_log(expected_msgs=['SetNetworkActive: true\n']): self.nodes[0].setnetworkactive(state=True) - # Connect nodes both ways. self.connect_nodes(0, 1) - self.connect_nodes(1, 0) info = self.nodes[1].getnetworkinfo() assert_equal(info['networkactive'], True) - assert_equal(info['connections'], 2) + assert_equal(info['connections'], 1) assert_equal(info['connections_in'], 1) - assert_equal(info['connections_out'], 1) + assert_equal(info['connections_out'], 0) assert_equal(info['connections_mn'], 0) assert_equal(info['connections_mn_in'], 0) assert_equal(info['connections_mn_out'], 0) @@ -135,15 +170,17 @@ def test_getnetworkinfo(self): assert "(ipv4, ipv6, onion, i2p)" in self.nodes[0].help("getnetworkinfo") self.log.info('Test extended connections info') - self.connect_nodes(1, 2) + # Connect nodes both ways. + self.connect_nodes(0, 1) + self.connect_nodes(1, 0) self.nodes[1].ping() self.wait_until(lambda: all(['pingtime' in n for n in self.nodes[1].getpeerinfo()])) - assert_equal(self.nodes[1].getnetworkinfo()['connections'], 3) + assert_equal(self.nodes[1].getnetworkinfo()['connections'], 2) assert_equal(self.nodes[1].getnetworkinfo()['connections_in'], 1) - assert_equal(self.nodes[1].getnetworkinfo()['connections_out'], 2) - assert_equal(self.nodes[1].getnetworkinfo()['connections_mn'], 1) + assert_equal(self.nodes[1].getnetworkinfo()['connections_out'], 1) + assert_equal(self.nodes[1].getnetworkinfo()['connections_mn'], 0) assert_equal(self.nodes[1].getnetworkinfo()['connections_mn_in'], 0) - assert_equal(self.nodes[1].getnetworkinfo()['connections_mn_out'], 1) + assert_equal(self.nodes[1].getnetworkinfo()['connections_mn_out'], 0) def test_getaddednodeinfo(self): self.log.info("Test getaddednodeinfo") @@ -165,40 +202,6 @@ def test_getaddednodeinfo(self): # check that a non-existent node returns an error assert_raises_rpc_error(-24, "Node has not been added", self.nodes[0].getaddednodeinfo, '1.1.1.1') - def test_getpeerinfo(self): - self.log.info("Test getpeerinfo") - # Create a few getpeerinfo last_block/last_transaction values. - self.wallet.send_self_transfer(from_node=self.nodes[0]) # Make a transaction so we can see it in the getpeerinfo results - self.nodes[1].generate(1) - self.sync_all() - time_now = self.mocktime - peer_info = [x.getpeerinfo() for x in self.nodes] - # Verify last_block and last_transaction keys/values. - for node, peer, field in product(range(self.num_nodes - self.mn_count), range(2), ['last_block', 'last_transaction']): - assert field in peer_info[node][peer].keys() - if peer_info[node][peer][field] != 0: - assert_approx(peer_info[node][peer][field], time_now, vspan=60) - # check both sides of bidirectional connection between nodes - # the address bound to on one side will be the source address for the other node - assert_equal(peer_info[0][0]['addrbind'], peer_info[1][0]['addr']) - assert_equal(peer_info[1][0]['addrbind'], peer_info[0][0]['addr']) - # check the `servicesnames` field - for info in peer_info: - assert_net_servicesnames(int(info[0]["services"], 0x10), info[0]["servicesnames"]) - - # Check dynamically generated networks list in getpeerinfo help output. - assert "(ipv4, ipv6, onion, i2p, not_publicly_routable)" in self.nodes[0].help("getpeerinfo") - # This part is slightly different comparing to the Bitcoin implementation. This is expected because we create connections on network setup a bit differently too. - # We also create more connection during the test itself to test mn specific stats - assert_equal(peer_info[0][0]['connection_type'], 'inbound') - assert_equal(peer_info[0][1]['connection_type'], 'inbound') - assert_equal(peer_info[0][2]['connection_type'], 'manual') - - assert_equal(peer_info[1][0]['connection_type'], 'manual') - assert_equal(peer_info[1][1]['connection_type'], 'inbound') - - assert_equal(peer_info[2][0]['connection_type'], 'manual') - def test_service_flags(self): self.log.info("Test service flags") self.nodes[0].add_p2p_connection(P2PInterface(), services=(1 << 4) | (1 << 63)) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 07c6efd4ab1ba..7fdceddffaa1b 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -334,7 +334,7 @@ 'feature_config_args.py', 'feature_settings.py', 'rpc_getdescriptorinfo.py', - 'rpc_getpeerinfo_banscore_deprecation.py', + 'rpc_getpeerinfo_deprecation.py', 'rpc_help.py', 'feature_help.py', 'feature_blockfilterindex_prune.py' diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index ad80f4d94cba4..f49612e6a95fd 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -126,7 +126,7 @@ def run_test(self): assert_raises_rpc_error(-8, "Invalid parameter, expected locked output", self.nodes[2].lockunspent, True, [unspent_0]) self.nodes[2].lockunspent(False, [unspent_0]) assert_raises_rpc_error(-8, "Invalid parameter, output already locked", self.nodes[2].lockunspent, False, [unspent_0]) - assert_raises_rpc_error(-4, "Insufficient funds", self.nodes[2].sendtoaddress, self.nodes[2].getnewaddress(), 200) + assert_raises_rpc_error(-6, "Insufficient funds", self.nodes[2].sendtoaddress, self.nodes[2].getnewaddress(), 200) assert_equal([unspent_0], self.nodes[2].listlockunspent()) self.nodes[2].lockunspent(True, [unspent_0]) assert_equal(len(self.nodes[2].listlockunspent()), 0) @@ -380,6 +380,9 @@ def run_test(self): assert_equal(tx_obj['amount'], Decimal('-0.0001')) # General checks for errors from incorrect inputs + # This will raise an exception because the amount is negative + assert_raises_rpc_error(-3, "Amount out of range", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "-1") + # This will raise an exception because the amount type is wrong assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "1f-4") @@ -608,7 +611,7 @@ def run_test(self): node0_balance = self.nodes[0].getbalance() # With walletrejectlongchains we will not create the tx and store it in our wallet. - assert_raises_rpc_error(-4, "Transaction has too long of a mempool chain", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01')) + assert_raises_rpc_error(-6, "Transaction has too long of a mempool chain", self.nodes[0].sendtoaddress, sending_addr, node0_balance - Decimal('0.01')) # Verify nothing new in wallet assert_equal(total_txs, len(self.nodes[0].listtransactions("*", 99999))) @@ -670,6 +673,18 @@ def run_test(self): assert_array_result(tx["details"], {"category": "receive"}, expected_receive_vout) assert_equal(tx[verbose_field], self.nodes[0].decoderawtransaction(tx["hex"])) + self.log.info("Test send* RPCs with verbose=True") + address = self.nodes[0].getnewaddress("test") + txid_feeReason_one = self.nodes[2].sendtoaddress(address=address, amount=5, verbose=True) + assert_equal(txid_feeReason_one["fee_reason"], "Fallback fee") + txid_feeReason_two = self.nodes[2].sendmany(dummy='', amounts={address: 5}, verbose=True) + assert_equal(txid_feeReason_two["fee_reason"], "Fallback fee") + self.log.info("Test send* RPCs with verbose=False") + txid_feeReason_three = self.nodes[2].sendtoaddress(address=address, amount=5, verbose=False) + assert_equal(self.nodes[2].gettransaction(txid_feeReason_three)['txid'], txid_feeReason_three) + txid_feeReason_four = self.nodes[2].sendmany(dummy='', amounts={address: 5}, verbose=False) + assert_equal(self.nodes[2].gettransaction(txid_feeReason_four)['txid'], txid_feeReason_four) + if __name__ == '__main__': WalletTest().main() diff --git a/test/functional/wallet_fallbackfee.py b/test/functional/wallet_fallbackfee.py index c481c6492b68e..b28f3ecebc4dc 100755 --- a/test/functional/wallet_fallbackfee.py +++ b/test/functional/wallet_fallbackfee.py @@ -24,7 +24,7 @@ def run_test(self): # test sending a tx with disabled fallback fee (must fail) self.restart_node(0, extra_args=["-fallbackfee=0"]) - assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)) + assert_raises_rpc_error(-6, "Fee estimation failed", lambda: self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 1)) assert_raises_rpc_error(-4, "Fee estimation failed", lambda: self.nodes[0].fundrawtransaction(self.nodes[0].createrawtransaction([], {self.nodes[0].getnewaddress(): 1}))) assert_raises_rpc_error(-6, "Fee estimation failed", lambda: self.nodes[0].sendmany("", {self.nodes[0].getnewaddress(): 1}))