From 079e73aa4cbbfb753f2f13f6dda8c0b4993243e5 Mon Sep 17 00:00:00 2001 From: efiten Date: Wed, 9 Sep 2026 16:00:15 +0200 Subject: [PATCH] fix(scope-audit): attribute forwarding to every hop, and pay for the wider scan (#1986) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Scope Audit credits a transmission to `path[last]` only. On a flood-family route every forwarder appends its own hash to the END of the path (`internal/packetpath/route.go`), so `path[last]` does not mean "forwarded this packet", it means "was the transmission an uplinked observer heard directly". Every earlier hop forwarded the same packet and is discarded. The last-hop rule is genuinely required for DIRECT routes, which consume hops from the front, so their `path[last]` is the route’s far end rather than the transmitter. But `scopeAuditForwarderScanQuery` already restricts to `route_type IN (0, 1)` via `scopeConformanceForwarderRouteTypesSQL`, where that hazard cannot arise, so inside this query the restriction only throws evidence away. ## What it costs the page today Measured on a live-shaped instance, 206 declared repeaters, 965k transmissions, 7d window: | | before | after | |---|---|---| | repeaters with no attributable evidence of any kind | 133 of 205 (65%) | 30 of 206 (15%) | On a 1000-packet flood sample the mean path length is 7.08 hops, so the last-hop rule keeps 394 of 2789 hop observations (14%), and 85% of the nodes seen forwarding never appear as a last hop at all. Those repeaters have every region they declare reported as "declared, not observed", which is the page presenting a gap in our own attribution as a finding about someone else’s repeater. ## Rule 0: what widening it costs, and what pays for it Reading every hop multiplies the rows the scan returns: a 7d window yields **3,470,188 hop rows** from 1,368,761 observations carrying a path. Cold cost before this change was 16.7s for 7d and 4.0s for 24h, of which SQLite accounts for 2.7s. The rest was the Go side reading rows. Three changes, in order of what they bought: 1. **The scan carried `scope_name` and `first_seen` on every hop row.** Both are columns of `transmissions`, and at 43 hop rows per transmission the same two values were re-read that many times. They now come from one query over the same window keyed by transmission id, both inside one read transaction so a transmission arriving between them cannot appear in the hop scan with no metadata to attribute it by. The hop scan carries two columns instead of four. 2. **The hop is lower-cased into a stack buffer** instead of through `strings.ToLower`. 1,026,814 of the 1,284,897 hops in a 24h window are stored uppercase, because `packetpath.DecodePathFromRawHex` writes them that way, and the great majority match no declared target, so that allocation was paid millions of times to answer "no". The `(target, txID)` de-duplication key became a struct for the same reason. 3. **The compute ran outside the cache mutex**, so every request arriving on a cold window ran its own full scan concurrently. It now sits behind a singleflight, the same treatment `/api/observers` and `/api/nodes/{pubkey}/reach` already have, and the 7d window gets a 5 minute TTL while 1h and 24h keep 30s. At 30s a single reader with 7d open keeps the instance recomputing more than half the time, for an aggregate that moves at the pace of a week of traffic. Result, warm process: | window | before | after | |---|---|---| | 1h | 0.155s | 0.140s | | 24h | 4.04s | 2.79-2.89s across six samples | | 7d | 16.7s | 11.6s | | repeat inside TTL | ~1ms | ~1ms | **Rejected alternatives, measured on the same database**, so the next reader does not have to re-derive them: | approach | rows returned | time in SQLite | |---|---|---| | the query as written | 3,470,188 | 2.7s | | pre-filter on the declared targets’ first 4 hex chars | 1,971,126 | 20.9s | | `GROUP BY t.id, hop` | 965,025 | 38.0s | | `SELECT DISTINCT t.id, path_json` | 1,229,966 | 17.7s | The query plan is already index-driven (`idx_transmissions_first_seen`, then `idx_observations_tx_ts`), so there is no missing index behind this: the rows are inherent to the data. Note for anyone attempting a hop comparison in SQL: a case-sensitive comparison silently drops most attributable hops, per the 80% figure above. ## Tests - a mid-path hop is attributed (the case behind the 65% blind spot) - a DIRECT transmission whose `path[last]` **is** the target is still not attributed. With the last-hop rule gone this is the only thing standing between the audit and misattribution, so it gets its own test rather than relying on the route filter being obvious - one transmission counted once per target even when it appears on several hops of the same path, which the `(target, txID)` de-duplication now carries alone - a hop longer than the 4-char floor resolved by its own length, which nothing pinned before: every other test seeds 4-char hops - the per-window TTL, so collapsing it back to one constant has to delete the reason - a second request inside the TTL served from cache rather than recomputed. The cache path had no test at all `cd cmd/server && go test ./...` passes (168s), `go vet` and `gofmt -l` clean. Server-side only, no API shape change, no frontend change. Browser validation: run against a live instance carrying this change, the Scope Audit renders 220 rows matching the API row for row, and the per-node scopes page still answers with its route-type mix. --- cmd/server/routes.go | 1 + cmd/server/scope_audit.go | 339 +++++++++++++++++++++++++++++---- cmd/server/scope_audit_test.go | 231 ++++++++++++++++++++++ 3 files changed, 533 insertions(+), 38 deletions(-) diff --git a/cmd/server/routes.go b/cmd/server/routes.go index c83fde8a..f9d6f1c5 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -75,6 +75,7 @@ type Server struct { scopeAuditMu sync.Mutex scopeAuditCache map[string]*ScopeAuditResponse scopeAuditCachedAt map[string]time.Time + scopeAuditSF singleflight.Group // #1975: /api/scope-audit window cache and its single-flight guard, so a // burst of viewers on a cold cache recomputes the network-wide scan once diff --git a/cmd/server/scope_audit.go b/cmd/server/scope_audit.go index d1aa719e..32249c8f 100644 --- a/cmd/server/scope_audit.go +++ b/cmd/server/scope_audit.go @@ -24,7 +24,6 @@ import ( "fmt" "net/http" "sort" - "strconv" "strings" "sync" "sync/atomic" @@ -320,6 +319,14 @@ func scopeAuditPrefixIndex(targets []string) map[int]map[string][]string { // call per pubkey — fine for one node, but 37+ repeater-sized loop of them // would each re-scan the same first_seen index range), this scans the // FLOOD-family window exactly once and returns every forwarder hop found. +// +// "Every forwarder hop" means every hop of the path, not only path[last] — +// see scopeConformanceQuery's doc comment for why, and note that this query +// returns one ROW PER HOP, so a single transmission now yields as many rows as +// it has hops (mean 7.08 on the live network). ScopeAuditForwarding's +// "|" de-duplication is what keeps that from counting a +// transmission twice for one target, and it is now load-bearing rather than +// belt-and-braces: one path can carry the same target on several hops. // It applies the SAME three conditions scopeConformanceQuery does — // minForwarderHopHexLen, scopeConformanceForwarderRouteTypesSQL, and the // explicit json_valid guard against a single malformed path_json row @@ -329,10 +336,10 @@ func scopeAuditPrefixIndex(targets []string) map[int]map[string][]string { // scopeAuditPrefixIndex, so the SQL cost stays O(rows in window) regardless // of len(targets). var scopeAuditForwarderScanQuery = ` - SELECT t.id, je.value, t.scope_name, t.first_seen + SELECT t.id, je.value FROM transmissions t JOIN observations o ON o.transmission_id = t.id - JOIN json_each(o.path_json) je ON je.key = json_array_length(o.path_json) - 1 + JOIN json_each(o.path_json) je WHERE t.first_seen >= ? AND ` + scopeConformanceForwarderRouteTypesSQL + ` AND o.path_json IS NOT NULL @@ -341,6 +348,26 @@ var scopeAuditForwarderScanQuery = ` AND LENGTH(je.value) >= ` + fmt.Sprint(minForwarderHopHexLen) + ` ` +// scopeAuditWindowMetaQuery reads the two per-TRANSMISSION facts the hop scan +// used to carry on every hop row: the scope name and the timestamp. It applies +// the identical window and route-type filter, so it covers every transmission +// the hop scan can produce, and both run inside one read transaction so the +// two see the same snapshot. +// +// Splitting these out is why the hop scan carries two columns instead of four. +// Measured on the live-shaped staging database on 2026-09-07, a 7d window +// yields 3,470,188 hop rows against 79,652 transmissions: 43 hop rows per +// transmission, each of which was re-reading the same scope_name and +// first_seen. SQLite spends 2.7s of the 16.7s that window cost; the rest was +// the Go side scanning columns it already knew. +// +// Every SQL-side attempt to shrink the hop scan itself measured worse on that +// same database and was rejected: a first-4-hex prefix filter against the +// declared targets takes 20.9s (and needs lower() on both sides, because 80% +// of stored hops are uppercase), GROUP BY t.id, hop takes 38.0s, and +// SELECT DISTINCT t.id, path_json takes 17.7s. The 3.47M rows are inherent: +// 1,368,761 observations carrying a path, ~2.5 usable hops each. + // ScopeAuditForwarding runs scopeAuditForwarderScanQuery once for the whole // window and attributes every forwarder hop it finds to targets, by the same // truncated-hash prefix match ScopeConformance uses for a single pubkey. @@ -362,11 +389,159 @@ var scopeAuditForwarderScanQuery = ` // the same de-duplication scopeConformanceQuery gets for free from EXISTS, // done explicitly here since this scan is not correlated per target. +// scopeAuditWindowMetaQuery reads the two per-TRANSMISSION facts the hop scan +// used to carry on every hop row: the scope name and the timestamp. It applies +// the identical window and route-type filter, so it covers every transmission +// the hop scan can produce, and both run inside one read transaction so the +// two see the same snapshot. +// +// Splitting these out is why the hop scan carries two columns instead of four. +// Measured on the live-shaped staging database on 2026-09-07, a 7d window +// yields 3,470,188 hop rows against 79,652 transmissions: 43 hop rows per +// transmission, each of which was re-reading the same scope_name and +// first_seen. SQLite spends 2.7s of the 16.7s that window cost; the rest was +// the Go side scanning columns it already knew. +// +// Every SQL-side attempt to shrink the hop scan itself measured worse on that +// same database and was rejected: a first-4-hex prefix filter against the +// declared targets takes 20.9s (and needs lower() on both sides, because 80% +// of stored hops are uppercase), GROUP BY t.id, hop takes 38.0s, and +// SELECT DISTINCT t.id, path_json takes 17.7s. The 3.47M rows are inherent: +// 1,368,761 observations carrying a path, ~2.5 usable hops each. +var scopeAuditWindowMetaQuery = ` + SELECT t.id, t.scope_name, t.first_seen + FROM transmissions t + WHERE t.first_seen >= ? + AND ` + scopeConformanceForwarderRouteTypesSQL + ` +` + +// scopeAuditTxMeta is one transmission's contribution to the aggregate, held +// once per transmission rather than once per hop. + +// scopeAuditTxMeta is one transmission's contribution to the aggregate, held +// once per transmission rather than once per hop. +type scopeAuditTxMeta struct { + scopeName sql.NullString + firstSeen string +} + +// scopeAuditWindowMeta loads scopeAuditWindowMetaQuery into a map keyed by +// transmission id. Runs on the caller's transaction so it shares the hop +// scan's snapshot. + +// scopeAuditWindowMeta loads scopeAuditWindowMetaQuery into a map keyed by +// transmission id. Runs on the caller's transaction so it shares the hop +// scan's snapshot. +func scopeAuditWindowMeta(tx *sql.Tx, sinceISO string) (map[int64]scopeAuditTxMeta, error) { + rows, err := tx.Query(scopeAuditWindowMetaQuery, sinceISO) + if err != nil { + return nil, fmt.Errorf("scope audit window meta: %w", err) + } + defer rows.Close() + + meta := map[int64]scopeAuditTxMeta{} + for rows.Next() { + var id int64 + var m scopeAuditTxMeta + if err := rows.Scan(&id, &m.scopeName, &m.firstSeen); err != nil { + return nil, fmt.Errorf("scope audit window meta scan: %w", err) + } + meta[id] = m + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("scope audit window meta rows: %w", err) + } + return meta, nil +} + +// scopeAuditSeenKey identifies one (target, transmission) pair for the +// de-duplication below. A struct key rather than the string it used to be +// built into: the hop scan reaches millions of rows on a 7d window, and every +// candidate hop was allocating a fresh "|" string to ask a +// question that a comparable struct answers without allocating. + +// scopeAuditSeenKey identifies one (target, transmission) pair for the +// de-duplication below. A struct key rather than the string it used to be +// built into: the hop scan reaches millions of rows on a 7d window, and every +// candidate hop was allocating a fresh "|" string to ask a +// question that a comparable struct answers without allocating. +type scopeAuditSeenKey struct { + target string + txID int64 +} + +// ScopeAuditForwarding runs scopeAuditForwarderScanQuery once for the whole +// window and attributes every forwarder hop it finds to targets, by the same +// truncated-hash prefix match ScopeConformance uses for a single pubkey. +// +// A hop is attributed only when its prefix matches EXACTLY ONE declared +// target. This endpoint exists to find a repeater that declares a region and +// is not actually forwarding it — crediting a hop to every target sharing +// its prefix would let a colliding neighbour's traffic silently paper over a +// real gap, and crediting nobody (the alternative of dropping the hop +// entirely) would invent failures for targets that simply share a collision- +// prone prefix. Instead, an ambiguous hop is credited to NEITHER candidate, +// and every candidate's ambiguousHops counter is incremented instead, so the +// row can say "this notObserved might just be a prefix collision" rather +// than presenting it as a confirmed finding. See scopeAuditTargetAgg's +// ambiguousHops field and ScopeAuditRow.AmbiguousHops. +// +// Each (target, transmission) pair is counted at most once even if seen via +// multiple observations OR via several hops of one path (a routing loop, or two +// hops colliding on the same truncated prefix), for both the attributed and the +// ambiguous count — the same de-duplication scopeConformanceQuery gets for free +// from EXISTS, done explicitly here since this scan is not correlated per +// target. Since the scan reads every hop rather than only path[last], this is +// the only thing keeping one transmission from counting several times for the +// same target; TestScopeAuditForwardingCountsOneTransmissionOncePerTarget pins +// it. + +// ScopeAuditForwarding runs scopeAuditForwarderScanQuery once for the whole +// window and attributes every forwarder hop it finds to targets, by the same +// truncated-hash prefix match ScopeConformance uses for a single pubkey. +// +// A hop is attributed only when its prefix matches EXACTLY ONE declared +// target. This endpoint exists to find a repeater that declares a region and +// is not actually forwarding it — crediting a hop to every target sharing +// its prefix would let a colliding neighbour's traffic silently paper over a +// real gap, and crediting nobody (the alternative of dropping the hop +// entirely) would invent failures for targets that simply share a collision- +// prone prefix. Instead, an ambiguous hop is credited to NEITHER candidate, +// and every candidate's ambiguousHops counter is incremented instead, so the +// row can say "this notObserved might just be a prefix collision" rather +// than presenting it as a confirmed finding. See scopeAuditTargetAgg's +// ambiguousHops field and ScopeAuditRow.AmbiguousHops. +// +// Each (target, transmission) pair is counted at most once even if seen via +// multiple observations OR via several hops of one path (a routing loop, or two +// hops colliding on the same truncated prefix), for both the attributed and the +// ambiguous count — the same de-duplication scopeConformanceQuery gets for free +// from EXISTS, done explicitly here since this scan is not correlated per +// target. Since the scan reads every hop rather than only path[last], this is +// the only thing keeping one transmission from counting several times for the +// same target; TestScopeAuditForwardingCountsOneTransmissionOncePerTarget pins +// it. func (s *PacketStore) ScopeAuditForwarding(sinceISO string, targets []string) (map[string]*scopeAuditTargetAgg, error) { byLen := scopeAuditPrefixIndex(targets) result := make(map[string]*scopeAuditTargetAgg, len(targets)) - rows, err := s.db.conn.Query(scopeAuditForwarderScanQuery, sinceISO) + // One read transaction for both queries. The hop scan and the per- + // transmission metadata are two passes over the same window, and a + // transmission arriving between them would otherwise appear in the hop scan + // with no metadata to attribute it by — rare, but the fix is a shared + // snapshot rather than a rule about what to do with the leftovers. + tx, err := s.db.conn.Begin() + if err != nil { + return nil, fmt.Errorf("scope audit forwarder scan begin: %w", err) + } + defer tx.Rollback() + + meta, err := scopeAuditWindowMeta(tx, sinceISO) + if err != nil { + return nil, err + } + + rows, err := tx.Query(scopeAuditForwarderScanQuery, sinceISO) if err != nil { return nil, fmt.Errorf("scope audit forwarder scan: %w", err) } @@ -381,21 +556,43 @@ func (s *PacketStore) ScopeAuditForwarding(sinceISO string, targets []string) (m return agg } - seen := make(map[string]bool) // "|" already counted (attributed or ambiguous) + seen := make(map[scopeAuditSeenKey]bool) // (target, txID) already counted (attributed or ambiguous) + + // hopBuf lower-cases the hop in place instead of through strings.ToLower. + // 80% of the hops in this database are stored uppercase (1,026,814 of + // 1,284,897 in a 24h window, measured 2026-09-07) because + // packetpath.DecodePathFromRawHex writes them that way, and the great + // majority of them match no declared target at all — so the allocation + // ToLower makes is paid millions of times to answer "no". byLen's keys are + // lowercase, and a map index expression on string(bytes) does not allocate. + var hopBuf [64]byte for rows.Next() { var txID int64 - var hop string - var scopeName sql.NullString - var firstSeen string - if err := rows.Scan(&txID, &hop, &scopeName, &firstSeen); err != nil { + var hopRaw sql.RawBytes + if err := rows.Scan(&txID, &hopRaw); err != nil { return nil, fmt.Errorf("scope audit forwarder scan scan: %w", err) } - hop = strings.ToLower(hop) - candidates := byLen[len(hop)][hop] + n := len(hopRaw) + if n > len(hopBuf) { + // Longer than a full pubkey: cannot be any target's prefix. The + // SQL floor guards the short end, this guards the long one. + continue + } + for i := 0; i < n; i++ { + c := hopRaw[i] + if 'A' <= c && c <= 'Z' { + c += 'a' - 'A' + } + hopBuf[i] = c + } + candidates := byLen[n][string(hopBuf[:n])] + if len(candidates) == 0 { + continue + } if len(candidates) > 1 { for _, target := range candidates { - key := target + "|" + strconv.FormatInt(txID, 10) + key := scopeAuditSeenKey{target: target, txID: txID} if seen[key] { continue } @@ -404,8 +601,16 @@ func (s *PacketStore) ScopeAuditForwarding(sinceISO string, targets []string) (m } continue } + txMeta, ok := meta[txID] + if !ok { + // Impossible while both queries share one snapshot and one WHERE + // clause; treated as "nothing to attribute" rather than silently + // counted as unscoped, which is what a zero-valued meta would do. + continue + } + scopeName, firstSeen := txMeta.scopeName, txMeta.firstSeen for _, target := range candidates { - key := target + "|" + strconv.FormatInt(txID, 10) + key := scopeAuditSeenKey{target: target, txID: txID} if seen[key] { continue } @@ -685,8 +890,6 @@ func nodeScopesWindowLookback(window string) (time.Duration, bool) { // pass — see scopes.go's AllCurrentDeclaredRegions and ScopeAuditForwarding // for why that stays a single scan rather than one query per repeater. func (s *Server) handleScopeAudit(w http.ResponseWriter, r *http.Request) { - const scopeAuditTTL = 30 * time.Second - window := r.URL.Query().Get("window") if window == "" { window = "24h" @@ -697,34 +900,103 @@ func (s *Server) handleScopeAudit(w http.ResponseWriter, r *http.Request) { return } - s.scopeAuditMu.Lock() - if s.scopeAuditCache != nil { - if cached, ok := s.scopeAuditCache[window]; ok && time.Since(s.scopeAuditCachedAt[window]) < scopeAuditTTL { - s.scopeAuditMu.Unlock() - writeJSON(w, cached) - return - } - } - s.scopeAuditMu.Unlock() + sinceISO := time.Now().Add(-lookback).UTC().Format(time.RFC3339) - declared, err := s.db.AllCurrentDeclaredRegions() + if cached, ok := s.scopeAuditCached(window); ok { + writeJSON(w, cached) + return + } + + // singleflight: the compute below runs outside the cache mutex, so without + // this every request arriving on a cold window ran its own full scan + // concurrently. Reading every hop makes that scan seconds of work over + // millions of rows, which is the shape that turns a thundering herd from + // wasteful into expensive. Same treatment /api/observers and + // /api/nodes/{pubkey}/reach already have. + v, err, _ := s.scopeAuditSF.Do(window, func() (interface{}, error) { + // Waiters that arrive while a scan is in flight are served by that + // scan's result; this second look is for the caller that acquires the + // group right after a winner stored one. + if cached, ok := s.scopeAuditCached(window); ok { + return cached, nil + } + resp, cErr := s.computeScopeAudit(window, sinceISO) + if cErr != nil { + return nil, cErr + } + s.scopeAuditStore(window, resp) + return resp, nil + }) if err != nil { writeError(w, 500, err.Error()) return } + writeJSON(w, v.(*ScopeAuditResponse)) +} + +// scopeAuditTTLFor is how long one window's computed audit stays fresh. +// +// 7d is not 30s because it does not cost what the others cost. Measured on a +// live-shaped database with 206 declared repeaters and 965k transmissions: +// 16.7s cold for 7d against 4.0s for 24h and 0.15s for 1h, with the 7d scan +// reading 3,470,188 hop rows. At a 30s TTL a single reader with that window +// open keeps the instance recomputing more than half the time, for an +// aggregate that moves at the pace of a week of traffic. Five minutes of +// staleness on a seven-day window is not a fact the reader can act on +// differently. +func scopeAuditTTLFor(window string) time.Duration { + if window == "7d" { + return 5 * time.Minute + } + return 30 * time.Second +} + +// scopeAuditCached returns the cached response for a window while it is within +// that window's TTL. +func (s *Server) scopeAuditCached(window string) (*ScopeAuditResponse, bool) { + s.scopeAuditMu.Lock() + defer s.scopeAuditMu.Unlock() + if s.scopeAuditCache == nil { + return nil, false + } + cached, ok := s.scopeAuditCache[window] + if !ok || time.Since(s.scopeAuditCachedAt[window]) >= scopeAuditTTLFor(window) { + return nil, false + } + return cached, true +} + +// scopeAuditStore publishes a freshly computed response for a window. +func (s *Server) scopeAuditStore(window string, resp *ScopeAuditResponse) { + s.scopeAuditMu.Lock() + defer s.scopeAuditMu.Unlock() + if s.scopeAuditCache == nil { + s.scopeAuditCache = make(map[string]*ScopeAuditResponse) + s.scopeAuditCachedAt = make(map[string]time.Time) + } + s.scopeAuditCache[window] = resp + s.scopeAuditCachedAt[window] = time.Now() +} + +// computeScopeAudit builds one window's audit response: the declared lists and +// the forwarding evidence attributed to them. Split out of the handler so the +// cache and its singleflight wrap a plain function instead of a request. +func (s *Server) computeScopeAudit(window, sinceISO string) (*ScopeAuditResponse, error) { + declared, err := s.db.AllCurrentDeclaredRegions() + if err != nil { + return nil, err + } targets := make([]string, 0, len(declared)) for _, d := range declared { targets = append(targets, strings.ToLower(d.Target)) } - sinceISO := time.Now().Add(-lookback).UTC().Format(time.RFC3339) forwarding := map[string]*scopeAuditTargetAgg{} if s.store != nil { forwarding, err = s.store.ScopeAuditForwarding(sinceISO, targets) if err != nil { - writeError(w, 500, err.Error()) - return + return nil, err } } @@ -838,14 +1110,5 @@ func (s *Server) handleScopeAudit(w http.ResponseWriter, r *http.Request) { return an < bn }) - s.scopeAuditMu.Lock() - if s.scopeAuditCache == nil { - s.scopeAuditCache = make(map[string]*ScopeAuditResponse) - s.scopeAuditCachedAt = make(map[string]time.Time) - } - s.scopeAuditCache[window] = resp - s.scopeAuditCachedAt[window] = time.Now() - s.scopeAuditMu.Unlock() - - writeJSON(w, resp) + return resp, nil } diff --git a/cmd/server/scope_audit_test.go b/cmd/server/scope_audit_test.go index 00ecb5e5..eaf283cb 100644 --- a/cmd/server/scope_audit_test.go +++ b/cmd/server/scope_audit_test.go @@ -853,3 +853,234 @@ func TestDeclaredRegionsMergeWithNoSourcesIsEmptyNotAnError(t *testing.T) { t.Errorf("rows = %+v, want empty", rows) } } + +// --- Every-hop forwarder attribution and the scan that pays for it --- +// +// These cover the change from crediting path[last] to crediting every hop of a +// flood-family route, and the two-column scan plus per-window TTL that keeps +// the wider scan affordable. + +// seedTransmissionPathAt seeds one transmission whose single observation +// carries a MULTI-hop path. A one-hop seed cannot tell the two reasons a node +// gets attributed apart — it is simultaneously path[0] and path[last] — so the +// mid-path cases below need a path with something after the target on it. +// +// Hops are upper-cased for the same reason seedTransmissionRoute does it: the +// decoder writes them that way (packetpath.DecodePathFromRawHex), and the join +// has to cope with that rather than with a lowercase convenience fiction. +func seedTransmissionPathAt(t *testing.T, s *PacketStore, hops []string, seed scopeSeed, routeType int, firstSeen string) { + t.Helper() + scopeSeedCounter++ + hash := fmt.Sprintf("scopehash%d", scopeSeedCounter) + + res, err := s.db.conn.Exec( + `INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, code1, code2, scope_name) + VALUES ('AA', ?, ?, ?, 1, ?, '00', ?)`, + hash, firstSeen, routeType, seed.code1, seed.scopeName, + ) + if err != nil { + t.Fatalf("seed transmission: %v", err) + } + txID, err := res.LastInsertId() + if err != nil { + t.Fatalf("seed transmission id: %v", err) + } + + quoted := make([]string, len(hops)) + for i, h := range hops { + quoted[i] = `"` + strings.ToUpper(h) + `"` + } + pathJSON := "[" + strings.Join(quoted, ",") + "]" + if _, err := s.db.conn.Exec( + `INSERT INTO observations (transmission_id, path_json, timestamp) VALUES (?, ?, ?)`, + txID, pathJSON, time.Now().Unix(), + ); err != nil { + t.Fatalf("seed observation: %v", err) + } +} + +// seedTransmission seeds a FLOOD packet (route_type=1) — path[last] is the +// actual transmitter, so forwarder is attributable. + +// TestScopeAuditForwardingAttributesMidPathHop is the fleet-wide half of the +// mid-path attribution fix. The audit runs a different query from +// ScopeConformance — one full-window scan instead of one EXISTS per pubkey — so +// the two share the rule but not the code, and both need pinning. +// +// This is the case behind the audit's 65% blind spot: a declared target that +// forwards steadily but is never the hop an observer hears directly had every +// region it declares reported as notObserved. +func TestScopeAuditForwardingAttributesMidPathHop(t *testing.T) { + s := newScopeTestStore(t) + recent := time.Now().UTC().Add(-time.Minute).Format(time.RFC3339) + seedTransmissionPathAt(t, s, []string{testFullPubkeyA[:4], "AAAA", "BBBB"}, scopeMatched("#be"), RouteFlood, recent) + + got, err := s.ScopeAuditForwarding("2026-01-01T00:00:00Z", []string{testFullPubkeyA}) + if err != nil { + t.Fatal(err) + } + agg := got[testFullPubkeyA] + if agg == nil || agg.scopes["be"] == nil || agg.scopes["be"].Packets != 1 { + t.Fatalf("want the mid-path hop attributed to its sole matching target, got %+v", got) + } + if agg.ambiguousHops != 0 { + t.Errorf("ambiguousHops = %d, want 0 — one target matches this hop", agg.ambiguousHops) + } +} + +// TestScopeAuditForwardingIgnoresDirectRoutes pins the route-type filter on the +// audit's own query. With the last-hop restriction gone it is the only guard +// against crediting a DIRECT route's remaining path plan as forwarding — and a +// DIRECT packet's hops are frequently the declared targets this audit judges. + +// TestScopeAuditForwardingIgnoresDirectRoutes pins the route-type filter on the +// audit's own query. With the last-hop restriction gone it is the only guard +// against crediting a DIRECT route's remaining path plan as forwarding — and a +// DIRECT packet's hops are frequently the declared targets this audit judges. +func TestScopeAuditForwardingIgnoresDirectRoutes(t *testing.T) { + s := newScopeTestStore(t) + recent := time.Now().UTC().Add(-time.Minute).Format(time.RFC3339) + seedTransmissionPathAt(t, s, []string{"AAAA", testFullPubkeyA[:4], "BBBB"}, scopeMatched("#be"), RouteDirect, recent) + seedTransmissionPathAt(t, s, []string{"AAAA", "BBBB", testFullPubkeyA[:4]}, scopeMatched("#be"), RouteTransportDirect, recent) + + got, err := s.ScopeAuditForwarding("2026-01-01T00:00:00Z", []string{testFullPubkeyA}) + if err != nil { + t.Fatal(err) + } + if agg := got[testFullPubkeyA]; agg != nil && (len(agg.scopes) != 0 || agg.unscopedPackets != 0 || agg.ambiguousHops != 0) { + t.Errorf("agg = %+v, want no attribution from DIRECT routes", agg) + } +} + +// TestScopeAuditForwardingCountsOneTransmissionOncePerTarget pins that the +// existing "|" de-duplication also absorbs the same target +// matching several hops of ONE path — which could not happen while only +// path[last] was read, and now can (a routing loop, or two hops colliding on the +// same truncated prefix). Without it a looping packet would inflate a target's +// packet count and quietly make a quiet region look busy. + +// TestScopeAuditForwardingCountsOneTransmissionOncePerTarget pins that the +// existing "|" de-duplication also absorbs the same target +// matching several hops of ONE path — which could not happen while only +// path[last] was read, and now can (a routing loop, or two hops colliding on the +// same truncated prefix). Without it a looping packet would inflate a target's +// packet count and quietly make a quiet region look busy. +func TestScopeAuditForwardingCountsOneTransmissionOncePerTarget(t *testing.T) { + s := newScopeTestStore(t) + recent := time.Now().UTC().Add(-time.Minute).Format(time.RFC3339) + seedTransmissionPathAt(t, s, []string{testFullPubkeyA[:4], "AAAA", testFullPubkeyA[:4]}, scopeMatched("#be"), RouteFlood, recent) + + got, err := s.ScopeAuditForwarding("2026-01-01T00:00:00Z", []string{testFullPubkeyA}) + if err != nil { + t.Fatal(err) + } + agg := got[testFullPubkeyA] + if agg == nil || agg.scopes["be"] == nil { + t.Fatalf("want #be attributed, got %+v", got) + } + if agg.scopes["be"].Packets != 1 { + t.Errorf("Packets = %d, want 1 — one transmission, matched on two of its hops", agg.scopes["be"].Packets) + } +} + +// TestScopeAuditForwardingAttributesLongerHopByItsOwnLength pins the +// length-indexed half of scopeAuditPrefixIndex, which every other test in this +// file leaves untested: they all seed 4-char hops, so a lookup that ignored hop +// length entirely would still pass them. +// +// pkOther shares the first 4 hex chars with testFullPubkeyA and diverges after +// that, so an 8-char hop has exactly one candidate while a 4-char hop would +// have two. Attribution must therefore key on the hop's OWN length: at 8 chars +// this is an unambiguous attribution, not an ambiguousHops row. + +// TestScopeAuditForwardingAttributesLongerHopByItsOwnLength pins the +// length-indexed half of scopeAuditPrefixIndex, which every other test in this +// file leaves untested: they all seed 4-char hops, so a lookup that ignored hop +// length entirely would still pass them. +// +// pkOther shares the first 4 hex chars with testFullPubkeyA and diverges after +// that, so an 8-char hop has exactly one candidate while a 4-char hop would +// have two. Attribution must therefore key on the hop's OWN length: at 8 chars +// this is an unambiguous attribution, not an ambiguousHops row. +func TestScopeAuditForwardingAttributesLongerHopByItsOwnLength(t *testing.T) { + s := newScopeTestStore(t) + pkOther := testFullPubkeyA[:4] + strings.Repeat("33", 30) + hop := testFullPubkeyA[:8] + recent := time.Now().UTC().Add(-time.Minute).Format(time.RFC3339) + seedTransmissionPathAt(t, s, []string{hop, "AAAA"}, scopeMatched("#be"), RouteFlood, recent) + + got, err := s.ScopeAuditForwarding("2026-01-01T00:00:00Z", []string{testFullPubkeyA, pkOther}) + if err != nil { + t.Fatal(err) + } + agg := got[testFullPubkeyA] + if agg == nil || agg.scopes["be"] == nil || agg.scopes["be"].Packets != 1 { + t.Fatalf("want the 8-char hop attributed to its sole matching target, got %+v", got) + } + if agg.ambiguousHops != 0 { + t.Errorf("ambiguousHops = %d, want 0 — the two targets diverge before hop length 8", agg.ambiguousHops) + } + if other := got[pkOther]; other != nil && (len(other.scopes) != 0 || other.ambiguousHops != 0) { + t.Errorf("pkOther = %+v, want no attribution and no ambiguity — the hop is not its prefix", other) + } +} + +// TestScopeAuditForwardingCountsUnmatchedPackets: a transport-scoped packet +// whose code1 matched no configured region key is stored with scope_name = "" +// (scopeNameForDB's "transport-scoped but unnameable" state). It is not a +// named scope, so it must not enter agg.scopes, and it is not an unscoped +// plain flood either, so it must not enter unscopedPackets. It is its own +// fact: this instance saw the target forward traffic it holds no key for. +// +// Without this counter the audit reports the declared region as "not +// observed", which reads as a finding about the repeater when it is really a +// gap in this instance's own hashRegions. + +// TestScopeAuditTTLForSevenDayWindow pins the per-window TTL. The 7d window +// costs a different order of magnitude than the others (16.7s against 4.0s and +// 0.15s, measured on the live-shaped staging database on 2026-09-07), so it is +// deliberately not on the 30s the other two share. A future edit that collapses +// this back to one constant should have to delete a test that says why. +func TestScopeAuditTTLForSevenDayWindow(t *testing.T) { + if got := scopeAuditTTLFor("7d"); got != 5*time.Minute { + t.Errorf("scopeAuditTTLFor(7d) = %s, want 5m", got) + } + for _, w := range []string{"1h", "24h", ""} { + if got := scopeAuditTTLFor(w); got != 30*time.Second { + t.Errorf("scopeAuditTTLFor(%q) = %s, want 30s", w, got) + } + } +} + +// TestHandleScopeAuditServesSecondRequestFromCache pins the cache path itself, +// which the singleflight rewrite moved out of the handler and into +// scopeAuditCached/scopeAuditStore. A declared row inserted between two +// requests inside the TTL must NOT appear in the second response: if it does, +// the response was recomputed and the cache is not being consulted. + +// TestHandleScopeAuditServesSecondRequestFromCache pins the cache path itself, +// which the singleflight rewrite moved out of the handler and into +// scopeAuditCached/scopeAuditStore. A declared row inserted between two +// requests inside the TTL must NOT appear in the second response: if it does, +// the response was recomputed and the cache is not being consulted. +func TestHandleScopeAuditServesSecondRequestFromCache(t *testing.T) { + srv, router := setupScopeAuditServer(t) + now := time.Now().UTC().Format(time.RFC3339) + insertDeclared(t, srv, testFullPubkeyA, now, "be", 0) + + first := getScopeAudit(t, router, "") + if len(first.Repeaters) != 1 { + t.Fatalf("first call repeaters = %d, want 1", len(first.Repeaters)) + } + + insertDeclared(t, srv, testFullPubkeyB, now, "be", 0) + second := getScopeAudit(t, router, "") + if len(second.Repeaters) != 1 { + t.Errorf("second call repeaters = %d, want 1 — the row added after the first call proves the cache was bypassed", len(second.Repeaters)) + } +} + +// TestHandleScopeAuditNormalisesHashPrefix pins trap 1: transmissions.scope_name +// keeps the '#' (hashRegions config), regions_csv arrives from the firmware +// with it already stripped. Declared "be-van" and observed "#be-van" must be +// recognised as the same scope, not reported as both missing and undeclared.