Optimize filter by topic queries

[mkysel]

Restructure topic filter query to order and limit meta rows before blob join and update message.Service.fetchEnvelopes to return []queries.GatewayEnvelopesView via internal conversion

Update the topic-based query to select and limit meta rows prior to joining blobs, introduce queries.SelectGatewayEnvelopesByTopicsRow as the return type for SelectGatewayEnvelopesByTopics, and add a message.transformRows helper to convert rows to []queries.GatewayEnvelopesView for message.Service.fetchEnvelopes. Adjust parameter order to $1/$2 for cursor arrays, $3 for row_limit, and $4 for topics, and propagate the new row type through validation interfaces, implementations, tests, and mocks.

📍Where to Start

Start with the SQL definition for SelectGatewayEnvelopesByTopics in envelopes_v2.sql, then review the Go bindings in envelopes_v2.sql.go, followed by the message.Service.fetchEnvelopes changes in service.go.

---

📊 Macroscope summarized f6453c3. 5 files reviewed, 6 issues evaluated, 5 issues filtered, 0 comments posted

🗂️ Filtered Issues

pkg/api/message/service.go — 0 comments posted, 3 evaluated, 2 filtered

• line 387: Potential integer truncation: in the originators path, OriginatorNodeIds are converted with int32(o). If query.GetOriginatorNodeIds() contains values outside the int32 range, this will silently truncate and yield incorrect IDs, causing wrong DB filtering or data leakage. Validate the range before casting or change the parameter type to accommodate larger IDs. [ Out of scope ]
• line 401: Missing validation for rowLimit: the code passes rowLimit directly to query params without guarding against negative values. If a negative rowLimit is provided, DB query behavior may be undefined or error-prone (e.g., database rejecting the limit, returning no rows, or causing an internal error). Add validation to ensure rowLimit is non-negative and handle zero appropriately (e.g., treat zero as no results or as a default limit). [ Out of scope ]

pkg/db/queries/envelopes_v2.sql.go — 0 comments posted, 2 evaluated, 2 filtered

• line 146: The SQL parameter order was changed compared to the previous query, but the constant retained the same name SelectGatewayEnvelopesByTopics. The new query now expects $1::INT[] (node IDs) and $2::BIGINT[] (sequence IDs) to build the cursors CTE, $3::INT for the LIMIT, and $4::BYTEA[] for topic filtering. Previously, the query expected $1::BYTEA[] (topics), $2::INT (limit), $3::INT[] (node IDs), and $4::BIGINT[] (sequence IDs). If the generated Go method signature and its callers were not updated to match this new parameter ordering, runtime execution will fail with PostgreSQL type errors (e.g., attempting to unnest($1::INT[]) when $1 is actually a BYTEA[] of topics), or worse, produce incorrect results if some casts were accepted. This is a breaking contract change for the query name and is a runtime bug unless all call sites were updated to pass parameters in the new order. [ Low confidence ]
• line 180: The query applies ORDER BY ... LIMIT NULLIF($3::INT, 0) within the filtered CTE, before performing the JOIN gateway_envelope_blobs AS b to fetch originator_envelope. If there exist meta rows without a corresponding blob row (e.g., due to transient inconsistency, delayed blob insertion, or partial replication), the inner join will drop those rows after the limit has already been applied. This can yield fewer than the requested limit of returned envelopes, and worse, it can permanently skip eligible later meta rows that do have blobs because they were excluded by the early limit. The previous implementation selected from gateway_envelopes_view (which likely joins meta and blobs) and applied LIMIT at the final step, ensuring the limit applies to actual joined rows. This change risks under-delivery and starvation under realistic data inconsistencies. [ Low confidence ]

pkg/mlsvalidate/service.go — 0 comments posted, 1 evaluated, 1 filtered

• line 110: GetAssociationStateFromEnvelopes does not validate newUpdate before passing it to GetAssociationState. If newUpdate is nil (which is reachable given callers like identity_update_storer.validateIdentityUpdate that do not check identityUpdate.IdentityUpdate for nil), the request will contain a slice with a nil element ([]*associations.IdentityUpdate{nil}). This can result in a malformed gRPC/protobuf request at marshal time or a server-side validation error. At minimum, this violates the implicit contract that NewUpdates contains valid messages. The function should explicitly check newUpdate != nil and return a clear error before making the RPC. [ Out of scope ]

[graphite-app]

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• Queue - adds this PR to the back of the merge queue
• Hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[neekolas]

Is this pure defensiveness against negative sequence IDs, or does it impact query perf by encouraging the planner to use an index it might skip otherwise?