net: introduce block tracker to retry to download blocks after failure #27837

1. 
   furszy commented at 7:01 pm on June 7, 2023: member

   Coming from #27652, part of #29183.

   The general idea is to keep track of the user requested blocks so, in case of a bad behaving peer or a network disconnection, they can be fetched from another one automatically without any further user interaction.

   This was requested by users because the getblockfrompeer RPC command lacks the functionality to notify them about block request failures or peer disconnections (which is expected due to the asynchronous nature of the block requests).

   Currently, this new functionality is limited to blocks requested by the user via the ‘getblockfrompeer’ RPC command.

   In the future, this class could expand its scope and be utilized in the regular chain synchronization process. Or, even could be employed in special procedures like a prune node rescan that uses BIP158 block filters, or even into BIP157 itself.

2. 
   DrahtBot commented at 7:01 pm on June 7, 2023: contributor

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

   Code Coverage

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

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31282 (refactor: Make node_id a const& in RemoveBlockRequest by maflcko)
   • #28488 (p2p: Evict outbound peers with high minFeeRate by naumenkogs)
   • #28055 (Bugfix: net_processing: Restore “Already requested” error for FetchBlock by luke-jr)

   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. DrahtBot added the label P2P on Jun 7, 2023
4. DrahtBot added the label CI failed on Jun 7, 2023
5. 
   maflcko commented at 7:27 am on June 13, 2023: member

   Maybe mark as draft if CI is red and this is still based on something else?
6. fanquake marked this as a draft on Jun 13, 2023
7. 
   furszy commented at 9:33 am on June 13, 2023: member

   Oh, didn’t know about the CI failure. Seems to be just an unused variable. But sure, draft until #27836 sounds good.
8. furszy force-pushed on Aug 1, 2023
9. DrahtBot removed the label CI failed on Aug 1, 2023
10. DrahtBot added the label Needs rebase on Oct 2, 2023
11. furszy force-pushed on Oct 18, 2023
12. DrahtBot removed the label Needs rebase on Oct 18, 2023
13. DrahtBot added the label Needs rebase on Oct 19, 2023
14. 
    furszy commented at 11:57 am on January 17, 2024: member

    Focus is on #28170, which is quite close. Will rebase it as soon as it is merged.
15. furszy force-pushed on Mar 11, 2024
16. furszy marked this as ready for review on Mar 11, 2024
17. furszy force-pushed on Mar 11, 2024
18. DrahtBot removed the label Needs rebase on Mar 11, 2024
19. DrahtBot added the label CI failed on Mar 12, 2024
20. 
    DrahtBot commented at 0:18 am on March 12, 2024: contributor

    🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the documentation.

    Possibly this is due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    Leave a comment here, if you need help tracking down a confusing failure.

    Debug: https://github.com/bitcoin/bitcoin/runs/22512552586

21. furszy force-pushed on Mar 12, 2024
22. furszy force-pushed on Apr 18, 2024
23. in test/functional/rpc_getblockfrompeer.py:163 in ad4fc1bf82 outdated

    158+        # Test reject fetching old block from a limited peer #
    159+        ######################################################
    160+
    161+        self.log.info("Test reject fetching old block from limited peer")
    162+        pruned_node.add_p2p_connection(P2PInterface(), services=NODE_NETWORK_LIMITED | NODE_WITNESS)
    163+        limited_peer_id = next(peer for peer in pruned_node.getpeerinfo() if peer["servicesnames"] == ['WITNESS', 'NETWORK_LIMITED'])["id"]
    

    ---

    ---

    

    DrahtBot commented at 6:43 am on April 23, 2024:

    0 test  2024-04-18T12:47:34.188000Z TestFramework (ERROR): Unexpected exception caught during testing 
    1                                   Traceback (most recent call last):
    2                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
    3                                       self.run_test()
    4                                     File "/ci_container_base/ci/scratch/build/bitcoin-i686-pc-linux-gnu/test/functional/rpc_getblockfrompeer.py", line 163, in run_test
    5                                       limited_peer_id = next(peer for peer in pruned_node.getpeerinfo() if peer["servicesnames"] == ['WITNESS', 'NETWORK_LIMITED'])["id"]
    6                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    7                                   StopIteration
    

    https://cirrus-ci.com/task/4803962228965376?logs=ci#L3152

    ---

    

    furszy commented at 2:22 pm on May 1, 2024:

    fixed
24. furszy force-pushed on Apr 29, 2024
25. furszy force-pushed on May 1, 2024
26. DrahtBot marked this as a draft on May 13, 2024
27. 
    DrahtBot commented at 8:06 am on May 13, 2024: contributor

    Turned into draft for now, due to failing CI. If this is no longer a WIP, you can move it out of draft.
28. furszy force-pushed on May 15, 2024
29. furszy force-pushed on May 22, 2024
30. DrahtBot removed the label CI failed on May 23, 2024
31. achow101 referenced this in commit e163d864d3 on May 23, 2024
32. furszy force-pushed on May 23, 2024
33. hebasto added the label Needs CMake port on Aug 16, 2024
34. 
    DrahtBot commented at 2:27 am on August 29, 2024: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/25334640418

    Make sure to run all tests locally, according to the documentation.

    The failure may happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

35. DrahtBot added the label CI failed on Aug 29, 2024
36. maflcko removed the label Needs CMake port on Aug 29, 2024
37. furszy force-pushed on Aug 29, 2024
38. DrahtBot added the label Needs rebase on Sep 2, 2024
39. furszy force-pushed on Oct 1, 2024
40. DrahtBot removed the label Needs rebase on Oct 1, 2024
41. UdjinM6 referenced this in commit 2c825b4b4a on Oct 3, 2024
42. net: FetchBlock, disallow requesting blocks from network limited peers f850847c73
43. net: FetchBlock, disallow requesting block to the same peer multiple times e64919f78c
44. net: introduce BlockRequestTracker 96e9b44436
45. rpc: getblockfrompeer, connect block tracking mechanism and add coverage

    If the initial block fetching process fails, the p2p layer will be in
    charge of fetching the block from 'any' connected peer.
    Re-trying to download the block from different peers until it is
    received.

    afcf77ddb2
46. RPC: getblockfrompeer, introduce fetch from "any" peer functionality

    If no 'peer_id' is provided, 'getblockfrompeer' will just delegate
    the peer selection to the internal block downloading logic.

    80839cb7b0
47. rpc: getblockfrompeer, add one-shot block requests capability

    Allowing what we had before, a single block request with no automatic
    retry nor tracking mechanism.

    b18c72cfcd
48. furszy force-pushed on Oct 13, 2024
49. panleone referenced this in commit e7c21c7e58 on Nov 11, 2024

Contributors
furszy DrahtBot maflcko

Labels
P2P CI failed