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 finalization bugs and issues #1290

Merged
merged 25 commits into from
Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
195c623
Attempt to fix finalization lag by loading missing justifications
Jul 14, 2022
04f68d8
The second iteration
Jul 14, 2022
88dc3a6
fix: log of next epoch digest
xDimon Jul 19, 2022
4459b32
fix: send neighbor message in case of passive round
xDimon Jul 19, 2022
681c7c5
fix: logging of ping protocol handling
xDimon Jul 19, 2022
0bf1be3
fix: return check if peer is alive
xDimon Jul 19, 2022
6a4677b
feature: remove disconnected peer from stream engine
xDimon Jul 19, 2022
bf2f158
feature: forget fingerprint of sync-request in case of empty response
xDimon Jul 19, 2022
c18c760
refactor: conditions of sync-request and normal catch-up of round
xDimon Jul 19, 2022
1ea42d2
feature: catch-up based sync-request in according to applyed blocks
xDimon Jul 19, 2022
d736237
fix: Mixing full and authority roles
xDimon Jul 21, 2022
415c7ae
refactor: use short peer_id in log
xDimon Jul 21, 2022
a1c245a
fix: crash at execute next round after applying of justification
xDimon Jul 22, 2022
a4aca09
refactor: make sit_id and round in peer data optional
xDimon Jul 22, 2022
6dfc212
refactor: order of peer data (about grandpa) initialization
xDimon Jul 22, 2022
49960d0
refactor: comment disconnecting of peer by liveliness
xDimon Jul 26, 2022
0408f7c
fix: getting of descending chain to block
xDimon Jul 28, 2022
381c90f
feature: suppression of catch-up request to one peer for time
xDimon Jul 28, 2022
687e2db
fix: log and some corner case in sync protocol
xDimon Jul 28, 2022
be61d47
refactor: use cli bootstrap nodes early then spec's ones
xDimon Jul 28, 2022
db09003
fix: allow to create next round by commit for non finalized
xDimon Jul 28, 2022
d94eb5c
fix: order of peer state update
xDimon Jul 28, 2022
025af5a
fix: typo
xDimon Jul 28, 2022
28e20dc
Merge branch 'master' into fix/finalisation-lag@xdimon
xDimon Jul 28, 2022
a861af5
fix: review issues
xDimon Aug 1, 2022
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
4 changes: 3 additions & 1 deletion core/application/impl/app_configuration_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ namespace kagome::application {
bool validator_mode = false;
load_bool(val, "validator", validator_mode);
if (validator_mode) {
roles_.flags.full = 0;
roles_.flags.authority = 1;
}

Expand Down Expand Up @@ -800,7 +801,7 @@ namespace kagome::application {
}
}

roles_.flags.full = 1;
roles_.flags.full = 0;
roles_.flags.authority = 1;
p2p_port_ = def_p2p_port;
rpc_http_host_ = def_rpc_http_host;
Expand All @@ -822,6 +823,7 @@ namespace kagome::application {
});

if (vm.end() != vm.find("validator")) {
roles_.flags.full = 0;
roles_.flags.authority = 1;
}

Expand Down
4 changes: 3 additions & 1 deletion core/blockchain/impl/block_tree_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -985,11 +985,13 @@ namespace kagome::blockchain {
}
const auto &header = header_res.value();

chain.emplace_back(hash);

if (header.parent_hash == primitives::BlockHash{}) {
break;
}

chain.emplace_back(header.parent_hash);
hash = header.parent_hash;
}

return std::vector<primitives::BlockHash>(chain.begin(), chain.end());
Expand Down
4 changes: 4 additions & 0 deletions core/consensus/babe/block_executor.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ namespace kagome::consensus {
virtual ~BlockExecutor() = default;

virtual outcome::result<void> applyBlock(primitives::BlockData &&block) = 0;

virtual outcome::result<void> applyJustification(
const primitives::BlockInfo &block_info,
const primitives::Justification &justification) = 0;
};

} // namespace kagome::consensus
Expand Down
13 changes: 9 additions & 4 deletions core/consensus/babe/impl/block_executor_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ namespace kagome::consensus {
SL_VERBOSE(logger_,
"Next epoch digest has gotten in block {} (slot {}). "
"Randomness: {}",
slot_number,
primitives::BlockInfo(block.header.number, block_hash),
slot_number,
next_epoch_digest.randomness);
}

