Merge pull request #751 from evoskuil/master

Fix version subscription race.

Author: evoskuil

include/bitcoin/network/protocols/protocol_version_70016.hpp

@@ -50,7 +50,7 @@ class BCT_API protocol_version_70016
     void shake(result_handler&& handle_event) NOEXCEPT override;
 
 protected:
-    void handle_send_version(const code& ec) NOEXCEPT override;
+    ////void handle_send_version(const code& ec) NOEXCEPT override;
     bool handle_receive_acknowledge(const code& ec,
         const messages::peer::version_acknowledge::cptr& message) NOEXCEPT override;
     virtual bool handle_receive_send_address_v2(const code& ec,

src/protocols/protocol_version_70016.cpp

@@ -72,28 +72,34 @@ void protocol_version_70016::shake(result_handler&& handle_event) NOEXCEPT
     if (started())
         return;
 
+    SUBSCRIBE_CHANNEL(send_address_v2, handle_receive_send_address_v2, _1, _2);
+    SUBSCRIBE_CHANNEL(witness_tx_id_relay, handle_receive_witness_tx_id_relay, _1, _2);
+
     // Protocol versions are cumulative, but reject is optional.
     if (reject_)
     {
         protocol_version_70002::shake(std::move(handle_event));
         return;
     }
-    
+
     protocol_version_70001::shake(std::move(handle_event));
 }
 
 // Incoming [send_address_v2     => negotiated state change].
 // Incoming [witness_tx_id_relay => negotiated state change].
 // ----------------------------------------------------------------------------
 
-void protocol_version_70016::handle_send_version(const code& ec) NOEXCEPT
-{
-    BC_ASSERT_MSG(stranded(), "protocol_version_70016");
-
-    SUBSCRIBE_CHANNEL(send_address_v2, handle_receive_send_address_v2, _1, _2);
-    SUBSCRIBE_CHANNEL(witness_tx_id_relay, handle_receive_witness_tx_id_relay, _1, _2);
-    protocol_version_70002::handle_send_version(ec);
-}
+////void protocol_version_70016::handle_send_version(const code& ec) NOEXCEPT
+////{
+////    BC_ASSERT_MSG(stranded(), "protocol_version_70016");
+////
+////    // BUGBUG: subtle race here, as the peer can send these after receiving
+////    // version but before this methods is invoked, despite both being stranded.
+////    // The resolution is to subscribe these in method where version is sent.
+////    SUBSCRIBE_CHANNEL(send_address_v2, handle_receive_send_address_v2, _1, _2);
+////    SUBSCRIBE_CHANNEL(witness_tx_id_relay, handle_receive_witness_tx_id_relay, _1, _2);
+////    protocol_version_70002::handle_send_version(ec);
+////}
 
 bool protocol_version_70016::handle_receive_acknowledge(const code& ec,
     const messages::peer::version_acknowledge::cptr& message) NOEXCEPT