eth/protocols/wit, consensus/bor: WIT2 — BP-signed witness announcements with transitive relay and pre-import serving#2208
Open
lucca30 wants to merge 15 commits into
Open
eth/protocols/wit, consensus/bor: WIT2 — BP-signed witness announcements with transitive relay and pre-import serving#2208lucca30 wants to merge 15 commits into
lucca30 wants to merge 15 commits into
Claude / Claude Code Review
completed
Jun 26, 2026 in 19m 57s
Code review found 1 important issue
Found 5 candidates, confirmed 4. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 1 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 0 |
| Severity | File:Line | Issue |
|---|---|---|
| 🔴 Important | eth/handler_wit2_announces.go:173-193 |
deferredAnnounceCache.put: malicious overwrite + per-peer cap bypass |
| 🟡 Nit | eth/handler_wit2_bodies.go:62-83 |
pendingWitnessBodyCache.put: needless eviction on overwrite at capacity |
| 🟡 Nit | eth/handler.go:701-720 |
Deferred WIT2 announces dropped on batch chain insert |
Annotations
Check failure on line 193 in eth/handler_wit2_announces.go
claude / Claude Code Review
deferredAnnounceCache.put: malicious overwrite + per-peer cap bypass
deferredAnnounceCache.put has two defects on the existing-key (overwrite) branch (eth/handler_wit2_announces.go:173-186): (1) it unconditionally overwrites the cached announcement without verifying that the new WitnessHash matches the existing one — asymmetric with signedWitnessCache.putIfNewer which DOES reject WitnessHash conflicts at lines 313-332; (2) it increments c.perPeer[peerID] at line 178 with no perPeerCap check, defeating the explicit DoS bound the function comment commits to (the ca
Check warning on line 83 in eth/handler_wit2_bodies.go
claude / Claude Code Review
pendingWitnessBodyCache.put: needless eviction on overwrite at capacity
**Minor:** `pendingWitnessBodyCache.put` at `eth/handler_wit2_bodies.go:62-83` runs the capacity-driven eviction unconditionally, even when the put is an overwrite for a key already in the cache. When the cache is full (capacity = 10) and a second put arrives for an already-present `blockHash` — which can happen when `cacheVerifiedWitnessForServing` (manager fetch success) and `acceptSignedBroadcast` (`handler_wit.go:153`, on a near-simultaneous broadcast push) both fire for the same block — the
Check warning on line 720 in eth/handler.go
claude / Claude Code Review
Deferred WIT2 announces dropped on batch chain insert
Deferred WIT2 announces for intermediate blocks of a batched `insertChain` (e.g. downloader catch-up) are never drained: `ChainHeadEvent` fires only once per batch with `lastCanon`, and `deferredAnnouncesLoop` only drains the head's hash, so announces for blocks N+1..N+k-1 silently expire after the 30s TTL. For those blocks the byte-correctness trust property quietly degrades to unsigned WIT1 handling and the transitive relay is lost. Fix: walk the deferred queue against any header that now reso
Loading