mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-06-07 08:31:37 +00:00
## Summary Fixes #673 - GRP_TXT packets whose message text contains a node's pubkey were incorrectly counted as packets for that node, inflating packet counts and type breakdowns - Two code paths in `store.go` used `strings.Contains` on the full `DecodedJSON` blob — this matched pubkeys appearing anywhere in the JSON, including inside chat message text - `filterPackets` slow path (combined node + other filters): replaced substring search with a hash-set membership check against `byNode[nodePK]` - `GetNodeAnalytics`: removed the full-packet-scan + text search branch entirely; always uses the `byNode` index (which already covers `pubKey`/`destPubKey`/`srcPubKey` via structured field indexing) ## Test Plan - [x] `TestGetNodeAnalytics_ExcludesGRPTXTWithPubkeyInText` — verifies a GRP_TXT packet with the node's pubkey in its text field is not counted in that node's analytics - [x] `TestFilterPackets_NodeQueryDoesNotMatchChatText` — verifies the combined-filter slow path of `filterPackets` returns only the indexed ADVERT, not the chat packet Both tests were written as failing tests against the buggy code and pass after the fix. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,107 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
_ "modernc.org/sqlite"
|
||||
)
|
||||
|
||||
const issue673NodePK = "7502f19f44cad6d7b626e1d811c00a914af452636182ccded3fd019803395ec9"
|
||||
|
||||
// setupIssue673Store builds an in-memory store with one repeater node having:
|
||||
// - one ADVERT packet (legitimately indexed in byNode)
|
||||
// - one GRP_TXT packet whose decoded text contains the node's pubkey (false-positive candidate)
|
||||
func setupIssue673Store(t *testing.T) (*PacketStore, *DB) {
|
||||
t.Helper()
|
||||
db := setupTestDB(t)
|
||||
|
||||
_, err := db.conn.Exec(
|
||||
"INSERT INTO nodes (public_key, name, role) VALUES (?, ?, ?)",
|
||||
issue673NodePK, "Quail Hollow Park", "repeater",
|
||||
)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
ps := NewPacketStore(db, nil)
|
||||
now := time.Now().UTC().Format(time.RFC3339)
|
||||
|
||||
pt4 := 4 // ADVERT
|
||||
pt5 := 5 // GRP_TXT
|
||||
|
||||
advertDecoded, _ := json.Marshal(map[string]interface{}{"pubKey": issue673NodePK})
|
||||
advert := &StoreTx{
|
||||
ID: 1,
|
||||
Hash: "advert_hash_673",
|
||||
PayloadType: &pt4,
|
||||
DecodedJSON: string(advertDecoded),
|
||||
FirstSeen: now,
|
||||
}
|
||||
|
||||
otherPK := "aabbccddaabbccddaabbccddaabbccddaabbccddaabbccddaabbccddaabbccdd"
|
||||
chatDecoded, _ := json.Marshal(map[string]interface{}{
|
||||
"srcPubKey": otherPK,
|
||||
"text": "Check out node " + issue673NodePK + " on the analyzer",
|
||||
})
|
||||
chat := &StoreTx{
|
||||
ID: 2,
|
||||
Hash: "chat_hash_673",
|
||||
PayloadType: &pt5,
|
||||
DecodedJSON: string(chatDecoded),
|
||||
FirstSeen: now,
|
||||
}
|
||||
|
||||
ps.mu.Lock()
|
||||
ps.packets = append(ps.packets, advert, chat)
|
||||
ps.byHash[advert.Hash] = advert
|
||||
ps.byHash[chat.Hash] = chat
|
||||
ps.byTxID[advert.ID] = advert
|
||||
ps.byTxID[chat.ID] = chat
|
||||
ps.byNode[issue673NodePK] = []*StoreTx{advert}
|
||||
ps.mu.Unlock()
|
||||
|
||||
return ps, db
|
||||
}
|
||||
|
||||
// TestGetNodeAnalytics_ExcludesGRPTXTWithPubkeyInText verifies that a GRP_TXT packet
|
||||
// whose message text contains a node's pubkey is not counted in that node's analytics.
|
||||
func TestGetNodeAnalytics_ExcludesGRPTXTWithPubkeyInText(t *testing.T) {
|
||||
ps, db := setupIssue673Store(t)
|
||||
defer db.Close()
|
||||
|
||||
analytics, err := ps.GetNodeAnalytics(issue673NodePK, 30)
|
||||
if err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
if analytics == nil {
|
||||
t.Fatal("expected analytics, got nil")
|
||||
}
|
||||
|
||||
for _, ptc := range analytics.PacketTypeBreakdown {
|
||||
if ptc.PayloadType == 5 {
|
||||
t.Errorf("GRP_TXT (type 5) should not appear in analytics for repeater node, got count=%d", ptc.Count)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestFilterPackets_NodeQueryDoesNotMatchChatText verifies that the slow path of
|
||||
// filterPackets (node filter combined with Since) does not return a GRP_TXT packet
|
||||
// whose pubkey appears only in message text, not in a structured pubkey field.
|
||||
func TestFilterPackets_NodeQueryDoesNotMatchChatText(t *testing.T) {
|
||||
ps, db := setupIssue673Store(t)
|
||||
defer db.Close()
|
||||
|
||||
yesterday := time.Now().Add(-24 * time.Hour).UTC().Format(time.RFC3339)
|
||||
result := ps.QueryPackets(PacketQuery{Node: issue673NodePK, Since: yesterday, Limit: 50})
|
||||
|
||||
if result.Total != 1 {
|
||||
t.Errorf("expected 1 packet for node (ADVERT only), got %d", result.Total)
|
||||
}
|
||||
for _, pkt := range result.Packets {
|
||||
if pkt["hash"] == "chat_hash_673" {
|
||||
t.Errorf("GRP_TXT with pubkey in message text was incorrectly returned for node query")
|
||||
}
|
||||
}
|
||||
}
|
||||
+13
-31
@@ -2126,9 +2126,15 @@ func (s *PacketStore) filterPackets(q PacketQuery) []*StoreTx {
|
||||
|
||||
// Pre-compute node filter parameters.
|
||||
var nodePK string
|
||||
var nodeHashSet map[string]bool
|
||||
hasNode := q.Node != ""
|
||||
if hasNode {
|
||||
nodePK = s.db.resolveNodePubkey(q.Node)
|
||||
indexed := s.byNode[nodePK]
|
||||
nodeHashSet = make(map[string]bool, len(indexed))
|
||||
for _, tx := range indexed {
|
||||
nodeHashSet[tx.Hash] = true
|
||||
}
|
||||
}
|
||||
|
||||
// Determine the source slice. Use index-based source when only node
|
||||
@@ -2182,10 +2188,7 @@ func (s *PacketStore) filterPackets(q PacketQuery) []*StoreTx {
|
||||
}
|
||||
}
|
||||
if hasNode {
|
||||
if tx.DecodedJSON == "" {
|
||||
return false
|
||||
}
|
||||
if !strings.Contains(tx.DecodedJSON, nodePK) && !strings.Contains(tx.DecodedJSON, q.Node) {
|
||||
if !nodeHashSet[tx.Hash] {
|
||||
return false
|
||||
}
|
||||
}
|
||||
@@ -6541,11 +6544,6 @@ func (s *PacketStore) GetNodeAnalytics(pubkey string, days int) (*NodeAnalyticsR
|
||||
return nil, err
|
||||
}
|
||||
|
||||
name := ""
|
||||
if n, ok := node["name"]; ok && n != nil {
|
||||
name = fmt.Sprintf("%v", n)
|
||||
}
|
||||
|
||||
fromTime := time.Now().Add(-time.Duration(days) * 24 * time.Hour)
|
||||
fromISO := fromTime.Format(time.RFC3339)
|
||||
toISO := time.Now().Format(time.RFC3339)
|
||||
@@ -6553,30 +6551,14 @@ func (s *PacketStore) GetNodeAnalytics(pubkey string, days int) (*NodeAnalyticsR
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
|
||||
// Collect packets from byNode index + text search (matches Node.js findPacketsForNode)
|
||||
// Collect packets from byNode index (time-filtered).
|
||||
// Raw JSON text search is intentionally avoided: a GRP_TXT packet whose message
|
||||
// text contains a node's pubkey is not a packet *for* that node.
|
||||
indexed := s.byNode[pubkey]
|
||||
hashSet := make(map[string]bool, len(indexed))
|
||||
for _, tx := range indexed {
|
||||
hashSet[tx.Hash] = true
|
||||
}
|
||||
var packets []*StoreTx
|
||||
if name != "" {
|
||||
for _, tx := range s.packets {
|
||||
if tx.FirstSeen <= fromISO {
|
||||
continue // Skip old packets early before expensive string matching
|
||||
}
|
||||
if hashSet[tx.Hash] {
|
||||
packets = append(packets, tx)
|
||||
} else if tx.DecodedJSON != "" && (strings.Contains(tx.DecodedJSON, name) || strings.Contains(tx.DecodedJSON, pubkey)) {
|
||||
packets = append(packets, tx)
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// Filter indexed packets by time range
|
||||
for _, p := range indexed {
|
||||
if p.FirstSeen > fromISO {
|
||||
packets = append(packets, p)
|
||||
}
|
||||
for _, p := range indexed {
|
||||
if p.FirstSeen > fromISO {
|
||||
packets = append(packets, p)
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user