aboutsummaryrefslogtreecommitdiff
path: root/src/wallet
diff options
context:
space:
mode:
Diffstat (limited to 'src/wallet')
-rw-r--r--src/wallet/rpcwallet.cpp141
-rw-r--r--src/wallet/wallet.cpp29
-rw-r--r--src/wallet/wallet.h34
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