UnlockModal has no optimistic update or rollback — an explicit TODO(#28) marks the gap in source

[prodbycorne]

Problem

UnlockModal.handleUnlock (src/components/UnlockModal/UnlockModal.tsx) submits the unlock transaction, waits for on-chain confirmation via unlockAssets, and on success only calls trackEvent('unlock_succeeded', ...) and shows a success toast — it never updates the USER_POSITION/USER_CREDITS React Query cache. The source itself flags this as unfinished, immediately after the success branch:

// TODO(#28): optimistic queryClient.getQueryData update attaches here pending
//            maintainer confirmation — see issue discussion

(line 217-218). Compare this to useLockAssets in src/hooks/useSorobanQuery.ts (lines 146-172), which does call queryClient.invalidateQueries(...) across POOLS, USER_POSITION (three different key variants), USER_CREDITS, PLATFORM_STATS, and stellarBalance on a successful deposit. Unlock has no equivalent invalidation at all in the modal's own success path — the closed position/reduced stake only becomes visible once the next scheduled poll (useUserPosition's 30s refetchInterval) or a useSorobanEvents invalidation fires. Combined with the useSorobanEvents permanent-stall bug described elsewhere in this batch, a user who successfully unlocks can be shown a stale, still-locked position for up to 30+ seconds with no optimistic feedback that their action actually took effect — undermining trust in the very confirmation screen that's supposed to reassure them the unlock succeeded.

Acceptance Criteria

• Implement an optimistic update on unlock submission (reduce the cached UserPosition.amount/adjust isLocked immediately) with a rollback path if the transaction ultimately fails or times out, following the useOptimisticUpdate helper already defined in src/hooks/useSorobanQuery.ts (currently unused by any component — verify and wire it up here).
• On confirmed success, invalidate the same set of query keys useUnlockAssets/useLockAssets already invalidate, from within UnlockModal's flow (or move the modal to use the useUnlockAssets mutation hook instead of calling unlockAssets directly, which would get invalidation for free).
• Resolve/close out the TODO(Migrate farm page state to Zustand store to eliminate prop-drilling and stale-closure bugs #28) marker with the actual implementation.
• Add a test asserting the cached position updates immediately on submission and correctly rolls back if the mutation fails.

Relevant Files

• src/components/UnlockModal/UnlockModal.tsx — TODO(Migrate farm page state to Zustand store to eliminate prop-drilling and stale-closure bugs #28) at L217-218 marks the missing optimistic update; handleUnlock never invalidates or optimistically updates any query cache on success
• src/hooks/useSorobanQuery.ts — useOptimisticUpdate (~L378-414) exists but has no callers; useUnlockAssets (~L189) does invalidate correctly but UnlockModal bypasses it by calling unlockAssets() directly

Additional Notes

Confirmed against current source

UnlockModal.handleUnlock's success branch (re-read directly, current lines ~197-217):

setStep("success");
toast.success(
  "Unlock Submitted",
  `${numericAmount} ${position.symbol} unlock transaction submitted successfully`
);
trackEvent("unlock_succeeded", {
  farm: position.name,
  symbol: position.symbol,
  amount: numericAmount,
  hash,
  partial: numericAmount < position.lockedAmount,
  processingTime: Date.now() - trackingStartTime,
});
// TODO(#28): optimistic queryClient.getQueryData update attaches here pending
//            maintainer confirmation — see issue discussion

Confirmed: no queryClient call of any kind follows — the modal does not even call .invalidateQueries, let alone an optimistic .setQueryData. useOptimisticUpdate (src/hooks/useSorobanQuery.ts lines 378-408) is confirmed to have zero callers anywhere in src/ (grep -rn "useOptimisticUpdate" src only matches its own definition and export). It exposes exactly three setters (updateUserPosition, updateCredits, updatePlatformStats), each a thin wrapper over queryClient.setQueryData with no built-in snapshot/rollback mechanism — the hook only writes forward; a caller wanting rollback-on-failure must manually capture the prior value via queryClient.getQueryData(...) before calling the setter and restore it manually in a catch block. This is an important detail the acceptance criteria doesn't spell out: "wire up useOptimisticUpdate" is necessary but not sufficient — the rollback logic has to be built new, not found already in the hook.

useUnlockAssets (src/hooks/useSorobanQuery.ts ~L189+) does its own separate invalidation in its onSuccess, but UnlockModal bypasses it entirely by calling the free function unlockAssets(...) directly (confirmed at the await unlockAssets({...}) call in handleUnlock) rather than using the useUnlockAssets() mutation hook's .mutate/.mutateAsync. This means switching the modal to consume the hook (acceptance criterion's alternate option) is a larger refactor than it sounds: handleUnlock currently drives its own local step/error/txHash state machine synchronized with onHash/onStep callbacks passed into the free function; moving to the mutation hook would require reconciling the hook's isPending/isError/data lifecycle with this local step machine, or discarding the local step machine in favor of derived state from the mutation — a real design decision, but a narrow, well-bounded one (unlike issue #83's broader architectural reconciliation).

Additional edge cases / failure modes not yet covered

