From 17c7c9caed3486797ce40ce56d0c383ebaf77ea6 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 18:45:22 +0000 Subject: [PATCH 1/7] epee: remove dodgy random code that nobody uses in case someone might want to use it --- contrib/epee/include/math_helper.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/contrib/epee/include/math_helper.h b/contrib/epee/include/math_helper.h index 9b8765e60..90398acbb 100644 --- a/contrib/epee/include/math_helper.h +++ b/contrib/epee/include/math_helper.h @@ -229,15 +229,6 @@ namespace math_helper } } -PRAGMA_WARNING_PUSH -PRAGMA_GCC("GCC diagnostic ignored \"-Wstrict-aliasing\"") - inline - uint64_t generated_random_uint64() - { - boost::uuids::uuid id___ = boost::uuids::random_generator()(); - return *reinterpret_cast(&id___.data[0]); //(*reinterpret_cast(&id___.data[0]) ^ *reinterpret_cast(&id___.data[8])); - } -PRAGMA_WARNING_POP template class once_a_time_seconds { From d97582cf95adc78e631286a609cbeccead9751de Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 18:45:53 +0000 Subject: [PATCH 2/7] epee: use generate_random_bytes for new random uuids Instead of using boost::uuids::generate_random, which uses uninitialized stuff *on purpose*, just to annoy people who use valgrind --- contrib/epee/include/net/abstract_tcp_server2.inl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/contrib/epee/include/net/abstract_tcp_server2.inl b/contrib/epee/include/net/abstract_tcp_server2.inl index 5bf65d7fd..934132ea2 100644 --- a/contrib/epee/include/net/abstract_tcp_server2.inl +++ b/contrib/epee/include/net/abstract_tcp_server2.inl @@ -141,7 +141,12 @@ PRAGMA_WARNING_DISABLE_VS(4355) context = boost::value_initialized(); long ip_ = boost::asio::detail::socket_ops::host_to_network_long(remote_ep.address().to_v4().to_ulong()); - context.set_details(boost::uuids::random_generator()(), ip_, remote_ep.port(), is_income); + // create a random uuid + boost::uuids::uuid random_uuid; + // that stuff turns out to be included, even though it's from src... Taking advantage + crypto::generate_random_bytes(sizeof(random_uuid), &random_uuid); + + context.set_details(random_uuid, ip_, remote_ep.port(), is_income); _dbg3("[sock " << socket_.native_handle() << "] new connection from " << print_connection_context_short(context) << " to " << local_ep.address().to_string() << ':' << local_ep.port() << ", total sockets objects " << m_ref_sock_count); From 7edfdd8f240e6cfb63998f4f5095ab5d174c5ad7 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 19:01:43 +0000 Subject: [PATCH 3/7] blockchain: fix m_sync_counter uninitialized variable use It counts the number of blocks added since last zeroing --- src/cryptonote_core/blockchain.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index b27927c0a..62c33f481 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -100,7 +100,7 @@ static const uint64_t testnet_hard_fork_version_1_till = 624633; //------------------------------------------------------------------ Blockchain::Blockchain(tx_memory_pool& tx_pool) : m_db(), m_tx_pool(tx_pool), m_timestamps_and_difficulties_height(0), m_current_block_cumul_sz_limit(0), m_is_in_checkpoint_zone(false), - m_is_blockchain_storing(false), m_enforce_dns_checkpoints(false), m_max_prepare_blocks_threads(4), m_db_blocks_per_sync(1), m_db_sync_mode(db_async), m_fast_sync(true) + m_is_blockchain_storing(false), m_enforce_dns_checkpoints(false), m_max_prepare_blocks_threads(4), m_db_blocks_per_sync(1), m_db_sync_mode(db_async), m_fast_sync(true), m_sync_counter(0) { LOG_PRINT_L3("Blockchain::" << __func__); } From 444e22f01a1a2a00cb5352d3d4b2e2a4f498368f Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 19:02:33 +0000 Subject: [PATCH 4/7] blockchain: remove unused timer --- src/cryptonote_core/blockchain.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 62c33f481..4b7add9eb 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2144,7 +2144,6 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc // make sure that output being spent matches up correctly with the // signature spending it. - TIME_MEASURE_START(aa); if (!check_tx_input(in_to_key, tx_prefix_hash, tx.signatures[sig_index], pubkeys[sig_index], pmax_used_block_height)) { it->second[in_to_key.k_image] = false; From 601ad76a5e651638aae1c124eeb286df4db8b347 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 22:13:20 +0000 Subject: [PATCH 5/7] hardfork: fix mixup in indexing variable in get_voting_info --- src/cryptonote_core/hardfork.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_core/hardfork.cpp b/src/cryptonote_core/hardfork.cpp index 463d9c065..66fb05010 100644 --- a/src/cryptonote_core/hardfork.cpp +++ b/src/cryptonote_core/hardfork.cpp @@ -373,7 +373,7 @@ bool HardFork::get_voting_info(uint8_t version, uint32_t &window, uint32_t &vote votes = 0; for (size_t n = version; n < 256; ++n) votes += last_versions[n]; - threshold = (window * heights[current_version].threshold + 99) / 100; + threshold = (window * heights[current_fork_index].threshold + 99) / 100; //assert((votes >= threshold) == enabled); earliest_height = get_earliest_ideal_height_for_version(version); voting = heights.back().version; From a7e817482cc85539d80eaa7bd71516edc9362ecf Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 22:14:09 +0000 Subject: [PATCH 6/7] tx_pool: fix serialization of new relayed data --- src/cryptonote_core/tx_pool.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h index 34dc1f72f..9667e6159 100644 --- a/src/cryptonote_core/tx_pool.h +++ b/src/cryptonote_core/tx_pool.h @@ -239,8 +239,6 @@ namespace boost ar & td.last_failed_height; ar & td.last_failed_id; ar & td.receive_time; - if (version < 9) - return; ar & td.last_relayed_time; ar & td.relayed; } From bcac1018af2315d9a6ce5986166c8016c6f1827f Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 30 Jan 2016 22:14:51 +0000 Subject: [PATCH 7/7] daemon: fix a few issues reported by valgrind In particular, ensure we check the status of RPC response structures, as some functions will return success, but with a BUSY status, when the daemon is not yet synced, and the response will not filled. --- src/daemon/rpc_command_executor.cpp | 72 +++++++++++++++++------------ 1 file changed, 43 insertions(+), 29 deletions(-) diff --git a/src/daemon/rpc_command_executor.cpp b/src/daemon/rpc_command_executor.cpp index 6809f68b9..6e147a8c8 100644 --- a/src/daemon/rpc_command_executor.cpp +++ b/src/daemon/rpc_command_executor.cpp @@ -117,7 +117,7 @@ bool t_rpc_command_executor::print_peer_list() { } else { - if (!m_rpc_server->on_get_peer_list(req, res)) + if (!m_rpc_server->on_get_peer_list(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << failure_message; return false; @@ -152,9 +152,10 @@ bool t_rpc_command_executor::save_blockchain() { } else { - if (!m_rpc_server->on_save_bc(req, res)) + if (!m_rpc_server->on_save_bc(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); + return true; } } @@ -179,7 +180,7 @@ bool t_rpc_command_executor::show_hash_rate() { } else { - if (!m_rpc_server->on_set_log_hash_rate(req, res)) + if (!m_rpc_server->on_set_log_hash_rate(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); } @@ -206,7 +207,7 @@ bool t_rpc_command_executor::hide_hash_rate() { } else { - if (!m_rpc_server->on_set_log_hash_rate(req, res)) + if (!m_rpc_server->on_set_log_hash_rate(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -233,7 +234,7 @@ bool t_rpc_command_executor::show_difficulty() { } else { - if (!m_rpc_server->on_get_info(req, res)) + if (!m_rpc_server->on_get_info(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -250,6 +251,7 @@ bool t_rpc_command_executor::show_difficulty() { static std::string get_mining_speed(uint64_t hr) { +std::cerr << "get_mining_speed called with " << hr << std::endl; if (hr>1e9) return (boost::format("%.2f GH/s") % (hr/1e9)).str(); if (hr>1e6) return (boost::format("%.2f MH/s") % (hr/1e6)).str(); if (hr>1e3) return (boost::format("%.2f kH/s") % (hr/1e3)).str(); @@ -267,6 +269,8 @@ bool t_rpc_command_executor::show_status() { std::string fail_message = "Problem fetching info"; + hfreq.version = 0; + bool mining_busy = false; if (m_is_rpc) { if (!m_rpc_client->rpc_request(ireq, ires, "/getinfo", fail_message.c_str())) @@ -284,12 +288,12 @@ bool t_rpc_command_executor::show_status() { } else { - if (!m_rpc_server->on_get_info(ireq, ires)) + if (!m_rpc_server->on_get_info(ireq, ires) || ires.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; } - if (!m_rpc_server->on_hard_fork_info(hfreq, hfres, error_resp)) + if (!m_rpc_server->on_hard_fork_info(hfreq, hfres, error_resp) || hfres.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -299,6 +303,16 @@ bool t_rpc_command_executor::show_status() { tools::fail_msg_writer() << fail_message.c_str(); return true; } + + if (mres.status == CORE_RPC_STATUS_BUSY) + { + mining_busy = true; + } + else if (mres.status != CORE_RPC_STATUS_OK) + { + tools::fail_msg_writer() << fail_message.c_str(); + return true; + } } tools::success_msg_writer() << boost::format("Height: %llu/%llu (%.1f%%) on %s, %s, net hash %s, v%u, %s, %u+%u connections") @@ -306,7 +320,7 @@ bool t_rpc_command_executor::show_status() { % (unsigned long long)(ires.target_height >= ires.height ? ires.target_height : ires.height) % (100.0f * ires.height / (ires.target_height ? ires.target_height < ires.height ? ires.height : ires.target_height : ires.height)) % (ires.testnet ? "testnet" : "mainnet") - % (mres.active ? "mining at " + get_mining_speed(mres.speed) : "not mining") + % (mining_busy ? "syncing" : mres.active ? "mining at " + get_mining_speed(mres.speed) : "not mining") % get_mining_speed(ires.difficulty / ires.target) % (unsigned)hfres.version % (hfres.state == cryptonote::HardFork::Ready ? "up to date" : hfres.state == cryptonote::HardFork::UpdateNeeded ? "update needed" : "out of date, likely forked") @@ -332,7 +346,7 @@ bool t_rpc_command_executor::print_connections() { } else { - if (!m_rpc_server->on_get_connections(req, res, error_resp)) + if (!m_rpc_server->on_get_connections(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -400,7 +414,7 @@ bool t_rpc_command_executor::print_blockchain_info(uint64_t start_block_index, u } else { - if (!m_rpc_server->on_getblockheadersrange(req, res, error_resp)) + if (!m_rpc_server->on_getblockheadersrange(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -438,7 +452,7 @@ bool t_rpc_command_executor::set_log_level(int8_t level) { } else { - if (!m_rpc_server->on_set_log_level(req, res)) + if (!m_rpc_server->on_set_log_level(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -465,7 +479,7 @@ bool t_rpc_command_executor::print_height() { } else { - if (!m_rpc_server->on_get_height(req, res)) + if (!m_rpc_server->on_get_height(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -495,7 +509,7 @@ bool t_rpc_command_executor::print_block_by_hash(crypto::hash block_hash) { } else { - if (!m_rpc_server->on_get_block(req, res, error_resp)) + if (!m_rpc_server->on_get_block(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -526,7 +540,7 @@ bool t_rpc_command_executor::print_block_by_height(uint64_t height) { } else { - if (!m_rpc_server->on_get_block(req, res, error_resp)) + if (!m_rpc_server->on_get_block(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -555,7 +569,7 @@ bool t_rpc_command_executor::print_transaction(crypto::hash transaction_hash) { else { req.txs_hashes.push_back(epee::string_tools::pod_to_hex(transaction_hash)); - if (!m_rpc_server->on_get_transactions(req, res)) + if (!m_rpc_server->on_get_transactions(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -608,7 +622,7 @@ bool t_rpc_command_executor::is_key_image_spent(const crypto::key_image &ki) { } else { - if (!m_rpc_server->on_is_key_image_spent(req, res)) + if (!m_rpc_server->on_is_key_image_spent(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -643,7 +657,7 @@ bool t_rpc_command_executor::print_transaction_pool_long() { } else { - if (!m_rpc_server->on_get_transaction_pool(req, res)) + if (!m_rpc_server->on_get_transaction_pool(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -723,7 +737,7 @@ bool t_rpc_command_executor::print_transaction_pool_short() { } else { - if (!m_rpc_server->on_get_transaction_pool(req, res)) + if (!m_rpc_server->on_get_transaction_pool(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -767,7 +781,7 @@ bool t_rpc_command_executor::start_mining(cryptonote::account_public_address add } else { - if (!m_rpc_server->on_start_mining(req, res)) + if (!m_rpc_server->on_start_mining(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -792,7 +806,7 @@ bool t_rpc_command_executor::stop_mining() { } else { - if (!m_rpc_server->on_stop_mining(req, res)) + if (!m_rpc_server->on_stop_mining(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -833,7 +847,7 @@ bool t_rpc_command_executor::stop_daemon() } else { - if (!m_rpc_server->on_stop_daemon(req, res)) + if (!m_rpc_server->on_stop_daemon(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -928,7 +942,7 @@ bool t_rpc_command_executor::fast_exit() else { - if (!m_rpc_server->on_fast_exit(req, res)) + if (!m_rpc_server->on_fast_exit(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -959,7 +973,7 @@ bool t_rpc_command_executor::out_peers(uint64_t limit) } else { - if (!m_rpc_server->on_out_peers(req, res)) + if (!m_rpc_server->on_out_peers(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -987,7 +1001,7 @@ bool t_rpc_command_executor::start_save_graph() else { - if (!m_rpc_server->on_start_save_graph(req, res)) + if (!m_rpc_server->on_start_save_graph(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -1013,7 +1027,7 @@ bool t_rpc_command_executor::stop_save_graph() else { - if (!m_rpc_server->on_stop_save_graph(req, res)) + if (!m_rpc_server->on_stop_save_graph(req, res) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -1040,7 +1054,7 @@ bool t_rpc_command_executor::hard_fork_info(uint8_t version) } else { - if (!m_rpc_server->on_hard_fork_info(req, res, error_resp)) + if (!m_rpc_server->on_hard_fork_info(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -1071,7 +1085,7 @@ bool t_rpc_command_executor::print_bans() } else { - if (!m_rpc_server->on_get_bans(req, res, error_resp)) + if (!m_rpc_server->on_get_bans(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -1115,7 +1129,7 @@ bool t_rpc_command_executor::ban(const std::string &ip, time_t seconds) } else { - if (!m_rpc_server->on_set_bans(req, res, error_resp)) + if (!m_rpc_server->on_set_bans(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true; @@ -1151,7 +1165,7 @@ bool t_rpc_command_executor::unban(const std::string &ip) } else { - if (!m_rpc_server->on_set_bans(req, res, error_resp)) + if (!m_rpc_server->on_set_bans(req, res, error_resp) || res.status != CORE_RPC_STATUS_OK) { tools::fail_msg_writer() << fail_message.c_str(); return true;