From bcb86ae6516ffd18bdd7bb4fa89be1def0588878 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 12 Apr 2019 13:36:46 +0000 Subject: [PATCH] wallet_rpc_server: fix inconsistent wallet caches on reload Loading the same wallet as the currently loaded one would autosave the current state after loading it, leading to some kind of rollback effect. We now save before loading to avoid this. If loading fails, it means the current wallet will be saved (or maybe not, depending on where the failure occurs: most of the sanity checks occur before saving). There is a new autosave_current flag to open/restore calls so the (enabled by default) autosave can be skipped. --- src/wallet/wallet_rpc_server.cpp | 89 +++++++++++--------- src/wallet/wallet_rpc_server_commands_defs.h | 11 ++- utils/python-rpc/framework/wallet.py | 24 +++++- 3 files changed, 78 insertions(+), 46 deletions(-) diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 71c64d3c1..83961e73a 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -3111,6 +3111,18 @@ namespace tools er.message = "Invalid filename"; return false; } + if (m_wallet && req.autosave_current) + { + try + { + m_wallet->store(); + } + catch (const std::exception& e) + { + handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + return false; + } + } std::string wallet_file = m_wallet_dir + "/" + req.filename; { po::options_description desc("dummy"); @@ -3141,18 +3153,7 @@ namespace tools } if (m_wallet) - { - try - { - m_wallet->store(); - } - catch (const std::exception& e) - { - handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); - return false; - } delete m_wallet; - } m_wallet = wal.release(); return true; } @@ -3161,14 +3162,17 @@ namespace tools { if (!m_wallet) return not_open(er); - try - { - m_wallet->store(); - } - catch (const std::exception& e) + if (req.autosave_current) { - handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); - return false; + try + { + m_wallet->store(); + } + catch (const std::exception& e) + { + handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + return false; + } } delete m_wallet; m_wallet = NULL; @@ -3385,6 +3389,20 @@ namespace tools return false; } + if (m_wallet && req.autosave_current) + { + try + { + if (!wallet_file.empty()) + m_wallet->store(); + } + catch (const std::exception &e) + { + handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + return false; + } + } + try { if (!req.spendkey.empty()) @@ -3433,19 +3451,7 @@ namespace tools } if (m_wallet) - { - try - { - if (!wallet_file.empty()) - m_wallet->store(); - } - catch (const std::exception &e) - { - handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); - return false; - } delete m_wallet; - } m_wallet = wal.release(); res.address = m_wallet->get_account().get_public_address_str(m_wallet->nettype()); return true; @@ -3511,6 +3517,18 @@ namespace tools return false; } } + if (m_wallet && req.autosave_current) + { + try + { + m_wallet->store(); + } + catch (const std::exception &e) + { + handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); + return false; + } + } // process seed_offset if given { @@ -3621,18 +3639,7 @@ namespace tools } if (m_wallet) - { - try - { - m_wallet->store(); - } - catch (const std::exception &e) - { - handle_rpc_exception(std::current_exception(), er, WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR); - return false; - } delete m_wallet; - } m_wallet = wal.release(); res.address = m_wallet->get_account().get_public_address_str(m_wallet->nettype()); res.info = "Wallet has been restored successfully."; diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index bb360ae01..fdf2c9d24 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -47,7 +47,7 @@ // advance which version they will stop working with // Don't go over 32767 for any of these #define WALLET_RPC_VERSION_MAJOR 1 -#define WALLET_RPC_VERSION_MINOR 9 +#define WALLET_RPC_VERSION_MINOR 10 #define MAKE_WALLET_RPC_VERSION(major,minor) (((major)<<16)|(minor)) #define WALLET_RPC_VERSION MAKE_WALLET_RPC_VERSION(WALLET_RPC_VERSION_MAJOR, WALLET_RPC_VERSION_MINOR) namespace tools @@ -2043,10 +2043,12 @@ namespace wallet_rpc { std::string filename; std::string password; + bool autosave_current; BEGIN_KV_SERIALIZE_MAP() KV_SERIALIZE(filename) KV_SERIALIZE(password) + KV_SERIALIZE_OPT(autosave_current, true) END_KV_SERIALIZE_MAP() }; typedef epee::misc_utils::struct_init request; @@ -2063,7 +2065,10 @@ namespace wallet_rpc { struct request_t { + bool autosave_current; + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE_OPT(autosave_current, true) END_KV_SERIALIZE_MAP() }; typedef epee::misc_utils::struct_init request; @@ -2108,6 +2113,7 @@ namespace wallet_rpc std::string spendkey; std::string viewkey; std::string password; + bool autosave_current; BEGIN_KV_SERIALIZE_MAP() KV_SERIALIZE_OPT(restore_height, (uint64_t)0) @@ -2116,6 +2122,7 @@ namespace wallet_rpc KV_SERIALIZE(spendkey) KV_SERIALIZE(viewkey) KV_SERIALIZE(password) + KV_SERIALIZE_OPT(autosave_current, true) END_KV_SERIALIZE_MAP() }; @@ -2141,6 +2148,7 @@ namespace wallet_rpc std::string seed_offset; std::string password; std::string language; + bool autosave_current; BEGIN_KV_SERIALIZE_MAP() KV_SERIALIZE_OPT(restore_height, (uint64_t)0) @@ -2149,6 +2157,7 @@ namespace wallet_rpc KV_SERIALIZE(seed_offset) KV_SERIALIZE(password) KV_SERIALIZE(language) + KV_SERIALIZE_OPT(autosave_current, true) END_KV_SERIALIZE_MAP() }; typedef epee::misc_utils::struct_init request; diff --git a/utils/python-rpc/framework/wallet.py b/utils/python-rpc/framework/wallet.py index a80aaefec..a55750e3a 100644 --- a/utils/python-rpc/framework/wallet.py +++ b/utils/python-rpc/framework/wallet.py @@ -265,7 +265,7 @@ class Wallet(object): } return self.rpc.send_json_rpc_request(query_key) - def restore_deterministic_wallet(self, seed = '', seed_offset = '', filename = '', restore_height = 0, password = '', language = ''): + def restore_deterministic_wallet(self, seed = '', seed_offset = '', filename = '', restore_height = 0, password = '', language = '', autosave_current = True): restore_deterministic_wallet = { 'method': 'restore_deterministic_wallet', 'params' : { @@ -274,14 +274,15 @@ class Wallet(object): 'seed': seed, 'seed_offset': seed_offset, 'password': password, - 'language': language + 'language': language, + 'autosave_current': autosave_current, }, 'jsonrpc': '2.0', 'id': '0' } return self.rpc.send_json_rpc_request(restore_deterministic_wallet) - def generate_from_keys(self, restore_height = 0, filename = "", password = "", address = "", spendkey = "", viewkey = ""): + def generate_from_keys(self, restore_height = 0, filename = "", password = "", address = "", spendkey = "", viewkey = "", autosave_current = True): generate_from_keys = { 'method': 'generate_from_keys', 'params' : { @@ -291,16 +292,31 @@ class Wallet(object): 'spendkey': spendkey, 'viewkey': viewkey, 'password': password, + 'autosave_current': autosave_current, }, 'jsonrpc': '2.0', 'id': '0' } return self.rpc.send_json_rpc_request(generate_from_keys) - def close_wallet(self): + def open_wallet(self, filename, password='', autosave_current = True): + open_wallet = { + 'method': 'open_wallet', + 'params' : { + 'filename': filename, + 'password': password, + 'autosave_current': autosave_current, + }, + 'jsonrpc': '2.0', + 'id': '0' + } + return self.rpc.send_json_rpc_request(open_wallet) + + def close_wallet(self, autosave_current = True): close_wallet = { 'method': 'close_wallet', 'params' : { + 'autosave_current': autosave_current }, 'jsonrpc': '2.0', 'id': '0'