db: restore unique partial index on user.npub with preflight dedup#93
Closed
escapedcat wants to merge 1 commit into
Closed
db: restore unique partial index on user.npub with preflight dedup#93escapedcat wants to merge 1 commit into
escapedcat wants to merge 1 commit into
Conversation
Migration 101 originally added this index as part of the Nostr-auth work (PR #89), but it was reverted in be18b50 to avoid a merge conflict with the multi-vendor import PR — the maintainer noted on the PR thread that the constraint would land in a follow-up. This is that follow-up. The endpoint handler in src/rest/v4/nostr.rs is already written to treat a user.npub UNIQUE violation as the lost-race recovery path for create_or_recover. Without the index, that branch is dormant and two simultaneous first-time Nostr sign-ins for the same pubkey can produce two rows. Pre-flight (per CodeRabbit's original concern on PR #89): between the endpoint going live on 2026-05-10 and this migration landing, duplicate-npub rows are possible. For each duplicate group, keep the oldest row's npub link and NULL out the rest. Losing rows retain all other state (saved items, existing tokens) — they just lose the Nostr identity link, so the user's next Nostr sign-in lands deterministically on the winning row. Schema bootstrap (schema.sql) updated to match. Note: src/rest/v4/nostr.rs::unknown_npub_concurrent_inserts_create_exactly_one_user passes today for the wrong reason — :memory: SQLite gives each connection its own private DB, so the two "concurrent" inserts never share state. Worth rewriting in a separate change to actually exercise the race the index now prevents.
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
1 participant
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.
Migration 101 originally added this index as part of the Nostr-auth work (PR #89), but it was reverted in be18b50 to avoid a merge conflict with the multi-vendor import PR — the maintainer noted on the PR thread that the constraint would land in a follow-up.
This is that follow-up.
The endpoint handler in src/rest/v4/nostr.rs is already written to treat a user.npub UNIQUE violation as the lost-race recovery path for create_or_recover. Without the index, that branch is dormant and two simultaneous first-time Nostr sign-ins for the same pubkey can produce two rows.
Pre-flight (per CodeRabbit's original concern on PR #89): between the endpoint going live on 2026-05-10 and this migration landing, duplicate-npub rows are possible. For each duplicate group, keep the oldest row's npub link and NULL out the rest. Losing rows retain all other state (saved items, existing tokens) — they just lose the Nostr identity link, so the user's next Nostr sign-in lands deterministically on the winning row.
Schema bootstrap (schema.sql) updated to match.
Note: src/rest/v4/nostr.rs::unknown_npub_concurrent_inserts_create_exactly_one_user passes today for the wrong reason — :memory: SQLite gives each connection its own private DB, so the two "concurrent" inserts never share state. Worth rewriting in a separate change to actually exercise the race the index now prevents.