mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-26 04:36:47 +00:00
fix: don't credit TransportedScopes from the ambiguous prefix bucket
Root-caused via a real report: "Repeaters by Region" showed a hyper-
local scope (#dk-fyn-middelfart) as transported by repeaters spread
across the whole country, which shouldn't be possible — MeshCore
firmware only relays a TRANSPORT_FLOOD/DIRECT packet when the
repeater's OWN configured region matches the packet's transport code
(examples/simple_repeater/MyMesh.cpp allowPacketForward, gated by
RegionMap::findMatch against the repeater's local region list).
Confirmed against the meshcore-dev/MeshCore source.
The bug: both TransportedScopes computation paths (bulk
computeRepeaterRelayInfoMap and per-node GetRepeaterRelayInfo) fold a
full pubkey's byPathHop entries together with its matching 1-byte
raw-prefix bucket — an intentional, existing fallback for RelayCount/
LastRelayed ("this node is probably active") that tolerates the
1-byte hash's inherent ambiguity (any node sharing that first byte
gets folded in). Applying the SAME fold to TransportedScopes asserted
something far more specific than the ambiguous signal can support,
and something the protocol itself wouldn't allow.
Scope accumulation now only happens on the exact-key (resolved,
unambiguous) pass; RelayCount/LastRelayed/RelayActive keep the
prefix-bucket fold unchanged, since those remain intentionally
approximate. Added relayEntry.fromPrefix to carry this distinction
through the per-node path, and rewrote the test that had pinned the
old (incorrect) folding behavior.
This commit is contained in:
@@ -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)
|
||||
|
||||
@@ -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{}{}
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user