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 prevote precommit #1642

Merged
merged 4 commits into from
Jun 2, 2023
Merged
Show file tree
Hide file tree
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
21 changes: 12 additions & 9 deletions core/consensus/grandpa/impl/grandpa_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,12 +446,18 @@ namespace kagome::consensus::grandpa {
peer_id);

auto info = peer_manager_->getPeerState(peer_id);
std::optional<VoterSetId> info_set;
std::optional<RoundNumber> info_round;
// copy values before `updatePeerState`
if (info) {
info_set = info->get().set_id;
info_round = info->get().round_number;
}

bool reputation_changed = false;
if (info.has_value() and info->get().set_id.has_value()
and info->get().round_number.has_value()) {
const auto prev_set_id = info->get().set_id.value();
const auto prev_round_number = info->get().round_number.value();
if (info_set and info_round) {
const auto prev_set_id = *info_set;
const auto prev_round_number = *info_round;

// bad order of set id
if (msg.voter_set_id < prev_set_id) {
Expand All @@ -477,11 +483,8 @@ namespace kagome::consensus::grandpa {
}

// If peer just reached one of recent round, then share known votes
if (not info.has_value()
or (info->get().set_id.has_value()
and msg.voter_set_id != info->get().set_id)
or (info->get().round_number.has_value()
and msg.round_number > info->get().round_number)) {
if (msg.voter_set_id != info_set or not info_round
or msg.round_number > *info_round) {
if (auto opt_round = selectRound(msg.round_number, msg.voter_set_id);
opt_round.has_value()) {
auto &round = opt_round.value();
Expand Down
22 changes: 19 additions & 3 deletions core/consensus/grandpa/impl/voting_round_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,18 @@ namespace kagome::consensus::grandpa {
if (stage_ == Stage::COMPLETED) {
return;
}
BOOST_ASSERT(stage_ == Stage::PRECOMMIT_RUNS);
BOOST_ASSERT(stage_ == Stage::PRECOMMIT_RUNS
|| stage_ == Stage::PRECOMMIT_WAITS_FOR_PREVOTES);

stage_timer_handle_.cancel();

// https://github.com/paritytech/finality-grandpa/blob/8c45a664c05657f0c71057158d3ba555ba7d20de/src/voter/voting_round.rs#L630-L633
if (not prevote_ghost_) {
stage_ = Stage::PRECOMMIT_WAITS_FOR_PREVOTES;
logger_->debug("Round #{}: Precommit waits for prevotes", round_number_);
return;
}

on_complete_handler_ = nullptr;

stage_ = Stage::END_PRECOMMIT;
Expand Down Expand Up @@ -579,8 +588,8 @@ namespace kagome::consensus::grandpa {
// we wait for the last round's estimate to be equal to or
// the ancestor of the current round's p-Ghost before precommitting.

const auto &prevote_ghost =
prevote_ghost_.value_or(previous_round_->bestFinalCandidate());
BOOST_ASSERT(prevote_ghost_.has_value());
const auto &prevote_ghost = prevote_ghost_.value();

auto last_round_estimate = previous_round_->bestFinalCandidate();

Expand Down Expand Up @@ -1076,6 +1085,13 @@ namespace kagome::consensus::grandpa {
if (updateGrandpaGhost()) {
need_to_update_estimate = true;
}
if (prevote_ghost_) {
scheduler_->schedule([&] {
if (stage_ == Stage::PRECOMMIT_WAITS_FOR_PREVOTES) {
endPrecommitStage();
}
});
}
}

if (need_to_update_estimate) {
Expand Down
1 change: 1 addition & 0 deletions core/consensus/grandpa/impl/voting_round_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ namespace kagome::consensus::grandpa {
// Stages for precommit mechanism
START_PRECOMMIT,
PRECOMMIT_RUNS,
PRECOMMIT_WAITS_FOR_PREVOTES,
END_PRECOMMIT,

// Stages for waiting finalisation
Expand Down
1 change: 1 addition & 0 deletions core/parachain/approval/approval_distribution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1212,6 +1212,7 @@ namespace kagome::parachain {
"Make exhaustive validation. Candidate hash {}, validator index "
"{}, block hash {}",
candidate_hash,
validator_index,
block_hash);

runtime::ValidationCode &validation_code = *result.value();
Expand Down