p2p: add validation checks for initial self-announcement #34297

1. 
   frankomosh commented at 8:26 am on January 15, 2026: contributor

   This is a follow up to #34146 . Adds validation check to the initial self-announcement code path. IsAddrCompatible() check can prevent sending non-routable addresses to peers that don’t support addrv2.
2. DrahtBot added the label P2P on Jan 15, 2026
3. 
   DrahtBot commented at 8:26 am on January 15, 2026: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34297.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	fjahr, Crypt-iQ, stratospher, sedited
   Stale ACK	0xB10C

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

4. 
   0xB10C commented at 12:30 pm on January 15, 2026: contributor

   Code Review ACK dea9d502d8c4cfb9fb5c9a3df1940e266b2a0146

   Having the same checks as before makes sense.

5. 
   0xB10C commented at 12:31 pm on January 15, 2026: contributor

   Some additional context for this PR: This change was suggested by reviewers in #34146 (review) but the PR was merged before that review was addressed (probably an oversight).
6. in src/net_processing.cpp:5524 in dea9d502d8

   5525-                if (peer.m_wants_addrv2) {
   5526-                    MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(self_announcement));
   5527-                } else {
   5528-                    MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(self_announcement));
   5529-                }
   5530+                if (local_addr.IsValid() && IsAddrCompatible(peer, local_addr)) {
   

   ---

   ---

   

   fjahr commented at 12:54 pm on January 15, 2026:

   I think checking IsAddrCompatible here is enough because GetLocalAddrForPeer only returns a service when IsRoutable is true and this includes an IsValid check.

   ---

   

   frankomosh commented at 1:13 pm on January 15, 2026:

   Thanks, have updated it
7. frankomosh force-pushed on Jan 15, 2026
8. in src/net_processing.cpp:5531 in 5392ee0321

   5532+                    if (peer.m_wants_addrv2) {
   5533+                        MakeAndPushMessage(node, NetMsgType::ADDRV2, CAddress::V2_NETWORK(self_announcement));
   5534+                    } else {
   5535+                        MakeAndPushMessage(node, NetMsgType::ADDR, CAddress::V1_NETWORK(self_announcement));
   5536+                    }
   5537+             }
   

   ---

   ---

   

   fjahr commented at 1:31 pm on January 15, 2026:

   nit: The indentation got messed up here it seems

   ---

   

   frankomosh commented at 1:46 pm on January 15, 2026:

   Fixed now. Thanks
9. 
   fjahr commented at 1:31 pm on January 15, 2026: contributor

   utACK 5392ee03216d45547e62866f2063f6d200662857

   Thanks for also fixing the space ;)

10. DrahtBot requested review from 0xB10C on Jan 15, 2026
11. p2p: add validation check for initial self-announcement

    The direct send path for the initial self-announcement was bypassing
    IsAddrCompatible() check that PushAddress() performs

    6a8dbf9b93
12. frankomosh force-pushed on Jan 15, 2026
13. DrahtBot added the label CI failed on Jan 15, 2026
14. 
    fjahr commented at 1:46 pm on January 15, 2026: contributor

    utACK 6a8dbf9b9352db48580cd81c8dac027f3138fedf
15. DrahtBot removed the label CI failed on Jan 15, 2026
16. 
    Crypt-iQ commented at 9:15 pm on January 16, 2026: contributor

    crACK 6a8dbf9b9352db48580cd81c8dac027f3138fedf

    I haven’t tested, but I think that onion / i2p / cjdns addresses would have been sent as all zero per logic in SerializeV1Array.

17. 
    stratospher commented at 2:13 pm on January 21, 2026: contributor

    ACK 6a8dbf9. preserves the existing behaviour. also learnt that Addr-fetch ADDR processing logic allows receiving a self-announcement with 1 address without disconnecting and won’t be affected.
18. sedited approved
19. 
    sedited commented at 10:38 am on January 22, 2026: contributor

    ACK 6a8dbf9b9352db48580cd81c8dac027f3138fedf
20. sedited merged this on Jan 22, 2026
21. sedited closed this on Jan 22, 2026

    ---

22. frankomosh deleted the branch on Jan 22, 2026

Contributors
frankomosh DrahtBot 0xB10C fjahr Crypt-iQ stratospher sedited

Review Requested
0xB10C

Labels
P2P