Repository: bitcoin/bitcoin
File: src/node/interfaces.cpp (C++)

Code before:
        args().LockSettings([&](const common::Settings& settings) {
            if (const common::SettingsValue* value = common::FindKey(settings.rw_settings, name)) {
                result = *value;
            }
        });
        return result;
    }
    bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write) override
    {
        args().LockSettings([&](common::Settings& settings) {
            if (value.isNull()) {
                settings.rw_settings.erase(name);
            } else {
                settings.rw_settings[name] = value;
            }
        });
        return !write || args().WriteSettingsFile();
    }
    void requestMempoolTransactions(Notifications& notifications) override
    {
        if (!m_node.mempool) return;
        LOCK2(::cs_main, m_node.mempool->cs);
        for (const CTxMemPoolEntry& entry : m_node.mempool->entryAll()) {
            notifications.transactionAddedToMempool(entry.GetSharedTx());
        }
    }
    bool hasAssumedValidChain() override
    {
        return chainman().IsSnapshotActive();
    }

    NodeContext* context() override { return &m_node; }
    ArgsManager& args() { return *Assert(m_node.args); }
    ChainstateManager& chainman() { return *Assert(m_node.chainman); }
    ValidationSignals& validation_signals() { return *Assert(m_node.validation_signals); }
    NodeContext& m_node;
};

class MinerImpl : public Mining
{
public:
    explicit MinerImpl(NodeContext& node) : m_node(node) {}

    bool isTestChain() override
    {
        return chainman().GetParams().IsTestChain();
    }

    bool isInitialBlockDownload() override
    {
        return chainman().IsInitialBlockDownload();

Review comment (refactor):
Moving an lvalue ref here isn't safe, I think? Not an issue with the current callsites, but function signature should either take an rvalue ref or make a copy here. E.g. impl with rvalue ref (which is sufficient for now, but may require an overload in future):

<details>
<summary>git diff on 1b41d45d46</summary>

```diff
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index be596b1765..20a801fa94 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -16,6 +16,7 @@
 #include <stddef.h>
 #include <stdint.h>
 #include <string>
+#include <utility>
 #include <vector>
 
 class ArgsManager;
@@ -361,7 +362,7 @@ public:
     virtual bool updateRwSetting(const std::string& name, const SettingsUpdate& update_function) = 0;
 
     //! Replace a setting in <datadir>/settings.json with a new value.
-    virtual bool overwriteRwSetting(const std::string& name, common::SettingsValue& value, bool write = true) = 0;
+    virtual bool overwriteRwSetting(const std::string& name, common::SettingsValue&& value, bool write = true) = 0;
 
     //! Delete a given setting in <datadir>/settings.json.
     virtual bool deleteRwSettings(const std::string& name, bool write = true) = 0;
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 54b986c926..0a89b5a3e4 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -828,7 +828,7 @@ public:
         // Now dump value to disk if requested
         return *action == interfaces::SettingsAction::SKIP_WRITE || args().WriteSettingsFile();
     }
-    bool overwriteRwSetting(const std::string& name, common::SettingsValue& value, bool write) override
+    bool overwriteRwSetting(const std::string& name, common::SettingsValue&& value, bool write) override
     {
         if (value.isNull()) return deleteRwSettings(name, write);
         return updateRwSetting(name, [&](common::SettingsValue& settings) {
diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp
index 129b5c7c2a..1149ce0a47 100644
--- a/src/wallet/load.cpp
+++ b/src/wallet/load.cpp
@@ -69,7 +69,7 @@ bool VerifyWallets(WalletContext& context)
             // Pass write=false because no need to write file and probably
             // better not to. If unnamed wallet needs to be added next startup
             // and the setting is empty, this code will just run again.
-            chain.overwriteRwSetting("wallet", wallets, /*write=*/false);
+            chain.overwriteRwSetting("wallet", std::move(wallets), /*write=*/false);
         }
     }
 

```
</details>


Code after:
        args().LockSettings([&](const common::Settings& settings) {
            if (const common::SettingsValue* value = common::FindKey(settings.rw_settings, name)) {
                result = *value;
            }
        });
        return result;
    }
    bool updateRwSetting(const std::string& name,
                         const interfaces::SettingsUpdate& update_settings_func) override
    {
        std::optional<interfaces::SettingsAction> action;
        args().LockSettings([&](common::Settings& settings) {
            auto* ptr_value = common::FindKey(settings.rw_settings, name);
            // Create value if it doesn't exist
            auto& value = ptr_value ? *ptr_value : settings.rw_settings[name];
            action = update_settings_func(value);
        });
        if (!action) return false;
        // Now dump value to disk if requested
        return *action == interfaces::SettingsAction::SKIP_WRITE || args().WriteSettingsFile();
    }
    bool overwriteRwSetting(const std::string& name, common::SettingsValue& value, bool write) override
    {
        if (value.isNull()) return deleteRwSettings(name, write);
        return updateRwSetting(name, [&](common::SettingsValue& settings) {
            settings = std::move(value);
            return write ? interfaces::SettingsAction::WRITE : interfaces::SettingsAction::SKIP_WRITE;
        });
    }
    bool deleteRwSettings(const std::string& name, bool write) override
    {
        args().LockSettings([&](common::Settings& settings) {
            settings.rw_settings.erase(name);
        });
        return !write || args().WriteSettingsFile();
    }
    void requestMempoolTransactions(Notifications& notifications) override
    {
        if (!m_node.mempool) return;
        LOCK2(::cs_main, m_node.mempool->cs);
        for (const CTxMemPoolEntry& entry : m_node.mempool->entryAll()) {
            notifications.transactionAddedToMempool(entry.GetSharedTx());
        }
    }
    bool hasAssumedValidChain() override
    {
        return chainman().IsSnapshotActive();
    }

    NodeContext* context() override { return &m_node; }
    ArgsManager& args() { return *Assert(m_node.args); }