tx_pool: avoid parsing a whole tx if only the prefix is needed

This commit is contained in:
moneromooo-monero 2018-09-16 18:30:39 +00:00
parent bb30a72367
commit 6844ae1b8d
No known key found for this signature in database
GPG Key ID: 686F07454D6CEFC3
5 changed files with 60 additions and 38 deletions

View File

@ -199,6 +199,16 @@ namespace cryptonote
return true; return true;
} }
//--------------------------------------------------------------- //---------------------------------------------------------------
bool parse_and_validate_tx_prefix_from_blob(const blobdata& tx_blob, transaction_prefix& tx)
{
std::stringstream ss;
ss << tx_blob;
binary_archive<false> ba(ss);
bool r = ::serialization::serialize_noeof(ba, tx);
CHECK_AND_ASSERT_MES(r, false, "Failed to parse transaction prefix from blob");
return true;
}
//---------------------------------------------------------------
bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash) bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash)
{ {
std::stringstream ss; std::stringstream ss;

View File

@ -48,6 +48,7 @@ namespace cryptonote
//--------------------------------------------------------------- //---------------------------------------------------------------
void get_transaction_prefix_hash(const transaction_prefix& tx, crypto::hash& h); void get_transaction_prefix_hash(const transaction_prefix& tx, crypto::hash& h);
crypto::hash get_transaction_prefix_hash(const transaction_prefix& tx); crypto::hash get_transaction_prefix_hash(const transaction_prefix& tx);
bool parse_and_validate_tx_prefix_from_blob(const blobdata& tx_blob, transaction_prefix& tx);
bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash); bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx, crypto::hash& tx_hash, crypto::hash& tx_prefix_hash);
bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx); bool parse_and_validate_tx_from_blob(const blobdata& tx_blob, transaction& tx);
bool parse_and_validate_tx_base_from_blob(const blobdata& tx_blob, transaction& tx); bool parse_and_validate_tx_base_from_blob(const blobdata& tx_blob, transaction& tx);

View File

