mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-16 15:42:37 +00:00
SaarlandpowerandClaude
0352c9a287
Closes #1816. Closes #1818 (confirmed duplicate of #1816 by the triage bot). ## Root cause `byNode` is an involvement index (`indexResolvedPathHops`, `store.go:1696-1705`, #1558/#1352): a transmission is indexed under every relay-hop pubkey found in an observation's `resolved_path`, not just its originator. `getNodeClockSkewLocked` (`clock_skew.go:489`) iterated every ADVERT transaction under a pubkey without checking who actually signed it, so a relay inherited the clock skew of every broken-clock node it forwarded as if it were its own. This produced: - Fleet-wide false `no_clock`/`bimodal_clock` classifications on healthy relays whose only "bad" samples were adverts they merely relayed. - Bit-identical `RecentMedianSkewSec` "clusters" across unrelated relays that all forwarded the same broken-clock originator. - Single relays showing a multi-day skew even though their own self-adverts are healthy, because 1-2 relayed adverts from a broken originator landed in the tail of their small recent-window sample (the #1818 "island" repro from @cwichura). ## Fix Add `txOriginatedBy(tx, pubkey)`: ADVERTs are self-signed, so `decoded["pubKey"]` is the originator per protocol (case-insensitive compare as a defensive measure). Apply it as a guard in both the main skew-aggregation loop and the per-hash evidence loop in `getNodeClockSkewLocked`. `byNode` itself is untouched — #1558/#1352 still rely on the broader involvement index for other consumers. ## Tests - Existing `clock_skew_test.go` / `clock_skew_issue1094_test.go` / `clock_skew_issue1285_test.go` fixtures built synthetic ADVERT transactions without a `pubKey` field and seeded `s.byNode` directly, bypassing the normal `indexByNode` path where every real ADVERT carries `pubKey`. Added `pubKey` to each fixture so it reflects a self-originated advert, which is what these tests already intended to represent. All pre-existing tests pass unchanged in behavior. - New `clock_skew_issue1816_test.go`: - `TestTxOriginatedBy` — unit coverage of the new guard (self, foreign, missing pubKey, case-insensitivity). - `TestIssue1816_RelayDoesNotInheritOriginatorSkew` — a relay with healthy self-adverts plus relayed adverts from a broken-clock originator (matching the report's +100.5k s band) must report `ok` severity based only on its own adverts. - `TestIssue1816_PureRelaysReportNoSkew_NoBitIdenticalCluster` — five relay pubkeys that only ever forward a broken originator's advert (never self-advert) must report `nil`, not a bit-identical copy of the originator's skew. - `TestIssue1818_TwoForeignAdvertsDoNotPoisonIslandNode` — reproduces the cwichura island scenario: 8 healthy self-adverts + 2 foreign adverts at ~10 days skew must not flip severity or pollute `RecentMedianSkewSec`. Full suite: `go test ./...` passes (one pre-existing, unrelated flaky test — `TestHandleNodePaths_PrefixCollision_1352`, an index-loading race — reproduces intermittently on unmodified `master` too). Operator context: running CoreScope for SaarMesh (SaarLorLux, DE/FR/LU, 800+ nodes); this bug was surfacing as fleet-wide clock-skew false positives on our infra nodes. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>