Perps

[aldin4u]

Description

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

• New Features

  • Full Perps trading experience: asset browser, live charts/ticker/sparkline, trade form (market/limit, leverage, SL/TP), positions, trade history, trade signals, copy-trade tiles, mobile pages.
  • Agent wallet UX: create/import PIN‑protected agent, unlock/reveal keys, approve via connected wallet, copy/download credentials, session timeout.
  • Deposit/Withdraw bridge flows for Arbitrum with balance/network checks and transaction feedback.
  • Improved UI library and themed toasts for consistent styling and dialogs.

• Chores

  • Project moved to ESM; package.json and dependency updates (UI, form, charts, validation libs).

• Tests

  • Added scripts and test adjustments for remote orders API.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Actionable comments posted: 5

🤖 Fix all issues with AI agents

In `@package.json`:
- Around line 5-9: The project set "type": "module" so CommonJS config files
must be converted or renamed: update each Tailwind config (token-atlas,
key-wallet, the-exchange, pulse, leaderboard, pillarx-app, insights, deposit)
and config-overrides.js to use ESM syntax (replace module.exports/require with
export default and import) or rename them to .cjs (e.g., config-overrides.cjs,
tailwind.config.cjs) so Node can load them; ensure any webpack/terser usage in
config-overrides is converted to ESM imports if you choose ESM, and verify build
scripts still resolve the new filenames.

In `@src/apps/perps/components/TradeForm.tsx`:
- Around line 453-461: The early return when size <= 0 leaves the form stuck
loading because setIsSubmitting(true) isn't undone; update the handler
containing this size check (the function that calls setIsSubmitting) to call
setIsSubmitting(false) before the return in the size <= 0 branch (and any other
early-return branches), ensuring the isSubmitting state is reset and the submit
button stops loading; reference the setIsSubmitting/isSubmitting state and the
size <= 0 check in TradeForm.tsx when making the change.

In `@src/apps/perps/components/ui/carousel.tsx`:
- Around line 86-97: The keyboard handler handleKeyDown currently only checks
ArrowLeft/ArrowRight; update it to also handle ArrowUp/ArrowDown when the
carousel is vertical by checking the component's orientation (e.g., orientation
or isVertical) and mapping ArrowUp to scrollPrev and ArrowDown to scrollNext;
keep event.preventDefault() for those keys and preserve the existing dependency
array ([scrollPrev, scrollNext, orientation or isVertical]) so the callback
updates when orientation changes.

In `@src/apps/perps/components/WithdrawModal.tsx`:
- Around line 94-96: Replace the fragile hostname check in WithdrawModal (the
local isTestnet assignment) with the centralized isTestnet constant from
src/utils/blockchain.ts: remove the window.location.hostname-based logic, import
{ isTestnet } from 'src/utils/blockchain', and use that imported isTestnet
wherever the local variable was referenced (e.g., in WithdrawModal component
logic) so network detection is consistent across the app.

In `@src/apps/perps/lib/hyperliquid/order.ts`:
- Around line 1-2: The file re-exports roundToSzDecimals but never imports it
for local use, causing a ReferenceError when roundToSzDecimals(size, szDecimals)
is called; fix by adding a local import from './math' (e.g. import {
roundToSzDecimals } from './math';) so the symbol is available to the code that
calls it (you may keep the existing export line if external re-export is still
desired).

♻️ Duplicate comments (45)

src/apps/perps/lib/hyperliquid/bridge.ts (1)

101-106: EIP-712 domain configuration is correct.

The domain name is properly set to 'USDC' matching Circle's USDC contract specification. This was addressed in a previous review.

src/apps/perps/components/ui/form.tsx (1)

42-54: Validate FormItem context before using id.
itemContext.id is used without a guard, so using useFormField outside <FormItem> yields undefined-* IDs and breaks ARIA linkage. Add a check similar to the fieldContext guard.

