Skip to content

plan to add state to accounts for expiring and soft deleting#960

Closed
gudnuf wants to merge 11 commits intomasterfrom
docs/account-state-lifecycle
Closed

plan to add state to accounts for expiring and soft deleting#960
gudnuf wants to merge 11 commits intomasterfrom
docs/account-state-lifecycle

Conversation

@gudnuf
Copy link
Copy Markdown
Contributor

@gudnuf gudnuf commented Mar 26, 2026

Summary

Design spec for adding active → expired → deleted lifecycle to wallet.accounts. For review/discussion before implementation.

  • Auto-expiry: pg_cron flips active → expired hourly when expires_at passes
  • Soft delete: Client-initiated RPC, hidden via restrictive RLS policy
  • One-way transitions: Enforced by DB trigger (no reactivation)
  • Partial unique index: Scoped to state = 'active' so expired/deleted accounts don't block new ones at the same mint

What this enables

  • Expired offer accounts stop showing as active without app-code changes to every query
  • Users can delete old offer accounts
  • New offers at the same mint work even after prior account expired

Open questions

  • Delete UI placement (hook is specced, UX is separate)
  • Expired balance recovery (proofs may still be swappable, separate feature)

No code changes — spec only.

gudnuf added 4 commits March 20, 2026 10:18
Two bugs with v2 keysets: getDecodedToken fails without keyset IDs
for resolution, and re-encoded tokens have truncated IDs that can't
be looked up. Fix uses dependency-injected keyset resolver with
cache-first strategy, plus raw token string passthrough in paste/scan.
- extractCashuTokenString validates via getTokenMetadata (not just regex)
- extractCashuToken uses try/catch like cashu.me for v2 detection
- Preserves immediate validation UX in paste/scan handlers
4 tasks: token functions + tests, resolver factory, paste/scan handlers,
route clientLoaders. Includes v2-specific tests with round-trip verification.
Design doc for adding active/expired/deleted lifecycle to wallet.accounts.
Covers DB migration (enum, transition trigger, partial unique index, RLS,
pg_cron auto-expiry), app-layer changes, and data flow diagrams.

For review/discussion before implementation.
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agicash Ready Ready Preview, Comment Apr 7, 2026 2:48am

Request Review

@supabase
Copy link
Copy Markdown

supabase bot commented Mar 26, 2026

This pull request has been ignored for the connected project hrebgkfhjpkbxpztqqke because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

gudnuf added 2 commits March 26, 2026 01:25
No longer needed — account state lifecycle replaces this approach.
Client-side expiry in upsert_user_with_accounts transitions stale accounts
before returning them. pg_cron remains as background cleanup for offline users.
Each DB function's WHERE clause only matches valid source states.
soft_delete_account gains AND state != 'deleted' guard.
Comment thread docs/plans/account-state-lifecycle.md Outdated
Comment thread docs/plans/account-state-lifecycle.md Outdated
Comment thread docs/plans/account-state-lifecycle.md Outdated
if not found then
raise exception
using
hint = 'NOT_FOUND',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if not found because account is already deleted should this return a different error message that its already deleted?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm I wonder if we should just make it idempotent and do nothing if already deleted

Comment thread docs/plans/account-state-lifecycle.md Outdated

This runs inside the existing transaction, before the `accounts_with_proofs` CTE that fetches accounts. The client receives already-expired accounts with `state = 'expired'` — no second round-trip needed.

### pg_cron job for auto-expiry
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we should keep this around for when the user doesn't open the app for a long time, but do you think its needed

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i actually think this is needed and not sure about one in upsert_user_with_accounts. see my comments above

Comment thread docs/plans/account-state-lifecycle.md Outdated
### `gift-cards.tsx` -- Simplify filter

