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 forced change #1534

Merged
merged 9 commits into from
Mar 23, 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
22 changes: 12 additions & 10 deletions core/consensus/babe/impl/block_header_appender_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,7 @@ namespace kagome::consensus::babe {
consistency_guard,
appender_->observeDigestsAndValidateHeader(block, block_context));

OUTCOME_TRY(appender_->applyJustifications(block_info,
justification));
OUTCOME_TRY(appender_->applyJustifications(block_info, justification));

auto now = std::chrono::high_resolution_clock::now();

Expand All @@ -102,14 +101,17 @@ namespace kagome::consensus::babe {
auto block_delta = block_info.number - speed_data_.block_number;
auto time_delta = now - speed_data_.time;
if (block_delta >= 10000 or time_delta >= std::chrono::minutes(1)) {
SL_LOG(logger_,
speed_data_.block_number ? log::Level::INFO
: static_cast<log::Level>(-1),
"Imported {} more headers of blocks. Average speed is {} bps",
block_delta,
block_delta
/ std::chrono::duration_cast<std::chrono::seconds>(time_delta)
.count());
SL_LOG(
logger_,
speed_data_.block_number ? log::Level::INFO
: static_cast<log::Level>(-1),
"Imported {} more headers of blocks {}-{}. Average speed is {} bps",
block_delta,
speed_data_.block_number,
block_info.number,
block_delta
/ std::chrono::duration_cast<std::chrono::seconds>(time_delta)
.count());
speed_data_.block_number = block_info.number;
speed_data_.time = now;
}
Expand Down
77 changes: 35 additions & 42 deletions core/consensus/grandpa/impl/authority_manager_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,13 @@ namespace kagome::consensus::grandpa {
Config config,
std::shared_ptr<application::AppStateManager> app_state_manager,
std::shared_ptr<blockchain::BlockTree> block_tree,
std::shared_ptr<storage::trie::TrieStorage> trie_storage,
std::shared_ptr<runtime::GrandpaApi> grandpa_api,
std::shared_ptr<crypto::Hasher> hasher,
std::shared_ptr<storage::SpacedStorage> persistent_storage,
std::shared_ptr<blockchain::BlockHeaderRepository> header_repo,
primitives::events::ChainSubscriptionEnginePtr chain_events_engine)
: config_{std::move(config)},
block_tree_(std::move(block_tree)),
trie_storage_(std::move(trie_storage)),
grandpa_api_(std::move(grandpa_api)),
hasher_(std::move(hasher)),
persistent_storage_{
Expand All @@ -55,7 +53,6 @@ namespace kagome::consensus::grandpa {
logger_{log::createLogger("AuthorityManager", "authority")} {
BOOST_ASSERT(block_tree_ != nullptr);
BOOST_ASSERT(grandpa_api_ != nullptr);
BOOST_ASSERT(trie_storage_ != nullptr);
BOOST_ASSERT(hasher_ != nullptr);
BOOST_ASSERT(persistent_storage_ != nullptr);
BOOST_ASSERT(header_repo_ != nullptr);
Expand Down Expand Up @@ -529,6 +526,15 @@ namespace kagome::consensus::grandpa {
ancestor_node->block,
ancestor_node->authorities->id);

for (auto &block : ancestor_node->forced_digests) {
if (directChainExists(context.block_info, block)) {
SL_DEBUG(logger_,
"Scheduled change digest {} ignored by forced change",
context.block_info.number);
return outcome::success();
}
}

auto schedule_change = [&](const std::shared_ptr<ScheduleNode> &node)
-> outcome::result<void> {
auto new_authorities = std::make_shared<primitives::AuthoritySet>(
Expand Down Expand Up @@ -630,39 +636,33 @@ namespace kagome::consensus::grandpa {
const primitives::AuthorityList &authorities,
primitives::BlockNumber delay_start,
size_t delay) {
auto forced_number = delay_start + delay;
SL_DEBUG(logger_,
"Applying forced change (delay start: {}, delay: {}) on block {} "
"to activate at block {}",
delay_start,
delay,
context.block_info,
delay_start + delay);
if (delay_start < root_->block.number) {
auto lag = root_->block.number - delay_start;
if (delay > lag) {
delay_start = root_->block.number;
delay -= lag;
} else {
delay_start = context.block_info.number;
delay = 0;
}
forced_number);
if (forced_number < root_->block.number) {
SL_DEBUG(
logger_,
"Applying forced change on block {} is delayed {} blocks. "
"Normalized to (delay start: {}, delay: {}) to activate at block {}",
context.block_info,
lag,
root_->block.number - forced_number,
delay_start,
delay,
delay_start + delay);
root_->block.number);
forced_number = root_->block.number;
}
auto delay_start_hash_res = header_repo_->getHashByNumber(delay_start);
auto delay_start_hash_res = header_repo_->getHashByNumber(forced_number);
if (delay_start_hash_res.has_error()) {
SL_ERROR(logger_, "Failed to obtain hash by number {}", delay_start);
SL_ERROR(logger_, "Failed to obtain hash by number {}", forced_number);
}
OUTCOME_TRY(delay_start_hash, delay_start_hash_res);
auto ancestor_node =
getNode({.block_info = {delay_start, delay_start_hash}});
getNode({.block_info = {forced_number, delay_start_hash}});

if (not ancestor_node) {
return AuthorityManagerError::ORPHAN_BLOCK_OR_ALREADY_FINALIZED;
Expand All @@ -673,26 +673,27 @@ namespace kagome::consensus::grandpa {
ancestor_node->block,
ancestor_node->authorities->id);

for (auto &block : ancestor_node->forced_digests) {
if (context.block_info == block) {
SL_DEBUG(logger_,
"Forced change digest {} already included",
context.block_info.number);
return outcome::success();
}
}

auto force_change = [&](const std::shared_ptr<ScheduleNode> &node)
-> outcome::result<void> {
auto new_authorities = std::make_shared<primitives::AuthoritySet>(
node->authorities->id + 1, authorities);

// Force changes
if (node->block.number >= delay_start + delay) {
node->authorities = new_authorities;
SL_VERBOSE(logger_,
"Change has been forced on block #{} (set id={})",
delay_start + delay,
node->authorities->id);
} else {
node->action =
ScheduleNode::ForcedChange{delay_start, delay, new_authorities};
SL_VERBOSE(logger_,
"Change will be forced on block #{} (set id={})",
delay_start + delay,
new_authorities->id);
}
node->forced_digests.emplace_back(context.block_info);

node->authorities = new_authorities;
SL_VERBOSE(logger_,
"Change has been forced on block #{} (set id={})",
forced_number,
node->authorities->id);

size_t index = 0;
for (auto &authority : *new_authorities) {
Expand All @@ -708,7 +709,7 @@ namespace kagome::consensus::grandpa {
};

auto new_node =
ancestor_node->makeDescendant({delay_start, delay_start_hash}, true);
ancestor_node->makeDescendant({forced_number, delay_start_hash}, true);

OUTCOME_TRY(force_change(new_node));

Expand Down Expand Up @@ -1021,14 +1022,6 @@ namespace kagome::consensus::grandpa {
new_node->block.number < descendant->block.number ? new_node : node;

// Apply if delay will be passed for descendant
if (auto *forced_change =
boost::get<ScheduleNode::ForcedChange>(&ancestor->action)) {
if (descendant->block.number
>= forced_change->delay_start + forced_change->delay_length) {
descendant->authorities = forced_change->new_authorities;
descendant->action = ScheduleNode::NoAction{};
}
}
if (auto *resume = boost::get<ScheduleNode::Resume>(&ancestor->action)) {
if (descendant->block.number >= resume->applied_block) {
descendant->enabled = true;
Expand Down
6 changes: 0 additions & 6 deletions core/consensus/grandpa/impl/authority_manager_impl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,6 @@ namespace kagome::runtime {
class Executor;
} // namespace kagome::runtime

namespace kagome::storage::trie {
class TrieStorage;
}

namespace kagome::consensus::grandpa {

class AuthorityManagerImpl final
Expand All @@ -69,7 +65,6 @@ namespace kagome::consensus::grandpa {
Config config,
std::shared_ptr<application::AppStateManager> app_state_manager,
std::shared_ptr<blockchain::BlockTree> block_tree,
std::shared_ptr<storage::trie::TrieStorage> trie_storage,
std::shared_ptr<runtime::GrandpaApi> grandpa_api,
std::shared_ptr<crypto::Hasher> hash,
std::shared_ptr<storage::SpacedStorage> persistent_storage,
Expand Down Expand Up @@ -153,7 +148,6 @@ namespace kagome::consensus::grandpa {

Config config_;
std::shared_ptr<const blockchain::BlockTree> block_tree_;
std::shared_ptr<storage::trie::TrieStorage> trie_storage_;
std::shared_ptr<runtime::GrandpaApi> grandpa_api_;
std::shared_ptr<crypto::Hasher> hasher_;
std::shared_ptr<storage::BufferStorage> persistent_storage_;
Expand Down
35 changes: 26 additions & 9 deletions core/consensus/grandpa/impl/grandpa_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ namespace {
} // namespace

namespace kagome::consensus::grandpa {
inline bool isWestendPastRound(const primitives::BlockHash &genesis,
const primitives::BlockInfo &block) {
static auto westend_genesis =
primitives::BlockHash::fromHex(
"e143f23803ac50e8f6f8e62695d1ce9e4e1d68aa36c1cd2cfd15340213f3423e")
.value();
static primitives::BlockInfo past_round{
198785,
primitives::BlockHash::fromHex(
"62caf6a8c99d63744f7093bceead8fdf4c7d8ef74f16163ed58b1c1aec67bf18")
.value(),
};
return genesis == westend_genesis && block == past_round;
}

namespace {
Clock::Duration getGossipDuration(const application::ChainSpec &chain) {
Expand Down Expand Up @@ -1213,8 +1227,15 @@ namespace kagome::consensus::grandpa {
return VotingRoundError::JUSTIFICATION_FOR_BLOCK_IN_PAST;
}

auto authorities_opt =
authority_manager_->authorities(block_info, IsBlockFinalized{false});
if (!authorities_opt) {
return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
}
auto &authority_set = authorities_opt.value();

auto prev_round_opt =
selectRound(justification.round_number - 1, std::nullopt);
selectRound(justification.round_number - 1, authority_set->id);

if (prev_round_opt.has_value()) {
const auto &prev_round = prev_round_opt.value();
Expand All @@ -1239,20 +1260,16 @@ namespace kagome::consensus::grandpa {
.votes = {},
.finalized = block_info};

auto authorities_opt = authority_manager_->authorities(
block_info, IsBlockFinalized{false});
if (!authorities_opt) {
return VotingRoundError::NO_KNOWN_AUTHORITIES_FOR_BLOCK;
}
auto &authority_set = authorities_opt.value();

// This is justification for non-actual round
if (authority_set->id < current_round_->voterSetId()) {
return VotingRoundError::JUSTIFICATION_FOR_AUTHORITY_SET_IN_PAST;
}
if (authority_set->id == current_round_->voterSetId()
&& justification.round_number < current_round_->roundNumber()) {
return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
if (not isWestendPastRound(block_tree_->getGenesisBlockHash(),
justification.block_info)) {
return VotingRoundError::JUSTIFICATION_FOR_ROUND_IN_PAST;
}
}

if (authority_set->id > current_round_->voterSetId() + 1) {
Expand Down
9 changes: 2 additions & 7 deletions core/consensus/grandpa/impl/schedule_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,14 @@ namespace kagome::consensus::grandpa {
if (scheduled_change->applied_block <= block.number) {
authorities = std::move(scheduled_change->new_authorities);
action = NoAction{};
forced_digests = {};
}
} else if (auto pause = boost::get<Pause>(&action);
finalized && pause != nullptr) {
if (pause->applied_block <= block.number) {
enabled = false;
action = NoAction{};
}
} else if (auto forced_change = boost::get<ForcedChange>(&action);
forced_change != nullptr) {
if (forced_change->delay_start + forced_change->delay_length
<= block.number) {
authorities = std::move(forced_change->new_authorities);
action = NoAction{};
}
} else if (auto resume = boost::get<Resume>(&action); resume != nullptr) {
if (resume->applied_block <= block.number) {
enabled = true;
Expand All @@ -60,6 +54,7 @@ namespace kagome::consensus::grandpa {
node->authorities = authorities;
node->enabled = enabled;
node->action = action;
node->forced_digests = forced_digests;
node->adjust(finalized);
return node;
}
Expand Down
20 changes: 6 additions & 14 deletions core/consensus/grandpa/impl/schedule_node.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,6 @@ namespace kagome::consensus::grandpa {
std::shared_ptr<const primitives::AuthoritySet> new_authorities{};
};

struct ForcedChange {
SCALE_TIE(3);

primitives::BlockNumber delay_start{};
size_t delay_length{};
std::shared_ptr<const primitives::AuthoritySet> new_authorities{};
};

struct Pause {
SCALE_TIE(1);

Expand All @@ -74,23 +66,23 @@ namespace kagome::consensus::grandpa {

friend inline ::scale::ScaleEncoderStream &operator<<(
::scale::ScaleEncoderStream &s, const ScheduleNode &node) {
return s << node.enabled << node.block << *node.authorities
<< node.action;
return s << node.enabled << node.block << *node.authorities << node.action
<< node.forced_digests;
}

friend inline ::scale::ScaleDecoderStream &operator>>(
::scale::ScaleDecoderStream &s, ScheduleNode &node) {
return s >> node.enabled
>> const_cast<primitives::BlockInfo &>(node.block)
>> node.authorities >> node.action;
>> const_cast<primitives::BlockInfo &>(node.block) >> node.authorities
>> node.action >> node.forced_digests;
}

const primitives::BlockInfo block{};
std::weak_ptr<const ScheduleNode> parent;
std::vector<std::shared_ptr<ScheduleNode>> descendants{};

boost::variant<NoAction, ScheduledChange, ForcedChange, Pause, Resume>
action;
boost::variant<NoAction, ScheduledChange, Pause, Resume> action;
std::vector<primitives::BlockInfo> forced_digests;
std::shared_ptr<const primitives::AuthoritySet> authorities;
bool enabled = true;
};
Expand Down
3 changes: 2 additions & 1 deletion core/network/warp/cache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@ namespace kagome::network {
auto r = start(std::move(chain_sub_engine));
if (not r) {
SL_WARN(log_, "start error {}", r.error());
return false;
// TODO(turuslan): #1536 warp sync forced change
return true;
}
return true;
});
Expand Down
1 change: 0 additions & 1 deletion core/utils/storage_explorer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,6 @@ int storage_explorer_main(int argc, const char **argv) {
std::make_shared<AuthorityManagerImpl>(AuthorityManagerImpl::Config{},
app_state_manager,
block_tree,
trie_storage,
grandpa_api,
hasher,
persistent_storage,
Expand Down
Loading