test: Fix intermittent issue in p2p_addr_relay.py #21785

1. 
   MarcoFalke commented at 11:12 am on April 27, 2021: member

   Fixes https://github.com/bitcoin/bitcoin/pull/21707/files#r621048952
2. fanquake added the label Tests on Apr 27, 2021
3. MarcoFalke force-pushed on Apr 27, 2021
4. 
   MarcoFalke commented at 11:25 am on April 27, 2021: member

   Can be reviewed with --color-moved=dimmed-zebra to reduce the review burden by 3 lines :sweat_smile:
5. 
   mzumsande commented at 11:35 am on April 27, 2021: member

   Thanks for the fix and Concept ACK. See #21707 (review) - there is another occurrence in blocksonly_mode_tests of p2p_addr_relay.py that should be changed as well - by calling the send_addr_msg() helper as suggested by John or directly there.
6. MarcoFalke added the label Waiting for author on Apr 27, 2021
7. 
   jonatack commented at 5:27 pm on April 27, 2021: member

   Concept ACK, hit this issue today.
8. in test/functional/test_framework/p2p.py:551 in fa5211d9e0 outdated

   547+        # been called at least once
   548+        self.sync_with_ping()
   549+        self.sync_with_ping()
   550+
   551     def sync_with_ping(self, timeout=60):
   552+        """Ensure any sent message on this connection is processed by ProcessMessages"""
   

   ---

   ---

   

   amitiuttarwar commented at 6:44 pm on April 27, 2021:

   thanks for adding description here & in sync_send_with_ping, but imo it could be clearer. offering a suggestion incase you agree this captures the mechanism better:

   “Ensure the recipient node has run ProcessMessages for all previously sent p2p messages”

   ---

   

   mzumsande commented at 7:38 pm on April 27, 2021:

   Isn’t ‘’any message" too strong (instead of “at least one message”), especially here but also above? ProcessMessages, ProcessMessage and SendMessages can all return early. With SendMessages, it goes together with a disconnect, but for ProcessMessage(s) there are many other ways, so that in case of multiple messages not all might have been processed in this iteration of the loop.

   ---

   

   amitiuttarwar commented at 8:19 pm on April 27, 2021:

   you make a good point, I was making an assumption about test setup that isn’t necessarily true.

   ---

   

   MarcoFalke commented at 6:20 am on April 28, 2021:

   Tried to fix, thanks
9. in test/functional/test_framework/p2p.py:543 in fa5211d9e0 outdated

   538@@ -539,8 +539,16 @@ def send_and_ping(self, message, timeout=60):
   539         self.send_message(message)
   540         self.sync_with_ping(timeout=timeout)
   541 
   542-    # Sync up with the node
   543+    def sync_send_with_ping(self, timeout=60):
   544+        """Ensure any to-be-received message on this connection is processed by SendMessages"""
   

   ---

   ---

   

   amitiuttarwar commented at 6:45 pm on April 27, 2021:

   to consider:

   “Ensure the recipient node has run ProcessMessages and SendMessages for all previously sent p2p messages”

   ---

   

   MarcoFalke commented at 6:20 am on April 28, 2021:

   Tried to fix, thanks
10. 
    amitiuttarwar commented at 6:47 pm on April 27, 2021: contributor

    ACK fa5211d9e0, thanks for this fix. agree with mzumsande’s #21785 (comment), ready to reACK if you incorporate. also left some wording suggestions to consider.
11. MarcoFalke removed the label Waiting for author on Apr 28, 2021
12. test: Use self.send_addr_msg fa37116c82
13. test: Fix intermittent issue in p2p_addr_relay.py faa51ef4d3
14. MarcoFalke force-pushed on Apr 28, 2021
15. 
    mzumsande commented at 2:49 pm on April 28, 2021: member

    ACK faa51ef4d33034f8c14cfd18f1c01c90883a1bb4
16. 
    amitiuttarwar commented at 7:59 pm on April 28, 2021: contributor

    ACK faa51ef4d33034f8c14cfd18f1c01c90883a1bb4
17. fanquake merged this on Apr 29, 2021
18. fanquake closed this on Apr 29, 2021

    ---

19. sidhujag referenced this in commit cabc2fb4b4 on Apr 29, 2021
20. MarcoFalke deleted the branch on Apr 29, 2021
21. 
    jnewbery commented at 6:56 am on April 29, 2021: member

    ACK faa51ef4d33034f8c14cfd18f1c01c90883a1bb4. Looks good. Thanks Marco.
22. luke-jr referenced this in commit 125ea425cf on Jun 27, 2021
23. Fabcien referenced this in commit c8c8603007 on Jan 21, 2022
24. gwillen referenced this in commit a02abbc35a on Jun 1, 2022
25. DrahtBot locked this on Aug 16, 2022

Contributors
MarcoFalke mzumsande jonatack amitiuttarwar jnewbery

Labels
Tests