tari-project / tari / 6627435623

60%

push

github

feat: consistent handling of edge cases for header sync (#5837)

Description
---
This PR fixes an edge case for header sync where the local node has more
headers, but the remote node has better proof-of-work (accumulated
difficulty), thus higher actual blockchain height. It also simplifies
the attempted header sync logic and error handling when a peer does not
return headers and introduces consistent naming, variable re-use and use
of information

Supersedes #5756

Motivation and Context
---
When doing header sync, and determining from which peer to sync, we need
to consider our actual blockchain state when comparing chains. It is
possible that our node has valid block headers that will translate into
a higher proof of work when fully synced than the remote syncing peer,
but lacking the blocks to back the block headers, thus the remote chain
has an actual higher proof of work blockchain.
With the current implementation, race conditions can exist when
determining if the peer is misbehaving, i.e. lying about their
proof-of-work or not wanting to supply block headers, due to a mismatch
in tip height used for the check.

This PR fixes the race conditions by always using the latest local chain
metadata and block headers, ignoring all sync peers that do not exceed
our own accumulated difficulty and using consistent information in all
the checks.

How Has This Been Tested?
---
Added integration-level unit tests:
- `test_header_sync_happy_path`
- `test_header_sync_with_fork_happy_path`
- `test_header_sync_uneven_headers_and_blocks_happy_path`
-
`test_header_sync_uneven_headers_and_blocks_peer_lies_about_pow_no_ban`
-
`test_header_sync_even_headers_and_blocks_peer_lies_about_pow_with_ban`
-
`test_header_sync_even_headers_and_blocks_peer_metadata_improve_with_reorg`

What process can a PR reviewer use to test or verify this change?
---
- Code walk-through
- Review unit tests

<!-- Checklist -->
<!-- 1. Is the titl... (continued)

173 of 173 new or added lines in 5 files covered. (100.0%)

73733 of 97839 relevant lines covered (75.36%)

202505.72 hits per line