From d6086f5b4ee7b35c006c46d8d69c4fcacabf37e7 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Nov 2016 12:53:33 +0000 Subject: [PATCH] Improve daemon RPC version handling Daemon RPC version is now composed of a major and minor number, so that incompatible changes bump the major version, while compatible changes can still bump the minor version without causing clients to unnecessarily complain. --- src/rpc/core_rpc_server_commands_defs.h | 11 ++++++++++- src/simplewallet/simplewallet.cpp | 19 ++++++++++++------- src/wallet/api/wallet.cpp | 6 +++--- src/wallet/wallet2.cpp | 8 ++++---- src/wallet/wallet2.h | 2 +- 5 files changed, 30 insertions(+), 16 deletions(-) diff --git a/src/rpc/core_rpc_server_commands_defs.h b/src/rpc/core_rpc_server_commands_defs.h index e19238c44..23fcb0a92 100644 --- a/src/rpc/core_rpc_server_commands_defs.h +++ b/src/rpc/core_rpc_server_commands_defs.h @@ -41,7 +41,16 @@ namespace cryptonote #define CORE_RPC_STATUS_BUSY "BUSY" #define CORE_RPC_STATUS_NOT_MINING "NOT MINING" -#define CORE_RPC_VERSION 5 +// When making *any* change here, bump minor +// If the change is incompatible, then bump major and set minor to 0 +// This ensures CORE_RPC_VERSION always increases, that every change +// has its own version, and that clients can just test major to see +// whether they can talk to a given daemon without having to know in +// advance which version they will stop working with +// Don't go over 32767 for any of these +#define CORE_RPC_VERSION_MAJOR 1 +#define CORE_RPC_VERSION_MINOR 0 +#define CORE_RPC_VERSION (((CORE_RPC_VERSION_MAJOR)<<16)|(CORE_RPC_VERSION_MINOR)) struct COMMAND_RPC_GET_HEIGHT { diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 462cdf58f..95f32924b 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -263,6 +263,10 @@ namespace return "invalid"; } + std::string get_version_string(uint32_t version) + { + return boost::lexical_cast(version >> 16) + "." + boost::lexical_cast(version & 0xffff); + } } @@ -1189,8 +1193,8 @@ bool simple_wallet::handle_command_line(const boost::program_options::variables_ //---------------------------------------------------------------------------------------------------- bool simple_wallet::try_connect_to_daemon(bool silent) { - bool same_version = false; - if (!m_wallet->check_connection(&same_version)) + uint32_t version = 0; + if (!m_wallet->check_connection(&version)) { if (!silent) fail_msg_writer() << tr("wallet failed to connect to daemon: ") << m_wallet->get_daemon_address() << ". " << @@ -1198,11 +1202,10 @@ bool simple_wallet::try_connect_to_daemon(bool silent) "Please make sure daemon is running or restart the wallet with the correct daemon address."); return false; } - if (!m_allow_mismatched_daemon_version && !same_version) + if (!m_allow_mismatched_daemon_version && ((version >> 16) != CORE_RPC_VERSION_MAJOR)) { if (!silent) - fail_msg_writer() << tr("Daemon uses a different RPC version that the wallet: ") << m_wallet->get_daemon_address() << ". " << - tr("Either update one of them, or use --allow-mismatched-daemon-version."); + fail_msg_writer() << boost::format(tr("Daemon uses a different RPC major version (%u) than the wallet (%u): %s. Either update one of them, or use --allow-mismatched-daemon-version.")) % (version>>16) % CORE_RPC_VERSION_MAJOR % m_wallet->get_daemon_address(); return false; } return true; @@ -3564,7 +3567,8 @@ bool simple_wallet::get_tx_note(const std::vector &args) bool simple_wallet::status(const std::vector &args) { uint64_t local_height = m_wallet->get_blockchain_current_height(); - if (!try_connect_to_daemon()) + uint32_t version = 0; + if (!m_wallet->check_connection(&version)) { success_msg_writer() << "Refreshed " << local_height << "/?, no daemon connected"; return true; @@ -3575,7 +3579,8 @@ bool simple_wallet::status(const std::vector &args) if (err.empty()) { bool synced = local_height == bc_height; - success_msg_writer() << "Refreshed " << local_height << "/" << bc_height << ", " << (synced ? "synced" : "syncing"); + success_msg_writer() << "Refreshed " << local_height << "/" << bc_height << ", " << (synced ? "synced" : "syncing") + << ", daemon RPC v" << get_version_string(version); } else { diff --git a/src/wallet/api/wallet.cpp b/src/wallet/api/wallet.cpp index 215b61aef..8514abfcd 100644 --- a/src/wallet/api/wallet.cpp +++ b/src/wallet/api/wallet.cpp @@ -906,11 +906,11 @@ bool WalletImpl::connectToDaemon() Wallet::ConnectionStatus WalletImpl::connected() const { - bool same_version = false; - bool is_connected = m_wallet->check_connection(&same_version); + uint32_t version = 0; + bool is_connected = m_wallet->check_connection(&version); if (!is_connected) return Wallet::ConnectionStatus_Disconnected; - if (!same_version) + if ((version >> 16) != CORE_RPC_VERSION_MAJOR) return Wallet::ConnectionStatus_WrongVersion; return Wallet::ConnectionStatus_Connected; } diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index ea3994435..a5444ec14 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2221,7 +2221,7 @@ bool wallet2::prepare_file_names(const std::string& file_path) return true; } //---------------------------------------------------------------------------------------------------- -bool wallet2::check_connection(bool *same_version) +bool wallet2::check_connection(uint32_t *version) { boost::lock_guard lock(m_daemon_rpc_mutex); @@ -2239,7 +2239,7 @@ bool wallet2::check_connection(bool *same_version) return false; } - if (same_version) + if (version) { epee::json_rpc::request req_t = AUTO_VAL_INIT(req_t); epee::json_rpc::response resp_t = AUTO_VAL_INIT(resp_t); @@ -2248,9 +2248,9 @@ bool wallet2::check_connection(bool *same_version) req_t.method = "get_version"; bool r = net_utils::invoke_http_json_remote_command2(m_daemon_address + "/json_rpc", req_t, resp_t, m_http_client); if (!r || resp_t.result.status != CORE_RPC_STATUS_OK) - *same_version = false; + *version = 0; else - *same_version = resp_t.result.version == CORE_RPC_VERSION; + *version = resp_t.result.version; } return true; diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index b6d3250b2..1c8b9f858 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -408,7 +408,7 @@ namespace tools std::vector create_transactions_all(const cryptonote::account_public_address &address, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector extra, bool trusted_daemon); std::vector create_transactions_from(const cryptonote::account_public_address &address, std::vector unused_transfers_indices, std::vector unused_dust_indices, const size_t fake_outs_count, const uint64_t unlock_time, uint32_t priority, const std::vector extra, bool trusted_daemon); std::vector create_unmixable_sweep_transactions(bool trusted_daemon); - bool check_connection(bool *same_version = NULL); + bool check_connection(uint32_t *version = NULL); void get_transfers(wallet2::transfer_container& incoming_transfers) const; void get_payments(const crypto::hash& payment_id, std::list& payments, uint64_t min_height = 0) const; void get_payments(std::list>& payments, uint64_t min_height, uint64_t max_height = (uint64_t)-1) const;