KEP-980 New node retries join request

pbft/pbft.cpp

@@ -50,6 +50,7 @@ pbft::pbft(
     , io_context(io_context)
     , audit_heartbeat_timer(this->io_context->make_unique_steady_timer())
     , new_config_timer(this->io_context->make_unique_steady_timer())
+    , join_retry_timer(this->io_context->make_unique_steady_timer())
     , crypto(std::move(crypto))
     , operation_manager(std::move(operation_manager))
 {
@@ -458,28 +459,13 @@ pbft::handle_join_or_leave(const pbft_membership_msg& msg, std::shared_ptr<bzn::
 }
 
 void
-pbft::handle_join_response(const pbft_membership_msg& msg)
+pbft::handle_join_response(const pbft_membership_msg& /*msg*/)
 {
     if (this->in_swarm == swarm_status::joining)
     {
-        if (msg.result())
-        {
-            this->in_swarm = swarm_status::waiting;
-            LOG(debug) << "Successfully joined the swarm, waiting for NEW_VIEW message...";
-        }
-        else
-        {
-            // since we're not persisting configuration to disk, we could be in one of two situations:
-            // either we've been rejected, or we restarted and the swarm still thinks we're a member.
-            // for now (since there's no blacklist yet) we'll assume the latter and try to continue.
-            // there is a potential for problems here if we don't have the latest configuration.
-            // TODO: change this to abort once we're persisting configuration
-            LOG(error) << "Request to join swarm rejected - did we exit and restart?";
-            this->in_swarm = swarm_status::joined;
-
-//            LOG(error) << "Request to join swarm rejected. Aborting...";
-//            throw (std::runtime_error("Request to join swarm rejected."));
-        }
+        this->join_retry_timer->cancel();
+        this->in_swarm = swarm_status::waiting;
+        LOG(debug) << "Successfully joined the swarm, waiting for NEW_VIEW message...";
     }
     else
     {
@@ -1823,6 +1809,24 @@ pbft::join_swarm()
     this->node->send_message(make_endpoint(this->current_peers()[selected]), msg_ptr, false);
 
     this->in_swarm = swarm_status::joining;
+    this->join_retry_timer->expires_from_now(JOIN_RETRY_INTERVAL);
+    this->join_retry_timer->async_wait(
+        std::bind(&pbft::handle_join_retry_timeout, shared_from_this(), std::placeholders::_1));
+}
+
+void
+pbft::handle_join_retry_timeout(const boost::system::error_code& ec)
+{
+    if (ec == boost::asio::error::operation_aborted)
+    {
+        return;
+    }
+
+    if (ec)
+    {
+        LOG(error) << "handle_new_join_retry_timeout error: " << ec.message();
+        return;
+    }
 
-    // TODO: set timer and retry with different peer if we don't get a response - KEP-980
+    this->join_swarm();
 }

pbft/pbft.hpp

@@ -35,6 +35,7 @@ namespace
 {
     const std::chrono::milliseconds HEARTBEAT_INTERVAL{std::chrono::milliseconds(5000)};
     const std::chrono::seconds NEW_CONFIG_INTERVAL{std::chrono::seconds(30)};
+    const std::chrono::seconds JOIN_RETRY_INTERVAL{std::chrono::seconds(30)};
     const std::string INITIAL_CHECKPOINT_HASH = "<null db state>";
     const uint64_t CHECKPOINT_INTERVAL = 100; //TODO: KEP-574
     const double HIGH_WATER_INTERVAL_IN_CHECKPOINTS = 2.0; //TODO: KEP-574
@@ -154,6 +155,7 @@ namespace bzn
 
         void handle_audit_heartbeat_timeout(const boost::system::error_code& ec);
         void handle_new_config_timeout(const boost::system::error_code& ec);
+        void handle_join_retry_timeout(const boost::system::error_code& ec);
 
         void notify_audit_failure_detected();
 
@@ -225,6 +227,7 @@ namespace bzn
 
         std::unique_ptr<bzn::asio::steady_timer_base> audit_heartbeat_timer;
         std::unique_ptr<bzn::asio::steady_timer_base> new_config_timer;
+        std::unique_ptr<bzn::asio::steady_timer_base> join_retry_timer;
 
         bool audit_enabled = true;
 

pbft/test/pbft_join_leave_test.cpp

@@ -347,12 +347,14 @@ namespace bzn
         auto mock_io_context2 = std::make_shared<NiceMock<bzn::asio::Mockio_context_base >>();
         auto audit_heartbeat_timer2 = std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
         auto new_config_timer2 = std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
+        auto join_retry_timer2 = std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
         auto mock_service2 = std::make_shared<NiceMock<bzn::mock_pbft_service_base>>();
         auto mock_options = std::make_shared<bzn::mock_options_base>();
         auto manager2 = std::make_shared<bzn::pbft_operation_manager>();
         EXPECT_CALL(*(mock_io_context2), make_unique_steady_timer())
-            .Times(AtMost(2)).WillOnce(Invoke([&](){return std::move(audit_heartbeat_timer2);}))
-            .WillOnce(Invoke([&](){return std::move(new_config_timer2);}));
+            .Times(AtMost(3)).WillOnce(Invoke([&](){return std::move(audit_heartbeat_timer2);}))
+            .WillOnce(Invoke([&](){return std::move(new_config_timer2);}))
+            .WillOnce(Invoke([&](){return std::move(join_retry_timer2);}));
         EXPECT_CALL(*mock_options, get_uuid()).WillRepeatedly(Return("uuid2"));
         EXPECT_CALL(*mock_options, get_simple_options()).WillRepeatedly(ReturnRef(this->options->get_simple_options()));
         auto pbft2 = std::make_shared<bzn::pbft>(mock_node2, mock_io_context2, TEST_PEER_LIST, mock_options, mock_service2,
@@ -469,12 +471,14 @@ namespace bzn
         auto mock_io_context3 = std::make_shared<NiceMock<bzn::asio::Mockio_context_base >>();
         auto audit_heartbeat_timer3 = std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
         auto new_config_timer3 = std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
+        auto join_retry_timer3 = std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
         auto mock_service3 = std::make_shared<NiceMock<bzn::mock_pbft_service_base>>();
         auto mock_options3 = std::make_shared<bzn::mock_options_base>();
         auto manager3 = std::make_shared<bzn::pbft_operation_manager>();
         EXPECT_CALL(*(mock_io_context3), make_unique_steady_timer())
-            .Times(AtMost(2)).WillOnce(Invoke([&](){return std::move(audit_heartbeat_timer3);}))
-            .WillOnce(Invoke([&](){return std::move(new_config_timer3);}));
+            .Times(AtMost(3)).WillOnce(Invoke([&](){return std::move(audit_heartbeat_timer3);}))
+            .WillOnce(Invoke([&](){return std::move(new_config_timer3);}))
+            .WillOnce(Invoke([&](){return std::move(join_retry_timer3);}));
         EXPECT_CALL(*mock_options3, get_uuid()).WillRepeatedly(Return("uuid3"));
         EXPECT_CALL(*mock_options3, get_simple_options()).WillRepeatedly(ReturnRef(this->options->get_simple_options()));
         auto pbft3 = std::make_shared<bzn::pbft>(mock_node3, mock_io_context3, TEST_PEER_LIST, mock_options3, mock_service3,
@@ -520,7 +524,7 @@ namespace bzn
                 response.set_result(true);
                 this->handle_membership_message(test::wrap_pbft_membership_msg(response, ""));
             }));
-
+        EXPECT_CALL(*this->join_retry_timer, async_wait(_));
         this->build_pbft();
     }
 

pbft/test/pbft_test_common.cpp

@@ -55,7 +55,7 @@ namespace bzn::test
                         ));
 
         EXPECT_CALL(*(this->mock_io_context), make_unique_steady_timer())
