backport: merge bitcoin#24024, #23880, #24909, #24178, #24171, #25404, #25514, #25720, partial bitcoin#23832, #24169, #25454 (headers backports)

[kwvg]

Additional Information

• Dependent on backport: merge bitcoin#21727, #22371, #21526, #23174, #23785, #23581, #23974, #22932, #24050, #24515 (blockstorage backports) #6085

• Dependency for backport: merge bitcoin#22278, #24103, #24235, #24177, #24299, #24917, #22564, #25349, #25571, #17487, #26999, #27011 (blockstorage backports: part 2) #6098

• bitcoin#25514 removes peers.spvNodeConnections and peers.fullNodeConnections reporting from CalculateNumConnectionsChangedStats as the services flags used to distingush between the two have been moved to the Peer struct, accessable only through PeerManager.

  As PeerManager isn't accessable to CConnman, even if a new public function was exposed through PeerManger (as we have for IsInvInFilter and others or we try to access the value through GetNodeStateStats), CConnman would be unable to leverage it. Resolved with patch by UdjinM6, thanks!

• bitcoin#23880 introduces code that is not valid C++20 (but valid C++17) and therefore, required a partial backport of bitcoin#24169 (fae6790) to make the code C++20 legal.

• bitcoin#25454 introduces a 10-point penalty for remitting more than MAX_BLOCKS_TO_ANNOUNCE unconnected block headers. blockheader_testnet3.hex (introduced in bitcoin#16551, part of dash#5963), unlike in Bitcoin, includes the genesis block.

  By definition of a genesis block, there is no block before it that connects to, which causes the 10-point penalty to trip and p2p_dos_header_tree.py to fail (see below). This has been remedied by removing the genesis block from the test data to match upstream and also because no node has a good reason to ever broadcast the genesis block as-is over P2P.

  Test Failure

  dash@6a2649cc721f:/src/dash$ ./test/functional/p2p_dos_header_tree.py
  2024-06-17T17:59:35.874000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_h5hfluy1
  2024-06-17T17:59:36.892000Z TestFramework (INFO): Read headers data
  2024-06-17T17:59:36.895000Z TestFramework (INFO): Feed all non-fork headers, including and up to the first checkpoint
  2024-06-17T17:59:38.411000Z TestFramework (ERROR): Assertion failed
  Traceback (most recent call last):
    File "/src/dash/test/functional/test_framework/test_framework.py", line 158, in main
      self.run_test()
    File "./test/functional/p2p_dos_header_tree.py", line 53, in run_test
      assert {
  AssertionError
  2024-06-17T17:59:38.913000Z TestFramework (INFO): Stopping nodes
  2024-06-17T17:59:39.917000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_h5hfluy1
  2024-06-17T17:59:39.917000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_h5hfluy1/test_framework.log
  2024-06-17T17:59:39.917000Z TestFramework (ERROR):
  2024-06-17T17:59:39.917000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_h5hfluy1' to consolidate all logs
  2024-06-17T17:59:39.917000Z TestFramework (ERROR):
  2024-06-17T17:59:39.917000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
  2024-06-17T17:59:39.917000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
  2024-06-17T17:59:39.917000Z TestFramework (ERROR):
  

• bitcoin#25454 has a goal similar to dash#2032 (and its predecessor, dash#1589), namely, avoiding getheaders(2) duplication to the same peer. Unfortunately, Dash's mitigation seems to conflict with Bitcoin's mitigation and this results in feature_minchainwork.py failing (see below). This has been remedied by partially reverting dash#2032.

  Test Failure

  dash@1bebec413839:/src/dash$ ./test/functional/feature_minchainwork.py
  2024-08-01T17:29:41.116000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_co8xkx43
  2024-08-01T17:29:42.145000Z TestFramework (INFO): Testing relay across node 1 (minChainWork = 101)
  2024-08-01T17:29:42.145000Z TestFramework (INFO): Generating 49 blocks on node0
  [...]
  2024-08-01T17:29:51.707000Z TestFramework (INFO): Generating one more block
  2024-08-01T17:29:51.709000Z TestFramework (INFO): Verifying nodes are all synced
  2024-08-01T17:30:51.989000Z TestFramework (ERROR): Assertion failed
  Traceback (most recent call last):
    File "/src/dash/test/functional/test_framework/test_framework.py", line 159, in main
      self.run_test()
    File "./test/functional/feature_minchainwork.py", line 101, in run_test
      self.sync_all()
    File "/src/dash/test/functional/test_framework/test_framework.py", line 811, in sync_all
      self.sync_blocks(nodes)
    File "/src/dash/test/functional/test_framework/test_framework.py", line 777, in sync_blocks
      raise AssertionError("Block sync timed out after {}s:{}".format(
  AssertionError: Block sync timed out after 60s:
    '00ab061e30aebd2f97153d26cd72f921af896f05c6469ad73c7de4fc283d9590'
    '00ab061e30aebd2f97153d26cd72f921af896f05c6469ad73c7de4fc283d9590'
    '000008ca1832a4baf228eb1553c03d3a2c8e02399550dd6ea8d65cec3ef23d2e'
  2024-08-01T17:30:52.490000Z TestFramework (INFO): Stopping nodes
  2024-08-01T17:30:53.495000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_co8xkx43
  2024-08-01T17:30:53.495000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_co8xkx43/test_framework.log
  2024-08-01T17:30:53.495000Z TestFramework (ERROR):
  2024-08-01T17:30:53.495000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_co8xkx43' to consolidate all logs
  2024-08-01T17:30:53.495000Z TestFramework (ERROR):
  2024-08-01T17:30:53.495000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
  2024-08-01T17:30:53.495000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
  2024-08-01T17:30:53.495000Z TestFramework (ERROR):
  

• bitcoin#25720 introduces a new test, p2p_initial_headers_sync.py, to validate that when a client has a stale tip, it will only engage in headers sync with one peer (emit a getheaders2* message).

  Unmodified, this test fails (see below) as while the backport deals with one source of getheaders2 messages, the test setup unwittingly triggers another (source), specifically, allowing the pindexBestHeader->GetBlockTime() > GetAdjustedTime() - nMaxTipAge condition to evaluate true.

  This is because, unlike in Bitcoin test suite's setup_chain() (source), Dash sets the mocktime to match the mock chain (source) during setup, while the test assumes that the mock chain is stale enough to not trigger this source of getheaders2 messages.

  As the tip is barely stale, it emits the getheaders2 message, which is detected, causing the test to fail. This has been remedied by overriding setup_chain() to behave more like Bitcoin's test suite.

  * - getheaders2 is a Dash-specific message that is courtesy of compressed headers, Bitcoin would be checking for getheaders

  Test Failure

  dash@6a2649cc721f:/src/dash$ ./test/functional/p2p_initial_headers_sync.py
  2024-06-17T21:24:09.921000Z TestFramework (INFO): Initializing test directory /tmp/dash_func_test_3gypo3ab
  2024-06-17T21:24:10.681000Z TestFramework (INFO): Adding a peer to node0
  2024-06-17T21:24:11.684000Z TestFramework (INFO): Connecting two more peers to node0
  2024-06-17T21:24:13.689000Z TestFramework (INFO): Verify that peer2 and peer3 don't receive a getheaders after connecting
  2024-06-17T21:24:15.193000Z TestFramework (ERROR): Assertion failed
  Traceback (most recent call last):
    File "/src/dash/test/functional/test_framework/test_framework.py", line 158, in main
      self.run_test()
    File "./test/functional/p2p_initial_headers_sync.py", line 60, in run_test
      assert "getheaders2" not in peer2.last_message
  AssertionError
  2024-06-17T21:24:15.695000Z TestFramework (INFO): Stopping nodes
  2024-06-17T21:24:16.698000Z TestFramework (WARNING): Not cleaning up dir /tmp/dash_func_test_3gypo3ab
  2024-06-17T21:24:16.698000Z TestFramework (ERROR): Test failed. Test logging available at /tmp/dash_func_test_3gypo3ab/test_framework.log
  2024-06-17T21:24:16.699000Z TestFramework (ERROR):
  2024-06-17T21:24:16.699000Z TestFramework (ERROR): Hint: Call /src/dash/test/functional/combine_logs.py '/tmp/dash_func_test_3gypo3ab' to consolidate all logs
  2024-06-17T21:24:16.699000Z TestFramework (ERROR):
  2024-06-17T21:24:16.699000Z TestFramework (ERROR): If this failure happened unexpectedly or intermittently, please file a bug and provide a link or upload of the combined log.
  2024-06-17T21:24:16.699000Z TestFramework (ERROR): https://github.com/dashpay/dash/issues
  2024-06-17T21:24:16.699000Z TestFramework (ERROR):
  

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

[github-actions]

This pull request has conflicts, please rebase.

[github-actions]

This pull request has conflicts, please rebase.

[UdjinM6]

bitcoin#25514 removes peers.spvNodeConnections and peers.fullNodeConnections reporting from CalculateNumConnectionsChangedStats as the services flags used to distingush between the two have been moved to the Peer struct, accessable only through PeerManager.
As PeerManager isn't accessable to CConnman, even if a new public function was exposed through PeerManger (as we have for IsInvInFilter and others or we try to access the value through GetNodeStateStats), CConnman would be unable to leverage it.
...
Breaking Changes
Statistics reporting that expect peers.spvNodeConnections and peers.fullNodeConnections to be reported will find that they are no longer available. The reasoning for which has already been documented above.

How about altering spv detection logic instead? d92560a

[UdjinM6]

utACK 14142f5

[PastaPastaPasta]

utACK 14142f5

[UdjinM6]

needs rebase to fix Check Merge Fast-Forward Only

[knst]

reviewed + tested with extra lock for cs_main in dsnotificationinterface.cpp:
https://gitlab.com/dashpay/dash/-/pipelines/1409559191

commits up to 26d477b LGTM