Skip to content

don't punish honest peer on reorg #442

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
auruya wants to merge 1 commit into
base: fcmp++-beta-stressnet
Choose a base branch
from
Open

don't punish honest peer on reorg #442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions src/cryptonote_core/blockchain.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,7 +1210,7 @@ bool Blockchain::rollback_blockchain_switching(std::list<block>& original_chain,
//------------------------------------------------------------------
// This function attempts to switch to an alternate chain, returning
// boolean based on success therein.
bool Blockchain::switch_to_alternative_blockchain(std::list<block_extended_info>& alt_chain, bool discard_disconnected_chain)
bool Blockchain::switch_to_alternative_blockchain(std::list<block_extended_info>& alt_chain, bool discard_disconnected_chain, block_verification_context& bvc)
{
LOG_PRINT_L3("Blockchain::" << __func__);
CRITICAL_REGION_LOCAL(m_blockchain_lock);
Expand All @@ -1219,12 +1219,18 @@ bool Blockchain::switch_to_alternative_blockchain(std::list<block_extended_info>
m_reset_timestamps_and_difficulties_height = true;

// if empty alt chain passed (not sure how that could happen), return false
CHECK_AND_ASSERT_MES(alt_chain.size(), false, "switch_to_alternative_blockchain: empty chain passed");
if (!alt_chain.size())
{
bvc.m_verifivation_failed = true;
MERROR("switch_to_alternative_blockchain: empty chain passed");
return false;
}

// verify that main chain has front of alt chain's parent block
if (!m_db->block_exists(alt_chain.front().bl.prev_id))
{
LOG_ERROR("Attempting to move to an alternate chain, but it doesn't appear to connect to the main chain!");
bvc.m_verifivation_failed = true;
return false;
}

Expand All @@ -1244,17 +1250,23 @@ bool Blockchain::switch_to_alternative_blockchain(std::list<block_extended_info>
for(auto alt_ch_iter = alt_chain.begin(); alt_ch_iter != alt_chain.end(); alt_ch_iter++)
{
const auto &bei = *alt_ch_iter;
block_verification_context bvc = {};
block_verification_context block_bvc = {};

// add block to main chain
bool r = handle_block_to_main_chain(bei.bl, bvc);

bool r = handle_block_to_main_chain(bei.bl, block_bvc);
// if adding block to main chain failed, rollback to previous state and
// return false
if(!r || !bvc.m_added_to_main_chain)
if(!r || !block_bvc.m_added_to_main_chain)
{
MERROR("Failed to switch to alternative blockchain");

// set bvc.m_missing_txs so that
// it'll re-requests the tx or re-fetches the alt chain
// instead of banning peer
bvc.m_verifivation_failed = true;
bvc.m_missing_txs = block_bvc.m_missing_txs;

// rollback_blockchain_switching should be moved to two different
// functions: rollback and apply_chain, but for now we pretend it is
// just the latter (because the rollback was done above).
Expand All @@ -1281,9 +1293,9 @@ bool Blockchain::switch_to_alternative_blockchain(std::list<block_extended_info>
//pushing old chain as alternative chain
for (auto& old_ch_ent : disconnected_chain)
{
block_verification_context bvc = {};
block_verification_context old_bvc = {};
pool_supplement ps{};
bool r = handle_alternative_block(old_ch_ent, get_block_hash(old_ch_ent), bvc, ps);
bool r = handle_alternative_block(old_ch_ent, get_block_hash(old_ch_ent), old_bvc, ps);
if(!r)
{
MERROR("Failed to push ex-main chain blocks to alternative chain ");
Expand Down Expand Up @@ -2286,23 +2298,19 @@ bool Blockchain::handle_alternative_block(const block& b, const crypto::hash& id
//do reorganize!
MGINFO_GREEN("###### REORGANIZE on height: " << alt_chain.front().height << " of " << m_db->height() - 1 << ", checkpoint is found in alternative chain on height " << bei.height);

bool r = switch_to_alternative_blockchain(alt_chain, true);
bool r = switch_to_alternative_blockchain(alt_chain, true, bvc);

if(r) bvc.m_added_to_main_chain = true;
else bvc.m_verifivation_failed = true;

return r;
}
else if(main_chain_cumulative_difficulty < bei.cumulative_difficulty) //check if difficulty bigger then in main chain
{
//do reorganize!
MGINFO_GREEN("###### REORGANIZE on height: " << alt_chain.front().height << " of " << m_db->height() - 1 << " with cum_difficulty " << m_db->get_block_cumulative_difficulty(m_db->height() - 1) << std::endl << " alternative blockchain size: " << alt_chain.size() << " with cum_difficulty " << bei.cumulative_difficulty);

bool r = switch_to_alternative_blockchain(alt_chain, false);
bool r = switch_to_alternative_blockchain(alt_chain, false, bvc);
if (r)
bvc.m_added_to_main_chain = true;
else
bvc.m_verifivation_failed = true;
return r;
}
else
Expand Down
3 changes: 2 additions & 1 deletion src/cryptonote_core/blockchain.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1432,10 +1432,11 @@ namespace cryptonote
*
* @param alt_chain the chain to switch to
* @param discard_disconnected_chain whether or not to keep the old chain as an alternate
* @param bvc
*
* @return false if the reorganization fails, otherwise true
*/
bool switch_to_alternative_blockchain(std::list<block_extended_info>& alt_chain, bool discard_disconnected_chain);
bool switch_to_alternative_blockchain(std::list<block_extended_info>& alt_chain, bool discard_disconnected_chain, block_verification_context& bvc);

/**
* @brief removes the most recent block from the blockchain
Expand Down
41 changes: 32 additions & 9 deletions src/cryptonote_protocol/cryptonote_protocol_handler.inl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,18 +752,41 @@ namespace cryptonote
need_tx_indices.push_back(tx_idx);
}

// Make request form
MDEBUG("We are missing " << need_tx_indices.size() << " txes for this fluffy block");
for (auto txidx: need_tx_indices)
MDEBUG(" tx " << new_block.tx_hashes[txidx]);
NOTIFY_REQUEST_FLUFFY_MISSING_TX::request missing_tx_req;
missing_tx_req.block_hash = new_block_hash;
missing_tx_req.current_blockchain_height = arg.current_blockchain_height;
missing_tx_req.missing_tx_indices = std::move(need_tx_indices);

// Post NOTIFY_REQUEST_FLUFFY_MISSING_TX request to peer
MLOG_P2P_MESSAGE("-->>NOTIFY_REQUEST_FLUFFY_MISSING_TX: missing_tx_indices.size()=" << missing_tx_req.missing_tx_indices.size() );
post_notify<NOTIFY_REQUEST_FLUFFY_MISSING_TX>(missing_tx_req, context);

// need_tx_indices only covers this block but we might have failed due to a missing tx
// from an ancestor block in case we just re-orged above and alt chain txs are dropped
// from mempool.
if (!need_tx_indices.empty()) {
NOTIFY_REQUEST_FLUFFY_MISSING_TX::request missing_tx_req;
missing_tx_req.block_hash = new_block_hash;
missing_tx_req.current_blockchain_height = arg.current_blockchain_height;
missing_tx_req.missing_tx_indices = std::move(need_tx_indices);

// Post NOTIFY_REQUEST_FLUFFY_MISSING_TX request to peer
MLOG_P2P_MESSAGE("-->>NOTIFY_REQUEST_FLUFFY_MISSING_TX: missing_tx_indices.size()=" << missing_tx_req.missing_tx_indices.size() );
post_notify<NOTIFY_REQUEST_FLUFFY_MISSING_TX>(missing_tx_req, context);
}
else
{
// we just re-orged and ancestor alt block's txs were removed from our pool
// so resync these blocks with its txs.
MDEBUG("Reorg for fluffy block " << new_block_hash << " needs txs we don't have for an "
"ancestor block; requesting chain to re-sync the alternative chain with its txs");
context.m_needed_objects.clear();
context.m_state = cryptonote_connection_context::state_synchronizing;
NOTIFY_REQUEST_CHAIN::request r = {};
m_core.get_short_chain_history(r.block_ids, context.m_expect_height);
handler_request_blocks_history( r.block_ids );
r.prune = m_sync_pruned_blocks;
context.m_last_request_time = boost::posix_time::microsec_clock::universal_time();
context.m_expect_response = NOTIFY_RESPONSE_CHAIN_ENTRY::ID;
MLOG_P2P_MESSAGE("-->>NOTIFY_REQUEST_CHAIN: m_block_ids.size()=" << r.block_ids.size() );
post_notify<NOTIFY_REQUEST_CHAIN>(r, context);
MLOG_PEER_STATE("requesting chain");
}
}
else // failure for some other reason besides missing txs...
{
Expand Down
Loading