test: BIP324: add checks for v1 prefix matching / wrong network magic detection #28588

1. 
   theStack commented at 3:04 PM on October 4, 2023: contributor

   This PR adds missing test coverage for the detection of incoming v1 connections and wrong network magic on BIP324-enabled (i.e. running with -v2transport=1) nodes. Both checks are using prefix sizes of 16 bytes (previously only 12 bytes were used for the v1 prefix matching, which was fixed by PR #28577).

2. 
   DrahtBot commented at 3:04 PM on October 4, 2023: contributor

   <!--e57a25ab6845829454e8d69fc972939a-->

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

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	Sjors, MarcoFalke
   Stale ACK	sipa

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

3. DrahtBot added the label Tests on Oct 4, 2023
4. fanquake requested review from sipa on Oct 4, 2023
5. 
   sipa commented at 3:23 PM on October 4, 2023: member

   utACK c8581d449efcd0c7e45a0a2476a6dc356efd45dc

   Happy to rebase #28577 on this if it goes in first.

6. DrahtBot removed review request from sipa on Oct 4, 2023
7. fanquake requested review from vasild on Oct 4, 2023
8. test: BIP324: add checks for v1 prefix matching / wrong network magic detection e1308967e1
9. theStack force-pushed on Oct 4, 2023
10. 
    theStack commented at 10:07 PM on October 4, 2023: contributor

    Rebased on master, now that #28577 is merged. Diff:

    diff --git a/test/functional/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py
    index 47828867b..dd564fed8 100755                                                                                                        
    --- a/test/functional/p2p_v2_transport.py
    +++ b/test/functional/p2p_v2_transport.py
    @@ -130,9 +130,8 @@ class V2TransportTest(BitcoinTestFramework):                                                                         
             assert_equal(self.nodes[4].getblockcount(), 11)            
                                                                        
             # Check v1 prefix detection                                                                                                     
    -        # TODO: this should have the same prefix size as the one used for wrong network magic detection (see below)
    -        V1_PREFIX = MAGIC_BYTES["regtest"] + b"version\x00"
    -        assert_equal(len(V1_PREFIX), 12)
    +        V1_PREFIX = MAGIC_BYTES["regtest"] + b"version\x00\x00\x00\x00\x00"
    +        assert_equal(len(V1_PREFIX), 16)
             with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:                                                   
                 num_peers = len(self.nodes[0].getpeerinfo())                                                                                
                 s.connect(("127.0.0.1", p2p_port(0)))                                                                                       
    @@ -143,11 +142,11 @@ class V2TransportTest(BitcoinTestFramework):                                                                       
                 self.wait_until(lambda: self.nodes[0].getpeerinfo()[-1]["transport_protocol_type"] == "v1")                                 
                                                                                                                                             
             # Check wrong network prefix detection (hits if the next 12 bytes correspond to a v1 version message)                           
    -        wrong_network_magic_prefix = MAGIC_BYTES["signet"] + V1_PREFIX[4:] + b"\x00\x00\x00\00"
    +        wrong_network_magic_prefix = MAGIC_BYTES["signet"] + V1_PREFIX[4:]
             with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s:
                 s.connect(("127.0.0.1", p2p_port(0)))
                 with self.nodes[0].assert_debug_log("V2 transport error: V1 peer with wrong MessageStart"):
    -                s.sendall(wrong_network_magic_prefix)
    +                s.sendall(wrong_network_magic_prefix + b"somepayload")
    

    (note that some more data has to be sent after the 12-bytes v1 message type string, to trigger calling the ProcessReceivedKeyBytes method which contains the detection of the wrong network magic prefix)

11. 
    Sjors commented at 7:29 AM on October 5, 2023: member

    utACK e1308967e1a7e0ab275ab4c6f2f94c4ca0ee517b

12. DrahtBot requested review from sipa on Oct 5, 2023
13. 
    maflcko commented at 10:50 AM on October 5, 2023: member

    lgtm ACK e1308967e1a7e0ab275ab4c6f2f94c4ca0ee517b

14. fanquake merged this on Oct 5, 2023
15. fanquake closed this on Oct 5, 2023

    ---

16. theStack deleted the branch on Oct 5, 2023
17. Frank-GER referenced this in commit 92cf68a6d8 on Oct 13, 2023
18. kwvg referenced this in commit 94cf35218f on Sep 17, 2024
19. kwvg referenced this in commit 0b6bebf50b on Sep 17, 2024
20. kwvg referenced this in commit 77310762fc on Sep 17, 2024
21. kwvg referenced this in commit b1d1ab53b0 on Oct 2, 2024
22. kwvg referenced this in commit 069df15eb6 on Oct 9, 2024
23. kwvg referenced this in commit c35ee2ef1b on Oct 14, 2024
24. kwvg referenced this in commit 9c609f6c61 on Oct 15, 2024
25. kwvg referenced this in commit 65eb194d82 on Oct 15, 2024
26. PastaPastaPasta referenced this in commit 5903fb7898 on Oct 16, 2024
27. bitcoin locked this on Dec 3, 2024

Contributors

theStackDrahtBotsipaSjorsmaflcko

Review Requested
vasildsipa