Skip to content

Commit 603fcc0

Browse files
fanquakefanquake
committed
Merge bitcoin/bitcoin#31896: refactor: Remove redundant and confusing calls to IsArgSet
0000fb3 doc: Remove outdated and stale todo comment (MarcoFalke) fa2b529 refactor: Remove redundant call to IsArgSet (MarcoFalke) fa29842 refactor: Remove IsArgSet guard when fallback value is provided (MarcoFalke) Pull request description: `IsArgSet` is problematic: * It returns whether an arg has been set, even if it has been negated. `IsArgSet` is sometimes used to check for a truthy value, which is wrong, but usually harmless. Cleanup of those cases may or may not be done in a follow-up. * In most other cases, calling it is redundant, because the immediately following `Get*Arg` calls can already return an `std::optional` nullopt value to indicate an unset arg. So relieve both issues by removing all `IsArgSet` that are redundant. ACKs for top commit: pablomartin4btc: re-ACK 0000fb3 ryanofsky: Code review ACK 0000fb3. No changes since last review other than rebase. Tree-SHA512: d142d71d136b2dbd5fd005667875099777704176f5e08fdeb38f05d6afce40b435a257c5bb6a1f545459fe4f81f967cee3083ab666cb0befdef3f6234f1e3d32
2 parents 84bbb40 + 0000fb3 commit 603fcc0

File tree

5 files changed

+42
-45
lines changed

5 files changed

+42
-45
lines changed

‎src/bitcoin-cli.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,7 @@ static void ParseGetInfoResult(UniValue& result)
10591059
}
10601060
#endif
10611061

1062-
if (gArgs.IsArgSet("-color")) {
1062+
{
10631063
const std::string color{gArgs.GetArg("-color", DEFAULT_COLOR_SETTING)};
10641064
if (color == "always") {
10651065
should_colorize = true;

‎src/init.cpp‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1036,22 +1036,20 @@ bool AppInitParameterInteraction(const ArgsManager& args)
10361036
return InitError(Untranslated("peertimeout must be a positive integer."));
10371037
}
10381038

1039-
// Sanity check argument for min fee for including tx in block
1040-
// 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", "")));
1039+
if (const auto arg{args.GetArg("-blockmintxfee")}) {
1040+
if (!ParseMoney(*arg)) {
1041+
return InitError(AmountErrMsg("blockmintxfee", *arg));
10441042
}
10451043
}
10461044

1047-
if (args.IsArgSet("-blockmaxweight")) {
1045+
{
10481046
const auto max_block_weight = args.GetIntArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT);
10491047
if (max_block_weight > MAX_BLOCK_WEIGHT) {
10501048
return InitError(strprintf(_("Specified -blockmaxweight (%d) exceeds consensus maximum block weight (%d)"), max_block_weight, MAX_BLOCK_WEIGHT));
10511049
}
10521050
}
10531051

1054-
if (args.IsArgSet("-blockreservedweight")) {
1052+
{
10551053
const auto block_reserved_weight = args.GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT);
10561054
if (block_reserved_weight > MAX_BLOCK_WEIGHT) {
10571055
return InitError(strprintf(_("Specified -blockreservedweight (%d) exceeds consensus maximum block weight (%d)"), block_reserved_weight, MAX_BLOCK_WEIGHT));
@@ -1183,11 +1181,10 @@ bool CheckHostPortOptions(const ArgsManager& args) {
11831181
"-port",
11841182
"-rpcport",
11851183
}) {
1186-
if (args.IsArgSet(port_option)) {
1187-
const std::string port = args.GetArg(port_option, "");
1184+
if (const auto port{args.GetArg(port_option)}) {
11881185
uint16_t n;
1189-
if (!ParseUInt16(port, &n) || n == 0) {
1190-
return InitError(InvalidPortErrMsg(port_option, port));
1186+
if (!ParseUInt16(*port, &n) || n == 0) {
1187+
return InitError(InvalidPortErrMsg(port_option, *port));
11911188
}
11921189
}
11931190
}

‎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)