diff --git a/cmd/ingestor/neighbor_builder.go b/cmd/ingestor/neighbor_builder.go index 7010c2fe..63cda056 100644 --- a/cmd/ingestor/neighbor_builder.go +++ b/cmd/ingestor/neighbor_builder.go @@ -35,6 +35,12 @@ const neighborBuilderSlowTickThreshold = 5 * time.Second // independent of the server package. const payloadADVERT = 0x04 +// minInteriorEdgeHashBytes is the minimum hop hash length (in bytes, +// hex-string length is double this) required before buildAndPersistNeighborEdges +// will emit an interior hop-to-hop edge. See the doc comment at its use +// site for why 1-byte prefixes are excluded (PR #1852 review). +const minInteriorEdgeHashBytes = 2 + // edgeRow is one row to upsert into neighbor_edges. (a, b) is already // canonical-ordered (a <= b). type edgeRow struct { @@ -237,7 +243,24 @@ func (s *Store) buildAndPersistNeighborEdges() (int, error) { // previous-hop lookup (path_resolver.go) has no adjacency data // for any interior hop, so a multi-hop path only ever resolves // its first hop in practice (#1547 follow-up). + // + // minInteriorEdgeHashBytes guards against a large-mesh false-edge + // risk flagged on PR #1852: resolvePrefix only requires a hop to + // be unique among nodes known TODAY. At 1 byte (2 hex chars) a + // prefix that's currently unique on a large mesh (SaarMesh + // reported ~1.2% of 1071 nodes) can silently become wrong once + // an unknown/new node sharing that byte appears -- and unlike + // the endpoint edges, an interior edge has no ADVERT/ANON_REQ to + // double-check it against. 2+ byte prefixes don't have this + // problem (SaarMesh: 99.4% of nodes uniquely resolvable there). + // Matches the conservative default chosen for the upstream + // #1824 pathTrust.minHashBytesForMapping config; inlined here + // rather than depending on that config landing first, since it + // is not yet wired into any call site. for i := 0; i+1 < len(path); i++ { + if len(path[i]) < minInteriorEdgeHashBytes*2 || len(path[i+1]) < minInteriorEdgeHashBytes*2 { + continue + } resolvedA, okA := resolvePrefix(prefixIdx, path[i]) if !okA { continue diff --git a/cmd/ingestor/neighbor_builder_test.go b/cmd/ingestor/neighbor_builder_test.go index 6cfefe7e..7c2d1450 100644 --- a/cmd/ingestor/neighbor_builder_test.go +++ b/cmd/ingestor/neighbor_builder_test.go @@ -133,10 +133,12 @@ func TestNeighborEdgesBuilderInteriorHopEdges(t *testing.T) { } txID, _ := res.LastInsertId() - // path = b -> c -> d (three hops). Expect interior edges b<->c and c<->d. + // path = b -> c -> d (three hops, 2-byte/4-hex-char prefixes -- the + // minimum length minInteriorEdgeHashBytes allows). Expect interior + // edges b<->c and c<->d. if _, err := store.db.Exec( `INSERT INTO observations (transmission_id, observer_idx, path_json, timestamp) VALUES (?, ?, ?, ?)`, - txID, obsRowid, `["bb","cc","dd"]`, int64(1735689600), + txID, obsRowid, `["bbbb","cccc","dddd"]`, int64(1735689600), ); err != nil { t.Fatal(err) } @@ -160,5 +162,79 @@ func TestNeighborEdgesBuilderInteriorHopEdges(t *testing.T) { } } +// TestNeighborEdgesBuilderInteriorHopEdges_ExcludesOneByte covers the +// PR #1852 review finding: a 1-byte (2-hex-char) prefix that happens to +// be unique among nodes known TODAY can silently become wrong once an +// unknown/new node sharing that byte appears later -- and unlike the +// endpoint edges, an interior edge has no ADVERT/ANON_REQ to +// double-check it against. minInteriorEdgeHashBytes=2 excludes 1-byte +// hops from interior-edge creation entirely, even when they'd otherwise +// resolve unambiguously against the current nodes table. +func TestNeighborEdgesBuilderInteriorHopEdges_ExcludesOneByte(t *testing.T) { + dir := t.TempDir() + dbPath := filepath.Join(dir, "build.db") + + store, err := OpenStore(dbPath) + if err != nil { + t.Fatalf("OpenStore: %v", err) + } + defer store.Close() + + // Same three repeaters as the sibling test, but referenced by their + // 1-byte (2-hex-char) prefixes below instead of 2-byte. + if _, err := store.db.Exec( + `INSERT INTO nodes (public_key, name) VALUES (?, ?), (?, ?), (?, ?)`, + "bbbbbbbbbb", "hop-b", + "cccccccccc", "hop-c", + "dddddddddd", "hop-d", + ); err != nil { + t.Fatal(err) + } + if _, err := store.db.Exec( + `INSERT INTO observers (id, name) VALUES (?, ?)`, + "obs-1", "observer-1", + ); err != nil { + t.Fatal(err) + } + var obsRowid int64 + if err := store.db.QueryRow(`SELECT rowid FROM observers WHERE id = ?`, "obs-1").Scan(&obsRowid); err != nil { + t.Fatal(err) + } + + res, err := store.db.Exec( + `INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, payload_version, decoded_json) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + "", "h3", "2026-01-01T00:00:00Z", 0, 5, 0, "{}", + ) + if err != nil { + t.Fatal(err) + } + txID, _ := res.LastInsertId() + + // path = b -> c -> d, but as 1-byte prefixes -- must NOT produce + // interior edges, even though each individually resolves unambiguously + // against the current (small, test-only) nodes table. + if _, err := store.db.Exec( + `INSERT INTO observations (transmission_id, observer_idx, path_json, timestamp) VALUES (?, ?, ?, ?)`, + txID, obsRowid, `["bb","cc","dd"]`, int64(1735689600), + ); err != nil { + t.Fatal(err) + } + + if _, err := store.buildAndPersistNeighborEdges(); err != nil { + t.Fatalf("buildAndPersistNeighborEdges: %v", err) + } + + for _, pair := range [][2]string{{"bbbbbbbbbb", "cccccccccc"}, {"cccccccccc", "dddddddddd"}} { + var got int + if err := store.db.QueryRow(`SELECT COUNT(*) FROM neighbor_edges WHERE node_a = ? AND node_b = ?`, pair[0], pair[1]).Scan(&got); err != nil { + t.Fatal(err) + } + if got != 0 { + t.Errorf("expected NO interior edge %s<->%s from 1-byte hops, got %d rows", pair[0], pair[1], got) + } + } +} + // (test ends here) diff --git a/cmd/server/db.go b/cmd/server/db.go index 4d075b52..7f03a77f 100644 --- a/cmd/server/db.go +++ b/cmd/server/db.go @@ -1556,9 +1556,15 @@ type PacketPathBranch struct { // one per distinct observer, kept at that observer's own deepest // observation -- used to draw the full flood spread on a map (the // ping-bot reply's "View path" link), not just the single farthest route. +// First is the single EARLIEST-arriving observation across every station +// (regardless of observer or hop depth) -- the same "first observation +// wins" pick already used for a ping message's own meta line -- included +// separately since it approximates where the message entered the visible +// mesh, a landmark the deepest-first Branches ordering doesn't surface. type PacketPathResponse struct { Hash string `json:"hash"` Branches []PacketPathBranch `json:"branches"` + First *PacketPathBranch `json:"first,omitempty"` } // GetPacketPath resolves every distinct station that observed a packet to @@ -1568,20 +1574,22 @@ type PacketPathResponse struct { // its deepest observation (by raw hop count, same "farthest leg" // reasoning as the ping-bot reply -- see pingBotReply's doc comment) is // kept, so each station contributes exactly one branch. Branches are -// returned deepest-first. +// returned deepest-first. The single earliest-arriving observation +// (usually 0 hops, close to the sender) is additionally surfaced via +// First, independent of which station it came from or how deep it was. func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { if !db.hasResolvedPath { return nil, fmt.Errorf("resolved_path not available on this server") } var querySQL string if db.isV3 { - querySQL = `SELECT obs.rowid, obs.id, obs.name, obs.iata, o.path_json, o.resolved_path, o.snr + querySQL = `SELECT obs.rowid, obs.id, obs.name, obs.iata, o.path_json, o.resolved_path, o.snr, o.timestamp FROM observations o JOIN transmissions t ON t.id = o.transmission_id LEFT JOIN observers obs ON obs.rowid = o.observer_idx WHERE t.hash = ?` } else { - querySQL = `SELECT o.observer_id, o.observer_id, o.observer_name, NULL, o.path_json, o.resolved_path, o.snr + querySQL = `SELECT o.observer_id, o.observer_id, o.observer_name, NULL, o.path_json, o.resolved_path, o.snr, o.timestamp FROM observations o JOIN transmissions t ON t.id = o.transmission_id WHERE t.hash = ?` @@ -1601,11 +1609,14 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { snr sql.NullFloat64 } best := make(map[string]*obsBranch) + var first *obsBranch + var firstTS int64 for rows.Next() { var obsKey, obsPubkey, obsName, obsIATA, pathJSON, resolvedPathJSON sql.NullString var snr sql.NullFloat64 - if err := rows.Scan(&obsKey, &obsPubkey, &obsName, &obsIATA, &pathJSON, &resolvedPathJSON, &snr); err != nil { + var ts sql.NullInt64 + if err := rows.Scan(&obsKey, &obsPubkey, &obsName, &obsIATA, &pathJSON, &resolvedPathJSON, &snr, &ts); err != nil { continue } if !pathJSON.Valid { @@ -1627,12 +1638,16 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { if key == "" { continue // no way to attribute this observation to a station } + branch := &obsBranch{ + hops: hops, resolvedPath: resolvedPath, + observerName: obsName.String, observerPubkey: strings.ToLower(strings.TrimSpace(obsPubkey.String)), + observerIATA: obsIATA, snr: snr, + } if existing, ok := best[key]; !ok || hops > existing.hops { - best[key] = &obsBranch{ - hops: hops, resolvedPath: resolvedPath, - observerName: obsName.String, observerPubkey: strings.ToLower(strings.TrimSpace(obsPubkey.String)), - observerIATA: obsIATA, snr: snr, - } + best[key] = branch + } + if ts.Valid && (first == nil || ts.Int64 < firstTS) { + first, firstTS = branch, ts.Int64 } } if err := rows.Err(); err != nil { @@ -1645,6 +1660,16 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { } pubkeySet := map[string]bool{} + if first != nil { + for _, pk := range first.resolvedPath { + if pk != nil && *pk != "" { + pubkeySet[*pk] = true + } + } + if first.observerPubkey != "" { + pubkeySet[first.observerPubkey] = true + } + } for _, b := range best { for _, pk := range b.resolvedPath { if pk != nil && *pk != "" { @@ -1713,15 +1738,19 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { // queried for observers the pubkey lookup left unpositioned, and only // applied if the pubkey lookup didn't already find something. nameFallbackNeeded := map[string]bool{} - for _, b := range best { - if b.observerName == "" { - continue + needsNameFallback := func(b *obsBranch) { + if b == nil || b.observerName == "" { + return } if ni, ok := nodeByPK[b.observerPubkey]; ok && ni.lat != nil && ni.lon != nil { - continue + return } nameFallbackNeeded[b.observerName] = true } + for _, b := range best { + needsNameFallback(b) + } + needsNameFallback(first) nodeByName := make(map[string]nodeInfo, len(nameFallbackNeeded)) if len(nameFallbackNeeded) > 0 { names := make([]string, 0, len(nameFallbackNeeded)) @@ -1760,7 +1789,7 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { } } - for _, b := range best { + buildBranch := func(b *obsBranch) PacketPathBranch { branch := PacketPathBranch{Hops: b.hops, Points: []PacketPathPoint{}} for _, pk := range b.resolvedPath { if pk == nil || *pk == "" { @@ -1807,10 +1836,19 @@ func (db *DB) GetPacketPath(hash string) (*PacketPathResponse, error) { v := b.snr.Float64 branch.SNR = &v } - resp.Branches = append(resp.Branches, branch) + return branch + } + + for _, b := range best { + resp.Branches = append(resp.Branches, buildBranch(b)) } sort.Slice(resp.Branches, func(i, j int) bool { return resp.Branches[i].Hops > resp.Branches[j].Hops }) + if first != nil { + fb := buildBranch(first) + resp.First = &fb + } + return resp, nil } diff --git a/cmd/server/db_test.go b/cmd/server/db_test.go index 6f9285b1..849fd861 100644 --- a/cmd/server/db_test.go +++ b/cmd/server/db_test.go @@ -677,6 +677,50 @@ func TestGetPacketPath(t *testing.T) { } } +// TestGetPacketPath_First covers the First field: the single +// earliest-arriving observation across every station, independent of +// which observer it came from or how many hops it took -- an approximate +// "where the message entered the mesh" landmark, distinct from Branches[0] +// (the deepest, i.e. farthest-traveled, branch). +func TestGetPacketPath_First(t *testing.T) { + db := setupTestDB(t) + defer db.Close() + + db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('obsEarly', 'Observer Early', 'SJC')`) + db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('obsDeep', 'Observer Deep', 'SFO')`) + db.conn.Exec(`INSERT INTO observers (id, name, iata) VALUES ('obsMid', 'Observer Mid', 'OAK')`) + + db.conn.Exec(`INSERT INTO transmissions (raw_hex, hash, first_seen, route_type, payload_type, decoded_json, channel_hash) + VALUES ('AA', 'pathtest00000007', '2026-01-15T10:00:00Z', 1, 5, + '{"type":"CHAN","channel":"#ping","text":"ping","sender":"Eve"}', '#ping')`) + // Earliest in time (timestamp=100), but shallow (0 hops, direct). + db.conn.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) + VALUES (1, 1, 9.0, -88, '[]', 100)`) + // Arrives later, but travels deepest (5 hops) -- this is Branches[0]. + db.conn.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) + VALUES (1, 2, 4.0, -95, '["aa","bb","cc","dd","ee"]', 200)`) + // Arrives last, middling depth. + db.conn.Exec(`INSERT INTO observations (transmission_id, observer_idx, snr, rssi, path_json, timestamp) + VALUES (1, 3, 6.0, -90, '["aa","bb"]', 300)`) + + resp, err := db.GetPacketPath("pathtest00000007") + if err != nil { + t.Fatal(err) + } + if resp.First == nil { + t.Fatalf("First = nil, want the earliest observation") + } + if resp.First.Observer == nil || resp.First.Observer.Name != "Observer Early" { + t.Errorf("First.Observer = %+v, want Observer Early (timestamp=100, the earliest)", resp.First.Observer) + } + if resp.First.Hops != 0 { + t.Errorf("First.Hops = %d, want 0 (Observer Early's own observation was direct)", resp.First.Hops) + } + if len(resp.Branches) == 0 || resp.Branches[0].Observer == nil || resp.Branches[0].Observer.Name != "Observer Deep" { + t.Fatalf("Branches[0] = %+v, want Observer Deep still first (deepest-first ordering unaffected by First)", resp.Branches) + } +} + // TestGetPacketPath_ObserverPositionPrefersOwnGPS covers an observer whose // configured IATA code isn't a real airport (a custom/regional code an // operator typed in, or a typo) and so isn't in the hardcoded iataCoords diff --git a/cmd/server/openapi.go b/cmd/server/openapi.go index e87096ff..41d6bfa2 100644 --- a/cmd/server/openapi.go +++ b/cmd/server/openapi.go @@ -141,7 +141,7 @@ func routeDescriptions() map[string]routeMeta { // Misc "GET /api/resolve-hops": {Summary: "Resolve hop path", Description: "Resolves hash prefixes in a hop path to node names. Returns affinity scores and best candidates.", Tag: "nodes", QueryParams: []paramMeta{{Name: "hops", Description: "Comma-separated hop hash prefixes", Type: "string", Required: true}}}, "GET /api/traces/{hash}": {Summary: "Get packet traces", Description: "Returns all observer sightings for a packet hash.", Tag: "packets"}, - "GET /api/packets/{hash}/path": {Summary: "Get a packet's full geographic flood spread", Description: "Resolves EVERY distinct station that observed a packet to its own branch: hop count (from that station's deepest observation) plus, where resolvable, each relay's name/role/lat/lon in path order and the station's own position (self-advertised GPS when known, same as /api/observers, else its configured IATA code). A station heard more than once (later flood copies via longer routes) contributes only its deepest observation. Lat/lon are null for any hop or observer that has no known position -- callers should draw a gap, not guess. Backs the Channels tab's ping-bot \"View path\" map link.", Tag: "packets", + "GET /api/packets/{hash}/path": {Summary: "Get a packet's full geographic flood spread", Description: "Resolves EVERY distinct station that observed a packet to its own branch: hop count (from that station's deepest observation) plus, where resolvable, each relay's name/role/lat/lon in path order and the station's own position (self-advertised GPS when known, same as /api/observers, else its configured IATA code). A station heard more than once (later flood copies via longer routes) contributes only its deepest observation. Lat/lon are null for any hop or observer that has no known position -- callers should draw a gap, not guess. Also returns `first`: the single earliest-arriving observation across every station (usually 0 hops, close to the sender) -- an approximate origin landmark, distinct from branches[0] which is the deepest/farthest-traveled branch. Backs the Channels tab's ping-bot \"View path\" map link.", Tag: "packets", Response: schemaRef("PacketPathResponse")}, "GET /api/iata-coords": {Summary: "Get IATA airport coordinates", Description: "Returns lat/lon for known airport codes (used for observer positioning).", Tag: "config"}, "GET /api/audio-lab/buckets": {Summary: "Audio lab frequency buckets", Description: "Returns frequency bucket data for audio analysis.", Tag: "analytics"}, @@ -369,6 +369,7 @@ func componentSchemas() map[string]interface{} { "properties": map[string]interface{}{ "hash": str("The packet hash this path was resolved for."), "branches": map[string]interface{}{"type": "array", "items": schemaRef("PacketPathBranch"), "description": "One branch per distinct station that observed the packet, each kept at that station's own deepest observation, sorted deepest-first -- shows the full flood spread, not just the single farthest route."}, + "first": schemaRef("PacketPathBranch"), }, }, } diff --git a/public/packet-path-map.js b/public/packet-path-map.js index ad88e2ef..e54359d3 100644 --- a/public/packet-path-map.js +++ b/public/packet-path-map.js @@ -6,8 +6,13 @@ position) ending at that station. The deepest branch is drawn on top in the accent color; every other branch is drawn muted underneath it, so the map reads as "how far AND how wide did this packet spread" - rather than a single route. Reuses node-reach-map.js's Leaflet setup - conventions (tile helper, circleMarker points, theme-aware colors). + rather than a single route. The response's `first` field (the single + earliest-arriving observation, usually 0 hops) is additionally drawn + as a distinct landmark ring on top of everything else -- an + approximate "where the message entered the mesh" anchor, since a + deepest-first branch list has no natural starting point of its own. + Reuses node-reach-map.js's Leaflet setup conventions (tile helper, + circleMarker points, theme-aware colors). Entry point today: the ping-bot reply's "View path" link (public/channels.js botReplyHtml) -- kept general (keyed by packet @@ -67,7 +72,7 @@ '' + '
How far and how wide this packet spread. The highlighted route is the farthest-traveled branch; every other station that heard it is shown too.
' + + 'How far and how wide this packet spread. The highlighted route is the farthest-traveled branch; every other station that heard it is shown too. The green ring marks whoever heard it first.
' + '' + '