core/forkid: include polygon-specific forks in wire forkid#2237
Conversation
NewID computes the eth p2p handshake forkid by calling gatherForks (lowercase), which uses reflection over the outer ChainConfig struct and is blind to the polygon-specific block fields nested under config.Bor (RioBlock, MadhugiriBlock, DandeliBlock, LisovoBlock, LisovoProBlock, GiuglianoBlock, ChicagoBlock). The polygon-aware GatherForks (uppercase) was added in #2063 to expose fork data via the new bor_forks RPC, but the wire path was never updated to use it. As a result, bor's wire forkid does not include any polygon-specific fork activation in its checksum. On chains where polygon forks activate at non-zero blocks (devnets, testnets, mainnet post-fork), bor's wire forkid is inconsistent with what the bor_forks RPC reports for the same node. This was latent until erigon v3.6.0 started including the new polygon forks (Lisovo, LisovoPro, Giugliano, Chicago) in its own GatherForks. erigon's wire forkid now correctly hashes those blocks while bor's still does not, causing the eth handshake to fail with 'fork ID rejected: local incompatible or needs update' on any deployment combining bor v2.8.0 with erigon v3.6.0 on a chain where those forks activate above block 0. Switch NewID to call GatherForks so the wire forkid includes the polygon-specific forks and matches what bor_forks RPC reports.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
This PR updates the EIP-2124 wire forkid computation so that Bor’s p2p handshake fork checksum includes Polygon/Bor-specific fork activation blocks (matching what bor_forks reports), preventing handshake incompatibilities with clients that already include those forks (e.g., newer Erigon).
Changes:
- Switch
core/forkid.NewIDto use the Polygon-awareGatherForks(instead of the reflection-basedgatherForks) when computing the wire forkid.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code ReviewBug:
|
newFilter (used by NewFilter / NewStaticFilter) builds the local table of valid fork checksums that the inbound eth handshake validator matches incoming peers against. It was calling lowercase gatherForks, which is the same polygon-blind reflection-only path the previous commit fixed in NewID. Without this, a patched bor (with the fixed NewID) would emit a forkid that includes the polygon-specific forks, but its own inbound filter would compute the local sums[] table without them. Incoming peer forkids would never match the table by exact rule #1 of EIP-2124 and would only be accepted via the rule #3 "remote is a superset" forgiveness path. The validator's accept/reject behaviour would be correct by accident rather than by design, and any nuance that depends on rule #1 (matching local fork state) would be wrong. Switch newFilter to GatherForks so the validator's sums[] table is symmetric with what NewID emits. After this, the eth handshake's forkid check is end-to-end polygon-aware on bor.
|
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2237 +/- ##
===========================================
+ Coverage 52.66% 52.70% +0.03%
===========================================
Files 885 885
Lines 156759 156752 -7
===========================================
+ Hits 82562 82613 +51
+ Misses 68982 68868 -114
- Partials 5215 5271 +56
... and 45 files with indirect coverage changes
🚀 New features to boost your workflow:
|
NewID computes the eth p2p handshake forkid by calling gatherForks (lowercase), which uses reflection over the outer ChainConfig struct and is blind to the polygon-specific block fields nested under config.Bor (RioBlock, MadhugiriBlock, DandeliBlock, LisovoBlock, LisovoProBlock, GiuglianoBlock, ChicagoBlock). The polygon-aware GatherForks (uppercase) was added in #2063 to expose fork data via the new bor_forks RPC, but the wire path was never updated to use it.
As a result, bor's wire forkid does not include any polygon-specific fork activation in its checksum. On chains where polygon forks activate at non-zero blocks (devnets, testnets, mainnet post-fork), bor's wire forkid is inconsistent with what the bor_forks RPC reports for the same node.
This was latent until erigon v3.6.0 started including the new polygon forks (see
p2p/forkid/forkid.go), Lisovo, LisovoPro, Giugliano and Chicago in its own GatherForks. erigon's wire forkid now correctly hashes those blocks while bor's still does not, causing the eth handshake to fail withfork ID rejected: local incompatible or needs updateon any deployment combining bor v2.8.0 with erigon v3.6.0 on a chain where those forks activate above block 0.