fix(relay): stop claiming an unpushable filter yields an exact COUNT - #3374
Open
beardthelion wants to merge 1 commit into
Open
fix(relay): stop claiming an unpushable filter yields an exact COUNT#3374beardthelion wants to merge 1 commit into
beardthelion wants to merge 1 commit into
Conversation
filter_fully_pushable() reports whether count_events() can produce an
exact count with no post-filtering. Several filter shapes broke that
promise: the constraint never reached SQL, so the pushed-down COUNT
returned every candidate row instead of the correct total.
A single #h value only becomes a SQL predicate when it parses as a
channel UUID, because callers derive channel_id by parsing it. A bare
NIP-29 group id ({"kinds":[9],"#h":["general"]}) left the query
unconstrained while still taking the fast path, counting every event in
every channel the caller can read instead of zero.
An empty value set is the same shape, and reaches every generic-tag arm
plus authors and ids. It matches no event, since filters_match rejects a
present constraint with no matching value, but filter_to_query_params
drops it rather than emitting a predicate. These parse straight from
client JSON. kinds: [] already had a match-nothing sentinel and is
unaffected.
All of them now fall back to the bounded query-plus-post-filter path,
which already computes the correct total. The REQ path was unaffected
throughout because it always post-filters.
Affects WS NIP-45 COUNT (handlers/count.rs) and HTTP /count
(api/bridge.rs).
Signed-off-by: beardthelion <56458543+beardthelion@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #3373.
filter_fully_pushabledecides whether a NIP-45 COUNT can be answered by a plain SQLCOUNT(*). When it returns true, both the WebSocket COUNT handler andPOST /countcallcount_events()with no post-filtering, on the promise in its doc comment of "an exact count without post-filtering".Some filter shapes broke that promise, because the constraint never reached SQL at all:
#hvalue that is not a channel UUID. Callers derivechannel_idby parsing the value, so{"kinds":[9],"#h":["general"]}, the shape a stock NIP-29 client sends, left the query with no channel predicate while still taking the fast path. The count came back as every matching event in every channel the caller can read, instead of zero.authorsandids.{"kinds":[9],"#t":[]}matches no event, sincefilters_matchrejects a present constraint with no matching value, butfilter_to_query_paramsdrops it rather than emitting a predicate. These parse straight from client JSON.kinds: []was already handled, via the match-nothing sentinel the DB layer short-circuits on. The REQ path was never affected, because it always post-filters.Fix
Report these shapes as not pushable, so they take the existing bounded
query_eventsplusfilters_matchfallback, which already computes the correct total. A single#hthat does parse as a UUID still pins the narrowerchannel_idpredicate and stays on the fast path.Falling back rather than short-circuiting to zero is deliberate for the
#hcase:filters_matchcompares the h-tag literal, andrequires_h_channel_scopeonly forces a resolvable h tag for channel-scoped kinds, so an event can legitimately carry a non-UUID h tag and match.For the empty-set case there is a cheaper option I did not take here: emit the match-nothing sentinel from
filter_to_query_paramsso the count stays exact and costs no query. That overloads the sentinel across every consumer of the shared query builder, so it seemed better as its own change if you want it.Testing
Unit tests in
handlers::req::testscover the non-UUID#h, empty tag sets on#h/#p/#e/#t/#d, and emptyauthors/ids, plus a guard test that a single UUID#hstays pushable. Each guard was confirmed load-bearing by deleting it on its own and watching its test fail.test_count_unpushable_filters_do_not_count_all_channelsine2e_relay.rscovers the wire behavior. Against a relay built frommainit fails, returning 4 for{"kinds":[9],"#h":["general"]}where the answer is 0, counting messages from an unrelated open channel. With the fix it returns 0. Its control assertion, a filter naming the channel's real UUID, returns 2 on both.cargo fmt --all --check,clippy --workspace --all-targets -D warnings, and thebuzz-relayunit suite are clean. One unrelated failure,api::mesh_demo::demo_join_forwarded_arm_round_trips_echo, reproduces identically onmain.Unrelated but noticed while working here:
uuid::Uuid::parse_straccepts simple, braced, urn, and uppercase spellings, so a non-canonical#hpinschannel_idwhilefilters_matchcompares the literal and rejects it, making COUNT and REQ disagree. That predates this change and I left it alone, since the right answer depends on whether UUID or literal equality is authoritative for#h.