🛠️ Proposed fix

 const useFormField = () => {
   const fieldContext = React.useContext(FormFieldContext);
   const itemContext = React.useContext(FormItemContext);
   const { getFieldState, formState } = useFormContext();

   if (!fieldContext.name) {
     throw new Error('useFormField should be used within <FormField>');
   }
+  if (!itemContext?.id) {
+    throw new Error('useFormField should be used within <FormItem>');
+  }

   const fieldState = getFieldState(fieldContext.name, formState);

   const { id } = itemContext;

src/apps/perps/components/mobile/MobilePositionsCard.tsx (1)

133-135: Fix leverage color conditional (both branches are green).

This makes the ternary redundant and likely miscolors SHORT positions.

🔧 Suggested fix

-                  <span className={`text-xs font-bold ${position.side === 'LONG' ? 'text-green-500' : 'text-green-500'}`}>
+                  <span className={`text-xs font-bold ${position.side === 'LONG' ? 'text-green-500' : 'text-red-500'}`}>
                     {position.leverage}x
                   </span>

src/apps/perps/components/AgentControls.tsx (3)

95-151: revealMode type mismatch + copy/download actions never run.

setRevealMode('copy'|'download') isn’t allowed by the current union, and unlock handling ignores copy/download, so those actions only unlock.

🔧 Suggested fix

-type RevealMode = 'copy' | 'download' | 'unlock';
+type RevealMode = 'copy' | 'download' | 'unlock' | 'reveal';

-const [revealMode, setRevealMode] = useState<'unlock' | 'reveal'>('unlock');
+const [revealMode, setRevealMode] = useState<RevealMode>('unlock');

 const handleUnlockClick = () => {
   setShowUnlockReveal(true);
-  setRevealMode('copy'); // Default mode, but actually we just want to unlock session
+  setRevealMode('unlock');
 };

 // In handleUnlockForReveal (apply in both unlock branches)
-        if (revealMode === 'reveal') {
+        if (revealMode === 'reveal') {
           setPrivateKeyModalState({ ... });
-        } else {
-          toast.success('Wallet unlocked');
-        }
+        } else if (revealMode === 'copy') {
+          await navigator.clipboard.writeText(unlocked.privateKey);
+          toast.success('Private key copied');
+        } else if (revealMode === 'download') {
+          downloadKeyFile(unlocked.accountAddress, unlocked.privateKey);
+          toast.success('Private key downloaded');
+        } else {
+          toast.success('Wallet unlocked');
+        }

Also applies to: 254-323, 679-695

---

634-641: Use viem.parseSignature and normalize v for wallet compatibility.

Manual slicing is brittle and can break with 0/1 v values or compact signatures.

🔧 Suggested fix

-import { createWalletClient, custom } from 'viem';
+import { createWalletClient, custom, parseSignature } from 'viem';

-      const payload = {
+      const { r, s, v, yParity } = parseSignature(signature);
+      const vNormalized =
+        v === undefined ? yParity + 27 : v <= 1 ? v + 27 : v;
+
+      const payload = {
         action: apiAction,
         nonce: actionConfig.nonce,
         signature: {
-          r: signature.slice(0, 66),
-          s: '0x' + signature.slice(66, 130),
-          v: parseInt(signature.slice(130, 132), 16),
+          r,
+          s,
+          v: vNormalized,
         },
         vaultAddress: null,
       };

---

780-829: Validate imported address matches the provided private key.

Right now a mismatched address/key pair can be stored, causing later cryptographic failures.

🔧 Suggested fix

      const account = privateKeyToAccount(formattedKey);

+     const entered = importAccountAddress.trim();
+     if (entered && account.address.toLowerCase() !== entered.toLowerCase()) {
+       toast.dismiss(loadingToast);
+       setValidationStatus('error');
+       setValidationData({
+         errorMessage: 'Private key does not match the account address.',
+       });
+       toast.error('Address/private key mismatch');
+       return;
+     }

src/apps/perps/lib/hyperliquid/client.ts (5)

2-10: Avoid import-time isTestnet evaluation if it reads localStorage.

If isTestnet touches window/localStorage at module load, this file will crash in SSR/Node/test environments. Consider deferring URL computation to runtime (inside functions).

#!/bin/bash
# Inspect isTestnet implementation for import-time localStorage access
rg -n "isTestnet" src/utils/blockchain.ts
sed -n '1,80p' src/utils/blockchain.ts

---

20-24: Add request timeouts to avoid indefinite hangs.

All API calls should include a timeout (or use a shared axios instance with defaults).

🔧 Suggested fix (apply to all requests)

     const response = await axios.post(EXCHANGE_URL, payload, {
       headers: {
         'Content-Type': 'application/json',
       },
+      timeout: 30000,
     });

---

122-129: Guard against non-array responses before calling .filter().

🔧 Suggested fix

-    const orders = response.data || [];
+    const orders: HyperliquidOrder[] = Array.isArray(response.data)
+      ? response.data
+      : [];

     if (symbol) {
       return orders.filter((order: any) => order.coin === symbol);
     }

     return orders;

-    return response.data || [];
+    return Array.isArray(response.data) ? response.data : [];

Also applies to: 152-153

---

196-199: Handle 0 and NaN correctly when parsing mark price.

🔧 Suggested fix

-    if (response.data && typeof response.data === 'object') {
-      const markPrice = response.data[symbol];
-      return markPrice ? parseFloat(markPrice) : null;
+    if (response.data && typeof response.data === 'object' && response.data !== null) {
+      const raw = (response.data as Record<string, unknown>)[symbol];
+      const px = typeof raw === 'string' || typeof raw === 'number' ? Number(raw) : NaN;
+      return Number.isFinite(px) ? px : null;
     }

---

222-228: Use nullish coalescing and validate universe array.

🔧 Suggested fix

-    if (response.data && response.data.universe) {
-      return response.data.universe.map((asset: any, index: number) => ({
+    const universe = Array.isArray(response.data?.universe) ? response.data.universe : [];
+    if (universe.length) {
+      return universe.map((asset: any, index: number) => ({
         id: index,
         symbol: asset.name,
-        szDecimals: asset.szDecimals || 3,
-        maxLeverage: asset.maxLeverage || 50,
+        szDecimals: asset.szDecimals ?? 3,
+        maxLeverage: asset.maxLeverage ?? 50,
       }));
     }

src/apps/perps/lib/hyperliquid/keystore.ts (5)

9-14: Import-time side effect causes SSR crash risk.

initializeSessionManager is called at module load (line 10), which starts timers immediately. This will fail during SSR, tests, or when the module is imported in non-browser contexts.

The past review suggested lazy initialization. Consider wrapping in a function that checks typeof window !== 'undefined' and calling it from public APIs that need the session manager.

---

91-96: Validate decrypted key before casting to Hex.

The decrypted string is cast directly to Hex (line 95) without validating the format. If decryption returns malformed data, this could cause downstream issues.

Suggested validation helper

function assertHexPrivateKey(value: string): asserts value is Hex {
  if (!/^0x[a-fA-F0-9]{64}$/.test(value)) {
    throw new Error('Decrypted key is not a valid 32-byte hex private key');
  }
}

// Usage:
const privateKey = await decryptWithPin(encryptedData, pin);
assertHexPrivateKey(privateKey);
cachedPrivateKey = privateKey;

---

371-384: Misleading deprecation comment.

The JSDoc states "This function stores keys in plaintext" (line 372), but the implementation delegates to storeAgentWalletEncrypted (line 383), which uses encrypted storage. Update the comment to reflect current behavior.

Corrected comment

 /**
- * `@deprecated` Use storeAgentWalletEncrypted instead. This function stores keys in plaintext.
+ * `@deprecated` Use storeAgentWalletEncrypted directly. This wrapper now delegates to encrypted storage.
  */

---

55-57: Cache populated before storage confirmation.

cachedPrivateKey is set (line 56) before all localStorage.setItem calls complete. If a later storage call fails (e.g., quota exceeded), the cache will contain a key that wasn't persisted, leading to inconsistent state.

Move cache population after all storage operations succeed, or wrap storage in try/catch that rolls back the cache on failure.

---

386-402: Cache mismatch vulnerability: cachedPrivateKey is not keyed by masterAddress.

If a user unlocks wallet A, then calls getAgentWallet(masterB), the function returns cachedPrivateKey (which belongs to A) as long as masterB has an address stored. This returns mismatched address/privateKey pairs.

Suggested fix: Key cache by master address

- let cachedPrivateKey: Hex | null = null;
+ const cachedAgentKeyByMaster = new Map<string, Hex>();
+ const norm = (addr: string) => addr.toLowerCase();

// In storeAgentWalletEncrypted:
- cachedPrivateKey = privateKey;
+ cachedAgentKeyByMaster.set(norm(masterAddress), privateKey);

// In getAgentWallet:
- if (cachedPrivateKey) {
+ const cached = cachedAgentKeyByMaster.get(norm(masterAddress));
+ if (cached) {
    // ...
-   return { address, privateKey: cachedPrivateKey, approved, builderApproved };
+   return { address, privateKey: cached, approved, builderApproved };
  }

src/apps/perps/lib/hyperliquid/signing.ts (5)

469-483: Duplicate generateAgentWallet implementation.

This function duplicates the one in keystore.ts (lines 16-23). The keystore version uses generatePrivateKey() from viem/accounts, while this version manually generates random bytes.

Suggested consolidation

- export function generateAgentWallet(): { address: string; privateKey: Hex } {
-   // Generate random private key
-   const randomBytes = new Uint8Array(32);
-   crypto.getRandomValues(randomBytes);
-   // ...
- }
+ // Re-export from keystore for single source of truth
+ export { generateAgentWallet } from './keystore';

---

86-91: Add v normalization for consistent signature format.

The parsed v value (line 89) is used as-is. Depending on the signer, v may be returned as 0/1 or 27/28. For consistency when sending to Hyperliquid API, normalize:

const v = parseInt(signature.slice(130, 132), 16);
const normalizedV = v < 2 ? v + 27 : v; // Ensure 27/28 format

This applies to all signature parsing locations in the file.

---

38-76: actionPayload and actionHash are computed but never used.

The actionPayload (lines 39-43) and actionHash (lines 66-76) variables are computed but never referenced. More critically, the signature only covers a static phantomAgent with hardcoded values, not the actual action and nonce parameters. This means the signature doesn't prove which action was authorized.

Per Hyperliquid's L1 signing protocol, the action should be msgpack-serialized with the nonce, then keccak256-hashed to derive the connectionId before signing.

---

401-433: action and nonce parameters are unused in signAgentAction.

The function accepts action: HyperliquidAction and nonce: number but only signs a hardcoded phantom agent message. The actual action data is never incorporated into the signature.

Either remove the unused parameters or incorporate them into the signed message to prevent replay attacks.

---

46-63: Inconsistent connectionId derivation between signing functions.

signUserAction uses hashTypedData() (lines 48-62) while signAgentAction uses keccak256(toHex(JSON.stringify(...))) (lines 410-418). These produce different hashes for the same logical input, which could break signature verification across functions.

Determine which format Hyperliquid expects and apply consistently.

Also applies to: 408-419

src/apps/perps/components/BalanceCard.tsx (2)

29-44: Guard against NaN and division by zero in PnL calculations.

parseFloat can return NaN for malformed strings, and accountEquity - totalPnl can be zero or negative, causing Infinity or NaN in pnlPercent.

Suggested defensive fix

- const accountEquity = parseFloat(
-   userState.marginSummary?.accountValue || '0'
- );
+ const accountEquityRaw = Number(userState.marginSummary?.accountValue ?? 0);
+ const accountEquity = Number.isFinite(accountEquityRaw) ? accountEquityRaw : 0;

  const totalPnl = useMemo(() => {
    if (!userState.assetPositions) return 0;
    return userState.assetPositions.reduce((sum, pos) => {
-     return sum + parseFloat(pos.position.unrealizedPnl || '0');
+     const pnl = Number(pos.position.unrealizedPnl ?? 0);
+     return sum + (Number.isFinite(pnl) ? pnl : 0);
    }, 0);
  }, [userState.assetPositions]);

- const pnlPercent =
-   accountEquity > 0
-     ? ((totalPnl / (accountEquity - totalPnl)) * 100).toFixed(2)
-     : '0.00';
+ const pnlBase = accountEquity - totalPnl;
+ const pnlPercent =
+   pnlBase > 0 && Number.isFinite(totalPnl)
+     ? ((totalPnl / pnlBase) * 100).toFixed(2)
+     : '0.00';

---

62-71: Add aria-label for icon-only buttons.

The mobile action buttons (Plus, Minus icons) and refresh button are icon-only without accessible names, making them inaccessible to screen readers.

Suggested fix

  <Button
    size="icon"
    variant="outline"
    className="h-8 w-8"
    disabled={isImported}
+   aria-label="Deposit"
  >
    <Plus className="h-4 w-4" />
  </Button>
  
  // Similarly for Minus button: aria-label="Withdraw"
  // For RefreshCw button: aria-label="Refresh balance"

Also applies to: 80-91, 93-105

src/apps/perps/components/PositionsCard.tsx (10)

966-985: Dead code in TP/SL fallback branches.

The else branches at lines 970-983 and 1070-1083 are unreachable. Since hasOrders = tps.length > 0 || sls.length > 0, when hasOrders is false, tps.length === 0 && sls.length === 0 is always true.

🧹 Simplified logic

                             ) : (
-                              <span className="flex gap-1 justify-end">
-                                {tps.length === 0 && sls.length === 0 ? (
-                                  <span>-</span>
-                                ) : (
-                                  <>
-                                    {tps.length > 0 && (
-                                      <span className="text-green-500">
-                                        ${formatPrice(tps[0].price)}
-                                      </span>
-                                    )}
-                                    {sls.length > 0 && (
-                                      <span className="text-red-500">
-                                        ${formatPrice(sls[0].price)}
-                                      </span>
-                                    )}
-                                  </>
-                                )}
-                              </span>
+                              <span>-</span>
                             )}

Apply the same simplification to the desktop version at lines 1066-1085.

Also applies to: 1066-1085

---

215-218: Add user feedback for data fetch errors.

When fetching positions or orders fails, the error is only logged to console. In a trading application, users should be informed when position data cannot be loaded.

🔔 Suggested fix

     } catch (error) {
       console.error('Error fetching data:', error);
+      toast.error('Failed to load positions', {
+        description: error instanceof Error ? error.message : 'Please try refreshing'
+      });
     } finally {
       setIsLoading(false);
     }

---

183-195: Fallback asset info uses hardcoded defaults that may not match actual specifications.

When asset metadata is missing, using szDecimals: 4 and maxLeverage: 50 could lead to incorrect decimal precision or wrong leverage limits. Consider showing a warning indicator in the UI when using fallback values.

---

90-90: Missing UserState type import causes TypeScript compilation error.

Line 90 uses useState<UserState | null> but UserState is not imported. Add it to the existing type import.

🐛 Proposed fix

 import type {
   HyperliquidPosition,
   HyperliquidOrder,
   UniverseAsset,
   EnrichedPosition,
+  UserState,
 } from '../lib/hyperliquid/types';

---

634-644: Loading state check order is inverted — skeletons never display during initial load.

The condition checks positions.length === 0 before isLoading. On initial render, both are true, so "No open positions" displays instead of skeleton loaders.

🐛 Proposed fix

-             {positions.length === 0 ? (
+             {isLoading ? (
+               <div className="space-y-3">
+                 {[1, 2, 3].map((i) => (
+                   <Skeleton key={i} className="h-24 w-full rounded-lg" />
+                 ))}
+               </div>
+             ) : positions.length === 0 ? (
                <p className="text-sm text-muted-foreground text-center py-4 bg-muted/20 rounded-lg border border-dashed">
                  No open positions
                </p>
-             ) : isLoading ? (
-               <div className="space-y-3">
-                 {[1, 2, 3].map((i) => (
-                   <Skeleton key={i} className="h-24 w-full rounded-lg" />
-                 ))}
-               </div>
              ) : expandedPositionIndex !== null &&

---

1113-1122: Same loading state issue for Open Orders section.

Apply the same fix — check isLoading before openOrders.length === 0.

🐛 Proposed fix

-             {openOrders.length === 0 ? (
+             {isLoading ? (
+               <div className="space-y-2">
+                 {[1, 2].map((i) => (
+                   <Skeleton key={i} className="h-12 w-full rounded-lg" />
+                 ))}
+               </div>
+             ) : openOrders.length === 0 ? (
                <p className="text-sm text-muted-foreground text-center py-4 bg-muted/20 rounded-lg border border-dashed">
                  No open orders
                </p>
-             ) : isLoading ? (
-               <div className="space-y-2">
-                 {[1, 2].map((i) => (
-                   <Skeleton key={i} className="h-12 w-full rounded-lg" />
-                 ))}
-               </div>
              ) : (

---

220-220: Circular dependency: internalUserState in fetchData dependency array.

fetchData sets internalUserState at line 128, but internalUserState is in the dependency array at line 220. This creates a circular dependency that could cause infinite re-renders.

🐛 Proposed fix

-  }, [masterAddress, userState, directPositions, externalOpenOrders, internalUserState]);
+  }, [masterAddress, userState, directPositions, externalOpenOrders]);

internalUserState is only used as a fallback and should not trigger data fetching.

---

325-328: Hardcoded .toFixed(6) ignores asset's actual szDecimals.

Some assets may have different precision requirements (e.g., BTC uses 4, smaller tokens may use 0-2). The position is enriched with coinInfo.szDecimals at line 200 — use it here.

🐛 Proposed fix

       const totalSize = Math.abs(parseFloat(positionToClose.szi));
       const sizeToClose = totalSize * (closePercentage / 100);
-      const sizeStr = sizeToClose.toFixed(6);
+      const decimals = positionToClose.coinInfo?.szDecimals ?? 6;
+      const sizeStr = sizeToClose.toFixed(decimals);
       const size = parseFloat(sizeStr);

---

509-546: Missing await on async function calls may cause race conditions.

handleExecuteClose and handleCancelOrder are async but called without await at lines 518, 520, 534, 536. This means setPendingAction(null) and return true execute before the action completes.

🐛 Proposed fix

   const handleUnlock = async (pin: string): Promise<boolean> => {
     try {
       if (isImportedAccountEncrypted()) {
         const unlocked = await unlockImportedAccount(pin);
         if (unlocked) {
           setShowUnlockModal(false);
+          setPendingAction(null);
           if (pendingAction?.type === 'close') {
-            handleExecuteClose();
+            await handleExecuteClose();
           } else if (pendingAction?.type === 'cancel' && pendingAction.data) {
-            handleCancelOrder(pendingAction.data);
+            await handleCancelOrder(pendingAction.data);
           }
-          setPendingAction(null);
           return true;
         }
       }
       // Apply same pattern to the agent wallet branch

---

343-348: Add response validation before showing success toast.

The success message appears immediately after placeMarketOrderAgent() completes, with no validation of the response. If the SDK returns a failure status rather than throwing, users will see "Order submitted" even though it was rejected.

🔧 Verify SDK response behavior

Check if placeMarketOrderAgent throws on failure or returns a response object with status. If it returns a response, validate it before showing success:

const result = await placeMarketOrderAgent(...);
if (result?.status === 'error' || result?.error) {
  throw new Error(result.error || 'Order rejected');
}
toast.success('Order submitted');

#!/bin/bash
# Check placeMarketOrderAgent implementation for response handling
rg -n "placeMarketOrderAgent" src/apps/perps/lib/hyperliquid/sdk.ts -A 20

src/apps/perps/index.tsx (4)

163-165: Use Promise.allSettled to prevent silent failures.

If either loadBalance() or fetchImportedAccount() fails, the other result is dropped. Use allSettled to ensure both complete independently.

♻️ Proposed fix

   const handleRefresh = async () => {
-    await Promise.all([loadBalance(), fetchImportedAccount()]);
+    await Promise.allSettled([loadBalance(), fetchImportedAccount()]);
   };

---

331-331: No-op render branch for disconnected state.

This line renders null but does nothing useful. Consider providing an empty state UI prompting users to connect a wallet or import an account.

💡 Suggested improvement

-        {!address && !agentAddress && !isLoadingAgent && null}
+        {!address && !agentAddress && !isLoadingAgent && (
+          <div className="text-center py-12">
+            <p className="text-muted-foreground">
+              Connect your wallet or import an account to start trading
+            </p>
+          </div>
+        )}

---

55-58: Remove debug console.log statement.

This will log addresses to the console in production. Either remove it or wrap in a development check.

🧹 Remove debug log

-  const handleSetAgentAddress = (addr: string | null) => {
-    console.log('DEBUG: index.tsx setAgentAddress called with:', addr);
-    setAgentAddress(addr);
-  };
+  const handleSetAgentAddress = (addr: string | null) => setAgentAddress(addr);

---

277-279: Incorrect agent vs master openOrders fallback logic.

When agentAddress is active but agentOpenOrders is empty, the current logic falls back to openOrders from the main address. This shows incorrect account data. Use agentAddress presence to determine which orders to show, not array length.

🐛 Proposed fix

              <SparklineChart
                selectedAsset={selectedAsset}
                userState={agentUserState || userState}
-               openOrders={
-                 agentOpenOrders.length > 0 ? agentOpenOrders : openOrders
-               }
+               openOrders={agentAddress ? agentOpenOrders : openOrders}
                accountAddress={agentAddress || address}
              />

              <PositionsCard
                masterAddress={agentAddress || address || ''}
                onPositionClick={handlePositionClick}
                onRefresh={handleRefresh}
                userState={agentUserState || userState}
-               openOrders={
-                 agentOpenOrders.length > 0 ? agentOpenOrders : openOrders
-               }
+               openOrders={agentAddress ? agentOpenOrders : openOrders}
              />

Also applies to: 304-306

src/apps/perps/components/TradeForm.tsx (8)

74-75: Missing EnhancedAsset type import causes TypeScript compilation error.

EnhancedAsset is referenced in TradeFormProps but not imported. This will fail TypeScript compilation.

🐛 Proposed fix

-import type { AssetInfo, UserState } from '../lib/hyperliquid/types';
+import type { AssetInfo, EnhancedAsset, UserState } from '../lib/hyperliquid/types';

---

67-70: Dead code: Schema .refine() always returns true.

The refinement function contains only a comment and return true, making it a no-op. Either implement validation or remove this dead code.

♻️ Remove dead code

   .object({
     side: z.enum(['long', 'short']),
     // ... other fields
-  })
-  .refine((data) => {
-    // Basic validation logic
-    return true;
-  });
+  });

---

428-431: Duplicate useBuilderFee variable declaration.

useBuilderFee is declared twice:

• Line 431: const useBuilderFee = signingAddress === masterAddress;
• Line 500: const useBuilderFee = !imported && (signingAddress === masterAddress);

The second declaration shadows the first, making Line 431 dead code. Based on the comments at lines 496-499, the second definition with the !imported check is the intended logic.

🐛 Proposed fix

-    // Only apply builder fee if the signing address matches the connected wallet (Master)
-    // If using an imported account that is different from the connected wallet, we cannot approve the builder fee
-    // because the connected wallet (Master) cannot sign for the imported account.
-    const useBuilderFee = signingAddress === masterAddress;
-
     setIsSubmitting(true);

Also applies to: 500-500

---

267-280: Prefilled TP ratios can sum to less than 100%, causing validation failure.

Using Math.floor(100 / tps.length) drops the remainder, so the total ratio won't equal 100% and will fail the submit-time check at lines 558-566. Use the existing getDistributedRatios helper for consistency.

🐛 Proposed fix

       if (prefilledData.takeProfits) {
         const tps = prefilledData.takeProfits
           .split(',')
           .map((s) => parseFloat(s.trim()))
           .filter((n) => !isNaN(n));
-        const ratio = tps.length > 0 ? Math.floor(100 / tps.length) : 0;
+        const ratios = getDistributedRatios(tps.length);
         setValue(
           'takeProfits',
-          tps.map((p) => ({
+          tps.map((p, i) => ({
             price: p,
-            ratio: ratio,
+            ratio: ratios[i] ?? 0,
             distance: calculateDistance(p, ep),
           }))
         );
       }

---

147-168: Use useFieldArray helpers (replace) instead of setValue to avoid field ID desync.

Calling setValue('takeProfits', ...) bypasses useFieldArray internals and can desync tpFields IDs, breaking field tracking, dirty/touched states, and validation behavior.

♻️ Proposed fix

 const {
   fields: tpFields,
   append: appendTp,
   remove: removeTp,
+  replace: replaceTp,
 } = useFieldArray({
   control,
   name: 'takeProfits',
 });

 const handleAddTp = () => {
   // ... ratio calculation ...
-  setValue('takeProfits', updatedValues);
+  replaceTp(updatedValues);
 };

 const handleRemoveTp = (index: number) => {
   // ... ratio calculation ...
-  setValue('takeProfits', updatedValues);
+  replaceTp(updatedValues);
   // ... else branch ...
-  setValue('takeProfits', []);
+  replaceTp([]);
 };

Also applies to: 171-186

---

1208-1219: Debug console.log left in production code.

This IIFE logs button state on every render and should be removed or wrapped in a development-only check before deployment.

🧹 Remove debug code

-        {/* Debug logging */}
-        {(() => {
-          console.log('[TradeForm] Button state:', {
-            isSubmitting,
-            'errors.amountUSD': errors.amountUSD,
-            isBelowMinimum,
-            minUSD,
-            amountUSD,
-            disabled: isSubmitting || !!errors.amountUSD || isBelowMinimum,
-          });
-          return null;
-        })()}

---

635-680: Unreachable auto-approval code for imported accounts.

The builder fee auto-approval at lines 644-670 can never execute. The condition at line 636 checks useBuilderFee, which is defined at line 500 as !imported && (signingAddress === masterAddress). Since we're inside a block where useBuilderFee is true, !imported must be true, meaning imported is false. Therefore, if (imported && ...) at line 644 is always false.

Restructure so the builder-fee error is detected first, then branch based on whether an imported account exists.

🔧 Suggested restructure

      // Handle "Builder fee has not been approved"
      if (
-       useBuilderFee &&
        (error?.message?.includes('Builder fee has not been approved') ||
          error?.response?.data?.includes('Builder fee has not been approved'))
      ) {
        const imported = getImportedAccount();
        if (imported && imported.accountAddress.toLowerCase() === masterAddress?.toLowerCase()) {
+         // Imported account matches master - can auto-approve
          try {
            // ... approval logic ...
          } catch { /* ... */ }
        } else {
+         // Using agent or non-matching imported account
          toast.error('Builder Fee Not Approved', {
            id: toastId,
            description: 'Please go to Settings > Perps Account and approve PillarX.',
            duration: 5000
          });
          return;
        }
      }

---

406-412: Early return missing toast ID — loading toast won't be dismissed.

When agent exists but builderApproved is false, toast.error() is called without { id: toastId }, leaving the "Placing order…" loading toast stuck.

🐛 Proposed fix

       if (!agent.builderApproved) {
         toast.error('PillarX Approval Required', {
+          id: toastId,
           description: 'Please go to Settings > Perps Account and approve PillarX to start trading.',
           duration: 5000,
         });
         setIsSubmitting(false);
         return;
       }

🧹 Nitpick comments (12)

src/apps/perps/components/mobile/MobileMarketsList.tsx (4)

3-3: Remove unused Input import.

Input is imported but never used in this component.

 import { Search, ChevronDown, ChevronRight } from 'lucide-react';
 import { Button } from '../ui/button';
-import { Input } from '../ui/input';
 import { TokenIcon } from '../TokenIcon';

---

6-14: Consider removing unused coinIcon field.

The coinIcon field is defined in the Market interface but never used. The TokenIcon component on line 64 uses market.coin directly to fetch icons. If coinIcon is not needed, consider removing it to avoid confusion.

---

62-63: Remove duplicate comment.

There are two identical {/* Coin Icon */} comments on consecutive lines.

             {/* Left: Icon and Info */}
             <div className="flex items-center gap-3">
               {/* Coin Icon */}
-              {/* Coin Icon */}
               <TokenIcon symbol={market.coin} size={48} />

---

80-87: Consider using the same field for color logic and display.

The color is determined by market.change.startsWith('-') (line 81), but the displayed value is market.changePercent (line 86). If these fields ever become inconsistent (e.g., change is positive but changePercent shows negative), the color would be misleading.

Consider using the same field for both:

               <p
-                className={`text-sm font-medium ${market.change.startsWith('-')
+                className={`text-sm font-medium ${market.changePercent.startsWith('-')
                   ? 'text-red-500'
                   : 'text-green-500'
                   }`}
               >
                 {market.changePercent}
               </p>

Alternatively, derive both values from a single numeric source to guarantee consistency.

src/apps/perps/lib/hyperliquid/bridge.ts (2)

69-82: Consider removing or documenting the unused recipient parameter.

The recipient parameter is accepted but never used since the bridge always credits msg.sender. This can mislead callers into thinking they can deposit to another address. Consider either:

1. Removing the parameter if compatibility is no longer needed.
2. Adding a JSDoc comment to clarify the limitation.

♻️ Option: Add JSDoc documentation

+/**
+ * Deposits USDC to the Hyperliquid bridge.
+ * `@param` recipient - Unused; deposits always credit msg.sender
+ * `@param` amount - Amount of USDC to deposit (in human-readable units)
+ * `@param` signer - Ethers signer for the transaction
+ */
 export async function bridgeUSDC(
   recipient: string,
   amount: string,
   signer: ethers.Signer
 ): Promise<ethers.ContractTransaction> {

---

126-128: Consider adding type safety for _signTypedData.

The cast to any bypasses TypeScript's type checking. In ethers v5, _signTypedData exists on JsonRpcSigner but may not be available on all signer implementations. Consider adding a runtime check or using a type guard.

♻️ Proposed improvement for type safety

   // Sign EIP-2612 Permit
-  const sigHex = await (signer as any)._signTypedData(domain, types, message);
+  if (typeof (signer as any)._signTypedData !== 'function') {
+    throw new Error('Signer does not support EIP-712 typed data signing');
+  }
+  const sigHex = await (signer as any)._signTypedData(domain, types, message);
   const sig = ethers.utils.splitSignature(sigHex);

src/apps/perps/components/mobile/MobilePositionsCard.tsx (1)

107-110: Prefer stable keys for list items.

Index keys can cause state reuse issues if positions/orders reorder. Use a stable identifier (coin+side, order id, etc.) instead.

🔧 Suggested tweak (positions list)

-        {positions.map((position, index) => (
+        {positions.map((position) => (
           <div
-            key={index}
+            key={`${position.coin}-${position.side}`}
             className="border-t border-gray-100 pt-3 cursor-pointer hover:bg-gray-50 transition-colors"
             onClick={() => onPositionClick?.(position.coin)}
           >

Also applies to: 230-235

src/apps/perps/components/BalanceCard.tsx (1)

14-14: Type walletClient properly instead of any.

The walletClient prop is typed as any, losing type safety. Use the proper type:

- walletClient: any;
+ walletClient: WalletClient | null;

And add the import:

import type { WalletClient } from 'viem';

src/apps/perps/components/TradeForm.tsx (1)

41-70: Schema allows amountUSD minimum of 10 but trades can fail for smaller positions.

The Zod schema enforces min(10) for amountUSD, but the actual minimum depends on the asset's szDecimals, leverage, and price. Consider documenting this or adjusting the error message at line 48 to be clearer that additional constraints apply.

src/apps/perps/components/PositionsCard.tsx (1)

64-64: Avoid any type for userState prop.

The comment acknowledges the issue but any bypasses TypeScript safety. Since UserState should be imported anyway (for line 90), use it here too.

♻️ Proposed fix

 interface PositionsCardProps {
   masterAddress: string;
   onPositionClick?: (symbol: string) => void;
   onRefresh?: () => void;
-  userState?: any; // Using any for now to avoid strict type issues with passed state, or  userState?: UserState;
+  userState?: UserState;
   assetPositions?: HyperliquidPosition[];
   openOrders?: HyperliquidOrder[];
 }

src/apps/perps/index.tsx (2)

111-111: Remove debug console.log statement.

🧹 Remove or replace with proper logging

           if (urlAsset) {
             setSelectedAsset(urlAsset);
-            console.log(`[URL] Auto-selected ${urlAsset.symbol} from URL`);
           }

---

43-52: Initial selectedAsset with hardcoded BTC may flash before URL asset loads.

The component initializes with a hardcoded BTC asset, but if the URL specifies a different symbol (e.g., /perps/eth), there will be a brief flash of BTC before ETH is selected. Consider initializing as null and showing a loading state until assets are loaded.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Look for CommonJS patterns in config files after switching to ESM
rg -n "module\.exports|require\(" -g "*config*.{js,ts,cjs,mjs}" -g ".eslintrc.*" .

Repository: pillarwallet/x

Length of output: 1034

---

🏁 Script executed:

#!/bin/bash
# Find all config files in the repository
fd -e "js" -e "ts" -e "cjs" -e "mjs" | rg "^[^/]*\.(config\.)?(js|ts|cjs|mjs)$|^\..*rc.*$" | head -30

Repository: pillarwallet/x

Length of output: 150

---

🏁 Script executed:

#!/bin/bash
# Check the package.json to confirm the "type": "module" setting
cat package.json | head -20

Repository: pillarwallet/x

Length of output: 935

---

Migrate CommonJS config files to ESM or rename to .cjs after adding "type": "module".

Multiple config files use CommonJS syntax that will break with ESM:

• 8 Tailwind config files across apps (token-atlas, key-wallet, the-exchange, pulse, leaderboard, pillarx-app, insights, deposit) use module.exports and require()
• config-overrides.js uses require() for webpack and terser
• These must be converted to ESM imports/exports or renamed to .cjs extension to remain loadable

🤖 Prompt for AI Agents

In `@package.json` around lines 5 - 9, The project set "type": "module" so
CommonJS config files must be converted or renamed: update each Tailwind config
(token-atlas, key-wallet, the-exchange, pulse, leaderboard, pillarx-app,
insights, deposit) and config-overrides.js to use ESM syntax (replace
module.exports/require with export default and import) or rename them to .cjs
(e.g., config-overrides.cjs, tailwind.config.cjs) so Node can load them; ensure
any webpack/terser usage in config-overrides is converted to ESM imports if you
choose ESM, and verify build scripts still resolve the new filenames.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Early return after size check doesn't reset isSubmitting state.

When size is zero due to rounding, the function returns without calling setIsSubmitting(false), leaving the button in a loading state.

🐛 Proposed fix

      if (size <= 0) {
        const minSize = Math.pow(10, -selectedAsset.szDecimals);
        const minRequired = (minSize * entryPrice) / data.leverage;
        toast.error(`Amount too small for ${selectedAsset.symbol}`, {
          id: toastId,
          description: `Minimum required: $${minRequired.toFixed(2)} at ${data.leverage}x leverage`,
        });
+       setIsSubmitting(false);
        return;
      }

🤖 Prompt for AI Agents

In `@src/apps/perps/components/TradeForm.tsx` around lines 453 - 461, The early
return when size <= 0 leaves the form stuck loading because
setIsSubmitting(true) isn't undone; update the handler containing this size
check (the function that calls setIsSubmitting) to call setIsSubmitting(false)
before the return in the size <= 0 branch (and any other early-return branches),
ensuring the isSubmitting state is reset and the submit button stops loading;
reference the setIsSubmitting/isSubmitting state and the size <= 0 check in
TradeForm.tsx when making the change.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/apps/perps/components/mobile/MobileHeader.tsx`:
- Around line 8-14: The icon-only Button instances in MobileHeader.tsx (the
Button wrapping the User icon and the similar button at the other instance) lack
accessible names; update each icon-only Button (the Button components containing
<User ... />) to include an accessible label by adding an appropriate aria-label
(e.g., aria-label="User menu" or aria-label="Open profile") or by providing
visually hidden text inside the Button using a VisuallyHidden component,
ensuring the label conveys the button's purpose for screen readers.

♻️ Duplicate comments (30)

src/apps/perps/components/mobile/MobileHeader.tsx (1)

8-14: Profile/History buttons still have no handlers.
Same concern as prior review: wire onClick handlers or mark them explicitly non-interactive.

Also applies to: 25-31

src/apps/perps/lib/hyperliquid/signing.ts (4)

469-482: Deduplicate generateAgentWallet.

This duplicates src/apps/perps/lib/hyperliquid/keystore.ts and risks divergence. Re-export the keystore implementation instead.

♻️ Suggested refactor

-export function generateAgentWallet(): { address: string; privateKey: Hex } {
-  // Generate random private key
-  const randomBytes = new Uint8Array(32);
-  crypto.getRandomValues(randomBytes);
-  const privateKey = ('0x' +
-    Array.from(randomBytes)
-      .map((b) => b.toString(16).padStart(2, '0'))
-      .join('')) as Hex;
-
-  const account = privateKeyToAccount(privateKey);
-  return { address: account.address, privateKey };
-}
+export { generateAgentWallet } from './keystore';

---

38-76: Signature not bound to action/nonce (replay risk).

actionPayload/actionHash are computed but the signature is over a static phantom agent with a zero connectionId, so the signature can be replayed for different actions. Bind the signature to the actual action + nonce (and vaultAddress) by hashing the serialized action (or using the official SDK helper) to derive connectionId, then sign that. citeturn2search5turn2search1

🔧 Suggested direction

-  const phantomAgent = {
-    source: 'a',
-    connectionId: hashTypedData({ ...zero... }),
-  };
+  const connectionId = createL1ActionHash({ action, nonce, vaultAddress: null });
+  const phantomAgent = { source: 'a', connectionId };
-  const actionHash = hashTypedData({ ... });

---

401-425: signAgentAction ignores action/nonce and signs a static payload.

This makes agent signatures reusable across actions and inconsistent with the user-signing path. Build the phantom agent connectionId from the provided action + nonce and use the same derivation as the user signing flow so verify/sign paths match. citeturn2search5turn2search1

---

86-90: Normalize v (0/1 vs 27/28) before returning signatures.

Some signers return v as 0/1. Normalize (or use viem.parseSignature) so downstream verification/API submission is consistent. citeturn2search5turn2search1

✅ Minimal normalization

-  const v = parseInt(signature.slice(130, 132), 16);
+  let v = parseInt(signature.slice(130, 132), 16);
+  if (v < 2) v += 27;

src/apps/perps/index.tsx (3)

133-140: Redundant dynamic import of getImportedAccount.

You already have a static import; remove the dynamic import to avoid duplication.

---

166-168: handleRefresh should tolerate partial failures.

Promise.all drops results if either call fails. Consider Promise.allSettled to refresh what you can.

---

277-283: Agent openOrders fallback shows master orders (wrong account).

When agentAddress is present but has no orders, the UI shows master orders. Use the agent’s list whenever agentAddress is truthy (even if empty).

Also applies to: 302-309

src/apps/perps/components/AgentControls.tsx (5)

88-143: revealMode typing is inconsistent with actual values (compile error).

setRevealMode('copy'|'download') conflicts with the state type ('unlock'|'reveal'). Align the type and state to the real union values.

✅ Fix typing

-type RevealMode = 'copy' | 'download' | 'unlock';
+type RevealMode = 'copy' | 'download' | 'unlock' | 'reveal';

-const [revealMode, setRevealMode] = useState<'unlock' | 'reveal'>('unlock');
+const [revealMode, setRevealMode] = useState<RevealMode>('unlock');

---

247-315: Unlock flow never performs copy/download actions.

When revealMode is 'copy' or 'download', the unlock handler only toasts “Wallet unlocked” and never executes the intended action.

🔧 Suggested fix

-        if (revealMode === 'reveal') {
+        if (revealMode === 'reveal') {
           setPrivateKeyModalState({ ... });
+        } else if (revealMode === 'copy') {
+          await navigator.clipboard.writeText(unlocked.privateKey);
+          toast.success('Private key copied');
+        } else if (revealMode === 'download') {
+          downloadKeyFile(unlocked.accountAddress, unlocked.privateKey);
+          toast.success('Private key downloaded');
         } else {
           toast.success('Wallet unlocked');
         }

---

783-818: Address/private-key mismatch isn’t validated (critical).

You derive account.address from the private key but never verify it matches the user-entered importAccountAddress. This can store mismatched credentials and break trading.

✅ Add match check

-      const account = privateKeyToAccount(formattedKey);
+      const account = privateKeyToAccount(formattedKey);
+      if (
+        account.address.toLowerCase() !== importAccountAddress.trim().toLowerCase()
+      ) {
+        toast.dismiss(loadingToast);
+        setValidationStatus('error');
+        setValidationData({ errorMessage: 'Private key does not match address.' });
+        toast.error('Address/private key mismatch');
+        return;
+      }

---

637-644: Normalize v before sending signatures.

v can be 0/1 from some signers; normalize to 27/28 (or use viem.parseSignature) before posting to the API. citeturn2search5turn2search1

✅ Minimal normalization

-        v: parseInt(signature.slice(130, 132), 16),
+        v: (() => {
+          let v = parseInt(signature.slice(130, 132), 16);
+          return v < 2 ? v + 27 : v;
+        })(),

---

438-442: accountToUse selection logic is incomplete.

The placeholder comment means the account might not match the connected provider, causing signature/account mismatches. Mirror the eth_accounts probe used in handleApproveAgent. citeturn2search5turn2search1

src/apps/perps/lib/hyperliquid/sdk.ts (2)

30-35: Price formatting can emit scientific notation.

parseFloat(...toPrecision(5)).toString() can yield "1e-7", which is typically rejected by Hyperliquid’s API. Use a decimal-string formatter that guarantees non-scientific notation. citeturn2search5turn2search1

---

177-184: ExchangeClient should receive an account, not raw private key.

approveAgentSDK passes masterPrivateKey directly, while other SDK calls convert with privateKeyToAccount. Align this to avoid runtime/type errors. citeturn2search5turn2search1

✅ Suggested fix

 export async function approveAgentSDK(
   masterPrivateKey: Hex,
   agentAddress: string,
   agentName?: string
 ): Promise<any> {
+  const account = privateKeyToAccount(masterPrivateKey);
   const transport = new HttpTransport();
-  const client = new ExchangeClient({ wallet: masterPrivateKey, transport });
+  const client = new ExchangeClient({ wallet: account, transport });

src/apps/perps/components/TradeForm.tsx (6)

37-76: Missing EnhancedAsset import (compile error).

TradeFormProps references EnhancedAsset, but it isn’t imported.

✅ Fix import

-import type { AssetInfo, UserState } from '../lib/hyperliquid/types';
+import type { AssetInfo, UserState, EnhancedAsset } from '../lib/hyperliquid/types';

---

147-186: useFieldArray desync risk from setValue.

Using setValue('takeProfits', ...) can desync tpFields/IDs. Use replace/update from useFieldArray instead.

🔧 Safer approach

 const {
   fields: tpFields,
   append: appendTp,
   remove: removeTp,
+  replace: replaceTp,
 } = useFieldArray({ control, name: 'takeProfits' });

 ...
-    setValue('takeProfits', updatedValues);
+    replaceTp(updatedValues);

---

268-279: Prefill TP ratios can sum to < 100%.

Math.floor(100 / tps.length) drops the remainder, causing submit-time validation failures. Use getDistributedRatios(...).

✅ Suggested fix

-        const ratio = tps.length > 0 ? Math.floor(100 / tps.length) : 0;
+        const ratios = getDistributedRatios(tps.length);
         setValue(
           'takeProfits',
-          tps.map((p) => ({
+          tps.map((p, i) => ({
             price: p,
-            ratio: ratio,
+            ratio: ratios[i] ?? 0,
             distance: calculateDistance(p, ep),
           }))
         );

---

453-461: isSubmitting never resets on size <= 0.

You return early without setIsSubmitting(false), leaving the button stuck in loading state.

✅ Fix

       if (size <= 0) {
         ...
         toast.error(...);
+        setIsSubmitting(false);
         return;
       }

---

405-411: Loading toast isn’t dismissed on builder-approval error.

The toast.loading created at Line 370 isn’t replaced because the error toast lacks { id: toastId }.

✅ Fix

-          toast.error('PillarX Approval Required', {
+          toast.error('PillarX Approval Required', {
+            id: toastId,
             description: 'Please go to Settings > Perps Account and approve PillarX to start trading.',
             duration: 5000,
           });

---

428-432: Two useBuilderFee definitions cause inconsistent behavior.

The inner useBuilderFee shadows the outer one; the catch block references the outer value, not the actual builder-fee decision used for the order. Compute it once and reuse.

Also applies to: 495-501

src/apps/perps/components/PositionsCard.tsx (9)

13-17: Missing UserState type import causes TypeScript compilation error.

UserState is referenced at line 89 (useState<UserState | null>) but is not included in the type imports. This will cause a TypeScript error.

Proposed fix

 import type {
   HyperliquidPosition,
   HyperliquidOrder,
   UniverseAsset,
+  UserState,
 } from '../lib/hyperliquid/types';

---

218-218: Circular dependency in fetchData useCallback.

fetchData both reads internalUserState (line 151) and sets it (line 127), while including it in the dependency array. This creates a potential for unnecessary re-renders when external data is not provided.

Suggested fix

Remove internalUserState from the dependency array since it's an internal state managed by this function:

- }, [masterAddress, userState, directPositions, externalOpenOrders, internalUserState]);
+ }, [masterAddress, userState, directPositions, externalOpenOrders]);

The effectiveState fallback at line 151 can use a ref if needed, or rely on the fact that when userState is provided externally, internalUserState isn't used.

---

213-216: Add user feedback for data fetch errors.

In a trading application, silently failing to load positions could lead to uninformed decisions. Users should be notified when position data cannot be loaded.

Suggested fix

     } catch (error) {
       console.error('Error fetching data:', error);
+      toast.error('Failed to load positions', {
+        description: 'Please try refreshing the page'
+      });
     } finally {

---

323-326: Hardcoded decimal precision may cause order rejection.

Using .toFixed(6) ignores the asset's actual szDecimals from universe metadata. The position is enriched with coinInfo.szDecimals at line 198, which should be used here.

Proposed fix

       const totalSize = Math.abs(parseFloat(positionToClose.szi));
       const sizeToClose = totalSize * (closePercentage / 100);
-      const sizeStr = sizeToClose.toFixed(6);
+      const decimals = positionToClose.coinInfo?.szDecimals ?? 6;
+      const sizeStr = sizeToClose.toFixed(decimals);
       const size = parseFloat(sizeStr);

---

341-346: Add response validation before showing success toast.

The success toast is displayed immediately after placeMarketOrderAgent() completes without validating the response. If the SDK returns a failure status instead of throwing, users will see "Order submitted" for rejected orders.

Suggested approach

-      await placeMarketOrderAgent(privateKey as `0x${string}`, {
+      const response = await placeMarketOrderAgent(privateKey as `0x${string}`, {
         coinId,
         isBuy: !isLong,
         size,
         currentPrice,
         reduceOnly: true,
       });

+      // Validate response if the SDK returns status instead of throwing
+      if (response?.status === 'error' || response?.error) {
+        throw new Error(response.error || 'Order submission failed');
+      }
+
       toast.success('Order submitted');

Also consider removing the 1-second delay before refresh, or adding cleanup for the timeout.

---

507-544: Missing await on async function calls may cause race conditions.

handleExecuteClose and handleCancelOrder are async functions called without await. The setPendingAction(null) and return true execute before these actions complete, potentially causing UI state inconsistencies.

Proposed fix

       if (unlocked) {
         setShowUnlockModal(false);
+        setPendingAction(null);
         // Retry pending action
         if (pendingAction?.type === 'close') {
-          handleExecuteClose();
+          await handleExecuteClose();
         } else if (pendingAction?.type === 'cancel' && pendingAction.data) {
-          handleCancelOrder(pendingAction.data);
+          await handleCancelOrder(pendingAction.data);
         }
-        setPendingAction(null);
         return true;
       }

Apply the same fix to both unlock branches (imported account and agent wallet).

---

632-642: Loading state check order is inverted — skeletons never display during initial load.

On initial render, both positions.length === 0 and isLoading are true. Since positions.length === 0 is checked first, "No open positions" displays instead of loading skeletons.

Proposed fix

-             {positions.length === 0 ? (
+             {isLoading ? (
+               <div className="space-y-3">
+                 {[1, 2, 3].map((i) => (
+                   <Skeleton key={i} className="h-24 w-full rounded-lg" />
+                 ))}
+               </div>
+             ) : positions.length === 0 ? (
                <p className="text-sm text-muted-foreground text-center py-4 bg-muted/20 rounded-lg border border-dashed">
                  No open positions
                </p>
-             ) : isLoading ? (
-               <div className="space-y-3">
-                 {[1, 2, 3].map((i) => (
-                   <Skeleton key={i} className="h-24 w-full rounded-lg" />
-                 ))}
-               </div>
              ) : expandedPositionIndex !== null &&

---

1111-1120: Same loading state issue for Open Orders section.

Apply the same fix — check isLoading before openOrders.length === 0.

Proposed fix

-             {openOrders.length === 0 ? (
+             {isLoading ? (
+               <div className="space-y-2">
+                 {[1, 2].map((i) => (
+                   <Skeleton key={i} className="h-12 w-full rounded-lg" />
+                 ))}
+               </div>
+             ) : openOrders.length === 0 ? (
                <p className="text-sm text-muted-foreground text-center py-4 bg-muted/20 rounded-lg border border-dashed">
                  No open orders
                </p>
-             ) : isLoading ? (
-               <div className="space-y-2">
-                 {[1, 2].map((i) => (
-                   <Skeleton key={i} className="h-12 w-full rounded-lg" />
-                 ))}
-               </div>
              ) : (

---

964-983: Dead code in TP/SL fallback branch.

When hasOrders is false (meaning tps.length === 0 && sls.length === 0), the inner else branch at lines 968-980 checking for TP/SL lengths is unreachable — the condition tps.length === 0 && sls.length === 0 is always true in this case.

Simplified code

                             ) : (
-                              <span className="flex gap-1 justify-end">
-                                {tps.length === 0 && sls.length === 0 ? (
-                                  <span>-</span>
-                                ) : (
-                                  <>
-                                    {tps.length > 0 && (
-                                      <span className="text-green-500">
-                                        ${formatPrice(tps[0].price)}
-                                      </span>
-                                    )}
-                                    {sls.length > 0 && (
-                                      <span className="text-red-500">
-                                        ${formatPrice(sls[0].price)}
-                                      </span>
-                                    )}
-                                  </>
-                                )}
-                              </span>
+                              <span>-</span>
                             )}

The same simplification applies to the desktop view (lines 1064-1083).

🧹 Nitpick comments (5)

src/apps/perps/components/mobile/MobileMarketsList.tsx (4)

3-3: Remove unused import.

Input is imported but never used in this component.

-import { Input } from '../ui/input';

---

5-13: Consider removing unused coinIcon field.

The coinIcon property is defined in the interface but the component uses getCoinColor and getCoinInitial helpers instead of rendering an actual icon image. If this field is intended for future use, consider adding a TODO comment; otherwise, remove it to keep the interface clean.

---

34-48: Header buttons lack click handlers.

The Search and "By Price" buttons are rendered but have no onClick handlers, making them non-functional. If these are placeholders for future functionality, consider adding TODO comments or disabling them to set user expectations.

---

84-91: Handle edge case for empty change string.

The color logic relies on market.change.startsWith('-'), which will return false for an empty string, defaulting to green. If an empty or zero change should display differently (e.g., neutral gray), add explicit handling.

 <p
-  className={`text-sm font-medium ${market.change.startsWith('-')
-      ? 'text-red-500'
-      : 'text-green-500'
-    }`}
+  className={`text-sm font-medium ${
+    !market.change || market.change === '0'
+      ? 'text-gray-500'
+      : market.change.startsWith('-')
+        ? 'text-red-500'
+        : 'text-green-500'
+  }`}
 >

src/apps/perps/components/PositionsCard.tsx (1)

54-55: Unused import.

getUserFills is imported but never used in this component.

Proposed fix

 import {
   getMarkPrice,
   getOpenOrders,
   getFrontendOpenOrders,
   getMetaAndAssetCtxs,
-  getUserFills,
 } from '../lib/hyperliquid/client';

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@package.json`:
- Line 27: The package.json lists dependencies "@hookform/resolvers" and
"@nktkas/hyperliquid" that appear unused; please search the repo for
imports/usages of "@hookform/resolvers" and "@nktkas/hyperliquid" (e.g., in
React forms or template rendering code) and if none are referenced remove them
from package.json and run npm/yarn install to update lockfile, otherwise keep
and add a brief code comment or usage where appropriate to justify keeping them;
ensure any removal also updates CI/build steps that might rely on them.

♻️ Duplicate comments (2)

package.json (2)

5-5: ESM migration requires config file updates.

This concern was already raised in a previous review. Ensure all CommonJS config files (Tailwind configs, config-overrides.js) are migrated to ESM syntax or renamed to .cjs.

---

100-100: ThemeProvider initialization still required.

This concern was already raised in a previous review. The next-themes dependency requires a ThemeProvider wrapper to function correctly with useTheme() in sonner.tsx.

🧹 Nitpick comments (1)

package.json (1)

101-101: Remove unused patch-package dependency.

The patches/ directory does not exist in the repository, and there is no postinstall script configured. Since there are no patches to apply and patch-package is not being used, it should be removed from the dependencies to keep the package.json clean.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

npm view `@hookform/resolvers`@^5.2.2 version 2>/dev/null || npm view `@hookform/resolvers` version
npm view `@nktkas/hyperliquid`@^0.30.2 version 2>/dev/null || npm view `@nktkas/hyperliquid` version

Repository: pillarwallet/x

Length of output: 70

---

🏁 Script executed:

cd /tmp && cat << 'EOF' > check_versions.js
const https = require('https');

function getPackageVersion(pkgName, specVersion) {
  return new Promise((resolve) => {
    https.get(`https://registry.npmjs.org/${pkgName}`, (res) => {
      let data = '';
      res.on('data', chunk => data += chunk);
      res.on('end', () => {
        try {
          const pkg = JSON.parse(data);
          const latest = pkg['dist-tags'].latest;
          resolve({ package: pkgName, specVersion, latest });
        } catch (e) {
          resolve({ package: pkgName, specVersion, error: 'Failed to parse' });
        }
      });
    }).on('error', err => resolve({ package: pkgName, specVersion, error: err.message }));
  });
}

Promise.all([
  getPackageVersion('@hookform/resolvers', '^5.2.2'),
  getPackageVersion('@nktkas/hyperliquid', '^0.30.2')
]).then(results => {
  results.forEach(r => console.log(JSON.stringify(r)));
});
EOF
node check_versions.js

Repository: pillarwallet/x

Length of output: 207

---

🏁 Script executed:

# Search for usage of these packages in the codebase
echo "=== Searching for `@hookform/resolvers` usage ==="
rg "@hookform/resolvers" --type ts --type tsx --type js --type jsx

echo -e "\n=== Searching for `@nktkas/hyperliquid` usage ==="
rg "@nktkas/hyperliquid" --type ts --type tsx --type js --type jsx

echo -e "\n=== Checking package.json context ==="
cat package.json | grep -A 2 -B 2 "@hookform/resolvers"
cat package.json | grep -A 2 -B 2 "@nktkas/hyperliquid"

Repository: pillarwallet/x

Length of output: 630

---

@hookform/resolvers and @nktkas/hyperliquid appear unused in the codebase.

Both dependencies are at the latest available versions (@hookform/resolvers@5.2.2, @nktkas/hyperliquid@0.30.2), but no usage was found in the source files. Before merging, verify these dependencies are actually needed or remove them if they are leftover from previous code.

🤖 Prompt for AI Agents

In `@package.json` at line 27, The package.json lists dependencies
"@hookform/resolvers" and "@nktkas/hyperliquid" that appear unused; please
search the repo for imports/usages of "@hookform/resolvers" and
"@nktkas/hyperliquid" (e.g., in React forms or template rendering code) and if
none are referenced remove them from package.json and run npm/yarn install to
update lockfile, otherwise keep and add a brief code comment or usage where
appropriate to justify keeping them; ensure any removal also updates CI/build
steps that might rely on them.