@ -250,7 +250,7 @@ namespace cryptonote
CRITICAL_REGION_LOCAL1(m_blockchain); CRITICAL_REGION_LOCAL1(m_blockchain);
LockedTXN lock(m_blockchain); LockedTXN lock(m_blockchain);
m_blockchain.add_txpool_tx(tx, meta); m_blockchain.add_txpool_tx(tx, meta);
if (!insert_key_images(tx, kept_by_block)) if (!insert_key_images(tx, id, kept_by_block))
return false; return false;
m_txs_by_fee_and_receive_time.emplace(std::pair<double, std::time_t>(fee / (double)tx_weight, receive_time), id); m_txs_by_fee_and_receive_time.emplace(std::pair<double, std::time_t>(fee / (double)tx_weight, receive_time), id);
} }
@ -290,9 +290,10 @@ namespace cryptonote
{ {
CRITICAL_REGION_LOCAL1(m_blockchain); CRITICAL_REGION_LOCAL1(m_blockchain);
LockedTXN lock(m_blockchain); LockedTXN lock(m_blockchain);
m_blockchain.remove_txpool_tx(get_transaction_hash(tx)); const crypto::hash txid = get_transaction_hash(tx);
m_blockchain.remove_txpool_tx(txid);
m_blockchain.add_txpool_tx(tx, meta); m_blockchain.add_txpool_tx(tx, meta);
if (!insert_key_images(tx, kept_by_block)) if (!insert_key_images(tx, txid, kept_by_block))
return false; return false;
m_txs_by_fee_and_receive_time.emplace(std::pair<double, std::time_t>(fee / (double)tx_weight, receive_time), id); m_txs_by_fee_and_receive_time.emplace(std::pair<double, std::time_t>(fee / (double)tx_weight, receive_time), id);
} }
@ -371,8 +372,8 @@ namespace cryptonote
continue; continue;
} }
cryptonote::blobdata txblob = m_blockchain.get_txpool_tx_blob(txid); cryptonote::blobdata txblob = m_blockchain.get_txpool_tx_blob(txid);
cryptonote::transaction tx; cryptonote::transaction_prefix tx;
if (!parse_and_validate_tx_from_blob(txblob, tx)) if (!parse_and_validate_tx_prefix_from_blob(txblob, tx))
{ {
MERROR("Failed to parse tx from txpool"); MERROR("Failed to parse tx from txpool");
return; return;
@ -381,7 +382,7 @@ namespace cryptonote
MINFO("Pruning tx " << txid << " from txpool: weight: " << it->first.second << ", fee/byte: " << it->first.first); MINFO("Pruning tx " << txid << " from txpool: weight: " << it->first.second << ", fee/byte: " << it->first.first);
m_blockchain.remove_txpool_tx(txid); m_blockchain.remove_txpool_tx(txid);
m_txpool_weight -= it->first.second; m_txpool_weight -= it->first.second;
remove_transaction_keyimages(tx); remove_transaction_keyimages(tx, txid);
MINFO("Pruned tx " << txid << " from txpool: weight: " << it->first.second << ", fee/byte: " << it->first.first); MINFO("Pruned tx " << txid << " from txpool: weight: " << it->first.second << ", fee/byte: " << it->first.first);
m_txs_by_fee_and_receive_time.erase(it--); m_txs_by_fee_and_receive_time.erase(it--);
changed = true; changed = true;
@ -398,11 +399,10 @@ namespace cryptonote
MINFO("Pool weight after pruning is larger than limit: " << m_txpool_weight << "/" << bytes); MINFO("Pool weight after pruning is larger than limit: " << m_txpool_weight << "/" << bytes);
} }
//--------------------------------------------------------------------------------- //---------------------------------------------------------------------------------
bool tx_memory_pool::insert_key_images(const transaction &tx, bool kept_by_block) bool tx_memory_pool::insert_key_images(const transaction_prefix &tx, const crypto::hash &id, bool kept_by_block)
{ {
for(const auto& in: tx.vin) for(const auto& in: tx.vin)
{ {
const crypto::hash id = get_transaction_hash(tx);
CHECKED_GET_SPECIFIC_VARIANT(in, const txin_to_key, txin, false); CHECKED_GET_SPECIFIC_VARIANT(in, const txin_to_key, txin, false);
std::unordered_set<crypto::hash>& kei_image_set = m_spent_key_images[txin.k_image]; std::unordered_set<crypto::hash>& kei_image_set = m_spent_key_images[txin.k_image];
CHECK_AND_ASSERT_MES(kept_by_block || kei_image_set.size() == 0, false, "internal error: kept_by_block=" << kept_by_block CHECK_AND_ASSERT_MES(kept_by_block || kei_image_set.size() == 0, false, "internal error: kept_by_block=" << kept_by_block
@ -418,19 +418,17 @@ namespace cryptonote
//FIXME: Can return early before removal of all of the key images. //FIXME: Can return early before removal of all of the key images.
// At the least, need to make sure that a false return here // At the least, need to make sure that a false return here
// is treated properly. Should probably not return early, however. // is treated properly. Should probably not return early, however.
bool tx_memory_pool::remove_transaction_keyimages(const transaction& tx) bool tx_memory_pool::remove_transaction_keyimages(const transaction_prefix& tx, const crypto::hash &actual_hash)
{ {
CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL(m_transactions_lock);
CRITICAL_REGION_LOCAL1(m_blockchain); CRITICAL_REGION_LOCAL1(m_blockchain);
// ND: Speedup // ND: Speedup
// 1. Move transaction hash calcuation outside of loop. ._.
crypto::hash actual_hash = get_transaction_hash(tx);
for(const txin_v& vi: tx.vin) for(const txin_v& vi: tx.vin)
{ {
CHECKED_GET_SPECIFIC_VARIANT(vi, const txin_to_key, txin, false); CHECKED_GET_SPECIFIC_VARIANT(vi, const txin_to_key, txin, false);
auto it = m_spent_key_images.find(txin.k_image); auto it = m_spent_key_images.find(txin.k_image);
CHECK_AND_ASSERT_MES(it != m_spent_key_images.end(), false, "failed to find transaction input in key images. img=" << txin.k_image << ENDL CHECK_AND_ASSERT_MES(it != m_spent_key_images.end(), false, "failed to find transaction input in key images. img=" << txin.k_image << ENDL
<< "transaction id = " << get_transaction_hash(tx)); << "transaction id = " << actual_hash);
std::unordered_set<crypto::hash>& key_image_set = it->second; std::unordered_set<crypto::hash>& key_image_set = it->second;
CHECK_AND_ASSERT_MES(key_image_set.size(), false, "empty key_image set, img=" << txin.k_image << ENDL CHECK_AND_ASSERT_MES(key_image_set.size(), false, "empty key_image set, img=" << txin.k_image << ENDL
<< "transaction id = " << actual_hash); << "transaction id = " << actual_hash);
@ -483,7 +481,7 @@ namespace cryptonote
// remove first, in case this throws, so key images aren't removed // remove first, in case this throws, so key images aren't removed
m_blockchain.remove_txpool_tx(id); m_blockchain.remove_txpool_tx(id);
m_txpool_weight -= tx_weight; m_txpool_weight -= tx_weight;
remove_transaction_keyimages(tx); remove_transaction_keyimages(tx, id);
} }
catch (const std::exception &e) catch (const std::exception &e)
{ {
@ -515,7 +513,7 @@ namespace cryptonote
{ {
CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL(m_transactions_lock);
CRITICAL_REGION_LOCAL1(m_blockchain); CRITICAL_REGION_LOCAL1(m_blockchain);
std::unordered_set<crypto::hash> remove; std::list<std::pair<crypto::hash, uint64_t>> remove;
m_blockchain.for_all_txpool_txes([this, &remove](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata*) { m_blockchain.for_all_txpool_txes([this, &remove](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata*) {
uint64_t tx_age = time(nullptr) - meta.receive_time; uint64_t tx_age = time(nullptr) - meta.receive_time;
@ -533,7 +531,7 @@ namespace cryptonote
m_txs_by_fee_and_receive_time.erase(sorted_it); m_txs_by_fee_and_receive_time.erase(sorted_it);
} }
m_timed_out_transactions.insert(txid); m_timed_out_transactions.insert(txid);
remove.insert(txid); remove.push_back(std::make_pair(txid, meta.weight));
} }
return true; return true;
}, false); }, false);
@ -541,13 +539,14 @@ namespace cryptonote
if (!remove.empty()) if (!remove.empty())
{ {
LockedTXN lock(m_blockchain); LockedTXN lock(m_blockchain);
for (const crypto::hash &txid: remove) for (const std::pair<crypto::hash, uint64_t> &entry: remove)
{ {
const crypto::hash &txid = entry.first;
try try
{ {
cryptonote::blobdata bd = m_blockchain.get_txpool_tx_blob(txid); cryptonote::blobdata bd = m_blockchain.get_txpool_tx_blob(txid);
cryptonote::transaction tx; cryptonote::transaction_prefix tx;
if (!parse_and_validate_tx_from_blob(bd, tx)) if (!parse_and_validate_tx_prefix_from_blob(bd, tx))
{ {
MERROR("Failed to parse tx from txpool"); MERROR("Failed to parse tx from txpool");
// continue // continue
@ -556,8 +555,8 @@ namespace cryptonote
{ {
// remove first, so we only remove key images if the tx removal succeeds // remove first, so we only remove key images if the tx removal succeeds
m_blockchain.remove_txpool_tx(txid); m_blockchain.remove_txpool_tx(txid);
m_txpool_weight -= get_transaction_weight(tx, bd.size()); m_txpool_weight -= entry.second;
remove_transaction_keyimages(tx); remove_transaction_keyimages(tx, txid);
} }
} }
catch (const std::exception &e) catch (const std::exception &e)
@ -1041,7 +1040,7 @@ namespace cryptonote
return true; return true;
} }
//--------------------------------------------------------------------------------- //---------------------------------------------------------------------------------
bool tx_memory_pool::have_key_images(const std::unordered_set<crypto::key_image>& k_images, const transaction& tx) bool tx_memory_pool::have_key_images(const std::unordered_set<crypto::key_image>& k_images, const transaction_prefix& tx)
{ {
for(size_t i = 0; i!= tx.vin.size(); i++) for(size_t i = 0; i!= tx.vin.size(); i++)
{ {
@ -1052,7 +1051,7 @@ namespace cryptonote
return false; return false;
} }
//--------------------------------------------------------------------------------- //---------------------------------------------------------------------------------
bool tx_memory_pool::append_key_images(std::unordered_set<crypto::key_image>& k_images, const transaction& tx) bool tx_memory_pool::append_key_images(std::unordered_set<crypto::key_image>& k_images, const transaction_prefix& tx)
{ {
for(size_t i = 0; i!= tx.vin.size(); i++) for(size_t i = 0; i!= tx.vin.size(); i++)
{ {
@ -1301,7 +1300,7 @@ namespace cryptonote
// remove tx from db first // remove tx from db first
m_blockchain.remove_txpool_tx(txid); m_blockchain.remove_txpool_tx(txid);
m_txpool_weight -= get_transaction_weight(tx, txblob.size()); m_txpool_weight -= get_transaction_weight(tx, txblob.size());
remove_transaction_keyimages(tx); remove_transaction_keyimages(tx, txid);
auto sorted_it = find_tx_in_sorted_container(txid); auto sorted_it = find_tx_in_sorted_container(txid);
if (sorted_it == m_txs_by_fee_and_receive_time.end()) if (sorted_it == m_txs_by_fee_and_receive_time.end())
{ {
@ -1344,13 +1343,13 @@ namespace cryptonote
bool r = m_blockchain.for_all_txpool_txes([this, &remove, kept](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata *bd) { bool r = m_blockchain.for_all_txpool_txes([this, &remove, kept](const crypto::hash &txid, const txpool_tx_meta_t &meta, const cryptonote::blobdata *bd) {
if (!!kept != !!meta.kept_by_block) if (!!kept != !!meta.kept_by_block)
return true; return true;
cryptonote::transaction tx; cryptonote::transaction_prefix tx;
if (!parse_and_validate_tx_from_blob(*bd, tx)) if (!parse_and_validate_tx_prefix_from_blob(*bd, tx))
{ {
MWARNING("Failed to parse tx from txpool, removing"); MWARNING("Failed to parse tx from txpool, removing");
remove.push_back(txid); remove.push_back(txid);
} }
if (!insert_key_images(tx, meta.kept_by_block)) if (!insert_key_images(tx, txid, meta.kept_by_block))
{ {
MFATAL("Failed to insert key images from txpool tx"); MFATAL("Failed to insert key images from txpool tx");
return false; return false;

View File

@ -434,7 +434,7 @@ namespace cryptonote
* *
* @return true on success, false on error * @return true on success, false on error
*/ */
bool insert_key_images(const transaction &tx, bool kept_by_block); bool insert_key_images(const transaction_prefix &tx, const crypto::hash &txid, bool kept_by_block);
/** /**
* @brief remove old transactions from the pool * @brief remove old transactions from the pool
@ -478,10 +478,11 @@ namespace cryptonote
* a transaction from the pool. * a transaction from the pool.
* *
* @param tx the transaction * @param tx the transaction
* @param txid the transaction's hash
* *
* @return false if any key images to be removed cannot be found, otherwise true * @return false if any key images to be removed cannot be found, otherwise true
*/ */
bool remove_transaction_keyimages(const transaction& tx); bool remove_transaction_keyimages(const transaction_prefix& tx, const crypto::hash &txid);
/** /**
* @brief check if any of a transaction's spent key images are present in a given set * @brief check if any of a transaction's spent key images are present in a given set
@ -491,7 +492,7 @@ namespace cryptonote
* *
* @return true if any key images present in the set, otherwise false * @return true if any key images present in the set, otherwise false
*/ */
static bool have_key_images(const std::unordered_set<crypto::key_image>& kic, const transaction& tx); static bool have_key_images(const std::unordered_set<crypto::key_image>& kic, const transaction_prefix& tx);
/** /**
* @brief append the key images from a transaction to the given set * @brief append the key images from a transaction to the given set
@ -501,7 +502,7 @@ namespace cryptonote
* *
* @return false if any append fails, otherwise true * @return false if any append fails, otherwise true
*/ */
static bool append_key_images(std::unordered_set<crypto::key_image>& kic, const transaction& tx); static bool append_key_images(std::unordered_set<crypto::key_image>& kic, const transaction_prefix& tx);
/** /**
* @brief check if a transaction is a valid candidate for inclusion in a block * @brief check if a transaction is a valid candidate for inclusion in a block
@ -509,11 +510,11 @@ namespace cryptonote
* @param txd the transaction to check (and info about it) * @param txd the transaction to check (and info about it)
* @param txid the txid of the transaction to check * @param txid the txid of the transaction to check
* @param txblob the transaction blob to check * @param txblob the transaction blob to check
* @param tx the parsed transaction, if successful * @param tx the parsed transaction prefix, if successful
* *
* @return true if the transaction is good to go, otherwise false * @return true if the transaction is good to go, otherwise false
*/ */
bool is_transaction_ready_to_go(txpool_tx_meta_t& txd, const crypto::hash &txid, const cryptonote::blobdata &txblob, transaction &tx) const; bool is_transaction_ready_to_go(txpool_tx_meta_t& txd, const crypto::hash &txid, const cryptonote::blobdata &txblob, transaction&tx) const;
/** /**
* @brief mark all transactions double spending the one passed * @brief mark all transactions double spending the one passed

View File

@ -318,7 +318,7 @@ namespace serialization {
* \brief self explanatory * \brief self explanatory
*/ */
template<class Stream> template<class Stream>
bool do_check_stream_state(Stream& s, boost::mpl::bool_<true>) bool do_check_stream_state(Stream& s, boost::mpl::bool_<true>, bool noeof)
{ {
return s.good(); return s.good();
} }
@ -329,13 +329,13 @@ namespace serialization {
* \detailed Also checks to make sure that the stream is not at EOF * \detailed Also checks to make sure that the stream is not at EOF
*/ */
template<class Stream> template<class Stream>
bool do_check_stream_state(Stream& s, boost::mpl::bool_<false>) bool do_check_stream_state(Stream& s, boost::mpl::bool_<false>, bool noeof)
{ {
bool result = false; bool result = false;
if (s.good()) if (s.good())
{ {
std::ios_base::iostate state = s.rdstate(); std::ios_base::iostate state = s.rdstate();
result = EOF == s.peek(); result = noeof || EOF == s.peek();
s.clear(state); s.clear(state);
} }
return result; return result;
@ -347,9 +347,9 @@ namespace serialization {
* \brief calls detail::do_check_stream_state for ar * \brief calls detail::do_check_stream_state for ar
*/ */
template<class Archive> template<class Archive>
bool check_stream_state(Archive& ar) bool check_stream_state(Archive& ar, bool noeof = false)
{ {
return detail::do_check_stream_state(ar.stream(), typename Archive::is_saving()); return detail::do_check_stream_state(ar.stream(), typename Archive::is_saving(), noeof);
} }
/*! \fn serialize /*! \fn serialize
@ -360,6 +360,17 @@ namespace serialization {
inline bool serialize(Archive &ar, T &v) inline bool serialize(Archive &ar, T &v)
{ {
bool r = do_serialize(ar, v); bool r = do_serialize(ar, v);
return r && check_stream_state(ar); return r && check_stream_state(ar, false);
}
/*! \fn serialize
*
* \brief serializes \a v into \a ar
*/
template <class Archive, class T>
inline bool serialize_noeof(Archive &ar, T &v)
{
bool r = do_serialize(ar, v);
return r && check_stream_state(ar, true);
} }
} }