From 09a5da17c6d96b6ac2131e73f81b48570835bd5c Mon Sep 17 00:00:00 2001 From: mfrankovi Date: Wed, 12 Apr 2023 15:32:12 +0200 Subject: [PATCH] chore: improve sync-gossip transition --- .../consensus/src/pbft/pbft_manager.cpp | 19 ++++++++++++++----- .../get_pbft_sync_packet_handler.hpp | 2 +- .../get_pbft_sync_packet_handler.cpp | 8 ++++++-- .../packets_handlers/vote_packet_handler.cpp | 10 +++++----- 4 files changed, 26 insertions(+), 13 deletions(-) diff --git a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp index 6bd9270dc3..e02c55c8fd 100644 --- a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp +++ b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp @@ -301,6 +301,11 @@ bool PbftManager::tryPushCertVotesBlock() { auto pbft_block = getValidPbftProposedBlock(current_pbft_period, certified_block_hash); if (!pbft_block) { LOG(log_er_) << "Invalid certified block " << certified_block_hash; + auto net = network_.lock(); + // If block/reward votes are missing but block is cert voted other nodes probably advanced, sync + if (net) { + net->restartSyncingPbft(); + } return false; } @@ -664,12 +669,16 @@ bool PbftManager::stateOperations_() { // Process synced blocks pushSyncedPbftBlocksIntoChain(); - // (Re)broadcast votes if needed - broadcastVotes(); + auto net = network_.lock(); + // Only broadcast votes and try to push cert voted block if node is not syncing + if (net && !net->pbft_syncing()) { + // (Re)broadcast votes if needed + broadcastVotes(); - // Check if there is 2t+1 cert votes for some valid block, if so - push it into the chain - if (tryPushCertVotesBlock()) { - return true; + // Check if there is 2t+1 cert votes for some valid block, if so - push it into the chain + if (tryPushCertVotesBlock()) { + return true; + } } // Check if there is 2t+1 next votes for some valid block, if so - advance round diff --git a/libraries/core_libs/network/include/network/tarcap/packets_handlers/get_pbft_sync_packet_handler.hpp b/libraries/core_libs/network/include/network/tarcap/packets_handlers/get_pbft_sync_packet_handler.hpp index 856bc52eca..bccf58638f 100644 --- a/libraries/core_libs/network/include/network/tarcap/packets_handlers/get_pbft_sync_packet_handler.hpp +++ b/libraries/core_libs/network/include/network/tarcap/packets_handlers/get_pbft_sync_packet_handler.hpp @@ -20,7 +20,7 @@ class GetPbftSyncPacketHandler final : public PacketHandler { std::shared_ptr vote_mgr, std::shared_ptr db, const addr_t& node_addr); - void sendPbftBlocks(dev::p2p::NodeID const& peer_id, PbftPeriod from_period, size_t blocks_to_transfer, + void sendPbftBlocks(const std::shared_ptr& peer, PbftPeriod from_period, size_t blocks_to_transfer, bool pbft_chain_synced); // Packet type that is processed by this handler diff --git a/libraries/core_libs/network/src/tarcap/packets_handlers/get_pbft_sync_packet_handler.cpp b/libraries/core_libs/network/src/tarcap/packets_handlers/get_pbft_sync_packet_handler.cpp index c5b0113f89..7d569f08ca 100644 --- a/libraries/core_libs/network/src/tarcap/packets_handlers/get_pbft_sync_packet_handler.cpp +++ b/libraries/core_libs/network/src/tarcap/packets_handlers/get_pbft_sync_packet_handler.cpp @@ -59,12 +59,13 @@ void GetPbftSyncPacketHandler::process(const PacketData &packet_data, } LOG(log_tr_) << "Will send " << blocks_to_transfer << " PBFT blocks to " << packet_data.from_node_id_; - sendPbftBlocks(packet_data.from_node_id_, height_to_sync, blocks_to_transfer, pbft_chain_synced); + sendPbftBlocks(peer, height_to_sync, blocks_to_transfer, pbft_chain_synced); } // api for pbft syncing -void GetPbftSyncPacketHandler::sendPbftBlocks(dev::p2p::NodeID const &peer_id, PbftPeriod from_period, +void GetPbftSyncPacketHandler::sendPbftBlocks(const std::shared_ptr &peer, PbftPeriod from_period, size_t blocks_to_transfer, bool pbft_chain_synced) { + const auto &peer_id = peer->getId(); LOG(log_tr_) << "sendPbftBlocks: peer want to sync from pbft chain height " << from_period << ", will send at most " << blocks_to_transfer << " pbft blocks to " << peer_id; @@ -95,6 +96,9 @@ void GetPbftSyncPacketHandler::sendPbftBlocks(dev::p2p::NodeID const &peer_id, P } LOG(log_dg_) << "Sending PbftSyncPacket period " << block_period << " to " << peer_id; sealAndSend(peer_id, SubprotocolPacketType::PbftSyncPacket, std::move(s)); + if (pbft_chain_synced && last_block) { + peer->syncing_ = false; + } } } diff --git a/libraries/core_libs/network/src/tarcap/packets_handlers/vote_packet_handler.cpp b/libraries/core_libs/network/src/tarcap/packets_handlers/vote_packet_handler.cpp index 3051d87122..5810b0518a 100644 --- a/libraries/core_libs/network/src/tarcap/packets_handlers/vote_packet_handler.cpp +++ b/libraries/core_libs/network/src/tarcap/packets_handlers/vote_packet_handler.cpp @@ -40,6 +40,11 @@ void VotePacketHandler::process(const PacketData &packet_data, const std::shared LOG(log_dg_) << "Received PBFT vote " << vote->getHash(); } + // Update peer's max chain size + if (peer_chain_size.has_value() && *peer_chain_size > peer->pbft_chain_size_) { + peer->pbft_chain_size_ = *peer_chain_size; + } + const auto vote_hash = vote->getHash(); if (!isPbftRelevantVote(vote)) { @@ -74,11 +79,6 @@ void VotePacketHandler::process(const PacketData &packet_data, const std::shared // Do not mark it before, as peers have small caches of known votes. Only mark gossiping votes peer->markVoteAsKnown(vote_hash); onNewPbftVote(vote, pbft_block); - - // Update peer's max chain size - if (peer_chain_size.has_value() && vote->getVoter() == peer->getId() && *peer_chain_size > peer->pbft_chain_size_) { - peer->pbft_chain_size_ = *peer_chain_size; - } } void VotePacketHandler::onNewPbftVote(const std::shared_ptr &vote, const std::shared_ptr &block,