Skip to content

fix(trade): prevent stale deposit balance reuse after wallet switches - #2430

Open
Bayyan16 wants to merge 2 commits into
dcccrypto:playgroundfrom
Bayyan16:fix/trade-wallet-switch-balance-lifecycle-2429
Open

fix(trade): prevent stale deposit balance reuse after wallet switches#2430
Bayyan16 wants to merge 2 commits into
dcccrypto:playgroundfrom
Bayyan16:fix/trade-wallet-switch-balance-lifecycle-2429

Conversation

@Bayyan16

@Bayyan16 Bayyan16 commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2429.

This PR makes the wallet-balance lifecycle in DepositWithdrawCard explicitly scoped to the currently active wallet and collateral mint.

Previously, the component stored asynchronously fetched token-balance state without binding that state to the wallet and mint that produced it. During a wallet switch, the previous wallet's balance and balance-derived actions could remain visible or actionable while the replacement wallet's balance request was still pending.

The original patch introduced a wallet-scoped balance snapshot and prevented obsolete asynchronous responses from being consumed under a different wallet scope.

This PR now also closes the remaining existing-account deposit path identified during review:

  • the visible deposit amount is cleared when the wallet/mint scope changes;
  • the exact raw value stored by the Max button is cleared at the same time;
  • deposit submission fails closed while the replacement wallet balance is unresolved;
  • the Deposit button remains disabled until the active wallet's balance has been verified.

Together, these protections ensure that neither displayed balance state nor a previously selected deposit amount can cross wallet or collateral-mint boundaries.


Problem

DepositWithdrawCard asynchronously fetches the connected wallet's collateral-token balance through:

connection.getTokenAccountBalance(associatedTokenAddress)

Before this change, the result was stored independently in local component state:

walletBalance
onChainDecimals

Those values did not carry any ownership identity proving which wallet and collateral mint produced them.

The component also keeps two representations of the selected transaction amount:

amount
maxRawRef.current

amount is the human-readable value displayed in the input, while maxRawRef.current preserves the exact native-unit bigint selected through the Max button.

Neither value was previously invalidated when the active wallet scope changed.


Stale balance race

The original balance-state race was:

  1. Wallet A is connected.
  2. Wallet A's balance request resolves and populates the component.
  3. The user switches to wallet B.
  4. A new balance request for wallet B starts but remains pending.
  5. Wallet A's previously resolved balance remains rendered.
  6. Wallet A-derived account-creation state remains visible or actionable under wallet B.
  7. A late response from an obsolete request may overwrite newer component state.

An effect cancellation flag prevented some writes after cleanup, but it did not provide a durable ownership relationship between the stored balance and the wallet for which that balance was fetched.


Stale Max deposit race

An additional existing-account path remained after the initial fix:

  1. Wallet A is active and has an existing trading account.

  2. Wallet A has 900 USDC.

  3. The user selects Max on the Deposit tab.

  4. The component stores:

    amount = "900"
    maxRawRef.current = 900_000_000n
  5. The user switches to wallet B.

  6. Wallet B's balance request remains pending.

  7. The wallet-scoped balance becomes unavailable:

    walletBalance === null
  8. The over-deposit check cannot compare the requested amount against wallet B because its balance has not resolved.

  9. Without additional protection, handleSubmit can still prefer the stale exact value:

    maxRawRef.current
  10. A deposit amount selected for wallet A may therefore remain actionable after wallet B becomes active.

The transaction would still require wallet B's signature and normal on-chain validation, but the client-side transaction intent and amount would incorrectly originate from wallet A's UI lifecycle.


Security and correctness invariant

Any balance or balance-derived value rendered or consumed by DepositWithdrawCard must belong to the exact active identity tuple:

connected wallet public key + collateral mint

After either part of that tuple changes:

  • the previous balance must no longer be displayed;
  • previous-wallet account-creation controls must not remain enabled;
  • the previous input amount must be invalidated;
  • the previous exact Max value must be invalidated;
  • account creation must not derive a starter deposit from stale state;
  • existing-account deposit submission must remain blocked while the new balance is unresolved;
  • obsolete asynchronous responses must not become consumable under the new scope;
  • the new balance must only become actionable after it has been verified for the active scope.

Root cause

The previous implementation had three related lifecycle gaps.

1. Balance data had no provenance

Balance and decimals were stored separately from the wallet and collateral mint that produced them:

const [walletBalance, setWalletBalance] = useState<bigint | null>(...);
const [onChainDecimals, setOnChainDecimals] = useState<number | null>(null);