```typescript
function useActiveOffers() {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is from code I haven't committed yet btw

@gudnuf gudnuf requested a review from jbojcic1 March 26, 2026 18:45
@orveth orveth assigned ditto-agent and gudnuf and unassigned ditto-agent and gudnuf Mar 27, 2026
@orveth orveth requested review from ditto-agent and removed request for jbojcic1 March 28, 2026 09:20
@gudnuf gudnuf changed the title Account state lifecycle spec plan to add state to accounts for expiring and soft deleting Mar 28, 2026
@orveth orveth added the plan Plan or spec document label Mar 28, 2026
Comment thread docs/plans/account-state-lifecycle.md Outdated

**Decision: App layer.**

Expired accounts remain visible in `getAll()` -- the RLS policy does not hide them. The existing `useActiveOffers()` filter in `gift-cards.tsx` is simplified from the `expiresAt > now()` check to `account.state === 'active'`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

depending on how we move state to expired there might be some time between expiresAt passing and state being change. we need to have that in mind and see if that poses any problems for us

Comment thread docs/plans/account-state-lifecycle.md Outdated

**Decision: Two layers — eager on user assertion, pg_cron as background cleanup.**

**Eager (on login):** `upsert_user_with_accounts` expires stale accounts before returning them. When a user opens the app, any account with `state = 'active'` and `expires_at <= now()` is transitioned to `expired` within the same transaction. The client gets correct state on first load — no stale-then-update flicker.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this adds any value since the account can expires after first load

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in favor of a 1 minute cron job plus client side error handling for expired accounts that have not yet transitioned to expired

Comment thread docs/plans/account-state-lifecycle.md Outdated

**Eager (on login):** `upsert_user_with_accounts` expires stale accounts before returning them. When a user opens the app, any account with `state = 'active'` and `expires_at <= now()` is transitioned to `expired` within the same transaction. The client gets correct state on first load — no stale-then-update flicker.

**Background (pg_cron, hourly):** A cron job catches accounts for users who haven't opened the app. This keeps the DB consistent for realtime broadcasts and prevents stale `active` accounts from accumulating. pg_cron is already installed and used for 8 daily cleanup jobs — no new infrastructure.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better to run every minute or see if there is some way to schedule some work to be done for the exact time

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude says there's no way to schedule an exact time so we just went for 1 minute

Comment thread docs/plans/account-state-lifecycle.md Outdated
Comment thread docs/plans/account-state-lifecycle.md Outdated
as restrictive
for select
to authenticated
using (state != 'deleted'::wallet.account_state);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this also requires index on state column to be efficient

Comment thread docs/plans/account-state-lifecycle.md Outdated
Comment thread docs/plans/account-state-lifecycle.md Outdated
Comment thread docs/plans/account-state-lifecycle.md Outdated
if not found then
raise exception
using
hint = 'NOT_FOUND',
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm I wonder if we should just make it idempotent and do nothing if already deleted

Comment thread docs/plans/account-state-lifecycle.md Outdated

This runs inside the existing transaction, before the `accounts_with_proofs` CTE that fetches accounts. The client receives already-expired accounts with `state = 'expired'` — no second round-trip needed.

### pg_cron job for auto-expiry
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i actually think this is needed and not sure about one in upsert_user_with_accounts. see my comments above

Comment thread docs/plans/account-state-lifecycle.md Outdated
```typescript
function useActiveOffers() {
const { data: offerAccounts } = useAccounts({ purpose: 'offer' });
return offerAccounts.filter((account) => account.state === 'active');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should use useQuery select for filtering like this. see other examples where we are already using it

orveth and others added 2 commits April 6, 2026 13:53
- Drop eager expiry from upsert_user_with_accounts, use 1-minute pg_cron only
- Mark deleted state as pending decision with Option A/B
- Rename soft_delete_account to delete_account
- Update enforce_accounts_limit to count only active accounts
- Add idx_accounts_state index
- Note useQuery select pattern for filtering
- Add graceful error handling for expired keysets
- Note PR #959 (offer mints) is merged

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…only

Decision: expired is the terminal state for v1. Removes the deleted enum
value, RLS restrictive policy, delete_account DB function, useDeleteAccount
hook, cache removal logic, and all PENDING DECISION sections. Simplifies
transitions to active -> expired only. Delete functionality deferred to
future PRs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move expired account filtering from client-side select to
AccountRepository.getAll() with .eq('state', 'active'). RLS is not
viable because security invoker DB functions need expired accounts
for in-flight operations. get(id) remains unfiltered for the same
reason. Simplify useActiveOffers() and update implementation phases.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Comment thread docs/plans/account-state-lifecycle.md Outdated

No eager expiry in `upsert_user_with_accounts` — the 1-minute cron granularity is sufficient for the UX. At most, a user sees a stale active account for up to 60 seconds before the next cron run corrects it.

**Graceful error handling:** If the client somehow triggers an operation on an expired account (race condition between cron expiring the account and user initiating an action), catch the mint rejection and show the user "this offer has expired". Trigger a cache invalidation to pull fresh account state from the server.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orveth Let's look into exactly when this error would occur and what the error code would be, if any, from the mint (nuts define these error codes). I want to know exactly how we will handle these errors and when they will occur.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Researched this. The error is KEYSET_INACTIVE (code 12002), already defined in error-codes.ts. It surfaces in 3 code paths:

  1. Melt — attempting to spend from an expired offer
  2. Send-swap — swapping tokens out of an expired keyset
  3. Receive-swap — swapping tokens into a keyset that expired between claim and swap

Handling approach: catch MintOperationError with code 12002, check account.purpose === 'offer', and throw a DomainError("This offer has expired"). All documented in the updated spec under the error handling section.

Comment thread docs/plans/account-state-lifecycle.md Outdated

### `gift-cards.tsx` -- Simplify filter

`useActiveOffers()` no longer needs to filter by `state === 'active'` since `getAll()` already excludes expired accounts at the query level. It becomes a simple query for `purpose: 'offer'` accounts:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orveth Let's just change this to useOfferCards or something that matches how the gift cards call its own hooks for a regular gift-card accounts. We don't need to call them active anymore.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done — renamed to useOfferCards throughout the spec in the latest push. Matches the gift-card hook naming convention.

Comment thread docs/plans/account-state-lifecycle.md Outdated
Client triggers operation on account just before cron expires it
-> Mint rejects operation (keyset expired)
-> Catch mint rejection, show "this offer has expired"
-> Trigger cache invalidation to pull fresh state
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orveth Is it really worth invalidating the cache Here, because we should get the real time update when it's actually invalidated, right?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed — the realtime ACCOUNT_UPDATED event handles this. Removed the explicit cache invalidation in the latest push; the spec now relies on the existing subscription.

Comment thread docs/plans/account-state-lifecycle.md Outdated

**Why not RLS?** All DB functions (`get_account_with_proofs`, `create_cashu_send_quote`, etc.) are `security invoker`, so they execute with the calling user's RLS context. A restrictive RLS policy hiding expired accounts would break any in-flight quote or swap operation whose account expires mid-transaction. The DB functions need to see expired accounts to complete already-started operations.

**Why not a `useQuery` `select` filter?** Since `getAll()` excludes expired accounts at the query level, there is no need for a client-side `select` filter -- the data simply never arrives at the client.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@orveth This line is redundant, I think. It's now obvious Because of the way that the query worksWe don't need to mention anything about this. Let's remove it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in the latest push. That paragraph was leftover context that the query structure already communicates.

…ve redundant section

- Add detailed graceful error handling for KEYSET_INACTIVE (12002) race window
- Scope error handling to offer accounts only via account.purpose check
- Rename useActiveOffers → useOfferCards throughout
- Remove redundant "Why not useQuery select" paragraph
- Replace explicit cache invalidation with realtime event handling
- Add 12002 catch task to Phase 3

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gudnuf
Copy link
Copy Markdown
Contributor Author

gudnuf commented Apr 13, 2026

we merged #984 so we don't need this

@gudnuf gudnuf closed this Apr 13, 2026
@gudnuf gudnuf deleted the docs/account-state-lifecycle branch April 13, 2026 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plan Plan or spec document

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants