Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wallet2: check wallet compatibility with daemon's hard fork version [release] #8544

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/cryptonote_basic/hardfork.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,11 @@ namespace cryptonote
*/
uint64_t get_window_size() const { return window_size; }

/**
* @brief returns info for all known hard forks
*/
const std::vector<hardfork_t>& get_hardforks() const { return heights; }

private:

uint8_t get_block_version(uint64_t height) const;
Expand Down
7 changes: 7 additions & 0 deletions src/cryptonote_core/blockchain.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,6 +899,13 @@ namespace cryptonote
*/
uint64_t get_earliest_ideal_height_for_version(uint8_t version) const { return m_hardfork->get_earliest_ideal_height_for_version(version); }

/**
* @brief returns info for all known hard forks
*
* @return the hardforks
*/
const std::vector<hardfork_t>& get_hardforks() const { return m_hardfork->get_hardforks(); }

/**
* @brief get information about hardfork voting for a version
*
Expand Down
4 changes: 4 additions & 0 deletions src/rpc/core_rpc_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2861,6 +2861,10 @@ namespace cryptonote

res.version = CORE_RPC_VERSION;
res.release = MONERO_VERSION_IS_RELEASE;
res.current_height = m_core.get_current_blockchain_height();
res.target_height = m_p2p.get_payload_object().is_synchronized() ? 0 : m_core.get_target_blockchain_height();
for (const auto &hf : m_core.get_blockchain_storage().get_hardforks())
res.hard_forks.push_back({hf.version, hf.height});
res.status = CORE_RPC_STATUS_OK;
return true;
}
Expand Down
21 changes: 20 additions & 1 deletion src/rpc/core_rpc_server_commands_defs.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ namespace cryptonote
// advance which version they will stop working with
// Don't go over 32767 for any of these
#define CORE_RPC_VERSION_MAJOR 3
#define CORE_RPC_VERSION_MINOR 10
#define CORE_RPC_VERSION_MINOR 11
#define MAKE_CORE_RPC_VERSION(major,minor) (((major)<<16)|(minor))
#define CORE_RPC_VERSION MAKE_CORE_RPC_VERSION(CORE_RPC_VERSION_MAJOR, CORE_RPC_VERSION_MINOR)

Expand Down Expand Up @@ -2123,15 +2123,34 @@ namespace cryptonote
};
typedef epee::misc_utils::struct_init<request_t> request;

struct hf_entry
{
uint8_t hf_version;
uint64_t height;

bool operator==(const hf_entry& hfe) const { return hf_version == hfe.hf_version && height == hfe.height; }

BEGIN_KV_SERIALIZE_MAP()
KV_SERIALIZE(hf_version)
KV_SERIALIZE(height)
END_KV_SERIALIZE_MAP()
};

struct response_t: public rpc_response_base
{
uint32_t version;
bool release;
uint64_t current_height;
uint64_t target_height;
std::vector<hf_entry> hard_forks;

BEGIN_KV_SERIALIZE_MAP()
KV_SERIALIZE_PARENT(rpc_response_base)
KV_SERIALIZE(version)
KV_SERIALIZE(release)
KV_SERIALIZE_OPT(current_height, (uint64_t)0)
KV_SERIALIZE_OPT(target_height, (uint64_t)0)
KV_SERIALIZE_OPT(hard_forks, std::vector<hf_entry>())
END_KV_SERIALIZE_MAP()
};
typedef epee::misc_utils::struct_init<response_t> response;
Expand Down
18 changes: 11 additions & 7 deletions src/simplewallet/simplewallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ namespace
const command_line::arg_descriptor<bool> arg_restore_from_seed = {"restore-from-seed", sw::tr("alias for --restore-deterministic-wallet"), false};
const command_line::arg_descriptor<bool> arg_restore_multisig_wallet = {"restore-multisig-wallet", sw::tr("Recover multisig wallet using Electrum-style mnemonic seed"), false};
const command_line::arg_descriptor<bool> arg_non_deterministic = {"non-deterministic", sw::tr("Generate non-deterministic view and spend keys"), false};
const command_line::arg_descriptor<bool> arg_allow_mismatched_daemon_version = {"allow-mismatched-daemon-version", sw::tr("Allow communicating with a daemon that uses a different RPC version"), false};
const command_line::arg_descriptor<uint64_t> arg_restore_height = {"restore-height", sw::tr("Restore from specific blockchain height"), 0};
const command_line::arg_descriptor<std::string> arg_restore_date = {"restore-date", sw::tr("Restore from estimated blockchain height on specified date"), ""};
const command_line::arg_descriptor<bool> arg_do_not_relay = {"do-not-relay", sw::tr("The newly created transaction will not be relayed to the monero network"), false};
Expand Down Expand Up @@ -3233,8 +3232,7 @@ bool simple_wallet::scan_tx(const std::vector<std::string> &args)
}

simple_wallet::simple_wallet()
: m_allow_mismatched_daemon_version(false)
, m_refresh_progress_reporter(*this)
: m_refresh_progress_reporter(*this)
, m_idle_run(true)
, m_auto_refresh_enabled(false)
, m_auto_refresh_refreshing(false)
Expand Down Expand Up @@ -4755,7 +4753,6 @@ bool simple_wallet::handle_command_line(const boost::program_options::variables_
m_restore_deterministic_wallet = command_line::get_arg(vm, arg_restore_deterministic_wallet) || command_line::get_arg(vm, arg_restore_from_seed);
m_restore_multisig_wallet = command_line::get_arg(vm, arg_restore_multisig_wallet);
m_non_deterministic = command_line::get_arg(vm, arg_non_deterministic);
m_allow_mismatched_daemon_version = command_line::get_arg(vm, arg_allow_mismatched_daemon_version);
m_restore_height = command_line::get_arg(vm, arg_restore_height);
m_restore_date = command_line::get_arg(vm, arg_restore_date);
m_do_not_relay = command_line::get_arg(vm, arg_do_not_relay);
Expand Down Expand Up @@ -4786,20 +4783,28 @@ bool simple_wallet::try_connect_to_daemon(bool silent, uint32_t* version)
uint32_t version_ = 0;
if (!version)
version = &version_;
if (!m_wallet->check_connection(version))
bool wallet_is_outdated, daemon_is_outdated = false;
if (!m_wallet->check_connection(version, NULL, 200000, &wallet_is_outdated, &daemon_is_outdated))
{
if (!silent)
{
if (m_wallet->is_offline())
fail_msg_writer() << tr("wallet failed to connect to daemon, because it is set to offline mode");
else if (wallet_is_outdated)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems all errors always take this branch, the elseif/else below won't activate. I tried using a v0.18 wallet on a couple v0.17 nodes as well (including the one linked: http://support.ablative.hosting:18081), same thing.

These all return the old wallet error:

  • monerod online, but it's v0.17.x.x
  • monerod online, but connected to wrong port
  • monerod offline, either local or remote
  • random fake ip: 1.2.3.4:18081
  • bad ip: 1:18081 (returns wallet outdated error after hanging for a bit)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @hinto-janaiyo . Should be fixed in #8585

fail_msg_writer() << tr("wallet failed to connect to daemon, because it is not up to date. ") <<
tr("Please make sure you are running the latest wallet.");
else if (daemon_is_outdated)
fail_msg_writer() << tr("wallet failed to connect to daemon: ") << m_wallet->get_daemon_address() << ". " <<
tr("Daemon is not up to date. "
"Please make sure the daemon is running the latest version or change the daemon address using the 'set_daemon' command.");
else
fail_msg_writer() << tr("wallet failed to connect to daemon: ") << m_wallet->get_daemon_address() << ". " <<
tr("Daemon either is not started or wrong port was passed. "
"Please make sure daemon is running or change the daemon address using the 'set_daemon' command.");
}
return false;
}
if (!m_allow_mismatched_daemon_version && ((*version >> 16) != CORE_RPC_VERSION_MAJOR))
if (!m_wallet->is_mismatched_daemon_version_allowed() && ((*version >> 16) != CORE_RPC_VERSION_MAJOR))
{
if (!silent)
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();
Expand Down Expand Up @@ -10629,7 +10634,6 @@ int main(int argc, char* argv[])
command_line::add_arg(desc_params, arg_restore_multisig_wallet );
command_line::add_arg(desc_params, arg_non_deterministic );
command_line::add_arg(desc_params, arg_electrum_seed );
command_line::add_arg(desc_params, arg_allow_mismatched_daemon_version);
command_line::add_arg(desc_params, arg_restore_height);
command_line::add_arg(desc_params, arg_restore_date);
command_line::add_arg(desc_params, arg_do_not_relay);
Expand Down
1 change: 0 additions & 1 deletion src/simplewallet/simplewallet.h
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,6 @@ namespace cryptonote
bool m_restore_deterministic_wallet; // recover flag
bool m_restore_multisig_wallet; // recover flag
bool m_non_deterministic; // old 2-random generation
bool m_allow_mismatched_daemon_version;
bool m_restoring; // are we restoring, by whatever method?
uint64_t m_restore_height; // optional
bool m_do_not_relay;
Expand Down
10 changes: 8 additions & 2 deletions src/wallet/api/wallet.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2173,9 +2173,15 @@ bool WalletImpl::connectToDaemon()
Wallet::ConnectionStatus WalletImpl::connected() const
{
uint32_t version = 0;
m_is_connected = m_wallet->check_connection(&version, NULL, DEFAULT_CONNECTION_TIMEOUT_MILLIS);
bool wallet_is_outdated, daemon_is_outdated = false;
m_is_connected = m_wallet->check_connection(&version, NULL, DEFAULT_CONNECTION_TIMEOUT_MILLIS, &wallet_is_outdated, &daemon_is_outdated);
if (!m_is_connected)
return Wallet::ConnectionStatus_Disconnected;
{
if (!m_wallet->light_wallet() && (wallet_is_outdated || daemon_is_outdated))
return Wallet::ConnectionStatus_WrongVersion;
else
return Wallet::ConnectionStatus_Disconnected;
}
// Version check is not implemented in light wallets nodes/wallets
if (!m_wallet->light_wallet() && (version >> 16) != CORE_RPC_VERSION_MAJOR)
return Wallet::ConnectionStatus_WrongVersion;
Expand Down
32 changes: 31 additions & 1 deletion src/wallet/node_rpc_proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,50 @@ void NodeRPCProxy::invalidate()
m_rpc_payment_seed_hash = crypto::null_hash;
m_rpc_payment_next_seed_hash = crypto::null_hash;
m_height_time = 0;
m_target_height_time = 0;
m_rpc_payment_diff = 0;
m_rpc_payment_credits_per_hash_found = 0;
m_rpc_payment_height = 0;
m_rpc_payment_cookie = 0;
m_daemon_hard_forks.clear();
}

boost::optional<std::string> NodeRPCProxy::get_rpc_version(uint32_t &rpc_version)
boost::optional<std::string> NodeRPCProxy::get_rpc_version(uint32_t &rpc_version, std::vector<std::pair<uint8_t, uint64_t>> &daemon_hard_forks, uint64_t &height, uint64_t &target_height)
{
if (m_offline)
return boost::optional<std::string>("offline");
if (m_rpc_version == 0)
{
const time_t now = time(NULL);
cryptonote::COMMAND_RPC_GET_VERSION::request req_t = AUTO_VAL_INIT(req_t);
cryptonote::COMMAND_RPC_GET_VERSION::response resp_t = AUTO_VAL_INIT(resp_t);
{
const boost::lock_guard<boost::recursive_mutex> lock{m_daemon_rpc_mutex};
bool r = net_utils::invoke_http_json_rpc("/json_rpc", "get_version", req_t, resp_t, m_http_client, rpc_timeout);
RETURN_ON_RPC_RESPONSE_ERROR(r, epee::json_rpc::error{}, resp_t, "get_version");
}

m_rpc_version = resp_t.version;
m_daemon_hard_forks.clear();
for (const auto &hf : resp_t.hard_forks)
m_daemon_hard_forks.push_back(std::make_pair(hf.hf_version, hf.height));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method is called multiple times on the same object, then it's m_daemon_hard_forks field will keep expanding indefinitely.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added m_daemon_hard_forks.clear() above the for loop for better defense

if (resp_t.current_height > 0 || resp_t.target_height > 0)
{
m_height = resp_t.current_height;
m_target_height = resp_t.target_height;
m_height_time = now;
m_target_height_time = now;
}
}

rpc_version = m_rpc_version;
daemon_hard_forks = m_daemon_hard_forks;
boost::optional<std::string> result = get_height(height);
if (result)
return result;
result = get_target_height(target_height);
if (result)
return result;
return boost::optional<std::string>();
}

Expand Down Expand Up @@ -138,6 +160,7 @@ boost::optional<std::string> NodeRPCProxy::get_info()
m_adjusted_time = resp_t.adjusted_time;
m_get_info_time = now;
m_height_time = now;
m_target_height_time = now;
}
return boost::optional<std::string>();
}
Expand All @@ -160,6 +183,13 @@ boost::optional<std::string> NodeRPCProxy::get_height(uint64_t &height)

boost::optional<std::string> NodeRPCProxy::get_target_height(uint64_t &height)
{
const time_t now = time(NULL);
if (now < m_target_height_time + 30) // re-cache every 30 seconds
{
height = m_target_height;
return boost::optional<std::string>();
}

auto res = get_info();
if (res)
return res;
Expand Down
4 changes: 3 additions & 1 deletion src/wallet/node_rpc_proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class NodeRPCProxy
void invalidate();
void set_offline(bool offline) { m_offline = offline; }

boost::optional<std::string> get_rpc_version(uint32_t &version);
boost::optional<std::string> get_rpc_version(uint32_t &rpc_version, std::vector<std::pair<uint8_t, uint64_t>> &daemon_hard_forks, uint64_t &height, uint64_t &target_height);
boost::optional<std::string> get_height(uint64_t &height);
void set_height(uint64_t h);
boost::optional<std::string> get_target_height(uint64_t &height);
Expand Down Expand Up @@ -103,6 +103,8 @@ class NodeRPCProxy
crypto::hash m_rpc_payment_next_seed_hash;
uint32_t m_rpc_payment_cookie;
time_t m_height_time;
time_t m_target_height_time;
std::vector<std::pair<uint8_t, uint64_t>> m_daemon_hard_forks;
};

}
Loading