KEP-902 Fixed a exception that occurs in is_valid_viewchange_message …

…when no checkpoint has been set
bluzelle · Dec 13, 2018 · 1634344 · 1634344
1 parent 51f707a
commit 1634344
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 1 deletion.
diff --git a/pbft/pbft.cpp b/pbft/pbft.cpp
@@ -1122,7 +1122,7 @@ pbft::is_valid_viewchange_message(const pbft_msg& viewchange_message, const bzn_
     }
 
     auto valid_checkpoint_hashes = this->validate_and_extract_checkpoint_hashes(viewchange_message);
-    if (valid_checkpoint_hashes.empty() && viewchange_message.sequence() != 0)
+    if (valid_checkpoint_hashes.empty() || viewchange_message.sequence() == 0)
     {
         LOG(error) << "is_valid_viewchange_message - the checkpoint is invalid";
         return false;

diff --git a/pbft/test/pbft_viewchange_test.cpp b/pbft/test/pbft_viewchange_test.cpp
@@ -427,4 +427,18 @@ namespace bzn
 
         EXPECT_EQ(this->pbft->view, 2U);
     }
+
+    TEST_F(pbft_viewchange_test, is_valid_viewchange_fails_if_no_checkpoint_yet)
+    {
+        // This test was written for KEP-902: is_valid_viewchange throws if no checkpoint yet
+        auto mock_crypto = this->build_pft_with_mock_crypto();
+        bzn_envelope original_message;
+        EXPECT_CALL(*mock_node, send_message(_, ResultOf(test::is_viewchange, Eq(true)))).WillRepeatedly(Invoke([&](const auto & /*endpoint*/, const auto &viewchange_env) { original_message = *viewchange_env; }));
+
+        this->pbft->handle_failure();
+
+        pbft_msg msg;
+        msg.ParseFromString(original_message.pbft());
+        EXPECT_FALSE(this->pbft->is_valid_viewchange_message(msg, original_message));
+    }
 }