From af2106122dffc1fb9ec2971c2b7348f58a4f2d60 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 30 Dec 2015 10:36:51 +0000 Subject: [PATCH 1/6] net_peerlist: move a couple functions from public to private They do not take the object lock, and are meant to be used only internally, called from a function which does take the lock. --- src/p2p/net_peerlist.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/p2p/net_peerlist.h b/src/p2p/net_peerlist.h index 3d8b08ce6..03f508195 100644 --- a/src/p2p/net_peerlist.h +++ b/src/p2p/net_peerlist.h @@ -80,8 +80,6 @@ namespace nodetool bool set_peer_just_seen(peerid_type peer, const net_address& addr); bool set_peer_unreachable(const peerlist_entry& pr); bool is_ip_allowed(uint32_t ip); - void trim_white_peerlist(); - void trim_gray_peerlist(); private: @@ -166,6 +164,8 @@ namespace nodetool private: bool peers_indexed_from_old(const peers_indexed_old& pio, peers_indexed& pi); + void trim_white_peerlist(); + void trim_gray_peerlist(); friend class boost::serialization::access; epee::critical_section m_peerlist_lock; From 92ef6b54fe00d6b2e065b9a50884418ecaa76d93 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 30 Dec 2015 12:43:15 +0000 Subject: [PATCH 2/6] wallet: protect against exceptions in the block pull thread This can happen when the daemon exits, which would also cause the wallet to crash via unhandled exception --- src/wallet/wallet2.cpp | 44 ++++++++++++++++++++++++++++-------------- src/wallet/wallet2.h | 2 +- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index cfa33b283..19b598af8 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -648,21 +648,30 @@ void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched) refresh(start_height, blocks_fetched, received_money); } //---------------------------------------------------------------------------------------------------- -void wallet2::pull_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::list &prev_blocks, std::list &blocks) +void wallet2::pull_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::list &prev_blocks, std::list &blocks, bool &error) { - // prepend the last 3 blocks, should be enough to guard against a block or two's reorg - cryptonote::block bl; - std::list::const_reverse_iterator i = prev_blocks.rbegin(); - for (size_t n = 0; n < std::min((size_t)3, prev_blocks.size()); ++n) - { - bool ok = cryptonote::parse_and_validate_block_from_blob(i->block, bl); - THROW_WALLET_EXCEPTION_IF(!ok, error::block_parse_error, i->block); - short_chain_history.push_front(cryptonote::get_block_hash(bl)); - ++i; - } + error = false; - // pull the new blocks - pull_blocks(start_height, blocks_start_height, short_chain_history, blocks); + try + { + // prepend the last 3 blocks, should be enough to guard against a block or two's reorg + cryptonote::block bl; + std::list::const_reverse_iterator i = prev_blocks.rbegin(); + for (size_t n = 0; n < std::min((size_t)3, prev_blocks.size()); ++n) + { + bool ok = cryptonote::parse_and_validate_block_from_blob(i->block, bl); + THROW_WALLET_EXCEPTION_IF(!ok, error::block_parse_error, i->block); + short_chain_history.push_front(cryptonote::get_block_hash(bl)); + ++i; + } + + // pull the new blocks + pull_blocks(start_height, blocks_start_height, short_chain_history, blocks); + } + catch(...) + { + error = true; + } } //---------------------------------------------------------------------------------------------------- void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched, bool& received_money) @@ -689,7 +698,8 @@ void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched, bool& re // pull the next set of blocks while we're processing the current one uint64_t next_blocks_start_height; std::list next_blocks; - pull_thread = std::thread([&]{pull_next_blocks(start_height, next_blocks_start_height, short_chain_history, blocks, next_blocks);}); + bool error = false; + pull_thread = std::thread([&]{pull_next_blocks(start_height, next_blocks_start_height, short_chain_history, blocks, next_blocks, error);}); process_blocks(blocks_start_height, blocks, added_blocks); blocks_fetched += added_blocks; @@ -700,6 +710,12 @@ void wallet2::refresh(uint64_t start_height, uint64_t & blocks_fetched, bool& re // switch to the new blocks from the daemon blocks_start_height = next_blocks_start_height; blocks = next_blocks; + + // handle error from async fetching thread + if (error) + { + throw std::runtime_error("proxy exception in refresh thread"); + } } catch (const std::exception&) { diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index adb7b13e2..77a9fd18e 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -359,7 +359,7 @@ namespace tools bool is_transfer_unlocked(const transfer_details& td) const; bool clear(); void pull_blocks(uint64_t start_height, uint64_t& blocks_start_height, const std::list &short_chain_history, std::list &blocks); - void pull_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::list &prev_blocks, std::list &blocks); + void pull_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::list &prev_blocks, std::list &blocks, bool &error); void process_blocks(uint64_t start_height, const std::list &blocks, uint64_t& blocks_added); uint64_t select_transfers(uint64_t needed_money, bool add_dust, uint64_t dust, std::list& selected_transfers); bool prepare_file_names(const std::string& file_path); From bc8a52efd8dcda4544856e724a17c9cba161d4a2 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 30 Dec 2015 12:58:15 +0000 Subject: [PATCH 3/6] wallet: add a rescan_bc command and rescan_blockchain RPC Blockchain hashes and key images are flushed, and blocks are pulled anew from the daemon. The console command is shortened to match bc_height. This should make it a lot easier on users who are currently told to remove this particular cache file but keep the keys one, etc, etc. --- src/simplewallet/simplewallet.cpp | 39 +++++++++++++------- src/simplewallet/simplewallet.h | 2 + src/wallet/wallet2.cpp | 15 ++++++++ src/wallet/wallet2.h | 1 + src/wallet/wallet_rpc_server.cpp | 23 ++++++++++++ src/wallet/wallet_rpc_server.h | 2 + src/wallet/wallet_rpc_server_commands_defs.h | 15 ++++++++ 7 files changed, 84 insertions(+), 13 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index ad4cd5622..b8586bf17 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -551,6 +551,7 @@ simple_wallet::simple_wallet() m_cmd_binder.set_handler("get_tx_key", boost::bind(&simple_wallet::get_tx_key, this, _1), tr("Get transaction key (r) for a given ")); m_cmd_binder.set_handler("check_tx_key", boost::bind(&simple_wallet::check_tx_key, this, _1), tr("Check amount going to
in ")); m_cmd_binder.set_handler("show_transfers", boost::bind(&simple_wallet::show_transfers, this, _1), tr("show_transfers [in|out] [ []] - Show incoming/outgoing transfers within an optional height range")); + m_cmd_binder.set_handler("rescan_bc", boost::bind(&simple_wallet::rescan_blockchain, this, _1), tr("Rescan blockchain from scratch")); m_cmd_binder.set_handler("help", boost::bind(&simple_wallet::help, this, _1), tr("Show this help")); } //---------------------------------------------------------------------------------------------------- @@ -1324,7 +1325,7 @@ void simple_wallet::on_skip_transaction(uint64_t height, const cryptonote::trans m_refresh_progress_reporter.update(height, true); } //---------------------------------------------------------------------------------------------------- -bool simple_wallet::refresh(const std::vector& args) +bool simple_wallet::refresh_main(uint64_t start_height, bool reset) { if (!try_connect_to_daemon()) return true; @@ -1336,21 +1337,12 @@ bool simple_wallet::refresh(const std::vector& args) std::unique_lock lock(m_auto_refresh_mutex); m_auto_refresh_cond.notify_one(); + if (reset) + m_wallet->rescan_blockchain(false); + message_writer() << tr("Starting refresh..."); uint64_t fetched_blocks = 0; - uint64_t start_height = 0; - if(!args.empty()){ - try - { - start_height = boost::lexical_cast( args[0] ); - } - catch(const boost::bad_lexical_cast &) - { - start_height = 0; - } - } - bool ok = false; std::ostringstream ss; try @@ -1408,6 +1400,22 @@ bool simple_wallet::refresh(const std::vector& args) return true; } //---------------------------------------------------------------------------------------------------- +bool simple_wallet::refresh(const std::vector& args) +{ + uint64_t start_height = 0; + if(!args.empty()){ + try + { + start_height = boost::lexical_cast( args[0] ); + } + catch(const boost::bad_lexical_cast &) + { + start_height = 0; + } + } + return refresh_main(start_height, false); +} +//---------------------------------------------------------------------------------------------------- bool simple_wallet::show_balance(const std::vector& args/* = std::vector()*/) { success_msg_writer() << tr("Balance: ") << print_money(m_wallet->balance()) << ", " @@ -2276,6 +2284,11 @@ bool simple_wallet::show_transfers(const std::vector &args_) return true; } //---------------------------------------------------------------------------------------------------- +bool simple_wallet::rescan_blockchain(const std::vector &args_) +{ + return refresh_main(0, true); +} +//---------------------------------------------------------------------------------------------------- void simple_wallet::wallet_refresh_thread() { while (true) diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index ef44d5374..486e197ae 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -132,6 +132,8 @@ namespace cryptonote bool get_tx_key(const std::vector &args); bool check_tx_key(const std::vector &args); bool show_transfers(const std::vector &args); + bool rescan_blockchain(const std::vector &args); + bool refresh_main(uint64_t start_height, bool reset = false); uint64_t get_daemon_blockchain_height(std::string& err); bool try_connect_to_daemon(); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 19b598af8..e58629dac 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -801,6 +801,7 @@ bool wallet2::clear() { m_blockchain.clear(); m_transfers.clear(); + m_key_images.clear(); m_local_bc_height = 1; return true; } @@ -1431,6 +1432,20 @@ void wallet2::rescan_spent() } } //---------------------------------------------------------------------------------------------------- +void wallet2::rescan_blockchain(bool refresh) +{ + clear(); + + cryptonote::block genesis; + generate_genesis(genesis); + crypto::hash genesis_hash = get_block_hash(genesis); + m_blockchain.push_back(genesis_hash); + m_local_bc_height = 1; + + if (refresh) + this->refresh(); +} +//---------------------------------------------------------------------------------------------------- bool wallet2::is_transfer_unlocked(const transfer_details& td) const { if(!is_tx_spendtime_unlocked(td.m_tx.unlock_time)) diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 77a9fd18e..88a5268bc 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -280,6 +280,7 @@ namespace tools void get_unconfirmed_payments_out(std::list>& unconfirmed_payments) const; uint64_t get_blockchain_current_height() const { return m_local_bc_height; } void rescan_spent(); + void rescan_blockchain(bool refresh = true); template inline void serialize(t_archive &a, const unsigned int ver) { diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 0c1ea48c1..9f23908f9 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -646,6 +646,29 @@ namespace tools return true; } + //------------------------------------------------------------------------------------------------------------------------------ + bool wallet_rpc_server::on_rescan_blockchain(const wallet_rpc::COMMAND_RPC_RESCAN_BLOCKCHAIN::request& req, wallet_rpc::COMMAND_RPC_RESCAN_BLOCKCHAIN::response& res, epee::json_rpc::error& er) + { + if (m_wallet.restricted()) + { + er.code = WALLET_RPC_ERROR_CODE_DENIED; + er.message = "Command unavailable in restricted mode."; + return false; + } + + try + { + m_wallet.rescan_blockchain(); + } + catch (std::exception& e) + { + er.code = WALLET_RPC_ERROR_CODE_UNKNOWN_ERROR; + er.message = e.what(); + return false; + } + return true; + } + //------------------------------------------------------------------------------------------------------------------------------ bool wallet_rpc_server::on_stop_wallet(const wallet_rpc::COMMAND_RPC_STOP_WALLET::request& req, wallet_rpc::COMMAND_RPC_STOP_WALLET::response& res, epee::json_rpc::error& er) { if (m_wallet.restricted()) diff --git a/src/wallet/wallet_rpc_server.h b/src/wallet/wallet_rpc_server.h index 97b84fcb6..fbc2c29c2 100644 --- a/src/wallet/wallet_rpc_server.h +++ b/src/wallet/wallet_rpc_server.h @@ -75,6 +75,7 @@ namespace tools MAP_JON_RPC_WE("make_integrated_address", on_make_integrated_address, wallet_rpc::COMMAND_RPC_MAKE_INTEGRATED_ADDRESS) MAP_JON_RPC_WE("split_integrated_address", on_split_integrated_address, wallet_rpc::COMMAND_RPC_SPLIT_INTEGRATED_ADDRESS) MAP_JON_RPC_WE("stop_wallet", on_stop_wallet, wallet_rpc::COMMAND_RPC_STOP_WALLET) + MAP_JON_RPC_WE("rescan_blockchain", on_rescan_blockchain, wallet_rpc::COMMAND_RPC_RESCAN_BLOCKCHAIN) END_JSON_RPC_MAP() END_URI_MAP2() @@ -93,6 +94,7 @@ namespace tools bool on_get_bulk_payments(const wallet_rpc::COMMAND_RPC_GET_BULK_PAYMENTS::request& req, wallet_rpc::COMMAND_RPC_GET_BULK_PAYMENTS::response& res, epee::json_rpc::error& er); bool on_incoming_transfers(const wallet_rpc::COMMAND_RPC_INCOMING_TRANSFERS::request& req, wallet_rpc::COMMAND_RPC_INCOMING_TRANSFERS::response& res, epee::json_rpc::error& er); bool on_stop_wallet(const wallet_rpc::COMMAND_RPC_STOP_WALLET::request& req, wallet_rpc::COMMAND_RPC_STOP_WALLET::response& res, epee::json_rpc::error& er); + bool on_rescan_blockchain(const wallet_rpc::COMMAND_RPC_RESCAN_BLOCKCHAIN::request& req, wallet_rpc::COMMAND_RPC_RESCAN_BLOCKCHAIN::response& res, epee::json_rpc::error& er); bool handle_command_line(const boost::program_options::variables_map& vm); diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index a20c8b969..1e1482c1a 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -391,6 +391,21 @@ namespace wallet_rpc }; }; + struct COMMAND_RPC_RESCAN_BLOCKCHAIN + { + struct request + { + BEGIN_KV_SERIALIZE_MAP() + END_KV_SERIALIZE_MAP() + }; + + struct response + { + BEGIN_KV_SERIALIZE_MAP() + END_KV_SERIALIZE_MAP() + }; + }; + } } From b2452151931ffe98337181567aaf797c1f530c3f Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 30 Dec 2015 16:05:52 +0000 Subject: [PATCH 4/6] core_tests: deinit core before destroying it This fixes a use after free by ioservice threads --- tests/core_tests/chaingen.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/core_tests/chaingen.h b/tests/core_tests/chaingen.h index 822ccfb11..17b78da8e 100644 --- a/tests/core_tests/chaingen.h +++ b/tests/core_tests/chaingen.h @@ -498,7 +498,9 @@ inline bool do_replay_events(std::vector& events) return false; } t_test_class validator; - return replay_events_through_core(c, events, validator); + bool ret = replay_events_through_core(c, events, validator); + c.deinit(); + return ret; } //-------------------------------------------------------------------------- template From ed5d017c0f95cfec4a068c78684b98675027f74e Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 30 Dec 2015 16:06:32 +0000 Subject: [PATCH 5/6] miner: minor fixes on stop - only try to stop if actually started - print number of threads before zeroing it This fixes the suspiciously doubled "Mining has been stopped" message on exit. --- src/cryptonote_core/miner.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cryptonote_core/miner.cpp b/src/cryptonote_core/miner.cpp index 00a474524..5fde94752 100644 --- a/src/cryptonote_core/miner.cpp +++ b/src/cryptonote_core/miner.cpp @@ -278,14 +278,17 @@ namespace cryptonote //----------------------------------------------------------------------------------------------------- bool miner::stop() { + if (!is_mining()) + return true; + send_stop_signal(); CRITICAL_REGION_LOCAL(m_threads_lock); BOOST_FOREACH(boost::thread& th, m_threads) th.join(); - m_threads.clear(); LOG_PRINT_L0("Mining has been stopped, " << m_threads.size() << " finished" ); + m_threads.clear(); return true; } //----------------------------------------------------------------------------------------------------- From 576effe11a513507b14275c7a2e6fc3b18de5e17 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 30 Dec 2015 16:08:30 +0000 Subject: [PATCH 6/6] blockchain: kill ioservice on scope end, rather than manually This ensures this will be done without fail, as the error prone matching of every return with a call to KILL_IOSERVICE leads to hard to debug corruption when one is missing. --- src/cryptonote_core/blockchain.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 141a42f49..e4d3d57fc 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -2078,6 +2078,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc boost::asio::io_service ioservice; boost::thread_group threadpool; + bool ioservice_active = false; std::unique_ptr < boost::asio::io_service::work > work(new boost::asio::io_service::work(ioservice)); if(threads > 1) @@ -2086,15 +2087,19 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc { threadpool.create_thread(boost::bind(&boost::asio::io_service::run, &ioservice)); } + ioservice_active = true; } #define KILL_IOSERVICE() \ - if(threads > 1) \ + if(ioservice_active) \ { \ work.reset(); \ threadpool.join_all(); \ ioservice.stop(); \ - } \ + ioservice_active = false; \ + } + + epee::misc_utils::auto_scope_leave_caller ioservice_killer = epee::misc_utils::create_scope_leave_handler([&]() { KILL_IOSERVICE(); }); for (const auto& txin : tx.vin) { @@ -2109,7 +2114,6 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc if(have_tx_keyimg_as_spent(in_to_key.k_image)) { LOG_PRINT_L1("Key image already spent in blockchain: " << epee::string_tools::pod_to_hex(in_to_key.k_image)); - KILL_IOSERVICE(); return false; } @@ -2123,7 +2127,6 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc if(!itk->second) { LOG_PRINT_L1("Failed ring signature for tx " << get_transaction_hash(tx) << " vin key with k_image: " << in_to_key.k_image << " sig_index: " << sig_index); - KILL_IOSERVICE(); return false; } @@ -2145,7 +2148,6 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc LOG_PRINT_L1(" *pmax_used_block_height: " << *pmax_used_block_height); } - KILL_IOSERVICE(); return false; } @@ -2168,7 +2170,6 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc LOG_PRINT_L1("*pmax_used_block_height: " << *pmax_used_block_height); } - KILL_IOSERVICE(); return false; } it->second[in_to_key.k_image] = true;