Conversation
Production release 🚀
Production release 🚀
Production release 🚀
Production Release 🚀
Production Release 🚀
Production Release 🚀
Production Release 🚀
Production release 🚀
Production release 🚀
🚀 Production release
Production release 🚀
Production release 🚀
Production release 🚀
Production release
Production release 🚀
Production release 🚀
Production release 🚀
Production release 🚀
Production release 🚀
Production Release 🚀
Production release 🚀
Production release 🚀
Production release 🚀
🚀 Production Release
Production Release 🚀
🚀 Production release
🚀 Production Release
🚀 Production release
🚀 Production release
Production release
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Fix all issues with AI agents
In @src/apps/perps/hooks/useHyperliquid.ts:
- Around line 189-207: Extract the duplicated wallet client creation into a
single async helper named createHyperliquidWalletClient(address, provider) that
returns a WalletClient|null: it should return null if no address, import
getImportedAccount from ../lib/hyperliquid/keystore and privateKeyToAccount from
viem/accounts when needed, compare importedAccount.accountAddress to address and
build createWalletClient({ account, chain: arbitrum, transport: custom(provider)
}) using the privateKey-derived account or the raw address otherwise; replace
the duplicated blocks in setupHyperliquid, checkSetupStatus, and
executeCopyTrade with calls to this helper and update any call sites to handle a
nullable return.
- Line 185: The checkSetupStatus callback closes over kit and provider but the
dependency array is empty, causing a stale closure; update the dependency array
for the useCallback (or the effect where checkSetupStatus is defined) to include
kit and provider so the function is recreated when those values change
(alternatively store provider in a ref if you must avoid re-running effects),
and then verify the useEffect that calls checkSetupStatus (around line ~390)
still behaves as intended when checkSetupStatus is recreated.
- Line 386: The dependency array for the executeCopyTrade callback is missing
provider, causing a stale closure; update the dependency array (currently
[address, setupStatus, loadBalance]) used by executeCopyTrade in
useHyperliquid.ts to include provider so the callback sees the latest provider
reference (also verify any other values used inside executeCopyTrade are present
in the array).
- Line 43: The walletClient state in the useHyperliquid hook is typed as any;
change it to a proper WalletClient type by importing the correct WalletClient
type from your wallet library and updating the state to useState<WalletClient |
null>(null). Update any places that call setWalletClient or read walletClient to
handle the nullable WalletClient type (e.g., add null checks or optional
chaining). Ensure the import uses a type-only import (import type { WalletClient
} from '...') to avoid runtime side effects.
- Around line 57-88: The code currently prints sensitive account information via
console.log for importedAccount.accountAddress, the EOA (eoa) and
client?.account?.address inside useHyperliquid; replace these direct logs by
either removing them or gating them behind a development-only check (e.g. only
log when process.env.NODE_ENV !== 'production' or a passed debug flag) and never
log private keys; if you must log addresses for debugging, log a masked form
(e.g. first 6 + last 4 chars) rather than the full value and ensure any existing
console.log statements referencing importedAccount.accountAddress, eoa, or
client?.account?.address are updated accordingly.
- Around line 341-345: The code is using hardcoded trading params (notional=5,
leverage=5, and roundToSzDecimals(..., 3)) which ignores user/tile settings and
asset.szDecimals; update the logic in the executeCopyTrade flow to source
notional and leverage from the CopyTile (e.g., tile.notional and tile.leverage)
or accept them as parameters to executeCopyTrade if not present on CopyTile, and
replace the fixed decimal 3 with asset.szDecimals when calling
roundToSzDecimals; ensure you update the CopyTile type to include
notional/leverage if needed and adjust callers to pass the values.
In @src/apps/perps/lib/hyperliquid/keystore.ts:
- Line 214: Imported account flows emit 'imported-account-changed' events but
agent wallet flows do not, causing inconsistent UI updates; update the agent
wallet code to dispatch a mirror event after changes: inside
storeAgentWalletEncrypted (after the successful store operation, e.g., where the
existing imported-account dispatch would occur) dispatch
window.dispatchEvent(new CustomEvent('agent-wallet-changed', { detail: {
masterAddress } })); and do the same in clearAgentWalletLocal (after clearing
the local agent wallet), using the same event name and detail shape so UI
listeners can handle agent wallet state changes consistently with
imported-account changes.
- Around line 253-262: getImportedAccountAddress currently calls JSON.parse on
localStorage.getItem(GLOBAL_ACCOUNT_KEY)! without error handling which will
throw on corrupted JSON; wrap the parse in a try-catch (or use a safeParse
helper) when LOCAL_ACCOUNT_KEY exists, extract accountAddress from the parsed
object only on successful parse, and return null (and optionally log or remove
the corrupted key) on parse failure so the function never throws. Use the
existing function name getImportedAccountAddress and the keys
GLOBAL_IMPORTED_ADDRESS_KEY and GLOBAL_ACCOUNT_KEY to locate where to add the
try-catch.
- Around line 55-57: The code assigns cachedPrivateKey and calls
resetInactivityTimer before confirming localStorage persistence; move the cache
population and timer reset so they occur only after all localStorage operations
(e.g., localStorage.setItem or whatever persistence functions used in this
block) complete successfully and without throwing. Wrap storage calls in a
try/catch, verify they succeeded, and only then set cachedPrivateKey =
privateKey and call resetInactivityTimer; on storage failure roll back any
partial state (don’t populate cache) and surface/log the error.
🧹 Nitpick comments (9)
src/apps/perps/lib/hyperliquid/keystore.ts (4)
16-23: Code duplication detected: generateAgentWallet exists in multiple files.This function duplicates logic from
src/apps/perps/lib/hyperliquid/signing.ts(lines 365-379). The implementation here usinggeneratePrivateKey()from viem is preferable to the manual crypto approach in signing.ts. Consider removing the duplicate and importing from a single source.
280-286: Cache returns key without verifying it matches the stored address.The cached key is returned based solely on the presence of an address in storage (lines 280-285), without verifying that the cached key actually corresponds to that address. If localStorage is manipulated between caching and retrieval, the wrong key could be returned for an address.
While an attacker with localStorage access has broader capabilities, defensive programming suggests verifying the match. Consider deriving the address from the cached key and comparing it to the stored address before returning.
357-383: Ambiguous null return: Can't distinguish "no wallet" from "wallet locked".
getAgentWalletreturnsnullfor both scenarios: when no wallet exists and when a wallet is encrypted but not unlocked (lines 373-379). Callers cannot distinguish between these states, which may require different UI flows (setup vs. unlock).Consider returning a status indicator or using different return types:
type WalletState = | { status: 'unlocked'; wallet: { address: string; privateKey: Hex; approved: boolean; builderApproved: boolean } } | { status: 'locked' } | { status: 'none' }; export async function getAgentWalletState(masterAddress: string): Promise<WalletState> { // Check cache... if (cachedPrivateKey) { // return { status: 'unlocked', wallet: {...} } } if (isAgentWalletEncrypted(masterAddress)) { return { status: 'locked' }; } const legacy = getAgentWalletLocal(masterAddress); if (legacy) { return { status: 'unlocked', wallet: legacy }; } return { status: 'none' }; }
405-408: Function declared async but performs no asynchronous operations.
checkWalletMatchis declared asasyncbut contains only synchronous operations. While this doesn't cause functional issues, it creates unnecessary Promise overhead.Remove async if not needed
-export async function checkWalletMatch(importedAddress: string, masterAddress: string): Promise<boolean> { +export function checkWalletMatch(importedAddress: string, masterAddress: string): boolean { // Normalize addresses for comparison const normalize = (addr: string) => addr.toLowerCase(); return normalize(importedAddress) === normalize(getAgentAddress(masterAddress) || ''); }Note: Keep async if you anticipate future changes that will require asynchronous operations.
src/apps/perps/hooks/useHyperliquid.ts (5)
89-90: Remove duplicate comment.The comment "Always fetch assets on load" appears twice on consecutive lines.
♻️ Proposed fix
- // Always fetch assets on load // Always fetch assets on load
145-158: Remove commented code or document with TODO.This large commented block (14 lines) for agent address switching should either be removed if no longer needed, or documented with a TODO comment explaining why it's disabled and when it will be re-enabled.
♻️ Proposed fix
If the feature is planned:
+ // TODO: Re-enable agent address switching after testing // 2. Check Agent Address // Unconditional switch: If an agent is linked, we use it.If the feature is not needed:
- // 2. Check Agent Address - // Unconditional switch: If an agent is linked, we use it. - // const agentAddress = getAgentAddress(eoa); - // if (agentAddress) { - // console.log( - // 'DEBUG: Found Agent Address, executing switch:', - // agentAddress - // ); - - // const agentState = await getUserState(agentAddress); - // const agentOrders = await getOpenOrders(agentAddress); - - // targetAddress = agentAddress as string; - // state = agentState; - // orders = agentOrders; - // }
248-260: Remove commented code or document with TODO.Another large commented block for agent address logic. Apply the same guidance as the previous commented block in
checkSetupStatus.
47-185: Consider refactoring for better separation of concerns.The
checkSetupStatusfunction is 138 lines and handles multiple responsibilities:
- Wallet client creation
- Asset fetching and parsing
- User state retrieval
- Setup status determination
Consider breaking this into smaller, focused functions:
resolveActiveWallet()- determine and create wallet clientfetchAvailableAssets()- load and parse asset datacheckUserSetupStatus(address)- verify user stateThis would improve testability, readability, and maintainability.
45-45: Consider validating provider before use.The
provideris obtained fromkit.getProvider()but is never validated for null or undefined before being used in wallet client creation (lines 67, 81, 199, 205, 303, 309). If the provider is unavailable, this will cause runtime errors when attempting to create wallet clients.♻️ Proposed fix
Add validation in functions that use the provider:
const setupHyperliquid = useCallback(async () => { + if (!provider) { + toast.error('Wallet provider not available'); + return; + } // ... rest of functionOr add validation once in the extracted
createHyperliquidWalletClienthelper function if you implement that refactor.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/perps/hooks/useHyperliquid.tssrc/apps/perps/lib/hyperliquid/keystore.ts
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/perps/hooks/useHyperliquid.ts
📚 Learning: 2026-01-07T12:27:46.789Z
Learnt from: vignesha22
Repo: pillarwallet/x PR: 486
File: src/apps/pulse/components/Onboarding/TopUpScreen.tsx:1005-1049
Timestamp: 2026-01-07T12:27:46.789Z
Learning: In the Pulse app TopUpScreen gasless flow (src/apps/pulse/components/Onboarding/TopUpScreen.tsx), when the Gasless Fee Estimate UI is displayed, the fee token (selectedFeeAsset) is always the same as the topup token (selectedToken) because selectedFeeAsset is only set when a matching paymaster is found where gasToken matches selectedToken.address (lines 377-391).
Applied to files:
src/apps/perps/hooks/useHyperliquid.ts
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/perps/hooks/useHyperliquid.ts
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/perps/hooks/useHyperliquid.ts
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/perps/hooks/useHyperliquid.ts
🧬 Code graph analysis (2)
src/apps/perps/hooks/useHyperliquid.ts (6)
src/apps/perps/lib/hyperliquid/types.ts (4)
UserState(28-55)HyperliquidOrder(106-125)EnhancedAsset(80-85)CopyTile(20-26)src/apps/perps/lib/hyperliquid/keystore.ts (1)
getImportedAccount(273-308)src/apps/perps/lib/hyperliquid/client.ts (5)
getMetaAndAssetCtxs(228-249)getAllAssets(198-226)getUserState(53-72)getOpenOrders(94-124)postExchange(12-51)src/apps/perps/lib/hyperliquid/signing.ts (2)
buildNoopAction(94-98)signUserAction(29-92)src/apps/perps/components/CopyTile.tsx (1)
CopyTile(21-137)src/apps/perps/lib/hyperliquid/math.ts (3)
getEntryPrice(18-23)calculatePositionSize(1-8)roundToSzDecimals(10-16)
src/apps/perps/lib/hyperliquid/keystore.ts (3)
src/apps/perps/lib/hyperliquid/session-manager.ts (3)
initializeSessionManager(19-22)resetInactivityTimer(28-41)clearSession(47-58)src/apps/perps/lib/hyperliquid/signing.ts (1)
generateAgentWallet(366-380)src/apps/perps/lib/encryption.ts (3)
encryptWithPin(50-72)EncryptedData(43-47)decryptWithPin(75-97)
🪛 ast-grep (0.40.4)
src/apps/perps/lib/hyperliquid/keystore.ts
[warning] 45-45: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(getStorageKey(masterAddress, 'address'), address)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
[warning] 130-130: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(getStorageKey(masterAddress, 'address'), address)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
[warning] 204-204: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(GLOBAL_IMPORTED_ADDRESS_KEY, address)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages: x
🔇 Additional comments (3)
src/apps/perps/lib/hyperliquid/keystore.ts (2)
64-108: LGTM: Well-implemented unlock with defensive validation.The defensive validation of boolean values (lines 74-77, 81-84) prevents type errors from corrupted data. The fallback to legacy plaintext storage is appropriate for migration support, and error handling properly delegates PIN validation to the UI.
10-14: This concern doesn't apply to a client-side bundled application.The keystore module is client-side only with no server-side rendering detected. In bundled JavaScript applications, module code executes once at load time regardless of how many files import it. The dynamic imports in useHyperliquid.ts load the already-initialized module. Even if
initializeSessionManager()were called multiple times, the session manager safely handles it by overwriting the callback and resetting the inactivity timer.Likely an incorrect or invalid review comment.
src/apps/perps/hooks/useHyperliquid.ts (1)
398-398: The custom event dispatch is already consistent. All code paths that modify the imported account state (storeImportedAccountEncryptedat line 214 andclearImportedAccountat line 317 in keystore.ts) dispatch the'imported-account-changed'event. TheunlockImportedAccountfunction does not dispatch because it only populates the in-memory cache without modifying stored state—this is the correct behavior.Likely an incorrect or invalid review comment.
| const [isLoading, setIsLoading] = useState(false); | ||
| const [activeAddress, setActiveAddress] = useState<string | null>(null); | ||
| const [address, setAddress] = useState<string | null>(null); | ||
| const [walletClient, setWalletClient] = useState<any>(null); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Replace any type with proper WalletClient type.
The walletClient state uses any, which bypasses TypeScript's type checking and can lead to runtime errors.
♻️ Proposed fix
+import type { WalletClient } from 'viem';
+
- const [walletClient, setWalletClient] = useState<any>(null);
+ const [walletClient, setWalletClient] = useState<WalletClient | null>(null);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const [walletClient, setWalletClient] = useState<any>(null); | |
| import type { WalletClient } from 'viem'; | |
| const [walletClient, setWalletClient] = useState<WalletClient | null>(null); |
🤖 Prompt for AI Agents
In @src/apps/perps/hooks/useHyperliquid.ts at line 43, The walletClient state in
the useHyperliquid hook is typed as any; change it to a proper WalletClient type
by importing the correct WalletClient type from your wallet library and updating
the state to useState<WalletClient | null>(null). Update any places that call
setWalletClient or read walletClient to handle the nullable WalletClient type
(e.g., add null checks or optional chaining). Ensure the import uses a type-only
import (import type { WalletClient } from '...') to avoid runtime side effects.
| console.log('DEBUG: Using Imported Account:', importedAccount.accountAddress); | ||
| targetAddress = importedAccount.accountAddress; | ||
|
|
||
| // Create WalletClient from Private Key | ||
| const { privateKeyToAccount } = await import('viem/accounts'); | ||
| const account = privateKeyToAccount(importedAccount.privateKey as `0x${string}`); | ||
|
|
||
| client = createWalletClient({ | ||
| account, | ||
| chain: arbitrum, | ||
| transport: custom(provider as any), | ||
| }); | ||
| isImported = true; | ||
| } else { | ||
| // 2. Fallback to Connected Wallet | ||
| const walletProvider = kit.getEtherspotProvider(); | ||
| const eoa = (await walletProvider.getSdk()).getEOAAddress() || null; | ||
| console.log('DEBUG: Wallet Provider EOA:', eoa); | ||
|
|
||
| if (eoa) { | ||
| targetAddress = eoa; | ||
| client = createWalletClient({ | ||
| account: eoa as `0x${string}`, | ||
| chain: arbitrum, | ||
| transport: custom(provider as any), | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| setAddress(targetAddress); | ||
| setWalletClient(client); | ||
| console.log('DEBUG: Active Client Account:', client?.account?.address); |
There was a problem hiding this comment.
Remove or protect debug logging that exposes PII.
Multiple console.log statements expose sensitive information like account addresses and private keys in use. These should be removed in production or wrapped in a development-only condition.
Lines with PII exposure:
- Line 57: Logs imported account address
- Line 74: Logs EOA address
- Line 88: Logs client account address
♻️ Proposed fix
- console.log('DEBUG: Using Imported Account:', importedAccount.accountAddress);
+ if (process.env.NODE_ENV === 'development') {
+ console.log('DEBUG: Using Imported Account');
+ }
...
- console.log('DEBUG: Wallet Provider EOA:', eoa);
+ if (process.env.NODE_ENV === 'development') {
+ console.log('DEBUG: Wallet Provider EOA');
+ }
...
- console.log('DEBUG: Active Client Account:', client?.account?.address);
+ if (process.env.NODE_ENV === 'development') {
+ console.log('DEBUG: Active Client Account');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('DEBUG: Using Imported Account:', importedAccount.accountAddress); | |
| targetAddress = importedAccount.accountAddress; | |
| // Create WalletClient from Private Key | |
| const { privateKeyToAccount } = await import('viem/accounts'); | |
| const account = privateKeyToAccount(importedAccount.privateKey as `0x${string}`); | |
| client = createWalletClient({ | |
| account, | |
| chain: arbitrum, | |
| transport: custom(provider as any), | |
| }); | |
| isImported = true; | |
| } else { | |
| // 2. Fallback to Connected Wallet | |
| const walletProvider = kit.getEtherspotProvider(); | |
| const eoa = (await walletProvider.getSdk()).getEOAAddress() || null; | |
| console.log('DEBUG: Wallet Provider EOA:', eoa); | |
| if (eoa) { | |
| targetAddress = eoa; | |
| client = createWalletClient({ | |
| account: eoa as `0x${string}`, | |
| chain: arbitrum, | |
| transport: custom(provider as any), | |
| }); | |
| } | |
| } | |
| setAddress(targetAddress); | |
| setWalletClient(client); | |
| console.log('DEBUG: Active Client Account:', client?.account?.address); | |
| if (process.env.NODE_ENV === 'development') { | |
| console.log('DEBUG: Using Imported Account'); | |
| } | |
| targetAddress = importedAccount.accountAddress; | |
| // Create WalletClient from Private Key | |
| const { privateKeyToAccount } = await import('viem/accounts'); | |
| const account = privateKeyToAccount(importedAccount.privateKey as `0x${string}`); | |
| client = createWalletClient({ | |
| account, | |
| chain: arbitrum, | |
| transport: custom(provider as any), | |
| }); | |
| isImported = true; | |
| } else { | |
| // 2. Fallback to Connected Wallet | |
| const walletProvider = kit.getEtherspotProvider(); | |
| const eoa = (await walletProvider.getSdk()).getEOAAddress() || null; | |
| if (process.env.NODE_ENV === 'development') { | |
| console.log('DEBUG: Wallet Provider EOA'); | |
| } | |
| if (eoa) { | |
| targetAddress = eoa; | |
| client = createWalletClient({ | |
| account: eoa as `0x${string}`, | |
| chain: arbitrum, | |
| transport: custom(provider as any), | |
| }); | |
| } | |
| } | |
| setAddress(targetAddress); | |
| setWalletClient(client); | |
| if (process.env.NODE_ENV === 'development') { | |
| console.log('DEBUG: Active Client Account'); | |
| } |
🤖 Prompt for AI Agents
In @src/apps/perps/hooks/useHyperliquid.ts around lines 57 - 88, The code
currently prints sensitive account information via console.log for
importedAccount.accountAddress, the EOA (eoa) and client?.account?.address
inside useHyperliquid; replace these direct logs by either removing them or
gating them behind a development-only check (e.g. only log when
process.env.NODE_ENV !== 'production' or a passed debug flag) and never log
private keys; if you must log addresses for debugging, log a masked form (e.g.
first 6 + last 4 chars) rather than the full value and ensure any existing
console.log statements referencing importedAccount.accountAddress, eoa, or
client?.account?.address are updated accordingly.
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Fix stale closure issue in dependency array.
The checkSetupStatus callback uses kit and provider (line 45, 72) but the dependency array is empty. This creates a stale closure where the function captures the initial values and won't update if kit changes. This can lead to using outdated provider instances.
🐛 Proposed fix
- }, []);
+ }, [kit, provider]);Note: This will cause checkSetupStatus to be recreated when kit changes, which may trigger the useEffect on line 390. Verify this behavior is acceptable, or consider using useRef to store the provider if you want to avoid re-running the effect.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, []); | |
| }, [kit, provider]); |
🤖 Prompt for AI Agents
In @src/apps/perps/hooks/useHyperliquid.ts at line 185, The checkSetupStatus
callback closes over kit and provider but the dependency array is empty, causing
a stale closure; update the dependency array for the useCallback (or the effect
where checkSetupStatus is defined) to include kit and provider so the function
is recreated when those values change (alternatively store provider in a ref if
you must avoid re-running effects), and then verify the useEffect that calls
checkSetupStatus (around line ~390) still behaves as intended when
checkSetupStatus is recreated.
| const { getImportedAccount } = await import('../lib/hyperliquid/keystore'); | ||
| const importedAccount = getImportedAccount(); | ||
|
|
||
| let client; | ||
| if (importedAccount && importedAccount.accountAddress === address) { | ||
| const { privateKeyToAccount } = await import('viem/accounts'); | ||
| const account = privateKeyToAccount(importedAccount.privateKey as `0x${string}`); | ||
| client = createWalletClient({ | ||
| account, | ||
| chain: arbitrum, | ||
| transport: custom(provider as any), | ||
| }); | ||
| } else { | ||
| client = createWalletClient({ | ||
| account: address as `0x${string}`, | ||
| chain: arbitrum, | ||
| transport: custom(provider as any), | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated wallet client creation logic.
The wallet client creation logic (checking imported account vs connected wallet) is duplicated across setupHyperliquid (lines 189-207), checkSetupStatus (lines 48-84), and executeCopyTrade (lines 293-311). This makes the code harder to maintain and increases the risk of inconsistencies.
♻️ Proposed refactor
Create a shared helper function:
async function createHyperliquidWalletClient(
address: string | null,
provider: any
): Promise<WalletClient | null> {
if (!address) return null;
const { getImportedAccount } = await import('../lib/hyperliquid/keystore');
const importedAccount = getImportedAccount();
if (importedAccount && importedAccount.accountAddress === address) {
const { privateKeyToAccount } = await import('viem/accounts');
const account = privateKeyToAccount(importedAccount.privateKey as `0x${string}`);
return createWalletClient({
account,
chain: arbitrum,
transport: custom(provider),
});
}
return createWalletClient({
account: address as `0x${string}`,
chain: arbitrum,
transport: custom(provider),
});
}Then use it in all three places:
- const { getImportedAccount } = await import('../lib/hyperliquid/keystore');
- const importedAccount = getImportedAccount();
- // ... 15+ lines of duplicate logic
- const walletClient = client;
+ const walletClient = await createHyperliquidWalletClient(address, provider);🤖 Prompt for AI Agents
In @src/apps/perps/hooks/useHyperliquid.ts around lines 189 - 207, Extract the
duplicated wallet client creation into a single async helper named
createHyperliquidWalletClient(address, provider) that returns a
WalletClient|null: it should return null if no address, import
getImportedAccount from ../lib/hyperliquid/keystore and privateKeyToAccount from
viem/accounts when needed, compare importedAccount.accountAddress to address and
build createWalletClient({ account, chain: arbitrum, transport: custom(provider)
}) using the privateKey-derived account or the raw address otherwise; replace
the duplicated blocks in setupHyperliquid, checkSetupStatus, and
executeCopyTrade with calls to this helper and update any call sites to handle a
nullable return.
| } finally { | ||
| setIsLoading(false); | ||
| } | ||
| }, [address, checkSetupStatus]); |
There was a problem hiding this comment.
Fix stale closure issue in dependency array.
Similar to checkSetupStatus, this function uses provider (line 199, 205) and kit (indirectly through provider) but they're missing from the dependency array. This can cause the function to use stale provider instances.
🐛 Proposed fix
- }, [address, checkSetupStatus]);
+ }, [address, checkSetupStatus, provider]);Alternatively, if you extract wallet client creation as suggested in the previous comment, the extracted function can handle the provider dependency directly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, [address, checkSetupStatus]); | |
| }, [address, checkSetupStatus, provider]); |
| setIsLoading(false); | ||
| } | ||
| }, | ||
| [address, setupStatus, loadBalance] |
There was a problem hiding this comment.
Fix stale closure issue in dependency array.
Similar to the other functions, executeCopyTrade uses provider (lines 303, 309) but it's missing from the dependency array, creating a stale closure risk.
🐛 Proposed fix
- }, [address, setupStatus, loadBalance]);
+ }, [address, setupStatus, loadBalance, provider]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| [address, setupStatus, loadBalance] | |
| }, [address, setupStatus, loadBalance, provider]); |
🤖 Prompt for AI Agents
In @src/apps/perps/hooks/useHyperliquid.ts at line 386, The dependency array for
the executeCopyTrade callback is missing provider, causing a stale closure;
update the dependency array (currently [address, setupStatus, loadBalance]) used
by executeCopyTrade in useHyperliquid.ts to include provider so the callback
sees the latest provider reference (also verify any other values used inside
executeCopyTrade are present in the array).
| // Hot-load the cache so it's immediately available without unlocking again | ||
| cachedPrivateKey = privateKey; | ||
| resetInactivityTimer(); // Reset session timeout on wallet storage |
There was a problem hiding this comment.
Cache populated before verifying localStorage operations succeeded.
The cache is updated immediately after encryption (line 56) but before verifying that all localStorage operations completed successfully. If a localStorage operation fails later (e.g., due to quota limits), the cache would contain a key that isn't actually stored, leading to inconsistent state.
🔄 Suggested fix: Populate cache after storage confirmation
localStorage.setItem(getStorageKey(masterAddress, 'approved'), String(approved));
localStorage.setItem(getStorageKey(masterAddress, 'builder_approved'), String(builderApproved));
+ // All storage operations succeeded, now populate cache
cachedPrivateKey = privateKey;
resetInactivityTimer(); // Reset session timeout on wallet storage📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Hot-load the cache so it's immediately available without unlocking again | |
| cachedPrivateKey = privateKey; | |
| resetInactivityTimer(); // Reset session timeout on wallet storage | |
| // Hot-load the cache so it's immediately available without unlocking again | |
| // All storage operations succeeded, now populate cache | |
| cachedPrivateKey = privateKey; | |
| resetInactivityTimer(); // Reset session timeout on wallet storage |
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/keystore.ts around lines 55 - 57, The code
assigns cachedPrivateKey and calls resetInactivityTimer before confirming
localStorage persistence; move the cache population and timer reset so they
occur only after all localStorage operations (e.g., localStorage.setItem or
whatever persistence functions used in this block) complete successfully and
without throwing. Wrap storage calls in a try/catch, verify they succeeded, and
only then set cachedPrivateKey = privateKey and call resetInactivityTimer; on
storage failure roll back any partial state (don’t populate cache) and
surface/log the error.
| // Cache it | ||
| cachedImportedKey = privateKey; | ||
| resetInactivityTimer(); // Reset session timeout on imported account storage | ||
| window.dispatchEvent(new Event('imported-account-changed')); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Inconsistent event dispatching between agent wallets and imported accounts.
Imported account operations dispatch 'imported-account-changed' events (lines 214, 317), but agent wallet operations don't dispatch corresponding events. This inconsistency could cause UI components to miss agent wallet state changes.
Consider adding event dispatching to agent wallet operations for consistent state management:
// In storeAgentWalletEncrypted, after line 57
window.dispatchEvent(new CustomEvent('agent-wallet-changed', {
detail: { masterAddress }
}));
// In clearAgentWalletLocal, after line 181
window.dispatchEvent(new CustomEvent('agent-wallet-changed', {
detail: { masterAddress }
}));Also applies to: 317-317
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/keystore.ts at line 214, Imported account
flows emit 'imported-account-changed' events but agent wallet flows do not,
causing inconsistent UI updates; update the agent wallet code to dispatch a
mirror event after changes: inside storeAgentWalletEncrypted (after the
successful store operation, e.g., where the existing imported-account dispatch
would occur) dispatch window.dispatchEvent(new
CustomEvent('agent-wallet-changed', { detail: { masterAddress } })); and do the
same in clearAgentWalletLocal (after clearing the local agent wallet), using the
same event name and detail shape so UI listeners can handle agent wallet state
changes consistently with imported-account changes.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/apps/perps/components/TradeForm.tsx:
- Around line 444-523: The variable useBuilderFee is declared twice (const
useBuilderFee = signingAddress === masterAddress; and later const useBuilderFee
= !imported && (signingAddress === masterAddress)), causing the first to be dead
code; remove the earlier definition and ensure the single useBuilderFee
declaration uses the intended logic that includes the imported check
(useBuilderFee = !imported && signingAddress === masterAddress) so all later
references (logs and fee logic) use the correct value.
- Around line 651-696: The current auto-approval branch is unreachable because
useBuilderFee is defined as !imported && (signingAddress === masterAddress), yet
the handler checks imported inside that block; fix by restructuring the
conditions so the builder-fee error is detected first (the existing error check
using error?.message/response), then branch based on whether an imported account
exists: call getImportedAccount() and if imported.accountAddress matches
masterAddress call approveBuilderFeeSDK(imported.privateKey, BUILDER_ADDRESS,
BUILDER_FEE_APPROVAL) with the existing toast flows; otherwise, if
signingAddress === masterAddress and no imported account handle the original
agent/master flow (or show the Settings > Perps Account toast). Ensure you
reference and update the logic around useBuilderFee, getImportedAccount(),
signingAddress, masterAddress and keep the toast.success/error and
approveBuilderFeeSDK calls intact.
In @src/apps/perps/lib/hyperliquid/sdk.ts:
- Around line 177-190: approveAgentSDK is passing the raw masterPrivateKey into
new ExchangeClient, causing inconsistent private key handling compared to
approveBuilderFeeSDK and getExchangeClientForAgent; fix by converting the raw
key to an account with privateKeyToAccount(masterPrivateKey) and pass that
account (not the raw Hex) into ExchangeClient so initialization is consistent
with approveBuilderFeeSDK/getExchangeClientForAgent and avoids type/behavior
mismatches.
🧹 Nitpick comments (2)
src/apps/perps/lib/hyperliquid/keystore.ts (2)
254-263: Add error handling for JSON.parse in getImportedAccountAddress.Line 260 uses
JSON.parseon localStorage data without a try-catch block. If the stored data is corrupted or malformed, this will throw an exception that propagates to callers. Additionally, the code callslocalStorage.getItem(GLOBAL_ACCOUNT_KEY)twice, which is inefficient.♻️ Proposed fix
export function getImportedAccountAddress(): string | null { if (typeof window === 'undefined') return null; - // Check new key first, then legacy - return ( - localStorage.getItem(GLOBAL_IMPORTED_ADDRESS_KEY) || - (localStorage.getItem(GLOBAL_ACCOUNT_KEY) - ? JSON.parse(localStorage.getItem(GLOBAL_ACCOUNT_KEY)!).accountAddress - : null) - ); + + // Check new key first + const newAddress = localStorage.getItem(GLOBAL_IMPORTED_ADDRESS_KEY); + if (newAddress) return newAddress; + + // Fallback to legacy key + const legacyData = localStorage.getItem(GLOBAL_ACCOUNT_KEY); + if (legacyData) { + try { + const parsed = JSON.parse(legacyData); + return parsed?.accountAddress || null; + } catch (error) { + console.warn('[Keystore] Failed to parse legacy imported account data:', error); + return null; + } + } + + return null; }
32-62: Consider privacy implications of storing addresses in localStorage.Static analysis tools flag lines 45, 130, and 204 for storing addresses in localStorage, which could have privacy implications if users want to maintain anonymity. However, the current implementation is reasonable because:
- Private keys ARE properly encrypted using PIN-based encryption (lines 43-44, 202-203)
- Wallet addresses are public information on the blockchain
- Addresses are necessary for the keystore functionality
The security posture is strong overall with encrypted private keys, session timeouts, and memory cache invalidation. The address storage is a lower-priority concern.
If enhanced privacy is desired, consider:
- Encrypting addresses alongside private keys
- Using IndexedDB instead of localStorage for additional isolation
- Implementing a "privacy mode" that clears all data on session end
Also applies to: 194-219
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/perps/components/TradeForm.tsxsrc/apps/perps/lib/hyperliquid/keystore.tssrc/apps/perps/lib/hyperliquid/sdk.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/perps/components/TradeForm.tsxsrc/apps/perps/lib/hyperliquid/sdk.ts
📚 Learning: 2025-12-11T12:40:09.964Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 478
File: src/apps/pulse/components/App/AppWrapper.tsx:22-23
Timestamp: 2025-12-11T12:40:09.964Z
Learning: In the Pulse app's AppWrapper component (src/apps/pulse/components/App/AppWrapper.tsx), when users enter via `/pulse?searching=true` from PillarX home and select a token or market, they should continue into Pulse to complete their transaction, not navigate back to home. Only when closing the search without making a selection should they return to PillarX home.
Applied to files:
src/apps/perps/components/TradeForm.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/perps/lib/hyperliquid/sdk.ts
🧬 Code graph analysis (1)
src/apps/perps/lib/hyperliquid/keystore.ts (3)
src/apps/perps/lib/hyperliquid/session-manager.ts (3)
initializeSessionManager(19-22)resetInactivityTimer(28-41)clearSession(47-58)src/apps/perps/lib/hyperliquid/signing.ts (1)
generateAgentWallet(366-380)src/apps/perps/lib/encryption.ts (3)
encryptWithPin(50-72)EncryptedData(43-47)decryptWithPin(75-97)
🪛 ast-grep (0.40.4)
src/apps/perps/lib/hyperliquid/keystore.ts
[warning] 45-45: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(getStorageKey(masterAddress, 'address'), address)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
[warning] 130-130: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(getStorageKey(masterAddress, 'address'), address)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
[warning] 204-204: Sensitive information detected in localStorage. Storing sensitive data like emails, usernames, or personal information in localStorage exposes it to malicious scripts and XSS attacks. Use secure storage alternatives or avoid storing sensitive data client-side.
Context: localStorage.setItem(GLOBAL_IMPORTED_ADDRESS_KEY, address)
Note: [CWE-312] Cleartext Storage of Sensitive Information [REFERENCES]
- https://cwe.mitre.org/data/definitions/312.html
- https://owasp.org/www-community/vulnerabilities/HTML5_Local_Storage_Security
(local-storage-sensitive-information)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages: x
| // 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); | ||
| try { | ||
| // Get entry price | ||
| let entryPrice = data.entryPrice; | ||
| if (isMarketOrder || !entryPrice) { | ||
| // toast.info('Fetching market price...'); | ||
| entryPrice = await getMarkPrice(selectedAsset.symbol); | ||
| if (!entryPrice) { | ||
| throw new Error('Failed to fetch market price'); | ||
| } | ||
| } | ||
|
|
||
| // Calculate size | ||
| const size = computeSizeUSD( | ||
| data.amountUSD, | ||
| data.leverage, | ||
| entryPrice, | ||
| selectedAsset.szDecimals | ||
| ); | ||
|
|
||
| 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`, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Update leverage and margin mode before placing orders | ||
| try { | ||
| await updateLeverageAgent(privateKey as `0x${string}`, { | ||
| coinId: selectedAsset.id, | ||
| leverage: data.leverage, | ||
| isCross: data.marginMode === 'cross', | ||
| }); | ||
| console.log( | ||
| `[TradeForm] Updated leverage: ${data.leverage}x ${data.marginMode}` | ||
| ); | ||
| } catch (leverageError: any) { | ||
| console.error('[TradeForm] Failed to update leverage:', leverageError); | ||
|
|
||
| const errorMessage = leverageError.message || ''; | ||
| if (errorMessage.includes('does not exist')) { | ||
| toast.error('Account not initialized', { | ||
| id: toastId, | ||
| description: 'Please deposit funds into your Hyperliquid account first to enable trading features.', | ||
| duration: 5000, | ||
| }); | ||
| } else { | ||
| toast.error('Failed to set leverage/margin mode', { | ||
| id: toastId, | ||
| description: errorMessage || 'Please try again', | ||
| }); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| // Place entry order via SDK | ||
| // toast.info('Placing entry order...'); | ||
|
|
||
| // Determine builder fee parameters | ||
| // 1. We are NOT using an imported account (i.e. we are using the Agent) | ||
| // 2. The signing address matches the master address (Agent Flow) | ||
| // If we are using an imported account, we NEVER apply builder fees for now to avoid the "approve" loop issue. | ||
| // Even if imported address match master, we treat it as "User Trading" not "Agent Trading". | ||
| const useBuilderFee = !imported && (signingAddress === masterAddress); | ||
|
|
||
| console.log('[TradeForm] Builder Fee Debug:', { | ||
| masterAddress, | ||
| signingAddress, | ||
| useBuilderFee, | ||
| isImported: !!imported | ||
| }); |
There was a problem hiding this comment.
Duplicate useBuilderFee variable definition.
The variable useBuilderFee is defined twice:
- Line 447:
const useBuilderFee = signingAddress === masterAddress; - Line 516:
const useBuilderFee = !imported && (signingAddress === masterAddress);
The second definition shadows the first, making the first assignment at line 447 dead code. Based on the detailed comments at lines 511-516 and the more comprehensive logic that includes the !imported check, the second definition appears to be the intended one.
🐛 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);
try {
// Get entry price📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 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); | |
| try { | |
| // Get entry price | |
| let entryPrice = data.entryPrice; | |
| if (isMarketOrder || !entryPrice) { | |
| // toast.info('Fetching market price...'); | |
| entryPrice = await getMarkPrice(selectedAsset.symbol); | |
| if (!entryPrice) { | |
| throw new Error('Failed to fetch market price'); | |
| } | |
| } | |
| // Calculate size | |
| const size = computeSizeUSD( | |
| data.amountUSD, | |
| data.leverage, | |
| entryPrice, | |
| selectedAsset.szDecimals | |
| ); | |
| 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`, | |
| }); | |
| return; | |
| } | |
| // Update leverage and margin mode before placing orders | |
| try { | |
| await updateLeverageAgent(privateKey as `0x${string}`, { | |
| coinId: selectedAsset.id, | |
| leverage: data.leverage, | |
| isCross: data.marginMode === 'cross', | |
| }); | |
| console.log( | |
| `[TradeForm] Updated leverage: ${data.leverage}x ${data.marginMode}` | |
| ); | |
| } catch (leverageError: any) { | |
| console.error('[TradeForm] Failed to update leverage:', leverageError); | |
| const errorMessage = leverageError.message || ''; | |
| if (errorMessage.includes('does not exist')) { | |
| toast.error('Account not initialized', { | |
| id: toastId, | |
| description: 'Please deposit funds into your Hyperliquid account first to enable trading features.', | |
| duration: 5000, | |
| }); | |
| } else { | |
| toast.error('Failed to set leverage/margin mode', { | |
| id: toastId, | |
| description: errorMessage || 'Please try again', | |
| }); | |
| } | |
| return; | |
| } | |
| // Place entry order via SDK | |
| // toast.info('Placing entry order...'); | |
| // Determine builder fee parameters | |
| // 1. We are NOT using an imported account (i.e. we are using the Agent) | |
| // 2. The signing address matches the master address (Agent Flow) | |
| // If we are using an imported account, we NEVER apply builder fees for now to avoid the "approve" loop issue. | |
| // Even if imported address match master, we treat it as "User Trading" not "Agent Trading". | |
| const useBuilderFee = !imported && (signingAddress === masterAddress); | |
| console.log('[TradeForm] Builder Fee Debug:', { | |
| masterAddress, | |
| signingAddress, | |
| useBuilderFee, | |
| isImported: !!imported | |
| }); | |
| setIsSubmitting(true); | |
| try { | |
| // Get entry price | |
| let entryPrice = data.entryPrice; | |
| if (isMarketOrder || !entryPrice) { | |
| // toast.info('Fetching market price...'); | |
| entryPrice = await getMarkPrice(selectedAsset.symbol); | |
| if (!entryPrice) { | |
| throw new Error('Failed to fetch market price'); | |
| } | |
| } | |
| // Calculate size | |
| const size = computeSizeUSD( | |
| data.amountUSD, | |
| data.leverage, | |
| entryPrice, | |
| selectedAsset.szDecimals | |
| ); | |
| 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`, | |
| }); | |
| return; | |
| } | |
| // Update leverage and margin mode before placing orders | |
| try { | |
| await updateLeverageAgent(privateKey as `0x${string}`, { | |
| coinId: selectedAsset.id, | |
| leverage: data.leverage, | |
| isCross: data.marginMode === 'cross', | |
| }); | |
| console.log( | |
| `[TradeForm] Updated leverage: ${data.leverage}x ${data.marginMode}` | |
| ); | |
| } catch (leverageError: any) { | |
| console.error('[TradeForm] Failed to update leverage:', leverageError); | |
| const errorMessage = leverageError.message || ''; | |
| if (errorMessage.includes('does not exist')) { | |
| toast.error('Account not initialized', { | |
| id: toastId, | |
| description: 'Please deposit funds into your Hyperliquid account first to enable trading features.', | |
| duration: 5000, | |
| }); | |
| } else { | |
| toast.error('Failed to set leverage/margin mode', { | |
| id: toastId, | |
| description: errorMessage || 'Please try again', | |
| }); | |
| } | |
| return; | |
| } | |
| // Place entry order via SDK | |
| // toast.info('Placing entry order...'); | |
| // Determine builder fee parameters | |
| // 1. We are NOT using an imported account (i.e. we are using the Agent) | |
| // 2. The signing address matches the master address (Agent Flow) | |
| // If we are using an imported account, we NEVER apply builder fees for now to avoid the "approve" loop issue. | |
| // Even if imported address match master, we treat it as "User Trading" not "Agent Trading". | |
| const useBuilderFee = !imported && (signingAddress === masterAddress); | |
| console.log('[TradeForm] Builder Fee Debug:', { | |
| masterAddress, | |
| signingAddress, | |
| useBuilderFee, | |
| isImported: !!imported | |
| }); |
🤖 Prompt for AI Agents
In @src/apps/perps/components/TradeForm.tsx around lines 444 - 523, The variable
useBuilderFee is declared twice (const useBuilderFee = signingAddress ===
masterAddress; and later const useBuilderFee = !imported && (signingAddress ===
masterAddress)), causing the first to be dead code; remove the earlier
definition and ensure the single useBuilderFee declaration uses the intended
logic that includes the imported check (useBuilderFee = !imported &&
signingAddress === masterAddress) so all later references (logs and fee logic)
use the correct value.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @Builders codes/fetch_builder_data.js:
- Around line 13-36: The Promise can hang because errors emitted by the request
returned from https.get are not handled; capture the request object returned by
https.get(...) (the value currently not stored) and attach a 'error' listener on
it that resolves null (and optionally logs the error) so DNS/connection-level
failures don't leave the promise unresolved; keep the existing response error
handling on the 'res' object and ensure you only resolve once in either the
response end/error handlers or the request 'error' handler.
In @Builders codes/package.json:
- Line 5: The package.json "main" entry currently points to index.js but the
actual script added in this PR is fetch_builder_data.js; update the "main" field
to "fetch_builder_data.js" (or alternatively rename the script to index.js) so
the package's main entry matches the actual module name referenced by the code
(adjust the "main" field in package.json accordingly).
In @Builders codes/verify_referrals.js:
- Around line 22-26: The JSON.parse call inside the res.on('end') callback can
throw on malformed responses; wrap the parse and subsequent formatOutput(body)
usage in a try-catch inside the res.on('end', ...) handler, catch the error, log
a useful message including the raw body and the parse error (using console.error
or the existing logger), and avoid calling formatOutput when parsing fails
(return/exit the handler after logging) so the process doesn't crash from an
uncaught exception.
In @src/apps/perps/components/PositionsCard.tsx:
- Around line 964-983: The conditional contains dead inner else branches because
hasOrders is defined as (tps.length > 0 || sls.length > 0), so the check
tps.length === 0 && sls.length === 0 is redundant; simplify both mobile and
desktop blocks in PositionsCard by replacing the nested ternary with a single
conditional: if no orders render "-" else render the TP and SL spans when
tps.length > 0 and sls.length > 0 respectively (using existing tps, sls and
formatPrice), and remove the unreachable else branches.
- Around line 507-544: The unlock handler calls async functions
handleExecuteClose and handleCancelOrder without awaiting them, causing
setPendingAction(null) and return true to run before those actions complete;
update handleUnlock to await handleExecuteClose() and await
handleCancelOrder(pendingAction.data) where invoked, and only clear
pendingAction (setPendingAction(null)) and return true after those awaits
complete (preserve existing try/catch behavior and apply to both the imported
account and agent wallet branches).
- Line 89: The component declares state with useState<UserState | null>
(internalUserState / setInternalUserState) but UserState isn't imported, causing
a TypeScript error; fix by adding UserState to the existing import list from
../lib/hyperliquid/types (i.e., import { UserState } from
'../lib/hyperliquid/types' along with any other imported symbols) so the type
reference resolves.
🧹 Nitpick comments (13)
Builders codes/verify_referrals.js (2)
3-6: Consider parameterizing the hardcoded user address.The Ethereum address is hardcoded, limiting script reusability. Additionally, the filename
verify_referrals.jsdoesn't align with theuserFundingrequest type.♻️ Suggested improvement
+const user = process.argv[2] || "0x5e1D081488a5e746c1a13Bf92103C1B9eE5962A2"; + +if (!user) { + console.error('Usage: node verify_referrals.js <user_address>'); + process.exit(1); +} + const data = JSON.stringify({ type: "userFunding", - user: "0x5e1D081488a5e746c1a13Bf92103C1B9eE5962A2" + user: user });
19-27: Add HTTP status code validation before processing the response.The script processes the response body regardless of HTTP status. On API errors (4xx/5xx), this could lead to confusing output or parse failures.
♻️ Suggested improvement
const req = https.request(options, (res) => { let body = ''; res.on('data', (chunk) => body += chunk); res.on('end', () => { + if (res.statusCode < 200 || res.statusCode >= 300) { + console.error(`Request failed with status ${res.statusCode}`); + console.error('Response:', body); + return; + } try { const parsed = JSON.parse(body); console.log(formatOutput(parsed)); } catch (e) { console.error('Failed to parse response:', e.message); console.error('Raw response:', body); } }); });Builders codes/fetch_builder_data.js (5)
1-3: Remove unusedfsimport.The
fsmodule is imported but never used in this script.Proposed fix
const https = require('https'); -const fs = require('fs'); const lz4js = require('lz4js');
39-52: Naive CSV parser may fail on quoted fields.The parser splits on commas without handling quoted fields. If any CSV field contains commas (e.g., in user addresses or notes), parsing will break. Consider using a CSV parsing library like
csv-parsefor robustness, or document that the input format is guaranteed simple.
99-111: Avoid parsing CSV twice; combine with first pass.The CSV is parsed once in lines 73-93 for daily stats, then again here for user breakdown. This redundant parsing can be eliminated by building
userBreakdownduring the first iteration.Proposed fix — compute userBreakdown in the first pass
let totalFees = 0; const uniqueUsers = new Set(); const dailyStats = {}; + const userBreakdown = {}; results.forEach(({ date, csv }) => { const records = parseCSV(csv); let dailyFees = 0; let dailyUsers = new Set(); records.forEach(r => { const fee = parseFloat(r.builder_fee || 0); dailyFees += fee; totalFees += fee; if (r.user) { uniqueUsers.add(r.user); dailyUsers.add(r.user); + userBreakdown[r.user] = (userBreakdown[r.user] || 0) + fee; } }); dailyStats[date] = { fees: dailyFees, users: dailyUsers.size, transactions: records.length }; }); console.table(dailyStats); console.log(`\nTotal Builder Fees: ${totalFees.toFixed(6)} USDC`); console.log(`Total Unique Users: ${uniqueUsers.size}`); console.log("\nUser Breakdown:"); - const userBreakdown = {}; - results.forEach(({ csv }) => { - const records = parseCSV(csv); - records.forEach(r => { - if (r.user) { - if (!userBreakdown[r.user]) { - userBreakdown[r.user] = 0; - } - userBreakdown[r.user] += parseFloat(r.builder_fee || 0); - } - }); - }); - Object.entries(userBreakdown).forEach(([user, fee]) => {
54-61: Consider parallel fetching for performance.The dates are fetched sequentially with
awaitin a loop. Since these requests are independent, usingPromise.allwould improve performance.Proposed parallel fetch
async function main() { - const results = []; - for (const date of dates) { - const result = await fetchAndProcess(date); - if (result) { - results.push(result); - } - } + const fetched = await Promise.all(dates.map(date => fetchAndProcess(date))); + const results = fetched.filter(Boolean);
118-118: Add error handling for the main() invocation.If
main()throws an unhandled rejection, the process may exit silently or with a cryptic error. Adding.catch()ensures errors are surfaced.Proposed fix
-main(); +main().catch(err => { + console.error('Fatal error:', err); + process.exit(1); +});Builders codes/package.json (1)
12-14: Consider pinninglz4jsto exact version0.2.0for reproducible builds.The current caret range
^0.2.0effectively resolves to version0.2.0only (the latest available), but explicitly pinning the version would clarify intent for reproducible builds. Change to"lz4js": "0.2.0"to ensure builds are fully deterministic.src/apps/perps/components/PositionsCard.tsx (5)
49-55: Remove unused imports.
getMarkPrice,getOpenOrders, andgetUserFillsare imported but never used in this component. OnlygetFrontendOpenOrdersandgetMetaAndAssetCtxsare actually called.🧹 Proposed cleanup
import { - getMarkPrice, - getOpenOrders, getFrontendOpenOrders, getMetaAndAssetCtxs, - getUserFills, } from '../lib/hyperliquid/client';
281-353: Close position handler has solid wallet resolution and error handling.The priority flow (imported account → agent wallet → unlock modal) is correctly implemented. A few observations:
- Line 325: Hardcoded 6 decimal places. Consider using the asset's
szDecimalsfrom universe metadata for precision:- const sizeStr = sizeToClose.toFixed(6); + const decimals = universe[coinIndex]?.szDecimals ?? 6; + const sizeStr = sizeToClose.toFixed(decimals);
- Line 294:
getAgentWalletappears synchronous based on the relevant snippets, soawaitis unnecessary (though harmless).
632-641: Loading skeleton shown during refresh may cause UI flicker.When
positions.length > 0andisLoadingis true (during a refresh), this shows skeleton placeholders instead of the existing positions. This creates a jarring UX where data disappears during refresh.Consider showing existing positions with a subtle loading indicator instead:
♻️ Proposed approach
- {positions.length === 0 ? ( + {isLoading && positions.length === 0 ? ( + <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 &&
819-842: Use unique identifier for list keys instead of array index.Using array
indexas the React key can cause reconciliation issues if positions reorder. Sinceposition.coinis guaranteed unique (per the deduplication logic at lines 166-170), use it as the key.♻️ Proposed fix
- {positions.map((position, index) => { + {positions.map((position) => { // ... existing code ... + const positionKey = position.coin; if (isMobile) { return ( <div - key={index} + key={positionKey} onClick={() => { handlePositionClick(position.coin); - setExpandedPositionIndex(index); + setExpandedPositionIndex(positions.findIndex(p => p.coin === position.coin)); }}Note: If you want to keep using index for
setExpandedPositionIndex, you can keep the index parameter but useposition.coinfor the key.Also applies to: 991-998
831-831: Unused variableleverage.The variable
leverageis calculated but never used in the desktop position row rendering. Line 1012 callscalculateLeverage(position)instead.🧹 Remove unused variable
const positionValue = Math.abs(parseFloat(position.szi)) * parseFloat(position.markPx || '0'); const marginUsed = parseFloat(position.marginUsed || '0'); - const leverage = position.leverage?.value || 0;
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
Builders codes/node_modules/.package-lock.jsonis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/.eslintrcis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/.npmignoreis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/.travis.ymlis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/README.mdis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/lz4.jsis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/package.jsonis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/test/cases/lz4.jsis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/test/cases/xxh32.jsis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/util.jsis excluded by!**/node_modules/**Builders codes/node_modules/lz4js/xxh32.jsis excluded by!**/node_modules/**Builders codes/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
Builders codes/fetch_builder_data.jsBuilders codes/package.jsonBuilders codes/verify_referrals.jssrc/apps/perps/components/PositionsCard.tsx
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-04-23T15:04:20.826Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 290
File: src/apps/pillarx-app/components/TileTitle/TitleTitle.tsx:6-10
Timestamp: 2025-04-23T15:04:20.826Z
Learning: In this repository, TileTitleProps and TileTitle are different types that serve different purposes. TileTitleProps is used for the TileTitle component and has optional fields (title?, leftDecorator?, rightDecorator?), while TileTitle in api.ts has a required text field. The TileTitleProps interface aligns with the TokensMarketData.title type in api.ts which also has optional fields.
Applied to files:
src/apps/perps/components/PositionsCard.tsx
📚 Learning: 2025-11-21T13:10:33.422Z
Learnt from: aldin4u
Repo: pillarwallet/x PR: 461
File: src/apps/pulse/components/Search/MarketList.tsx:10-15
Timestamp: 2025-11-21T13:10:33.422Z
Learning: In the Pulse app's MarketList component (src/apps/pulse/components/Search/MarketList.tsx), markets should display liquidity (not price) in the right-hand column. This is per the design specification.
Applied to files:
src/apps/perps/components/PositionsCard.tsx
📚 Learning: 2025-08-20T14:01:31.715Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 387
File: src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx:118-125
Timestamp: 2025-08-20T14:01:31.715Z
Learning: In src/apps/leaderboard/components/LeaderboardTab/LeaderboardTab.tsx, the "My rank" calculation uses 'trading' context while the list uses 'migration' context when calling calculateFinalPxPoints. This is intentional design to control where the 200-point bonus is applied - the 'trading' context with 'all' timeTab bypasses the bonus while 'migration' context allows it, preventing points from being added in the wrong place.
Applied to files:
src/apps/perps/components/PositionsCard.tsx
📚 Learning: 2026-01-07T12:27:46.789Z
Learnt from: vignesha22
Repo: pillarwallet/x PR: 486
File: src/apps/pulse/components/Onboarding/TopUpScreen.tsx:1005-1049
Timestamp: 2026-01-07T12:27:46.789Z
Learning: In the Pulse app TopUpScreen gasless flow (src/apps/pulse/components/Onboarding/TopUpScreen.tsx), when the Gasless Fee Estimate UI is displayed, the fee token (selectedFeeAsset) is always the same as the topup token (selectedToken) because selectedFeeAsset is only set when a matching paymaster is found where gasToken matches selectedToken.address (lines 377-391).
Applied to files:
src/apps/perps/components/PositionsCard.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/perps/components/PositionsCard.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.
Applied to files:
src/apps/perps/components/PositionsCard.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/perps/components/PositionsCard.tsx
🧬 Code graph analysis (3)
Builders codes/verify_referrals.js (1)
Builders codes/fetch_builder_data.js (2)
https(1-1)data(42-42)
Builders codes/fetch_builder_data.js (1)
Builders codes/verify_referrals.js (2)
https(1-1)data(3-6)
src/apps/perps/components/PositionsCard.tsx (5)
src/apps/perps/lib/hyperliquid/types.ts (3)
HyperliquidPosition(88-103)HyperliquidOrder(106-125)UniverseAsset(128-133)src/apps/perps/hooks/use-mobile.tsx (1)
useIsMobile(5-21)src/apps/perps/lib/hyperliquid/keystore.ts (6)
getImportedAccount(274-309)getAgentWallet(358-384)isImportedAccountEncrypted(249-252)isAgentWalletEncrypted(110-113)unlockImportedAccount(221-247)unlockAgentWallet(64-108)src/apps/perps/lib/hyperliquid/sdk.ts (2)
placeMarketOrderAgent(17-57)cancelOrderAgent(149-172)src/apps/perps/components/UnlockWalletModal.tsx (1)
UnlockWalletModal(25-133)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
- GitHub Check: Cloudflare Pages: x
🔇 Additional comments (4)
Builders codes/verify_referrals.js (1)
29-31: LGTM!The
formatOutputhelper is clean and straightforward.src/apps/perps/components/PositionsCard.tsx (3)
103-218: Data fetching logic is well-structured with appropriate fallbacks.The
fetchDatacallback properly handles multiple data sources (external props vs internal fetch), builds a price map, and enriches positions with mark prices and universe metadata. The fallback logic for missing universe metadata (lines 184-193) is a good defensive measure.One minor observation:
internalUserStateis both a dependency and set within the callback. This is safe here since it's only read at line 151 as a fallback whenuserStateprop is not provided, but consider if this could cause unnecessary re-fetches.
546-598: Dialog and modal setup looks good.The
UnlockWalletModaland close positionDialogare properly integrated with appropriate state management. The slider for close percentage with preset buttons (25%, 50%, 75%, 100%) provides good UX.
438-504: TP/SL aggregation helper is well-implemented.The
getOpenTP_SLfunction correctly:
- Filters reduce-only orders for the position
- Determines order direction based on position side
- Classifies orders as TP or SL using
orderTypewith price-based fallback- Returns sorted arrays
The fallback classification logic (lines 478-496) handles edge cases where
orderTypeis not available.
| return new Promise((resolve) => { | ||
| https.get(url, (res) => { | ||
| if (res.statusCode !== 200) { | ||
| // console.log(`No data for ${date} (Status: ${res.statusCode})`); | ||
| resolve(null); | ||
| return; | ||
| } | ||
|
|
||
| const chunks = []; | ||
| res.on('data', chunk => chunks.push(chunk)); | ||
| res.on('end', () => { | ||
| const buffer = Buffer.concat(chunks); | ||
| try { | ||
| const decompressed = lz4js.decompress(buffer); | ||
| const csv = Buffer.from(decompressed).toString('utf8'); | ||
| resolve({ date, csv }); | ||
| } catch (e) { | ||
| console.error(`Error decompressing ${date}:`, e.message); | ||
| resolve(null); | ||
| } | ||
| }); | ||
| res.on('error', () => resolve(null)); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing error handler on the request object.
The https.get call returns a request object that can emit errors (e.g., DNS failures, connection refused). Currently only response-level errors are handled, but request-level errors would leave the promise hanging.
Proposed fix
return new Promise((resolve) => {
- https.get(url, (res) => {
+ const req = https.get(url, (res) => {
if (res.statusCode !== 200) {
// console.log(`No data for ${date} (Status: ${res.statusCode})`);
resolve(null);
return;
}
const chunks = [];
res.on('data', chunk => chunks.push(chunk));
res.on('end', () => {
const buffer = Buffer.concat(chunks);
try {
const decompressed = lz4js.decompress(buffer);
const csv = Buffer.from(decompressed).toString('utf8');
resolve({ date, csv });
} catch (e) {
console.error(`Error decompressing ${date}:`, e.message);
resolve(null);
}
});
res.on('error', () => resolve(null));
});
+ req.on('error', () => resolve(null));
});🤖 Prompt for AI Agents
In @Builders codes/fetch_builder_data.js around lines 13 - 36, The Promise can
hang because errors emitted by the request returned from https.get are not
handled; capture the request object returned by https.get(...) (the value
currently not stored) and attach a 'error' listener on it that resolves null
(and optionally logs the error) so DNS/connection-level failures don't leave the
promise unresolved; keep the existing response error handling on the 'res'
object and ensure you only resolve once in either the response end/error
handlers or the request 'error' handler.
| "name": "builder-codes-fetcher", | ||
| "version": "1.0.0", | ||
| "description": "Fetch builder codes", | ||
| "main": "index.js", |
There was a problem hiding this comment.
Main entry doesn't match actual script filename.
The main entry is set to index.js, but the actual script in this PR is fetch_builder_data.js. Update to match:
Proposed fix
- "main": "index.js",
+ "main": "fetch_builder_data.js",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "main": "index.js", | |
| "main": "fetch_builder_data.js", |
🤖 Prompt for AI Agents
In @Builders codes/package.json at line 5, The package.json "main" entry
currently points to index.js but the actual script added in this PR is
fetch_builder_data.js; update the "main" field to "fetch_builder_data.js" (or
alternatively rename the script to index.js) so the package's main entry matches
the actual module name referenced by the code (adjust the "main" field in
package.json accordingly).
| res.on('end', () => { | ||
| // console.log(body); | ||
| const parsed = JSON.parse(body); | ||
| console.log(formatOutput(parsed)); | ||
| }); |
There was a problem hiding this comment.
Wrap JSON.parse in a try-catch to prevent uncaught exceptions.
If the API returns malformed JSON or an unexpected response, the script will crash with an unhandled exception. The request error handler (line 33) won't catch JSON parse errors.
🐛 Proposed fix
res.on('end', () => {
- // console.log(body);
- const parsed = JSON.parse(body);
- console.log(formatOutput(parsed));
+ try {
+ const parsed = JSON.parse(body);
+ console.log(formatOutput(parsed));
+ } catch (e) {
+ console.error('Failed to parse response:', e.message);
+ console.error('Raw response:', body);
+ }
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| res.on('end', () => { | |
| // console.log(body); | |
| const parsed = JSON.parse(body); | |
| console.log(formatOutput(parsed)); | |
| }); | |
| res.on('end', () => { | |
| try { | |
| const parsed = JSON.parse(body); | |
| console.log(formatOutput(parsed)); | |
| } catch (e) { | |
| console.error('Failed to parse response:', e.message); | |
| console.error('Raw response:', body); | |
| } | |
| }); |
🤖 Prompt for AI Agents
In @Builders codes/verify_referrals.js around lines 22 - 26, The JSON.parse call
inside the res.on('end') callback can throw on malformed responses; wrap the
parse and subsequent formatOutput(body) usage in a try-catch inside the
res.on('end', ...) handler, catch the error, log a useful message including the
raw body and the parse error (using console.error or the existing logger), and
avoid calling formatOutput when parsing fails (return/exit the handler after
logging) so the process doesn't crash from an uncaught exception.
| const [closeDialogOpen, setCloseDialogOpen] = useState(false); | ||
| const [universe, setUniverse] = useState<UniverseAsset[]>([]); | ||
| const [openOrders, setOpenOrders] = useState<HyperliquidOrder[]>([]); | ||
| const [internalUserState, setInternalUserState] = useState<UserState | null>(null); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check if the file exists and view the imports and line 89
cat -n src/apps/perps/components/PositionsCard.tsx | head -120Repository: pillarwallet/x
Length of output: 4748
🏁 Script executed:
# Check if UserState is exported from the types file
rg -n "export.*UserState" --type=tsRepository: pillarwallet/x
Length of output: 254
🏁 Script executed:
# Search for all imports in PositionsCard.tsx
rg -n "^import" src/apps/perps/components/PositionsCard.tsxRepository: pillarwallet/x
Length of output: 845
UserState type is used but not imported, causing a TypeScript compilation error.
At line 89, useState<UserState | null>(null) references UserState, but the type is not imported. The type is exported from ../lib/hyperliquid/types and should be added to the existing import statement.
🐛 Proposed fix
import type {
HyperliquidPosition,
HyperliquidOrder,
UniverseAsset,
+ UserState,
} from '../lib/hyperliquid/types';🤖 Prompt for AI Agents
In @src/apps/perps/components/PositionsCard.tsx at line 89, The component
declares state with useState<UserState | null> (internalUserState /
setInternalUserState) but UserState isn't imported, causing a TypeScript error;
fix by adding UserState to the existing import list from
../lib/hyperliquid/types (i.e., import { UserState } from
'../lib/hyperliquid/types' along with any other imported symbols) so the type
reference resolves.
| const handleUnlock = async (pin: string): Promise<boolean> => { | ||
| try { | ||
| // Try unlocking imported account first | ||
| if (isImportedAccountEncrypted()) { | ||
| const unlocked = await unlockImportedAccount(pin); | ||
| if (unlocked) { | ||
| setShowUnlockModal(false); | ||
| // Retry pending action | ||
| if (pendingAction?.type === 'close') { | ||
| handleExecuteClose(); | ||
| } else if (pendingAction?.type === 'cancel' && pendingAction.data) { | ||
| handleCancelOrder(pendingAction.data); | ||
| } | ||
| setPendingAction(null); | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| // Try unlocking agent wallet | ||
| if (isAgentWalletEncrypted(masterAddress)) { | ||
| const unlocked = await unlockAgentWallet(masterAddress, pin); | ||
| if (unlocked) { | ||
| setShowUnlockModal(false); | ||
| // Retry pending action | ||
| if (pendingAction?.type === 'close') { | ||
| handleExecuteClose(); | ||
| } else if (pendingAction?.type === 'cancel' && pendingAction.data) { | ||
| handleCancelOrder(pendingAction.data); | ||
| } | ||
| setPendingAction(null); | ||
| return true; | ||
| } | ||
| } | ||
| } catch (e) { | ||
| console.error('Unlock failed', e); | ||
| } | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
Missing await on async function calls may cause race conditions.
handleExecuteClose and handleCancelOrder are async functions, but they're called without await. This means setPendingAction(null) and return true execute before the action completes, potentially causing UI state inconsistencies.
🔧 Proposed fix
const handleUnlock = async (pin: string): Promise<boolean> => {
try {
// Try unlocking imported account first
if (isImportedAccountEncrypted()) {
const unlocked = await unlockImportedAccount(pin);
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;
}
}
// Try unlocking agent wallet
if (isAgentWalletEncrypted(masterAddress)) {
const unlocked = await unlockAgentWallet(masterAddress, pin);
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;
}
}🤖 Prompt for AI Agents
In @src/apps/perps/components/PositionsCard.tsx around lines 507 - 544, The
unlock handler calls async functions handleExecuteClose and handleCancelOrder
without awaiting them, causing setPendingAction(null) and return true to run
before those actions complete; update handleUnlock to await handleExecuteClose()
and await handleCancelOrder(pendingAction.data) where invoked, and only clear
pendingAction (setPendingAction(null)) and return true after those awaits
complete (preserve existing try/catch behavior and apply to both the imported
account and agent wallet branches).
| ) : ( | ||
| <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> | ||
| )} |
There was a problem hiding this comment.
Dead code in TP/SL fallback branches.
The else branches at lines 968-981 and 1068-1081 are unreachable. Since hasOrders = tps.length > 0 || sls.length > 0, when hasOrders is false, tps.length === 0 && sls.length === 0 is always true, making the inner else branch dead code.
🧹 Simplified logic for mobile (lines 964-984)
)}
</span>
) : (
- <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>
)}🧹 Simplified logic for desktop (lines 1064-1083)
) : (
- <div className="flex flex-col items-end text-xs leading-tight">
- {tps.length === 0 && sls.length === 0 ? (
- <span className="text-muted-foreground">-</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>
- )}
- </>
- )}
- </div>
+ <span className="text-muted-foreground">-</span>
)}Also applies to: 1064-1083
🤖 Prompt for AI Agents
In @src/apps/perps/components/PositionsCard.tsx around lines 964 - 983, The
conditional contains dead inner else branches because hasOrders is defined as
(tps.length > 0 || sls.length > 0), so the check tps.length === 0 && sls.length
=== 0 is redundant; simplify both mobile and desktop blocks in PositionsCard by
replacing the nested ternary with a single conditional: if no orders render "-"
else render the TP and SL spans when tps.length > 0 and sls.length > 0
respectively (using existing tps, sls and formatPrice), and remove the
unreachable else branches.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.npmrc:
- Around line 6-7: The comment above the git-tag-version=false setting is
misleading; change it to accurately state its effect (it disables automatic git
commit/tag creation during npm version bumps). Update the comment that precedes
git-tag-version=false to something like "Disable automatic git commit and tag
creation during npm version bumps" so it correctly documents the purpose of the
git-tag-version=false config.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.npmrc
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: x
🔇 Additional comments (1)
.npmrc (1)
1-4: Clarify the purpose of these settings—they don't disable git operations.These settings don't "disable git operations during npm install." The project uses
patch-packagewith a postinstall hook that requires git to track and apply patches. The actual issue was thatpatch-packagewas failing in the Cloudflare Pages CI environment due to git permission errors.The fix works like this:
unsafe-perm=trueallows the postinstall script to run with elevated privileges, which resolves git permission issues in containerized CI environments. This is the key setting that fixes the stated problem.ignore-scripts=falseis npm's default behavior and is redundant unless overriding a parent configuration.git-tag-version=falseonly affects thenpm versioncommand and doesn't prevent git operations during install.Security consideration:
unsafe-perm=trueallows lifecycle scripts from dependencies to run with elevated privileges. While necessary for this fix, ensure you understand the risk—malicious postinstall scripts could execute with elevated permissions. This is a standard tradeoff in CI environments, but warrants awareness.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Configure git to use system config only | ||
| git-tag-version=false |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
git-tag-version npm config setting documentation
💡 Result:
Summary: The npm config key git-tag-version (boolean, default: true) controls whether npm version creates a git commit and tag. Set it per-project in .npmrc or globally with npm config set.
Examples:
- Disable globally: npm config set git-tag-version false. [1][2]
- Disable per-repo: add git-tag-version=false to the repository's .npmrc. [1][2]
References:
- npm docs (npm version / config: git-tag-version). [1]
- Example / discussion (Stack Overflow). [2]
🌐 Web query:
npm install git config errors CI/CD environment solutions
💡 Result:
Here are concise, practical fixes for common "npm install" + git/config errors in CI/CD, with quick actionable commands and references.
- Authentication to private Git repos or registries
- Use CI secrets + project .npmrc: add
//registry.npmjs.org/:_authToken=${NPM_TOKEN}and set NPM_TOKEN in CI secrets. [1] - For Git-hosted deps, prefer git+https with a token or use SSH keys (deploy key) + ssh-agent in the runner. Example:
"pkg": "git+https://<user>:<token>@gitlab.com/group/repo.git#tag"or"git+ssh://[email protected]/group/repo.git#tag". [2][3]
- Ensure git auth scheme used by CI (ssh vs https)
- Rewrite URLs in CI to match available auth: set git config replacement so SSH URLs use HTTPS with CI token:
git config --global url."https://gitlab-ci-token:[TOKEN]@gitlab.com/".insteadOf "ssh://[email protected]/"
This lets npm fetch private git deps in pipelines. [4][5]
- SSH keys/agent in CI
- Install private key as secret, start ssh-agent, add key, and ensure known_hosts contains host (or use StrictHostKeyChecking=no carefully). Many CI images need ssh-agent setup before npm install. [3]
- npm registry 401 / token scope & expiry
- Use granular tokens with correct scopes for private registries (read:packages or appropriate scopes). Store token as secret and use project .npmrc with ${ENV_VAR}. Rotate tokens per provider rules. [1][6]
- git config user/email & permission scope errors
- If build steps run as root (sudo), git config may be different. Set required git config in the pipeline before npm install:
git config --global user.name "CI Bot"
git config --global user.email "[email protected]"
Also checkgit config --global url."https://github.com/".insteadof "ssh://git@github.com/"when needed. [5]
- npm client / environment issues
- Avoid running npm install with sudo; ensure correct user has access to SSH keys and .gitconfig.
- Upgrade npm if you hit known bugs (some git install bugs fixed in newer npm versions). [5]
- Debugging tips (quick)
- Print effective .npmrc and git config in CI job (mask tokens) to verify injected values.
- Try cloning the dependency repo manually inside the CI job to isolate git vs npm issues.
- Check CI provider docs for built‑in token variables (e.g., CI_JOB_TOKEN) and any known registry bugs. [1][6]
References
- Use private packages in CI/CD (npm docs). [1]
- Installing private GitHub/GitLab deps (Stack Overflow / advice for git+https & tokens). [2][7]
- GitLab CI: ssh vs https auth for git dependencies (GitLab issue / examples using git config rewrite). [3][4]
- Stack Overflow: git config insteadOf and npm/git install fixes. [5]
- CircleCI / provider notes on 401 and token policies (token scope/rotation). [6]
If you want, tell me: (a) your CI provider (GitHub Actions / GitLab / CircleCI / Jenkins / other), (b) whether you prefer SSH or token-based HTTPS, and I’ll give you an exact pipeline snippet to fix it.
🌐 Web query:
npm config settings disable git operations during install
💡 Result:
Short answer — two practical ways:
- Force offline (no network/git calls):
- npm install --offline
- or npm config set offline true
(uses npm’s offline mode which prevents network requests). [1]
- Prevent npm from invoking the git binary:
- npm config set git /bin/false (on Windows set to a nonexistent path or a harmless executable)
This makes npm’s configured git command fail, so git-based installs will not run. Note: packages that specify git URLs will then fail to install. [2]
Sources:
[1] npm config docs — offline/prefer-offline.
[2] npm config docs — git (command to use for git operations).
Update the .npmrc comment to accurately reflect git-tag-version=false.
The comment "Configure git to use system config only" is misleading. The git-tag-version=false setting only prevents npm from creating a git commit and tag during the npm version command. It does not affect npm install operations or git configuration usage.
If this was added to address git config errors during npm install (as suggested in the PR objectives), this setting won't help. Update the comment to clarify the actual purpose: "Disable automatic git tag creation during version bumps" or similar.
🤖 Prompt for AI Agents
In @.npmrc around lines 6 - 7, The comment above the git-tag-version=false
setting is misleading; change it to accurately state its effect (it disables
automatic git commit/tag creation during npm version bumps). Update the comment
that precedes git-tag-version=false to something like "Disable automatic git
commit and tag creation during npm version bumps" so it correctly documents the
purpose of the git-tag-version=false config.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@package.json`:
- Line 99: The project includes "next-themes" in package.json but no
ThemeProvider is configured while useTheme() is called in the Sonner component;
either remove the dependency and all uses/imports of useTheme from
src/apps/perps/components/ui/sonner.tsx, or add the missing ThemeProvider
initialization by wrapping your app root with ThemeProvider from "next-themes"
(e.g., in your App or RootLayout component) so useTheme() receives a defined
theme; update imports accordingly and remove unused package if you choose the
first option.
♻️ Duplicate comments (1)
package.json (1)
32-32: Dependency updated as previously suggested.The
@nktkas/hyperliquidpackage has been updated to ^0.30.2 as recommended in the prior review. This addresses the patch update.
🧹 Nitpick comments (1)
package.json (1)
8-8: Build memory increase is reasonable, but single-quote syntax may fail on Windows.The 8GB heap limit is appropriate for the expanded codebase. However, the single-quoted
NODE_OPTIONS='--max-old-space-size=8192'syntax is POSIX-specific and will fail on Windows CMD/PowerShell.If Windows development/CI environments are used, consider using
cross-envor a platform-agnostic approach:♻️ Suggested cross-platform fix
- "build": "NODE_OPTIONS='--max-old-space-size=8192' VITE_COMMIT_SHA=$CF_PAGES_COMMIT_SHA VITE_VERSION=$npm_package_version vite build", + "build": "cross-env NODE_OPTIONS=--max-old-space-size=8192 VITE_COMMIT_SHA=$CF_PAGES_COMMIT_SHA VITE_VERSION=$npm_package_version vite build",Add
cross-envto devDependencies:+ "cross-env": "^7.0.3",If the project only targets Unix-like CI (e.g., Cloudflare Pages), this can be ignored.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
- GitHub Check: Cloudflare Pages: x
🔇 Additional comments (4)
package.json (4)
26-26: LGTM —@hookform/resolversis compatible with the installedreact-hook-formversion.Version ^5.2.2 of
@hookform/resolversrequiresreact-hook-formv7.x, which is satisfied by ^7.67.0 at line 111. This supports the new form validation flows in TradeForm and related components.
93-93: LGTM —lightweight-chartsaddition is appropriate.Version ^5.1.0 is a well-maintained charting library suitable for the SparklineChart and trading UI components introduced in this PR.
134-135: LGTM —zodaddition supports schema validation for trading forms.Version ^3.25.76 is appropriate for use with
@hookform/resolversand the TradeForm validation flows. Note that zod v4.x is available with breaking changes — staying on v3.x is reasonable for stability.
99-100: Removepatch-packagefrom dependencies — it's no longer in use.The
patches/directory does not exist and thepostinstallscript has been removed. Sincepatch-packageis no longer applied during installation, it should be removed fromdevDependenciesat line 100 to keep dependencies clean.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "luxon": "3.5.0", | ||
| "mime": "4.0.4", | ||
| "moment": "2.30.1", | ||
| "next-themes": "^0.4.6", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how next-themes is used in the codebase
echo "=== Searching for next-themes usage ==="
rg -n "next-themes" --type ts --type tsx -C 3 2>/dev/null || rg -n "next-themes" -g "*.ts" -g "*.tsx" -C 3
echo ""
echo "=== Checking ThemeProvider setup ==="
rg -n "ThemeProvider" -g "*.tsx" -g "*.ts" -C 5Repository: pillarwallet/x
Length of output: 32303
🏁 Script executed:
cat -n src/apps/perps/components/ui/sonner.tsxRepository: pillarwallet/x
Length of output: 1161
🏁 Script executed:
# Search for next-themes ThemeProvider setup
rg "ThemeProvider.*next-themes" -g "*.ts" -g "*.tsx"
rg "from.*next-themes" -g "*.ts" -g "*.tsx"Repository: pillarwallet/x
Length of output: 137
Remove unused next-themes or add required ThemeProvider initialization.
next-themes is imported and useTheme() is called in src/apps/perps/components/ui/sonner.tsx, but no ThemeProvider from next-themes is configured anywhere in the codebase. The useTheme() hook requires a ThemeProvider wrapper to function; without it, the theme will be undefined. Either remove the next-themes dependency (since the project uses styled-components for theming) or add the required ThemeProvider initialization.
🤖 Prompt for AI Agents
In `@package.json` at line 99, The project includes "next-themes" in package.json
but no ThemeProvider is configured while useTheme() is called in the Sonner
component; either remove the dependency and all uses/imports of useTheme from
src/apps/perps/components/ui/sonner.tsx, or add the missing ThemeProvider
initialization by wrapping your app root with ThemeProvider from "next-themes"
(e.g., in your App or RootLayout component) so useTheme() receives a defined
theme; update imports accordingly and remove unused package if you choose the
first option.
- Added EIP-712 signing functions for withdraw3 (signWithdraw3Action, signWithdraw3AgentAction) - Updated WithdrawModal to use master wallet (walletClient) instead of agent wallet - Added walletClient prop flow: index.tsx -> BalanceCard -> WithdrawModal - Fixed keystore.ts runtime safety guards for SSR environments - Updated UI to show Arbitrum network, ~5 min transfer time, and 1 USDC fee
- Fixes ERR_REQUIRE_ESM error in Cloudflare Pages build - Vite requires ES module support in package.json
- Remove unused useState import from CompactHeader - Add production safeguards to keystore localStorage functions - Fix EIP-712 domain name from 'USD Coin' to 'USDC' in bridge - Fix form field context check to properly detect missing provider - Remove duplicate roundToSzDecimals function, import from math.ts - Fix carousel memory leak by adding reInit listener cleanup - Add defensive checks for OTP context and index bounds
- Add warning and success color tokens - Add accordion-down and accordion-up keyframes and animations - Add gradient-primary utility class for StatusBanner
- Import ChevronRight from lucide-react instead of duplicate component - Use stable market.coin as key instead of array index - Remove duplicate ChevronRight component implementation
- Convert postcss.config.js and tailwind.config.js to ESM (export default) - Update vite.config.js to define __dirname for ESM - Fix sidebar.tsx cookie access for SSR
There was a problem hiding this comment.
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 usingid.
itemContext.idis used without a guard, so usinguseFormFieldoutside<FormItem>yieldsundefined-*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:revealModetype 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: Useviem.parseSignatureand normalizevfor 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-timeisTestnetevaluation if it reads localStorage.If
isTestnettoucheswindow/localStorageat 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: Handle0andNaNcorrectly 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 validateuniversearray.🔧 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.
initializeSessionManageris 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 toHex.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.
cachedPrivateKeyis set (line 56) before alllocalStorage.setItemcalls 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:cachedPrivateKeyis not keyed bymasterAddress.If a user unlocks wallet A, then calls
getAgentWallet(masterB), the function returnscachedPrivateKey(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: DuplicategenerateAgentWalletimplementation.This function duplicates the one in
keystore.ts(lines 16-23). The keystore version usesgeneratePrivateKey()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
vvalue (line 89) is used as-is. Depending on the signer,vmay 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 formatThis applies to all signature parsing locations in the file.
38-76:actionPayloadandactionHashare computed but never used.The
actionPayload(lines 39-43) andactionHash(lines 66-76) variables are computed but never referenced. More critically, the signature only covers a staticphantomAgentwith hardcoded values, not the actualactionandnonceparameters. 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
connectionIdbefore signing.
401-433:actionandnonceparameters are unused insignAgentAction.The function accepts
action: HyperliquidActionandnonce: numberbut 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: InconsistentconnectionIdderivation between signing functions.
signUserActionuseshashTypedData()(lines 48-62) whilesignAgentActionuseskeccak256(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 againstNaNand division by zero in PnL calculations.
parseFloatcan returnNaNfor malformed strings, andaccountEquity - totalPnlcan be zero or negative, causingInfinityorNaNinpnlPercent.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: Addaria-labelfor 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, whenhasOrdersis false,tps.length === 0 && sls.length === 0is 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: 4andmaxLeverage: 50could lead to incorrect decimal precision or wrong leverage limits. Consider showing a warning indicator in the UI when using fallback values.
90-90: MissingUserStatetype import causes TypeScript compilation error.Line 90 uses
useState<UserState | null>butUserStateis 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 === 0beforeisLoading. 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
isLoadingbeforeopenOrders.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:internalUserStateinfetchDatadependency array.
fetchDatasetsinternalUserStateat line 128, butinternalUserStateis 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]);
internalUserStateis only used as a fallback and should not trigger data fetching.
325-328: Hardcoded.toFixed(6)ignores asset's actualszDecimals.Some assets may have different precision requirements (e.g., BTC uses 4, smaller tokens may use 0-2). The position is enriched with
coinInfo.szDecimalsat 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: Missingawaiton async function calls may cause race conditions.
handleExecuteCloseandhandleCancelOrderare async but called withoutawaitat lines 518, 520, 534, 536. This meanssetPendingAction(null)andreturn trueexecute 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
placeMarketOrderAgentthrows 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 20src/apps/perps/index.tsx (4)
163-165: UsePromise.allSettledto prevent silent failures.If either
loadBalance()orfetchImportedAccount()fails, the other result is dropped. UseallSettledto 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
nullbut 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 debugconsole.logstatement.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 masteropenOrdersfallback logic.When
agentAddressis active butagentOpenOrdersis empty, the current logic falls back toopenOrdersfrom the main address. This shows incorrect account data. UseagentAddresspresence 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: MissingEnhancedAssettype import causes TypeScript compilation error.
EnhancedAssetis referenced inTradeFormPropsbut 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: DuplicateuseBuilderFeevariable declaration.
useBuilderFeeis 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
!importedcheck 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 existinggetDistributedRatioshelper 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: UseuseFieldArrayhelpers (replace) instead ofsetValueto avoid field ID desync.Calling
setValue('takeProfits', ...)bypassesuseFieldArrayinternals and can desynctpFieldsIDs, 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: Debugconsole.logleft 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 whereuseBuilderFeeis true,!importedmust be true, meaningimportedis 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
builderApprovedis 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 unusedInputimport.
Inputis 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 unusedcoinIconfield.The
coinIconfield is defined in theMarketinterface but never used. TheTokenIconcomponent on line 64 usesmarket.coindirectly to fetch icons. IfcoinIconis 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 ismarket.changePercent(line 86). If these fields ever become inconsistent (e.g.,changeis positive butchangePercentshows 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 unusedrecipientparameter.The
recipientparameter is accepted but never used since the bridge always creditsmsg.sender. This can mislead callers into thinking they can deposit to another address. Consider either:
- Removing the parameter if compatibility is no longer needed.
- 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
anybypasses TypeScript's type checking. In ethers v5,_signTypedDataexists onJsonRpcSignerbut 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: TypewalletClientproperly instead ofany.The
walletClientprop is typed asany, 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 allowsamountUSDminimum of 10 but trades can fail for smaller positions.The Zod schema enforces
min(10)foramountUSD, but the actual minimum depends on the asset'sszDecimals, 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: Avoidanytype foruserStateprop.The comment acknowledges the issue but
anybypasses TypeScript safety. SinceUserStateshould 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 debugconsole.logstatement.🧹 Remove or replace with proper logging
if (urlAsset) { setSelectedAsset(urlAsset); - console.log(`[URL] Auto-selected ${urlAsset.symbol} from URL`); }
43-52: InitialselectedAssetwith 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 asnulland showing a loading state until assets are loaded.
| "type": "module", | ||
| "scripts": { | ||
| "dev": "vite", | ||
| "dev:functions": "wrangler pages dev build --port 5173 --live-reload", | ||
| "build": "VITE_COMMIT_SHA=$CF_PAGES_COMMIT_SHA VITE_VERSION=$npm_package_version vite build", | ||
| "build": "NODE_OPTIONS='--max-old-space-size=8192' VITE_COMMIT_SHA=$CF_PAGES_COMMIT_SHA VITE_VERSION=$npm_package_version vite build", |
There was a problem hiding this comment.
🧩 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 -30Repository: pillarwallet/x
Length of output: 150
🏁 Script executed:
#!/bin/bash
# Check the package.json to confirm the "type": "module" setting
cat package.json | head -20Repository: 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.exportsandrequire() config-overrides.jsusesrequire()for webpack and terser- These must be converted to ESM imports/exports or renamed to
.cjsextension 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.
| 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`, | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
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.
This reverts commit 7506c20.
There was a problem hiding this comment.
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: wireonClickhandlers or mark them explicitly non-interactive.Also applies to: 25-31
src/apps/perps/lib/hyperliquid/signing.ts (4)
469-482: DeduplicategenerateAgentWallet.This duplicates
src/apps/perps/lib/hyperliquid/keystore.tsand 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/actionHashare 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 deriveconnectionId, then sign that. citeturn2search5turn2search1🔧 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:signAgentActionignoresaction/nonceand signs a static payload.This makes agent signatures reusable across actions and inconsistent with the user-signing path. Build the phantom agent
connectionIdfrom the provided action + nonce and use the same derivation as the user signing flow so verify/sign paths match. citeturn2search5turn2search1
86-90: Normalizev(0/1 vs 27/28) before returning signatures.Some signers return
vas 0/1. Normalize (or useviem.parseSignature) so downstream verification/API submission is consistent. citeturn2search5turn2search1✅ 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 ofgetImportedAccount.You already have a static import; remove the dynamic import to avoid duplication.
166-168:handleRefreshshould tolerate partial failures.
Promise.alldrops results if either call fails. ConsiderPromise.allSettledto refresh what you can.
277-283: Agent openOrders fallback shows master orders (wrong account).When
agentAddressis present but has no orders, the UI shows master orders. Use the agent’s list wheneveragentAddressis truthy (even if empty).Also applies to: 302-309
src/apps/perps/components/AgentControls.tsx (5)
88-143:revealModetyping 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
revealModeis'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.addressfrom the private key but never verify it matches the user-enteredimportAccountAddress. 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: Normalizevbefore sending signatures.
vcan be 0/1 from some signers; normalize to 27/28 (or useviem.parseSignature) before posting to the API. citeturn2search5turn2search1✅ 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:accountToUseselection logic is incomplete.The placeholder comment means the account might not match the connected provider, causing signature/account mismatches. Mirror the
eth_accountsprobe used inhandleApproveAgent. citeturn2search5turn2search1src/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. citeturn2search5turn2search1
177-184: ExchangeClient should receive an account, not raw private key.
approveAgentSDKpassesmasterPrivateKeydirectly, while other SDK calls convert withprivateKeyToAccount. Align this to avoid runtime/type errors. citeturn2search5turn2search1✅ 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: MissingEnhancedAssetimport (compile error).
TradeFormPropsreferencesEnhancedAsset, 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:useFieldArraydesync risk fromsetValue.Using
setValue('takeProfits', ...)can desynctpFields/IDs. Usereplace/updatefromuseFieldArrayinstead.🔧 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. UsegetDistributedRatios(...).✅ 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:isSubmittingnever 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.loadingcreated 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: TwouseBuilderFeedefinitions cause inconsistent behavior.The inner
useBuilderFeeshadows 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: MissingUserStatetype import causes TypeScript compilation error.
UserStateis 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 infetchDatauseCallback.
fetchDataboth readsinternalUserState(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
internalUserStatefrom the dependency array since it's an internal state managed by this function:- }, [masterAddress, userState, directPositions, externalOpenOrders, internalUserState]); + }, [masterAddress, userState, directPositions, externalOpenOrders]);The
effectiveStatefallback at line 151 can use a ref if needed, or rely on the fact that whenuserStateis provided externally,internalUserStateisn'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 actualszDecimalsfrom universe metadata. The position is enriched withcoinInfo.szDecimalsat 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: Missingawaiton async function calls may cause race conditions.
handleExecuteCloseandhandleCancelOrderare async functions called withoutawait. ThesetPendingAction(null)andreturn trueexecute 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 === 0andisLoadingare true. Sincepositions.length === 0is 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
isLoadingbeforeopenOrders.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
hasOrdersis false (meaningtps.length === 0 && sls.length === 0), the inner else branch at lines 968-980 checking for TP/SL lengths is unreachable — the conditiontps.length === 0 && sls.length === 0is 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.
Inputis imported but never used in this component.-import { Input } from '../ui/input';
5-13: Consider removing unusedcoinIconfield.The
coinIconproperty is defined in the interface but the component usesgetCoinColorandgetCoinInitialhelpers 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
onClickhandlers, 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 emptychangestring.The color logic relies on
market.change.startsWith('-'), which will returnfalsefor 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.
getUserFillsis imported but never used in this component.Proposed fix
import { getMarkPrice, getOpenOrders, getFrontendOpenOrders, getMetaAndAssetCtxs, - getUserFills, } from '../lib/hyperliquid/client';
There was a problem hiding this comment.
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-themesdependency requires aThemeProviderwrapper to function correctly withuseTheme()insonner.tsx.
🧹 Nitpick comments (1)
package.json (1)
101-101: Remove unusedpatch-packagedependency.The
patches/directory does not exist in the repository, and there is nopostinstallscript 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.
| "@etherspot/intent-sdk": "1.0.0-alpha.12", | ||
| "@etherspot/modular-sdk": "6.1.1", | ||
| "@etherspot/transaction-kit": "2.1.4", | ||
| "@hookform/resolvers": "^5.2.2", |
There was a problem hiding this comment.
🧩 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` versionRepository: 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.jsRepository: 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.
4 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.