diff --git a/cmd/server/db.go b/cmd/server/db.go index 6aa14293..64733dc7 100644 --- a/cmd/server/db.go +++ b/cmd/server/db.go @@ -2155,13 +2155,15 @@ func (db *DB) GetNodeLocationsByKeys(keys []string) map[string]map[string]interf return result } -// GetNodeNamesByKeys batch-resolves pubkey -> display name for the given -// keys. Missing/unnamed nodes are simply absent from the result map — the -// caller falls back to a truncated pubkey. Used to label repeaters in the -// scope-stats "repeaters by region" breakdown without pulling full node -// rows for a set that's typically small (repeaters that have transported -// at least one scoped packet). -func (db *DB) GetNodeNamesByKeys(keys []string) map[string]string { +// GetRepeaterNamesByKeys batch-resolves pubkey -> display name, restricted +// to role IN ('repeater','room'). The candidate key set (e.g. from +// PacketStore.byPathHop) mixes full pubkeys with short hex-prefix bucket +// keys used internally for ambiguous-hop resolution (#1751 follow-up) — +// those never match a real nodes.public_key row, so the role-filtered IN +// query doubles as the "is this actually a distinct node" existence check. +// A matched pubkey with an empty/unset name falls back to itself so a +// real repeater is never silently dropped just because it has no name yet. +func (db *DB) GetRepeaterNamesByKeys(keys []string) map[string]string { result := make(map[string]string) if len(keys) == 0 { return result @@ -2172,7 +2174,7 @@ func (db *DB) GetNodeNamesByKeys(keys []string) map[string]string { placeholders[i] = "?" args[i] = strings.ToLower(k) } - query := "SELECT public_key, name FROM nodes WHERE public_key IN (" + strings.Join(placeholders, ",") + ")" + query := "SELECT public_key, name FROM nodes WHERE role IN ('repeater','room') AND public_key IN (" + strings.Join(placeholders, ",") + ")" rows, err := db.conn.Query(query, args...) if err != nil { return result @@ -2181,8 +2183,14 @@ func (db *DB) GetNodeNamesByKeys(keys []string) map[string]string { for rows.Next() { var pk string var name sql.NullString - if rows.Scan(&pk, &name) == nil && name.Valid && name.String != "" { - result[strings.ToLower(pk)] = name.String + if rows.Scan(&pk, &name) != nil { + continue + } + pk = strings.ToLower(pk) + if name.Valid && name.String != "" { + result[pk] = name.String + } else { + result[pk] = pk } } return result diff --git a/cmd/server/routes.go b/cmd/server/routes.go index 10baf3dc..86ebe014 100644 --- a/cmd/server/routes.go +++ b/cmd/server/routes.go @@ -3541,18 +3541,28 @@ func (s *Server) handleScopeStats(w http.ResponseWriter, r *http.Request) { for pk := range pubkeySet { pubkeys = append(pubkeys, pk) } - names := s.db.GetNodeNamesByKeys(pubkeys) + // byPathHop mixes full pubkeys with short hex-prefix bucket + // keys (ambiguous-hop resolution fallback) — the role-filtered + // lookup only returns entries for keys that are actually a + // repeater/room node, so it doubles as the existence filter. + // Any key NOT in `names` is a bucket key, not a real repeater, + // and must be excluded below rather than falling back to + // showing the raw key as a fake "repeater". + names := s.db.GetRepeaterNamesByKeys(pubkeys) repeaters := make([]ScopeRegionRepeaters, 0, len(byRegion)) for region, pks := range byRegion { refs := make([]RepeaterRef, 0, len(pks)) for _, pk := range pks { - name := names[pk] - if name == "" { - name = pk + name, ok := names[pk] + if !ok { + continue } refs = append(refs, RepeaterRef{Name: name, PublicKey: pk}) } + if len(refs) == 0 { + continue + } sort.Slice(refs, func(i, j int) bool { return refs[i].Name < refs[j].Name }) repeaters = append(repeaters, ScopeRegionRepeaters{Region: region, Count: len(refs), Repeaters: refs}) } diff --git a/cmd/server/routes_test.go b/cmd/server/routes_test.go index 265f0130..d5d24ad3 100644 --- a/cmd/server/routes_test.go +++ b/cmd/server/routes_test.go @@ -4309,7 +4309,7 @@ func TestHandleScopeStats_RepeatersByRegion(t *testing.T) { srv.db.hasScopeName = true if _, err := srv.db.conn.Exec( - `INSERT INTO nodes (public_key, name) VALUES ('aabbccdd0011', 'TestRepeater1')`, + `INSERT INTO nodes (public_key, name, role) VALUES ('aabbccdd0011', 'TestRepeater1', 'repeater')`, ); err != nil { t.Fatalf("seed node: %v", err) } @@ -4322,8 +4322,22 @@ func TestHandleScopeStats_RepeatersByRegion(t *testing.T) { PayloadType: &pt5, ScopeName: "#belgium", } + // #1751 follow-up regression: byPathHop also indexes short hex-prefix + // "bucket" keys (ambiguous-hop resolution fallback) alongside full + // pubkeys — "aabb" here mimics that. It must NOT be surfaced as a + // distinct "repeater" since it never matches a real nodes.public_key. + bucketTx := &StoreTx{ + ID: 2, + Hash: "txhash2", + FirstSeen: time.Now().UTC().Add(-5 * time.Minute).Format(time.RFC3339Nano), + PayloadType: &pt5, + ScopeName: "#belgium", + } srv.store = &PacketStore{ - byPathHop: map[string][]*StoreTx{"aabbccdd0011": {tx}}, + byPathHop: map[string][]*StoreTx{ + "aabbccdd0011": {tx}, + "aabb": {bucketTx}, + }, } req := httptest.NewRequest("GET", "/api/scope-stats?window=24h", nil)