A render after a wallet switch could therefore consume values fetched for the former wallet.

2. User-entered and Max-derived amounts were not scope-bound

The deposit input and exact raw Max value persisted independently from the active wallet scope:

const [amount, setAmount] = useState("");
const maxRawRef = useRef<bigint | null>(null);

Invalidating only the balance snapshot did not invalidate an amount already derived from that balance.

3. Deposit validation was incomplete while balance was pending

The over-deposit check can only compare values when the active wallet balance is known:

walletBalance !== null

While the replacement balance remained unresolved, that comparison was intentionally unavailable. Without a separate fail-closed guard, a stale raw Max amount could remain eligible for submission.


Implementation

1. Introduce a wallet-scoped balance snapshot

The component now stores balance data together with the ownership scope that produced it:

type WalletBalanceSnapshot = {
  scopeKey: string;
  amount: bigint | null;
  decimals: number | null;
};

The nullable amount allows the component to represent a known active scope whose balance is still pending or unavailable without reusing data from another scope.


2. Derive a scope from wallet and collateral mint

The scope key is derived from both active identity components:

const walletBalanceScopeKey =
  publicKey && mktConfig?.collateralMint
    ? `${publicKey.toBase58()}:${mktConfig.collateralMint.toBase58()}`
    : null;

This prevents balance data for:

  • another wallet;
  • another collateral mint; or
  • an incomplete/disconnected scope

from being treated as current.


3. Expose balance state only for a matching scope

The rendered balance is derived only when the stored snapshot belongs to the currently active wallet/mint scope:

const walletBalance = mockMode
  ? 500_000_000n
  : walletBalanceSnapshot?.scopeKey === walletBalanceScopeKey
    ? walletBalanceSnapshot.amount
    : null;

Decimals are protected by the same scope comparison.

This provides a render-time ownership check in addition to the asynchronous request lifecycle guards.

Even before an effect finishes processing the wallet change, a snapshot belonging to wallet A cannot be consumed while wallet B is active because its scopeKey no longer matches.


4. Invalidate cross-scope balance state

When a request begins for a different wallet or collateral mint, the component replaces the previous snapshot with a pending snapshot owned by the new scope:

setWalletBalanceSnapshot((current) =>
  current?.scopeKey === requestScopeKey
    ? current
    : {
        scopeKey: requestScopeKey,
        amount: null,
        decimals: null,
      },
);

This prevents the previous wallet's:

  • balance label;
  • token availability state;
  • account-creation prompt;
  • starter-deposit derivation; and
  • balance-dependent actions

from leaking into the replacement wallet lifecycle.

Same-scope post-transaction refreshes retain the already verified value while the refreshed request is pending.


5. Bind asynchronous requests to their originating scope

Each balance request captures the scope that initiated it:

const requestScopeKey = walletBalanceScopeKey;

A successful response is stored together with that exact scope:

setWalletBalanceSnapshot({
  scopeKey: requestScopeKey,
  amount: BigInt(info.value.amount),
  decimals: info.value.decimals ?? null,
});

A response from an obsolete request may still complete asynchronously, but it cannot become consumable under a different active scope.


6. Ignore cancelled request executions

Each effect execution retains a cancellation flag:

let cancelled = false;

Cleanup marks the request execution as cancelled:

return () => {
  cancelled = true;
};

Successful and failed requests update state only while their effect execution remains active.

A failed current-scope request leaves the scope in a fail-closed state with:

amount: null
decimals: null

It does not restore or reuse a previously verified balance from another wallet.


7. Clear visible and raw Max state on scope changes

The follow-up fix binds the selected deposit amount to the wallet/mint lifecycle:

useEffect(() => {
  maxRawRef.current = null;
  setAmount("");
}, [walletBalanceScopeKey]);

When either the wallet public key or collateral mint changes:

  • the human-readable input is cleared;
  • the exact native-unit value stored by Max is cleared;
  • an amount selected for wallet A cannot remain available under wallet B;
  • an amount selected for one collateral mint cannot cross into another market scope.

Both representations must be cleared because handleSubmit intentionally prioritizes maxRawRef.current to preserve exact token precision.


8. Fail closed while the active balance is unverified

The component derives an explicit pending/unverified condition for deposits:

const isDepositBalanceUnverified =
  !mockMode && mode === "deposit" && walletBalance === null;

The submit handler refuses to proceed while that condition is true:

