From f07657e355dafe1fe0491e004eadc7b4eb0b8408 Mon Sep 17 00:00:00 2001 From: Igor Egorov Date: Tue, 16 Apr 2019 17:06:29 +0300 Subject: [PATCH 1/2] Yac gate behaves itself when a commit message from the future arrives Signed-off-by: Igor Egorov --- irohad/consensus/yac/impl/yac_gate_impl.cpp | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/irohad/consensus/yac/impl/yac_gate_impl.cpp b/irohad/consensus/yac/impl/yac_gate_impl.cpp index ee03b4bae7..20e0c1113d 100644 --- a/irohad/consensus/yac/impl/yac_gate_impl.cpp +++ b/irohad/consensus/yac/impl/yac_gate_impl.cpp @@ -130,7 +130,6 @@ namespace iroha { current_hash_.vote_round, current_ledger_state_, block)); } - current_hash_ = hash; auto public_keys = boost::copy_range< shared_model::interface::types::PublicKeyCollectionType>( msg.votes | boost::adaptors::transformed([](auto &vote) { @@ -141,17 +140,15 @@ namespace iroha { // if consensus agreed on nothing for commit log_->info("Consensus skipped round, voted for nothing"); current_block_ = boost::none; - return rxcpp::observable<>::just( - AgreementOnNone(current_hash_.vote_round, - current_ledger_state_, - std::move(public_keys))); + return rxcpp::observable<>::just(AgreementOnNone( + hash.vote_round, current_ledger_state_, std::move(public_keys))); } log_->info("Voted for another block, waiting for sync"); current_block_ = boost::none; auto model_hash = hash_provider_->toModelHash(hash); return rxcpp::observable<>::just( - VoteOther(current_hash_.vote_round, + VoteOther(hash.vote_round, current_ledger_state_, std::move(public_keys), std::move(model_hash))); @@ -182,16 +179,12 @@ namespace iroha { }); if (not has_same_proposals) { log_->info("Proposal reject since all hashes are different"); - return rxcpp::observable<>::just( - ProposalReject(current_hash_.vote_round, - current_ledger_state_, - std::move(public_keys))); + return rxcpp::observable<>::just(ProposalReject( + hash.vote_round, current_ledger_state_, std::move(public_keys))); } log_->info("Block reject since proposal hashes match"); - return rxcpp::observable<>::just( - BlockReject(current_hash_.vote_round, - current_ledger_state_, - std::move(public_keys))); + return rxcpp::observable<>::just(BlockReject( + hash.vote_round, current_ledger_state_, std::move(public_keys))); } } // namespace yac } // namespace consensus From d97ebcb43fb1dd5ec494c7ea6cd507a8ef0c9bcb Mon Sep 17 00:00:00 2001 From: Igor Egorov Date: Tue, 16 Apr 2019 17:07:02 +0300 Subject: [PATCH 2/2] Add tests for the fix Signed-off-by: Igor Egorov --- .../irohad/consensus/yac/yac_gate_test.cpp | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/test/module/irohad/consensus/yac/yac_gate_test.cpp b/test/module/irohad/consensus/yac/yac_gate_test.cpp index 6a253acbad..f881a6739d 100644 --- a/test/module/irohad/consensus/yac/yac_gate_test.cpp +++ b/test/module/irohad/consensus/yac/yac_gate_test.cpp @@ -295,6 +295,113 @@ TEST_F(YacGateTest, DifferentCommit) { ASSERT_TRUE(gate_wrapper.validate()); } +/** + * The fixture checks the following case for different types of commit messages + * (VoteOther, AgreementOnNone, BlockReject, ProposalReject): + * @given yac gate, in round (i, j) -> last block height is (i - 1) + * @when reject for round (i, j + 1) is received + * @then peer goes to round (i, j + 1) + */ +class CommitFromTheFuture : public YacGateTest { + public: + void SetUp() override { + YacGateTest::SetUp(); + // make hash from block + EXPECT_CALL(*hash_provider, makeHash(_)).WillOnce(Return(expected_hash)); + + // generate order of peers + EXPECT_CALL(*peer_orderer, getOrdering(_, _)) + .WillOnce(Return(ClusterOrdering::create({makePeer("fake_node")}))); + + EXPECT_CALL(*hash_gate, vote(expected_hash, _)).Times(1); + + block_notifier.get_subscriber().on_next(BlockCreatorEvent{ + RoundData{expected_proposal, expected_block}, round, ledger_state}); + + Hash actual_hash("actual_hash"); + PublicKey actual_pubkey("actual_pubkey"); + auto signature = std::make_shared(); + EXPECT_CALL(*signature, publicKey()) + .WillRepeatedly(ReturnRefOfCopy(actual_pubkey)); + + future_round = + iroha::consensus::Round(round.block_round, round.reject_round + 1); + message.hash = YacHash(future_round, "actual_proposal", "actual_block"); + message.signature = signature; + } + + template + void validate() { + // verify that yac gate emit expected block + auto gate_wrapper = make_test_subscriber(gate->onOutcome(), 1); + gate_wrapper.subscribe([this](auto outcome) { + auto concrete_outcome = boost::get(outcome); + + ASSERT_EQ(future_round, concrete_outcome.round); + }); + + outcome_notifier.get_subscriber().on_next(expected_commit); + ASSERT_TRUE(gate_wrapper.validate()); + } + + iroha::consensus::Round future_round; +}; + +/** + * @given yac gate, in round (i, j) -> last block height is (i - 1) + * @when reject for round (i, j + 1) is received + * @then peer goes to round (i, j + 1) + */ +TEST_F(CommitFromTheFuture, BlockReject) { + expected_commit = RejectMessage({message}); + + validate(); +} + +/** + * @given yac gate, in round (i, j) -> last block height is (i - 1) + * @when reject with two proposals for round (i, j + 1) is received + * @then peer goes to round (i, j + 1) + */ +TEST_F(CommitFromTheFuture, ProposalReject) { + PublicKey second_actual_pubkey("actual_pubkey_2"); + auto second_signature = std::make_shared(); + EXPECT_CALL(*second_signature, publicKey()) + .WillRepeatedly(ReturnRefOfCopy(second_actual_pubkey)); + + VoteMessage second_message; + second_message.hash = + YacHash(future_round, "actual_proposal_2", "actual_block_2"); + second_message.signature = second_signature; + expected_commit = RejectMessage({message, second_message}); + + validate(); +} + +/** + * @given yac gate, in round (i, j) -> last block height is (i - 1) + * @when commit for round (i, j + 1) is received + * @then peer goes to round (i, j + 1) + */ +TEST_F(CommitFromTheFuture, VoteOther) { + expected_commit = CommitMessage({message}); + + validate(); +} + +/** + * @given yac gate, in round (i, j) -> last block height is (i - 1) + * @when commit without proposal (empty proposal hash) for round (i, j + 1) is + * received + * @then peer goes to round (i, j + 1) + */ +TEST_F(CommitFromTheFuture, AgreementOnNone) { + message.hash = YacHash(future_round, "", ""); + expected_commit = CommitMessage({message}); + + validate(); +} + class YacGateOlderTest : public YacGateTest { void SetUp() override { YacGateTest::SetUp();