Skip to content
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.

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

fix: use correct round when checking reward votes #2316

Merged
merged 1 commit into from
Feb 13, 2023
Merged
Changes from all commits
Commits
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
49 changes: 35 additions & 14 deletions libraries/core_libs/consensus/src/vote_manager/vote_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -512,11 +512,16 @@ PbftPeriod VoteManager::getRewardVotesPbftBlockPeriod() {
}

void VoteManager::resetRewardVotesInfo(PbftPeriod period, PbftRound round, const blk_hash_t& block_hash) {
std::scoped_lock lock(reward_votes_info_mutex_);
{
std::scoped_lock lock(reward_votes_info_mutex_);

reward_votes_block_hash_ = block_hash;
reward_votes_period_ = period;
reward_votes_round_ = round;
}

reward_votes_block_hash_ = block_hash;
reward_votes_period_ = period;
reward_votes_round_ = round;
LOG(log_dg_) << "Reward votes info reset to: block_hash: " << block_hash << ", period: " << period
<< ", round: " << round;
}

bool VoteManager::isValidRewardVote(const std::shared_ptr<Vote>& vote) const {
Expand Down Expand Up @@ -560,37 +565,46 @@ std::pair<bool, std::vector<std::shared_ptr<Vote>>> VoteManager::checkRewardVote
return {true, {}};
}

auto getRewardVotes = [](const std::map<PbftRound, VerifiedVotes>::iterator round_votes_it,
const std::vector<vote_hash_t>& vote_hashes, const blk_hash_t& block_hash,
bool copy_votes) -> std::pair<bool, std::vector<std::shared_ptr<Vote>>> {
auto getRewardVotes = [this](const std::map<PbftRound, VerifiedVotes>::iterator round_votes_it,
const std::vector<vote_hash_t>& vote_hashes, const blk_hash_t& block_hash,
bool copy_votes) -> std::pair<bool, std::vector<std::shared_ptr<Vote>>> {
// Get cert votes
const auto found_step_votes_it =
round_votes_it->second.step_votes.find(static_cast<PbftStep>(PbftVoteTypes::cert_vote));
if (found_step_votes_it == round_votes_it->second.step_votes.end()) {
LOG(log_dg_) << "getRewardVotes: No votes found for certify step "
<< static_cast<PbftStep>(PbftVoteTypes::cert_vote);
return {false, {}};
}

// Find verified votes for specified block_hash
const auto found_verified_votes_it = found_step_votes_it->second.votes.find(block_hash);
if (found_verified_votes_it == found_step_votes_it->second.votes.end()) {
LOG(log_dg_) << "getRewardVotes: No votes found for block_hash " << block_hash;
return {false, {}};
}

std::vector<std::shared_ptr<Vote>> found_reward_votes;
const auto& potential_reward_votes = found_verified_votes_it->second.second;
bool found_all_votes = true;
for (const auto& vote_hash : vote_hashes) {
const auto found_vote = potential_reward_votes.find(vote_hash);
if (found_vote == potential_reward_votes.end()) {
// Reward vote not found
return {false, {}};
LOG(log_tr_) << "getRewardVotes: Vote " << vote_hash << " not found";
found_all_votes = false;
}

if (copy_votes) {
if (found_all_votes && copy_votes) {
found_reward_votes.push_back(found_vote->second);
}
}

return {true, std::move(found_reward_votes)};
if (found_all_votes) {
return {true, std::move(found_reward_votes)};
} else {
return {false, {}};
}
};

std::shared_lock reward_votes_info_lock(reward_votes_info_mutex_);
Expand Down Expand Up @@ -621,17 +635,24 @@ std::pair<bool, std::vector<std::shared_ptr<Vote>>> VoteManager::checkRewardVote
// It could happen though in some edge cases that some nodes pushed the same block in different round than we did
// and when they included the reward votes in new block, these votes have different round than what saved in
// reward_votes_round_ -> therefore we have to iterate over all rounds and find the correct round
for (auto round_it = found_period_it->second.rbegin(); round_it != found_period_it->second.rend(); round_it++) {
const auto tmp_reward_votes =
getRewardVotes(found_round_it, reward_votes_hashes, reward_votes_block_hash_, copy_votes);
for (auto round_it = found_period_it->second.begin(); round_it != found_period_it->second.end(); round_it++) {
const auto tmp_reward_votes = getRewardVotes(round_it, reward_votes_hashes, reward_votes_block_hash_, copy_votes);
if (!tmp_reward_votes.first) {
LOG(log_dg_) << "No (or not enough) reward votes found for block " << pbft_block->getBlockHash()
<< ", period: " << pbft_block->getPeriod()
<< ", prev. block hash: " << pbft_block->getPrevBlockHash()
<< ", reward_votes_period_: " << reward_votes_period_ << ", reward_votes_round_: " << round_it->first
<< ", reward_votes_block_hash_: " << reward_votes_block_hash_;
continue;
}

return {true, std::move(tmp_reward_votes.second)};
}

LOG(log_er_) << "No reward votes found for round " << reward_votes_block_hash_;
LOG(log_er_) << "No (or not enough) reward votes found for block " << pbft_block->getBlockHash()
<< ", period: " << pbft_block->getPeriod() << ", prev. block hash: " << pbft_block->getPrevBlockHash()
<< ", reward_votes_period_: " << reward_votes_period_ << ", reward_votes_round_: " << reward_votes_round_
<< ", reward_votes_block_hash_: " << reward_votes_block_hash_;
return {false, {}};
}

Expand Down