KEP-969: Send response to join request

bluzelle · Jan 9, 2019 · 9ace891 · 9ace891
1 parent 201b5ba
commit 9ace891
Show file tree

Hide file tree

Showing 4 changed files with 28 additions and 24 deletions.
diff --git a/pbft/pbft.cpp b/pbft/pbft.cpp
@@ -188,7 +188,7 @@ pbft::handle_membership_message(const bzn_envelope& msg, std::shared_ptr<bzn::se
                 this->forward_request_to_primary(msg);
                 return;
             }
-            this->handle_join_or_leave(inner_msg, session);
+            this->handle_join_or_leave(inner_msg, session, hash);
             break;
         case PBFT_MMSG_GET_STATE:
             this->handle_get_state(inner_msg, std::move(session));
@@ -408,7 +408,8 @@ pbft::handle_commit(const pbft_msg& msg, const bzn_envelope& original_msg)
 }
 
 void
-pbft::handle_join_or_leave(const pbft_membership_msg& msg, std::shared_ptr<bzn::session_base> session)
+pbft::handle_join_or_leave(const pbft_membership_msg& msg, std::shared_ptr<bzn::session_base> session
+    , const std::string& msg_hash)
 {
     if (msg.has_peer_info())
     {
@@ -447,7 +448,7 @@ pbft::handle_join_or_leave(const pbft_membership_msg& msg, std::shared_ptr<bzn::
         }
 
         this->configurations.add(config);
-        this->broadcast_new_configuration(config);
+        this->broadcast_new_configuration(config, msg.type() == PBFT_MMSG_JOIN ? msg_hash : "");
     }
     else
     {
@@ -663,17 +664,22 @@ pbft::do_committed(const std::shared_ptr<pbft_operation>& op)
         }
 
         // send response to new node
-        auto session = op->session();
-        if (session && session->is_open())
+        auto session_it = this->sessions_waiting_on_forwarded_requests.find(op->get_config_request().join_request_hash());
+        if (session_it != this->sessions_waiting_on_forwarded_requests.end())
         {
-            pbft_membership_msg response;
-            response.set_type(PBFT_MMSG_JOIN_RESPONSE);
-            response.set_result(true);
-            auto env = this->wrap_message(response);
-            session->send_message(std::make_shared<std::string>(env.SerializeAsString()), true);
+            if (session_it->second->is_open())
+            {
+                pbft_membership_msg response;
+                response.set_type(PBFT_MMSG_JOIN_RESPONSE);
+                response.set_result(true);
+                auto env = this->wrap_message(response);
+                session_it->second->send_message(std::make_shared<std::string>(env.SerializeAsString()), true);
+
+                // TODO: start timer for sending viewchange KEP-825
 
-            // TODO: start timer for sending viewchange KEP-825
+            }
 
+            this->sessions_waiting_on_forwarded_requests.erase(session_it);
         }
         else
         {
@@ -1559,10 +1565,11 @@ pbft::get_peer_by_uuid(const std::string& uuid) const
 }
 
 void
-pbft::broadcast_new_configuration(pbft_configuration::shared_const_ptr config)
+pbft::broadcast_new_configuration(pbft_configuration::shared_const_ptr config, const std::string& join_request_hash)
 {
     auto cfg_msg = new pbft_config_msg;
     cfg_msg->set_configuration(config->to_string());
+    cfg_msg->set_join_request_hash(join_request_hash);
     bzn_envelope req;
     req.set_pbft_internal_request(cfg_msg->SerializeAsString());
 
@@ -1700,10 +1707,10 @@ pbft::join_swarm()
     // are we already in the peers list?
     if (this->is_peer(this->uuid))
     {
+        this->in_swarm = true;
         return;
     }
 
-#if 0 // disabling dynamic peering for now
     auto info = new pbft_peer_info;
     info->set_host(this->options->get_listener().address().to_string());
     info->set_port(this->options->get_listener().port());
@@ -1724,9 +1731,5 @@ pbft::join_swarm()
     auto msg_ptr = std::make_shared<bzn_envelope>(this->wrap_message(join_msg));
     this->node->send_message(make_endpoint(this->current_peers()[selected]), msg_ptr, false);
 
-    // TODO: set timer and retry with different peer if we don't get a response
-#else
-    LOG(fatal) << "This node is not a member of the swarm, exiting.";
-    throw std::runtime_error("This node is not a member of the swarm, exiting.");
-#endif
+    // TODO: set timer and retry with different peer if we don't get a response - KEP-980
 }
diff --git a/pbft/pbft.hpp b/pbft/pbft.hpp
@@ -124,7 +124,7 @@ namespace bzn
         void handle_prepare(const pbft_msg& msg, const bzn_envelope& original_msg);
         void handle_commit(const pbft_msg& msg, const bzn_envelope& original_msg);
         void handle_checkpoint(const pbft_msg& msg, const bzn_envelope& original_msg);
-        void handle_join_or_leave(const pbft_membership_msg& msg, std::shared_ptr<bzn::session_base> session);
+        void handle_join_or_leave(const pbft_membership_msg& msg, std::shared_ptr<bzn::session_base> session, const std::string& msg_hash);
         void handle_join_response(const pbft_membership_msg& msg);
         void handle_get_state(const pbft_membership_msg& msg, std::shared_ptr<bzn::session_base> session) const;
         void handle_set_state(const pbft_membership_msg& msg);
@@ -173,7 +173,7 @@ namespace bzn
         std::shared_ptr<const std::vector<bzn::peer_address_t>> current_peers_ptr() const;
         const std::vector<bzn::peer_address_t>& current_peers() const;
         const peer_address_t& get_peer_by_uuid(const std::string& uuid) const;
-        void broadcast_new_configuration(pbft_configuration::shared_const_ptr config);
+        void broadcast_new_configuration(pbft_configuration::shared_const_ptr config, const std::string& join_request_hash);
         bool is_configuration_acceptable_in_new_view(hash_t config_hash);
         bool move_to_new_configuration(hash_t config_hash);
         bool proposed_config_is_acceptable(std::shared_ptr<pbft_configuration> config);

diff --git a/pbft/test/pbft_join_leave_test.cpp b/pbft/test/pbft_join_leave_test.cpp
@@ -391,7 +391,7 @@ namespace bzn
         EXPECT_EQ(this->configurations().get(current_config->get_hash()), nullptr);
     }
 
