From d5472bd87b8e93706295d8aa7ff99e5ad594277d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 15 Oct 2019 12:25:26 +0000 Subject: [PATCH] wallet2: do not send an unnecessary last getblocks.bin call on refresh MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "everything refreshed" state was detected when a refresh call did not return any new blocks. This can be detected without that extra "empty" call by comparing the claimed node height to the height of the last block retrieved. Doing this avoids that last call, saves some bandwidth, and makes the common refresh case use only one call rather than two. As a side effect, it prevents an information leak reported by Tramèr et al: if the wallet retrieves a set of blocks which includes an output sent to the refreshing wallet, the wallet will prompt the user for the password to decode the amount and calculate the key image for the new output, and this will delay subsequent calls to getblocks.bin, allowing a passive adversary to note the delay and deduce when the wallet receives at least one output. This can still happen if the wallet downloads more than 1000 blocks, since this will be split in several calls, but then the most the adversary can tell is which 1000 block section the user received some monero (the adversary can estimate the heights of the blocks by calculating how many "large" transfers are done, which will be sections of blocks, the last of which will usually be below 1000, but the size of the data should allow the actual number of blocks sent to be determined fairly accurately). This timing trick still be used via the subsequent scan for incoming txes in the txpool, which will be fixed later. --- src/wallet/wallet2.cpp | 20 +++++++++++++++----- src/wallet/wallet2.h | 4 ++-- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 4f0e2c26a..c7c49f445 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2519,12 +2519,14 @@ void wallet2::parse_block_round(const cryptonote::blobdata &blob, cryptonote::bl error = !cryptonote::parse_and_validate_block_from_blob(blob, bl, bl_id); } //---------------------------------------------------------------------------------------------------- -void wallet2::pull_blocks(uint64_t start_height, uint64_t &blocks_start_height, const std::list &short_chain_history, std::vector &blocks, std::vector &o_indices) +void wallet2::pull_blocks(uint64_t start_height, uint64_t &blocks_start_height, const std::list &short_chain_history, std::vector &blocks, std::vector &o_indices, uint64_t ¤t_height) { cryptonote::COMMAND_RPC_GET_BLOCKS_FAST::request req = AUTO_VAL_INIT(req); cryptonote::COMMAND_RPC_GET_BLOCKS_FAST::response res = AUTO_VAL_INIT(res); req.block_ids = short_chain_history; + MDEBUG("Pulling blocks: start_height " << start_height); + req.prune = true; req.start_height = start_height; req.no_miner_tx = m_refresh_type == RefreshNoCoinbase; @@ -2544,6 +2546,10 @@ void wallet2::pull_blocks(uint64_t start_height, uint64_t &blocks_start_height, blocks_start_height = res.start_height; blocks = std::move(res.blocks); o_indices = std::move(res.output_indices); + current_height = res.current_height; + + MDEBUG("Pulled blocks: blocks_start_height " << blocks_start_height << ", count " << blocks.size() + << ", height " << blocks_start_height + blocks.size() << ", node height " << res.current_height); } //---------------------------------------------------------------------------------------------------- void wallet2::pull_hashes(uint64_t start_height, uint64_t &blocks_start_height, const std::list &short_chain_history, std::vector &hashes) @@ -2726,9 +2732,10 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo refresh(trusted_daemon, start_height, blocks_fetched, received_money); } //---------------------------------------------------------------------------------------------------- -void wallet2::pull_and_parse_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::vector &prev_blocks, const std::vector &prev_parsed_blocks, std::vector &blocks, std::vector &parsed_blocks, bool &error, std::exception_ptr &exception) +void wallet2::pull_and_parse_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::vector &prev_blocks, const std::vector &prev_parsed_blocks, std::vector &blocks, std::vector &parsed_blocks, bool &last, bool &error, std::exception_ptr &exception) { error = false; + last = false; exception = NULL; try @@ -2746,7 +2753,8 @@ void wallet2::pull_and_parse_next_blocks(uint64_t start_height, uint64_t &blocks // pull the new blocks std::vector o_indices; - pull_blocks(start_height, blocks_start_height, short_chain_history, blocks, o_indices); + uint64_t current_height; + pull_blocks(start_height, blocks_start_height, short_chain_history, blocks, o_indices, current_height); THROW_WALLET_EXCEPTION_IF(blocks.size() != o_indices.size(), error::wallet_internal_error, "Mismatched sizes of blocks and o_indices"); tools::threadpool& tpool = tools::threadpool::getInstance(); @@ -2784,6 +2792,7 @@ void wallet2::pull_and_parse_next_blocks(uint64_t start_height, uint64_t &blocks } } waiter.wait(&tpool); + last = !blocks.empty() && cryptonote::get_block_height(parsed_blocks.back().block) + 1 == current_height; } catch(...) { @@ -3250,7 +3259,7 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo }); auto scope_exit_handler_hwdev = epee::misc_utils::create_scope_leave_handler([&](){hwdev.computing_key_images(false);}); - bool first = true; + bool first = true, last = false; while(m_run.load(std::memory_order_relaxed)) { uint64_t next_blocks_start_height; @@ -3272,7 +3281,8 @@ void wallet2::refresh(bool trusted_daemon, uint64_t start_height, uint64_t & blo refreshed = true; break; } - tpool.submit(&waiter, [&]{pull_and_parse_next_blocks(start_height, next_blocks_start_height, short_chain_history, blocks, parsed_blocks, next_blocks, next_parsed_blocks, error, exception);}); + if (!last) + tpool.submit(&waiter, [&]{pull_and_parse_next_blocks(start_height, next_blocks_start_height, short_chain_history, blocks, parsed_blocks, next_blocks, next_parsed_blocks, last, error, exception);}); if (!first) { diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 640565a4e..d99e6e8c5 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -1403,10 +1403,10 @@ private: void get_short_chain_history(std::list& ids, uint64_t granularity = 1) const; bool clear(); void clear_soft(bool keep_key_images=false); - void pull_blocks(uint64_t start_height, uint64_t& blocks_start_height, const std::list &short_chain_history, std::vector &blocks, std::vector &o_indices); + void pull_blocks(uint64_t start_height, uint64_t& blocks_start_height, const std::list &short_chain_history, std::vector &blocks, std::vector &o_indices, uint64_t ¤t_height); void pull_hashes(uint64_t start_height, uint64_t& blocks_start_height, const std::list &short_chain_history, std::vector &hashes); void fast_refresh(uint64_t stop_height, uint64_t &blocks_start_height, std::list &short_chain_history, bool force = false); - void pull_and_parse_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::vector &prev_blocks, const std::vector &prev_parsed_blocks, std::vector &blocks, std::vector &parsed_blocks, bool &error, std::exception_ptr &exception); + void pull_and_parse_next_blocks(uint64_t start_height, uint64_t &blocks_start_height, std::list &short_chain_history, const std::vector &prev_blocks, const std::vector &prev_parsed_blocks, std::vector &blocks, std::vector &parsed_blocks, bool &last, bool &error, std::exception_ptr &exception); void process_parsed_blocks(uint64_t start_height, const std::vector &blocks, const std::vector &parsed_blocks, uint64_t& blocks_added, std::map, size_t> *output_tracker_cache = NULL); uint64_t select_transfers(uint64_t needed_money, std::vector unused_transfers_indices, std::vector& selected_transfers) const; bool prepare_file_names(const std::string& file_path);