doc: comment “add only reachable addresses to addrman” #26040

1. 
   kristapsk commented at 6:36 pm on September 7, 2022: contributor

   Proposed by Sjors during review of #25678, was likely just missed, as it also for me looks a code where comment will not hurt.

   #25678 (review)

2. DrahtBot added the label Docs on Sep 7, 2022
3. in src/net.cpp:1651 in 9094951cd0 outdated

   1647@@ -1648,6 +1648,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
   1648 
   1649             if (add_fixed_seeds_now) {
   1650                 std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())};
   1651+                // We will not make outgoing connection to peers that are unreachable
   

   ---

   ---

   

   mzumsande commented at 10:12 pm on September 7, 2022:

   nit: connections

   ---

   

   kristapsk commented at 10:20 pm on September 7, 2022:

   Thanks, fixed!
4. 
   mzumsande commented at 10:13 pm on September 7, 2022: contributor

   ACK 9094951cd09c62aa6fd2214f90e3eb87ca2d98af
5. kristapsk force-pushed on Sep 7, 2022
6. in src/net.cpp:1653 in a7b4f97b01 outdated

   1647@@ -1648,6 +1648,9 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
   1648 
   1649             if (add_fixed_seeds_now) {
   1650                 std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())};
   1651+                // We will not make outgoing connections to peers that are unreachable
   1652+                // (e.g. because of -onlynet configuration).
   1653+                // Therefore, it makes no sense to add them to addrman in the first place.
   

   ---

   ---

   

   Zero-1729 commented at 0:03 am on September 8, 2022:

   Slight rewording nit

   0                // Therefore, we do not add them to addrman in the first place.
   

   ---

   

   Sjors commented at 3:32 pm on September 8, 2022:

   or: , so don't add them to addrman.
7. 
   Zero-1729 commented at 0:03 am on September 8, 2022: contributor

   crACK a7b4f97b016eb7728d6381765269d228a4093492
8. vasild approved
9. 
   vasild commented at 8:05 am on September 8, 2022: contributor

   ACK a7b4f97b016eb7728d6381765269d228a4093492
10. 
    Sjors commented at 3:34 pm on September 8, 2022: member

    We should also mention the caveat in #26035, unless that’s fixed.
11. 
    kristapsk commented at 10:39 pm on September 8, 2022: contributor

    We should also mention the caveat in #26035, unless that’s fixed.

    I would hope “until it’s fixed”. :)

12. doc: comment "add only reachable addresses to addrman" ce42570266
13. kristapsk force-pushed on Sep 8, 2022
14. 
    kristapsk commented at 10:42 pm on September 8, 2022: contributor

    Addressed additional review comments.
15. 
    amovfx commented at 1:26 am on September 9, 2022: none

    I’m confused about the broken checks, all I see is a doc string.
16. in src/net.cpp:1651 in ce42570266

    1647@@ -1648,6 +1648,14 @@ void CConnman::ThreadOpenConnections(const std::vector<std::string> connect)
    1648 
    1649             if (add_fixed_seeds_now) {
    1650                 std::vector<CAddress> seed_addrs{ConvertSeeds(Params().FixedSeeds())};
    1651+                // We will not make outgoing connections to peers that are unreachable
    

    ---

    ---

    

    Sjors commented at 11:20 am on September 9, 2022:

    Maybe it wants:

    0/* Blah
    1 * blah
    2 * blah */
    

    ---

    

    kristapsk commented at 12:12 pm on September 9, 2022:

    No, there are two random funcional tests failing in each failed CI run for some reason (different one in each case), should not be related with this change.

    ---

    

    mzumsande commented at 3:11 pm on September 9, 2022:

    The second one is new to me, I opened #26051 - but yes, obviously unrelated.
17. vasild approved
18. 
    vasild commented at 11:47 am on September 9, 2022: contributor

    ACK ce4257026622287c8c981fb932681730a3c6387f
19. 
    Zero-1729 commented at 1:17 pm on September 9, 2022: contributor

    re-ACK ce4257026622287c8c981fb932681730a3c6387f
20. 
    mzumsande commented at 3:13 pm on September 9, 2022: contributor

    ACK ce4257026622287c8c981fb932681730a3c6387f
21. MarcoFalke merged this on Sep 9, 2022
22. MarcoFalke closed this on Sep 9, 2022

    ---

23. kristapsk deleted the branch on Sep 9, 2022
24. sidhujag referenced this in commit bdc177ef80 on Sep 11, 2022
25. bitcoin locked this on Sep 9, 2023

Contributors
kristapsk mzumsande Zero-1729 Sjors vasild amovfx

Labels
Docs