Skip to content
This repository has been archived by the owner on Apr 17, 2019. It is now read-only.

Fix consensus VoteOther case #1834

Merged
merged 6 commits into from
Nov 9, 2018
Merged

Conversation

lebdron
Copy link
Contributor

@lebdron lebdron commented Nov 6, 2018

Description of the Change

The information in VoteOther struct was not enough to handle block loading, so it is replaced with

  • public keys of peers who made the latest commit
  • hash of committed block

Benefits

Can synchronize in case different block was committed

Possible Drawbacks

None?

Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
@lebdron lebdron requested review from l4l and nickaleks November 6, 2018 08:59
@@ -35,15 +36,19 @@ namespace iroha {
* apply the missing blocks
*/
SynchronizationEvent downloadMissingBlocks(
std::shared_ptr<shared_model::interface::Block> commit_message,
const shared_model::interface::types::PublicKeyCollectionType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe pass VoteOther to avoid specifying all parameters in functions?

void init() {
gate = std::make_shared<YacGateImpl>(std::move(hash_gate),
std::move(peer_orderer),
EXPECT_CALL(*block_creator, on_block())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use ON_CALL so that we do not set any unnecessary invariants for tests?

// verify that yac gate emit expected block
auto gate_wrapper = make_test_subscriber<CallExact>(gate->onOutcome(), 1);
gate_wrapper.subscribe([actual_hash, actual_pubkey](auto outcome) {
auto concete_outcome = boost::get<iroha::consensus::VoteOther>(outcome);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo in variable name?

@l4l l4l added needs-review pr awaits review from maintainers consensus labels Nov 6, 2018

hash_gate = make_unique<MockHashGate>();
peer_orderer = make_unique<MockYacPeerOrderer>();
auto hash_gate_ptr = make_unique<MockHashGate>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using namespace std
😱 (not a pr issue)

@@ -119,8 +119,19 @@ namespace iroha {
PairValid{block, current_hash_.vote_round});
}
log_->info("Voted for another block, waiting for sync");
auto public_keys = std::accumulate(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe use transform instead?

Signed-off-by: Andrei Lebedev <lebdron@gmail.com>
@l4l l4l removed the needs-review pr awaits review from maintainers label Nov 9, 2018
@lebdron lebdron merged commit e37760e into trunk/bft-os-integration Nov 9, 2018
@lebdron lebdron deleted the fix/vote-other-case branch November 9, 2018 09:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

3 participants