refactor(eth/downloader): drop eth/63 - #2499
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Drops legacy eth/63 protocol support across the eth protocol stack and downloader, updating version gating, handshake/status structures, and the test suite to only exercise supported protocol versions.
Changes:
- Removes eth/63 as a supported protocol version and updates version checks to require
>= xdc100. - Renames/updates legacy status packet handling to
statusData100and adjusts handshake/read paths accordingly. - Updates downloader and eth protocol tests to stop using eth/63 and to align peer/version expectations with the new minimum protocol version.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| eth/sync_test.go | Updates fast-sync disabling tests to use xdc100/xdc165 and runs helper in parallel. |
| eth/protocol.go | Removes eth/63 constant and updates protocol version/length declarations; renames legacy status packet type. |
| eth/protocol_test.go | Removes eth/63 status tests and updates status-message error coverage for xdc100. |
| eth/peer.go | Removes eth/63 handshake paths and switches legacy status handling to xdc100 only. |
| eth/metrics.go | Updates metrics gating from >= eth63 to >= xdc100. |
| eth/helper_test.go | Updates test peer handshake to send xdc100 status packet. |
| eth/handler.go | Updates handler message gating from >= eth63 to >= xdc100. |
| eth/handler_test.go | Updates handler tests away from eth/63; refactors malformed-block propagation test. |
| eth/downloader/queue.go | Adjusts peer-id logging and log callsites in header delivery path. |
| eth/downloader/peer.go | Bumps downloader minimum protocol version to xdc100. |
| eth/downloader/peer_test.go | Updates peer-set version eligibility tests to exclude eth/63. |
| eth/downloader/downloader.go | Updates “too old” checks/messages to require >= 100; trims peer IDs in logs. |
| eth/downloader/downloader_test.go | Updates downloader test matrix from 63/64 to 100/164. |
d39f943 to
83d6819
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
Suppressed comments (1)
eth/downloader/peer_test.go:29
- TestIdlePeersProtocolVersions is described as covering "every supported" protocol version, but it currently only registers peers for 100 and 164. Since the downloader peer-set supports up to maxProtocolVer=165, this test should include a 165 peer as well (and update the expected totals), otherwise regressions affecting xdc165 eligibility won't be caught.
// TestIdlePeersProtocolVersions verifies that peers running every supported
// protocol version (xdc100, xdc164) are eligible for concurrent
// downloads. A regression here silently disables skeleton filling and body,
605cef7 to
75fc9c4
Compare
eth/63 predates the XDC-specific protocols and is no longer spoken by any supported client, so remove it from the advertised capabilities and from every version-gated branch: - drop eth63 from ProtocolVersions and protocolLengths, so the node no longer advertises the "eth/63" capability - rename statusData63 to statusData100 and readStatusLegacy to readStatus100, xdc/100 now being the only legacy status format - raise minProtocolVer to 100 and derive errTooOld and the syncWithPeer guard from it instead of hardcoding the number in three places - replace the eth63 version gates in handleMsg and the metered read/write wrapper with xdc100 - drop the stale [eth/62] and [eth/63] tags from downloader and queue field comments - stop advertising eth/63 in the devp2p ethtest suite Tests are renamed to the xdc100/xdc164 constants instead of bare literals and gain xdc165 coverage where it was missing. This is a breaking p2p change: peers that only speak eth/63 can no longer complete the handshake.
Proposed changes
eth/63 predates the XDC-specific protocols and is no longer spoken by any
supported client, so remove it from the advertised capabilities and from
every version-gated branch:
longer advertises the "eth/63" capability
readStatus100, xdc/100 now being the only legacy status format
guard from it instead of hardcoding the number in three places
wrapper with xdc100
field comments
Tests are renamed to the xdc100/xdc164 constants instead of bare literals
and gain xdc165 coverage where it was missing.
This is a breaking p2p change: peers that only speak eth/63 can no longer
complete the handshake.
Types of changes
What types of changes does your code introduce to XDC network?
Put an
✅in the boxes that applyImpacted Components
Which parts of the codebase does this PR touch?
Put an
✅in the boxes that applyChecklist
Put an
✅in the boxes once you have confirmed below actions (or provide reasons on not doing so) that