Skip to content

Commit fa2b529

Browse files
author
MarcoFalke
committed
refactor: Remove redundant call to IsArgSet
Checking for IsArgSet before calling GetArg while providing an arbitrary default value as fallback is both confusing and fragile. It is confusing, because the provided fallback is dead code. So it would be better to just call GetArg without a fallback. Even better would be to provide the true fallback value and sanitize it as if it were user-input, but this can be done in a follow-up. Removing the redundant call to IsArgSet will have to be done either way, so do it now.
1 parent fa29842 commit fa2b529

File tree

4 files changed

+39
-40
lines changed

4 files changed

+39
-40
lines changed

‎src/init.cpp

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1038,9 +1038,9 @@ bool AppInitParameterInteraction(const ArgsManager& args)
10381038

10391039
// Sanity check argument for min fee for including tx in block
10401040
// TODO: Harmonize which arguments need sanity checking and where that happens
1041-
if (args.IsArgSet("-blockmintxfee")) {
1042-
if (!ParseMoney(args.GetArg("-blockmintxfee", ""))) {
1043-
return InitError(AmountErrMsg("blockmintxfee", args.GetArg("-blockmintxfee", "")));
1041+
if (const auto arg{args.GetArg("-blockmintxfee")}) {
1042+
if (!ParseMoney(*arg)) {
1043+
return InitError(AmountErrMsg("blockmintxfee", *arg));
10441044
}
10451045
}
10461046

@@ -1183,11 +1183,10 @@ bool CheckHostPortOptions(const ArgsManager& args) {
11831183
"-port",
11841184
"-rpcport",
11851185
}) {
1186-
if (args.IsArgSet(port_option)) {
1187-
const std::string port = args.GetArg(port_option, "");
1186+
if (const auto port{args.GetArg(port_option)}) {
11881187
uint16_t n;
1189-
if (!ParseUInt16(port, &n) || n == 0) {
1190-
return InitError(InvalidPortErrMsg(port_option, port));
1188+
if (!ParseUInt16(*port, &n) || n == 0) {
1189+
return InitError(InvalidPortErrMsg(port_option, *port));
11911190
}
11921191
}
11931192
}

‎src/node/mempool_args.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,20 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP
4848

4949
// incremental relay fee sets the minimum feerate increase necessary for replacement in the mempool
5050
// and the amount the mempool min fee increases above the feerate of txs evicted due to mempool limiting.
51-
if (argsman.IsArgSet("-incrementalrelayfee")) {
52-
if (std::optional<CAmount> inc_relay_fee = ParseMoney(argsman.GetArg("-incrementalrelayfee", ""))) {
51+
if (const auto arg{argsman.GetArg("-incrementalrelayfee")}) {
52+
if (std::optional<CAmount> inc_relay_fee = ParseMoney(*arg)) {
5353
mempool_opts.incremental_relay_feerate = CFeeRate{inc_relay_fee.value()};
5454
} else {
55-
return util::Error{AmountErrMsg("incrementalrelayfee", argsman.GetArg("-incrementalrelayfee", ""))};
55+
return util::Error{AmountErrMsg("incrementalrelayfee", *arg)};
5656
}
5757
}
5858

59-
if (argsman.IsArgSet("-minrelaytxfee")) {
60-
if (std::optional<CAmount> min_relay_feerate = ParseMoney(argsman.GetArg("-minrelaytxfee", ""))) {
59+
if (const auto arg{argsman.GetArg("-minrelaytxfee")}) {
60+
if (std::optional<CAmount> min_relay_feerate = ParseMoney(*arg)) {
6161
// High fee check is done afterward in CWallet::Create()
6262
mempool_opts.min_relay_feerate = CFeeRate{min_relay_feerate.value()};
6363
} else {
64-
return util::Error{AmountErrMsg("minrelaytxfee", argsman.GetArg("-minrelaytxfee", ""))};
64+
return util::Error{AmountErrMsg("minrelaytxfee", *arg)};
6565
}
6666
} else if (mempool_opts.incremental_relay_feerate > mempool_opts.min_relay_feerate) {
6767
// Allow only setting incremental fee to control both
@@ -71,11 +71,11 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP
7171

7272
// Feerate used to define dust. Shouldn't be changed lightly as old
7373
// implementations may inadvertently create non-standard transactions
74-
if (argsman.IsArgSet("-dustrelayfee")) {
75-
if (std::optional<CAmount> parsed = ParseMoney(argsman.GetArg("-dustrelayfee", ""))) {
74+
if (const auto arg{argsman.GetArg("-dustrelayfee")}) {
75+
if (std::optional<CAmount> parsed = ParseMoney(*arg)) {
7676
mempool_opts.dust_relay_feerate = CFeeRate{parsed.value()};
7777
} else {
78-
return util::Error{AmountErrMsg("dustrelayfee", argsman.GetArg("-dustrelayfee", ""))};
78+
return util::Error{AmountErrMsg("dustrelayfee", *arg)};
7979
}
8080
}
8181

‎src/qt/intro.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,9 @@ Intro::Intro(QWidget *parent, int64_t blockchain_size_gb, int64_t chain_state_si
140140

141141
const int min_prune_target_GB = std::ceil(MIN_DISK_SPACE_FOR_BLOCK_FILES / 1e9);
142142
ui->pruneGB->setRange(min_prune_target_GB, std::numeric_limits<int>::max());
143-
if (gArgs.IsArgSet("-prune")) {
143+
if (const auto arg{gArgs.GetIntArg("-prune")}) {
144144
m_prune_checkbox_is_default = false;
145-
ui->prune->setChecked(gArgs.GetIntArg("-prune", 0) >= 1);
145+
ui->prune->setChecked(*arg >= 1);
146146
ui->prune->setEnabled(false);
147147
}
148148
ui->pruneGB->setValue(m_prune_target_gb);

‎src/wallet/wallet.cpp

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3124,10 +3124,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
31243124
walletInstance->m_default_change_type = parsed.value();
31253125
}
31263126

3127-
if (args.IsArgSet("-mintxfee")) {
3128-
std::optional<CAmount> min_tx_fee = ParseMoney(args.GetArg("-mintxfee", ""));
3127+
if (const auto arg{args.GetArg("-mintxfee")}) {
3128+
std::optional<CAmount> min_tx_fee = ParseMoney(*arg);
31293129
if (!min_tx_fee) {
3130-
error = AmountErrMsg("mintxfee", args.GetArg("-mintxfee", ""));
3130+
error = AmountErrMsg("mintxfee", *arg);
31313131
return nullptr;
31323132
} else if (min_tx_fee.value() > HIGH_TX_FEE_PER_KB) {
31333133
warnings.push_back(AmountHighWarn("-mintxfee") + Untranslated(" ") +
@@ -3137,8 +3137,8 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
31373137
walletInstance->m_min_fee = CFeeRate{min_tx_fee.value()};
31383138
}
31393139

3140-
if (args.IsArgSet("-maxapsfee")) {
3141-
const std::string max_aps_fee{args.GetArg("-maxapsfee", "")};
3140+
if (const auto arg{args.GetArg("-maxapsfee")}) {
3141+
const std::string& max_aps_fee{*arg};
31423142
if (max_aps_fee == "-1") {
31433143
walletInstance->m_max_aps_fee = -1;
31443144
} else if (std::optional<CAmount> max_fee = ParseMoney(max_aps_fee)) {
@@ -3153,10 +3153,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
31533153
}
31543154
}
31553155

3156-
if (args.IsArgSet("-fallbackfee")) {
3157-
std::optional<CAmount> fallback_fee = ParseMoney(args.GetArg("-fallbackfee", ""));
3156+
if (const auto arg{args.GetArg("-fallbackfee")}) {
3157+
std::optional<CAmount> fallback_fee = ParseMoney(*arg);
31583158
if (!fallback_fee) {
3159-
error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-fallbackfee", args.GetArg("-fallbackfee", ""));
3159+
error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-fallbackfee", *arg);
31603160
return nullptr;
31613161
} else if (fallback_fee.value() > HIGH_TX_FEE_PER_KB) {
31623162
warnings.push_back(AmountHighWarn("-fallbackfee") + Untranslated(" ") +
@@ -3168,10 +3168,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
31683168
// Disable fallback fee in case value was set to 0, enable if non-null value
31693169
walletInstance->m_allow_fallback_fee = walletInstance->m_fallback_fee.GetFeePerK() != 0;
31703170

3171-
if (args.IsArgSet("-discardfee")) {
3172-
std::optional<CAmount> discard_fee = ParseMoney(args.GetArg("-discardfee", ""));
3171+
if (const auto arg{args.GetArg("-discardfee")}) {
3172+
std::optional<CAmount> discard_fee = ParseMoney(*arg);
31733173
if (!discard_fee) {
3174-
error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-discardfee", args.GetArg("-discardfee", ""));
3174+
error = strprintf(_("Invalid amount for %s=<amount>: '%s'"), "-discardfee", *arg);
31753175
return nullptr;
31763176
} else if (discard_fee.value() > HIGH_TX_FEE_PER_KB) {
31773177
warnings.push_back(AmountHighWarn("-discardfee") + Untranslated(" ") +
@@ -3180,12 +3180,12 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
31803180
walletInstance->m_discard_rate = CFeeRate{discard_fee.value()};
31813181
}
31823182

3183-
if (args.IsArgSet("-paytxfee")) {
3183+
if (const auto arg{args.GetArg("-paytxfee")}) {
31843184
warnings.push_back(_("-paytxfee is deprecated and will be fully removed in v31.0."));
31853185

3186-
std::optional<CAmount> pay_tx_fee = ParseMoney(args.GetArg("-paytxfee", ""));
3186+
std::optional<CAmount> pay_tx_fee = ParseMoney(*arg);
31873187
if (!pay_tx_fee) {
3188-
error = AmountErrMsg("paytxfee", args.GetArg("-paytxfee", ""));
3188+
error = AmountErrMsg("paytxfee", *arg);
31893189
return nullptr;
31903190
} else if (pay_tx_fee.value() > HIGH_TX_FEE_PER_KB) {
31913191
warnings.push_back(AmountHighWarn("-paytxfee") + Untranslated(" ") +
@@ -3196,34 +3196,34 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
31963196

31973197
if (chain && walletInstance->m_pay_tx_fee < chain->relayMinFee()) {
31983198
error = strprintf(_("Invalid amount for %s=<amount>: '%s' (must be at least %s)"),
3199-
"-paytxfee", args.GetArg("-paytxfee", ""), chain->relayMinFee().ToString());
3199+
"-paytxfee", *arg, chain->relayMinFee().ToString());
32003200
return nullptr;
32013201
}
32023202
}
32033203

3204-
if (args.IsArgSet("-maxtxfee")) {
3205-
std::optional<CAmount> max_fee = ParseMoney(args.GetArg("-maxtxfee", ""));
3204+
if (const auto arg{args.GetArg("-maxtxfee")}) {
3205+
std::optional<CAmount> max_fee = ParseMoney(*arg);
32063206
if (!max_fee) {
3207-
error = AmountErrMsg("maxtxfee", args.GetArg("-maxtxfee", ""));
3207+
error = AmountErrMsg("maxtxfee", *arg);
32083208
return nullptr;
32093209
} else if (max_fee.value() > HIGH_MAX_TX_FEE) {
32103210
warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee"));
32113211
}
32123212

32133213
if (chain && CFeeRate{max_fee.value(), 1000} < chain->relayMinFee()) {
32143214
error = strprintf(_("Invalid amount for %s=<amount>: '%s' (must be at least the minrelay fee of %s to prevent stuck transactions)"),
3215-
"-maxtxfee", args.GetArg("-maxtxfee", ""), chain->relayMinFee().ToString());
3215+
"-maxtxfee", *arg, chain->relayMinFee().ToString());
32163216
return nullptr;
32173217
}
32183218

32193219
walletInstance->m_default_max_tx_fee = max_fee.value();
32203220
}
32213221

3222-
if (args.IsArgSet("-consolidatefeerate")) {
3223-
if (std::optional<CAmount> consolidate_feerate = ParseMoney(args.GetArg("-consolidatefeerate", ""))) {
3222+
if (const auto arg{args.GetArg("-consolidatefeerate")}) {
3223+
if (std::optional<CAmount> consolidate_feerate = ParseMoney(*arg)) {
32243224
walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate);
32253225
} else {
3226-
error = AmountErrMsg("consolidatefeerate", args.GetArg("-consolidatefeerate", ""));
3226+
error = AmountErrMsg("consolidatefeerate", *arg);
32273227
return nullptr;
32283228
}
32293229
}

0 commit comments

Comments
 (0)