diff options
Diffstat (limited to 'src/wallet')
-rw-r--r-- | src/wallet/rpcwallet.cpp | 141 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 29 | ||||
-rw-r--r-- | src/wallet/wallet.h | 34 |
3 files changed, 151 insertions, 53 deletions
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8272bdc43f..36753d1116 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -668,8 +668,19 @@ UniValue getbalance(const JSONRPCRequest& request) "Note that the account \"\" is not the same as leaving the parameter out.\n" "The server total may be different to the balance in the default \"\" account.\n" "\nArguments:\n" - "1. \"account\" (string, optional) DEPRECATED. The selected account, or \"*\" for entire wallet. It may be the default account using \"\".\n" - "2. minconf (numeric, optional, default=1) Only include transactions confirmed at least this many times.\n" + "1. \"account\" (string, optional) DEPRECATED. The account string may be given as a\n" + " specific account name to find the balance associated with wallet keys in\n" + " a named account, or as the empty string (\"\") to find the balance\n" + " associated with wallet keys not in any named account, or as \"*\" to find\n" + " the balance associated with all wallet keys regardless of account.\n" + " When this option is specified, it calculates the balance in a different\n" + " way than when it is not specified, and which can count spends twice when\n" + " there are conflicting pending transactions (such as those created by\n" + " the bumpfee command), temporarily resulting in low or even negative\n" + " balances. In general, account balance calculation is not considered\n" + " reliable and has resulted in confusing outcomes, so it is recommended to\n" + " avoid passing this argument.\n" + "2. minconf (numeric, optional, default=1) Only include transactions confirmed at least this many times.\n" "3. include_watchonly (bool, optional, default=false) Also include balance in watch-only addresses (see 'importaddress')\n" "\nResult:\n" "amount (numeric) The total amount in " + CURRENCY_UNIT + " received for this account.\n" @@ -696,9 +707,12 @@ UniValue getbalance(const JSONRPCRequest& request) filter = filter | ISMINE_WATCH_ONLY; if (request.params[0].get_str() == "*") { - // Calculate total balance a different way from GetBalance() - // (GetBalance() sums up all unspent TxOuts) - // getbalance and "getbalance * 1 true" should return the same number + // Calculate total balance in a very different way from GetBalance(). + // The biggest difference is that GetBalance() sums up all unspent + // TxOuts paying to the wallet, while this sums up both spent and + // unspent TxOuts paying to the wallet, and then subtracts the values of + // TxIns spending from the wallet. This also has fewer restrictions on + // which unconfirmed transactions are considered trusted. CAmount nBalance = 0; for (map<uint256, CWalletTx>::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it) { @@ -2650,6 +2664,33 @@ UniValue fundrawtransaction(const JSONRPCRequest& request) return result; } +// Calculate the size of the transaction assuming all signatures are max size +// Use DummySignatureCreator, which inserts 72 byte signatures everywhere. +// TODO: re-use this in CWallet::CreateTransaction (right now +// CreateTransaction uses the constructed dummy-signed tx to do a priority +// calculation, but we should be able to refactor after priority is removed). +// NOTE: this requires that all inputs must be in mapWallet (eg the tx should +// be IsAllFromMe). +int64_t CalculateMaximumSignedTxSize(const CTransaction &tx) +{ + CMutableTransaction txNew(tx); + std::vector<pair<CWalletTx *, unsigned int>> vCoins; + // Look up the inputs. We should have already checked that this transaction + // IsAllFromMe(ISMINE_SPENDABLE), so every input should already be in our + // wallet, with a valid index into the vout array. + for (auto& input : tx.vin) { + const auto mi = pwalletMain->mapWallet.find(input.prevout.hash); + assert(mi != pwalletMain->mapWallet.end() && input.prevout.n < mi->second.tx->vout.size()); + vCoins.emplace_back(make_pair(&(mi->second), input.prevout.n)); + } + if (!pwalletMain->DummySignTx(txNew, vCoins)) { + // This should never happen, because IsAllFromMe(ISMINE_SPENDABLE) + // implies that we can sign for every input. + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction contains inputs that cannot be signed"); + } + return GetVirtualTransactionSize(txNew); +} + UniValue bumpfee(const JSONRPCRequest& request) { if (!EnsureWalletIsAvailable(request.fHelp)) { @@ -2668,8 +2709,8 @@ UniValue bumpfee(const JSONRPCRequest& request) "By default, the new fee will be calculated automatically using estimatefee.\n" "The user can specify a confirmation target for estimatefee.\n" "Alternatively, the user can specify totalFee, or use RPC setpaytxfee to set a higher fee rate.\n" - "At a minimum, the new fee rate must be high enough to pay a new relay fee (relay fee amount returned\n" - "by getnetworkinfo RPC) and to enter the node's mempool.\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" "\nArguments:\n" "1. txid (string, required) The txid to be bumped\n" "2. options (object, optional)\n" @@ -2690,8 +2731,9 @@ UniValue bumpfee(const JSONRPCRequest& request) "\nResult:\n" "{\n" " \"txid\": \"value\", (string) The id of the new transaction\n" - " \"oldfee\": n, (numeric) Fee of the replaced transaction\n" - " \"fee\": n, (numeric) Fee of the new transaction\n" + " \"origfee\": n, (numeric) Fee of the replaced transaction\n" + " \"fee\": n, (numeric) Fee of the new transaction\n" + " \"errors\": [ str... ] (json array of strings) Errors encountered during processing (may be empty)\n" "}\n" "\nExamples:\n" "\nBump the fee, get the new transaction\'s txid\n" + @@ -2755,6 +2797,10 @@ UniValue bumpfee(const JSONRPCRequest& request) throw JSONRPCError(RPC_MISC_ERROR, "Transaction does not have a change output"); } + // Calculate the expected size of the new transaction. + int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); + const int64_t maxNewTxSize = CalculateMaximumSignedTxSize(*wtx.tx); + // optional parameters bool specifiedConfirmTarget = false; int newConfirmTarget = nTxConfirmTarget; @@ -2780,10 +2826,11 @@ UniValue bumpfee(const JSONRPCRequest& request) } } else if (options.exists("totalFee")) { totalFee = options["totalFee"].get_int64(); - if (totalFee <= 0) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid totalFee (cannot be <= 0)"); - } else if (totalFee > maxTxFee) { - throw JSONRPCError(RPC_INVALID_PARAMETER, "Invalid totalFee (cannot be higher than maxTxFee)"); + CAmount requiredFee = CWallet::GetRequiredFee(maxNewTxSize); + if (totalFee < requiredFee ) { + throw JSONRPCError(RPC_INVALID_PARAMETER, + strprintf("Insufficient totalFee (cannot be less than required fee %s)", + FormatMoney(requiredFee))); } } @@ -2792,42 +2839,57 @@ UniValue bumpfee(const JSONRPCRequest& request) } } - // signature sizes can vary by a byte, so add 1 for each input when calculating the new fee - int64_t txSize = GetVirtualTransactionSize(*(wtx.tx)); - const int64_t maxNewTxSize = txSize + wtx.tx->vin.size(); - // calculate the old fee and fee-rate CAmount nOldFee = wtx.GetDebit(ISMINE_SPENDABLE) - wtx.tx->GetValueOut(); CFeeRate nOldFeeRate(nOldFee, txSize); CAmount nNewFee; CFeeRate nNewFeeRate; + // The wallet uses a 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. + CFeeRate walletIncrementalRelayFee = CFeeRate(WALLET_INCREMENTAL_RELAY_FEE); + if (::incrementalRelayFee > walletIncrementalRelayFee) { + walletIncrementalRelayFee = ::incrementalRelayFee; + } if (totalFee > 0) { - CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + minRelayTxFee.GetFee(maxNewTxSize); + CAmount minTotalFee = nOldFeeRate.GetFee(maxNewTxSize) + ::incrementalRelayFee.GetFee(maxNewTxSize); if (totalFee < minTotalFee) { - throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Invalid totalFee, must be at least %s (oldFee %s + relayFee %s)", FormatMoney(minTotalFee), nOldFeeRate.GetFee(maxNewTxSize), minRelayTxFee.GetFee(maxNewTxSize))); + throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Insufficient totalFee, must be at least %s (oldFee %s + incrementalFee %s)", + FormatMoney(minTotalFee), FormatMoney(nOldFeeRate.GetFee(maxNewTxSize)), FormatMoney(::incrementalRelayFee.GetFee(maxNewTxSize)))); } nNewFee = totalFee; nNewFeeRate = CFeeRate(totalFee, maxNewTxSize); } else { - // use the user-defined payTxFee if possible, otherwise use smartfee / fallbackfee - if (!specifiedConfirmTarget && payTxFee.GetFeePerK() != 0) { - nNewFeeRate = payTxFee; - } else { - nNewFeeRate = mempool.estimateSmartFee(newConfirmTarget); + // if user specified a confirm target then don't consider any global payTxFee + if (specifiedConfirmTarget) { + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool, CAmount(0)); } - if (nNewFeeRate.GetFeePerK() == 0) { - nNewFeeRate = CWallet::fallbackFee; + // otherwise use the regular wallet logic to select payTxFee or default confirm target + else { + nNewFee = CWallet::GetMinimumFee(maxNewTxSize, newConfirmTarget, mempool); } - // new fee rate must be at least old rate + minimum relay rate - if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + ::minRelayTxFee.GetFeePerK()) { - nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + ::minRelayTxFee.GetFeePerK()); - } + nNewFeeRate = CFeeRate(nNewFee, maxNewTxSize); - nNewFee = nNewFeeRate.GetFee(maxNewTxSize); + // New fee rate must be at least old rate + minimum incremental relay rate + // walletIncrementalRelayFee.GetFeePerK() should be exact, because it's initialized + // in that unit (fee per kb). + // However, nOldFeeRate is a calculated value from the tx fee/size, so + // add 1 satoshi to the result, because it may have been rounded down. + if (nNewFeeRate.GetFeePerK() < nOldFeeRate.GetFeePerK() + 1 + walletIncrementalRelayFee.GetFeePerK()) { + nNewFeeRate = CFeeRate(nOldFeeRate.GetFeePerK() + 1 + walletIncrementalRelayFee.GetFeePerK()); + nNewFee = nNewFeeRate.GetFee(maxNewTxSize); + } } + // Check that in all cases the new fee doesn't violate maxTxFee + if (nNewFee > maxTxFee) { + throw JSONRPCError(RPC_MISC_ERROR, + strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)", + FormatMoney(nNewFee), FormatMoney(maxTxFee))); + } + // 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 TotalFee or paytxfee too low, if fallbackfee is too low, or, perhaps, @@ -2850,7 +2912,7 @@ UniValue bumpfee(const JSONRPCRequest& request) // If the output would become dust, discard it (converting the dust to fee) poutput->nValue -= nDelta; - if (poutput->nValue <= poutput->GetDustThreshold(::minRelayTxFee)) { + if (poutput->nValue <= poutput->GetDustThreshold(::dustRelayFee)) { LogPrint("rpc", "Bumping fee and discarding dust output\n"); nNewFee += poutput->nValue; tx.vout.erase(tx.vout.begin() + nOutput); @@ -2884,23 +2946,32 @@ UniValue bumpfee(const JSONRPCRequest& request) CWalletTx wtxBumped(pwalletMain, MakeTransactionRef(std::move(tx))); wtxBumped.mapValue["replaces_txid"] = hash.ToString(); CValidationState state; - if (!pwalletMain->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state) || !state.IsValid()) { + if (!pwalletMain->CommitTransaction(wtxBumped, reservekey, g_connman.get(), state)) { + // NOTE: CommitTransaction never returns false, so this should never happen. throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Error: The transaction was rejected! Reason given: %s", state.GetRejectReason())); } + UniValue vErrors(UniValue::VARR); + if (state.IsInvalid()) { + // This can happen if the mempool rejected the transaction. Report + // what happened in the "errors" response. + vErrors.push_back(strprintf("Error: The transaction was rejected: %s", FormatStateMessage(state))); + } + // mark the original tx as bumped if (!pwalletMain->MarkReplaced(wtx.GetHash(), wtxBumped.GetHash())) { // TODO: see if JSON-RPC has a standard way of returning a response // along with an exception. It would be good to return information about // wtxBumped to the caller even if marking the original transaction // replaced does not succeed for some reason. - throw JSONRPCError(RPC_WALLET_ERROR, "Error: Created new bumpfee transaction but could not mark the original transaction as replaced."); + vErrors.push_back("Error: Created new bumpfee transaction but could not mark the original transaction as replaced."); } UniValue result(UniValue::VOBJ); result.push_back(Pair("txid", wtxBumped.GetHash().GetHex())); - result.push_back(Pair("oldfee", ValueFromAmount(nOldFee))); + result.push_back(Pair("origfee", ValueFromAmount(nOldFee))); result.push_back(Pair("fee", ValueFromAmount(nNewFee))); + result.push_back(Pair("errors", vErrors)); return result; } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index b4715622cf..a7b8022bd9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2576,28 +2576,16 @@ bool CWallet::CreateTransaction(const vector<CRecipient>& vecSend, CWalletTx& wt // BIP125 defines opt-in RBF as any nSequence < maxint-1, so // we use the highest possible value in that range (maxint-2) // to avoid conflicting with other possible uses of nSequence, - // and in the spirit of "smallest posible change from prior + // and in the spirit of "smallest possible change from prior // behavior." for (const auto& coin : setCoins) txNew.vin.push_back(CTxIn(coin.first->GetHash(),coin.second,CScript(), std::numeric_limits<unsigned int>::max() - (fWalletRbf ? 2 : 1))); // Fill in dummy signatures for fee calculation. - int nIn = 0; - for (const auto& coin : setCoins) - { - const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey; - SignatureData sigdata; - - if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) - { - strFailReason = _("Signing transaction failed"); - return false; - } else { - UpdateTransaction(txNew, nIn, sigdata); - } - - nIn++; + if (!DummySignTx(txNew, setCoins)) { + strFailReason = _("Signing transaction failed"); + return false; } unsigned int nBytes = GetVirtualTransactionSize(txNew); @@ -2802,8 +2790,13 @@ CAmount CWallet::GetRequiredFee(unsigned int nTxBytes) CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool) { - // payTxFee is user-set "I want to pay this much" - CAmount nFeeNeeded = payTxFee.GetFee(nTxBytes); + // payTxFee is the user-set global for desired feerate + return GetMinimumFee(nTxBytes, nConfirmTarget, pool, payTxFee.GetFee(nTxBytes)); +} + +CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, CAmount targetFee) +{ + CAmount nFeeNeeded = targetFee; // User didn't set: use -txconfirmtarget to estimate... if (nFeeNeeded == 0) { int estimateFoundTarget = nConfirmTarget; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a7fc05b62d..1de04ae16a 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -13,6 +13,7 @@ #include "utilstrencodings.h" #include "validationinterface.h" #include "script/ismine.h" +#include "script/sign.h" #include "wallet/crypter.h" #include "wallet/walletdb.h" #include "wallet/rpcwallet.h" @@ -48,6 +49,8 @@ static const CAmount DEFAULT_TRANSACTION_FEE = 0; static const CAmount DEFAULT_FALLBACK_FEE = 20000; //! -mintxfee default static const CAmount DEFAULT_TRANSACTION_MINFEE = 1000; +//! minimum recommended increment for BIP 125 replacement txs +static const CAmount WALLET_INCREMENTAL_RELAY_FEE = 5000; //! target minimum change amount static const CAmount MIN_CHANGE = CENT; //! final minimum change amount after paying for fees @@ -794,6 +797,8 @@ public: void ListAccountCreditDebit(const std::string& strAccount, std::list<CAccountingEntry>& entries); bool AddAccountingEntry(const CAccountingEntry&); bool AddAccountingEntry(const CAccountingEntry&, CWalletDB *pwalletdb); + template <typename ContainerType> + bool DummySignTx(CMutableTransaction &txNew, const ContainerType &coins); static CFeeRate minTxFee; static CFeeRate fallbackFee; @@ -803,6 +808,11 @@ public: */ static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool); /** + * Estimate the minimum fee considering required fee and targetFee or if 0 + * then fee estimation for nConfirmTarget + */ + static CAmount GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarget, const CTxMemPool& pool, CAmount targetFee); + /** * Return the minimum required fee taking into account the * floating relay fee and user set minimum transaction fee */ @@ -1021,4 +1031,28 @@ public: } }; +// Helper for producing a bunch of max-sized low-S signatures (eg 72 bytes) +// ContainerType is meant to hold pair<CWalletTx *, int>, and be iterable +// so that each entry corresponds to each vIn, in order. +template <typename ContainerType> +bool CWallet::DummySignTx(CMutableTransaction &txNew, const ContainerType &coins) +{ + // Fill in dummy signatures for fee calculation. + int nIn = 0; + for (const auto& coin : coins) + { + const CScript& scriptPubKey = coin.first->tx->vout[coin.second].scriptPubKey; + SignatureData sigdata; + + if (!ProduceSignature(DummySignatureCreator(this), scriptPubKey, sigdata)) + { + return false; + } else { + UpdateTransaction(txNew, nIn, sigdata); + } + + nIn++; + } + return true; +} #endif // BITCOIN_WALLET_WALLET_H |