Save and load exchange rates with persistent JSON file (#13)

* Refactor exchange rates loading and saving

* Use given path for exchange rates config, not relative path from data directory

* Add separate tests for exchange rates RPCs

src/exchangerates.cpp

@@ -1,6 +1,8 @@
 #include <assetsdir.h>
 #include <exchangerates.h>
 #include <policy/policy.h>
+#include <util/settings.h>
+#include <util/system.h>
 #include <univalue.h>
 
 #include <fstream>
@@ -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<std::string>& 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<std::string>& errors) {
+    std::map <std::string, UniValue> json;
+    if (!util::ReadSettings(file_path, json, errors)) {
         return false;
     }
-    std::map<std::string, UniValue> 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<std::string>& errors) {
+    UniValue json = this->ToJSON();
+    std::map<std::string, util::SettingsValue> 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<std::string, UniValue> json, std::vector<std::string>& errors) {
+    bool hasError = false;
+    std::map<CAsset, CAmount> 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; 
 }

src/exchangerates.h

@@ -4,8 +4,10 @@
 
 #include <fs.h>
 #include <policy/policy.h>
+#include <univalue.h>
 
 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<CAsset, CAssetExchangeRate>
      */
     CAmount CalculateExchangeValue(const CAmount& amount, const CAsset& asset);
 
+/**
+     * Load the exchange rate map from the default JSON config file in <datadir>/exchangerates.json.
+     *
+     * @param[in]   errors        Vector for storing error messages, if there are any.
+     * @return true on success
+     */
+    bool LoadFromDefaultJSONFile(std::vector<std::string>& 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<std::string>& 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<std::string>& errors);
+
+    UniValue ToJSON();
+
+    bool LoadFromJSON(std::map<std::string, UniValue> json, std::vector<std::string>& error);
 };
 
 #endif // BITCOIN_EXCHANGERATES_H

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=<file>", 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=<file>", 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<std::string> 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));
+            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)));
+            };
+        } 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

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<std::string, UniValue> rawRates;
-    ratesField.getObjMap(rawRates);
-    std::map<CAsset, CAmount> 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<std::string, UniValue> jsonRates;
+    json.getObjMap(jsonRates);
     auto& exchangeRateMap = ExchangeRateMap::GetInstance();
-    exchangeRateMap.clear();
-    for (auto rate : parsedRates) {
-        exchangeRateMap[rate.first] = rate.second;
+    std::vector<std::string> 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;
 },

test/functional/data/initialexchangerates.json

@@ -0,0 +1,3 @@
+{
+    "gasset": 100000000
+}

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