Expand Down Expand Up @@ -284,8 +284,7 @@ namespace kagome::consensus {
if (not justifications_.empty()) {
std::vector<primitives::BlockInfo> to_remove;
for (const auto &[block_info, justification] : justifications_) {
auto res = grandpa_environment_->applyJustification(block_info,
justification);
auto res = applyJustification(block_info, justification);
if (res) {
to_remove.push_back(block_info);
}
Expand All @@ -297,7 +296,7 @@ namespace kagome::consensus {
}
}

auto res = grandpa_environment_->applyJustification(
auto res = applyJustification(
primitives::BlockInfo(block.header.number, block_hash),
b.justification.value());
if (res.has_error()) {
Expand Down Expand Up @@ -358,6 +357,12 @@ namespace kagome::consensus {
return outcome::success();
}

outcome::result<void> BlockExecutorImpl::applyJustification(
const primitives::BlockInfo &block_info,
const primitives::Justification &justification) {
return grandpa_environment_->applyJustification(block_info, justification);
}

void BlockExecutorImpl::rollbackBlock(const primitives::BlockInfo &block) {
// Remove possible authority changes scheduled on block
authority_update_observer_->cancel(block);
Expand Down
4 changes: 4 additions & 0 deletions core/consensus/babe/impl/block_executor_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ namespace kagome::consensus {

outcome::result<void> applyBlock(primitives::BlockData &&block) override;

outcome::result<void> applyJustification(
const primitives::BlockInfo &block_info,
const primitives::Justification &justification) override;

private:
void rollbackBlock(const primitives::BlockInfo &block);

Expand Down
185 changes: 121 additions & 64 deletions core/consensus/grandpa/impl/grandpa_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,8 @@ namespace kagome::consensus::grandpa {

std::shared_ptr<VotingRound> GrandpaImpl::makeNextRound(
const std::shared_ptr<VotingRound> &round) {
BOOST_ASSERT(round->finalizedBlock().has_value());

BlockInfo best_block = round->finalizedBlock().value();
BlockInfo best_block =
round->finalizedBlock().value_or(round->lastFinalizedBlock());

auto authorities_opt =
authority_manager_->authorities(best_block, IsBlockFinalized{true});
Expand Down Expand Up @@ -298,6 +297,11 @@ namespace kagome::consensus::grandpa {
metric_highest_round_->set(current_round_->roundNumber());
if (keypair_) {
current_round_->play();
} else {
auto round = std::dynamic_pointer_cast<VotingRoundImpl>(current_round_);
if (round) {
round->sendNeighborMessage();
}
}
}

Expand All @@ -322,28 +326,66 @@ namespace kagome::consensus::grandpa {
msg.last_finalized,
peer_id);

// Ignore peer whose voter_set is different
if (msg.voter_set_id != current_round_->voterSetId()) {
return;
}

// Check if needed to catch-up peer, then do that
if (msg.round_number >= current_round_->roundNumber() + kCatchUpThreshold) {
std::ignore = environment_->onCatchUpRequested(
peer_id, msg.voter_set_id, msg.round_number - 1);
return;
}
auto info = peer_manager_->getPeerState(peer_id);

// Iff peer just reached one of recent round, then share known votes
auto info = peer_manager_->getPeerState(peer_id);
if (not info.has_value() || msg.voter_set_id != info->set_id
|| msg.round_number > info->round_number) {
if (not info.has_value()
or (info->set_id.has_value() and msg.voter_set_id != info->set_id)
or (info->round_number.has_value()
and msg.round_number > info->round_number)) {
if (auto opt_round = selectRound(msg.round_number, msg.voter_set_id);
opt_round.has_value()) {
auto &round = opt_round.value();
environment_->sendState(peer_id, round->state(), msg.voter_set_id);
}
}

peer_manager_->updatePeerState(peer_id, msg);

// If peer has the same voter set id
if (msg.voter_set_id == current_round_->voterSetId()) {
// Check if needed to catch-up peer, then do that
if (msg.round_number
>= current_round_->roundNumber() + kCatchUpThreshold) {
std::ignore = environment_->onCatchUpRequested(
peer_id, msg.voter_set_id, msg.round_number - 1);
}

return;
}

// Ignore peer whose voter set id lower than our current
if (msg.voter_set_id < current_round_->voterSetId()) {
return;
}

if (info->last_finalized <= block_tree_->deepestLeaf().number) {
// Trying to substitute with justifications' request only
auto last_finalized = block_tree_->getLastFinalized();
synchronizer_->syncMissingJustifications(
peer_id,
last_finalized,
std::nullopt,
[wp = weak_from_this(), last_finalized, msg](auto res) {
auto self = wp.lock();
if (not self) {
return;
}
if (res.has_error()) {
SL_WARN(self->logger_,
"Missing justifications between blocks {} and "
"{} was not loaded: {}",
last_finalized,
msg.last_finalized,
res.error().message());
} else {
SL_DEBUG(self->logger_,
"Loaded justifications for blocks in range {} - {}",
last_finalized,
res.value());
}
});
}
}

void GrandpaImpl::onCatchUpRequest(const libp2p::peer::PeerId &peer_id,
Expand Down Expand Up @@ -766,73 +808,88 @@ namespace kagome::consensus::grandpa {

outcome::result<void> GrandpaImpl::applyJustification(
const BlockInfo &block_info, const GrandpaJustification &justification) {
auto opt_round = selectRound(justification.round_number, std::nullopt);
auto round_opt = selectRound(justification.round_number, std::nullopt);
std::shared_ptr<VotingRound> round;
bool need_to_make_round_current = false;
if (!opt_round.has_value()) {
if (round_opt.has_value()) {
round = std::move(round_opt.value());
} else {
// This is justification for already finalized block
if (current_round_->lastFinalizedBlock().number > block_info.number) {
return VotingRoundError::JUSTIFICATION_FOR_BLOCK_IN_PAST;
}

MovableRoundState round_state{
.round_number = justification.round_number,
.last_finalized_block = current_round_->lastFinalizedBlock(),
.votes = {},
.finalized = block_info};

auto authorities_opt =
authority_manager_->authorities(block_info, IsBlockFinalized{false});
if (!authorities_opt) {
SL_WARN(logger_,
"Can't retrieve authorities to apply a justification "
"at block {}",
block_info);
return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
}
auto &authorities = authorities_opt.value();

// This is justification for non-actual round
if (authorities->id < current_round_->voterSetId()
or (authorities->id == current_round_->voterSetId()
&& justification.round_number < current_round_->roundNumber())) {
return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
}
auto prev_round_opt =
selectRound(justification.round_number - 1, std::nullopt);

if (prev_round_opt.has_value()) {
const auto &prev_round = prev_round_opt.value();
round = makeNextRound(prev_round);
need_to_make_round_current = true;
BOOST_ASSERT(round);

SL_DEBUG(logger_,
"Hop grandpa to round #{} by received justification",
justification.round_number);
} else {
MovableRoundState round_state{
.round_number = justification.round_number,
.last_finalized_block = current_round_->lastFinalizedBlock(),
.votes = {},
.finalized = block_info};

auto authorities_opt = authority_manager_->authorities(
block_info, IsBlockFinalized{false});
if (!authorities_opt) {
SL_WARN(logger_,
"Can't retrieve authorities to apply a justification "
"at block {}",
block_info);
return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
}
auto &authorities = authorities_opt.value();

// This is justification for non-actual round
if (authorities->id < current_round_->voterSetId()
or (authorities->id == current_round_->voterSetId()
&& justification.round_number
< current_round_->roundNumber())) {
return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
}

if (authorities->id > current_round_->voterSetId() + 1) {
return VotingRoundError::WRONG_ORDER_OF_VOTER_SET_ID;
}
if (authorities->id > current_round_->voterSetId() + 1) {
return VotingRoundError::WRONG_ORDER_OF_VOTER_SET_ID;
}

auto voters = std::make_shared<VoterSet>(authorities->id);
for (const auto &authority : *authorities) {
auto res = voters->insert(
primitives::GrandpaSessionKey(authority.id.id), authority.weight);
if (res.has_error()) {
SL_CRITICAL(
logger_, "Can't make voter set: {}", res.error().message());
return res.as_failure();
auto voters = std::make_shared<VoterSet>(authorities->id);
for (const auto &authority : *authorities) {
auto res = voters->insert(
primitives::GrandpaSessionKey(authority.id.id), authority.weight);
if (res.has_error()) {
SL_CRITICAL(
logger_, "Can't make voter set: {}", res.error().message());
return res.as_failure();
}
}
}

round = makeInitialRound(round_state, std::move(voters));
need_to_make_round_current = true;
BOOST_ASSERT(round);
round = makeInitialRound(round_state, std::move(voters));
need_to_make_round_current = true;
BOOST_ASSERT(round);

SL_DEBUG(logger_,
"Rewind grandpa till round #{} by received justification",
justification.round_number);
} else {
round = *opt_round;
SL_DEBUG(logger_,
"Rewind grandpa till round #{} by received justification",
justification.round_number);
}
}

OUTCOME_TRY(round->applyJustification(block_info, justification));

if (need_to_make_round_current) {
current_round_->end();
current_round_ = std::move(round);
current_round_ = round;
}

tryExecuteNextRound(current_round_);
tryExecuteNextRound(round);

// if round == current round, then execution of the next round will be
// elsewhere
Expand Down
4 changes: 2 additions & 2 deletions core/consensus/grandpa/impl/voting_round_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ namespace kagome::consensus::grandpa {
clock,
scheduler) {
BOOST_ASSERT(previous_round != nullptr);
BOOST_ASSERT(previous_round->finalizedBlock().has_value());

previous_round_ = previous_round;
last_finalized_block_ = previous_round->finalizedBlock().value();
last_finalized_block_ = previous_round->finalizedBlock().value_or(
previous_round->lastFinalizedBlock());
}

VotingRoundImpl::VotingRoundImpl(
Expand Down
3 changes: 2 additions & 1 deletion core/consensus/grandpa/impl/voting_round_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ namespace kagome::consensus::grandpa {
*/
MovableRoundState state() const override;

void sendNeighborMessage();

private:
/// Check if peer \param id is primary
bool isPrimary(const Id &id) const;
Expand Down Expand Up @@ -290,7 +292,6 @@ namespace kagome::consensus::grandpa {
outcome::result<void> validatePrecommitJustification(
const BlockInfo &vote, const GrandpaJustification &justification) const;

void sendNeighborMessage();
void sendProposal(const PrimaryPropose &primary_proposal);
void sendPrevote(const Prevote &prevote);
void sendPrecommit(const Precommit &precommit);
Expand Down
Loading