Skip to content

Commit

Permalink
[wallet] Move maxTxFee to wallet
Browse files Browse the repository at this point in the history
This commit moves the maxtxfee setting to the wallet. There is only
one minor behavior change:

- an error message in feebumper now refers to -maxtxfee instead of
maxTxFee.
  • Loading branch information
jnewbery committed Apr 18, 2019
1 parent bb68abe commit 5c759c7
Show file tree
Hide file tree
Showing 17 changed files with 45 additions and 50 deletions.
2 changes: 1 addition & 1 deletion src/dummywallet.cpp
Expand Up @@ -24,7 +24,7 @@ class DummyWalletInit : public WalletInitInterface {
void DummyWalletInit::AddWalletOptions() const
{
std::vector<std::string> opts = {"-addresstype", "-changetype", "-disablewallet", "-discardfee=<amt>", "-fallbackfee=<amt>",
"-keypool=<n>", "-mintxfee=<amt>", "-paytxfee=<amt>", "-rescan", "-salvagewallet", "-spendzeroconfchange", "-txconfirmtarget=<n>",
"-keypool=<n>", "-maxtxfee=<amt>", "-mintxfee=<amt>", "-paytxfee=<amt>", "-rescan", "-salvagewallet", "-spendzeroconfchange", "-txconfirmtarget=<n>",
"-upgradewallet", "-wallet=<path>", "-walletbroadcast", "-walletdir=<dir>", "-walletnotify=<cmd>", "-walletrbf", "-zapwallettxes=<mode>",
"-dblogsize=<n>", "-flushwallet", "-privdb", "-walletrejectlongchains"};
gArgs.AddHiddenArgs(opts);
Expand Down
18 changes: 0 additions & 18 deletions src/init.cpp
Expand Up @@ -502,8 +502,6 @@ void SetupServerArgs()
gArgs.AddArg("-mocktime=<n>", "Replace actual time with <n> seconds since epoch (default: 0)", true, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-maxsigcachesize=<n>", strprintf("Limit sum of signature cache and script execution cache sizes to <n> MiB (default: %u)", DEFAULT_MAX_SIG_CACHE_SIZE), true, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-maxtipage=<n>", strprintf("Maximum tip age in seconds to consider node in initial block download (default: %u)", DEFAULT_MAX_TIP_AGE), true, OptionsCategory::DEBUG_TEST);
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)", // TODO move setting to wallet
CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), false, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-printpriority", strprintf("Log transaction fee per kB when mining blocks (default: %u)", DEFAULT_PRINTPRIORITY), true, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-printtoconsole", "Send trace/debug info to console (default: 1 when no -daemon. To disable logging to file, set -nodebuglogfile)", false, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-shrinkdebugfile", "Shrink debug.log file on client startup (default: 1 when no -debug)", false, OptionsCategory::DEBUG_TEST);
Expand Down Expand Up @@ -1123,22 +1121,6 @@ bool AppInitParameterInteraction()
dustRelayFee = CFeeRate(n);
}

// This is required by both the wallet and node
if (gArgs.IsArgSet("-maxtxfee"))
{
CAmount nMaxFee = 0;
if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee))
return InitError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", "")));
if (nMaxFee > HIGH_MAX_TX_FEE)
InitWarning(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction."));
maxTxFee = nMaxFee;
if (CFeeRate(maxTxFee, 1000) < ::minRelayTxFee)
{
return InitError(strprintf(_("Invalid amount for -maxtxfee=<amount>: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"),
gArgs.GetArg("-maxtxfee", ""), ::minRelayTxFee.ToString()));
}
}

fRequireStandard = !gArgs.GetBoolArg("-acceptnonstdtxn", !chainparams.RequireStandard());
if (chainparams.RequireStandard() && !fRequireStandard)
return InitError(strprintf("acceptnonstdtxn is not currently supported for %s chain", chainparams.NetworkIDString()));
Expand Down
1 change: 0 additions & 1 deletion src/interfaces/chain.cpp
Expand Up @@ -339,7 +339,6 @@ class ChainImpl : public Chain
CFeeRate relayMinFee() override { return ::minRelayTxFee; }
CFeeRate relayIncrementalFee() override { return ::incrementalRelayFee; }
CFeeRate relayDustFee() override { return ::dustRelayFee; }
CAmount maxTxFee() override { return ::maxTxFee; }
bool getPruneMode() override { return ::fPruneMode; }
bool p2pEnabled() override { return g_connman != nullptr; }
bool isReadyToBroadcast() override { return !::fImporting && !::fReindex && !IsInitialBlockDownload(); }
Expand Down
6 changes: 0 additions & 6 deletions src/interfaces/chain.h
Expand Up @@ -216,12 +216,6 @@ class Chain
//! Relay dust fee setting (-dustrelayfee), reflecting lowest rate it's economical to spend.
virtual CFeeRate relayDustFee() = 0;

