Merge fa0cf040baa74b161b4b00ddea95bfb5aac0bb53 into cd8089c20baaaee329cbf7951195953a9f86d7cf

doc: Remove outdated and stale todo comment
If anything is left to be done, a new discussion issue or pull request can be created.
2025-03-17 21:32:00 +01:00 · 2025-03-16 15:39:15 +01:00 · 2025-03-16 14:18:06 +01:00 · 2025-02-18 15:45:28 +01:00 · 2025-02-18 12:52:14 +01:00
5 changed files with 42 additions and 45 deletions
--- a/src/bitcoin-cli.cpp
+++ b/src/bitcoin-cli.cpp
@ -1078,7 +1078,7 @@ static void ParseGetInfoResult(UniValue& result)
    }
 #endif

-    if (gArgs.IsArgSet("-color")) {
+    {
        const std::string color{gArgs.GetArg("-color", DEFAULT_COLOR_SETTING)};
        if (color == "always") {
            should_colorize = true;
--- a/src/init.cpp
+++ b/src/init.cpp
@ -1036,22 +1036,20 @@ bool AppInitParameterInteraction(const ArgsManager& args)
        return InitError(Untranslated("peertimeout must be a positive integer."));
    }

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

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

-    if (args.IsArgSet("-blockreservedweight")) {
+    {
        const auto block_reserved_weight = args.GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT);
        if (block_reserved_weight > MAX_BLOCK_WEIGHT) {
            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) {
        "-port",
        "-rpcport",
    }) {
-        if (args.IsArgSet(port_option)) {
-            const std::string port = args.GetArg(port_option, "");
+        if (const auto port{args.GetArg(port_option)}) {
            uint16_t n;
-            if (!ParseUInt16(port, &n) || n == 0) {
-                return InitError(InvalidPortErrMsg(port_option, port));
+            if (!ParseUInt16(*port, &n) || n == 0) {
+                return InitError(InvalidPortErrMsg(port_option, *port));
            }
        }
    }
--- a/src/node/mempool_args.cpp
+++ b/src/node/mempool_args.cpp
@ -48,20 +48,20 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& argsman, const CChainP

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

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

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

--- a/src/qt/intro.cpp
+++ b/src/qt/intro.cpp
@ -140,9 +140,9 @@ Intro::Intro(QWidget *parent, int64_t blockchain_size_gb, int64_t chain_state_si

    const int min_prune_target_GB = std::ceil(MIN_DISK_SPACE_FOR_BLOCK_FILES / 1e9);
    ui->pruneGB->setRange(min_prune_target_GB, std::numeric_limits<int>::max());
-    if (gArgs.IsArgSet("-prune")) {
+    if (const auto arg{gArgs.GetIntArg("-prune")}) {
        m_prune_checkbox_is_default = false;
-        ui->prune->setChecked(gArgs.GetIntArg("-prune", 0) >= 1);
+        ui->prune->setChecked(*arg >= 1);
        ui->prune->setEnabled(false);
    }
    ui->pruneGB->setValue(m_prune_target_gb);
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@ -3124,10 +3124,10 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri
        walletInstance->m_default_change_type = parsed.value();
    }

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

-    if (args.IsArgSet("-maxapsfee")) {
-        const std::string max_aps_fee{args.GetArg("-maxapsfee", "")};
+    if (const auto arg{args.GetArg("-maxapsfee")}) {
+        const std::string& max_aps_fee{*arg};
        if (max_aps_fee == "-1") {
            walletInstance->m_max_aps_fee = -1;
        } 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
        }
    }

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

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

-    if (args.IsArgSet("-paytxfee")) {
-        std::optional<CAmount> pay_tx_fee = ParseMoney(args.GetArg("-paytxfee", ""));
+    if (const auto arg{args.GetArg("-paytxfee")}) {
+        std::optional<CAmount> pay_tx_fee = ParseMoney(*arg);
        if (!pay_tx_fee) {
-            error = AmountErrMsg("paytxfee", args.GetArg("-paytxfee", ""));
+            error = AmountErrMsg("paytxfee", *arg);
            return nullptr;
        } else if (pay_tx_fee.value() > HIGH_TX_FEE_PER_KB) {
            warnings.push_back(AmountHighWarn("-paytxfee") + Untranslated(" ") +
@ -3194,15 +3194,15 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri

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

-    if (args.IsArgSet("-maxtxfee")) {
-        std::optional<CAmount> max_fee = ParseMoney(args.GetArg("-maxtxfee", ""));
+    if (const auto arg{args.GetArg("-maxtxfee")}) {
+        std::optional<CAmount> max_fee = ParseMoney(*arg);
        if (!max_fee) {
-            error = AmountErrMsg("maxtxfee", args.GetArg("-maxtxfee", ""));
+            error = AmountErrMsg("maxtxfee", *arg);
            return nullptr;
        } else if (max_fee.value() > HIGH_MAX_TX_FEE) {
            warnings.push_back(strprintf(_("%s is set very high! Fees this large could be paid on a single transaction."), "-maxtxfee"));
@ -3210,18 +3210,18 @@ std::shared_ptr<CWallet> CWallet::Create(WalletContext& context, const std::stri

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

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

-    if (args.IsArgSet("-consolidatefeerate")) {
-        if (std::optional<CAmount> consolidate_feerate = ParseMoney(args.GetArg("-consolidatefeerate", ""))) {
+    if (const auto arg{args.GetArg("-consolidatefeerate")}) {
+        if (std::optional<CAmount> consolidate_feerate = ParseMoney(*arg)) {
            walletInstance->m_consolidate_feerate = CFeeRate(*consolidate_feerate);
        } else {
-            error = AmountErrMsg("consolidatefeerate", args.GetArg("-consolidatefeerate", ""));
+            error = AmountErrMsg("consolidatefeerate", *arg);
            return nullptr;
        }
    }
Author	SHA1	Message	Date
maflcko	14e92e76fe	Merge fa0cf040baa74b161b4b00ddea95bfb5aac0bb53 into cd8089c20baaaee329cbf7951195953a9f86d7cf	2025-03-16 15:39:15 +01:00
MarcoFalke	fa0cf040ba	doc: Remove outdated and stale todo comment If anything is left to be done, a new discussion issue or pull request can be created.	2025-03-16 14:18:06 +01:00
MarcoFalke	fa3fb3c23f	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.	2025-02-18 15:45:28 +01:00
MarcoFalke	fa41685f43	refactor: Remove IsArgSet guard when fallback value is provided Checking for IsArgSet before calling GetArg while providing the args 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. However, ignoring the fallback value is fragile, because it would not be sanitized. Fix all issues by sanitizing the fallback value.	2025-02-18 12:52:14 +01:00