Skip to content

fix(api): key challenge disbursement reads by specifier, not recipient#872

Merged
rickyrombo merged 3 commits into
mainfrom
mp/challenges-is-disbursed-by-specifier
May 29, 2026
Merged

fix(api): key challenge disbursement reads by specifier, not recipient#872
rickyrombo merged 3 commits into
mainfrom
mp/challenges-is-disbursed-by-specifier

Conversation

@rickyrombo
Copy link
Copy Markdown
Contributor

@rickyrombo rickyrombo commented May 29, 2026

Problem

v_challenge_disbursements exposes sol_reward_disbursements in the legacy challenge_disbursements shape and resolves user_id by INNER JOINing users on the recipient wallet. Any disbursement whose recipient doesn't resolve to a current user (wallet migrations, deactivated users, case mismatches) is silently dropped from the view.

Most callers only ask the identity-independent question "has this (challenge_id, specifier) been disbursed?" — and a reward is disbursed exactly once per (challenge_id, specifier) on-chain, regardless of recipient. For those callers, a dropped row makes a paid reward look unpaid:

  • /users/{id}/challenges — showed is_complete=true, is_disbursed=false, so the client offered an already-paid reward as claimable; the claim then failed ("No rewards to claim" / on-chain "specifier already used"). This was the original bug.
  • /challenges/undisbursed (HTTP handler and the sqlc query used by the claim path) — same: a dropped disbursement makes a paid specifier claimable, and the claim fails on-chain. This is the exact failure mode migration 0204 documents.
  • /challenges/infoweekly_pool_remaining overstated by any dropped disbursement.
  • coins redeem — the double-redeem guard counts 0 and allows a second redemption.

Fix

All specifier/challenge-keyed callers now read sol_reward_disbursements directly by (challenge_id, specifier), aligning them with how the reward manager dedupes disbursements on-chain. is_disbursed on /users/{id}/challenges is now keyed by specifier rather than the querying user.

The one caller that genuinely needs the recipient user_id/challenges/disbursements (admin listing) — inlines the users join, using LOWER(users.wallet) = recipient_eth_address per migration 0204 (the stale ddl/views file had the pre-0204 case-sensitive join and, because views/ applies after migrations/, was silently reverting the fix — so the deployed view was the broken one; inlining fixes that too).

The view is now unused but retained in this PR so the binary can deploy before the view is dropped. The drop is a stacked follow-up: #873.

Why reads, not a data migration

user_challenges is indexer-derived; a manual reattribution would be reverted on reindex and doesn't address the read inconsistency. Keying reads by specifier is self-healing for past and future occurrences. (Attribution in user_challenges is intentionally left unchanged.)

Tests

  • New TestUserChallengesDisbursedToDifferentUser: a disbursement attributed (by wallet) to a different user now reports is_disbursed=true for the completer.
  • Existing TestGetChallengeDisbursements covers the inlined join across all sorts/filters incl. challenge_user_id.
  • Full ./api + ./api/dbv1 suites pass.

Rollout

  1. Merge + deploy this (reads no longer touch the view; view still present).
  2. Then merge chore(api): drop the unused v_challenge_disbursements view #873 to drop the view.

🤖 Generated with Claude Code

The user-facing /users/{id}/challenges endpoint computed is_disbursed by
joining disbursements filtered to the querying user's own user_id. But a
reward is disbursed exactly once per (challenge_id, specifier) on-chain,
independent of which wallet/user received it.

When a disbursement's recipient resolves to a different user than
user_challenges records (e.g. trending ranks recomputed after payout, or a
wallet that no longer maps to that user), the completing user saw
is_complete=true / is_disbursed=false and the client offered the reward as
claimable. The claim/undisbursed path keys on (challenge_id, specifier)
only, so it correctly refused — leaving a stuck reward that errors on claim.

