[p2p] Small follow-ups to 21528 #22618

1. 
   amitiuttarwar commented at 4:49 pm on August 3, 2021: contributor

   Adds a release note & addresses this review comment to make expectations more explicit.
2. amitiuttarwar renamed this:
   [p2p] Small followups to #21528
   [p2p] Small follow-ups to 21528
   on Aug 3, 2021
3. 
   MarcoFalke commented at 5:03 pm on August 3, 2021: member

   Missing #21528 (review) ? :sweat_smile:
4. amitiuttarwar force-pushed on Aug 3, 2021
5. 
   amitiuttarwar commented at 5:19 pm on August 3, 2021: contributor

   @MarcoFalke fixed 😛
6. DrahtBot added the label P2P on Aug 3, 2021
7. in test/functional/p2p_addr_relay.py:182 in 7ac9c1f22b outdated

   178@@ -179,7 +179,7 @@ def relay_tests(self):
   179         # of the outbound peer which is often sent before the GETADDR response.
   180         assert_equal(inbound_peer.num_ipv4_received, 0)
   181 
   182-        # Send an empty ADDR message to intialize address relay on this connection.
   183+        # Send an empty ADDR message to initialize address relay on this connection.
   

   ---

   ---

   

   jonatack commented at 7:50 pm on August 3, 2021:

   nit, maybe fix the other outstanding one too:

   0$ test/lint/lint-spelling.sh 
   1src/wallet/test/spend_tests.cpp:57: neccesary ==> necessary
   2test/functional/p2p_addr_relay.py:182: intialize ==> initialize
   

   ---

   

   amitiuttarwar commented at 7:37 pm on August 4, 2021:

   done
8. in doc/release-notes.md:60 in 7ac9c1f22b outdated

   56@@ -57,6 +57,10 @@ Notable changes
   57 P2P and network changes
   58 -----------------------
   59 
   60+- Nodes will no longer forward address gossip to inbound peers by default. The
   

   ---

   ---

   

   jonatack commented at 7:55 pm on August 3, 2021:

   ideas, for better or worse

   0-- Nodes will no longer forward address gossip to inbound peers by default. The
   1-  peer will become eligible for address gossip upon sending an ADDR, ADDRV2, or
   2-  GETADDR message. (#21528)
   3+- By default, bitcoind no longer rumours addresses to inbound peers. They only
   4+  become eligible for address gossip after sending an ADDR, ADDRV2, or GETADDR
   5+  message. (#21528)
   

   ---

   

   amitiuttarwar commented at 7:38 pm on August 4, 2021:

   took some of these suggestions 👍
9. 
   jonatack commented at 7:59 pm on August 3, 2021: member

   ACK 7ac9c1f22b15509ad1775ecd707ea8615c05d306

   Feel free to ignore the optional ideas below.

10. in src/net_processing.cpp:3749 in 7ac9c1f22b outdated

    3743@@ -3744,7 +3744,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
    3744             return;
    3745         }
    3746 
    3747-        SetupAddressRelay(pfrom, *peer);
    3748+        Assume(SetupAddressRelay(pfrom, *peer));
    

    ---

    ---

    

    jonatack commented at 8:01 pm on August 3, 2021:

    Good idea.

    ---

    

    jnewbery commented at 9:13 am on August 4, 2021:

    The commit log is a good explanation of why you can make this assumption. Perhaps also add something similar as a code comment:

    0        // SetupAddressRelay will never fail since this must be an outbound connection.
    1        Assume(SetupAddressRelay(pfrom, *peer));
    

    ---

    

    MarcoFalke commented at 2:39 pm on August 4, 2021:

        // SetupAddressRelay will never fail since this must be an outbound connection.
    

    it can only be an inbound connection ;)

    ---

    

    jnewbery commented at 2:52 pm on August 4, 2021:

    oops!

    ---

    

    amitiuttarwar commented at 7:38 pm on August 4, 2021:

    added a comment
11. 
    Zero-1729 commented at 11:54 pm on August 3, 2021: contributor

    ACK 7ac9c1f, esp. with @jonatack’s suggestions above (e.g. spelling and release notes).
12. 
    MarcoFalke commented at 5:52 am on August 4, 2021: member

    cr ACK 7ac9c1f22b15509ad1775ecd707ea8615c05d306
13. 
    jnewbery commented at 9:13 am on August 4, 2021: member

    ACK 7ac9c1f22b15509ad1775ecd707ea8615c05d306
14. [docs] Add release notes for #21528

    And fix a typo in the test.

    aa79c91260
15. [net_processing] Provide debug error if code assumptions change.

    Currently, this call to SetupAddressRelay will never return false because of
    the previous guard that returns early if the peer is not an inbound connection.
    Rather than implicitly relying on this guarantee, throw an error in the debug
    build if it ever changes.

    9778b0fec1
16. amitiuttarwar force-pushed on Aug 4, 2021
17. 
    amitiuttarwar commented at 7:39 pm on August 4, 2021: contributor

    thanks for reviews! updated to incorporate feedback
18. 
    Zero-1729 commented at 7:42 pm on August 4, 2021: contributor

    re-ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81
19. 
    jonatack commented at 7:11 am on August 5, 2021: member

    ACK 9778b0fec13c047a4c7f3ae425d044da26eadc81
20. MarcoFalke merged this on Aug 5, 2021
21. MarcoFalke closed this on Aug 5, 2021

    ---

22. sidhujag referenced this in commit a5623f1bbc on Aug 5, 2021
23. Fabcien referenced this in commit e4ef397e3c on Feb 1, 2022
24. DrahtBot locked this on Aug 16, 2022

Contributors
amitiuttarwar MarcoFalke jonatack jnewbery Zero-1729

Labels
P2P