p2p/sentry: don't duplicate SendMessageById across shared-store sentries

[yperbasis]

Summary

Fixes the hive devp2p BlobViolations flake (expected disconnect on blob violation, got msg code: 24) seen on the CI Gate run for #21524 (parallel leg) and on a runner-experiment branch the day before (serial leg) — and the gossip-duplication regression behind it.

Root cause

Since #21335 all per-eth-version sentry GrpcServers share one p2p.Server and one PeerStore. SendMessageById resolves the target peer in the shared store, so callers that fan a by-id send out across every sentry client — e.g. the txpool's new-peer pool sync (PropagatePooledTxnsToPeersList) — now deliver the same frame once per sentry: three identical NewPooledTransactionHashes messages per new peer on the default eth/69+70+71 build. Before #21335 each sentry had its own peer set, so the off-sentry sends were silent no-ops.

SendMessageToAll and SendMessageToRandomPeers already guard against this with protocolVersions.Contains(peerInfo.EthProtocol()); SendMessageById was the only outbound path without the negotiated-version check.

The BlobViolations failure sequence, reproduced locally with wire-level logging (hive --sim devp2p --sim.limit eth against an instrumented image; failed on the second loop iteration with the exact CI error):

1. The test peers, announces two blob txs (one with a lying size/type), delivers them, and waits for a disconnect.
2. The violating PooledTransactions reply sits in the txpool fetcher's 250 ms inbound batch before the announcement check kicks the peer.
3. If the 5 s syncToNewPeersEvery tick lands in that window, the all-pool announcement (~70 KB — about 2000 hashes accumulated by earlier suite tests) is written to the test peer three times:

08:16:32.606  inbound NPTH68            peer=de6008eb count=2
08:16:32.607  WRITE GET_POOLED_TXS_66   peer=de6008eb
08:16:32.607  sync-to-new-peers         entries=2002            <- 5s tick fires
08:16:32.608  WRITE NPTH68 70084 bytes  peer=de6008eb           <- one write
08:16:32.608  WRITE NPTH68 70084 bytes  peer=de6008eb           <- per
08:16:32.608  WRITE NPTH68 70084 bytes  peer=de6008eb           <- sentry
08:16:32.855  VIOLATION -> PenalizePeer peer=de6008eb           <- next 250ms batch flush

4. The devp2p test deliberately tolerates one pre-disconnect hash announcement (go-ethereum commit 88c8459, Sept 2024, "sometimes we'll get a blob transaction hashes announcement before the disconnect"); the duplicated second copy arrives before the kick and fails the test.

In the failed CI run the timing lines up exactly: the txpool started at 06:33:04.64 and the test failed at 06:33:14.644 — the tick+10s sync.

With the duplication fixed, at most one announcement can precede the disconnect in that window, which the test tolerates by design. Beyond the test, every new peer was receiving the full pool sync in triplicate.

Fix

Apply the same negotiated-eth-version guard in SendMessageById that the other outbound paths use. Eth-protocol messages only; wit is unaffected (it is deduplicated to a single GrpcServer at shared-server construction). This also restores the routing contract documented at FetchBlockAccessLists ("sentryIndex MUST be the sentry where peerID is actually connected"): sends via a non-matching sentry are silent no-ops again, as they were with per-sentry peer sets.

Not present on release/3.4 (#21335 is main-only), so no backport is needed.

Testing

• New regression test TestGrpcServer_SendMessageById_SharedStore_NoDuplicateWrites (TDD): three GrpcServers (eth/69/70/71) sharing a PeerStore, peer negotiated eth/70, by-id fan-out across all three must produce exactly one write. Red before the fix (expected: 1, actual: 3), green after.
• go test ./p2p/sentry/... ./txnprovider/txpool/... clean.
• Local hive validation in a loop: before the fix not ok 18 BlobViolations reproduced on iteration 2; after the fix 12/12 iterations green, with instrumented logs showing exactly one NEW_POOLED_TRANSACTION_HASHES_68 write per new peer where there were three.
• make lint (repeatedly) and make erigon integration clean.

[Copilot]

Pull request overview

This PR fixes an outbound message duplication bug introduced by shared PeerStore usage across multiple per-eth-version sentry GrpcServers. The key change ensures SendMessageById only writes eth-protocol frames via the sentry instance matching the peer’s negotiated eth protocol version, aligning it with the existing routing behavior of SendMessageToAll / SendMessageToRandomPeers.

Changes:

• Add an eth-version routing guard to GrpcServer.SendMessageById to avoid duplicate writes when multiple sentries share a PeerStore.
• Add a regression test that constructs multiple eth-version GrpcServers sharing one PeerStore and verifies a by-id fan-out results in exactly one write.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
p2p/sentry/sentry_grpc_server.go	Adds negotiated-eth-version filtering to SendMessageById for eth-protocol messages to prevent duplicate writes in shared-store mode.
p2p/sentry/sentry_grpc_server_test.go	Adds a regression test ensuring shared-store SendMessageById fan-out does not duplicate outbound writes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.