KEP-1618 - don't add requests to newview if we don't have them

bluzelle · Aug 2, 2019 · 37f385c · 37f385c
1 parent a1d210f
commit 37f385c
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 5 deletions.
diff --git a/pbft/pbft.cpp b/pbft/pbft.cpp
@@ -1307,7 +1307,7 @@ pbft::is_valid_newview_message(const pbft_msg& theirs, const bzn_envelope& origi
         viewchange_envelopes_from_senders[viewchange_env.sender()] = viewchange_env;
     }
 
-    pbft_msg ours = this->build_newview(theirs.view(), viewchange_envelopes_from_senders);
+    pbft_msg ours = this->build_newview(theirs.view(), viewchange_envelopes_from_senders, false);
     if (ours.pre_prepare_messages_size() != theirs.pre_prepare_messages_size())
     {
         LOG(error) << "is_valid_newview_message - expected " << ours.pre_prepare_messages_size()
@@ -1425,7 +1425,8 @@ pbft::make_newview(
 }
 
 pbft_msg
-pbft::build_newview(uint64_t new_view, const std::map<uuid_t,bzn_envelope>& viewchange_envelopes_from_senders) const
+pbft::build_newview(uint64_t new_view, const std::map<uuid_t,bzn_envelope>& viewchange_envelopes_from_senders
+    , bool attach_reqs) const
 {
     //  Computing O (set of new preprepares for new-view message)
     std::map<uint64_t, bzn_envelope> pre_prepares;
@@ -1470,7 +1471,7 @@ pbft::build_newview(uint64_t new_view, const std::map<uuid_t,bzn_envelope>& view
             }
 
             auto env = this->wrap_message(pre_prepare);
-            if (pre_prepare.request_type() == pbft_request_type::PBFT_REQUEST_PAYLOAD)
+            if (pre_prepare.request_type() == pbft_request_type::PBFT_REQUEST_PAYLOAD && attach_reqs)
             {
                 auto config = this->configurations->get(old_view);
                 if (!config)
@@ -1480,7 +1481,15 @@ pbft::build_newview(uint64_t new_view, const std::map<uuid_t,bzn_envelope>& view
                 }
                 auto op = this->operation_manager->find_or_construct(old_view, pre_prepare.sequence()
                     , pre_prepare.request_hash(), config->get_peers());
-                *(env.add_piggybacked_requests()) = op->get_request();
+                if (op->has_request())
+                {
+                    *(env.add_piggybacked_requests()) = op->get_request();
+                }
+                else
+                {
+                    LOG(error) << "No request found for operation " << pre_prepare.sequence() << " in view "
+                            << old_view;
+                }
             }
             pre_prepares[pre_prepare.sequence()] = env;
         }

diff --git a/pbft/pbft.hpp b/pbft/pbft.hpp
@@ -229,7 +229,7 @@ namespace bzn
         void initiate_viewchange();
         std::shared_ptr<bzn_envelope> make_viewchange(uint64_t new_view, uint64_t n, const std::unordered_map<bzn::uuid_t, std::string>& stable_checkpoint_proof, const std::map<uint64_t, std::shared_ptr<bzn::pbft_operation>>& prepared_operations);
         pbft_msg make_newview(uint64_t new_view_index,  const std::map<uuid_t,bzn_envelope>& viewchange_envelopes_from_senders, const std::map<uint64_t, bzn_envelope>& pre_prepare_messages) const;
-        pbft_msg build_newview(uint64_t new_view, const std::map<uuid_t,bzn_envelope>& viewchange_envelopes_from_senders) const;
+        pbft_msg build_newview(uint64_t new_view, const std::map<uuid_t,bzn_envelope>& viewchange_envelopes_from_senders, bool attach_reqs = false) const;
         std::map<bzn::checkpoint_t , std::set<bzn::uuid_t>> validate_and_extract_checkpoint_hashes(const pbft_msg &viewchange_message) const;
         void save_checkpoint(const pbft_msg& msg);
         void fill_in_missing_pre_prepares(uint64_t max_checkpoint_sequence, uint64_t new_view, std::map<uint64_t, bzn_envelope>& pre_prepares) const;