-                .Times(AtMost(2))
+                .Times(AtMost(3))
                 .WillOnce(
                         Invoke(
                                 [&]()
@@ -65,6 +65,11 @@ namespace bzn::test
                     Invoke(
                         [&]()
                         { return std::move(this->new_config_timer); }
+                    ))
+                .WillOnce(
+                    Invoke(
+                        [&]()
+                        { return std::move(this->join_retry_timer); }
                     ));
 
         EXPECT_CALL(*(this->audit_heartbeat_timer), async_wait(_))

pbft/test/pbft_test_common.hpp

@@ -79,6 +79,9 @@ namespace bzn::test
         std::unique_ptr<bzn::asio::Mocksteady_timer_base> new_config_timer =
                 std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
 
+        std::unique_ptr<bzn::asio::Mocksteady_timer_base> join_retry_timer =
+            std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
+
         bzn::asio::wait_handler audit_heartbeat_timer_callback;
         bzn::asio::wait_handler new_config_timer_callback;
 

pbft/test/pbft_viewchange_test.cpp

@@ -334,14 +334,18 @@ namespace bzn
                 std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
         std::unique_ptr<bzn::asio::Mocksteady_timer_base> new_config_timer2 =
             std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
+        std::unique_ptr<bzn::asio::Mocksteady_timer_base> join_retry_timer2 =
+            std::make_unique<NiceMock<bzn::asio::Mocksteady_timer_base >>();
         std::shared_ptr<bzn::mock_pbft_service_base> mock_service2 =
                 std::make_shared<NiceMock<bzn::mock_pbft_service_base>>();
 
         EXPECT_CALL(*(mock_io_context2), make_unique_steady_timer())
-            .Times(AtMost(2)).WillOnce(Invoke([&]()
+            .Times(AtMost(3)).WillOnce(Invoke([&]()
             { return std::move(audit_heartbeat_timer2); }))
             .WillOnce(Invoke([&]()
-            { return std::move(new_config_timer2); }));
+            { return std::move(new_config_timer2); }))
+            .WillOnce(Invoke([&]()
+            { return std::move(join_retry_timer2); }));
 
         auto mock_options = std::make_shared<bzn::mock_options_base>();