-    TEST_F(pbft_join_leave_test, DISABLED_node_not_in_swarm_asks_to_join)
+    TEST_F(pbft_join_leave_test, node_not_in_swarm_asks_to_join)
     {
         this->uuid = "somenode";
         EXPECT_CALL(*this->mock_node, send_message(_, ResultOf(test::is_join, Eq(true)), _))
@@ -414,7 +414,7 @@ namespace bzn
         this->build_pbft();
     }
 
-    TEST_F(pbft_join_leave_test, DISABLED_new_node_can_join_swarm)
+    TEST_F(pbft_join_leave_test, new_node_can_join_swarm)
     {
         this->build_pbft();
 
@@ -495,7 +495,7 @@ namespace bzn
         this->handle_membership_message(test::wrap_pbft_membership_msg(join_msg, this->pbft->get_uuid()), this->mock_session);
     }
 
-    TEST_F(pbft_join_leave_test, DISABLED_existing_node_cant_join_swarm)
+    TEST_F(pbft_join_leave_test, existing_node_cant_join_swarm)
     {
         this->build_pbft();
 
@@ -523,7 +523,7 @@ namespace bzn
         this->handle_membership_message(test::wrap_pbft_membership_msg(join_msg, peer.uuid), this->mock_session);
     }
 
-    TEST_F(pbft_join_leave_test, DISABLED_node_handles_unsolicited_join_rejection)
+    TEST_F(pbft_join_leave_test, node_handles_unsolicited_join_rejection)
     {
         this->build_pbft();
 

diff --git a/proto/pbft.proto b/proto/pbft.proto
@@ -57,6 +57,7 @@ message pbft_config_msg
 {
     // for new_config
     string configuration = 1;
+    bytes join_request_hash = 2;
 }
 
 enum pbft_msg_type