fix: PoW re-sync downloads already processed blocks

[batrr]

After a successful sync, if a re-sync is triggered (block gap, etc.) and the same best peer is re-allocated, _currentBestPeer still holds the stale reference so OnNewBestPeer is never called. This leaves
_currentNumber stale, causing the downloader to re-request already-processed headers.

Changes

• Clear _currentBestPeer when sync finishes (peer returns no headers), so the next re-sync resets _currentNumber to BestKnownNumber instead of the stale position.
• Add regression test covering the scenario

Types of changes

• Bugfix (a non-breaking change that fixes an issue)

Testing

Requires testing

• Yes

If yes, did you write tests?

• Yes

[claude]

Claude finished @batrr's task in 2m 48s —— View job

---

Code Review

• Gather context and load relevant rules
• Read changed files (PowForwardHeaderProvider.cs, BlockDownloaderTests.cs)
• Analyze correctness, edge cases, and potential regressions
• Check test coverage
• Post findings

---

Summary

The fix is correct and well-targeted. The root cause is accurately identified: when the same peer is re-allocated for a re-sync, _currentBestPeer == peerInfo prevents OnNewBestPeer from being called, leaving _currentNumber stale at the previously-synced position instead of being reset to blockTree.BestKnownNumber. Setting _currentBestPeer = null on null response forces the reset on the next allocation.

The test correctly exercises the scenario: first full sync to block 10, P2P-propagated headers advance BestKnownNumber to 15, peer extended to 20, then verifies the next download batch starts beyond block 15.

---

Findings

No Critical / High / Medium issues.

Low — Stale TODO comment now addressed

src/Nethermind/Nethermind.Synchronization/Blocks/PowForwardHeaderProvider.cs, line 69–70:

// Provide a way so that it does not redownload if part of the. I guess it does not care about skiplastn and maxheaders.
// TODO: Unit test this mechanism.

The // TODO: Unit test this mechanism. is now satisfied by the new regression test. The first line of that comment is also an incomplete/stale thought. Both can be removed. Fix this →

Low — _currentBestPeer = null also fires on transient errors

The new line is placed at the level of the outer GetBlockHeaders lambda — it triggers for every null return from the inner method, including timeout/weak-peer paths (which call ReportWeakPeer before returning null). In those error cases, _currentNumber will be reset to BestKnownNumber on the next allocation instead of resuming from the interrupted position. This is safe (at worst causes a small redundant re-request), but it's a minor behavioral difference from the pre-fix code. Worth noting in comments if this ever causes confusion, though no action required.

---

Verdict

Fix is correct, minimal, and regression-tested. No blocking issues. Mergeable.