//! Node max tx fee setting (-maxtxfee).
//! This could be replaced by a per-wallet max fee, as proposed at
//! https://github.com/bitcoin/bitcoin/issues/15355
//! But for the time being, wallets call this to access the node setting.
virtual CAmount maxTxFee() = 0;

//! Check if pruning is enabled.
virtual bool getPruneMode() = 0;

Expand Down
1 change: 0 additions & 1 deletion src/interfaces/node.cpp
Expand Up @@ -207,7 +207,6 @@ class NodeImpl : public Node
}
}
bool getNetworkActive() override { return g_connman && g_connman->GetNetworkActive(); }
CAmount getMaxTxFee() override { return ::maxTxFee; }
CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) override
{
FeeCalculation fee_calc;
Expand Down
3 changes: 0 additions & 3 deletions src/interfaces/node.h
Expand Up @@ -159,9 +159,6 @@ class Node
//! Get network active.
virtual bool getNetworkActive() = 0;

//! Get max tx fee.
virtual CAmount getMaxTxFee() = 0;

//! Estimate smart fee.
virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, int* returned_target = nullptr) = 0;

Expand Down
1 change: 1 addition & 0 deletions src/interfaces/wallet.cpp
Expand Up @@ -464,6 +464,7 @@ class WalletImpl : public Wallet
bool IsWalletFlagSet(uint64_t flag) override { return m_wallet->IsWalletFlagSet(flag); }
OutputType getDefaultAddressType() override { return m_wallet->m_default_address_type; }
OutputType getDefaultChangeType() override { return m_wallet->m_default_change_type; }
CAmount getDefaultMaxTxFee() override { return m_wallet->m_default_max_tx_fee; }
void remove() override
{
RemoveWallet(m_wallet);
Expand Down
3 changes: 3 additions & 0 deletions src/interfaces/wallet.h
Expand Up @@ -247,6 +247,9 @@ class Wallet
// Get default change type.
virtual OutputType getDefaultChangeType() = 0;

//! Get max tx fee.
virtual CAmount getDefaultMaxTxFee() = 0;

// Remove wallet.
virtual void remove() = 0;

Expand Down
2 changes: 1 addition & 1 deletion src/qt/sendcoinsdialog.cpp
Expand Up @@ -578,7 +578,7 @@ void SendCoinsDialog::processSendCoinsReturn(const WalletModel::SendCoinsReturn
msgParams.second = CClientUIInterface::MSG_ERROR;
break;
case WalletModel::AbsurdFee:
msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->node().getMaxTxFee()));
msgParams.first = tr("A fee higher than %1 is considered an absurdly high fee.").arg(BitcoinUnits::formatWithUnit(model->getOptionsModel()->getDisplayUnit(), model->wallet().getDefaultMaxTxFee()));
break;
case WalletModel::PaymentRequestExpired:
msgParams.first = tr("Payment request expired.");
Expand Down
4 changes: 2 additions & 2 deletions src/qt/walletmodel.cpp
Expand Up @@ -222,9 +222,9 @@ WalletModel::SendCoinsReturn WalletModel::prepareTransaction(WalletModelTransact
}

// reject absurdly high fee. (This can never happen because the
// wallet caps the fee at maxTxFee. This merely serves as a
// wallet caps the fee at m_default_max_tx_fee. This merely serves as a
// belt-and-suspenders check)
if (nFeeRequired > m_node.getMaxTxFee())
if (nFeeRequired > m_wallet->getDefaultMaxTxFee())
return AbsurdFee;
}

Expand Down
1 change: 0 additions & 1 deletion src/validation.cpp
Expand Up @@ -252,7 +252,6 @@ uint256 hashAssumeValid;
arith_uint256 nMinimumChainWork;

CFeeRate minRelayTxFee = CFeeRate(DEFAULT_MIN_RELAY_TX_FEE);
CAmount maxTxFee = DEFAULT_TRANSACTION_MAXFEE;

CBlockPolicyEstimator feeEstimator;
CTxMemPool mempool(&feeEstimator);
Expand Down
8 changes: 0 additions & 8 deletions src/validation.h
Expand Up @@ -53,12 +53,6 @@ static const bool DEFAULT_WHITELISTRELAY = true;
static const bool DEFAULT_WHITELISTFORCERELAY = false;
/** Default for -minrelaytxfee, minimum relay fee for transactions */
static const unsigned int DEFAULT_MIN_RELAY_TX_FEE = 1000;
//! -maxtxfee default
static const CAmount DEFAULT_TRANSACTION_MAXFEE = COIN / 10;
//! Discourage users to set fees higher than this amount (in satoshis) per kB
static const CAmount HIGH_TX_FEE_PER_KB = COIN / 100;
//! -maxtxfee will warn if called with a higher fee than this amount (in satoshis)
static const CAmount HIGH_MAX_TX_FEE = 100 * HIGH_TX_FEE_PER_KB;
/** Default for -limitancestorcount, max number of in-mempool ancestors */
static const unsigned int DEFAULT_ANCESTOR_LIMIT = 25;
/** Default for -limitancestorsize, maximum kilobytes of tx + all in-mempool ancestors */
Expand Down Expand Up @@ -162,8 +156,6 @@ extern bool fCheckpointsEnabled;
extern size_t nCoinCacheUsage;
/** A fee rate smaller than this is considered zero fee (for relaying, mining and transaction creation) */
extern CFeeRate minRelayTxFee;
/** Absolute maximum transaction fee (in satoshis) used by wallet and mempool (rejects high fee in sendrawtransaction) */
extern CAmount maxTxFee;
/** If the tip is older than this (in seconds), the node is considered to be in initial block download. */
extern int64_t nMaxTipAge;
extern bool fEnableReplacement;
Expand Down
4 changes: 2 additions & 2 deletions src/wallet/feebumper.cpp
Expand Up @@ -162,9 +162,9 @@ Result CreateTransaction(const CWallet* wallet, const uint256& txid, const CCoin
}

// Check that in all cases the new fee doesn't violate maxTxFee
const CAmount max_tx_fee = wallet->chain().maxTxFee();
const CAmount max_tx_fee = wallet->m_default_max_tx_fee;
if (new_fee > max_tx_fee) {
errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than maxTxFee %s)",
errors.push_back(strprintf("Specified or calculated fee %s is too high (cannot be higher than -maxtxfee %s)",
FormatMoney(new_fee), FormatMoney(max_tx_fee)));
return Result::WALLET_ERROR;
}
Expand Down
2 changes: 1 addition & 1 deletion src/wallet/fees.cpp
Expand Up @@ -22,7 +22,7 @@ CAmount GetMinimumFee(const CWallet& wallet, unsigned int nTxBytes, const CCoinC
{
CAmount fee_needed = GetMinimumFeeRate(wallet, coin_control, feeCalc).GetFee(nTxBytes);
// Always obey the maximum
const CAmount max_tx_fee = wallet.chain().maxTxFee();
const CAmount max_tx_fee = wallet.m_default_max_tx_fee;
if (fee_needed > max_tx_fee) {
fee_needed = max_tx_fee;
if (feeCalc) feeCalc->reason = FeeReason::MAXTXFEE;
Expand Down
6 changes: 2 additions & 4 deletions src/wallet/init.cpp
Expand Up @@ -48,6 +48,8 @@ void WalletInit::AddWalletOptions() const
gArgs.AddArg("-fallbackfee=<amt>", strprintf("A fee rate (in %s/kB) that will be used when fee estimation has insufficient data (default: %s)",
CURRENCY_UNIT, FormatMoney(DEFAULT_FALLBACK_FEE)), false, OptionsCategory::WALLET);
gArgs.AddArg("-keypool=<n>", strprintf("Set key pool size to <n> (default: %u)", DEFAULT_KEYPOOL_SIZE), false, 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)",
CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MAXFEE)), false, OptionsCategory::DEBUG_TEST);
gArgs.AddArg("-mintxfee=<amt>", strprintf("Fees (in %s/kB) smaller than this are considered zero fee for transaction creation (default: %s)",
CURRENCY_UNIT, FormatMoney(DEFAULT_TRANSACTION_MINFEE)), false, OptionsCategory::WALLET);
gArgs.AddArg("-paytxfee=<amt>", strprintf("Fee (in %s/kB) to add to transactions you send (default: %s)",
Expand Down Expand Up @@ -124,10 +126,6 @@ bool WalletInit::ParameterInteraction() const
if (gArgs.GetArg("-prune", 0) && gArgs.GetBoolArg("-rescan", false))
return InitError(_("Rescans are not possible in pruned mode. You will need to use -reindex which will download the whole blockchain again."));

if (::minRelayTxFee.GetFeePerK() > HIGH_TX_FEE_PER_KB)
InitWarning(AmountHighWarn("-minrelaytxfee") + " " +
_("The wallet will avoid paying less than the minimum relay fee."));

return true;
}

Expand Down
25 changes: 24 additions & 1 deletion src/wallet/wallet.cpp
Expand Up @@ -4204,6 +4204,29 @@ std::shared_ptr<CWallet> CWallet::CreateWalletFromFile(interfaces::Chain& chain,
return nullptr;
}
}