Match disbursements by (challenge_id, specifier) so an already-paid
specifier is never surfaced as claimable, aligning this endpoint with the
claim path. Read sol_reward_disbursements directly instead of
v_challenge_disbursements so disbursements whose recipient wallet does not
resolve to a current user are still counted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rickyrombo rickyrombo force-pushed the mp/challenges-is-disbursed-by-specifier branch from 1083657 to 5369712 Compare May 29, 2026 06:08
@rickyrombo rickyrombo changed the title fix(api): key challenge is_disbursed by specifier, not user fix(api): key challenge disbursement reads by specifier; drop v_challenge_disbursements May 29, 2026
…isbursements

Apply the specifier-keyed reasoning from the is_disbursed fix to every other
consumer of the v_challenge_disbursements compatibility view.

The view INNER JOINs sol_reward_disbursements to users on the recipient
wallet, so any disbursement whose recipient does not resolve to a current
user is silently dropped. Callers asking the identity-independent question
"has this (challenge_id, specifier) been disbursed?" then wrongly treat a
paid reward as unpaid:

- challenges/undisbursed (HTTP + the sqlc query used by the claim path):
  a dropped disbursement makes a paid specifier look claimable; the claim
  then fails on-chain as "specifier already used" (the failure mode
  migration 0204 documents).
- challenges/info: weekly_pool_remaining is overstated by any dropped
  disbursement.
- coins redeem: the double-redeem guard counts 0 and allows a second
  redemption.

These now read sol_reward_disbursements directly by (challenge_id,
specifier). The one endpoint that genuinely needs the recipient user_id,
challenges/disbursements, inlines the users join (using LOWER(users.wallet)
per 0204, which the stale ddl/views file was silently reverting).

The view itself is now unused but retained here so this binary can deploy
before it is dropped. A follow-up PR drops it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rickyrombo rickyrombo force-pushed the mp/challenges-is-disbursed-by-specifier branch from 5369712 to ff97aca Compare May 29, 2026 06:14
@rickyrombo rickyrombo changed the title fix(api): key challenge disbursement reads by specifier; drop v_challenge_disbursements fix(api): key challenge disbursement reads by specifier, not recipient May 29, 2026
Condense the repeated "read the raw table, not v_challenge_disbursements"
rationale: keep the load-bearing notes (the view-drops-unresolved-recipients
reasoning on the claim path, and the LOWER()/0204 join) and reduce the rest
to a single line each.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rickyrombo rickyrombo merged commit e1fbcdb into main May 29, 2026
5 checks passed
@rickyrombo rickyrombo deleted the mp/challenges-is-disbursed-by-specifier branch May 29, 2026 17:10
rickyrombo added a commit that referenced this pull request May 29, 2026
All API reads have been migrated off v_challenge_disbursements (#872), so the
compatibility view has no remaining consumers. Drop it via migration 0212,
remove the ddl/views definition, and update the schema dump.

Deploy ordering: merge/migrate this only after #872's binary is rolled out,
so no running code references the dropped view.

Note: the removed ddl/views file still carried the pre-0204 case-sensitive
join and, because views/ applies after migrations/, was silently reverting
the 0204 LOWER(users.wallet) fix. Dropping it removes that footgun.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
rickyrombo added a commit that referenced this pull request May 30, 2026
Stacked on #872. **Merge and migrate only after #872's binary is
deployed.**

#872 migrates every API read off `v_challenge_disbursements`, leaving it
unused. This PR removes it:

- migration `0208` — `DROP VIEW IF EXISTS v_challenge_disbursements;`
- deletes the `ddl/views/v_challenge_disbursements.sql` definition
- regenerates `sql/01_schema.sql` (clean −23-line, view-only) and the
migration tracker

### Why split from #872

`0208` drops the view while the *old* binary still references it.
Shipping the read migrations first (#872) means that by the time this
migration runs, no running code touches the view — no missing-relation
errors during a rolling deploy.

### Rollout
1. Merge + deploy #872 (binary no longer reads the view; view still
present).
2. Confirm rollout complete.
3. Merge this; `0208` drops the view.

### Note
The deleted `ddl/views` file still had the pre-`0204` case-sensitive
join, and since `views/` applies after `migrations/` it was silently
reverting `0204`'s `LOWER(users.wallet)` fix — so the deployed view was
the broken one. Dropping it removes that footgun (and #872 inlines the
correct `LOWER` join where the resolution is still needed).

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant