fix: exclude byPathHop bucket keys from repeaters-by-region

byPathHop indexes both full pubkeys and short hex-prefix "bucket" keys
used internally for ambiguous-hop resolution — GetRepeaterRelayInfoMap
returns TransportedScopes for every one of those keys indiscriminately.
The first deploy of repeatersByRegion iterated the raw map and fell
back to showing the bucket key itself when no name matched, so a
handful of short internal keys were counted as "repeaters" (stg showed
594 "repeaters" for #dk — every active repeater plus every 2-6 char
bucket key that ever touched a #dk packet).

GetNodeNamesByKeys is now GetRepeaterNamesByKeys and filters
`role IN ('repeater','room')` in the SQL itself, so a key only survives
if it's a real node — bucket keys never match a nodes.public_key row
and are dropped rather than falling back to the raw key.
This commit is contained in:
dborup
2026-07-17 17:00:40 +02:00
parent 87d2f479a7
commit 93144ee654
3 changed files with 48 additions and 16 deletions
+18 -10
View File
@@ -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
+14 -4
View File
@@ -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})
}
+16 -2
View File
@@ -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)