From f065234b71f2e3d374d44435d47db4cd75ab1294 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 22 Mar 2017 18:01:09 +0000 Subject: [PATCH 1/4] core: cache tx and block hashes in the respective classes An idea from smooth --- src/cryptonote_basic/cryptonote_basic.h | 32 ++++++++ .../cryptonote_format_utils.cpp | 79 ++++++++++++++++++- .../cryptonote_format_utils.h | 3 + src/cryptonote_basic/miner.cpp | 2 + src/cryptonote_core/cryptonote_tx_utils.cpp | 6 ++ tests/unit_tests/serialization.cpp | 14 ++++ 6 files changed, 134 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_basic/cryptonote_basic.h b/src/cryptonote_basic/cryptonote_basic.h index ed2787cf7..3492ace2b 100644 --- a/src/cryptonote_basic/cryptonote_basic.h +++ b/src/cryptonote_basic/cryptonote_basic.h @@ -190,11 +190,24 @@ namespace cryptonote std::vector > signatures; //count signatures always the same as inputs count rct::rctSig rct_signatures; + // hash cash + mutable crypto::hash hash; + mutable size_t blob_size; + mutable bool hash_valid; + mutable bool blob_size_valid; + transaction(); virtual ~transaction(); void set_null(); + void invalidate_hashes(); BEGIN_SERIALIZE_OBJECT() + if (!typename Archive::is_saving()) + { + hash_valid = false; + blob_size_valid = false; + } + FIELDS(*static_cast(this)) if (version == 1) @@ -299,6 +312,15 @@ namespace cryptonote extra.clear(); signatures.clear(); rct_signatures.type = rct::RCTTypeNull; + hash_valid = false; + blob_size_valid = false; + } + + inline + void transaction::invalidate_hashes() + { + hash_valid = false; + blob_size_valid = false; } inline @@ -339,10 +361,20 @@ namespace cryptonote struct block: public block_header { + block(): block_header(), hash_valid(false) {} + void invalidate_hashes() { hash_valid = false; } + transaction miner_tx; std::vector tx_hashes; + // hash cash + mutable crypto::hash hash; + mutable bool hash_valid; + BEGIN_SERIALIZE_OBJECT() + if (!typename Archive::is_saving()) + hash_valid = false; + FIELDS(*static_cast(this)) FIELD(miner_tx) FIELD(tx_hashes) diff --git a/src/cryptonote_basic/cryptonote_format_utils.cpp b/src/cryptonote_basic/cryptonote_format_utils.cpp index 00e7b9d80..2f23194c8 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.cpp +++ b/src/cryptonote_basic/cryptonote_format_utils.cpp @@ -43,6 +43,8 @@ using namespace epee; #define ENCRYPTED_PAYMENT_ID_TAIL 0x8d +// #define ENABLE_HASH_CASH_INTEGRITY_CHECK + static const uint64_t valid_decomposed_outputs[] = { (uint64_t)1, (uint64_t)2, (uint64_t)3, (uint64_t)4, (uint64_t)5, (uint64_t)6, (uint64_t)7, (uint64_t)8, (uint64_t)9, // 1 piconero (uint64_t)10, (uint64_t)20, (uint64_t)30, (uint64_t)40, (uint64_t)50, (uint64_t)60, (uint64_t)70, (uint64_t)80, (uint64_t)90, @@ -68,6 +70,11 @@ static const uint64_t valid_decomposed_outputs[] = { static std::atomic default_decimal_point(CRYPTONOTE_DISPLAY_DECIMAL_POINT); +static std::atomic tx_hashes_calculated_count(0); +static std::atomic tx_hashes_cached_count(0); +static std::atomic block_hashes_calculated_count(0); +static std::atomic block_hashes_cached_count(0); + namespace cryptonote { //--------------------------------------------------------------- @@ -93,6 +100,8 @@ namespace cryptonote binary_archive ba(ss); bool r = ::serialization::serialize(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); + tx.hash_valid = false; + tx.blob_size_valid = false; return true; } //--------------------------------------------------------------- @@ -113,6 +122,8 @@ namespace cryptonote binary_archive ba(ss); bool r = ::serialization::serialize(ba, tx); CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction from blob"); + tx.hash_valid = false; + tx.blob_size_valid = false; //TODO: validate tx get_transaction_hash(tx, tx_hash); @@ -592,7 +603,7 @@ namespace cryptonote return get_transaction_hash(t, res, NULL); } //--------------------------------------------------------------- - bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size) + bool calculate_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size) { // v1 transactions hash the entire blob if (t.version == 1) @@ -647,6 +658,40 @@ namespace cryptonote return true; } //--------------------------------------------------------------- + bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size) + { + if (t.hash_valid) + { +#ifdef ENABLE_HASH_CASH_INTEGRITY_CHECK + CHECK_AND_ASSERT_THROW_MES(!calculate_transaction_hash(t, res, blob_size) || t.hash == res, "tx hash cash integrity failure"); +#endif + res = t.hash; + if (blob_size) + { + if (!t.blob_size_valid) + { + t.blob_size = get_object_blobsize(t); + t.blob_size_valid = true; + } + *blob_size = t.blob_size; + } + ++tx_hashes_cached_count; + return true; + } + ++tx_hashes_calculated_count; + bool ret = calculate_transaction_hash(t, res, blob_size); + if (!ret) + return false; + t.hash = res; + t.hash_valid = true; + if (blob_size) + { + t.blob_size = *blob_size; + t.blob_size_valid = true; + } + return true; + } + //--------------------------------------------------------------- bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t& blob_size) { return get_transaction_hash(t, res, &blob_size); @@ -661,7 +706,7 @@ namespace cryptonote return blob; } //--------------------------------------------------------------- - bool get_block_hash(const block& b, crypto::hash& res) + bool calculate_block_hash(const block& b, crypto::hash& res) { // EXCEPTION FOR BLOCK 202612 const std::string correct_blob_hash_202612 = "3a8a2b3a29b50fc86ff73dd087ea43c6f0d6b8f936c849194d5c84c737903966"; @@ -688,6 +733,26 @@ namespace cryptonote return hash_result; } //--------------------------------------------------------------- + bool get_block_hash(const block& b, crypto::hash& res) + { + if (b.hash_valid) + { +#ifdef ENABLE_HASH_CASH_INTEGRITY_CHECK + CHECK_AND_ASSERT_THROW_MES(!calculate_block_hash(b, res) || b.hash == res, "block hash cash integrity failure"); +#endif + res = b.hash; + ++block_hashes_cached_count; + return true; + } + ++block_hashes_calculated_count; + bool ret = calculate_block_hash(b, res); + if (!ret) + return false; + b.hash = res; + b.hash_valid = true; + return true; + } + //--------------------------------------------------------------- crypto::hash get_block_hash(const block& b) { crypto::hash p = null_hash; @@ -744,6 +809,9 @@ namespace cryptonote binary_archive ba(ss); bool r = ::serialization::serialize(ba, b); CHECK_AND_ASSERT_MES(r, false, "Failed to parse block from blob"); + b.hash_valid = false; + b.miner_tx.hash_valid = false; + b.miner_tx.blob_size_valid = false; return true; } //--------------------------------------------------------------- @@ -798,4 +866,11 @@ namespace cryptonote return std::binary_search(begin, end, amount); } //--------------------------------------------------------------- + void get_hash_stats(uint64_t &tx_hashes_calculated, uint64_t &tx_hashes_cached, uint64_t &block_hashes_calculated, uint64_t & block_hashes_cached) + { + tx_hashes_calculated = tx_hashes_calculated_count; + tx_hashes_cached = tx_hashes_cached_count; + block_hashes_calculated = block_hashes_calculated_count; + block_hashes_cached = block_hashes_cached_count; + } } diff --git a/src/cryptonote_basic/cryptonote_format_utils.h b/src/cryptonote_basic/cryptonote_format_utils.h index 7b09235b8..5c10907fd 100644 --- a/src/cryptonote_basic/cryptonote_format_utils.h +++ b/src/cryptonote_basic/cryptonote_format_utils.h @@ -85,7 +85,9 @@ namespace cryptonote bool get_transaction_hash(const transaction& t, crypto::hash& res); bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t& blob_size); bool get_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size); + bool calculate_transaction_hash(const transaction& t, crypto::hash& res, size_t* blob_size); blobdata get_block_hashing_blob(const block& b); + bool calculate_block_hash(const block& b, crypto::hash& res); bool get_block_hash(const block& b, crypto::hash& res); crypto::hash get_block_hash(const block& b); bool get_block_longhash(const block& b, crypto::hash& res, uint64_t height); @@ -209,6 +211,7 @@ namespace cryptonote crypto::hash get_tx_tree_hash(const std::vector& tx_hashes); crypto::hash get_tx_tree_hash(const block& b); bool is_valid_decomposed_amount(uint64_t amount); + void get_hash_stats(uint64_t &tx_hashes_calculated, uint64_t &tx_hashes_cached, uint64_t &block_hashes_calculated, uint64_t & block_hashes_cached); #define CHECKED_GET_SPECIFIC_VARIANT(variant_var, specific_type, variable_name, fail_return_val) \ CHECK_AND_ASSERT_MES(variant_var.type() == typeid(specific_type), fail_return_val, "wrong variant type: " << variant_var.type().name() << ", expected " << typeid(specific_type).name()); \ diff --git a/src/cryptonote_basic/miner.cpp b/src/cryptonote_basic/miner.cpp index 1725b7541..5a2a0d009 100644 --- a/src/cryptonote_basic/miner.cpp +++ b/src/cryptonote_basic/miner.cpp @@ -355,9 +355,11 @@ namespace cryptonote if(check_hash(h, diffic)) { + bl.hash_valid = false; return true; } } + bl.hash_valid = false; return false; } //----------------------------------------------------------------------------------------------------- diff --git a/src/cryptonote_core/cryptonote_tx_utils.cpp b/src/cryptonote_core/cryptonote_tx_utils.cpp index 672a763fc..47211ff95 100644 --- a/src/cryptonote_core/cryptonote_tx_utils.cpp +++ b/src/cryptonote_core/cryptonote_tx_utils.cpp @@ -132,6 +132,9 @@ namespace cryptonote //lock tx.unlock_time = height + CRYPTONOTE_MINED_MONEY_UNLOCK_WINDOW; tx.vin.push_back(in); + + tx.hash_valid = tx.blob_size_valid = false; + //LOG_PRINT("MINER_TX generated ok, block_reward=" << print_money(block_reward) << "(" << print_money(block_reward - fee) << "+" << print_money(fee) // << "), current_block_size=" << current_block_size << ", already_generated_coins=" << already_generated_coins << ", tx_id=" << get_transaction_hash(tx), LOG_LEVEL_2); return true; @@ -451,6 +454,8 @@ namespace cryptonote MCINFO("construct_tx", "transaction_created: " << get_transaction_hash(tx) << ENDL << obj_to_json_str(tx) << ENDL); } + tx.hash_valid = tx.blob_size_valid = false; + return true; } //--------------------------------------------------------------- @@ -487,6 +492,7 @@ namespace cryptonote bl.timestamp = 0; bl.nonce = nonce; miner::find_nonce_for_given_block(bl, 1, 0); + bl.hash_valid = false; return true; } //--------------------------------------------------------------- diff --git a/tests/unit_tests/serialization.cpp b/tests/unit_tests/serialization.cpp index 86e20266b..7afc0d60c 100644 --- a/tests/unit_tests/serialization.cpp +++ b/tests/unit_tests/serialization.cpp @@ -331,6 +331,7 @@ TEST(Serialization, serializes_transacion_signatures_correctly) // Miner tx with empty signatures 2nd vector tx.signatures.resize(1); + tx.invalidate_hashes(); ASSERT_TRUE(serialization::dump_binary(tx, blob)); ASSERT_EQ(7, blob.size()); // 5 bytes + 2 bytes vin[0] + 0 bytes extra + 0 bytes signatures ASSERT_TRUE(serialization::parse_binary(blob, tx1)); @@ -345,16 +346,19 @@ TEST(Serialization, serializes_transacion_signatures_correctly) tx.signatures.resize(2); tx.signatures[0].resize(0); tx.signatures[1].resize(0); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // Miner tx with 2 signatures tx.signatures[0].resize(1); tx.signatures[1].resize(1); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // Two txin_gen, no signatures tx.vin.push_back(txin_gen1); tx.signatures.resize(0); + tx.invalidate_hashes(); ASSERT_TRUE(serialization::dump_binary(tx, blob)); ASSERT_EQ(9, blob.size()); // 5 bytes + 2 * 2 bytes vins + 0 bytes extra + 0 bytes signatures ASSERT_TRUE(serialization::parse_binary(blob, tx1)); @@ -363,10 +367,12 @@ TEST(Serialization, serializes_transacion_signatures_correctly) // Two txin_gen, signatures vector contains only one empty element tx.signatures.resize(1); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // Two txin_gen, signatures vector contains two empty elements tx.signatures.resize(2); + tx.invalidate_hashes(); ASSERT_TRUE(serialization::dump_binary(tx, blob)); ASSERT_EQ(9, blob.size()); // 5 bytes + 2 * 2 bytes vins + 0 bytes extra + 0 bytes signatures ASSERT_TRUE(serialization::parse_binary(blob, tx1)); @@ -375,18 +381,21 @@ TEST(Serialization, serializes_transacion_signatures_correctly) // Two txin_gen, signatures vector contains three empty elements tx.signatures.resize(3); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // Two txin_gen, signatures vector contains two non empty elements tx.signatures.resize(2); tx.signatures[0].resize(1); tx.signatures[1].resize(1); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // A few bytes instead of signature tx.vin.clear(); tx.vin.push_back(txin_gen1); tx.signatures.clear(); + tx.invalidate_hashes(); ASSERT_TRUE(serialization::dump_binary(tx, blob)); blob.append(std::string(sizeof(crypto::signature) / 2, 'x')); ASSERT_FALSE(serialization::parse_binary(blob, tx1)); @@ -406,6 +415,7 @@ TEST(Serialization, serializes_transacion_signatures_correctly) tx.vin.push_back(txin_to_key1); tx.signatures.resize(1); tx.signatures[0].resize(2); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // Too much signatures for two inputs @@ -413,24 +423,28 @@ TEST(Serialization, serializes_transacion_signatures_correctly) tx.signatures[0].resize(2); tx.signatures[1].resize(2); tx.signatures[2].resize(2); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // First signatures vector contains too little elements tx.signatures.resize(2); tx.signatures[0].resize(1); tx.signatures[1].resize(2); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // First signatures vector contains too much elements tx.signatures.resize(2); tx.signatures[0].resize(3); tx.signatures[1].resize(2); + tx.invalidate_hashes(); ASSERT_FALSE(serialization::dump_binary(tx, blob)); // There are signatures for each input tx.signatures.resize(2); tx.signatures[0].resize(2); tx.signatures[1].resize(2); + tx.invalidate_hashes(); ASSERT_TRUE(serialization::dump_binary(tx, blob)); ASSERT_TRUE(serialization::parse_binary(blob, tx1)); ASSERT_EQ(tx, tx1); From 558cfc31caeb88398fe5bab9292246739586374d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 22 Mar 2017 18:03:23 +0000 Subject: [PATCH 2/4] core, wallet: faster tx pool scanning Includes a new RPC to get tx pool hashes fast. --- src/cryptonote_core/cryptonote_core.cpp | 8 +- src/cryptonote_core/cryptonote_core.h | 7 + src/cryptonote_core/tx_pool.cpp | 7 + src/cryptonote_core/tx_pool.h | 7 + src/rpc/core_rpc_server.cpp | 8 + src/rpc/core_rpc_server.h | 2 + src/rpc/core_rpc_server_commands_defs.h | 22 ++- src/wallet/wallet2.cpp | 209 ++++++++++++------------ 8 files changed, 166 insertions(+), 104 deletions(-) diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index 1b20776a5..4d4b9d4d2 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1036,7 +1036,13 @@ namespace cryptonote m_mempool.get_transactions(txs); return true; } - //----------------------------------------------------------------------------------------------- + //----------------------------------------------------------------------------------------------- + bool core::get_pool_transaction_hashes(std::vector& txs) const + { + m_mempool.get_transaction_hashes(txs); + return true; + } + //----------------------------------------------------------------------------------------------- bool core::get_pool_transaction(const crypto::hash &id, transaction& tx) const { return m_mempool.get_transaction(id, tx); diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 02d58691d..24b0f0614 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -396,6 +396,13 @@ namespace cryptonote */ bool get_pool_transactions(std::list& txs) const; + /** + * @copydoc tx_memory_pool::get_transactions + * + * @note see tx_memory_pool::get_transactions + */ + bool get_pool_transaction_hashes(std::vector& txs) const; + /** * @copydoc tx_memory_pool::get_transaction * diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index ea19b6b48..b2a7120cb 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -423,6 +423,13 @@ namespace cryptonote txs.push_back(tx_vt.second.tx); } //------------------------------------------------------------------ + void tx_memory_pool::get_transaction_hashes(std::vector& txs) const + { + CRITICAL_REGION_LOCAL(m_transactions_lock); + for(const auto& tx_vt: m_transactions) + txs.push_back(get_transaction_hash(tx_vt.second.tx)); + } + //------------------------------------------------------------------ //TODO: investigate whether boolean return is appropriate bool tx_memory_pool::get_transactions_and_spent_keys_info(std::vector& tx_infos, std::vector& key_image_infos) const { diff --git a/src/cryptonote_core/tx_pool.h b/src/cryptonote_core/tx_pool.h index d71032943..d19f83b2e 100644 --- a/src/cryptonote_core/tx_pool.h +++ b/src/cryptonote_core/tx_pool.h @@ -233,6 +233,13 @@ namespace cryptonote */ void get_transactions(std::list& txs) const; + /** + * @brief get a list of all transaction hashes in the pool + * + * @param txs return-by-reference the list of transactions + */ + void get_transaction_hashes(std::vector& txs) const; + /** * @brief get information about all transactions and key images in the pool * diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index 9f7e8aa38..a800dbb35 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -789,6 +789,14 @@ namespace cryptonote return true; } //------------------------------------------------------------------------------------------------------------------------------ + bool core_rpc_server::on_get_transaction_pool_hashes(const COMMAND_RPC_GET_TRANSACTION_POOL_HASHES::request& req, COMMAND_RPC_GET_TRANSACTION_POOL_HASHES::response& res) + { + CHECK_CORE_BUSY(); + m_core.get_pool_transaction_hashes(res.tx_hashes); + res.status = CORE_RPC_STATUS_OK; + return true; + } + //------------------------------------------------------------------------------------------------------------------------------ bool core_rpc_server::on_stop_daemon(const COMMAND_RPC_STOP_DAEMON::request& req, COMMAND_RPC_STOP_DAEMON::response& res) { // FIXME: replace back to original m_p2p.send_stop_signal() after diff --git a/src/rpc/core_rpc_server.h b/src/rpc/core_rpc_server.h index 900f473a9..f8eff5f8c 100644 --- a/src/rpc/core_rpc_server.h +++ b/src/rpc/core_rpc_server.h @@ -92,6 +92,7 @@ namespace cryptonote MAP_URI_AUTO_JON2_IF("/set_log_level", on_set_log_level, COMMAND_RPC_SET_LOG_LEVEL, !m_restricted) MAP_URI_AUTO_JON2_IF("/set_log_categories", on_set_log_categories, COMMAND_RPC_SET_LOG_CATEGORIES, !m_restricted) MAP_URI_AUTO_JON2("/get_transaction_pool", on_get_transaction_pool, COMMAND_RPC_GET_TRANSACTION_POOL) + MAP_URI_AUTO_JON2("/get_transaction_pool_hashes.bin", on_get_transaction_pool_hashes, COMMAND_RPC_GET_TRANSACTION_POOL_HASHES) MAP_URI_AUTO_JON2_IF("/stop_daemon", on_stop_daemon, COMMAND_RPC_STOP_DAEMON, !m_restricted) MAP_URI_AUTO_JON2("/getinfo", on_get_info, COMMAND_RPC_GET_INFO) MAP_URI_AUTO_JON2_IF("/out_peers", on_out_peers, COMMAND_RPC_OUT_PEERS, !m_restricted) @@ -145,6 +146,7 @@ namespace cryptonote bool on_set_log_level(const COMMAND_RPC_SET_LOG_LEVEL::request& req, COMMAND_RPC_SET_LOG_LEVEL::response& res); bool on_set_log_categories(const COMMAND_RPC_SET_LOG_CATEGORIES::request& req, COMMAND_RPC_SET_LOG_CATEGORIES::response& res); bool on_get_transaction_pool(const COMMAND_RPC_GET_TRANSACTION_POOL::request& req, COMMAND_RPC_GET_TRANSACTION_POOL::response& res); + bool on_get_transaction_pool_hashes(const COMMAND_RPC_GET_TRANSACTION_POOL_HASHES::request& req, COMMAND_RPC_GET_TRANSACTION_POOL_HASHES::response& res); bool on_stop_daemon(const COMMAND_RPC_STOP_DAEMON::request& req, COMMAND_RPC_STOP_DAEMON::response& res); bool on_out_peers(const COMMAND_RPC_OUT_PEERS::request& req, COMMAND_RPC_OUT_PEERS::response& res); bool on_start_save_graph(const COMMAND_RPC_START_SAVE_GRAPH::request& req, COMMAND_RPC_START_SAVE_GRAPH::response& res); diff --git a/src/rpc/core_rpc_server_commands_defs.h b/src/rpc/core_rpc_server_commands_defs.h index 99aa9815e..6f60093ac 100644 --- a/src/rpc/core_rpc_server_commands_defs.h +++ b/src/rpc/core_rpc_server_commands_defs.h @@ -49,7 +49,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 1 -#define CORE_RPC_VERSION_MINOR 7 +#define CORE_RPC_VERSION_MINOR 8 #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) @@ -1025,6 +1025,26 @@ namespace cryptonote }; }; + struct COMMAND_RPC_GET_TRANSACTION_POOL_HASHES + { + struct request + { + BEGIN_KV_SERIALIZE_MAP() + END_KV_SERIALIZE_MAP() + }; + + struct response + { + std::string status; + std::vector tx_hashes; + + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(status) + KV_SERIALIZE_CONTAINER_POD_AS_BLOB(tx_hashes) + END_KV_SERIALIZE_MAP() + }; + }; + struct COMMAND_RPC_GET_CONNECTIONS { struct request diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index dce1ee9de..b3791b99c 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1384,13 +1384,13 @@ void wallet2::update_pool_state() MDEBUG("update_pool_state start"); // get the pool state - cryptonote::COMMAND_RPC_GET_TRANSACTION_POOL::request req; - cryptonote::COMMAND_RPC_GET_TRANSACTION_POOL::response res; + cryptonote::COMMAND_RPC_GET_TRANSACTION_POOL_HASHES::request req; + cryptonote::COMMAND_RPC_GET_TRANSACTION_POOL_HASHES::response res; m_daemon_rpc_mutex.lock(); - bool r = epee::net_utils::invoke_http_json("/get_transaction_pool", req, res, m_http_client, rpc_timeout); + bool r = epee::net_utils::invoke_http_json("/get_transaction_pool_hashes.bin", req, res, m_http_client, rpc_timeout); m_daemon_rpc_mutex.unlock(); - THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "get_transaction_pool"); - THROW_WALLET_EXCEPTION_IF(res.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "get_transaction_pool"); + THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "get_transaction_pool_hashes.bin"); + THROW_WALLET_EXCEPTION_IF(res.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "get_transaction_pool_hashes.bin"); THROW_WALLET_EXCEPTION_IF(res.status != CORE_RPC_STATUS_OK, error::get_tx_pool_error); MDEBUG("update_pool_state got pool"); @@ -1398,11 +1398,11 @@ void wallet2::update_pool_state() std::unordered_map::iterator it = m_unconfirmed_txs.begin(); while (it != m_unconfirmed_txs.end()) { - const std::string txid = epee::string_tools::pod_to_hex(it->first); + const crypto::hash &txid = it->first; bool found = false; - for (auto it2: res.transactions) + for (const auto &it2: res.tx_hashes) { - if (it2.id_hash == txid) + if (it2 == txid) { found = true; break; @@ -1453,11 +1453,11 @@ void wallet2::update_pool_state() std::unordered_map::iterator uit = m_unconfirmed_payments.begin(); while (uit != m_unconfirmed_payments.end()) { - const std::string txid = string_tools::pod_to_hex(uit->first); + const crypto::hash &txid = uit->first; bool found = false; - for (auto it2: res.transactions) + for (const auto &it2: res.tx_hashes) { - if (it2.id_hash == txid) + if (it2 == txid) { found = true; break; @@ -1466,114 +1466,119 @@ void wallet2::update_pool_state() auto pit = uit++; if (!found) { + MDEBUG("Removing " << txid << " from unconfirmed payments, not found in pool"); m_unconfirmed_payments.erase(pit); } } MDEBUG("update_pool_state done second loop"); - // add new pool txes to us - for (auto it: res.transactions) + // gather txids of new pool txes to us + std::vector txids; + for (const auto &txid: res.tx_hashes) { - cryptonote::blobdata txid_data; - if(epee::string_tools::parse_hexstr_to_binbuff(it.id_hash, txid_data) && txid_data.size() == sizeof(crypto::hash)) + if (m_scanned_pool_txs[0].find(txid) != m_scanned_pool_txs[0].end() || m_scanned_pool_txs[1].find(txid) != m_scanned_pool_txs[1].end()) { - const crypto::hash txid = *reinterpret_cast(txid_data.data()); - if (m_scanned_pool_txs[0].find(txid) != m_scanned_pool_txs[0].end() || m_scanned_pool_txs[1].find(txid) != m_scanned_pool_txs[1].end()) + LOG_PRINT_L2("Already seen " << txid << ", skipped"); + continue; + } + if (m_unconfirmed_payments.find(txid) == m_unconfirmed_payments.end()) + { + LOG_PRINT_L1("Found new pool tx: " << txid); + bool found = false; + for (const auto &i: m_unconfirmed_txs) { - LOG_PRINT_L2("Already seen " << txid << ", skipped"); - continue; + if (i.first == txid) + { + found = true; + break; + } } - if (m_unconfirmed_payments.find(txid) == m_unconfirmed_payments.end()) + if (!found) { - LOG_PRINT_L1("Found new pool tx: " << txid); - bool found = false; - for (const auto &i: m_unconfirmed_txs) - { - if (i.first == txid) - { - found = true; - break; - } - } - if (!found) - { - // not one of those we sent ourselves - cryptonote::COMMAND_RPC_GET_TRANSACTIONS::request req; - cryptonote::COMMAND_RPC_GET_TRANSACTIONS::response res; - req.txs_hashes.push_back(it.id_hash); - MDEBUG("asking for " << it.id_hash); - req.decode_as_json = false; - m_daemon_rpc_mutex.lock(); - bool r = epee::net_utils::invoke_http_json("/gettransactions", req, res, m_http_client, rpc_timeout); - MDEBUG("asked for " << it.id_hash << ", got " << r << " and " << res.status); - m_daemon_rpc_mutex.unlock(); - if (r && res.status == CORE_RPC_STATUS_OK) - { - if (res.txs.size() == 1) - { - // might have just been put in a block - if (res.txs[0].in_pool) - { - cryptonote::transaction tx; - cryptonote::blobdata bd; - crypto::hash tx_hash, tx_prefix_hash; - if (epee::string_tools::parse_hexstr_to_binbuff(res.txs[0].as_hex, bd)) - { - if (cryptonote::parse_and_validate_tx_from_blob(bd, tx, tx_hash, tx_prefix_hash)) - { - if (tx_hash == txid) - { - process_new_transaction(txid, tx, std::vector(), 0, time(NULL), false, true); - m_scanned_pool_txs[0].insert(txid); - if (m_scanned_pool_txs[0].size() > 5000) - { - std::swap(m_scanned_pool_txs[0], m_scanned_pool_txs[1]); - m_scanned_pool_txs[0].clear(); - } - } - else - { - LOG_PRINT_L0("Mismatched txids when processing unconfimed txes from pool"); - } - } - else - { - LOG_PRINT_L0("failed to validate transaction from daemon"); - } - } - else - { - LOG_PRINT_L0("Failed to parse tx " << txid); - } - } - else - { - LOG_PRINT_L1("Tx " << txid << " was in pool, but is no more"); - } - } - else - { - LOG_PRINT_L0("Expected 1 tx, got " << res.txs.size()); - } - } - else - { - LOG_PRINT_L0("Error calling gettransactions daemon RPC: r " << r << ", status " << res.status); - } - } - else - { - LOG_PRINT_L1("We sent that one"); - } + // not one of those we sent ourselves + txids.push_back(txid); } else { - LOG_PRINT_L1("Already saw that one, it's for us"); + LOG_PRINT_L1("We sent that one"); } } else { - LOG_PRINT_L0("Failed to parse txid"); + LOG_PRINT_L1("Already saw that one, it's for us"); + } + } + + // get those txes + if (!txids.empty()) + { + cryptonote::COMMAND_RPC_GET_TRANSACTIONS::request req; + cryptonote::COMMAND_RPC_GET_TRANSACTIONS::response res; + for (const auto &txid: txids) + req.txs_hashes.push_back(epee::string_tools::pod_to_hex(txid)); + MDEBUG("asking for " << txids.size() << " transactions"); + req.decode_as_json = false; + m_daemon_rpc_mutex.lock(); + bool r = epee::net_utils::invoke_http_json("/gettransactions", req, res, m_http_client, rpc_timeout); + m_daemon_rpc_mutex.unlock(); + MDEBUG("Got " << r << " and " << res.status); + if (r && res.status == CORE_RPC_STATUS_OK) + { + if (res.txs.size() == txids.size()) + { + size_t n = 0; + for (const auto &txid: txids) + { + // might have just been put in a block + if (res.txs[n].in_pool) + { + cryptonote::transaction tx; + cryptonote::blobdata bd; + crypto::hash tx_hash, tx_prefix_hash; + if (epee::string_tools::parse_hexstr_to_binbuff(res.txs[n].as_hex, bd)) + { + if (cryptonote::parse_and_validate_tx_from_blob(bd, tx, tx_hash, tx_prefix_hash)) + { + if (tx_hash == txid) + { + process_new_transaction(txid, tx, std::vector(), 0, time(NULL), false, true); + m_scanned_pool_txs[0].insert(txid); + if (m_scanned_pool_txs[0].size() > 5000) + { + std::swap(m_scanned_pool_txs[0], m_scanned_pool_txs[1]); + m_scanned_pool_txs[0].clear(); + } + } + else + { + LOG_PRINT_L0("Mismatched txids when processing unconfimed txes from pool"); + } + } + else + { + LOG_PRINT_L0("failed to validate transaction from daemon"); + } + } + else + { + LOG_PRINT_L0("Failed to parse tx " << txid); + } + } + else + { + LOG_PRINT_L1("Tx " << txid << " was in pool, but is no more"); + } + ++n; + } + } + else + { + LOG_PRINT_L0("Expected " << txids.size() << " tx(es), got " << res.txs.size()); + } + } + else + { + LOG_PRINT_L0("Error calling gettransactions daemon RPC: r " << r << ", status " << res.status); } } MDEBUG("update_pool_state end"); From aaeb164cf6fa5ff4c8631dd77e434c3259d6b14e Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 22 Mar 2017 20:21:54 +0000 Subject: [PATCH 3/4] tx_pool: remove transactions if they're in the blockchain When starting up, if the pool state was not saved, the pool might contain transactions which made it into the blockchain, so these need removing --- src/cryptonote_core/tx_pool.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index b2a7120cb..93fe57232 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -717,8 +717,16 @@ namespace cryptonote size_t n_removed = 0; size_t tx_size_limit = get_transaction_size_limit(version); for (auto it = m_transactions.begin(); it != m_transactions.end(); ) { + bool remove = false; if (it->second.blob_size >= tx_size_limit) { - LOG_PRINT_L1("Transaction " << get_transaction_hash(it->second.tx) << " is too big (" << it->second.blob_size << " bytes), removing it from pool"); + LOG_PRINT_L1("Transaction " << it->first << " is too big (" << it->second.blob_size << " bytes), removing it from pool"); + remove = true; + } + else if (m_blockchain.have_tx(it->first)) { + LOG_PRINT_L1("Transaction " << it->first << " is in the blockchain, removing it from pool"); + remove = true; + } + if (remove) { remove_transaction_keyimages(it->second.tx); auto sorted_it = find_tx_in_sorted_container(it->first); if (sorted_it == m_txs_by_fee_and_receive_time.end()) From 91d410902399befd81589fd0153e5b7994bcdaa2 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 22 Mar 2017 20:26:56 +0000 Subject: [PATCH 4/4] tx_pool: ensure txes loaded from poolstate.bin have their txid cached The txid is not saved, and we want to make sure the transactions have their txid cached while in the pool, since get_transactions copies the transaction object, so any txid calculation on those copies would not benefit any later caller, since the original tx would be left without a cached txid. --- src/cryptonote_core/tx_pool.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index 93fe57232..cde97b52f 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -718,20 +718,21 @@ namespace cryptonote size_t tx_size_limit = get_transaction_size_limit(version); for (auto it = m_transactions.begin(); it != m_transactions.end(); ) { bool remove = false; + const crypto::hash &txid = get_transaction_hash(it->second.tx); if (it->second.blob_size >= tx_size_limit) { - LOG_PRINT_L1("Transaction " << it->first << " is too big (" << it->second.blob_size << " bytes), removing it from pool"); + LOG_PRINT_L1("Transaction " << txid << " is too big (" << it->second.blob_size << " bytes), removing it from pool"); remove = true; } - else if (m_blockchain.have_tx(it->first)) { - LOG_PRINT_L1("Transaction " << it->first << " is in the blockchain, removing it from pool"); + else if (m_blockchain.have_tx(txid)) { + LOG_PRINT_L1("Transaction " << txid << " is in the blockchain, removing it from pool"); remove = true; } if (remove) { remove_transaction_keyimages(it->second.tx); - auto sorted_it = find_tx_in_sorted_container(it->first); + auto sorted_it = find_tx_in_sorted_container(txid); if (sorted_it == m_txs_by_fee_and_receive_time.end()) { - LOG_PRINT_L1("Removing tx " << it->first << " from tx pool, but it was not found in the sorted txs container!"); + LOG_PRINT_L1("Removing tx " << txid << " from tx pool, but it was not found in the sorted txs container!"); } else {