async function handleSubmit() {
  if (
    !amount ||
    !userAccount ||
    validationError ||
    isDepositBalanceUnverified
  ) {
    return;
  }

  // Existing submission logic
}

The Deposit button uses the same guard:

disabled={
  loading ||
  !amount ||
  !!validationError ||
  isDepositBalanceUnverified
}

This creates two layers of protection:

  1. the UI disables the action while the replacement balance is unresolved;
  2. the submission handler independently rejects the action.

The handler-level guard is required even though the input is cleared by an effect because it closes any timing window between the scope-changing render and effect execution.


9. Preserve exact Max precision after verification

Once the active wallet's balance resolves, the existing Max behavior remains unchanged:

maxRawRef.current = walletBalance;
setAmount(formatTokenAmount(walletBalance, decimals));

The displayed amount may be formatted for human readability, while submission continues to use the exact native-unit bigint.

The difference is that the raw value is now valid only for the active wallet/mint lifecycle and is cleared whenever that lifecycle changes.


10. Preserve valid same-scope refresh behavior

Post-transaction balance refreshes triggered through lastSig continue to operate normally.

When the wallet and collateral mint remain unchanged:

  • walletBalanceScopeKey remains stable;
  • the input is not cleared solely because of a same-scope balance refresh;
  • the verified same-scope balance may remain visible while the refresh is pending;
  • the refreshed result replaces only the snapshot for the same ownership scope.

Regression tests

Added and extended:

app/__tests__/components/trade/DepositWithdrawCard.wallet-switch.test.tsx

Test 1: stale account-creation state is withheld while wallet B is pending

The test reproduces the new-account race:

  1. Render with wallet A.
  2. Resolve wallet A's collateral balance as 900 USDC.
  3. Confirm wallet A's balance is displayed.
  4. Confirm Create Trading Account is available.
  5. Switch the mocked active wallet to wallet B.
  6. Keep wallet B's balance request unresolved.
  7. Confirm a second balance request is made for wallet B's associated token account.
  8. Confirm wallet A's balance is no longer visible.
  9. Confirm Create Trading Account is no longer available.
  10. Confirm the stale action cannot call initUser.
  11. Confirm the 500 USDC starter-deposit cap is not derived from wallet A while wallet B remains unverified.

This test fails against the vulnerable implementation because wallet A's balance and account-creation action remain available after wallet B becomes active.


Test 2: the component updates after wallet B resolves

The second test verifies the successful replacement lifecycle:

  1. Wallet A resolves with 900 USDC.
  2. The active wallet changes to wallet B.
  3. Wallet B's request resolves with 25 USDC.
  4. The component updates to display wallet B's verified balance.
  5. The associated token-address lookup is confirmed to use wallet B.

This confirms the fix does not permanently suppress balance state. It withholds data only until the active wallet's balance has been verified.


Test 3: a Max-derived amount cannot cross wallet scopes

The third test covers the existing trading-account deposit path:

  1. Render an existing trading account under wallet A.

  2. Resolve wallet A's balance as 900 USDC.

  3. Click Max.

  4. Confirm the input displays 900.

  5. Switch the active wallet to wallet B.

  6. Keep wallet B's balance request pending.

  7. Confirm the previous input value is cleared.

  8. Confirm the Deposit button is disabled.

  9. Attempt to click Deposit.

  10. Confirm deposit() is not called.

  11. Resolve wallet B's balance as 25 USDC.

  12. Click Max again.

  13. Confirm the input displays 25.

  14. Submit the deposit.

  15. Confirm the submitted amount is exactly:

    25_000_000 native units
    
  16. Confirm the submission uses wallet B's portfolio identity.

This verifies that both the visible amount and the exact raw Max value are invalidated when the wallet scope changes.


Validation

Focused regression suite

pnpm --filter @percolator/app exec vitest run \
  __tests__/components/trade/DepositWithdrawCard.wallet-switch.test.tsx \
  --reporter=verbose

Result:

Test Files  1 passed
Tests       3 passed

The focused suite validates:

  • stale wallet A account-creation state is withheld;
  • wallet B's verified balance replaces wallet A's state;
  • a wallet A Max-derived deposit amount cannot be submitted under wallet B.

TypeScript validation

pnpm --filter @percolator/app exec tsc --noEmit

Result:

Passed

Production build

NEXT_PUBLIC_API_URL=http://127.0.0.1:3001 \
  pnpm --filter @percolator/app build

Result:

Passed

Repository whitespace validation

git diff --check

Result:

Passed

Formatting note

A full-file Prettier check reports existing formatting differences in:

