Fix StateSyncDownloader to fallback to alternative protocols on empty responses
Problem
StateSyncDownloader was using mutually exclusive if-else if chains to select protocols. Once a protocol was selected, the downloader would not try alternative protocols even if the peer returned an empty response. This caused sync failures on networks like OP Base where peers advertise protocol support but return empty data.
Solution
Modified the Dispatch method to use a sequential try pattern with automatic fallback. Protocols are now tried in order of preference: Snap → Eth66 → NodeData, prioritizing the newest and most commonly served protocol (Snap) first.
Key Changes:
-
Refactored Dispatch Method: The main
Dispatchmethod now tries protocols sequentially with fallback support instead of using mutually exclusive if-else chains. -
Protocol Try Methods: Created three helper methods:
-
TryGetNodeDataViaSnapProtocol: Attempts to fetch data via Snap protocol -
TryGetNodeDataViaEthProtocol: Attempts to fetch data via Eth66 protocol -
TryGetNodeDataViaNodeDataProtocol: Attempts to fetch data via NodeData protocol
-
-
Protocol Ordering: Protocols are tried in this order (prioritizing newest and most served):
- Snap protocol (newest and most commonly served)
- Eth66 protocol (if version < 67)
- NodeData protocol (fallback)
-
Fallback Logic: Each protocol method:
- Returns
trueif data is successfully retrieved (Count > 0) - Returns
falseif response is empty or an error occurs - Properly disposes of resources (HashList, GetTrieNodesRequest)
- Logs trace messages for debugging
- Returns
-
Graceful Handling: When all protocols return empty responses:
- Logs a debug message indicating peer might be out of sync
- Returns without throwing exception (allows sync to continue)
-
Test Updates:
- Updated existing tests to provide mock responses
- Updated
SnapPeer_FallbackToNodeDataWhenEmptytest (was NodeDataPeer_FallbackToSnapWhenEmpty) - Updated
SnapPeer_FallbackToEth66WhenEmptytest (was Eth66Peer_FallbackToSnapWhenEmpty) - Tests now reflect the new protocol ordering where Snap is tried first
- Improved test code with collection initializers and proper using statements
-
Code Quality Improvements:
- Added using statements for better readability
- Used collection initializer syntax
- Removed unnecessary null checks (HashList.Rent never returns null)
Behavior:
- Before: If any protocol returned empty, sync would fail with no fallback
- After: Tries Snap first (most common), then Eth66, then NodeData. If any protocol returns empty, tries the next one. If all fail, logs debug message and continues gracefully (peer might be out of sync)
Fixes NethermindEth/nethermind#9531
Original prompt
This section details on the original issue you should resolve
<issue_title>Make
StateSyncDownloadertry other protocol when empty.</issue_title> <issue_description>Is your feature request related to a problem? Please describe.
StateSyncDownloaderwill try nodedata, eth66, then snap. If nodedata or eth66 is unable to serve node, then it will not try snap. Modify the downloader so that if the peer returned empty response, then try another protocol.Describe the solution you'd like
- Modify the downloader so that if the peer returned empty response, then try another protocol.
Describe alternatives you've considered
- Technically this is fine if the network is spread out.
Additional context
- Cannot sync op base. </issue_description>
Comments on the Issue (you are @copilot in this section)
Fixes NethermindEth/nethermind#9531
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.
@copilot dont throw when all response is empty. It could be that the peer is simply out of sync.
Done in commit 5311e86. The exception has been removed - now logs a debug message when all protocols return empty.
It does not solve it.
It does not solve it.
Didn't mean it was a bad change, I am fine with merging it.
@copilot reverse the protocol ordering
@copilot reverse the protocol ordering
Done in commit 596709b. Protocol ordering reversed to prioritize Snap first.
@copilot tests are failing, fix them
@copilot tests are failing, fix them
Fixed in commit 0e501be. Updated the fallback tests to reflect the reversed protocol ordering - tests now correctly expect Snap to be tried first, with fallback to Eth66 or NodeData when Snap returns empty.