diff options
Diffstat (limited to 'src/wallet')
| -rw-r--r-- | src/wallet/feebumper.cpp | 117 | ||||
| -rw-r--r-- | src/wallet/init.cpp | 3 | ||||
| -rw-r--r-- | src/wallet/rpcwallet.cpp | 135 | ||||
| -rw-r--r-- | src/wallet/test/wallet_tests.cpp | 78 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 6 | ||||
| -rw-r--r-- | src/wallet/wallet.h | 6 |
6 files changed, 251 insertions, 94 deletions
diff --git a/src/wallet/feebumper.cpp b/src/wallet/feebumper.cpp index 619197a57..b87231293 100644 --- a/src/wallet/feebumper.cpp +++ b/src/wallet/feebumper.cpp @@ -57,6 +57,86 @@ static feebumper::Result PreconditionChecks(interfaces::Chain::Lock& locked_chai return feebumper::Result::OK; } +//! Check if the user provided a valid feeRate +static feebumper::Result CheckFeeRate(const CWallet* wallet, const CWalletTx& wtx, const CFeeRate& newFeerate, const int64_t maxTxSize, std::vector<std::string>& errors) { + // check that fee rate is higher than mempool's minimum fee + // (no point in bumping fee if we know that the new tx won't be accepted to the mempool) + // This may occur if the user set FeeRate, TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps, + // in a rare situation where the mempool minimum fee increased significantly since the fee estimation just a + // moment earlier. In this case, we report an error to the user, who may adjust the fee. + CFeeRate minMempoolFeeRate = wallet->chain().mempoolMinFee(); + + if (newFeerate.GetFeePerK() < minMempoolFeeRate.GetFeePerK()) { + errors.push_back(strprintf( + "New fee rate (%s) is lower than the minimum fee rate (%s) to get into the mempool -- ", + FormatMoney(newFeerate.GetFeePerK()), + FormatMoney(minMempoolFeeRate.GetFeePerK()))); + return feebumper::Result::WALLET_ERROR; + } + + CAmount new_total_fee = newFeerate.GetFee(maxTxSize); + + CFeeRate incrementalRelayFee = std::max(wallet->chain().relayIncrementalFee(), CFeeRate(WALLET_INCREMENTAL_RELAY_FEE)); + + // Given old total fee and transaction size, calculate the old feeRate + CAmount old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut(); + const int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); + CFeeRate nOldFeeRate(old_fee, txSize); + // Min total fee is old fee + relay fee + CAmount minTotalFee = nOldFeeRate.GetFee(maxTxSize) + incrementalRelayFee.GetFee(maxTxSize); + + if (new_total_fee < minTotalFee) { + errors.push_back(strprintf("Insufficient total fee %s, must be at least %s (oldFee %s + incrementalFee %s)", + FormatMoney(new_total_fee), FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxTxSize)), FormatMoney(incrementalRelayFee.GetFee(maxTxSize)))); + return feebumper::Result::INVALID_PARAMETER; + } + + CAmount requiredFee = GetRequiredFee(*wallet, maxTxSize); + if (new_total_fee < requiredFee) { + errors.push_back(strprintf("Insufficient total fee (cannot be less than required fee %s)", + FormatMoney(requiredFee))); + return feebumper::Result::INVALID_PARAMETER; + } + + // Check that in all cases the new fee doesn't violate maxTxFee + const CAmount max_tx_fee = wallet->m_default_max_tx_fee; + if (new_total_fee > max_tx_fee) { + errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)", + FormatMoney(new_total_fee), FormatMoney(max_tx_fee))); + return feebumper::Result::WALLET_ERROR; + } + + return feebumper::Result::OK; +} + +static CFeeRate EstimateFeeRate(CWallet* wallet, const CWalletTx& wtx, CCoinControl& coin_control, CAmount& old_fee) +{ + // Get the fee rate of the original transaction. This is calculated from + // the tx fee/vsize, so it may have been rounded down. Add 1 satoshi to the + // result. + old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut(); + int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); + CFeeRate feerate(old_fee, txSize); + feerate += CFeeRate(1); + + // The node has a configurable incremental relay fee. Increment the fee by + // the minimum of that and the wallet's conservative + // WALLET_INCREMENTAL_RELAY_FEE value to future proof against changes to + // network wide policy for incremental relay fee that our node may not be + // aware of. This ensures we're over the over the required relay fee rate + // (BIP 125 rule 4). The replacement tx will be at least as large as the + // original tx, so the total fee will be greater (BIP 125 rule 3) + CFeeRate node_incremental_relay_fee = wallet->chain().relayIncrementalFee(); + CFeeRate wallet_incremental_relay_fee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE); + feerate += std::max(node_incremental_relay_fee, wallet_incremental_relay_fee); + + // Fee rate must also be at least the wallet's GetMinimumFeeRate + CFeeRate min_feerate(GetMinimumFeeRate(*wallet, coin_control, /* feeCalc */ nullptr)); + + // Set the required fee rate for the replacement transaction in coin control. + return std::max(feerate, min_feerate); +} + namespace feebumper { bool TransactionCanBeBumped(const CWallet* wallet, const uint256& txid) @@ -230,31 +310,18 @@ Result CreateRateBumpTransaction(CWallet* wallet, const uint256& txid, const CCo } } - // Get the fee rate of the original transaction. This is calculated from - // the tx fee/vsize, so it may have been rounded down. Add 1 satoshi to the - // result. - old_fee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut(); - int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); - // Feerate of thing we are bumping - CFeeRate feerate(old_fee, txSize); - feerate += CFeeRate(1); - - // The node has a configurable incremental relay fee. Increment the fee by - // the minimum of that and the wallet's conservative - // WALLET_INCREMENTAL_RELAY_FEE value to future proof against changes to - // network wide policy for incremental relay fee that our node may not be - // aware of. This ensures we're over the over the required relay fee rate - // (BIP 125 rule 4). The replacement tx will be at least as large as the - // original tx, so the total fee will be greater (BIP 125 rule 3) - CFeeRate node_incremental_relay_fee = wallet->chain().relayIncrementalFee(); - CFeeRate wallet_incremental_relay_fee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE); - feerate += std::max(node_incremental_relay_fee, wallet_incremental_relay_fee); - - // Fee rate must also be at least the wallet's GetMinimumFeeRate - CFeeRate min_feerate(GetMinimumFeeRate(*wallet, new_coin_control, /* feeCalc */ nullptr)); - - // Set the required fee rate for the replacement transaction in coin control. - new_coin_control.m_feerate = std::max(feerate, min_feerate); + if (coin_control.m_feerate) { + // The user provided a feeRate argument. + // We calculate this here to avoid compiler warning on the cs_wallet lock + const int64_t maxTxSize = CalculateMaximumSignedTxSize(*wtx.tx, wallet); + Result res = CheckFeeRate(wallet, wtx, *(new_coin_control.m_feerate), maxTxSize, errors); + if (res != Result::OK) { + return res; + } + } else { + // The user did not provide a feeRate argument + new_coin_control.m_feerate = EstimateFeeRate(wallet, wtx, new_coin_control, old_fee); + } // Fill in required inputs we are double-spending(all of them) // N.B.: bip125 doesn't require all the inputs in the replaced transaction to be diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index e766deadb..43b6ead02 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -41,7 +41,8 @@ void WalletInit::AddWalletOptions() const gArgs.AddArg("-discardfee=<amt>", strprintf("The fee rate (in %s/kB) that indicates your tolerance for discarding change by adding it to the fee (default: %s). " "Note: An output is discarded if it is dust at this rate, but we will always discard up to the dust relay fee and a discard fee above that is limited by the fee estimate for the longest target", CURRENCY_UNIT, FormatMoney(DEFAULT_DISCARD_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); - gArgs.AddArg("-fallbackfee=<amt>", strprintf("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)", + + gArgs.AddArg("-fallbackfee=<amt>", strprintf("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data. 0 to entirely disable the fallbackfee feature. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_FALLBACK_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-keypool=<n>", strprintf("Set key pool size to <n> (default: %u)", DEFAULT_KEYPOOL_SIZE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); gArgs.AddArg("-maxtxfee=<amt>", strprintf("Maximum total fees (in %s) to use in a single wallet transaction; setting this too low may abort large transactions (default: %s)", diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 216205ed6..dc437e5cd 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1190,7 +1190,7 @@ static UniValue listreceivedbyaddress(const JSONRPCRequest& request) RPCResult{ "[\n" " {\n" - " \"involvesWatchonly\" : true, (bool) Only returned if imported addresses were involved in transaction\n" + " \"involvesWatchonly\" : true, (bool) Only returns true if imported addresses were involved in transaction.\n" " \"address\" : \"receivingaddress\", (string) The receiving address\n" " \"amount\" : x.xxx, (numeric) The total amount in " + CURRENCY_UNIT + " received by the address\n" " \"confirmations\" : n, (numeric) The number of confirmations of the most recent transaction included\n" @@ -1240,7 +1240,7 @@ static UniValue listreceivedbylabel(const JSONRPCRequest& request) RPCResult{ "[\n" " {\n" - " \"involvesWatchonly\" : true, (bool) Only returned if imported addresses were involved in transaction\n" + " \"involvesWatchonly\" : true, (bool) Only returns true if imported addresses were involved in transaction.\n" " \"amount\" : x.xxx, (numeric) The total amount received by addresses with this label\n" " \"confirmations\" : n, (numeric) The number of confirmations of the most recent transaction included\n" " \"label\" : \"label\" (string) The label of the receiving address. The default label is \"\".\n" @@ -1359,6 +1359,27 @@ static void ListTransactions(interfaces::Chain::Lock& locked_chain, CWallet* con } } +static const std::string TransactionDescriptionString() +{ + return " \"confirmations\": n, (numeric) The number of confirmations for the transaction. Negative confirmations means the\n" + " transaction conflicted that many blocks ago.\n" + " \"generated\": xxx, (bool) Only present if transaction only input is a coinbase one.\n" + " \"trusted\": xxx, (bool) Only present if we consider transaction to be trusted and so safe to spend from.\n" + " \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction.\n" + " \"blockindex\": n, (numeric) The index of the transaction in the block that includes it.\n" + " \"blocktime\": xxx, (numeric) The block time in seconds since epoch (1 Jan 1970 GMT).\n" + " \"txid\": \"transactionid\", (string) The transaction id.\n" + " \"walletconflicts\": [ (array) Conflicting transaction ids.\n" + " \"txid\", (string) The transaction id.\n" + " ...\n" + " ],\n" + " \"time\": xxx, (numeric) The transaction time in seconds since epoch (midnight Jan 1 1970 GMT).\n" + " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (midnight Jan 1 1970 GMT).\n" + " \"comment\": \"...\", (string) If a comment is associated with the transaction, only present if not empty.\n" + " \"bip125-replaceable\": \"yes|no|unknown\", (string) Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" + " may be unknown for unconfirmed transactions not in the mempool\n"; +} + UniValue listtransactions(const JSONRPCRequest& request) { std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request); @@ -1381,6 +1402,7 @@ UniValue listtransactions(const JSONRPCRequest& request) RPCResult{ "[\n" " {\n" + " \"involvesWatchonly\": xxx, (bool) Only returns true if imported addresses were involved in transaction.\n" " \"address\":\"address\", (string) The bitcoin address of the transaction.\n" " \"category\": (string) The transaction category.\n" " \"send\" Transactions sent.\n" @@ -1394,19 +1416,8 @@ UniValue listtransactions(const JSONRPCRequest& request) " \"vout\": n, (numeric) the vout value\n" " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" " 'send' category of transactions.\n" - " \"confirmations\": n, (numeric) The number of confirmations for the transaction. Negative confirmations indicate the\n" - " transaction conflicts with the block chain\n" - " \"trusted\": xxx, (bool) Whether we consider the outputs of this unconfirmed transaction safe to spend.\n" - " \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction.\n" - " \"blockindex\": n, (numeric) The index of the transaction in the block that includes it.\n" - " \"blocktime\": xxx, (numeric) The block time in seconds since epoch (1 Jan 1970 GMT).\n" - " \"txid\": \"transactionid\", (string) The transaction id.\n" - " \"time\": xxx, (numeric) The transaction time in seconds since epoch (midnight Jan 1 1970 GMT).\n" - " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (midnight Jan 1 1970 GMT).\n" - " \"comment\": \"...\", (string) If a comment is associated with the transaction.\n" - " \"bip125-replaceable\": \"yes|no|unknown\", (string) Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - " may be unknown for unconfirmed transactions not in the mempool\n" - " \"abandoned\": xxx (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + + TransactionDescriptionString() + + " \"abandoned\": xxx (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" " 'send' category of transactions.\n" " }\n" "]\n" @@ -1515,6 +1526,7 @@ static UniValue listsinceblock(const JSONRPCRequest& request) RPCResult{ "{\n" " \"transactions\": [\n" + " \"involvesWatchonly\": xxx, (bool) Only returns true if imported addresses were involved in transaction.\n" " \"address\":\"address\", (string) The bitcoin address of the transaction.\n" " \"category\": (string) The transaction category.\n" " \"send\" Transactions sent.\n" @@ -1526,17 +1538,8 @@ static UniValue listsinceblock(const JSONRPCRequest& request) " for all other categories\n" " \"vout\" : n, (numeric) the vout value\n" " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the 'send' category of transactions.\n" - " \"confirmations\": n, (numeric) The number of confirmations for the transaction.\n" - " When it's < 0, it means the transaction conflicted that many blocks ago.\n" - " \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction.\n" - " \"blockindex\": n, (numeric) The index of the transaction in the block that includes it.\n" - " \"blocktime\": xxx, (numeric) The block time in seconds since epoch (1 Jan 1970 GMT).\n" - " \"txid\": \"transactionid\", (string) The transaction id.\n" - " \"time\": xxx, (numeric) The transaction time in seconds since epoch (Jan 1 1970 GMT).\n" - " \"timereceived\": xxx, (numeric) The time received in seconds since epoch (Jan 1 1970 GMT).\n" - " \"bip125-replaceable\": \"yes|no|unknown\", (string) Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - " may be unknown for unconfirmed transactions not in the mempool\n" - " \"abandoned\": xxx, (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the 'send' category of transactions.\n" + + TransactionDescriptionString() + + " \"abandoned\": xxx, (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the 'send' category of transactions.\n" " \"comment\": \"...\", (string) If a comment is associated with the transaction.\n" " \"label\" : \"label\" (string) A comment for the address/transaction, if any\n" " \"to\": \"...\", (string) If a comment to is associated with the transaction.\n" @@ -1655,40 +1658,33 @@ static UniValue gettransaction(const JSONRPCRequest& request) }, RPCResult{ "{\n" - " \"amount\" : x.xxx, (numeric) The transaction amount in " + CURRENCY_UNIT + "\n" - " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" + " \"amount\" : x.xxx, (numeric) The transaction amount in " + CURRENCY_UNIT + "\n" + " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" " 'send' category of transactions.\n" - " \"confirmations\" : n, (numeric) The number of confirmations\n" - " \"blockhash\" : \"hash\", (string) The block hash\n" - " \"blockindex\" : xx, (numeric) The index of the transaction in the block that includes it\n" - " \"blocktime\" : ttt, (numeric) The time in seconds since epoch (1 Jan 1970 GMT)\n" - " \"txid\" : \"transactionid\", (string) The transaction id.\n" - " \"time\" : ttt, (numeric) The transaction time in seconds since epoch (1 Jan 1970 GMT)\n" - " \"timereceived\" : ttt, (numeric) The time received in seconds since epoch (1 Jan 1970 GMT)\n" - " \"bip125-replaceable\": \"yes|no|unknown\", (string) Whether this transaction could be replaced due to BIP125 (replace-by-fee);\n" - " may be unknown for unconfirmed transactions not in the mempool\n" - " \"details\" : [\n" - " {\n" - " \"address\" : \"address\", (string) The bitcoin address involved in the transaction\n" - " \"category\" : (string) The transaction category.\n" - " \"send\" Transactions sent.\n" - " \"receive\" Non-coinbase transactions received.\n" - " \"generate\" Coinbase transactions received with more than 100 confirmations.\n" - " \"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" - " \"orphan\" Orphaned coinbase transactions received.\n" - " \"amount\" : x.xxx, (numeric) The amount in " + CURRENCY_UNIT + "\n" - " \"label\" : \"label\", (string) A comment for the address/transaction, if any\n" - " \"vout\" : n, (numeric) the vout value\n" - " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" + + TransactionDescriptionString() + + " \"details\" : [\n" + " {\n" + " \"involvesWatchonly\": xxx, (bool) Only returns true if imported addresses were involved in transaction.\n" + " \"address\" : \"address\", (string) The bitcoin address involved in the transaction\n" + " \"category\" : (string) The transaction category.\n" + " \"send\" Transactions sent.\n" + " \"receive\" Non-coinbase transactions received.\n" + " \"generate\" Coinbase transactions received with more than 100 confirmations.\n" + " \"immature\" Coinbase transactions received with 100 or fewer confirmations.\n" + " \"orphan\" Orphaned coinbase transactions received.\n" + " \"amount\" : x.xxx, (numeric) The amount in " + CURRENCY_UNIT + "\n" + " \"label\" : \"label\", (string) A comment for the address/transaction, if any\n" + " \"vout\" : n, (numeric) the vout value\n" + " \"fee\": x.xxx, (numeric) The amount of the fee in " + CURRENCY_UNIT + ". This is negative and only available for the \n" " 'send' category of transactions.\n" - " \"abandoned\": xxx (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" + " \"abandoned\": xxx (bool) 'true' if the transaction has been abandoned (inputs are respendable). Only available for the \n" " 'send' category of transactions.\n" - " }\n" - " ,...\n" - " ],\n" - " \"hex\" : \"data\" (string) Raw data for transaction\n" - " \"decoded\" : transaction (json object) Optional, the decoded transaction (only present when `verbose` is passed), equivalent to the\n" - " RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed.\n" + " }\n" + " ,...\n" + " ],\n" + " \"hex\" : \"data\" (string) Raw data for transaction\n" + " \"decoded\" : transaction (json object) Optional, the decoded transaction (only present when `verbose` is passed), equivalent to the\n" + " RPC decoderawtransaction method, or the RPC getrawtransaction method when `verbose` is passed.\n" "}\n" }, RPCExamples{ @@ -2327,7 +2323,7 @@ static UniValue settxfee(const JSONRPCRequest& request) CAmount nAmount = AmountFromValue(request.params[0]); CFeeRate tx_fee_rate(nAmount, 1000); - if (tx_fee_rate == 0) { + if (tx_fee_rate == CFeeRate(0)) { // automatic selection } else if (tx_fee_rate < pwallet->chain().relayMinFee()) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("txfee cannot be less than min relay tx fee (%s)", pwallet->chain().relayMinFee().ToString())); @@ -3135,7 +3131,9 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request) {"changeAddress", RPCArg::Type::STR, /* default */ "pool address", "The bitcoin address to receive the change"}, {"changePosition", RPCArg::Type::NUM, /* default */ "random", "The index of the change output"}, {"change_type", RPCArg::Type::STR, /* default */ "set by -changetype", "The output type to use. Only valid if changeAddress is not specified. Options are \"legacy\", \"p2sh-segwit\", and \"bech32\"."}, - {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only"}, + {"includeWatching", RPCArg::Type::BOOL, /* default */ "true for watch-only wallets, otherwise false", "Also select inputs which are watch only.\n" + "Only solvable inputs can be used. Watch-only destinations are solvable if the public key and/or output script was imported,\n" + "e.g. with 'importpubkey' or 'importmulti' with the 'pubkeys' or 'desc' field."}, {"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", "A json array of integers.\n" @@ -3307,7 +3305,7 @@ static UniValue bumpfee(const JSONRPCRequest& request) "The command will fail if the wallet or mempool contains a transaction that spends one of T's outputs.\n" "By default, the new fee will be calculated automatically using estimatesmartfee.\n" "The user can specify a confirmation target for estimatesmartfee.\n" - "Alternatively, the user can specify totalFee (DEPRECATED), or use RPC settxfee to set a higher fee rate.\n" + "Alternatively, the user can specify totalFee (DEPRECATED), or fee_rate (" + CURRENCY_UNIT + " per kB) for the new transaction .\n" "At a minimum, the new fee rate must be high enough to pay an additional new relay fee (incrementalfee\n" "returned by getnetworkinfo) to enter the node's mempool.\n", { @@ -3319,6 +3317,9 @@ static UniValue bumpfee(const JSONRPCRequest& request) " In rare cases, the actual fee paid might be slightly higher than the specified\n" " totalFee if the tx change output has to be removed because it is too close to\n" " the dust threshold."}, + {"fee_rate", RPCArg::Type::NUM, /* default */ "fallback to 'confTarget'", "FeeRate (NOT total fee) to pay, in " + CURRENCY_UNIT + " per kB\n" + " Specify a fee rate instead of relying on the built-in fee estimator.\n" + " Must be at least 0.0001 BTC per kB higher than the current transaction fee rate.\n"}, {"replaceable", RPCArg::Type::BOOL, /* default */ "true", "Whether the new transaction should still be\n" " marked bip-125 replaceable. If true, the sequence numbers in the transaction will\n" " be left unchanged from the original. If false, any input sequence numbers in the\n" @@ -3360,13 +3361,15 @@ static UniValue bumpfee(const JSONRPCRequest& request) { {"confTarget", UniValueType(UniValue::VNUM)}, {"totalFee", UniValueType(UniValue::VNUM)}, + {"fee_rate", UniValueType(UniValue::VNUM)}, {"replaceable", UniValueType(UniValue::VBOOL)}, {"estimate_mode", UniValueType(UniValue::VSTR)}, }, true, true); - - if (options.exists("confTarget") && options.exists("totalFee")) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget and totalFee options should not both be set. Please provide either a confirmation target for fee estimation or an explicit total fee for the transaction."); + if (options.exists("confTarget") && (options.exists("totalFee") || options.exists("fee_rate"))) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "confTarget can't be set with totalFee or fee_rate. Please provide either a confirmation target in blocks for automatic fee estimation, or an explicit fee rate."); + } else if (options.exists("fee_rate") && options.exists("totalFee")) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "fee_rate can't be set along with totalFee."); } else if (options.exists("confTarget")) { // TODO: alias this to conf_target coin_control.m_confirm_target = ParseConfirmTarget(options["confTarget"], pwallet->chain().estimateMaxBlocks()); } else if (options.exists("totalFee")) { @@ -3377,6 +3380,12 @@ static UniValue bumpfee(const JSONRPCRequest& request) if (totalFee <= 0) { throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid totalFee %s (must be greater than 0)", FormatMoney(totalFee))); } + } else if (options.exists("fee_rate")) { + CFeeRate fee_rate(AmountFromValue(options["fee_rate"])); + if (fee_rate <= CFeeRate(0)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid fee_rate %s (must be greater than 0)", fee_rate.ToString())); + } + coin_control.m_feerate = fee_rate; } if (options.exists("replaceable")) { diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index fc3be2b6a..73523ca36 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -338,6 +338,84 @@ BOOST_AUTO_TEST_CASE(LoadReceiveRequests) BOOST_CHECK_EQUAL(values[1], "val_rr1"); } +// Test some watch-only wallet methods by the procedure of loading (LoadWatchOnly), +// checking (HaveWatchOnly), getting (GetWatchPubKey) and removing (RemoveWatchOnly) a +// given PubKey, resp. its corresponding P2PK Script. Results of the the impact on +// the address -> PubKey map is dependent on whether the PubKey is a point on the curve +static void TestWatchOnlyPubKey(CWallet& wallet, const CPubKey& add_pubkey) +{ + CScript p2pk = GetScriptForRawPubKey(add_pubkey); + CKeyID add_address = add_pubkey.GetID(); + CPubKey found_pubkey; + LOCK(wallet.cs_wallet); + + // all Scripts (i.e. also all PubKeys) are added to the general watch-only set + BOOST_CHECK(!wallet.HaveWatchOnly(p2pk)); + wallet.LoadWatchOnly(p2pk); + BOOST_CHECK(wallet.HaveWatchOnly(p2pk)); + + // only PubKeys on the curve shall be added to the watch-only address -> PubKey map + bool is_pubkey_fully_valid = add_pubkey.IsFullyValid(); + if (is_pubkey_fully_valid) { + BOOST_CHECK(wallet.GetWatchPubKey(add_address, found_pubkey)); + BOOST_CHECK(found_pubkey == add_pubkey); + } else { + BOOST_CHECK(!wallet.GetWatchPubKey(add_address, found_pubkey)); + BOOST_CHECK(found_pubkey == CPubKey()); // passed key is unchanged + } + + wallet.RemoveWatchOnly(p2pk); + BOOST_CHECK(!wallet.HaveWatchOnly(p2pk)); + + if (is_pubkey_fully_valid) { + BOOST_CHECK(!wallet.GetWatchPubKey(add_address, found_pubkey)); + BOOST_CHECK(found_pubkey == add_pubkey); // passed key is unchanged + } +} + +// Cryptographically invalidate a PubKey whilst keeping length and first byte +static void PollutePubKey(CPubKey& pubkey) +{ + std::vector<unsigned char> pubkey_raw(pubkey.begin(), pubkey.end()); + std::fill(pubkey_raw.begin()+1, pubkey_raw.end(), 0); + pubkey = CPubKey(pubkey_raw); + assert(!pubkey.IsFullyValid()); + assert(pubkey.IsValid()); +} + +// Test watch-only wallet logic for PubKeys +BOOST_AUTO_TEST_CASE(WatchOnlyPubKeys) +{ + CKey key; + CPubKey pubkey; + + BOOST_CHECK(!m_wallet.HaveWatchOnly()); + + // uncompressed valid PubKey + key.MakeNewKey(false); + pubkey = key.GetPubKey(); + assert(!pubkey.IsCompressed()); + TestWatchOnlyPubKey(m_wallet, pubkey); + + // uncompressed cryptographically invalid PubKey + PollutePubKey(pubkey); + TestWatchOnlyPubKey(m_wallet, pubkey); + + // compressed valid PubKey + key.MakeNewKey(true); + pubkey = key.GetPubKey(); + assert(pubkey.IsCompressed()); + TestWatchOnlyPubKey(m_wallet, pubkey); + + // compressed cryptographically invalid PubKey + PollutePubKey(pubkey); + TestWatchOnlyPubKey(m_wallet, pubkey); + + // invalid empty PubKey + pubkey = CPubKey(); + TestWatchOnlyPubKey(m_wallet, pubkey); +} + class ListCoinsTestingSetup : public TestChain100Setup { public: diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 7cf09d554..09f08220d 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3376,6 +3376,7 @@ DBErrors CWallet::ZapSelectTx(std::vector<uint256>& vHashIn, std::vector<uint256 const auto& it = mapWallet.find(hash); wtxOrdered.erase(it->second.m_it_wtxOrdered); mapWallet.erase(it); + NotifyTransactionChanged(this, hash, CT_DELETED); } if (nZapSelectTxRet == DBErrors::NEED_REWRITE) @@ -4420,7 +4421,6 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, walletInstance->m_min_fee = CFeeRate(n); } - walletInstance->m_allow_fallback_fee = Params().IsTestChain(); if (gArgs.IsArgSet("-fallbackfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) { @@ -4432,8 +4432,10 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain, _("This is the transaction fee you may pay when fee estimates are not available.").translated); } walletInstance->m_fallback_fee = CFeeRate(nFeePerK); - walletInstance->m_allow_fallback_fee = nFeePerK != 0; //disable fallback fee in case value was set to 0, enable if non-null value } + // Disable fallback fee in case value was set to 0, enable if non-null value + walletInstance->m_allow_fallback_fee = walletInstance->m_fallback_fee.GetFeePerK() != 0; + if (gArgs.IsArgSet("-discardfee")) { CAmount nFeePerK = 0; if (!ParseMoney(gArgs.GetArg("-discardfee", ""), nFeePerK)) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3428e8e00..006775e83 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -63,7 +63,7 @@ static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000; //! -paytxfee default constexpr CAmount DEFAULT_PAY_TX_FEE = 0; //! -fallbackfee default -static const CAmount DEFAULT_FALLBACK_FEE = 20000; +static const CAmount DEFAULT_FALLBACK_FEE = 0; //! -discardfee default static const CAmount DEFAULT_DISCARD_FEE = 10000; //! -mintxfee default @@ -120,7 +120,7 @@ enum WalletFeature }; //! Default for -addresstype -constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::P2SH_SEGWIT}; +constexpr OutputType DEFAULT_ADDRESS_TYPE{OutputType::BECH32}; //! Default for -changetype constexpr OutputType DEFAULT_CHANGE_TYPE{OutputType::CHANGE_AUTO}; @@ -1167,7 +1167,7 @@ public: unsigned int m_confirm_target{DEFAULT_TX_CONFIRM_TARGET}; bool m_spend_zero_conf_change{DEFAULT_SPEND_ZEROCONF_CHANGE}; bool m_signal_rbf{DEFAULT_WALLET_RBF}; - bool m_allow_fallback_fee{true}; //!< will be defined via chainparams + bool m_allow_fallback_fee{true}; //!< will be false if -fallbackfee=0 CFeeRate m_min_fee{DEFAULT_TRANSACTION_MINFEE}; //!< Override with -mintxfee /** * If fee estimation does not have enough data to provide estimates, use this fee instead. |