test: fix intermittent failure in `rpc_setban.py --v2transport`, run it in CI #29372

pull mzumsande wants to merge 1 commits into bitcoin:master from mzumsande:202402_fix_setban_v2transport changing 2 files +9 −1
  1. mzumsande commented at 6:22 PM on February 2, 2024: contributor

    This test failed for me on master locally: The reason is that when initiating a v2 connection and being immediately disconnected, a node cannot know if the disconnect happens because the peer only supports v1, or because it has banned you, so it schedules to reconnect with v1. If the test doesn't wait for that, the reconnect can happen at a bad time, resulting in failure in a later connect_nodes call. Also add the test with --v2transport to the test runner because banning with v2 seems like a useful thing to have test coverage for.

  2. DrahtBot commented at 6:23 PM on February 2, 2024: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK delta1, stratospher, achow101
    Concept ACK epiccurious

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #26863 (test: merge banning test from p2p_disconnect_ban to rpc_setban by brunoerg)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. test: fix intermittent failure in rpc_setban.py --v2transport
    When initiating a v2 connection and being immediately disconnected,
    a node cannot know if the disconnect happens because the peer only
    supports v1, or because it has banned you, so it schedules to reconnect with v1.
    If the test doesn't wait for that, the reconnect can happen at a bad time,
    resulting in failure in a later connect_nodes call.
    Also add the test with --v2transport to the test runner.
    cc87ee4c39
  4. mzumsande force-pushed on Feb 2, 2024
  5. mzumsande renamed this:
    fix intermittent failure in `rpc_setban.py --v2transport`, run it in CI
    test: fix intermittent failure in `rpc_setban.py --v2transport`, run it in CI
    on Feb 2, 2024
  6. DrahtBot added the label Tests on Feb 2, 2024
  7. delta1 commented at 6:05 PM on February 4, 2024: none

    tested ACK cc87ee4c3934028e78a59de509951ff7226ec80d

  8. stratospher commented at 6:13 AM on February 5, 2024: contributor

    tested ACK cc87ee4. nice find!

  9. edilmedeiros commented at 5:53 PM on February 5, 2024: contributor

    I don't think this is something to stall a merge, but I could not make the test fail in master. I tried with different loads in my machine from other work.

    How are you calling it?

    ❯ python test/functional/rpc_setban.py
    2024-02-05T17:48:04.516000Z TestFramework (INFO): PRNG seed is: 2620849867157253531
    2024-02-05T17:48:04.517000Z TestFramework (INFO): Initializing test directory /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_tqvcwu8u
    2024-02-05T17:48:06.209000Z TestFramework (INFO): Test that a non-IP address can be banned/unbanned
    2024-02-05T17:48:06.210000Z TestFramework (INFO): Test the ban list is preserved through restart
    2024-02-05T17:48:06.890000Z TestFramework (INFO): Test -bantime
    2024-02-05T17:48:07.369000Z TestFramework (INFO): Stopping nodes
    2024-02-05T17:48:07.477000Z TestFramework (INFO): Cleaning up /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_tqvcwu8u on exit
    2024-02-05T17:48:07.477000Z TestFramework (INFO): Tests successful
    
  10. mzumsande commented at 5:59 PM on February 5, 2024: contributor

    I could not make the test fail in master

    ❯ python test/functional/rpc_setban.py

    You have to call it with v2, see PR title: rpc_setban.py --v2transport.

  11. edilmedeiros commented at 6:59 PM on February 5, 2024: contributor

    I'm sorry for not being clear in my comment. I tried that with your code, and it runs something that passes (see below).

    My point is that running rpc_setban.py in 5b8c5970 does not seem to fail in any way in my box. The mentioned commit passed the CI tests. I understand I'm a newcomer, and I'm trying to learn how the tests work by reproducing the error you found instead of just reading the code in the PR.

    Yet, I don't see this as a problem to merge. It might be an issue triggered by some specificity in your box, but not mine.

    ❯ python test/functional/rpc_setban.py
    2024-02-05T18:36:28.549000Z TestFramework (INFO): PRNG seed is: 7243494276342072324
    2024-02-05T18:36:28.550000Z TestFramework (INFO): Initializing test directory /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_w4vk2rv2
    2024-02-05T18:36:30.718000Z TestFramework (INFO): Test that a non-IP address can be banned/unbanned
    2024-02-05T18:36:30.719000Z TestFramework (INFO): Test the ban list is preserved through restart
    2024-02-05T18:36:31.403000Z TestFramework (INFO): Test -bantime
    2024-02-05T18:36:31.882000Z TestFramework (INFO): Stopping nodes
    2024-02-05T18:36:31.993000Z TestFramework (INFO): Cleaning up /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_w4vk2rv2 on exit
    2024-02-05T18:36:31.993000Z TestFramework (INFO): Tests successful
    
    ❯ python test/functional/rpc_setban.py --v2transport
    2024-02-05T18:36:38.617000Z TestFramework (INFO): PRNG seed is: 3082398001401933771
    2024-02-05T18:36:38.618000Z TestFramework (INFO): Initializing test directory /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_lj9gxqtg
    2024-02-05T18:36:41.687000Z TestFramework (INFO): Test that a non-IP address can be banned/unbanned
    2024-02-05T18:36:41.688000Z TestFramework (INFO): Test the ban list is preserved through restart
    2024-02-05T18:36:42.371000Z TestFramework (INFO): Test -bantime
    2024-02-05T18:36:42.850000Z TestFramework (INFO): Stopping nodes
    2024-02-05T18:36:42.961000Z TestFramework (INFO): Cleaning up /var/folders/pk/trlzyy614z11wn_y1t8vhsz80000gn/T/bitcoin_func_test_lj9gxqtg on exit
    2024-02-05T18:36:42.961000Z TestFramework (INFO): Tests successful
    
  12. epiccurious commented at 2:13 AM on February 7, 2024: contributor

    Concept ACK cc87ee4c3934028e78a59de509951ff7226ec80d.

  13. achow101 commented at 10:50 PM on February 8, 2024: member

    ACK cc87ee4c3934028e78a59de509951ff7226ec80d

  14. achow101 merged this on Feb 8, 2024
  15. achow101 closed this on Feb 8, 2024

  16. mzumsande deleted the branch on Feb 9, 2024
  17. kwvg referenced this in commit 5c27481f9c on Oct 15, 2024
  18. kwvg referenced this in commit 406e4840d3 on Oct 23, 2024
  19. kwvg referenced this in commit dab9aa2e54 on Oct 23, 2024
  20. kwvg referenced this in commit b3fd308ebc on Oct 23, 2024
  21. kwvg referenced this in commit 01fd4fcfa6 on Oct 24, 2024
  22. kwvg referenced this in commit 5ee15faba0 on Oct 24, 2024
  23. PastaPastaPasta referenced this in commit 2e162da06f on Oct 24, 2024
  24. bitcoin locked this on Feb 8, 2025

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-17 03:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me