BIP155: change when sendaddrv2 is to be sent

[vasild]

Mandate to send sendaddrv2 to the peer before sending our verack
to them.

This way we know that the peer does not support addrv2 if we did not
receive sendaddrv2 from them before receiving their verack.

[vasild]

If the implementation is to be changed instead, then this patch is to be applied to it:

change net_processing.cpp

diff --git i/src/net_processing.cpp w/src/net_processing.cpp
index 1b4a05f0b..e46cbe487 100644
--- i/src/net_processing.cpp
+++ w/src/net_processing.cpp
@@ -2361,15 +2361,12 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
         if (greatest_common_version >= WTXID_RELAY_VERSION) {
             m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::WTXIDRELAY));
         }
 
         m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::VERACK));
 
-        // Signal ADDRv2 support (BIP155).
-        m_connman.PushMessage(&pfrom, msg_maker.Make(NetMsgType::SENDADDRV2));
-
         pfrom.nServices = nServices;
         pfrom.SetAddrLocal(addrMe);
         {
             LOCK(pfrom.cs_SubVer);
             pfrom.cleanSubVer = cleanSubVer;
         }
@@ -2509,12 +2506,14 @@ void PeerManager::ProcessMessage(CNode& pfrom, const std::string& msg_type, CDat
             uint64_t nCMPCTBLOCKVersion = 2;
             if (pfrom.GetLocalServices() & NODE_WITNESS)
                 m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion));
             nCMPCTBLOCKVersion = 1;
             m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion));
         }
+        // Signal ADDRv2 support (BIP155).
+        m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::SENDADDRV2));
         pfrom.fSuccessfullyConnected = true;
         return;
     }
 
     // Feature negotiation of wtxidrelay should happen between VERSION and
     // VERACK, to avoid relay problems from switching after a connection is up

that would send sendaddrv2 at the same time as sending sendheaders and sendcmpct.

[maflcko]

A third option would be to use the same feature negotiation order that was used for wtxidrelay

[vasild]

A third option would be to use the same feature negotiation order that was used for wtxidrelay

That would be "sendaddrv2 SHOULD be sent before sending the verack message to the peer" and would require changing both the BIP and the code. Any of the 3 would work wrt the sendaddrv2 / addrv2 logic (= it does not matter when sendaddrv2 is sent).

[jonatack]

In the same way as wtxidrelay SGTM for being simpler rather than a different mechanism for each.

[vasild]

Changed to send it before sending verack (like wtxidrelay).

[laanwj]

ACK 50613bb

[benthecarman]

ACK 50613bb

[jonatack]

Concept ACK

[maflcko]

ACK c2c55cf

[vasild]

There are many ways to say it, I hope the current phrasing is clear enough and good for everybody.

[maflcko]

ACK e549ed3

[harding]

ACK e549ed3

Thanks for updating!

[jnewbery]

ACK e549ed3

[jonatack]

ACK e549ed3

[hebasto]

ACK e549ed3, I believe that the establishing of connection invariants in a such manner--in response to the version and prior to sending the verack--is the right way both for new addrv2 message and for other future features.

[maflcko]

According to the BIP process, this needs an ACK by the BIP champion @laanwj :)

[laanwj]

re-ACK e549ed3

[jaysonmald35]

Thank you