if (gArgs.IsArgSet("-maxtxfee"))
{
CAmount nMaxFee = 0;
if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) {
chain.initError(AmountErrMsg("maxtxfee", gArgs.GetArg("-maxtxfee", "")));
return nullptr;
}
if (nMaxFee > HIGH_MAX_TX_FEE) {
chain.initWarning(_("-maxtxfee is set very high! Fees this large could be paid on a single transaction."));
}
if (CFeeRate(nMaxFee, 1000) < chain.relayMinFee()) {
chain.initError(strprintf(_("Invalid amount for -maxtxfee=<amount>: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"),
gArgs.GetArg("-maxtxfee", ""), chain.relayMinFee().ToString()));
return nullptr;
}
walletInstance->m_default_max_tx_fee = nMaxFee;
}

if (chain.relayMinFee().GetFeePerK() > HIGH_TX_FEE_PER_KB)
chain.initWarning(AmountHighWarn("-minrelaytxfee") + " " +
_("The wallet will avoid paying less than the minimum relay fee."));

walletInstance->m_confirm_target = gArgs.GetArg("-txconfirmtarget", DEFAULT_TX_CONFIRM_TARGET);
walletInstance->m_spend_zero_conf_change = gArgs.GetBoolArg("-spendzeroconfchange", DEFAULT_SPEND_ZEROCONF_CHANGE);
walletInstance->m_signal_rbf = gArgs.GetBoolArg("-walletrbf", DEFAULT_WALLET_RBF);
Expand Down Expand Up @@ -4394,7 +4417,7 @@ bool CWalletTx::AcceptToMemoryPool(interfaces::Chain::Lock& locked_chain, CValid
// user could call sendmoney in a loop and hit spurious out of funds errors
// because we think that this newly generated transaction's change is
// unavailable as we're not yet aware that it is in the mempool.
bool ret = locked_chain.submitToMemoryPool(tx, pwallet->chain().maxTxFee(), state);
bool ret = locked_chain.submitToMemoryPool(tx, pwallet->m_default_max_tx_fee, state);
fInMempool |= ret;
return ret;
}
Expand Down
8 changes: 8 additions & 0 deletions src/wallet/wallet.h
Expand Up @@ -73,6 +73,12 @@ static const unsigned int DEFAULT_TX_CONFIRM_TARGET = 6;
static const bool DEFAULT_WALLET_RBF = false;
static const bool DEFAULT_WALLETBROADCAST = true;
static const bool DEFAULT_DISABLE_WALLET = false;
//! -maxtxfee default
constexpr CAmount DEFAULT_TRANSACTION_MAXFEE{COIN / 10};
//! Discourage users to set fees higher than this amount (in satoshis) per kB
constexpr CAmount HIGH_TX_FEE_PER_KB{COIN / 100};
//! -maxtxfee will warn if called with a higher fee than this amount (in satoshis)
constexpr CAmount HIGH_MAX_TX_FEE{100 * HIGH_TX_FEE_PER_KB};

//! Pre-calculated constants for input size estimation in *virtual size*
static constexpr size_t DUMMY_NESTED_P2WPKH_INPUT_SIZE = 91;
Expand Down Expand Up @@ -983,6 +989,8 @@ class CWallet final : public CCryptoKeyStore, private interfaces::Chain::Notific
CFeeRate m_discard_rate{DEFAULT_DISCARD_FEE};
OutputType m_default_address_type{DEFAULT_ADDRESS_TYPE};
OutputType m_default_change_type{DEFAULT_CHANGE_TYPE};
/** Absolute maximum transaction fee (in satoshis) used by default for the wallet */
CAmount m_default_max_tx_fee{DEFAULT_TRANSACTION_MAXFEE};

bool NewKeyPool();
size_t KeypoolCountExternalKeys() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
Expand Down

0 comments on commit 5c759c7

Please sign in to comment.