diff --git a/cmd/server/repeater_enrich_bulk.go b/cmd/server/repeater_enrich_bulk.go index 220d4720..51494fee 100644 --- a/cmd/server/repeater_enrich_bulk.go +++ b/cmd/server/repeater_enrich_bulk.go @@ -129,7 +129,19 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin } } } - visit := func(txs []*StoreTx) { + // includeScope gates TransportedScopes accumulation: the 1-byte + // prefix-bucket fallback below folds in transmissions whose hop + // hash was NEVER resolved to this specific pubkey — MeshCore + // firmware (examples/simple_repeater/MyMesh.cpp allowPacketForward) + // only relays a TRANSPORT_FLOOD/DIRECT packet when the repeater's + // own locally configured region matches, so crediting a scope to a + // node based on nothing but a shared 1-byte hash prefix produces + // claims the protocol itself would never allow (e.g. a repeater + // hundreds of km away "transporting" a hyper-local town scope). + // RelayCount/LastRelayed/RelayActive keep the fallback — those are + // intentionally approximate "is this node active" signals, not a + // specific factual claim about which region it carried. + visit := func(txs []*StoreTx, includeScope bool) { for _, tx := range txs { if tx == nil { continue @@ -147,12 +159,12 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin if p.pt == payloadTypeAdvert { continue } - // #1751: scope accumulation is intentionally NOT gated on - // p.ok (timestamp parseability) — a packet with an - // unparseable first_seen still proves the repeater - // transported that scope. RelayCount/LastRelayed below - // remain timestamp-gated. - if tx.ScopeName != "" { + // #1751 (tightened, see includeScope doc above): scope + // accumulation is intentionally NOT gated on p.ok + // (timestamp parseability) — a packet with an unparseable + // first_seen still proves the repeater transported that + // scope, as long as the hop resolved unambiguously to it. + if includeScope && tx.ScopeName != "" { if scopeSet == nil { scopeSet = map[string]struct{}{} } @@ -180,11 +192,11 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin } } } - visit(list) + visit(list, true) if seen != nil { prefix := key[:2] if prefix != key { - visit(snap[prefix]) + visit(snap[prefix], false) } } info.TransportedScopes = sortedCappedScopes(scopeSet) diff --git a/cmd/server/repeater_liveness.go b/cmd/server/repeater_liveness.go index e2d44152..d89d9e50 100644 --- a/cmd/server/repeater_liveness.go +++ b/cmd/server/repeater_liveness.go @@ -114,6 +114,15 @@ type relayEntry struct { // scope is the tx's region scope name (transmissions.scope_name). // Empty when absent / on older schemas. Used for TransportedScopes (#1751). scope string + // fromPrefix marks entries that came from the 1-byte raw-prefix + // fallback bucket rather than this exact (resolved-pubkey) key — see + // collectRelayEntriesLocked. computeRelayInfoFromEntries must not + // accumulate `scope` for these: MeshCore firmware only relays a + // TRANSPORT_FLOOD/DIRECT packet when the repeater's own configured + // region matches (allowPacketForward in examples/simple_repeater/ + // MyMesh.cpp), so crediting a scope based on nothing but a shared + // 1-byte hash prefix asserts something the protocol wouldn't allow. + fromPrefix bool } // collectRelayEntriesLocked returns deduplicated relayEntry snapshots for @@ -147,7 +156,7 @@ func (s *PacketStore) collectRelayEntriesLocked(key string) []relayEntry { hint := len(txList) + len(prefixList) entries := make([]relayEntry, 0, hint) seen := make(map[int]bool, hint) - collect := func(list []*StoreTx) { + collect := func(list []*StoreTx, fromPrefix bool) { for _, tx := range list { if tx == nil || seen[tx.ID] { continue @@ -161,11 +170,11 @@ func (s *PacketStore) collectRelayEntriesLocked(key string) []relayEntry { if tx.RouteType != nil { rt = *tx.RouteType } - entries = append(entries, relayEntry{ts: tx.FirstSeen, pt: pt, rt: rt, scope: tx.ScopeName}) + entries = append(entries, relayEntry{ts: tx.FirstSeen, pt: pt, rt: rt, scope: tx.ScopeName, fromPrefix: fromPrefix}) } } - collect(txList) - collect(prefixList) + collect(txList, false) + collect(prefixList, true) return entries } @@ -188,8 +197,9 @@ func computeRelayInfoFromEntries(entries []relayEntry, windowHours float64) Repe } // #1751: accumulate transported scopes BEFORE the timestamp gate — // a non-advert path-hop tx proves scope transport even if its - // first_seen is unparseable. Mirrors the bulk path. - if e.scope != "" { + // first_seen is unparseable. Mirrors the bulk path. Skipped for + // fromPrefix entries — see relayEntry.fromPrefix doc. + if !e.fromPrefix && e.scope != "" { if scopeSet == nil { scopeSet = map[string]struct{}{} } diff --git a/cmd/server/transported_scopes_1751_test.go b/cmd/server/transported_scopes_1751_test.go index 49cf3706..4e9770da 100644 --- a/cmd/server/transported_scopes_1751_test.go +++ b/cmd/server/transported_scopes_1751_test.go @@ -100,14 +100,19 @@ func TestTransportedScopes_EmptyWhenNoScope(t *testing.T) { } } -// TestTransportedScopes_CrossBucketFold covers the bulk path's prefix fold: -// for a full-pubkey key it also folds in the matching 1-byte raw-prefix bucket -// (deduping by tx.ID). A scope seen only in the prefix bucket must surface on -// the full key, and a tx present in BOTH buckets must not be double-processed. -func TestTransportedScopes_CrossBucketFold(t *testing.T) { - full := scopeTx(1, 2, "region-direct") // only in the full-key bucket - prefixOnly := scopeTx(2, 2, "region-via-prefix") // only in the 1-byte bucket - shared := scopeTx(3, 2, "region-shared") // in BOTH buckets (dedup by ID) +// TestTransportedScopes_PrefixBucketExcludedFromScope covers the fix for +// the false-attribution bug found via a real stg.meshview.dk report: the +// 1-byte raw-prefix bucket (ambiguous hop hash — could be ANY node sharing +// that first byte) still folds into RelayCount/LastRelayed as before, but +// must NOT contribute to TransportedScopes. MeshCore firmware only relays a +// TRANSPORT_FLOOD/DIRECT packet when the repeater's own configured region +// matches the packet's (examples/simple_repeater/MyMesh.cpp +// allowPacketForward + RegionMap::findMatch), so crediting a scope from an +// unresolved hop would assert something the protocol itself wouldn't allow. +func TestTransportedScopes_PrefixBucketExcludedFromScope(t *testing.T) { + full := scopeTx(1, 2, "region-direct") // only in the full-key bucket — must count + prefixOnly := scopeTx(2, 2, "region-via-prefix") // only in the 1-byte bucket — must NOT count + shared := scopeTx(3, 2, "region-shared") // in BOTH buckets — must count (present in the full-key list) store := &PacketStore{ byPathHop: map[string][]*StoreTx{ @@ -118,9 +123,34 @@ func TestTransportedScopes_CrossBucketFold(t *testing.T) { } got := store.computeRepeaterRelayInfoMap(24)[scope1751Key].TransportedScopes - want := []string{"region-direct", "region-shared", "region-via-prefix"} + want := []string{"region-direct", "region-shared"} if !reflect.DeepEqual(got, want) { - t.Fatalf("cross-bucket fold TransportedScopes = %v, want %v", got, want) + t.Fatalf("TransportedScopes = %v, want %v (region-via-prefix must be excluded — it only appeared in the ambiguous prefix bucket)", got, want) + } +} + +// TestTransportedScopes_PerNodePrefixBucketExcludedFromScope is the +// per-node-path (GetRepeaterRelayInfo) counterpart to +// TestTransportedScopes_PrefixBucketExcludedFromScope, keeping the single- +// node detail endpoint (Nodes page "Transported scopes" badges) in parity +// with the bulk /api/nodes path. +func TestTransportedScopes_PerNodePrefixBucketExcludedFromScope(t *testing.T) { + full := scopeTx(1, 2, "region-direct") + prefixOnly := scopeTx(2, 2, "region-via-prefix") + shared := scopeTx(3, 2, "region-shared") + + store := &PacketStore{ + byPathHop: map[string][]*StoreTx{ + scope1751Key: {full, shared}, + scope1751Key[:2]: {prefixOnly, shared}, + }, + mu: sync.RWMutex{}, + } + + got := store.GetRepeaterRelayInfo(scope1751Key, 24).TransportedScopes + want := []string{"region-direct", "region-shared"} + if !reflect.DeepEqual(got, want) { + t.Fatalf("per-node TransportedScopes = %v, want %v (region-via-prefix must be excluded)", got, want) } }