feat(core/forkid): announce forkid via ENR and remove eth/62 #19738#2372
feat(core/forkid): announce forkid via ENR and remove eth/62 #19738#2372gzliudan wants to merge 1 commit into
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 Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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
This PR adds an eth ENR entry to advertise the chain fork ID (EIP-2124) via discovery and refactors the eth protocol configuration to stop announcing eth/62 during capability negotiation.
Changes:
- Expose
p2p.Server.LocalNode()so services can update the local ENR record. - Add an
ethENR entry containingforkid.IDand update it on chain head events. - Update eth protocol configuration/refactor protocol construction and remove eth/62 from the advertised
ProtocolVersions.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
p2p/server.go |
Adds LocalNode() accessor to retrieve the server’s local ENR node record. |
p2p/enr/enr_test.go |
Updates a test comment to match the UDP ENR key being tested. |
eth/protocol.go |
Refactors protocol constants/vars and removes eth/62 from advertised versions. |
eth/peer.go |
Updates message size checks to use the renamed max message size constant. |
eth/handler.go |
Refactors protocol construction and replaces cached chain config usage with blockchain.Config(). |
eth/handler_test.go |
Removes the protocol compatibility test (fast sync vs versions). |
eth/enr_entry.go |
Introduces the eth ENR entry (forkid) and a goroutine to refresh it on head updates. |
eth/backend.go |
Attaches the eth ENR entry to protocols and starts the ENR update loop on service start. |
core/blockchain_reader.go |
Updates the comment on BlockChain.Config(). |
Comments suppressed due to low confidence (1)
eth/handler_test.go:45
ProtocolVersionsno longer advertises eth/62, but this file still runs eth/62 variants (e.g.TestGetBlockHeaders62). If eth/62 support is truly being removed, these eth/62-specific tests should be removed/updated; otherwise,ProtocolVersions/docs should clarify that eth/62 is still supported (just not announced).
// Tests that block headers can be retrieved from a remote chain based on user queries.
func TestGetBlockHeaders62(t *testing.T) { testGetBlockHeaders(t, 62) }
// TestGetBlockHeaders63 tests get block headers 63.
func TestGetBlockHeaders63(t *testing.T) { testGetBlockHeaders(t, 63) }
Proposed changes
Ref: ethereum#19738
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