From 0e927ebc05272e56c3eb167204244849b482384b Mon Sep 17 00:00:00 2001 From: JBetz Date: Tue, 16 Apr 2024 19:27:35 +0200 Subject: [PATCH 1/3] Refactor exchange rates loading and saving --- src/exchangerates.cpp | 80 ++++++++++++++++++++++++--------------- src/exchangerates.h | 30 +++++++++++++-- src/init.cpp | 22 ++++++++--- src/rpc/exchangerates.cpp | 36 ++++++------------ 4 files changed, 102 insertions(+), 66 deletions(-) diff --git a/src/exchangerates.cpp b/src/exchangerates.cpp index d722029f5b..039e7a66f0 100644 --- a/src/exchangerates.cpp +++ b/src/exchangerates.cpp @@ -1,6 +1,8 @@ #include #include #include +#include +#include #include #include @@ -19,44 +21,60 @@ CAmount ExchangeRateMap::CalculateExchangeValue(const CAmount& amount, const CAs } } -bool ExchangeRateMap::LoadExchangeRatesFromJSONFile(fs::path file_path, std::string& error) { - // Read config file - std::ifstream ifs(file_path); - if (!ifs.is_open()) { - error = "Failed to open file"; - return false; +bool ExchangeRateMap::LoadFromDefaultJSONFile(std::vector& errors) { + fs::path file_path = AbsPathForConfigVal(fs::PathFromString(exchange_rates_config_file)); + if (fs::exists(file_path)) { + return LoadFromJSONFile(file_path, errors); + } else { + return true; } - std::stringstream buffer; - buffer << ifs.rdbuf(); +} - // Parse as JSON - std::string rawJson = buffer.str(); - UniValue json; - if (!json.read(rawJson)) { - error = "Cannot parse JSON"; +bool ExchangeRateMap::LoadFromJSONFile(fs::path file_path, std::vector& errors) { + std::map json; + if (!util::ReadSettings(file_path, json, errors)) { return false; } - std::map assetMap; - json.getObjMap(assetMap); + return this->LoadFromJSON(json, errors); +} - // Load exchange rates into map - this->clear(); - for (auto assetEntry : assetMap) { - auto assetIdentifier = assetEntry.first; - auto assetData = assetEntry.second; - CAsset asset = GetAssetFromString(assetIdentifier); - if (asset.IsNull()) { - error = strprintf("Unknown label and invalid asset hex: %s", assetIdentifier); - return false; +bool ExchangeRateMap::SaveToJSONFile(std::vector& errors) { + UniValue json = this->ToJSON(); + std::map settings; + json.getObjMap(settings); + fs::path file_path = AbsPathForConfigVal(fs::PathFromString(exchange_rates_config_file)); + return util::WriteSettings(file_path, settings, errors); +} + +UniValue ExchangeRateMap::ToJSON() { + UniValue json = UniValue{UniValue::VOBJ}; + for (auto rate : *this) { + std::string label = gAssetsDir.GetLabel(rate.first); + if (label == "") { + label = rate.first.GetHex(); } - CAmount exchangeRateValue; - if (assetData.isNum()) { - exchangeRateValue = assetData.get_int64(); + json.pushKV(label, rate.second.m_scaled_value); + } + return json; +} + +bool ExchangeRateMap::LoadFromJSON(std::map json, std::vector& errors) { + bool hasError = false; + std::map parsedRates; + for (auto rate : json) { + CAsset asset = GetAssetFromString(rate.first); + if (asset.IsNull()) { + errors.push_back(strprintf("Unknown label and invalid asset hex: %s", rate.first)); + hasError = true; } else { - error = strprintf("Invalid value for asset %s: %d", assetIdentifier, assetData.getValStr()); - return false; + CAmount newRateValue = rate.second.get_int64(); + parsedRates[asset] = newRateValue; } - (*this)[asset] = exchangeRateValue; } - return true; + if (hasError) return false; + this->clear(); + for (auto rate : parsedRates) { + (*this)[rate.first] = rate.second; + } + return true; } diff --git a/src/exchangerates.h b/src/exchangerates.h index d82e12ab52..5240898e05 100644 --- a/src/exchangerates.h +++ b/src/exchangerates.h @@ -4,8 +4,10 @@ #include #include +#include constexpr const CAmount exchange_rate_scale = COIN; +const std::string exchange_rates_config_file = "exchangerates.json"; class CAssetExchangeRate { @@ -40,14 +42,34 @@ class ExchangeRateMap : public std::map */ CAmount CalculateExchangeValue(const CAmount& amount, const CAsset& asset); +/** + * Load the exchange rate map from the default JSON config file in /exchangerates.json. + * + * @param[in] errors Vector for storing error messages, if there are any. + * @return true on success + */ + bool LoadFromDefaultJSONFile(std::vector& errors); + + /** + * Load the exchange rate map from a JSON config file. + * + * @param[in] file_path File path to JSON config file where keys are asset labels and values are exchange rates. + * @param[in] errors Vector for storing error messages, if there are any. + * @return true on success + */ + bool LoadFromJSONFile(fs::path file_path, std::vector& errors); + /** - * Populate the exchange rate map using a config file. + * Save the exchange rate map to a JSON config file in the node's data directory. * - * @param[in] file_path File path to INI config file where keys are asset labels and values are exchange rates. - * @param[in] error String reference for storing error message, if there is any. + * @param[in] errors Vector for storing error messages, if there are any. * @return true on success */ - bool LoadExchangeRatesFromJSONFile(fs::path file_path, std::string& error); + bool SaveToJSONFile(std::vector& errors); + + UniValue ToJSON(); + + bool LoadFromJSON(std::map json, std::vector& error); }; #endif // BITCOIN_EXCHANGERATES_H diff --git a/src/init.cpp b/src/init.cpp index d5f70bfb62..c82dcf398e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -642,7 +642,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-ct_bits", strprintf("The default number of hiding bits in a rangeproof. Will be exceeded to cover amounts exceeding the maximum hiding value. (default: %d)", 52), ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-ct_exponent", strprintf("The hiding exponent. (default: %s)", 0), ArgsManager::ALLOW_ANY, OptionsCategory::CHAINPARAMS); argsman.AddArg("-con_any_asset_fees", "Enable transation sees to be paid with any asset (default: false)", ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); - argsman.AddArg("-exchangeratesjsonfile=", strprintf("Specify path to read-only configuration file with asset valuations. Only used when con_any_asset_fees is enabled. Relative paths will be prefixed by datadir location. (default: %s)", "exchangerates.json"), ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); + argsman.AddArg("-initialexchangeratesjsonfile=", strprintf("Specify path to read-only configuration file with asset valuations. Only used when con_any_asset_fees is enabled. Relative paths will be prefixed by datadir location. (default: %s)", "exchangerates.json"), ArgsManager::ALLOW_ANY, OptionsCategory::ELEMENTS); #if defined(USE_SYSCALL_SANDBOX) @@ -1330,21 +1330,31 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) #endif // ELEMENTS: + policyAsset = CAsset(uint256S(gArgs.GetArg("-feeasset", chainparams.GetConsensus().pegged_asset.GetHex()))); + g_con_any_asset_fees = gArgs.GetBoolArg("-con_any_asset_fees", false); if (g_con_any_asset_fees) { // If fees can be paid in any asset, node operators need to be able to specify asset exchange // rates using either the static config file and/or the exchange rates RPCs. RegisterExchangeRatesRPCCommands(tableRPC); - std::string file_path_string = gArgs.GetArg("-exchangeratesjsonfile", ""); + ExchangeRateMap& exchangeRateMap = ExchangeRateMap::GetInstance(); + std::string file_path_string = gArgs.GetArg("-initialexchangeratesjsonfile", ""); + std::vector errors; if (!file_path_string.empty()) { fs::path file_path = AbsPathForConfigVal(fs::PathFromString(file_path_string)); - std::string error; - if (!ExchangeRateMap::GetInstance().LoadExchangeRatesFromJSONFile(file_path, error)) { - return InitError(strprintf(_("Unable to load exchange rates from JSON file %s: %s"), file_path_string, error)); + if (!exchangeRateMap.LoadFromJSONFile(file_path, errors)) { + return InitError(strprintf(_("Unable to load exchange rates from JSON file %s: \n%s\n"), file_path_string, MakeUnorderedList(errors))); + }; + } else { + if (!exchangeRateMap.LoadFromDefaultJSONFile(errors)) { + return InitError(strprintf(_("Unable to load exchange rates from default JSON file %s: \n%s\n"), exchange_rates_config_file, MakeUnorderedList(errors))); }; } + errors.clear(); + if (!exchangeRateMap.SaveToJSONFile(errors)) { + return InitError(strprintf(_("Unable to save exchange rates to JSON file %s: \n%s\n"), exchange_rates_config_file, MakeUnorderedList(errors))); + }; } - policyAsset = CAsset(uint256S(gArgs.GetArg("-feeasset", chainparams.GetConsensus().pegged_asset.GetHex()))); /* Start the RPC server already. It will be started in "warmup" mode * and not really process calls already (but it will signify connections diff --git a/src/rpc/exchangerates.cpp b/src/rpc/exchangerates.cpp index 5ddc4b0c57..44a341076f 100644 --- a/src/rpc/exchangerates.cpp +++ b/src/rpc/exchangerates.cpp @@ -29,17 +29,9 @@ static RPCHelpMan getfeeexchangerates() + HelpExampleRpc("getfeeexchangerates", "") }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue -{ - UniValue response = UniValue{UniValue::VOBJ}; - for (auto rate : ExchangeRateMap::GetInstance()) { - std::string label = gAssetsDir.GetLabel(rate.first); - if (label == "") { - label = rate.first.GetHex(); - } - response.pushKV(label, rate.second.m_scaled_value); + { + return ExchangeRateMap::GetInstance().ToJSON(); } - return response; -}, }; } @@ -59,23 +51,17 @@ static RPCHelpMan setfeeexchangerates() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { - UniValue ratesField = request.params[0].get_obj(); - std::map rawRates; - ratesField.getObjMap(rawRates); - std::map parsedRates; - for (auto rate : rawRates) { - CAsset asset = GetAssetFromString(rate.first); - if (asset.IsNull()) { - throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Unknown label and invalid asset hex: %s", rate.first)); - } - CAmount newRateValue = rate.second.get_int64(); - parsedRates[asset] = newRateValue; - } + UniValue json = request.params[0].get_obj(); + std::map jsonRates; + json.getObjMap(jsonRates); auto& exchangeRateMap = ExchangeRateMap::GetInstance(); - exchangeRateMap.clear(); - for (auto rate : parsedRates) { - exchangeRateMap[rate.first] = rate.second; + std::vector errors; + if (!exchangeRateMap.LoadFromJSON(jsonRates, errors)) { + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Error loading rates from JSON: %s", MakeUnorderedList(errors))); } + if (!exchangeRateMap.SaveToJSONFile(errors)) { + return JSONRPCError(RPC_WALLET_ERROR, strprintf("Error saving exchange rates to JSON file %s: \n%s\n", exchange_rates_config_file, MakeUnorderedList(errors))); + }; EnsureAnyMemPool(request.context).RecomputeFees(); return NullUniValue; }, From 5226ce1c7ef129cca5414b2a13d7a120385412ed Mon Sep 17 00:00:00 2001 From: JBetz Date: Wed, 17 Apr 2024 18:16:57 +0200 Subject: [PATCH 2/3] Use given path for exchange rates config, not relative path from data directory --- src/init.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/init.cpp b/src/init.cpp index c82dcf398e..008e70fef5 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1341,7 +1341,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) std::string file_path_string = gArgs.GetArg("-initialexchangeratesjsonfile", ""); std::vector errors; if (!file_path_string.empty()) { - fs::path file_path = AbsPathForConfigVal(fs::PathFromString(file_path_string)); + fs::path file_path = GetConfigFile(file_path_string); if (!exchangeRateMap.LoadFromJSONFile(file_path, errors)) { return InitError(strprintf(_("Unable to load exchange rates from JSON file %s: \n%s\n"), file_path_string, MakeUnorderedList(errors))); }; From 9260e6469083ce747d8721f56f2117857612b0c2 Mon Sep 17 00:00:00 2001 From: JBetz Date: Thu, 18 Apr 2024 08:03:44 +0200 Subject: [PATCH 3/3] Add separate tests for exchange rates RPCs --- .../functional/data/initialexchangerates.json | 3 + test/functional/rpc_exchangerates.py | 78 +++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 test/functional/data/initialexchangerates.json create mode 100755 test/functional/rpc_exchangerates.py diff --git a/test/functional/data/initialexchangerates.json b/test/functional/data/initialexchangerates.json new file mode 100644 index 0000000000..e9c13f9247 --- /dev/null +++ b/test/functional/data/initialexchangerates.json @@ -0,0 +1,3 @@ +{ + "gasset": 100000000 +} \ No newline at end of file diff --git a/test/functional/rpc_exchangerates.py b/test/functional/rpc_exchangerates.py new file mode 100755 index 0000000000..fddc9a68d0 --- /dev/null +++ b/test/functional/rpc_exchangerates.py @@ -0,0 +1,78 @@ +#!/usr/bin/env python3 +# Copyright (c) 2017-2020 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Tests exchange rates RPCs""" + +from test_framework.blocktools import COINBASE_MATURITY +from test_framework.test_framework import BitcoinTestFramework +from decimal import Decimal +from test_framework.util import ( + assert_equal, + assert_raises_rpc_error, +) +import json +import os +from pathlib import Path + +TESTSDIR = os.path.dirname(os.path.realpath(__file__)) + +class ExchangeRatesTest(BitcoinTestFramework): + def set_test_params(self): + self.setup_clean_chain = True + self.num_nodes = 1 + self.exchange_rates_json_file = os.path.join(TESTSDIR, "data/initialexchangerates.json") + self.extra_args = [[ + "-con_any_asset_fees=1", + "-defaultpeggedassetname=gasset", + "-initialexchangeratesjsonfile=%s" % self.exchange_rates_json_file + ]] + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + node = self.nodes[0] + self.generate(node, COINBASE_MATURITY + 1) + + # Initial rates + assert node.dumpassetlabels() == {'gasset': 'b2e15d0d7a0c94e4e2ce0fe6e8691b9e451377f6e46e8045a86f7c4b5d4f0f23'} + initial_rates = { 'gasset': 100000000 } + assert node.getfeeexchangerates() == initial_rates + assert self.get_exchange_rates_from_database(node) == initial_rates + + # Add issued asset + self.issue_amount = Decimal('100') + self.issuance = node.issueasset(self.issue_amount, 1) + self.asset = self.issuance['asset'] + self.test_exchange_rates_update(node, initial_rates | { self.asset: 100000000 }) + + # Clear rates + self.test_exchange_rates_update(node, {}) + + # Invalid rates + self.test_invalid_exchange_rates_update(node, "invalid", 1) + self.test_invalid_exchange_rates_update(node, 1, "invalid") + + # Restore rates + self.test_exchange_rates_update(node, initial_rates | { self.asset: 100000000 }) + + def test_exchange_rates_update(self, node, new_rates): + node.setfeeexchangerates(new_rates) + assert node.getfeeexchangerates() == new_rates + assert self.get_exchange_rates_from_database(node) == new_rates + + def test_invalid_exchange_rates_update(self, node, asset_name, value): + current_rates = node.getfeeexchangerates() + assert_raises_rpc_error(-4, "Error loading rates from JSON: - Unknown label and invalid asset hex: %s" % asset_name, node.setfeeexchangerates, { asset_name: value }) + assert node.getfeeexchangerates() == current_rates + + def get_exchange_rates_from_database(self, node): + database_file_path = Path(node.datadir, self.chain, "exchangerates.json") + database_file = open(database_file_path) + data = json.load(database_file) + database_file.close() + return data + +if __name__ == '__main__': + ExchangeRatesTest().main()