dandri/meshcore-analyzer
1
0
Fork 0
mirror of https://github.com/Kpa-clawbot/meshcore-analyzer.git synced 2026-09-14 17:45:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(#1902): don't attribute transported scopes from the 1-byte hop prefix (#1903)

Browse Source 
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
2026-09-02 09:11:10 +02:00
committed by GitHub
co-authored by Claude Opus 5
parent 9bd5f5a3a2
commit 4c45dec79f
4 changed files with 69 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
cmd/server/repeater_enrich_bulk.go
+11 -4
View File
@@ -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 { for _, tx := range txs {
if tx == nil { if tx == nil {
continue continue
@@ -152,7 +155,11 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin
// unparseable first_seen still proves the repeater // unparseable first_seen still proves the repeater
// transported that scope. RelayCount/LastRelayed below // transported that scope. RelayCount/LastRelayed below
// remain timestamp-gated. // 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 { if scopeSet == nil {
scopeSet = map[string]struct{}{} scopeSet = map[string]struct{}{}
} }
@@ -180,11 +187,11 @@ func (s *PacketStore) computeRepeaterRelayInfoMap(windowHours float64) map[strin
} }
} }
} }
visit(list) visit(list, false)
if seen != nil { if seen != nil {
prefix := key[:2] prefix := key[:2]
if prefix != key { if prefix != key {
visit(snap[prefix]) visit(snap[prefix], true)
} }
} }
info.TransportedScopes = sortedCappedScopes(scopeSet) info.TransportedScopes = sortedCappedScopes(scopeSet)
cmd/server/repeater_liveness.go
+18 -6
View File
@@ -114,6 +114,11 @@ type relayEntry struct {
// scope is the tx's region scope name (transmissions.scope_name). // scope is the tx's region scope name (transmissions.scope_name).
// Empty when absent / on older schemas. Used for TransportedScopes (#1751). // Empty when absent / on older schemas. Used for TransportedScopes (#1751).
scope string 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 // 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 // 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 // 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 { func (s *PacketStore) collectRelayEntriesLocked(key string) []relayEntry {
txList := s.byPathHop[key] txList := s.byPathHop[key]
var prefixList []*StoreTx var prefixList []*StoreTx
@@ -147,7 +154,7 @@ func (s *PacketStore) collectRelayEntriesLocked(key string) []relayEntry {
hint := len(txList) + len(prefixList) hint := len(txList) + len(prefixList)
entries := make([]relayEntry, 0, hint) entries := make([]relayEntry, 0, hint)
seen := make(map[int]bool, hint) seen := make(map[int]bool, hint)
collect := func(list []*StoreTx) { collect := func(list []*StoreTx, viaPrefix bool) {
for _, tx := range list { for _, tx := range list {
if tx == nil || seen[tx.ID] { if tx == nil || seen[tx.ID] {
continue continue
@@ -161,11 +168,11 @@ func (s *PacketStore) collectRelayEntriesLocked(key string) []relayEntry {
if tx.RouteType != nil { if tx.RouteType != nil {
rt = *tx.RouteType 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(txList, false)
collect(prefixList) collect(prefixList, true)
return entries return entries
} }
@@ -189,7 +196,12 @@ func computeRelayInfoFromEntries(entries []relayEntry, windowHours float64) Repe
// #1751: accumulate transported scopes BEFORE the timestamp gate — // #1751: accumulate transported scopes BEFORE the timestamp gate —
// a non-advert path-hop tx proves scope transport even if its // a non-advert path-hop tx proves scope transport even if its
// first_seen is unparseable. Mirrors the bulk path. // 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 { if scopeSet == nil {
scopeSet = map[string]struct{}{} scopeSet = map[string]struct{}{}
} }
cmd/server/transported_scopes_1751_test.go
+39 -14
View File
@@ -100,27 +100,52 @@ func TestTransportedScopes_EmptyWhenNoScope(t *testing.T) {
} }
} }
// TestTransportedScopes_CrossBucketFold covers the bulk path's prefix fold: // TestTransportedScopes_PrefixBucketNotAttributed is the #1902 regression.
// 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 // A 1-byte hop prefix is shared by every node whose pubkey starts with that
// the full key, and a tx present in BOTH buckets must not be double-processed. // byte, so the prefix bucket holds other nodes' traffic. Scopes are a
func TestTransportedScopes_CrossBucketFold(t *testing.T) { // categorical claim about which regions a repeater serves — a 1-byte hop
full := scopeTx(1, 2, "region-direct") // only in the full-key bucket // names one of N candidates and cannot substantiate it. On the live network
prefixOnly := scopeTx(2, 2, "region-via-prefix") // only in the 1-byte bucket // this badged Belgian repeaters with #de/#de-nw purely because a German
shared := scopeTx(3, 2, "region-shared") // in BOTH buckets (dedup by ID) // 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{ store := &PacketStore{
byPathHop: map[string][]*StoreTx{ byPathHop: map[string][]*StoreTx{
scope1751Key: {full, shared}, scope1751Key: {direct, shared},
scope1751Key[:2]: {prefixOnly, shared}, scope1751Key[:2]: {foreign, shared},
}, },
mu: sync.RWMutex{}, mu: sync.RWMutex{},
} }
got := store.computeRepeaterRelayInfoMap(24)[scope1751Key].TransportedScopes // "region-foreign" is only evidenced by a 1-byte hop — it must not be
want := []string{"region-direct", "region-shared", "region-via-prefix"} // attributed. "region-shared" appears under the full key too, so it stays.
if !reflect.DeepEqual(got, want) { want := []string{"region-direct", "region-shared"}
t.Fatalf("cross-bucket fold TransportedScopes = %v, want %v", got, want)
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)
} }
} }
public/nodes.js
+1 -1
View File
@@ -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."; 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>`; 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>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>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> <tr><td>Packets Today</td><td>${stats.packetsToday || 0}</td></tr>