app/components/trade/DepositWithdrawCard.tsx

The same warning is reproducible against the previous PR HEAD before the review follow-up patch.

The follow-up therefore intentionally preserves a minimal behavioral diff instead of reformatting the entire component and introducing unrelated formatting churn.

The regression-test file itself does not produce the reported formatting warning.


GitHub Actions

The latest PR head passes:

PR Check (Build & Fast Tests)
Test Suite

Files changed

app/components/trade/DepositWithdrawCard.tsx
app/__tests__/components/trade/DepositWithdrawCard.wallet-switch.test.tsx

User impact

After this change, switching wallets cannot temporarily expose or reuse the previous wallet's collateral balance inside the trade deposit/account-creation card.

For a replacement wallet:

  • the previous wallet's balance is immediately treated as invalid;
  • the previous wallet's input amount is cleared;
  • the previous wallet's exact Max value is cleared;
  • account creation remains unavailable while the replacement balance is unresolved;
  • existing-account deposits remain disabled while the replacement balance is unresolved;
  • deposit actions become available only after the replacement wallet's balance has been verified;
  • subsequent Max and deposit actions use only the replacement wallet's verified balance.

This prevents stale client-side transaction intent from crossing wallet or collateral-mint ownership boundaries.


Scope

This change is intentionally limited to the wallet-balance and deposit-input lifecycle inside DepositWithdrawCard.

It does not alter:

  • on-chain deposit or withdrawal instructions;
  • program-level authorization;
  • account ownership rules;
  • transaction signing;
  • token-account ownership validation;
  • collateral calculations;
  • position margin calculations;
  • withdrawal free-margin calculations; or
  • unrelated trade-page state.

The patch ensures that client-side balance data and balance-derived deposit values remain bound to the wallet and collateral mint that produced them.

@Bayyan16
Bayyan16 requested a review from dcccrypto as a code owner July 16, 2026 07:22
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

@Bayyan16 is attempting to deploy a commit to the Khubair Nasir's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: efb0205f-f252-42bd-b248-a6530c3585bf

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1db0a2105b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread app/components/trade/DepositWithdrawCard.tsx
@dcccrypto

Copy link
Copy Markdown
Owner

Independent verification — not an approval (QA/Security own that), just evidence for whoever reviews. Verdict: the tests genuinely bind to the fix.

Including a correction to my own first attempt, because it nearly produced a false accusation and the distinction is worth recording.

Attempt 1 — unfaithful, and I almost reported it as a defect

I removed only the render-time scope check:

: walletBalanceSnapshot?.scopeKey === walletBalanceScopeKey      : walletBalanceSnapshot

All 3 tests still passed. On its own that reads as "the tests don't catch the bug".

It isn't. This PR has two mechanisms, and I'd only disabled one:

  1. the effect at ~:137, which immediately replaces a snapshot whose scopeKey differs with a fresh {amount: null} placeholder
  2. the render-time scope check at :87 / :102

With (1) still live, the stale balance is cleared regardless, so the tests correctly stayed green. My mutation never reproduced #2429.

Attempt 2 — faithful

Neutralised both guards, so a foreign-scope snapshot actually survives a wallet switch:

both guards removed → 1 FAILED
  × withholds wallet A balance and prevents wallet A-derived account creation
    while wallet B balance is pending

That's the load-bearing assertion for the reported bug, and it fails exactly when the bug is present. The other two — "updates to wallet B balance after the replacement request resolves" and "clears a wallet A Max amount and blocks deposit while wallet B balance is unverified" — stay green under mutation, which is right; they cover the recovery path rather than the leak.

Two observations

1. This is tested at the component layer. That's the standard worth holding: the guard is verified where a user actually experiences it, so it survives a refactor that moves the internals. Contrast #2437 and #2442, where the coverage sits below the layer the bug lives at.

2. The render-time check is redundant for this scenario. The effect's invalidation is what the test binds to. That's fine as belt-and-braces on a balance that gates spending, but worth knowing: if someone later deletes :87/:102 as dead weight, these tests stay green.

No changes requested from me.

@Bayyan16

Copy link
Copy Markdown
Contributor Author

Thank you for the careful independent verification and for documenting the correction to the first mutation attempt.

Understood: the component-level regression genuinely binds to #2429, the effect-level invalidation is the load-bearing protection for this scenario, and the render-time scope check remains defense-in-depth.

Since no changes are requested, I’ll leave the implementation and regression coverage unchanged on the current head and await the formal QA/Security review.

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.

2 participants