1. Rollback timing vs. the TIMEOUT status. handleUnlock's failure branch distinguishes result.status === "TIMEOUT" (confirmation taking too long, transaction may still land) from a genuine error/rejected transaction. An optimistic update must NOT be rolled back on a TIMEOUT result the same way it's rolled back on a hard failure — the transaction may still confirm later. Rolling back optimistically-reduced stake on a timeout, only to have the transaction actually succeed moments later, would show the user a confusing "your unlock disappeared, then came back" flicker. The acceptance criteria's "rollback path if the transaction ultimately fails or times out" phrasing conflates these two cases; a correct implementation should roll back on error, and on TIMEOUT should likely leave the optimistic state in place until the next real poll/event resolves it definitively (or show a distinct "unconfirmed" visual state rather than reverting).
2. Partial unlocks. handleUnlock already computes partial: numericAmount < position.lockedAmount for analytics — the optimistic update needs to reduce UserPosition.amount by exactly numericAmount, not zero it out, for the common partial-unlock case; a naive optimistic implementation that just sets isLocked: false/amount to 0 would be wrong for any unlock that isn't a full withdrawal.
3. Interaction with the useSorobanEvents permanent-stall bug (issue useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72). This issue's own body already notes that in the absence of an optimistic update, the user relies on the 30s refetchInterval or useSorobanEvents to eventually show the real state. Issue useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72 in this batch documents that useSorobanEvents can permanently stop invalidating after a single failed getEvents call. If both bugs are present simultaneously, a user's optimistic update (once implemented) becomes the only signal of the unlock's success — meaning this issue's rollback correctness matters even more once useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72 is fixed independently, and a PR for either issue should not assume the other is also fixed. Recommend the reviewer check this issue's fix doesn't implicitly depend on useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72 being resolved to look correct in the happy path.
4. Precision consideration shared with issue Unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits, risking precision loss #76. Issue Unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits, risking precision loss #76 in this batch documents that unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits. If the optimistic update computes the new UserPosition.amount by subtracting a float-derived numericAmount from the cached BigInt-derived position amount, the optimistic value and the eventual real value (from the next authoritative fetch) could differ by a rounding epsilon, causing a visible "flicker" correction once the real data arrives. Whoever implements this optimistic update should use the same precise (BigInt-based) amount conversion path Unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits, risking precision loss #76 is asking unlockAssets to adopt, not re-derive a separate float-based delta locally in the modal — otherwise this fix and Unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits, risking precision loss #76 could ship independently and still leave a residual rounding-driven flicker.

Implementation sketch

// before calling unlockAssets:
const snapshot = queryClient.getQueryData([QUERY_KEYS.USER_POSITION, selectedPoolContractId, publicKey]);
updateUserPosition(selectedPoolContractId, publicKey, (old) =>
  old ? { ...old, amount: subtractPrecise(old.amount, amount) } : old
);

try {
  const result = await unlockAssets({ ... });
  if (!result.success) {
    if (result.status !== "TIMEOUT") {
      queryClient.setQueryData([QUERY_KEYS.USER_POSITION, selectedPoolContractId, publicKey], snapshot);
    }
    // ... existing error handling
    return;
  }
  // on confirmed success: invalidate for authoritative refresh
  queryClient.invalidateQueries({ queryKey: [QUERY_KEYS.USER_POSITION] });
  queryClient.invalidateQueries({ queryKey: [QUERY_KEYS.USER_CREDITS, selectedPoolContractId] });
  queryClient.invalidateQueries({ queryKey: [QUERY_KEYS.PLATFORM_STATS] });
  queryClient.invalidateQueries({ queryKey: ['stellarBalance', publicKey] });
} catch (err) {
  queryClient.setQueryData([QUERY_KEYS.USER_POSITION, selectedPoolContractId, publicKey], snapshot);
  // ... existing catch handling
}

Testing strategy for reviewers

• The required test ("cached position updates immediately on submission and correctly rolls back if the mutation fails") should explicitly cover three outcomes, not just "success" vs. "failure": success, hard failure (rollback expected), and TIMEOUT (rollback behavior is a judgment call per edge case 1 above — the PR should state and test whichever behavior it picks).
• Verify partial-unlock math: submit an unlock for less than the full locked amount and assert the optimistically-updated cache reflects the reduced (not zeroed) amount.
• Verify the invalidation set on confirmed success matches (or the PR explains why it deliberately doesn't match) the set useLockAssets/useUnlockAssets already invalidate on their own onSuccess — POOLS, USER_POSITION (all key variants), USER_CREDITS, PLATFORM_STATS, stellarBalance.
• Confirm the TODO(#28) comment itself is deleted as part of the fix — a PR that adds the optimistic update but leaves the stale TODO comment in place should be flagged.
• Cross-reference: issue Unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits, risking precision loss #76 (unlock's float vs BigInt stroop-conversion precision) and issue useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72 (useSorobanEvents permanent invalidation stall) both interact with this fix as described above; a thorough reviewer should check whether this PR's optimistic-update math independently reintroduces the Unlock's float-based stroop conversion diverges from the exact BigInt conversion used for deposits, risking precision loss #76 precision mismatch, and whether this PR's correctness claims implicitly assume useSorobanEvents permanently stalls real-time invalidation after a single failed getEvents call #72 is already fixed.