From 1b83c67eb6371e816eba9239d6eceadc53f4476c Mon Sep 17 00:00:00 2001 From: Alexander Suprunenko Date: Mon, 27 May 2019 17:15:43 +0300 Subject: [PATCH] Disqualification transaction check is performed on tx_memory_pool::add_tx instead of on a new block. --- .../stake_transaction_processor.cpp | 120 ++++++++++++------ .../stake_transaction_processor.h | 4 + src/cryptonote_core/tx_pool.cpp | 9 +- 3 files changed, 90 insertions(+), 43 deletions(-) diff --git a/src/cryptonote_core/stake_transaction_processor.cpp b/src/cryptonote_core/stake_transaction_processor.cpp index c33db14df..f0c8c7063 100644 --- a/src/cryptonote_core/stake_transaction_processor.cpp +++ b/src/cryptonote_core/stake_transaction_processor.cpp @@ -185,44 +185,49 @@ Ids fromIndexes(const Tiers& bbl_tiers, const std::vector& idxs) } //namespace -void StakeTransactionProcessor::process_disqualification_transaction(const transaction& tx, const crypto::hash tx_hash, uint64_t block_index, const crypto::hash& block_hash, StakeTransactionStorage::disqualification_array& disquals) +bool StakeTransactionProcessor::check_disqualification_transaction(const transaction& tx, const crypto::hash tx_hash) { assert(tx.version == 123); + //TODO: find out if such disqualification transaction with tx_hash already exists + tx_extra_graft_disqualification disq_extra; - if(graft_check_disqualification(tx, &disq_extra)) + if(!graft_check_disqualification(tx, &disq_extra)) { - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash); - return; + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash); + return false; } + //TODO: it should be checked somewhere instead of this + /* if(block_index <= disq_extra.item.block_height) { - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash << "; invalid block_height " << disq_extra.item.block_height << " current " << block_index); - return; + return false; } + */ crypto::hash b_hash = m_blockchain.get_block_id_by_height(disq_extra.item.block_height); if(b_hash != disq_extra.item.block_hash) { - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash << "; invalid block_hash "); - return; + return false; } //get BBL size_t depth = m_blockchain_based_list->block_height() - disq_extra.item.block_height; if(m_blockchain_based_list->history_depth() <= depth) { - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash << "; out of history "); - return; + return false; } if(disq_extra.signers.size() < graft::generator::REQUIRED_BBQS_VOTES) { - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash << "; lack of signers "); - return; + return false; } auto& tiers = m_blockchain_based_list->tiers(depth); @@ -236,9 +241,9 @@ void StakeTransactionProcessor::process_disqualification_transaction(const trans if(std::none_of(qcl.begin(), qcl.end(), [&disq_extra](crypto::public_key& v)->bool { return v == disq_extra.item.id; } )) { std::string id_str = epee::string_tools::pod_to_hex(disq_extra.item.id); - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash << "; disqualified id " << id_str << " is not in QCL"); - return; + return false; } for(auto& si : disq_extra.signers) @@ -246,61 +251,58 @@ void StakeTransactionProcessor::process_disqualification_transaction(const trans if(std::none_of(bbqs.begin(), bbqs.end(), [&si](crypto::public_key& v)->bool { return v == si.signer_id; } )) { std::string id_str = epee::string_tools::pod_to_hex(si.signer_id); - MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification transaction, tx_hash=" << tx_hash << "; signer id " << id_str << " is not in BBQS"); - return; + return false; } } - disqualification disq; - serialization::dump_binary(disq_extra, disq.blob); - disq.block_index = block_index; - disq.id = disq_extra.item.id; - disq.id_str = epee::string_tools::pod_to_hex(disq.id); - - MDEBUG("New disqualification transaction found at block #" << block_index << ", tx_hash=" << tx_hash << ", supernode_id '" << disq.id_str << "'"); - - disquals.push_back(std::move(disq)); + return true; } -void StakeTransactionProcessor::process_disqualification2_transaction(const transaction& tx, const crypto::hash tx_hash, uint64_t block_index, const crypto::hash& block_hash, StakeTransactionStorage::disqualification2_storage_array& disquals2) +bool StakeTransactionProcessor::check_disqualification2_transaction(const transaction& tx, const crypto::hash tx_hash) { assert(tx.version == 124); + //TODO: find out if such disqualification transaction with tx_hash already exists + tx_extra_graft_disqualification2 disq_extra; - if(graft_check_disqualification2(tx, &disq_extra)) + if(!graft_check_disqualification2(tx, &disq_extra)) { - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash); - return; + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash); + return false; } + //TODO: it should be checked somewhere instead of this +/* if(block_index <= disq_extra.item.block_height) { - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash << "; invalid block_height " << disq_extra.item.block_height << " current " << block_index); - return; + return false; } +*/ crypto::hash b_hash = m_blockchain.get_block_id_by_height(disq_extra.item.block_height); if(b_hash != disq_extra.item.block_hash) { - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash << "; invalid block_hash "); - return; + return false; } //get BBL size_t depth = m_blockchain_based_list->block_height() - disq_extra.item.block_height; if(m_blockchain_based_list->history_depth() <= depth) { - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash << "; out of history "); - return; + return false; } if(disq_extra.signers.size() < graft::generator::REQUIRED_DISQUAL2_VOTES) { - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash << "; lack of signers "); - return; + return false; } auto& tiers = m_blockchain_based_list->tiers(depth); @@ -315,9 +317,9 @@ void StakeTransactionProcessor::process_disqualification2_transaction(const tran if(std::none_of(auths.begin(), auths.end(), [&id](crypto::public_key& v)->bool { return v == id; } )) { std::string id_str = epee::string_tools::pod_to_hex(id); - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash << "; disqualified id " << id_str << " is not in the auth sample"); - return; + return false; } } @@ -326,12 +328,48 @@ void StakeTransactionProcessor::process_disqualification2_transaction(const tran if(std::none_of(auths.begin(), auths.end(), [&si](crypto::public_key& v)->bool { return v == si.signer_id; } )) { std::string id_str = epee::string_tools::pod_to_hex(si.signer_id); - MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash + MWARNING("Ignore invalid disqualification2 transaction, tx_hash=" << tx_hash << "; signer id " << id_str << " is not in the auth sample"); - return; + return false; } } + return true; +} + +void StakeTransactionProcessor::process_disqualification_transaction(const transaction& tx, const crypto::hash tx_hash, uint64_t block_index, const crypto::hash& block_hash, StakeTransactionStorage::disqualification_array& disquals) +{ + assert(tx.version == 123); + + tx_extra_graft_disqualification disq_extra; + if(!graft_get_disqualification(tx, disq_extra)) + { + MWARNING("Ignore invalid disqualification transaction at block #" << block_index << ", tx_hash=" << tx_hash); + return; + } + + disqualification disq; + serialization::dump_binary(disq_extra, disq.blob); + disq.block_index = block_index; + disq.id = disq_extra.item.id; + disq.id_str = epee::string_tools::pod_to_hex(disq.id); + + MDEBUG("New disqualification transaction found at block #" << block_index << ", tx_hash=" << tx_hash << ", supernode_id '" << disq.id_str << "'"); + + disquals.push_back(std::move(disq)); +} + +void StakeTransactionProcessor::process_disqualification2_transaction(const transaction& tx, const crypto::hash tx_hash, uint64_t block_index, const crypto::hash& block_hash, StakeTransactionStorage::disqualification2_storage_array& disquals2) +{ + assert(tx.version == 124); + + tx_extra_graft_disqualification2 disq_extra; + if(!graft_get_disqualification2(tx, disq_extra)) + { + MWARNING("Ignore invalid disqualification2 transaction at block #" << block_index << ", tx_hash=" << tx_hash); + return; + } + disqualification2_storage_item disq; serialization::dump_binary(disq_extra, disq.blob); disq.block_index = block_index; diff --git a/src/cryptonote_core/stake_transaction_processor.h b/src/cryptonote_core/stake_transaction_processor.h index 0ec5f5a7c..44519c1e1 100644 --- a/src/cryptonote_core/stake_transaction_processor.h +++ b/src/cryptonote_core/stake_transaction_processor.h @@ -44,6 +44,10 @@ class StakeTransactionProcessor /// Force invoke update handler for blockchain based list void invoke_update_blockchain_based_list_handler(bool force = true, size_t depth = 1); + /// Check that transaction with tx_hash does not exist yet. Those to be disqualified, and signers are in corresponding sets. + bool check_disqualification_transaction(const transaction& tx, const crypto::hash tx_hash); + bool check_disqualification2_transaction(const transaction& tx, const crypto::hash tx_hash); + private: void init_storages_impl(); void process_block(uint64_t block_index, const block& block, const crypto::hash& block_hash, bool update_storage = true); diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index b88101f95..4550262b0 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -193,7 +193,12 @@ namespace cryptonote bool is_rta_tx = tx.type == transaction::tx_type_rta; if(is_disqualification_tx) { - if(!graft_check_disqualification(tx)) + //kept_by_block means that we already have checked the transaction once, and the check should pass. + //But in case we pop more than one block, and as such pop others stake and disqualification transactions in the popped blocks, + //it is possible that check of disqualification transaction will not pass again because of the history change, + //Note, that history change means a change of BBL as a function of block_height and previous stakes and disqualifications. + //Currently the fact is ignored, if we had checked once I assume the disqualifications will be valid, even if cannot be checked against previous history. + if(!kept_by_block && !m_stp->check_disqualification_transaction(tx, id)) { MERROR("Invalid disqualification transaction with id " << id); tvc.m_verifivation_failed = true; @@ -202,7 +207,7 @@ namespace cryptonote } else if (is_disqualification2_tx) { - if(!graft_check_disqualification2(tx)) + if(!kept_by_block && !m_stp->check_disqualification2_transaction(tx, id)) { MERROR("Invalid disqualification2 transaction with id " << id); tvc.m_verifivation_failed = true;