fix(coverage): attribute node-discover replies to the responder (#2111)

## Problem

A CoreDrive RX companion that runs node-discover gets no coverage on an
upstream instance. Reported today by an operator on v3.13.0: the app
logged 32 discover replies from one repeater in 16 minutes (`heard
751a49f0c5dadc70 (8B, discover)`), all published, and
`client_receptions` stayed at 0 rows for that companion while
`client_rx_observations` held 93.

Two gaps, both on the upstream side:

1. **Ingestor.** `deriveHeardKey` attributes a FLOOD `path[last]` and a
0-hop advert, and drops a 0-hop `CONTROL/DISCOVER_RESP` (firmware
`CTL_TYPE_NODE_DISCOVER_RESP`). The reply carries the responder's own
pubkey at offset 6 of the payload, which `decoder.go` already parses
into `CtrlPubKey` (#1802), but the coverage path never used it.
2. **Server.** CoreDrive RX asks for `DISCOVER_PREFIX_ONLY`, so the
firmware answers with an 8-byte prefix
(`simple_repeater/MyMesh.cpp:799-805`). `coverageHeardKeyCandidates`
only built the 64, 6 and 4 hex candidates, so a 16-hex `heard_key` would
be stored and then matched by no per-node coverage query.

## Change

- `cmd/ingestor/client_reception.go`: a third branch in `deriveHeardKey`
for a discover response with no hops. Accepts exactly 8 or 32 bytes,
nothing truncated, stored with `src='discover'`.
- `cmd/server/rx_coverage.go`: adds the 16-hex prefix to
`coverageHeardKeyCandidates`.
- `docs/client-rx-coverage.md`: documents the `discover` source, the
8-byte keylen and the four-candidate lookup.

The leaderboard and `/api/rx-coverage` read `client_receptions` without
a key filter, so they pick the rows up without a change. Name resolution
goes through `batchResolveHeardKeys`, which is a prefix lookup and
handles 16 hex as is.

## Evidence from a deployment that has had this since 2026-08-19

On analyzer.on8ar.eu, `client_receptions` over the last 7 days by `src`:
discover 4232, rxlog 4909, geo 548, advert 34. Discover replies are 44%
of all coverage rows there (4232 of 9723); on an upstream instance those
rows are not written. They cannot be backfilled afterwards either:
`client_rx_observations` keeps no raw bytes, so the responder pubkey is
gone.

## Tests

- `TestDeriveHeardKey` and `TestBuildClientReception` gain discover
cases: 8-byte and 32-byte keys accepted (32-byte uppercase input
lowercased), a 3-byte and an empty key rejected, a non-discover CONTROL
rejected, a discover response with hops not attributed.
- `TestHandleClientPacketDiscoverRespWritesReception`: end to end, a raw
0-hop DISCOVER_RESP on the client topic writes one `client_receptions`
row with `src='discover'`.
- `TestCoverageHeardKeyCandidatesIncludesDiscoverPrefix`: the 16-hex
prefix is among the per-node candidates.
- Ran locally on Windows: `go test ./...` in `cmd/server` passes; in
`cmd/ingestor` everything passes except
`TestWriteStatsAtomic_SymlinkAtDestIsReplaced`, which needs the symlink
privilege on Windows and fails on clean master too.

Not done: no browser validation, as the change is ingestor and server
only and the frontend reads the same endpoints. Not included: geographic
resolution of 1-byte hops (`src='geo'`) and the RF noise layer, which
are separate changes.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
efiten
2026-10-04 17:15:50 +02:00
committed by GitHub
co-authored by Claude Opus 5
parent c2a9500060
commit 00ee4f9e23
5 changed files with 147 additions and 22 deletions
+19 -2
View File
@@ -84,6 +84,7 @@ func handleClientPacket(store *Store, cfg *Config, tag, rxPubkey string, msg map
rxAt := rxTime.Format(time.RFC3339)
ingestedAt := time.Now().UTC().Format(time.RFC3339)
isAdvert := decoded.Header.PayloadTypeName == "ADVERT"
isDiscoverResp := decoded.Header.PayloadTypeName == "CONTROL" && decoded.Payload.CtrlSubtype == "DISCOVER_RESP"
if cfg.ClientRxObservationsEnabled() {
// rxAtMillis: UNIQUE(rx_pubkey, pkt_hash, rx_at) needs sub-second
@@ -102,6 +103,7 @@ func handleClientPacket(store *Store, cfg *Config, tag, rxPubkey string, msg map
rec, ok := buildClientReception(
rxPubkey,
direction, decoded.Header.RouteType, decoded.Header.PayloadType, decoded.Path.Hops, decoded.Payload.PubKey, isAdvert,
isDiscoverResp, decoded.Payload.CtrlPubKey,
snrPtr, rssiPtr, lat, lon, accPtr, rxAt, ingestedAt,
)
if !ok {
@@ -179,10 +181,16 @@ type ClientReception struct {
// consume the next hop from the FRONT (firmware Mesh.cpp removeSelfFromPath),
// so path[len-1] is the route's destination-side end, not who was heard.
// - hops empty + isAdvert → the 0-hop advertiser, by its full pubkey.
// - hops empty + isDiscoverResp → the 0-hop responder, by its pubkey (8 or
// 32 bytes — CTL_TYPE_NODE_DISCOVER_RESP always carries an 8-byte prefix
// or the full 32-byte key; anything else is rejected outright, not
// truncated). A node-discover reply is always a direct, zero-hop RX —
// stronger identification than a path hash, since it carries the
// responder's own identity rather than an inferred forwarder.
// - otherwise → not attributable (ok=false).
//
// Returns (heardKey lowercased, keylenBytes, src, ok).
func deriveHeardKey(direction string, routeType, payloadType int, hops []string, advertPubkey string, isAdvert bool) (string, int, string, bool) {
func deriveHeardKey(direction string, routeType, payloadType int, hops []string, advertPubkey string, isAdvert bool, isDiscoverResp bool, discoverPubkey string) (string, int, string, bool) {
if !strings.EqualFold(direction, "rx") {
return "", 0, "", false
}
@@ -208,6 +216,14 @@ func deriveHeardKey(direction string, routeType, payloadType int, hops []string,
pk := strings.ToLower(strings.TrimSpace(advertPubkey))
return pk, len(pk) / 2, "advert", true
}
if isDiscoverResp && discoverPubkey != "" {
pk := strings.ToLower(strings.TrimSpace(discoverPubkey))
keylen := len(pk) / 2
if keylen != 8 && keylen != 32 { // discover pubkeys are always 8B (prefix) or 32B (full) — no floor, an exact set
return "", 0, "", false
}
return pk, keylen, "discover", true
}
return "", 0, "", false
}
@@ -215,6 +231,7 @@ func deriveHeardKey(direction string, routeType, payloadType int, hops []string,
// returns ok=false when the packet is not attributable / out of range.
func buildClientReception(
rxPubkey, direction string, routeType, payloadType int, hops []string, advertPubkey string, isAdvert bool,
isDiscoverResp bool, discoverPubkey string,
snr *float64, rssi *int, lat, lon float64, posAccM *float64, rxAt, ingestedAt string,
) (*ClientReception, bool) {
if rxPubkey == "" || rxAt == "" {
@@ -223,7 +240,7 @@ func buildClientReception(
if lat < -90 || lat > 90 || lon < -180 || lon > 180 {
return nil, false
}
heardKey, keylen, src, ok := deriveHeardKey(direction, routeType, payloadType, hops, advertPubkey, isAdvert)
heardKey, keylen, src, ok := deriveHeardKey(direction, routeType, payloadType, hops, advertPubkey, isAdvert, isDiscoverResp, discoverPubkey)
if !ok {
return nil, false
}
+77 -12
View File
@@ -261,56 +261,89 @@ func TestRxLeaderboardQueryIsIndexBacked(t *testing.T) {
func TestDeriveHeardKey(t *testing.T) {
full := "abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789"
k, l, src, ok := deriveHeardKey("rx", packetpath.RouteFlood, PayloadADVERT, nil, strings.ToUpper(full), true)
k, l, src, ok := deriveHeardKey("rx", packetpath.RouteFlood, PayloadADVERT, nil, strings.ToUpper(full), true, false, "")
if !ok || l != 32 || src != "advert" || k != full {
t.Fatalf("0-hop advert: got k=%q l=%d src=%q ok=%v", k, l, src, ok)
}
k, l, src, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false)
k, l, src, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false, false, "")
if !ok || k != "bbccdd" || l != 3 || src != "rxlog" {
t.Fatalf("flood path: got k=%q l=%d src=%q ok=%v", k, l, src, ok)
}
// DIRECT route: path[last] is the route's far end, not the transmitter — must be rejected.
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteDirect, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false); ok {
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteDirect, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false, false, ""); ok {
t.Fatalf("direct-route path must be rejected")
}
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteTransportDirect, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false); ok {
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteTransportDirect, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false, false, ""); ok {
t.Fatalf("transport-direct-route path must be rejected")
}
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aa", "bb"}, "", false); ok {
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aa", "bb"}, "", false, false, ""); ok {
t.Fatalf("1-byte last hop should be rejected")
}
if _, _, _, ok = deriveHeardKey("tx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aabbcc"}, "", false); ok {
if _, _, _, ok = deriveHeardKey("tx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aabbcc"}, "", false, false, ""); ok {
t.Fatalf("tx must be rejected")
}
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadGRP_TXT, nil, "", false); ok {
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadGRP_TXT, nil, "", false, false, ""); ok {
t.Fatalf("no hops + non-advert must be rejected")
}
// TRACE repurposes the header path bytes as per-hop SNR values, not node
// hashes — a FLOOD-routed TRACE must never be attributable, even though the
// route type and hop shape are otherwise identical to the accepted case above.
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadTRACE, []string{"aa", "bbccdd"}, "", false); ok {
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadTRACE, []string{"aa", "bbccdd"}, "", false, false, ""); ok {
t.Fatalf("FLOOD-routed TRACE must be rejected (path bytes are SNR values, not node hashes)")
}
// --- discover-response cases (0x90 CTL_TYPE_NODE_DISCOVER_RESP) ---
discover32 := strings.Repeat("ab", 32)
discover8 := "abcdef0123456789"
k, l, src, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadCONTROL, nil, "", false, true, discover8)
if !ok || l != 8 || src != "discover" || k != discover8 {
t.Fatalf("8-byte discover pubkey: got k=%q l=%d src=%q ok=%v", k, l, src, ok)
}
k, l, src, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadCONTROL, nil, "", false, true, strings.ToUpper(discover32))
if !ok || l != 32 || src != "discover" || k != discover32 {
t.Fatalf("32-byte discover pubkey: got k=%q l=%d src=%q ok=%v", k, l, src, ok)
}
// Malformed/short discover payload (neither 8 nor 32 bytes) must be rejected outright, not truncated.
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadCONTROL, nil, "", false, true, "aabbcc"); ok {
t.Fatalf("short discover pubkey (3 bytes) must be rejected")
}
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadCONTROL, nil, "", false, true, ""); ok {
t.Fatalf("empty discover pubkey must be rejected")
}
// Wrong control type (not a DISCOVER_RESP) must be rejected even with a well-formed pubkey.
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteFlood, PayloadCONTROL, nil, "", false, false, discover8); ok {
t.Fatalf("non-discover CONTROL payload must be rejected")
}
// A discover response that arrived with hops is not a direct reception.
if _, _, _, ok = deriveHeardKey("rx", packetpath.RouteDirect, PayloadCONTROL, []string{"aa", "bbccdd"}, "", false, true, discover8); ok {
t.Fatalf("discover response with hops (DIRECT route) must be rejected")
}
}
func TestBuildClientReception(t *testing.T) {
acc := 8.0
rec, ok := buildClientReception("companionpk", "rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false,
rec, ok := buildClientReception("companionpk", "rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"aa", "bbccdd"}, "", false, false, "",
crF(-7.5), crI(-92), 51.05, 3.72, &acc, "2026-06-09T12:00:00Z", "2026-06-09T12:00:01Z")
if !ok || rec.HeardKey != "bbccdd" || rec.HeardKeyLen != 3 || rec.Src != "rxlog" {
t.Fatalf("bad reception: %+v ok=%v", rec, ok)
}
if _, ok := buildClientReception("c", "rx", packetpath.RouteDirect, PayloadGRP_TXT, []string{"bbccdd"}, "", false,
if _, ok := buildClientReception("c", "rx", packetpath.RouteDirect, PayloadGRP_TXT, []string{"bbccdd"}, "", false, false, "",
crF(-7.5), crI(-92), 51.05, 3.72, nil, "t", "t"); ok {
t.Fatal("direct-route path must be rejected (not the transmitter)")
}
if _, ok := buildClientReception("c", "rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"bbccdd"}, "", false, nil, nil, 99.0, 3.72, nil, "t", "t"); ok {
if _, ok := buildClientReception("c", "rx", packetpath.RouteFlood, PayloadGRP_TXT, []string{"bbccdd"}, "", false, false, "", nil, nil, 99.0, 3.72, nil, "t", "t"); ok {
t.Fatal("out-of-range lat must be rejected")
}
if _, ok := buildClientReception("c", "rx", packetpath.RouteFlood, PayloadTRACE, []string{"aa", "bbccdd"}, "", false,
if _, ok := buildClientReception("c", "rx", packetpath.RouteFlood, PayloadTRACE, []string{"aa", "bbccdd"}, "", false, false, "",
crF(-7.5), crI(-92), 51.05, 3.72, nil, "t", "t"); ok {
t.Fatal("FLOOD-routed TRACE must be rejected (path bytes are SNR values, not node hashes)")
}
acc32 := strings.Repeat("ab", 32)
if rec, ok := buildClientReception("c", "rx", packetpath.RouteFlood, PayloadCONTROL, nil, "", false, true, acc32,
crF(-7.5), crI(-92), 51.05, 3.72, nil, "t", "t"); !ok || rec.HeardKey != acc32 || rec.HeardKeyLen != 32 || rec.Src != "discover" {
t.Fatalf("discover reception: %+v ok=%v", rec, ok)
}
}
func TestInsertClientReceptionRoundTripAndIdempotent(t *testing.T) {
@@ -504,6 +537,38 @@ func TestClientPacketFloodWritesBoth(t *testing.T) {
}
}
// TestHandleClientPacketDiscoverRespWritesReception is the end-to-end
// regression test for the discover-response gap: a zero-hop CONTROL packet
// carrying a CTL_TYPE_NODE_DISCOVER_RESP (firmware
// examples/simple_repeater/MyMesh.cpp:780) must attribute a client_receptions
// row keyed on the responder's own pubkey, src='discover' — not be silently
// dropped as it was before this fix.
func TestHandleClientPacketDiscoverRespWritesReception(t *testing.T) {
s := newTestStore(t)
// header 0x2D = route_type 1 (FLOOD), payload_type 0x0B (CONTROL).
// path byte 0x00 = 0 hops (a discover response is always heard direct).
// payload: flags(0x92 = CTL_TYPE_NODE_DISCOVER_RESP|ADV_TYPE_REPEATER) +
// snr(0x0A) + tag(4B LE) + 32-byte pubkey.
pubkey := strings.Repeat("ab", 32)
raw := "2d" + "00" + "920a44332211" + pubkey
msg := map[string]interface{}{
"raw": raw, "direction": "rx", "SNR": 4.5, "RSSI": -101.0,
"timestamp": "2026-08-17T10:00:00.123Z",
"gps": map[string]interface{}{"lat": 51.2, "lon": 4.4, "acc_m": 8.0},
}
handleClientPacket(s, cfgWithObservations(), "test", "aa11", msg, nil, nil)
var heardKey, src string
var keylen int
if err := s.db.QueryRow(`SELECT heard_key, heard_keylen, src FROM client_receptions WHERE rx_pubkey=?`, "aa11").
Scan(&heardKey, &keylen, &src); err != nil {
t.Fatalf("expected a discover-response reception: %v", err)
}
if heardKey != pubkey || keylen != 32 || src != "discover" {
t.Fatalf("discover reception: want heard_key=%s/32/discover, got %s/%d/%s", pubkey, heardKey, keylen, src)
}
}
// TestClientObservationsGateOff verifies that with clientRxObservations
// disabled, a client packet writes zero observation rows.
func TestClientObservationsGateOff(t *testing.T) {
+12 -4
View File
@@ -224,16 +224,24 @@ func sortedCoverageNodes(m map[string]*covNodeAgg) (nodes []CoverageNode, trunca
type bbox struct{ MinLat, MinLon, MaxLat, MaxLon float64 }
// coverageHeardKeyCandidates returns the exact heard_key values that identify a
// node: its full pubkey (stored with heard_keylen 32) and the 2-byte (4 hex) and
// 3-byte (6 hex) prefixes a relay logs. Matching heard_key IN (these) is
// node: its full pubkey (stored with heard_keylen 32), the 8-byte (16 hex) prefix
// a node-discover response carries, and the 2-byte (4 hex) and 3-byte (6 hex)
// prefixes a relay logs.
//
// The 16-hex entry is load-bearing, not defensive: corescope-rx asks for
// DISCOVER_PREFIX_ONLY (src/app.js sends CTRL_NODE_DISCOVER_REQ|0x01 to save 24
// bytes of airtime per reply), and the firmware then answers with 6+8 bytes
// (simple_repeater/MyMesh.cpp:799-805). So essentially EVERY discover-attributed
// reception is an 8-byte key; without this candidate they are stored and then
// never matched by any per-node coverage query. Matching heard_key IN (these) is
// equivalent to the old "heard_keylen=32 AND heard_key=? OR heard_keylen IN (2,3)
// AND substr(?,1,keylen*2)=heard_key", but sargable — so the (heard_key, …)
// composite index seeks the few matching rows instead of scanning the bbox (#5).
func coverageHeardKeyCandidates(pubkey string) []string {
pk := strings.ToLower(pubkey)
seen := map[string]bool{}
out := make([]string, 0, 3)
for _, c := range []string{pk, prefixOrEmpty(pk, 6), prefixOrEmpty(pk, 4)} {
out := make([]string, 0, 4)
for _, c := range []string{pk, prefixOrEmpty(pk, 16), prefixOrEmpty(pk, 6), prefixOrEmpty(pk, 4)} {
if c != "" && !seen[c] {
seen[c] = true
out = append(out, c)
+30
View File
@@ -239,3 +239,33 @@ func TestHexSizeRendersConstantPx(t *testing.T) {
}
}
}
// TestCoverageHeardKeyCandidatesIncludesDiscoverPrefix pins the 8-byte (16 hex)
// candidate. corescope-rx requests DISCOVER_PREFIX_ONLY, so the firmware answers
// with a 6+8 byte body and essentially every discover-attributed reception is
// stored under an 8-byte heard_key. Without this candidate those rows exist in
// client_receptions and are matched by no per-node coverage query — stored, and
// invisible on the node page.
func TestCoverageHeardKeyCandidatesIncludesDiscoverPrefix(t *testing.T) {
pk := "efef7943505052b47f1809488ea4b4d3942d4ed72d2b1953b90a9f5e62a65fb5"
got := coverageHeardKeyCandidates(pk)
want := map[string]bool{
pk: false,
"efef7943505052b4": false, // 8-byte discover prefix
"efef79": false, // 3-byte relay hop
"efef": false, // 2-byte relay hop
}
for _, c := range got {
if _, ok := want[c]; !ok {
t.Errorf("unexpected candidate %q", c)
continue
}
want[c] = true
}
for c, seen := range want {
if !seen {
t.Errorf("candidate %q missing from %v", c, got)
}
}
}
+9 -4
View File
@@ -46,7 +46,7 @@ packet (promiscuous, incl. overheard flood traffic), not just messages addressed
is connected over BLE (`_serial->isConnected()`).
So per received packet the app gets SNR + RSSI + the raw bytes. It decodes the raw packet (standard
MeshCore format) to derive the directly-heard node (`path[last]` or 0-hop advert pubkey) and pairs it
MeshCore format) to derive the directly-heard node (`path[last]`, a 0-hop advert pubkey, or a 0-hop node-discover response pubkey) and pairs it
with the phone's GPS. The bare advert push (`PUSH_CODE_ADVERT` 0x80) carries only a pubkey (no SNR/
RSSI/path) and is NOT used — 0x88 already covers adverts (the raw advert is in its payload).
@@ -140,11 +140,16 @@ client_receptions(
UNIQUE(rx_pubkey, heard_key, rx_at)) -- idempotent re-ingest
```
`heard_keylen` is 32 for a full pubkey (0-hop advert) or 2/3 for a multibyte prefix. `src` is
`advert` or `rxlog`. No hex cell is stored — binning is computed server-side from lat/lon.
`heard_keylen` is 32 for a full pubkey (0-hop advert, or a node-discover response that carried the
full key) or 2, 3 or 8 for a multibyte prefix. `src` is `advert`, `rxlog` or `discover`.
`discover` rows come from a 0-hop CONTROL/DISCOVER_RESP, which carries the responder's OWN pubkey
in its payload — a first-hand identification, not a path inference. corescope-rx asks for
DISCOVER_PREFIX_ONLY to save airtime, so those keys are normally 8 bytes; per-node coverage queries
must therefore include the 16-hex prefix among their heard_key candidates. No hex cell is stored — binning is computed server-side from lat/lon.
Indexes: a composite `(heard_key, heard_keylen, lat, lon)` and a `(lat, lon)` index back the coverage
queries; the per-node query matches a sargable `heard_key IN (pubkey, prefix6, prefix4)` list so the
queries; the per-node query matches a sargable `heard_key IN (pubkey, prefix16, prefix6, prefix4)` list so the
composite is used instead of a table scan (see the benchmark in `cmd/ingestor`).
Retention: the table grows on every submission, so set `retention.clientRxDays` (ingestor) to delete