mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-14 10:05:37 +00:00
Fixes #1902. ## The bug `byPathHop` is keyed on the raw hop string from `path_json`, and both relay-info paths look up the full pubkey **and** fold in `key[:2]` — the 1-byte wire prefix. `TransportedScopes` (#1751) accumulated over that folded set, so every node sharing a pubkey first byte reported the same scopes. On the live network all four active nodes with prefix `f7` returned an identical set: ``` f79616... BE repeater ['#be','#be-van','#de','#de-nw','#nl'] f7e718... BE repeater ['#be','#be-van','#de','#de-nw','#nl'] f788ad... BE repeater ['#be','#be-van','#de','#de-nw','#nl'] f752c2... DE/NRW repeat. ['#be','#be-van','#de','#de-nw','#nl'] ``` Their real sets, from unambiguous full-pubkey hops over the same 7 days, are disjoint: ``` f79616... (BE) #be 471, #eu 9, #nl 6, #de 3, #be-van 1 f752c2... (DE/NRW) #de 13, #de-nw 11 f7e718... (BE) (none) ``` A sysop reads a scope badge as a statement about how their repeater is configured, so a Belgian repeater badged `#de-nw` is a wrong answer, not an imprecise one. ## The change The prefix fold stays for the counters — that is the documented #662 trade-off, "a possible over-count for clearly false zeros", and `RelayCount1h/24h`, `LastRelayed` and `UnscopedRelayCount24h` are magnitudes where an over-count is tolerable. Scopes are not a magnitude. A 1-byte hop names one of N nodes and cannot substantiate a categorical claim. Entries reached only through the prefix bucket are now flagged (`relayEntry.viaPrefix` / a `viaPrefix` argument to the bulk `visit` closure) and excluded from scope accumulation only. Both computation paths are changed together so `/api/nodes` (bulk) and the node-detail endpoint (per-node) stay in parity: - `cmd/server/repeater_liveness.go` — `collectRelayEntriesLocked` / `computeRelayInfoFromEntries` - `cmd/server/repeater_enrich_bulk.go` — `computeRepeaterRelayInfoMap` The `public/nodes.js` tooltip is updated to describe what the field now actually means. Attribution does not collapse: `observations.resolved_path` carries full pubkeys for ~27% of observations on the live instance (408k of 1.54M over 7 days), and those rows produce the correct per-node sets above. A node with no resolved hop yet shows no badge rather than a borrowed one. ## Tests `TestTransportedScopes_CrossBucketFold` pinned the old behaviour ("a scope seen only in the prefix bucket must surface on the full key"), which is the bug. It is replaced by `TestTransportedScopes_PrefixBucketNotAttributed`, which asserts on **both** paths that: 1. a scope evidenced only by a 1-byte hop is not attributed; 2. a scope also present under the full key still is; 3. `RelayCount24h` still counts all three packets — narrowing scopes must not narrow the counters, i.e. the #662 fold is untouched. Red before the change, green after. ``` cd cmd/server && go test ./... ok github.com/corescope/server 98.2s node test-packet-filter.js 92 passed, 0 failed node test-aging.js 18 passed, 0 failed node test-frontend-helpers.js 625 passed, 2 failed ``` The two frontend failures (`favStar returns filled star for favorite`, `favStar returns empty star for non-favorite`) and `cmd/ingestor`'s `TestWriteStatsAtomic_SymlinkAtDestIsReplaced` are **pre-existing** — I ran them on a pristine `upstream/master` worktree and got byte-identical results (the ingestor one is a Windows symlink-privilege limitation, not a code failure). ## Perf No new work in any loop. The bulk path gains one bool argument to an existing closure and one `&& !viaPrefix` on a branch that already ran; the per-node path gains one bool field on `relayEntry`, which is stack/slice-local and not retained. Same complexity, same allocations. ## What I could not verify end-to-end, and why I built a fixture from live data (2512 nodes, 17k transmissions, 529k observations, including all eight `f7` nodes) and ran the before/after binaries against it. Neither reproduced the live field — both returned no `transported_scopes` and `relay_count_24h: 0` for every node. That turns out to be a **separate cold-start bug**: `LoadChunked` calls `indexResolvedPathHops` per observation while scanning chunks, which adds full-pubkey keys to `byPathHop`, and then the post-load block at `cmd/server/chunked_load.go:459` calls `buildPathHopIndex()`, which begins with `s.byPathHop = make(...)` and rebuilds from raw hops only. Every resolved full-pubkey key from the scan is discarded: ``` [store] Built path-hop index: 2924 unique keys <- raw hops only [store] LoadChunked: 17056 transmissions (527331 observations) ``` So on a freshly started server the full-pubkey buckets are empty and only refill from live ingestion. That is being filed separately; it is orthogonal to this change, but it does mean `transported_scopes` will be sparse for a while after any restart until it is fixed. This PR is therefore verified by unit tests on both computation paths plus the live-data derivation above, not by a local end-to-end run. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
efiten
@@ -129,7 +129,10 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin
|
||||
}
|
||||
}
|
||||
}
|
||||
visit := func(txs []*StoreTx) {
|
||||
// viaPrefix marks the 1-byte wire-prefix bucket, shared by every
|
||||
// node with the same first pubkey byte. See collectRelayEntriesLocked
|
||||
// for the counters-vs-scopes split (#662 / #1902).
|
||||
visit := func(txs []*StoreTx, viaPrefix bool) {
|
||||
for _, tx := range txs {
|
||||
if tx == nil {
|
||||
continue
|
||||
@@ -152,7 +155,11 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin
|
||||
// unparseable first_seen still proves the repeater
|
||||
// transported that scope. RelayCount/LastRelayed below
|
||||
// remain timestamp-gated.
|
||||
if tx.ScopeName != "" {
|
||||
//
|
||||
// #1902: it IS gated on full-pubkey attribution — a 1-byte
|
||||
// hop cannot prove which of the nodes sharing that byte
|
||||
// carried the packet.
|
||||
if tx.ScopeName != "" && !viaPrefix {
|
||||
if scopeSet == nil {
|
||||
scopeSet = map[string]struct{}{}
|
||||
}
|
||||
@@ -180,11 +187,11 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin
|
||||
}
|
||||
}
|
||||
}
|
||||
visit(list)
|
||||
visit(list, false)
|
||||
if seen != nil {
|
||||
prefix := key[:2]
|
||||
if prefix != key {
|
||||
visit(snap[prefix])
|
||||
visit(snap[prefix], true)
|
||||
}
|
||||
}
|
||||
info.TransportedScopes = sortedCappedScopes(scopeSet)
|
||||
|
||||
@@ -114,6 +114,11 @@ 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
|
||||
// viaPrefix marks an entry that was only reachable through the 1-byte
|
||||
// wire-prefix bucket, i.e. some node whose pubkey starts with the same
|
||||
// byte carried it — not necessarily this one. Counters accept that
|
||||
// ambiguity (#662); scope attribution does not (#1902).
|
||||
viaPrefix bool
|
||||
}
|
||||
|
||||
// collectRelayEntriesLocked returns deduplicated relayEntry snapshots for
|
||||
@@ -128,7 +133,9 @@ type relayEntry struct {
|
||||
//
|
||||
// The 1-byte prefix lookup CAN over-count when multiple nodes share the
|
||||
// same first byte. This trades a possible over-count for clearly false
|
||||
// zeros (issue #662).
|
||||
// zeros (issue #662). Entries reached only that way are flagged viaPrefix
|
||||
// so TransportedScopes can refuse them (#1902) while the counters keep the
|
||||
// trade-off.
|
||||
func (s *PacketStore) collectRelayEntriesLocked(key string) []relayEntry {
|
||||
txList := s.byPathHop[key]
|
||||
var prefixList []*StoreTx
|
||||
@@ -147,7 +154,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, viaPrefix bool) {
|
||||
for _, tx := range list {
|
||||
if tx == nil || seen[tx.ID] {
|
||||
continue
|
||||
@@ -161,11 +168,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, viaPrefix: viaPrefix})
|
||||
}
|
||||
}
|
||||
collect(txList)
|
||||
collect(prefixList)
|
||||
collect(txList, false)
|
||||
collect(prefixList, true)
|
||||
return entries
|
||||
}
|
||||
|
||||
@@ -189,7 +196,12 @@ 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 != "" {
|
||||
//
|
||||
// #1902: but only when the tx named this node by its full pubkey.
|
||||
// A 1-byte hop names one of every node sharing that byte, which is
|
||||
// enough for an approximate count and not enough to claim the node
|
||||
// serves a region.
|
||||
if e.scope != "" && !e.viaPrefix {
|
||||
if scopeSet == nil {
|
||||
scopeSet = map[string]struct{}{}
|
||||
}
|
||||
|
||||
@@ -100,27 +100,52 @@ 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_PrefixBucketNotAttributed is the #1902 regression.
|
||||
//
|
||||
// A 1-byte hop prefix is shared by every node whose pubkey starts with that
|
||||
// byte, so the prefix bucket holds other nodes' traffic. Scopes are a
|
||||
// categorical claim about which regions a repeater serves — a 1-byte hop
|
||||
// names one of N candidates and cannot substantiate it. On the live network
|
||||
// this badged Belgian repeaters with #de/#de-nw purely because a German
|
||||
// repeater shared their first pubkey byte.
|
||||
//
|
||||
// So TransportedScopes must come from the full-key bucket only, while the
|
||||
// counters keep the #662 prefix fold (over-count beats false zeros).
|
||||
// Asserted on BOTH computation paths so they stay in parity.
|
||||
func TestTransportedScopes_PrefixBucketNotAttributed(t *testing.T) {
|
||||
direct := scopeTx(1, 2, "region-direct") // only in the full-key bucket
|
||||
foreign := scopeTx(2, 2, "region-foreign") // only in the 1-byte bucket
|
||||
shared := scopeTx(3, 2, "region-shared") // in BOTH buckets (dedup by ID)
|
||||
|
||||
store := &PacketStore{
|
||||
byPathHop: map[string][]*StoreTx{
|
||||
scope1751Key: {full, shared},
|
||||
scope1751Key[:2]: {prefixOnly, shared},
|
||||
scope1751Key: {direct, shared},
|
||||
scope1751Key[:2]: {foreign, shared},
|
||||
},
|
||||
mu: sync.RWMutex{},
|
||||
}
|
||||
|
||||
got := store.computeRepeaterRelayInfoMap(24)[scope1751Key].TransportedScopes
|
||||
want := []string{"region-direct", "region-shared", "region-via-prefix"}
|
||||
if !reflect.DeepEqual(got, want) {
|
||||
t.Fatalf("cross-bucket fold TransportedScopes = %v, want %v", got, want)
|
||||
// "region-foreign" is only evidenced by a 1-byte hop — it must not be
|
||||
// attributed. "region-shared" appears under the full key too, so it stays.
|
||||
want := []string{"region-direct", "region-shared"}
|
||||
|
||||
bulk := store.computeRepeaterRelayInfoMap(24)[scope1751Key]
|
||||
if !reflect.DeepEqual(bulk.TransportedScopes, want) {
|
||||
t.Fatalf("bulk TransportedScopes = %v, want %v (prefix-bucket scopes excluded)", bulk.TransportedScopes, want)
|
||||
}
|
||||
|
||||
perNode := store.GetRepeaterRelayInfo(scope1751Key, 24)
|
||||
if !reflect.DeepEqual(perNode.TransportedScopes, want) {
|
||||
t.Fatalf("per-node TransportedScopes = %v, want %v (prefix-bucket scopes excluded)", perNode.TransportedScopes, want)
|
||||
}
|
||||
|
||||
// The #662 fold itself is untouched: all three in-window packets still
|
||||
// count, deduped by tx ID. Narrowing scopes must not narrow the counters.
|
||||
if bulk.RelayCount24h != 3 {
|
||||
t.Fatalf("bulk RelayCount24h = %d, want 3 (prefix fold must still feed the counters)", bulk.RelayCount24h)
|
||||
}
|
||||
if perNode.RelayCount24h != 3 {
|
||||
t.Fatalf("per-node RelayCount24h = %d, want 3 (prefix fold must still feed the counters)", perNode.RelayCount24h)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+1
-1
@@ -673,7 +673,7 @@
|
||||
const btooltip = "Normalized betweenness centrality (0..1). How often this node sits on the shortest path between other pairs of nodes in the affinity graph. 1.0 = the most structurally critical node on the mesh. High Bridge + low Traffic share = a quiet but irreplaceable chokepoint.";
|
||||
return `<tr id="row-bridge-score" data-bridge-score="${b.toFixed(4)}"><td title="${btooltip}">Bridge score <span style="color:var(--text-muted);cursor:help" aria-label="help">ⓘ</span></td><td><span style="display:inline-block;vertical-align:middle;width:80px;height:8px;background:var(--bg-secondary,#333);border-radius:4px;overflow:hidden;margin-right:6px"><span style="display:block;width:${bbarWidth}%;height:100%;background:${bcolor}"></span></span><span style="color:${bcolor};font-weight:600">${bpct}%</span> <span style="color:var(--text-muted);font-size:11px;margin-left:4px">${blabel}</span></td></tr>`;
|
||||
})() : ''}
|
||||
${(n.role === 'repeater' || n.role === 'room') && Array.isArray(n.transported_scopes) && n.transported_scopes.length ? `<tr id="row-transported-scopes"><td title="Distinct region scopes (transmissions.scope_name) of all non-advert packets in which this repeater appears as a path hop. Shows which regions' traffic this repeater has carried (#1751).">Transported scopes</td><td><span style="display:inline-flex;flex-wrap:wrap;gap:3px;vertical-align:middle">${n.transported_scopes.map(sc => '<span class="badge">' + escapeHtml(String(sc)) + '</span>').join('')}</span></td></tr>` : ''}
|
||||
${(n.role === 'repeater' || n.role === 'room') && Array.isArray(n.transported_scopes) && n.transported_scopes.length ? `<tr id="row-transported-scopes"><td title="Distinct region scopes (transmissions.scope_name) of the non-advert packets whose path names this repeater by its full pubkey. Shows which regions' traffic it has carried (#1751). Packets that only carry a 1-byte hop are excluded: that byte is shared by every node with the same pubkey prefix, so it cannot say which of them relayed (#1902).">Transported scopes</td><td><span style="display:inline-flex;flex-wrap:wrap;gap:3px;vertical-align:middle">${n.transported_scopes.map(sc => '<span class="badge">' + escapeHtml(String(sc)) + '</span>').join('')}</span></td></tr>` : ''}
|
||||
<tr><td>First Seen</td><td>${renderNodeTimestampHtml(n.first_seen)}</td></tr>
|
||||
<tr><td>Total Packets</td><td>${stats.totalTransmissions || stats.totalPackets || n.advert_count || 0}${stats.totalObservations && stats.totalObservations !== (stats.totalTransmissions || stats.totalPackets) ? ' <span class="text-muted" style="font-size:0.85em">(seen ' + stats.totalObservations + '×)</span>' : ''}</td></tr>
|
||||
<tr><td>Packets Today</td><td>${stats.packetsToday || 0}</td></tr>
|
||||
|
||||
Reference in New Issue
Block a user