aboutsummaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/interfaces/chain.h24
-rw-r--r--src/node/interfaces.cpp30
-rw-r--r--src/wallet/load.cpp2
-rw-r--r--src/wallet/test/wallet_tests.cpp34
-rw-r--r--src/wallet/wallet.cpp35
5 files changed, 100 insertions, 25 deletions
diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h
index af45f81f95..be596b1765 100644
--- a/src/interfaces/chain.h
+++ b/src/interfaces/chain.h
@@ -96,6 +96,17 @@ struct BlockInfo {
BlockInfo(const uint256& hash LIFETIMEBOUND) : hash(hash) {}
};
+//! The action to be taken after updating a settings value.
+//! WRITE indicates that the updated value must be written to disk,
+//! while SKIP_WRITE indicates that the change will be kept in memory-only
+//! without persisting it.
+enum class SettingsAction {
+ WRITE,
+ SKIP_WRITE
+};
+
+using SettingsUpdate = std::function<std::optional<interfaces::SettingsAction>(common::SettingsValue&)>;
+
//! Interface giving clients (wallet processes, maybe other analysis tools in
//! the future) ability to access to the chain state, receive notifications,
//! estimate fees, and submit transactions.
@@ -344,9 +355,16 @@ public:
//! Return <datadir>/settings.json setting value.
virtual common::SettingsValue getRwSetting(const std::string& name) = 0;
- //! Write a setting to <datadir>/settings.json. Optionally just update the
- //! setting in memory and do not write the file.
- virtual bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write=true) = 0;
+ //! Updates a setting in <datadir>/settings.json.
+ //! Depending on the action returned by the update function, this will either
+ //! update the setting in memory or write the updated settings to disk.
+ 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;
+
+ //! Delete a given setting in <datadir>/settings.json.
+ virtual bool deleteRwSettings(const std::string& name, bool write = true) = 0;
//! Synchronously send transactionAddedToMempool notifications about all
//! current mempool transactions to the specified handler and return after
diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp
index 9fe08eb3dd..54b986c926 100644
--- a/src/node/interfaces.cpp
+++ b/src/node/interfaces.cpp
@@ -814,14 +814,32 @@ public:
});
return result;
}
- bool updateRwSetting(const std::string& name, const common::SettingsValue& value, bool write) override
+ bool updateRwSetting(const std::string& name,
+ const interfaces::SettingsUpdate& update_settings_func) override
{
+ std::optional<interfaces::SettingsAction> action;
args().LockSettings([&](common::Settings& settings) {
- if (value.isNull()) {
- settings.rw_settings.erase(name);
- } else {
- settings.rw_settings[name] = value;
- }
+ 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();
}
diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp
index e26347d437..129b5c7c2a 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.updateRwSetting("wallet", wallets, /* write= */ false);
+ chain.overwriteRwSetting("wallet", wallets, /*write=*/false);
}
}
diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp
index 44ffddb168..12d5a3b3eb 100644
--- a/src/wallet/test/wallet_tests.cpp
+++ b/src/wallet/test/wallet_tests.cpp
@@ -329,6 +329,40 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
}
}
+// This test verifies that wallet settings can be added and removed
+// concurrently, ensuring no race conditions occur during either process.
+BOOST_FIXTURE_TEST_CASE(write_wallet_settings_concurrently, TestingSetup)
+{
+ WalletContext context;
+ context.chain = m_node.chain.get();
+ const auto NUM_WALLETS{5};
+
+ // Since we're counting the number of wallets, ensure we start without any.
+ BOOST_REQUIRE(context.chain->getRwSetting("wallet").isNull());
+
+ const auto& check_concurrent_wallet = [&](const auto& settings_function, int num_expected_wallets) {
+ std::vector<std::thread> threads;
+ threads.reserve(NUM_WALLETS);
+ for (auto i{0}; i < NUM_WALLETS; ++i) threads.emplace_back(settings_function, i);
+ for (auto& t : threads) t.join();
+
+ auto wallets = context.chain->getRwSetting("wallet");
+ BOOST_CHECK_EQUAL(wallets.getValues().size(), num_expected_wallets);
+ };
+
+ // Add NUM_WALLETS wallets concurrently, ensure we end up with NUM_WALLETS stored.
+ check_concurrent_wallet([&context](int i) {
+ Assert(AddWalletSetting(*context.chain, strprintf("wallet_%d", i)));
+ },
+ /*num_expected_wallets=*/NUM_WALLETS);
+
+ // Remove NUM_WALLETS wallets concurrently, ensure we end up with 0 wallets.
+ check_concurrent_wallet([&context](int i) {
+ Assert(RemoveWalletSetting(*context.chain, strprintf("wallet_%d", i)));
+ },
+ /*num_expected_wallets=*/0);
+}
+
// Check that GetImmatureCredit() returns a newly calculated value instead of
// the cached value after a MarkDirty() call.
//
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 8df39b9f75..be630cec8c 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -93,25 +93,30 @@ namespace wallet {
bool AddWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
{
- common::SettingsValue setting_value = chain.getRwSetting("wallet");
- if (!setting_value.isArray()) setting_value.setArray();
- for (const common::SettingsValue& value : setting_value.getValues()) {
- if (value.isStr() && value.get_str() == wallet_name) return true;
- }
- setting_value.push_back(wallet_name);
- return chain.updateRwSetting("wallet", setting_value);
+ const auto update_function = [&wallet_name](common::SettingsValue& setting_value) {
+ if (!setting_value.isArray()) setting_value.setArray();
+ for (const auto& value : setting_value.getValues()) {
+ if (value.isStr() && value.get_str() == wallet_name) return interfaces::SettingsAction::SKIP_WRITE;
+ }
+ setting_value.push_back(wallet_name);
+ return interfaces::SettingsAction::WRITE;
+ };
+ return chain.updateRwSetting("wallet", update_function);
}
bool RemoveWalletSetting(interfaces::Chain& chain, const std::string& wallet_name)
{
- common::SettingsValue setting_value = chain.getRwSetting("wallet");
- if (!setting_value.isArray()) return true;
- common::SettingsValue new_value(common::SettingsValue::VARR);
- for (const common::SettingsValue& value : setting_value.getValues()) {
- if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value);
- }
- if (new_value.size() == setting_value.size()) return true;
- return chain.updateRwSetting("wallet", new_value);
+ const auto update_function = [&wallet_name](common::SettingsValue& setting_value) {
+ if (!setting_value.isArray()) return interfaces::SettingsAction::SKIP_WRITE;
+ common::SettingsValue new_value(common::SettingsValue::VARR);
+ for (const auto& value : setting_value.getValues()) {
+ if (!value.isStr() || value.get_str() != wallet_name) new_value.push_back(value);
+ }
+ if (new_value.size() == setting_value.size()) return interfaces::SettingsAction::SKIP_WRITE;
+ setting_value = std::move(new_value);
+ return interfaces::SettingsAction::WRITE;
+ };
+ return chain.updateRwSetting("wallet", update_function);
}
static void UpdateWalletSetting(interfaces::Chain& chain,
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-20 17:53:07 +0000