Skip to content

fix(wallet): recover interrupted max unshield - #524

Draft
zotho wants to merge 2 commits into
mainfrom
ask-1865-frontend-recover-max-unshield-after-rpc-interruption
Draft

fix(wallet): recover interrupted max unshield#524
zotho wants to merge 2 commits into
mainfrom
ask-1865-frontend-recover-max-unshield-after-rpc-interruption

Conversation

@zotho

@zotho zotho commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • fail closed when an unshield attempt cannot produce a confirmed base-layer unshield signature
  • re-read strict live deposit state for every MAX attempt so an interrupted flow can be retried without stale amounts or a double decrease
  • preserve the last confirmed portfolio on RPC refresh failures; render unknown balances as $— and expose Retry instead of showing authoritative $0.00
  • add the verifier-first ASK-1865 regression script and acceptance document

Root cause

On the ASK-1845 preview, the second shield transaction landed, but concurrent base RPC/WebSocket failures interrupted the following unshield/portfolio refresh. No second base-layer unshield transaction landed. Legacy deposit reads converted RPC failures to null, while an unavailable portfolio could be presented as an empty/zero wallet.

Verification

  • cd frontend && bun run verify:max-unshield-recovery
  • cd frontend && bun run lint (passes with existing repository warnings)
  • cd frontend && bunx tsc --noEmit --incremental false
  • git diff --check
  • staged diff checked for unrelated files, Cyrillic, and secret-like material

A local frontend production build was intentionally not run because this checkout explicitly forbids it; Vercel remains the build/deploy gate.

Linear: ASK-1865
Related: ASK-1845

@vercel

vercel Bot commented Jul 24, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
loyal-admin Ready Ready Preview, Comment Jul 24, 2026 11:43am
loyal-app Ready Ready Preview, Comment Jul 24, 2026 11:43am
loyal-dashboard Ready Ready Preview, Comment Jul 24, 2026 11:43am
loyal-frontend Ready Ready Preview, Comment Jul 24, 2026 11:43am
solana-telegram-transactions Ready Ready Preview, Comment Jul 24, 2026 11:43am

Request Review

@charliecreates charliecreates Bot left a comment

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.

Reviewed commit a45cc0f3a511f5f3ca4bf01caafd4e232ecaa287. I found no high-confidence blocking correctness, reliability, security, or data-integrity issues in the changed paths.\n\nI inspected the full 12-file diff and relevant surrounding SDK/client call sites, including the fail-closed unshield lifecycle, live deposit rereads, portfolio refresh state propagation, retry UI, and verifier. Existing checks for this head are successful; I did not rerun local CI/build commands per the repository review constraints.

zotho commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

PullGuard

Risk: 72/100 - medium

The follow-up improves fail-closed behavior, but key post-unshield integration paths still bypass the promised stale and Retry state, and the verifier does not exercise them.

Main concerns

  • high / issue_mismatch frontend/src/hooks/use-wallet-desktop-data.ts: subscribePortfolio reports setup and refresh failures through options.onError and resolves, but this hook supplies no onError and relies on an outer catch that never fires, so retained balances remain marked current without Retry after RPC or WebSocket failures.
  • high / issue_mismatch frontend/src/components/wallet-sidebar/index.tsx: Both ShieldContent call sites omit onSuccess, so successful unshields in this sidebar never invoke walletDesktopData.refresh and depend solely on the WebSocket path whose interruption this PR is intended to recover.
  • medium / issue_mismatch frontend/src/hooks/use-shield.ts: executeUnshield reads the live deposit only for the tracked Kamino token, while non-Kamino MAX requests still use the UI-supplied requestedRawAmount, contradicting the stated every-MAX recovery behavior.
  • medium / weak_test frontend/scripts/verify-max-unshield-recovery.ts: The manual verifier invokes extracted helpers with synthetic callbacks and never exercises useShield, ShieldContent, useWalletDesktopData, or subscribePortfolio wiring, so it passes despite the integration gaps above.

Review first

  • frontend/src/hooks/use-wallet-desktop-data.ts
  • packages/solana-wallet/src/client.ts
  • frontend/src/components/wallet-sidebar/index.tsx
  • frontend/src/hooks/use-shield.ts
  • frontend/scripts/verify-max-unshield-recovery.ts

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