Perps #489
Perps #489
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: 5
🤖 Fix all issues with AI agents
In @src/apps/perps/lib/hyperliquid/keystore.ts:
- Around line 328-338: Rename the second parameter in updateBuilderApproval from
approved to builderApproved and change its type to unknown (or any) so the
runtime type-check is meaningful; then coerce it to a boolean before storing
(e.g. check typeof builderApproved !== 'boolean' and warn, then const approved =
Boolean(builderApproved)); keep the storage key getStorageKey(masterAddress,
'builder_approved') and write String(approved) to localStorage.
- Around line 252-261: getImportedAccountAddress can throw when
GLOBAL_ACCOUNT_KEY contains malformed JSON; modify the function to safely handle
corrupt legacy JSON by wrapping the JSON.parse of
localStorage.getItem(GLOBAL_ACCOUNT_KEY) in a try/catch (or use a safe JSON
parse helper), return null on parse error (or fallback to the imported key), and
optionally clear the bad GLOBAL_ACCOUNT_KEY entry; keep the existing preference
order (GLOBAL_IMPORTED_ADDRESS_KEY first, then legacy
GLOBAL_ACCOUNT_KEY.accountAddress) and reference getImportedAccountAddress,
GLOBAL_IMPORTED_ADDRESS_KEY and GLOBAL_ACCOUNT_KEY when making the change.
- Around line 387-397: clearAllAgentWalletsLocal currently accesses localStorage
unguarded causing SSR crashes and it doesn't clear in-memory caches; wrap the
localStorage logic in a browser guard (e.g., check typeof window !== 'undefined'
&& window.localStorage) before iterating/removing keys inside
clearAllAgentWalletsLocal, and also clear any related in-memory caches used by
your module (e.g., agentWalletCache or agentWalletsMap — whatever cache
variable(s) hold agent wallets) so state is consistent between memory and
storage.
- Around line 355-368: getAgentWallet reads localStorage unguarded and will
throw in SSR/test environments; add a runtime guard (e.g., check typeof window
!== 'undefined' && window.localStorage) before any access to localStorage inside
getAgentWallet (including the cachedPrivateKey branch) and return null or
fallback if localStorage is unavailable; update uses of getStorageKey,
localStorage.getItem, and resetInactivityTimer within that guarded block so no
localStorage calls happen when window is undefined.
🧹 Nitpick comments (1)
src/apps/perps/lib/hyperliquid/keystore.ts (1)
403-407:checkWalletMatchcan be synchronous.
No awaits; consider droppingasync/Promiseunless required by an interface.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/perps/lib/hyperliquid/keystore.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/perps/lib/hyperliquid/keystore.ts (2)
src/apps/perps/lib/hyperliquid/session-manager.ts (3)
initializeSessionManager(19-22)resetInactivityTimer(28-41)clearSession(47-58)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: unit-tests
- GitHub Check: lint
- GitHub Check: Cloudflare Pages: x
- GitHub Check: build
🔇 Additional comments (2)
src/apps/perps/lib/hyperliquid/keystore.ts (2)
64-107: Validate decrypted key format before casting to Hex and prevent unexpected plaintext fallback.The review identifies two concerns: (1)
decryptWithPinreturns a string that is cast toHexwithout validation, and (2) the plaintext fallback viagetAgentWalletLocalignores the provided PIN entirely.The proposed fix references using an
isHexutility for validation. Viem v2 does provide theisHex(value, { strict?: boolean })function (strict mode by default validates the full "0x[0-9a-fA-F]*" pattern), which makes the validation approach sound.However, the caching strategy change from
cachedPrivateKey = privateKey as HextocachedAgentKeyByMaster.set(norm(masterAddress), privateKey as Hex)requires confirmation thatcachedAgentKeyByMasterexists and is the correct pattern in the codebase.
16-23: Type safety for wallet generation is correctly handled by viem.
generatePrivateKey()returns a properly formattedHextype (0x-prefixed string with 64 hex characters for 32-byte secp256k1 keys), andprivateKeyToAccount()correctly expects and processes this type. The return type annotation{ address: string; privateKey: Hex }matches the actual output. No additional validation is needed for generated keys sincegeneratePrivateKey()ensures the correct format internally.If the code later needs to handle decrypted private key strings, viem provides validation utilities (
isHex(),isHash()for 32-byte validation) and conversion helpers (hexToBytes(),bytesToHex()) as needed.
| // Module-level cache for unlocked private key (cleared on page refresh or session timeout) | ||
| let cachedPrivateKey: Hex | null = null; | ||
|
|
There was a problem hiding this comment.
Critical: cachedPrivateKey is not keyed by masterAddress (can return the wrong key).
If a user unlocks/stores for master A, then calls getAgentWallet(masterB), the function can return A’s cachedPrivateKey as long as master B has an address stored.
Proposed fix (cache per master)
-// Module-level cache for unlocked private key (cleared on page refresh or session timeout)
-let cachedPrivateKey: Hex | null = null;
+// Cache unlocked keys per master address (cleared on refresh or session timeout)
+const cachedAgentKeyByMaster = new Map<string, Hex>();
+const norm = (addr: string) => addr.toLowerCase();
initializeSessionManager(() => {
console.log('[Keystore] Clearing cached private keys due to session timeout');
- cachedPrivateKey = null;
+ cachedAgentKeyByMaster.clear();
cachedImportedKey = null; // Also clear imported account cache
});
// ...
export async function storeAgentWalletEncrypted(
masterAddress: string,
address: string,
privateKey: Hex,
pin: string,
approved: boolean = false,
builderApproved: boolean = false
): Promise<void> {
if (typeof window === 'undefined') return;
try {
const encrypted = await encryptWithPin(privateKey, pin);
const storageData = JSON.stringify(encrypted);
localStorage.setItem(getStorageKey(masterAddress, 'address'), address);
localStorage.setItem(getStorageKey(masterAddress, 'encrypted_key'), storageData);
// ...
- cachedPrivateKey = privateKey;
+ cachedAgentKeyByMaster.set(norm(masterAddress), privateKey);
resetInactivityTimer();
} catch (error) {
// ...
}
}
export async function unlockAgentWallet(
masterAddress: string,
pin: string
): Promise<{ address: string; privateKey: Hex; approved: boolean; builderApproved: boolean } | null> {
if (typeof window === 'undefined') return null;
// ...
try {
const encryptedData: EncryptedData = JSON.parse(encryptedkeyData);
const privateKey = await decryptWithPin(encryptedData, pin);
- cachedPrivateKey = privateKey as Hex;
+ cachedAgentKeyByMaster.set(norm(masterAddress), privateKey as Hex);
resetInactivityTimer();
return { address, privateKey: privateKey as Hex, approved, builderApproved };
} catch (error) {
// ...
}
}
export async function getAgentWallet(
masterAddress: string
): Promise<{ address: string; privateKey: Hex; approved: boolean; builderApproved: boolean } | null> {
+ if (typeof window === 'undefined') return null;
// Check memory cache first
- if (cachedPrivateKey) {
+ const cached = cachedAgentKeyByMaster.get(norm(masterAddress));
+ if (cached) {
const address = localStorage.getItem(getStorageKey(masterAddress, 'address'));
if (address) {
const approvedItem = localStorage.getItem(getStorageKey(masterAddress, 'approved'));
const approved = approvedItem === 'true';
const builderApprovedItem = localStorage.getItem(getStorageKey(masterAddress, 'builder_approved'));
const builderApproved = builderApprovedItem === 'true';
resetInactivityTimer();
- return { address, privateKey: cachedPrivateKey, approved, builderApproved };
+ return { address, privateKey: cached, approved, builderApproved };
}
}
// ...
}Also applies to: 32-62, 64-108, 355-381
| export function updateBuilderApproval( | ||
| masterAddress: string, | ||
| approved: boolean | ||
| ): void { | ||
| if (typeof window === 'undefined') return; | ||
| if (typeof approved !== 'boolean') { | ||
| console.warn('[Keystore] updateBuilderApproval called with non-boolean:', approved); | ||
| approved = !!approved; | ||
| } | ||
| localStorage.setItem(getStorageKey(masterAddress, 'builder_approved'), String(approved)); | ||
| } |
There was a problem hiding this comment.
updateBuilderApproval param name/type-check is inconsistent.
The parameter is named approved but represents builderApproved; and since it’s typed boolean, the runtime typeof check is mostly dead code (unless called from untyped JS).
Proposed fix
export function updateBuilderApproval(
masterAddress: string,
- approved: boolean
+ builderApproved: boolean
): void {
if (typeof window === 'undefined') return;
- if (typeof approved !== 'boolean') {
- console.warn('[Keystore] updateBuilderApproval called with non-boolean:', approved);
- approved = !!approved;
- }
- localStorage.setItem(getStorageKey(masterAddress, 'builder_approved'), String(approved));
+ localStorage.setItem(getStorageKey(masterAddress, 'builder_approved'), String(builderApproved));
}📝 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.
| export function updateBuilderApproval( | |
| masterAddress: string, | |
| approved: boolean | |
| ): void { | |
| if (typeof window === 'undefined') return; | |
| if (typeof approved !== 'boolean') { | |
| console.warn('[Keystore] updateBuilderApproval called with non-boolean:', approved); | |
| approved = !!approved; | |
| } | |
| localStorage.setItem(getStorageKey(masterAddress, 'builder_approved'), String(approved)); | |
| } | |
| export function updateBuilderApproval( | |
| masterAddress: string, | |
| builderApproved: boolean | |
| ): void { | |
| if (typeof window === 'undefined') return; | |
| localStorage.setItem(getStorageKey(masterAddress, 'builder_approved'), String(builderApproved)); | |
| } |
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/keystore.ts around lines 328 - 338, Rename
the second parameter in updateBuilderApproval from approved to builderApproved
and change its type to unknown (or any) so the runtime type-check is meaningful;
then coerce it to a boolean before storing (e.g. check typeof builderApproved
!== 'boolean' and warn, then const approved = Boolean(builderApproved)); keep
the storage key getStorageKey(masterAddress, 'builder_approved') and write
String(approved) to localStorage.
| export async function getAgentWallet( | ||
| masterAddress: string | ||
| ): Promise<{ address: string; privateKey: Hex; approved: boolean; builderApproved: boolean } | null> { | ||
| // Check memory cache first | ||
| if (cachedPrivateKey) { | ||
| const address = localStorage.getItem(getStorageKey(masterAddress, 'address')); | ||
| if (address) { | ||
| const approvedItem = localStorage.getItem(getStorageKey(masterAddress, 'approved')); | ||
| const approved = approvedItem === 'true'; | ||
| const builderApprovedItem = localStorage.getItem(getStorageKey(masterAddress, 'builder_approved')); | ||
| const builderApproved = builderApprovedItem === 'true'; | ||
| resetInactivityTimer(); // Reset session timeout on wallet access | ||
| return { address, privateKey: cachedPrivateKey, approved, builderApproved }; | ||
| } |
There was a problem hiding this comment.
Critical: getAgentWallet needs a typeof window guard.
It reads localStorage unconditionally, which can crash SSR/test environments.
Proposed fix
export async function getAgentWallet(
masterAddress: string
): Promise<{ address: string; privateKey: Hex; approved: boolean; builderApproved: boolean } | null> {
+ if (typeof window === 'undefined') return null;
// ...
}🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/keystore.ts around lines 355 - 368,
getAgentWallet reads localStorage unguarded and will throw in SSR/test
environments; add a runtime guard (e.g., check typeof window !== 'undefined' &&
window.localStorage) before any access to localStorage inside getAgentWallet
(including the cachedPrivateKey branch) and return null or fallback if
localStorage is unavailable; update uses of getStorageKey, localStorage.getItem,
and resetInactivityTimer within that guarded block so no localStorage calls
happen when window is undefined.
| export function clearAllAgentWalletsLocal(): void { | ||
| const keysToRemove: string[] = []; | ||
| for (let i = 0; i < localStorage.length; i++) { | ||
| const key = localStorage.key(i); | ||
| if (key?.startsWith('hl_agent_')) { | ||
| keysToRemove.push(key); | ||
| } | ||
| } | ||
| keysToRemove.forEach((key) => localStorage.removeItem(key)); | ||
| } | ||
|
|
There was a problem hiding this comment.
clearAllAgentWalletsLocal missing window guard (SSR crash) + consider also clearing in-memory caches.
Proposed fix
export function clearAllAgentWalletsLocal(): void {
+ if (typeof window === 'undefined') return;
const keysToRemove: string[] = [];
for (let i = 0; i < localStorage.length; i++) {
const key = localStorage.key(i);
if (key?.startsWith('hl_agent_')) {
keysToRemove.push(key);
}
}
keysToRemove.forEach((key) => localStorage.removeItem(key));
+ // Optionally: clear session/caches if this is meant to be a “full reset”
+ // clearSession();
}🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/keystore.ts around lines 387 - 397,
clearAllAgentWalletsLocal currently accesses localStorage unguarded causing SSR
crashes and it doesn't clear in-memory caches; wrap the localStorage logic in a
browser guard (e.g., check typeof window !== 'undefined' && window.localStorage)
before iterating/removing keys inside clearAllAgentWalletsLocal, and also clear
any related in-memory caches used by your module (e.g., agentWalletCache or
agentWalletsMap — whatever cache variable(s) hold agent wallets) so state is
consistent between memory and storage.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/apps/perps/components/UnlockWalletModal.tsx:
- Around line 91-93: The modal in UnlockWalletModal is currently non-dismissible
(Dialog uses an empty onOpenChange, DialogContent prevents pointer/escape and
there is no call to onClose), which likely harms UX; either document this
intentional security behavior or restore dismissibility by wiring the Dialog
onOpenChange to call the provided onClose callback, remove or loosen the
onPointerDownOutside/onEscapeKeyDown preventDefault handlers on DialogContent,
and add a visible "Cancel" or "Close" button in the component that invokes
onClose(); update references to onOpenChange, onClose, DialogContent,
onPointerDownOutside, and onEscapeKeyDown accordingly.
🧹 Nitpick comments (2)
src/apps/perps/components/UnlockWalletModal.tsx (2)
49-77: Consolidate duplicate error handling logic.The
elseblock (lines 58-65) andcatchblock (lines 66-73) contain identical error handling code. This duplication reduces maintainability.♻️ Proposed refactor to eliminate duplication
const handleUnlockAttempt = useCallback(async (pinValue: string) => { if (pinValue.length !== 4) return; setIsLoading(true); try { const success = await onUnlock(pinValue); if (success) { toast.success('Wallet unlocked!'); setPin(''); // Clear PIN on success } else { - // Handle explicit failure (false returned) - toast.error('Incorrect PIN'); - setPin(''); - setTimeout(() => { - inputRef.current?.focus(); - }, 100); + throw new Error('Incorrect PIN'); } } catch (error) { console.error('Unlock failed', error); toast.error('Incorrect PIN'); setPin(''); // Clear PIN on failure to allow retry // Refocus input on failure setTimeout(() => { inputRef.current?.focus(); }, 100); } finally { setIsLoading(false); } }, [onUnlock]);
107-121: Potential redundancy: bothautoFocusprop and manual focus attempts.The InputOTP has
autoFocus={true}(line 113) while the useEffect also manually focuses the input viainputRef.current?.focus()with timeouts (lines 38-45).This dual approach might be an intentional workaround for mobile reliability (per the commit message "improve mobile PIN input auto-focus"), but could cause redundant focus calls or minor race conditions.
Verify whether:
- The
autoFocusprop is necessary alongside manual focus- Mobile devices handle
autoFocusreliably in dialogsIf manual focus is sufficient, consider removing the
autoFocusprop to reduce redundancy.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/perps/components/UnlockWalletModal.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 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/UnlockWalletModal.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/UnlockWalletModal.tsx
🧬 Code graph analysis (1)
src/apps/perps/components/UnlockWalletModal.tsx (1)
src/apps/perps/components/ui/input-otp.tsx (3)
InputOTP(69-69)InputOTPGroup(69-69)InputOTPSlot(69-69)
⏰ 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). (2)
- GitHub Check: unit-tests
- GitHub Check: build
🔇 Additional comments (5)
src/apps/perps/components/UnlockWalletModal.tsx (5)
1-17: LGTM!Imports are appropriate and well-organized for the component's requirements.
19-23: LGTM!The interface is well-typed and the
Promise<boolean>return type foronUnlockprovides clear success/failure signaling.
26-28: LGTM!State and ref initialization are appropriate for managing PIN input, loading state, and focus control.
79-82: LGTM!Form submission handler follows standard patterns correctly.
84-89: LGTM!Auto-submit logic is correct and the
isLoadingcheck properly prevents double submission.
| // Reset PIN when modal opens/closes | ||
| useEffect(() => { | ||
| if (!isOpen) { | ||
| setPin(''); | ||
| setIsLoading(false); | ||
| } else { | ||
| // Auto-focus the input when modal opens | ||
| // Attempt 1: Fast (for desktop/fast devices) | ||
| setTimeout(() => { | ||
| inputRef.current?.focus(); | ||
| }, 50); | ||
|
|
||
| // Attempt 2: Slower (for mobile animations to finish) | ||
| setTimeout(() => { | ||
| inputRef.current?.focus(); | ||
| }, 500); | ||
| } | ||
| }, [isOpen]); |
There was a problem hiding this comment.
Add cleanup for setTimeout calls to prevent memory leaks and stale references.
The effect creates two timers but doesn't clean them up if isOpen changes or the component unmounts. This can cause focus attempts on unmounted/closed modals and memory leaks.
🐛 Proposed fix with proper cleanup
// Reset PIN when modal opens/closes
useEffect(() => {
if (!isOpen) {
setPin('');
setIsLoading(false);
} else {
// Auto-focus the input when modal opens
// Attempt 1: Fast (for desktop/fast devices)
- setTimeout(() => {
+ const timer1 = setTimeout(() => {
inputRef.current?.focus();
}, 50);
// Attempt 2: Slower (for mobile animations to finish)
- setTimeout(() => {
+ const timer2 = setTimeout(() => {
inputRef.current?.focus();
}, 500);
+
+ return () => {
+ clearTimeout(timer1);
+ clearTimeout(timer2);
+ };
}
}, [isOpen]);…api env selection
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @src/apps/perps/components/TradeForm.tsx:
- Around line 366-418: In onSubmit, several early-return branches (e.g., when
!selectedAsset, !masterAddress, imported missing and agent.builderApproved is
false, and when privateKey is not found) call toast.error or return without
clearing the initial toast created by toast.loading; update all early-return
error paths within onSubmit (including the branch checking agent.builderApproved
and the final privateKey check) to dismiss or replace the loading toast by
passing { id: toastId } to toast.error (or call toast.dismiss(toastId) /
toast.remove(toastId)) before returning, and ensure setIsSubmitting(false) is
invoked where loading state must be cleared; look for functions/vars referenced
here (onSubmit, toastId, getAgentWallet, getImportedAccount, setIsSubmitting,
masterAddress, selectedAsset) and apply the change to every exit path so the
"Placing order..." toast is always cleared.
- Around line 121-184: The current handlers mutate takeProfits via setValue
which desyncs useFieldArray's tpFields/IDs; instead use useFieldArray helpers:
in handleAddTp call appendTp for the new TP (with its price/distance/ratio
placeholder) then compute redistributed ratios and call update for each index
(use update(index, { ...tp, ratio: newRatio })) to preserve field IDs and RHF
metadata; in handleRemoveTp call removeTp(index) to remove the entry, then
recompute ratios for the remaining entries and call update for each remaining
index (or call replace(updatedArray) if you want to replace all while preserving
array IDs via replace) rather than calling setValue('takeProfits', ...).
- Around line 246-282: The prefill logic in the TradeForm useEffect creates TP
ratios with Math.floor(100 / tps.length) which can sum to less than 100 and fail
validation; replace that manual calculation by calling the existing
getDistributedRatios(count) to get an array of ratios that sums to 100, and then
setValue('takeProfits', tps.map((p, i) => ({ price: p, ratio: ratios[i],
distance: calculateDistance(p, ep) }))); ensure you parse and filter takeProfits
into tps as before and obtain ratios = getDistributedRatios(tps.length) before
mapping.
- Around line 500-567: The stop-loss is posted at full size before take-profits,
so if partial TPs fill first the SL may be clamped or rejected; update the flow
in TradeForm.tsx to either (a) post SL after submitting TPs and adjust its size
to the remaining position, or (b) keep posting SL first but handle
ReduceOnly/clamp responses from placeTriggerOrderAgent by detecting the
error/response, calculating the new remaining size from executed TPs, and
retrying the SL with the adjusted size (or cancelling if zero). Locate the SL
placement block and the take-profit loop (references: placeTriggerOrderAgent,
data.stopLoss, data.takeProfits, and the tps loop) and implement one of the two
strategies plus error handling for ReduceOnly responses to ensure SL uses the
correct reduce-only size.
In @src/apps/perps/lib/hyperliquid/sdk.ts:
- Around line 17-57: The price string generation in placeMarketOrderAgent (the
marketPrice variable) uses parseFloat(...toPrecision(5)).toString(), which can
produce scientific notation; replace this with a decimal-notation formatter
helper (e.g., formatPriceDecimal(value, sig=5)) that returns a plain decimal
string (no "e" notation) while honoring 5 significant figures and trimming
trailing zeros; call that helper wherever prices are formatted (marketPrice here
and the other spots referenced) before putting p into orderRequest so
Hyperliquid receives a proper decimal string.
In @src/apps/perps/lib/hyperliquid/signing.ts:
- Around line 29-92: signUserAction currently builds an unused actionPayload and
signs a static phantom agent; replace the dead-code path by msgpack-serializing
the actual actionPayload (action, nonce, vaultAddress), keccak256-hashing it (or
call createL1ActionHash if available) to obtain the connectionId, set
phantomAgent.connectionId to that hash, then call walletClient.signTypedData
with that phantomAgent so the signature binds to the action+nonce; remove the
unused actionHash and actionPayload variables and ensure you still parse r/s/v
from the returned signature as before.
- Around line 234-296: The signature parsing extracts v directly from signature
bytes but doesn't normalize it, which can yield 0/1 or 27/28 depending on
signer; update signApproveAgentAction, signApproveBuilderFeeAction,
signAgentAction, and signUserAction to normalize v after parsing (if parsed v <
2 then add 27 to produce 27/28) before returning or sending to the API so all
code and API submissions use a consistent v format.
🧹 Nitpick comments (4)
src/apps/perps/components/TradeForm.tsx (2)
40-69:tradeSchemahas a no-op.refineand misses key invariants (entryPrice, TP ratios).
Right now validation effectively doesn’t enforce anything beyond field-level constraints; alsoentryPriceisn’t required for limit orders, and TP ratio sum isn’t enforced until submit-time.Proposed direction (keep schema meaningful)
- Remove the placeholder
.refine.- Add an
orderTypefield ('market' | 'limit') to schema + keep it in sync withisMarketOrder, so you can requireentryPriceifforderType === 'limit'.- Add a schema-level check for TP ratio sum (and optionally
>0when TP exists) so errors surface inline before submit.
1127-1138: Remove render-time debug logging before merging to staging.
The IIFEconsole.login render and click handler logs will spam prod consoles and can leak sensitive trading context during normal use.Proposed fix
- {/* Debug logging */} - {(() => { - console.log('[TradeForm] Button state:', { - isSubmitting, - 'errors.amountUSD': errors.amountUSD, - isBelowMinimum, - minUSD, - amountUSD, - disabled: isSubmitting || !!errors.amountUSD || isBelowMinimum, - }); - return null; - })()} - <Button type="submit" disabled={isSubmitting || !!errors.amountUSD || isBelowMinimum} - onClick={(e) => { - console.log('[TradeForm] Submit button clicked event', { - disabled: isSubmitting || !!errors.amountUSD || isBelowMinimum, - defaultPrevented: e.defaultPrevented, - }); - }} className={`w-full ${errors.amountUSD || isBelowMinimumAlso applies to: 1143-1148
src/apps/perps/lib/hyperliquid/signing.ts (1)
366-380: DuplicategenerateAgentWalletvssrc/apps/perps/lib/hyperliquid/keystore.ts.
This is easy to drift (and it already differs). Prefer a single implementation (likely keystore) and re-export it where needed.src/apps/perps/lib/hyperliquid/sdk.ts (1)
8-12: UseprivateKeyToAccountconsistently inapproveAgentSDKandapproveBuilderFeeSDKfor code consistency.While both
approveAgentSDK(line 183) andapproveBuilderFeeSDK(line 201) pass a rawHexstring as thewalletparameter toExchangeClient, andgetExchangeClientForAgent(line 11) converts the key viaprivateKeyToAccount, the @nktkas/hyperliquid library accepts both formats. Standardize to useprivateKeyToAccountfor consistency across all three functions: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 }); ... } export async function approveBuilderFeeSDK( masterPrivateKey: Hex, builderAddress: string, maxFeeRate: 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 }); ... }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/apps/perps/components/TradeForm.tsxsrc/apps/perps/lib/hyperliquid/client.tssrc/apps/perps/lib/hyperliquid/sdk.tssrc/apps/perps/lib/hyperliquid/signing.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/perps/lib/hyperliquid/client.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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.tsx
🧬 Code graph analysis (1)
src/apps/perps/lib/hyperliquid/signing.ts (2)
src/apps/perps/lib/hyperliquid/types.ts (1)
HyperliquidAction(57-60)src/apps/perps/lib/hyperliquid/keystore.ts (1)
generateAgentWallet(16-23)
⏰ 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
| const { | ||
| fields: tpFields, | ||
| append: appendTp, | ||
| remove: removeTp, | ||
| } = useFieldArray({ | ||
| control, | ||
| name: 'takeProfits', | ||
| }); | ||
|
|
||
| const entryPrice = watch('entryPrice'); | ||
| const stopLoss = watch('stopLoss'); | ||
| const takeProfits = watch('takeProfits'); | ||
| const marginMode = watch('marginMode'); | ||
|
|
||
| // Helper to calculate distributed ratios | ||
| const getDistributedRatios = (count: number) => { | ||
| if (count <= 0) return []; | ||
| const base = Math.floor(100 / count); | ||
| const remainder = 100 % count; | ||
| return Array(count) | ||
| .fill(base) | ||
| .map((val, i) => (i < remainder ? val + 1 : val)); | ||
| }; | ||
|
|
||
| // Handle adding TP with auto-redistribution | ||
| const handleAddTp = () => { | ||
| const newCount = tpFields.length + 1; | ||
| const ratios = getDistributedRatios(newCount); | ||
|
|
||
| // Convert existing fields to new ratios | ||
| // We need to flush updates to existing fields first | ||
| const currentValues = getValues('takeProfits') || []; | ||
| const updatedValues = currentValues.map((tp, i) => ({ | ||
| ...tp, | ||
| ratio: ratios[i], | ||
| })); | ||
|
|
||
| // Add new field with its calculated ratio | ||
| updatedValues.push({ | ||
| price: 0, | ||
| ratio: ratios[newCount - 1], | ||
| distance: 0, | ||
| }); | ||
|
|
||
| // Replace all with new values | ||
| setValue('takeProfits', updatedValues); | ||
| }; | ||
|
|
||
| // Handle removing TP with auto-redistribution | ||
| const handleRemoveTp = (index: number) => { | ||
| const currentValues = getValues('takeProfits') || []; | ||
| const keptValues = currentValues.filter((_, i) => i !== index); | ||
|
|
||
| if (keptValues.length > 0) { | ||
| const ratios = getDistributedRatios(keptValues.length); | ||
| const updatedValues = keptValues.map((tp, i) => ({ | ||
| ...tp, | ||
| ratio: ratios[i], | ||
| })); | ||
| setValue('takeProfits', updatedValues); | ||
| } else { | ||
| setValue('takeProfits', []); | ||
| } | ||
| }; |
There was a problem hiding this comment.
TP add/remove should use useFieldArray helpers (replace/update) to avoid stale fields/IDs.
setValue('takeProfits', ...) can desync tpFields/field IDs and tends to lose the field-array ergonomics (dirty/touched/validation behavior).
Proposed fix
const {
fields: tpFields,
append: appendTp,
remove: removeTp,
+ replace: replaceTp,
} = useFieldArray({
control,
name: 'takeProfits',
});
// Handle adding TP with auto-redistribution
const handleAddTp = () => {
const newCount = tpFields.length + 1;
const ratios = getDistributedRatios(newCount);
const currentValues = getValues('takeProfits') || [];
const updatedValues = currentValues.map((tp, i) => ({
...tp,
ratio: ratios[i],
}));
updatedValues.push({
price: 0,
ratio: ratios[newCount - 1],
distance: 0,
});
- setValue('takeProfits', updatedValues);
+ replaceTp(updatedValues);
};
const handleRemoveTp = (index: number) => {
const currentValues = getValues('takeProfits') || [];
const keptValues = currentValues.filter((_, i) => i !== index);
if (keptValues.length > 0) {
const ratios = getDistributedRatios(keptValues.length);
const updatedValues = keptValues.map((tp, i) => ({
...tp,
ratio: ratios[i],
}));
- setValue('takeProfits', updatedValues);
+ replaceTp(updatedValues);
} else {
- setValue('takeProfits', []);
+ replaceTp([]);
}
};🤖 Prompt for AI Agents
In @src/apps/perps/components/TradeForm.tsx around lines 121 - 184, The current
handlers mutate takeProfits via setValue which desyncs useFieldArray's
tpFields/IDs; instead use useFieldArray helpers: in handleAddTp call appendTp
for the new TP (with its price/distance/ratio placeholder) then compute
redistributed ratios and call update for each index (use update(index, { ...tp,
ratio: newRatio })) to preserve field IDs and RHF metadata; in handleRemoveTp
call removeTp(index) to remove the entry, then recompute ratios for the
remaining entries and call update for each remaining index (or call
replace(updatedArray) if you want to replace all while preserving array IDs via
replace) rather than calling setValue('takeProfits', ...).
| // Apply prefilled data when it changes | ||
| useEffect(() => { | ||
| if (prefilledData) { | ||
| if (prefilledData.side) { | ||
| setValue('side', prefilledData.side); | ||
| } | ||
| const ep = prefilledData.entryPrice || entryPrice || 0; | ||
| const isLong = (prefilledData.side || side) === 'long'; | ||
|
|
||
| if (prefilledData.entryPrice) { | ||
| setValue('entryPrice', prefilledData.entryPrice); | ||
| setIsMarketOrder(false); | ||
| } | ||
| if (prefilledData.stopLoss) { | ||
| setValue('stopLoss', { | ||
| price: prefilledData.stopLoss, | ||
| distance: calculateDistance(prefilledData.stopLoss, ep), | ||
| }); | ||
| } | ||
| 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; | ||
| setValue( | ||
| 'takeProfits', | ||
| tps.map((p) => ({ | ||
| price: p, | ||
| ratio: ratio, | ||
| distance: calculateDistance(p, ep), | ||
| })) | ||
| ); | ||
| } | ||
| } | ||
| }, [prefilledData, setValue, entryPrice, side]); | ||
|
|
There was a problem hiding this comment.
Prefill TP ratios can sum to < 100%, causing immediate submit failure.
prefilledData.takeProfits uses Math.floor(100 / tps.length) for every TP and drops the remainder, so totalRatio !== 100 and the submit-time check rejects.
Proposed fix (reuse getDistributedRatios)
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),
}))
);
}📝 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.
| // Apply prefilled data when it changes | |
| useEffect(() => { | |
| if (prefilledData) { | |
| if (prefilledData.side) { | |
| setValue('side', prefilledData.side); | |
| } | |
| const ep = prefilledData.entryPrice || entryPrice || 0; | |
| const isLong = (prefilledData.side || side) === 'long'; | |
| if (prefilledData.entryPrice) { | |
| setValue('entryPrice', prefilledData.entryPrice); | |
| setIsMarketOrder(false); | |
| } | |
| if (prefilledData.stopLoss) { | |
| setValue('stopLoss', { | |
| price: prefilledData.stopLoss, | |
| distance: calculateDistance(prefilledData.stopLoss, ep), | |
| }); | |
| } | |
| 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; | |
| setValue( | |
| 'takeProfits', | |
| tps.map((p) => ({ | |
| price: p, | |
| ratio: ratio, | |
| distance: calculateDistance(p, ep), | |
| })) | |
| ); | |
| } | |
| } | |
| }, [prefilledData, setValue, entryPrice, side]); | |
| // Apply prefilled data when it changes | |
| useEffect(() => { | |
| if (prefilledData) { | |
| if (prefilledData.side) { | |
| setValue('side', prefilledData.side); | |
| } | |
| const ep = prefilledData.entryPrice || entryPrice || 0; | |
| const isLong = (prefilledData.side || side) === 'long'; | |
| if (prefilledData.entryPrice) { | |
| setValue('entryPrice', prefilledData.entryPrice); | |
| setIsMarketOrder(false); | |
| } | |
| if (prefilledData.stopLoss) { | |
| setValue('stopLoss', { | |
| price: prefilledData.stopLoss, | |
| distance: calculateDistance(prefilledData.stopLoss, ep), | |
| }); | |
| } | |
| if (prefilledData.takeProfits) { | |
| const tps = prefilledData.takeProfits | |
| .split(',') | |
| .map((s) => parseFloat(s.trim())) | |
| .filter((n) => !isNaN(n)); | |
| const ratios = getDistributedRatios(tps.length); | |
| setValue( | |
| 'takeProfits', | |
| tps.map((p, i) => ({ | |
| price: p, | |
| ratio: ratios[i] ?? 0, | |
| distance: calculateDistance(p, ep), | |
| })) | |
| ); | |
| } | |
| } | |
| }, [prefilledData, setValue, entryPrice, side]); |
🤖 Prompt for AI Agents
In @src/apps/perps/components/TradeForm.tsx around lines 246 - 282, The prefill
logic in the TradeForm useEffect creates TP ratios with Math.floor(100 /
tps.length) which can sum to less than 100 and fail validation; replace that
manual calculation by calling the existing getDistributedRatios(count) to get an
array of ratios that sums to 100, and then setValue('takeProfits', tps.map((p,
i) => ({ price: p, ratio: ratios[i], distance: calculateDistance(p, ep) })));
ensure you parse and filter takeProfits into tps as before and obtain ratios =
getDistributedRatios(tps.length) before mapping.
| // Place stop loss if provided | ||
| if (data.stopLoss && data.stopLoss.price) { | ||
| // toast.info('Placing stop loss...'); | ||
|
|
||
| // Calculate limit price with slippage buffer | ||
| // For Long: SL triggers below entry, so limit should be even lower (0.99x) | ||
| // For Short: SL triggers above entry, so limit should be even higher (1.01x) | ||
| const slLimitPrice = | ||
| data.side === 'long' | ||
| ? data.stopLoss.price * 0.99 | ||
| : data.stopLoss.price * 1.01; | ||
|
|
||
| await placeTriggerOrderAgent(privateKey as `0x${string}`, { | ||
| coinId: selectedAsset.id, | ||
| isBuy: data.side === 'short', // Opposite side for reduce-only | ||
| size, | ||
| triggerPrice: data.stopLoss.price, | ||
| limitPrice: slLimitPrice, | ||
| tpsl: 'sl', | ||
| reduceOnly: true, | ||
| builder: { b: BUILDER_ADDRESS, f: BUILDER_FEE_ORDER }, | ||
| }); | ||
| } | ||
|
|
||
| // Place take profits if provided | ||
| if (data.takeProfits && data.takeProfits.length > 0) { | ||
| const tps = data.takeProfits; | ||
|
|
||
| // Validate total ratio | ||
| const totalRatio = tps.reduce((sum, tp) => sum + (tp.ratio || 0), 0); | ||
| if (Math.abs(totalRatio - 100) > 0.1) { | ||
| toast.error( | ||
| `Total Take Profit ratio must be 100% (Currently: ${totalRatio.toFixed(0)}%)`, | ||
| { id: toastId } | ||
| ); | ||
| setIsSubmitting(false); | ||
| return; | ||
| } | ||
|
|
||
| for (let i = 0; i < tps.length; i++) { | ||
| const tp = tps[i]; | ||
| if (!tp.price || !tp.ratio) continue; | ||
|
|
||
| // toast.info(`Placing take profit ${i + 1}/${tps.length}...`); | ||
|
|
||
| const rawTpSize = size * (tp.ratio / 100); | ||
| const tpSize = roundToSzDecimals(rawTpSize, selectedAsset.szDecimals); | ||
|
|
||
| if (tpSize <= 0) continue; | ||
|
|
||
| // Calculate limit price with slippage buffer | ||
| // For Long: TP triggers above entry, so limit should be slightly lower (1.01x is generous) | ||
| // For Short: TP triggers below entry, so limit should be slightly higher (0.99x) | ||
| const tpLimitPrice = | ||
| data.side === 'long' ? tp.price * 0.99 : tp.price * 1.01; | ||
|
|
||
| await placeTriggerOrderAgent(privateKey as `0x${string}`, { | ||
| coinId: selectedAsset.id, | ||
| isBuy: data.side === 'short', // Opposite side for reduce-only | ||
| size: tpSize, | ||
| triggerPrice: tp.price, | ||
| limitPrice: tpLimitPrice, | ||
| tpsl: 'tp', | ||
| reduceOnly: true, | ||
| builder: { b: BUILDER_ADDRESS, f: BUILDER_FEE_ORDER }, | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Hyperliquid reduce-only trigger order size exceeds position clamp or reject
💡 Result:
Short answer: Hyperliquid enforces reduce-only semantics on trigger (TP/SL) orders — if your trigger order size would increase or flip the position it will be limited or canceled rather than opening a larger/ opposite position. The platform either (a) clamps the executed amount to the portion that actually reduces your current position, or (b) rejects/cancels the reduce-only order with a reduce-only error/status (e.g., "ReduceOnly" / "reduceOnlyCanceled") when it would increase/flip the position. [1][2][3]
What to expect and how to handle it
- If you place a reduce-only trigger whose size > current position, fills will be limited to the quantity needed to reduce/close the position. If no position exists (or the order would flip sides) the order is canceled/rejected. [1]
- The API returns explicit statuses/errors for these cases (ReduceOnly error at placement or reduceOnlyCanceled in order status). Use the info/order-status endpoint to inspect the final status. [2][3]
- Practical fixes: compute and submit trigger size <= current position (query position via API before placing the trigger), or handle the ReduceOnly/rejected response in your bot to adjust/cancel the trigger automatically.
Sources: Hyperliquid order-types (reduce-only behavior), API error responses, and info endpoint docs. [1][2][3]
🌐 Web query:
Hyperliquid API documentation trigger orders reduce-only position size behavior
💡 Result:
Short answer: Trigger orders with reduce-only set will not increase or flip your position. Hyperliquid enforces reduce-only at placement/execution: trigger orders use mark price and can be market or limit, and the API accepts a reduceOnly flag; if executing a trigger would increase/flip your position it will be prevented (you’ll get a ReduceOnly error), and fills will be limited to the portion that actually reduces/closes the existing position. [1][2][3][4]
Sources
- Exchange API (place order fields, reduceOnly). [1]
- Order types — Reduce Only description. [2]
- Error list (ReduceOnly error string). [3]
- Trigger/TP/SL docs (trigger orders use mark price; stop-market example with reduce_only flag). [4]
SL may be rejected or clamped if partial TPs execute first.
Hyperliquid clamps reduce-only orders to the remaining position size or rejects them with a ReduceOnly error—it will not over-close. However, the SL is posted at full size regardless of TP execution. If some TPs fill first, the SL may be unexpectedly clamped or rejected rather than execute at full size. Consider tracking which TPs execute and adjusting or canceling the SL accordingly, or handle ReduceOnly rejection responses to retry with adjusted size.
🤖 Prompt for AI Agents
In @src/apps/perps/components/TradeForm.tsx around lines 500 - 567, The
stop-loss is posted at full size before take-profits, so if partial TPs fill
first the SL may be clamped or rejected; update the flow in TradeForm.tsx to
either (a) post SL after submitting TPs and adjust its size to the remaining
position, or (b) keep posting SL first but handle ReduceOnly/clamp responses
from placeTriggerOrderAgent by detecting the error/response, calculating the new
remaining size from executed TPs, and retrying the SL with the adjusted size (or
cancelling if zero). Locate the SL placement block and the take-profit loop
(references: placeTriggerOrderAgent, data.stopLoss, data.takeProfits, and the
tps loop) and implement one of the two strategies plus error handling for
ReduceOnly responses to ensure SL uses the correct reduce-only size.
| export async function placeMarketOrderAgent( | ||
| privateKey: Hex, | ||
| params: { | ||
| coinId: number; | ||
| isBuy: boolean; | ||
| size: number; | ||
| currentPrice: number; | ||
| reduceOnly?: boolean; | ||
| builder?: { b: string; f: number }; | ||
| } | ||
| ): Promise<any> { | ||
| const client = getExchangeClientForAgent(privateKey); | ||
|
|
||
| // Use 5% slippage to stay within Hyperliquid's 95% rule | ||
| // For closing positions (reduce-only), checking price might be less critical if we just want out, | ||
| // but we still need a limit price for the IOc order. | ||
| const marketPrice = params.isBuy | ||
| ? parseFloat((params.currentPrice * 1.05).toPrecision(5)).toString() | ||
| : parseFloat((params.currentPrice * 0.95).toPrecision(5)).toString(); | ||
|
|
||
| const orderRequest = { | ||
| orders: [ | ||
| { | ||
| a: params.coinId, | ||
| b: params.isBuy, | ||
| p: marketPrice, | ||
| s: params.size.toString(), | ||
| r: params.reduceOnly ?? false, | ||
| t: { limit: { tif: 'Ioc' as const } }, | ||
| }, | ||
| ], | ||
| grouping: 'na' as const, | ||
| builder: params.builder, | ||
| }; | ||
|
|
||
| console.log('[SDK] Placing market order:', orderRequest); | ||
| const response = await client.order(orderRequest); | ||
| console.log('[SDK] Order response:', response); | ||
|
|
||
| return response; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the file structure and the full functions mentioned
git ls-files | grep -E "hyperliquid|perps" | head -20Repository: pillarwallet/x
Length of output: 907
🏁 Script executed:
# Check the file size to determine how to read it
wc -l src/apps/perps/lib/hyperliquid/sdk.tsRepository: pillarwallet/x
Length of output: 99
🏁 Script executed:
# Read the relevant sections of the file
cat -n src/apps/perps/lib/hyperliquid/sdk.tsRepository: pillarwallet/x
Length of output: 7885
🏁 Script executed:
# Test the toPrecision behavior with various price values
python3 << 'EOF'
# Simulate JavaScript's toPrecision and toString behavior
# Test cases covering typical crypto prices and edge cases
test_prices = [
100, # Normal price
0.01, # 2 decimal places
0.001, # 3 decimal places
0.0001, # 4 decimal places
0.00001, # 5 decimal places
0.000001, # 6 decimal places (very small)
1.05, # With decimal multiplier
0.95 * 100, # Regular calculation
]
for price in test_prices:
# In JavaScript, toPrecision returns a string
# Let's check what Python's equivalent would be
# Python's f-string with .5g gives similar behavior
precision_result = f"{price:.5g}"
# Simulate parseFloat (which JavaScript uses)
back_to_num = float(precision_result)
# Then toString()
final_string = str(back_to_num)
print(f"Price: {price:15} -> toPrecision(5): {precision_result:15} -> toString(): {final_string}")
EOFRepository: pillarwallet/x
Length of output: 144
🏁 Script executed:
# Check Hyperliquid SDK type definitions or examples to understand expected price format
fd -e ts -e js . --search-path src/apps/perps | xargs grep -l "hyperliquid\|ExchangeClient" | head -5Repository: pillarwallet/x
Length of output: 217
🏁 Script executed:
# Look for any existing tests or examples that show order placement
fd -e test.ts -e spec.ts -e "test.js" . | head -10Repository: pillarwallet/x
Length of output: 460
🌐 Web query:
Hyperliquid API order placement endpoint price parameter format specification
💡 Result:
Summary — price parameter format for Hyperliquid order placement
- Field type: price is sent as a string in the order action (e.g. "p": "43250.0" or in unified API "price": ""). [1][2][3]
- No trailing zeros: trailing zeros and unnecessary decimal points must be removed before signing/sending (e.g. 12345.0 → "12345", 0.123450 → "0.12345"). SDKs usually normalize this automatically. [4][1]
- Precision rules: prices must follow the exchange tick/significant‑figure rules (Hyperliquid enforces tick size / max significant figures — typically up to 5 significant figures; perp/spot max decimal places may differ). Confirm exact tick/precision for the symbol via market metadata before formatting. [4][2]
- Signing note: use the exact string format used in the signed action (so formatting/trailing‑zero removal matters for signature). [1][6]
Examples
- Valid: "p": "65000" or "price": "65000"
- Valid small: "p": "0.00123"
- Invalid (trailing zero): "p": "12345.0" (should be "12345"). [3][4]
Sources: Hyperliquid exchange/placeOrder docs and SDK notes. [1][2][3][4]
References
[1] Hyperliquid Exchange endpoint (order fields and signing).
[2] Hyperliquid unified placeOrder documentation (price as string).
[3] Hypereth / Hyperliquid API examples (placeOrder).
[4] SDK / package notes on trailing‑zero handling and precision.
🌐 Web query:
@nktkas/hyperliquid SDK order price string format requirements
💡 Result:
Summary — price string rules for @nktkas/hyperliquid SDK:
- Allowed types: string or number. [1]
- No trailing zeros in the string (e.g. "12345.0" → "12345", "0.123450" → "0.12345"). The SDK auto-normalizes this for high-level methods. [2][3]
- Significant-figure limit: up to 5 significant figures (integer prices are always allowed). [1]
- Maximum decimal places:
- Perpetuals (perp): max (6 - szDecimals) decimal places (default perp behavior described as max 6 decimals overall). [1]
- Spot: max (8 - szDecimals) decimal places (docs note spot allows more decimals). [1]
(Get szDecimals from SymbolConverter.getSzDecimals() or market meta.) [1]
If you need implementation examples or a formatter call, tell me which language/runtime and I’ll paste the snippet.
Use a decimal-notation formatter instead of toPrecision(5) to avoid scientific notation in price strings.
The pattern parseFloat(number.toPrecision(5)).toString() can produce scientific notation (e.g., "1e-7" for small prices). Hyperliquid's API expects prices as decimal strings (e.g., "0.00001" or "65000"), and scientific notation may cause order placement to fail. While the exchange enforces 5 significant figures, proper formatting should use a decimal formatter to ensure compliant price strings.
Applies to lines 34–35, 80, and 121–129.
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/sdk.ts around lines 17 - 57, The price string
generation in placeMarketOrderAgent (the marketPrice variable) uses
parseFloat(...toPrecision(5)).toString(), which can produce scientific notation;
replace this with a decimal-notation formatter helper (e.g.,
formatPriceDecimal(value, sig=5)) that returns a plain decimal string (no "e"
notation) while honoring 5 significant figures and trimming trailing zeros; call
that helper wherever prices are formatted (marketPrice here and the other spots
referenced) before putting p into orderRequest so Hyperliquid receives a proper
decimal string.
| export async function signUserAction( | ||
| walletClient: WalletClient, | ||
| action: HyperliquidAction, | ||
| nonce: number | ||
| ): Promise<{ r: string; s: string; v: number }> { | ||
| if (!walletClient.account) { | ||
| throw new Error('No account connected'); | ||
| } | ||
|
|
||
| // Construct the action payload | ||
| const actionPayload = { | ||
| action, | ||
| nonce, | ||
| vaultAddress: null, | ||
| }; | ||
|
|
||
| // Convert to phantom agent format for signing | ||
| const phantomAgent = { | ||
| source: 'a', | ||
| connectionId: hashTypedData({ | ||
| domain: HL_DOMAIN, | ||
| types: { | ||
| Agent: [ | ||
| { name: 'source', type: 'string' }, | ||
| { name: 'connectionId', type: 'bytes32' }, | ||
| ], | ||
| }, | ||
| primaryType: 'Agent', | ||
| message: { | ||
| source: 'a', | ||
| connectionId: | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000' as Hex, | ||
| }, | ||
| }), | ||
| }; | ||
|
|
||
| // Sign the action | ||
| const actionHash = hashTypedData({ | ||
| domain: HL_DOMAIN, | ||
| types: { | ||
| HyperliquidTransaction: [ | ||
| { name: 'source', type: 'string' }, | ||
| { name: 'connectionId', type: 'bytes32' }, | ||
| ], | ||
| }, | ||
| primaryType: 'HyperliquidTransaction', | ||
| message: phantomAgent, | ||
| }); | ||
|
|
||
| const signature = await walletClient.signTypedData({ | ||
| account: walletClient.account, | ||
| domain: HL_DOMAIN, | ||
| types: HL_TYPES, | ||
| primaryType: 'HyperliquidTransaction', | ||
| message: phantomAgent, | ||
| }); | ||
|
|
||
| // Parse signature | ||
| const r = signature.slice(0, 66); | ||
| const s = '0x' + signature.slice(66, 130); | ||
| const v = parseInt(signature.slice(130, 132), 16); | ||
|
|
||
| return { r, s, v }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/apps/perps/lib/hyperliquid/signing.ts | head -40Repository: pillarwallet/x
Length of output: 1320
🏁 Script executed:
# Search for keystore.ts mentioned in the comment
fd -t f keystore.tsRepository: pillarwallet/x
Length of output: 100
🏁 Script executed:
# Look for HL_DOMAIN and HL_TYPES definitions
rg -n "HL_DOMAIN|HL_TYPES" src/apps/perps/lib/hyperliquid/ -A 2 -B 2Repository: pillarwallet/x
Length of output: 2821
🏁 Script executed:
# Check if actionPayload or actionHash are used anywhere else in the file or related files
rg -n "actionPayload|actionHash" src/apps/perps/lib/hyperliquid/Repository: pillarwallet/x
Length of output: 210
🏁 Script executed:
# Look for test files or examples of Hyperliquid signing
fd -t f "(test|spec|example)" src/apps/perps/lib/hyperliquid/Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
cat -n src/apps/perps/lib/hyperliquid/keystore.ts | head -100Repository: pillarwallet/x
Length of output: 4797
🏁 Script executed:
# Look for any comments or documentation about Hyperliquid signing protocol
rg -n "phantom|agent|replay" src/apps/perps/lib/hyperliquid/ -iRepository: pillarwallet/x
Length of output: 7872
🏁 Script executed:
# Check if there are any integration tests or usage examples
fd -t f "." src/apps/perps | grep -E "(test|spec|__)" | head -20Repository: pillarwallet/x
Length of output: 103
🏁 Script executed:
# Look at the full keystore.ts to compare signing approaches
wc -l src/apps/perps/lib/hyperliquid/keystore.tsRepository: pillarwallet/x
Length of output: 104
🏁 Script executed:
# See the full context of signUserAction including what happens after
cat -n src/apps/perps/lib/hyperliquid/signing.ts | sed -n '29,100p'Repository: pillarwallet/x
Length of output: 2258
🏁 Script executed:
# Look at signAgentAction for comparison
cat -n src/apps/perps/lib/hyperliquid/signing.ts | sed -n '298,330p'Repository: pillarwallet/x
Length of output: 1103
🏁 Script executed:
# Check if there are any comments or documentation files about Hyperliquid protocol
fd -t f "README|SPEC|protocol|hyperliquid" src/apps/perps/ -iRepository: pillarwallet/x
Length of output: 142
🏁 Script executed:
# Search for how Hyperliquid SDK itself handles signing
rg -n "phantomAgent|connectionId" src/apps/perps/lib/hyperliquid/ -B 3 -A 3Repository: pillarwallet/x
Length of output: 6146
🌐 Web query:
Hyperliquid EIP-712 signing protocol phantom agent wallet
💡 Result:
Short summary:
-
Hyperliquid uses two EIP‑712 signing flows:
-
L1 actions (trading) use a "phantom agent" construction: action is msgpack‑serialized, nonce/vault appended, keccak256 hashed to produce a connectionId (the phantom agent), then signed as EIP‑712 typed data. Chain ID for L1/agent signing is 1337 and EIP‑712 domain = "Exchange". Agent (bot) keypairs — called agent wallets — are authorized to sign these L1 actions. [1][2][3]
-
User‑signed actions (admin, approvals, transfers, approving agent wallets) use direct EIP‑712 typed data (domain "HyperliquidSignTransaction", Chain ID 0x66eee / 421614). Approving an agent wallet is done via a user‑signed action. [1][2]
-
-
Phantom (the wallet) exposes EIP‑712 signing APIs (signTypedData / ethereum signing) in its SDK/browser connect, so a web app can call Phantom to produce the required EIP‑712 signatures (either for direct user‑signed actions or for signing the agent/phantom agent typed data when the agent’s private key is held in an appropriate signer). Confirm you call the correct signTypedData path for Ethereum messages. [4][5]
Notes for implementation:
- For L1 actions you must serialize with msgpack and follow Hyperliquid’s exact type/schema so the SDK or your implementation produces the same connectionId hash before building the EIP‑712 typed data. The Hyperliquid SDKs provide helpers (e.g., signL1Action / createL1ActionHash) to avoid manual errors. [2][3][4]
- For approving or registering agent wallets, use the user‑signed EIP‑712 flow (domain "HyperliquidSignTransaction"). [1][2]
- Test on Hyperliquid testnet endpoints and verify signatures locally first. [1]
Sources:
[1] Hyperliquid signing overview / Chainstack documentation.
[2] Hyperliquid L1 action signing (msgpack → phantom agent) / Chainstack documentation.
[3] Hyperliquid user‑signed actions / Chainstack documentation.
[4] Hyperliquid signing utilities (createL1ActionHash, signL1Action) / Hyperliquid docs.
[5] Phantom developer docs — signTypedData / Ethereum signing in Phantom SDK.
If you want, I can: (a) show a minimal code example using Phantom's signTypedData to sign a Hyperliquid typed payload, or (b) walk through approving an agent wallet step‑by‑step. Which would you like?
🌐 Web query:
Hyperliquid user action signing connectionId
💡 Result:
ConnectionId (in Hyperliquid L1-action signing) is the keccak256 hash of the serialized L1 action (often produced by the SDK function createL1ActionHash). That hash is used as the "phantom agent" identifier and is included in the EIP‑712 typed-data payload (the connectionId field) when signing L1 actions (trading ops). To produce it you generally:
- MsgPack‑serialize the L1 action object with the exact field ordering used by the SDK.
- Include any required fields (nonce / timestamp, vault address when applicable) exactly as the SDK expects.
- Take keccak256 of the resulting bytes — that result is the connectionId used in the EIP‑712 structure and signing flow.
References / implementation notes: SDKs expose createL1ActionHash / signL1Action helpers and recommend using them (they handle msgpack order, chain id, domain, etc.). See Hyperliquid signing docs and the community SDK docs for examples. [1][2]
Sources:
[1] Hyperliquid — Signing (developer docs / SDK guidance). [2] Hyperliquid signing utilities (createL1ActionHash explanation) in community SDK.
Remove dead code and bind signature to the actual action and nonce via msgpack serialization.
signUserAction constructs actionPayload but never uses it. The signature is computed over a static "phantom agent" with hardcoded zero values, not the actual action and nonce parameters. Per the Hyperliquid protocol, L1 actions must be msgpack-serialized (including nonce and vault address), then keccak256-hashed to derive the connectionId, before signing. The current implementation skips serialization entirely, meaning:
- The signature does not prove which action was authorized
- The nonce is not bound to the signature (potential replay vulnerability)
actionPayloadandactionHashvariables are dead code
Use Hyperliquid SDK helpers (e.g., createL1ActionHash) or properly serialize the action with msgpack, hash it, and include that hash as the connectionId in the phantom agent before signing.
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/signing.ts around lines 29 - 92,
signUserAction currently builds an unused actionPayload and signs a static
phantom agent; replace the dead-code path by msgpack-serializing the actual
actionPayload (action, nonce, vaultAddress), keccak256-hashing it (or call
createL1ActionHash if available) to obtain the connectionId, set
phantomAgent.connectionId to that hash, then call walletClient.signTypedData
with that phantomAgent so the signature binds to the action+nonce; remove the
unused actionHash and actionPayload variables and ensure you still parse r/s/v
from the returned signature as before.
| // Convert to phantom agent format for signing | ||
| const phantomAgent = { | ||
| source: 'a', | ||
| connectionId: hashTypedData({ | ||
| domain: HL_DOMAIN, | ||
| types: { | ||
| Agent: [ | ||
| { name: 'source', type: 'string' }, | ||
| { name: 'connectionId', type: 'bytes32' }, | ||
| ], | ||
| }, | ||
| primaryType: 'Agent', | ||
| message: { | ||
| source: 'a', | ||
| connectionId: | ||
| '0x0000000000000000000000000000000000000000000000000000000000000000' as Hex, | ||
| }, | ||
| }), | ||
| }; | ||
|
|
||
| // Sign the action | ||
| const actionHash = hashTypedData({ | ||
| domain: HL_DOMAIN, | ||
| types: { | ||
| HyperliquidTransaction: [ | ||
| { name: 'source', type: 'string' }, | ||
| { name: 'connectionId', type: 'bytes32' }, | ||
| ], | ||
| }, | ||
| primaryType: 'HyperliquidTransaction', | ||
| message: phantomAgent, | ||
| }); |
There was a problem hiding this comment.
Inconsistent “phantom agent” derivation between user vs agent signing paths.
signUserAction uses hashTypedData(...) to derive connectionId, while signAgentAction uses keccak256(toHex(JSON.stringify(...))). If these are meant to represent the same message, they should be generated identically.
Also applies to: 298-330
| export async function signApproveAgentAction( | ||
| walletClient: WalletClient, | ||
| action: any | ||
| ): Promise<{ r: string; s: string; v: number }> { | ||
| if (!walletClient.account) { | ||
| throw new Error('No account connected'); | ||
| } | ||
|
|
||
| const { domain, types, primaryType, message } = getApproveAgentTypedData( | ||
| action.hyperliquidChain, | ||
| action.signatureChainId, | ||
| action.agentAddress, | ||
| action.agentName, | ||
| action.nonce | ||
| ); | ||
|
|
||
| const signature = await walletClient.signTypedData({ | ||
| account: walletClient.account, | ||
| domain, | ||
| types, | ||
| primaryType, | ||
| message, | ||
| }); | ||
|
|
||
| // Parse signature | ||
| const r = signature.slice(0, 66); | ||
| const s = '0x' + signature.slice(66, 130); | ||
| const v = parseInt(signature.slice(130, 132), 16); | ||
|
|
||
| return { r, s, v }; | ||
| } | ||
|
|
||
| export async function signApproveBuilderFeeAction( | ||
| walletClient: WalletClient, | ||
| action: any | ||
| ): Promise<{ r: string; s: string; v: number }> { | ||
| if (!walletClient.account) { | ||
| throw new Error('No account connected'); | ||
| } | ||
|
|
||
| const { domain, types, primaryType, message } = getApproveBuilderFeeTypedData( | ||
| action.hyperliquidChain, | ||
| action.signatureChainId, | ||
| action.maxFeeRate, | ||
| action.builder, | ||
| action.nonce | ||
| ); | ||
|
|
||
| const signature = await walletClient.signTypedData({ | ||
| account: walletClient.account, | ||
| domain, | ||
| types, | ||
| primaryType, | ||
| message, | ||
| }); | ||
|
|
||
| // Parse signature | ||
| const r = signature.slice(0, 66); | ||
| const s = '0x' + signature.slice(66, 130); | ||
| const v = parseInt(signature.slice(130, 132), 16); | ||
|
|
||
| return { r, s, v }; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the signing.ts file to see the full context
wc -l src/apps/perps/lib/hyperliquid/signing.tsRepository: pillarwallet/x
Length of output: 103
🏁 Script executed:
# Read the relevant sections and more context around them
cat -n src/apps/perps/lib/hyperliquid/signing.ts | sed -n '220,370p'Repository: pillarwallet/x
Length of output: 4781
🏁 Script executed:
# Search for how the returned r, s, v values are used in the codebase
rg -B 3 -A 3 'signApproveAgentAction|signApproveBuilderFeeAction' --type tsRepository: pillarwallet/x
Length of output: 1980
🏁 Script executed:
# Search for where r, s, v from signApproveAgentAction/signApproveBuilderFeeAction are used
rg -B 5 -A 5 'signApproveAgentAction|signApproveBuilderFeeAction' src/apps/perps/components/ --type ts --type tsxRepository: pillarwallet/x
Length of output: 85
🏁 Script executed:
# Look for API calls that use the signature data
rg 'postExchange|postBuilder' -A 10 -B 5 src/apps/perps/ --type ts --type tsxRepository: pillarwallet/x
Length of output: 85
🏁 Script executed:
# Check package.json for viem version
cat package.json | grep -A 5 '"viem"'Repository: pillarwallet/x
Length of output: 281
🏁 Script executed:
# Search for any signature reconstruction or verification patterns
rg 'signature.*toString.*16|padStart.*0|slice.*130' src/apps/perps/lib/hyperliquid/Repository: pillarwallet/x
Length of output: 1020
🏁 Script executed:
# Search for API client code that uses the signature
rg 'postExchange|postBuilder|submitSignature' src/apps/perps/lib/hyperliquid/ -A 10Repository: pillarwallet/x
Length of output: 915
🏁 Script executed:
# Find the builder client and see how it uses the signature
find src/apps/perps/lib/hyperliquid -type f -name '*.ts' | head -20Repository: pillarwallet/x
Length of output: 553
🏁 Script executed:
# Look for all calls to the sign functions to see the flow
rg -B 10 -A 10 'const signature = await sign' src/apps/perps/Repository: pillarwallet/x
Length of output: 4326
🏁 Script executed:
# Check the SignedAction type definition
rg 'type SignedAction|interface SignedAction' -A 10 src/apps/perps/lib/hyperliquid/types.tsRepository: pillarwallet/x
Length of output: 235
🏁 Script executed:
# Check builder.ts to see how signatures are used
cat -n src/apps/perps/lib/hyperliquid/builder.ts | head -100Repository: pillarwallet/x
Length of output: 324
🏁 Script executed:
# Look for v normalization patterns or checks
rg 'v\s*[><=]|normalize.*v|v.*27|v.*28' src/apps/perps/lib/hyperliquid/Repository: pillarwallet/x
Length of output: 437
🌐 Web query:
viem 2.37.1 signTypedData v encoding 0/1 or 27/28 recoverTypedDataAddress
💡 Result:
Short answer: viem's recoverTypedDataAddress accepts signatures with v encoded as 0/1 or 27/28 (and also handles the EIP‑2098 compact form). recoverTypedDataAddress simply hashes the typed data then calls recoverAddress, and recoverAddress normalizes the v/yParity formats before recovering the public address. Sources: viem docs and viem implementation + EIP‑2098 spec. [1][2][3]
Sources
- viem recoverTypedDataAddress (docs). [1]
- viem recoverTypedDataAddress implementation (calls recoverAddress). [2]
- ERC‑2098 (compact signature / yParity 0/1 vs 27/28 explanation). [3]
🌐 Web query:
viem signTypedData return value v encoding format 0/1 or 27/28
💡 Result:
Viem returns a 0x‑prefixed hex signature (r||s||v or compact). Its signature parser accepts both v formats: 0/1 (yParity) or 27/28 — mapping 27→yParity 0 and 28→yParity 1. See parseSignature implementation. [1]
Sources:
- viem parseSignature implementation. [1]
🌐 Web query:
viem walletClient.signTypedData signature format r s v hex
💡 Result:
Viem's walletClient.signTypedData returns a 0x-prefixed hex signature in the standard 65-byte (r, s, v) layout used by EIP-712: r (32 bytes) || s (32 bytes) || v (1 byte). The signature can be parsed like:
- r = signature.slice(2, 66)
- s = signature.slice(66, 130)
- v = parseInt(signature.slice(130, 132), 16)
Note: some signers return v as 0/1 while others return 27/28 — if you need the 27/28 form add 27 when v is 0/1. Viem accepts/returns the hex signature for verifyTypedData and EIP‑712 signing. [1][2]
Sources:
[1] Viem signTypedData (walletClient) docs.
[2] EIP‑712 specification (typed data hashing & signing).
🌐 Web query:
Hyperliquid API signature format v 0/1 or 27/28 EIP-712
💡 Result:
Short answer: Hyperliquid uses EIP‑712 typed-data signatures. The signature is the usual r, s, v tuple (r: 32 bytes, s: 32 bytes, v: recovery id). Many SDKs/clients encode v as 27/28 (classic Ethereum) but some libraries/tooling represent the recovery id as 0/1 — both are the same recovery bit with a different offset. Use whichever representation your signer/library expects; the Hyperliquid servers accept the standard r/s plus recovery-id form.
Important protocol details:
- Two signing schemes:
- L1 Actions (trading): Domain name "Exchange", ChainId = 1337, action serialized with msgpack then hashed (phantom‑agent construction). Use the sign_l1_action flow. [1][4][2]
- User‑signed actions (admin/transfers): Domain name "HyperliquidSignTransaction", ChainId = 0x66eee (421614 dec), version "1", sign the JSON typed data directly. [4][7]
- Signature object format shown in docs: { "r": "0x...", "s": "0x...", "v": 27 } (v can be 27/28 or represented as 0/1 by client libraries). Ensure addresses are lowercased, msgpack field order and numeric trailing zeros match SDK expectations. [2][3][4]
Sources: [1] Chainstack Hyperliquid authentication guide; [2] Hyperliquid GitBook signing docs; [3] hyperliquid SDK signing utilities; [4] Chainstack Hyperliquid signing overview.
🏁 Script executed:
# Search for any v normalization or conversion in the codebase
rg 'v\s*\+\s*27|v\s*[<>]|yParity' src/apps/perps/lib/hyperliquid/Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
# Check if there are any comments or docs about signature format
rg 'signature|sign|v.*parity|0/1|27/28' src/apps/perps/lib/hyperliquid/types.ts -A 2 -B 2Repository: pillarwallet/x
Length of output: 148
Add explicit v normalization to ensure consistent signature format across signing, recovery, and API submission.
The code parses v directly from the signature bytes without normalizing to a canonical format (0/1 or 27/28). While viem's signTypedData can return v as either 0/1 (yParity) or 27/28 depending on the signer, and recoverTypedDataAddress handles both formats transparently, the parsed v is sent to the Hyperliquid API as-is. For consistency and to prevent subtle bugs if signers vary, explicitly normalize v to your chosen format (typically add 27 if v < 2):
const v = parseInt(signature.slice(130, 132), 16);
const normalizedV = v < 2 ? v + 27 : v; // Ensure 27/28 formatApply to all functions in this file that parse signatures: signApproveAgentAction, signApproveBuilderFeeAction, signAgentAction, and signUserAction.
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/signing.ts around lines 234 - 296, The
signature parsing extracts v directly from signature bytes but doesn't normalize
it, which can yield 0/1 or 27/28 depending on signer; update
signApproveAgentAction, signApproveBuilderFeeAction, signAgentAction, and
signUserAction to normalize v after parsing (if parsed v < 2 then add 27 to
produce 27/28) before returning or sending to the API so all code and API
submissions use a consistent v format.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @src/apps/perps/lib/hyperliquid/client.ts:
- Around line 198-226: In getAllAssets: avoid overriding valid zero values by
replacing uses of `asset.szDecimals || 3` and `asset.maxLeverage || 50` with the
nullish coalescing operator (`asset.szDecimals ?? 3`, `asset.maxLeverage ??
50`), and ensure `response.data.universe` is an array before mapping (use
`Array.isArray(response.data.universe)`); if it's not an array, return an empty
list or handle accordingly so mapping only runs on a validated array.
- Around line 172-196: The getMarkPrice function currently uses a truthiness
check and parseFloat that treats 0 as falsy and can return NaN; change the logic
to explicitly check that response.data[symbol] is not null or undefined (use
markPrice !== undefined && markPrice !== null), convert it to a number safely
(e.g., Number(markPrice) or parseFloat), then validate the result with
Number.isFinite (or !Number.isNaN) and return the numeric value only when valid;
otherwise return null. Ensure references to markPrice, parseFloat/Number
conversion, and the getMarkPrice function are updated so 0 is preserved and
invalid parses yield null.
- Around line 94-147: Wrap and validate response.data with Array.isArray before
using array methods: in getOpenOrders replace const orders = response.data ||
[]; with something like const orders = Array.isArray(response.data) ?
response.data : []; and only call orders.filter when orders is an array (so
return [] if not); do the same in getFrontendOpenOrders to return
Array.isArray(response.data) ? response.data : []; this prevents runtime errors
if the API returns non-array payloads.
- Around line 5-11: The import-time evaluation of isTestnet (used to build
BASE_URL, EXCHANGE_URL, INFO_URL) reads localStorage and can crash in SSR/Node;
change isTestnet from an IIFE/constant to a runtime function (e.g., isTestnet())
in src/utils/blockchain.ts that guards access with typeof window !== 'undefined'
&& window.localStorage, then update this module to compute
BASE_URL/EXCHANGE_URL/INFO_URL at runtime (call isTestnet() inside the functions
or factory that use these URLs) so no localStorage access happens during module
import.
🧹 Nitpick comments (3)
src/apps/perps/lib/hyperliquid/client.ts (3)
12-51: Harden axios error handling + add timeouts (avoidany, preserve details)
catch (error: any)+error.response?.datais fragile for non-Axios errors, and requests have no timeout/abort (can hang UI). Preferunknown+axios.isAxiosError, and centralize axios config.Proposed refactor (axios instance + safer error parsing)
import axios from 'axios'; import { isTestnet } from '../../../../utils/blockchain'; import type { SignedAction, UserState, AssetInfo, HyperliquidOrder, UniverseAsset, AssetContext } from './types'; const BASE_URL = isTestnet ? 'https://api.hyperliquid-testnet.xyz' : 'https://api.hyperliquid.xyz'; -const EXCHANGE_URL = `${BASE_URL}/exchange`; -const INFO_URL = `${BASE_URL}/info`; +const http = axios.create({ + baseURL: BASE_URL, + timeout: 15_000, + headers: { 'Content-Type': 'application/json' }, +}); +const EXCHANGE_URL = `/exchange`; +const INFO_URL = `/info`; export async function postExchange(signedAction: SignedAction): Promise<any> { try { // Explicitly set vaultAddress to undefined if null/empty to exclude from JSON const payload = { ...signedAction, vaultAddress: signedAction.vaultAddress || undefined, }; - const response = await axios.post(EXCHANGE_URL, payload, { - headers: { - 'Content-Type': 'application/json', - }, - }); + const response = await http.post(EXCHANGE_URL, payload); const data = response.data; // Check if the exchange rejected the action if (data?.status && data.status !== 'ok') { const errorMsg = data.response?.error || data.response || 'Exchange rejected action'; throw new Error(errorMsg); } return data; - } catch (error: any) { - console.error('Exchange API error:', error.response?.data || error.message); + } catch (error: unknown) { + const axiosData = axios.isAxiosError(error) ? error.response?.data : undefined; + const axiosMsg = axios.isAxiosError(error) ? error.message : undefined; + console.error('Exchange API error:', axiosData || axiosMsg || error); // Enhanced error message for signature mismatches const errorMessage = - error.response?.data?.error || - error.message || + (typeof axiosData === 'object' && axiosData && 'error' in axiosData ? (axiosData as any).error : undefined) || + axiosMsg || + (error instanceof Error ? error.message : undefined) || 'Failed to execute action'; if (errorMessage.includes('does not exist')) { console.error( '⚠️ Likely EIP-712 typed data mismatch. Verify domain, types, primaryType, and chainId.' ); } throw new Error(errorMessage); } }
53-92: DRY: factor a typedpostInfo<T>()helper for INFO_URL callsAll info methods repeat the same axios boilerplate and error-shape assumptions; a
postInfo<T>(payload)helper would reduce duplication and keep runtime guards consistent.
228-249: Consider typinggetMetaAndAssetCtxs()instead ofanySince you already have
UniverseAsset/AssetContexttypes imported, returning a typed tuple/object here would reduce downstream casting and runtime mistakes.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/perps/lib/hyperliquid/client.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/perps/lib/hyperliquid/client.ts (2)
src/utils/blockchain.ts (1)
isTestnet(33-39)src/apps/perps/lib/hyperliquid/types.ts (4)
SignedAction(62-71)UserState(28-55)HyperliquidOrder(106-125)AssetInfo(73-78)
⏰ 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: Cloudflare Pages: x
- GitHub Check: build
| const BASE_URL = isTestnet | ||
| ? 'https://api.hyperliquid-testnet.xyz' | ||
| : 'https://api.hyperliquid.xyz'; | ||
|
|
||
| const EXCHANGE_URL = `${BASE_URL}/exchange`; | ||
| const INFO_URL = `${BASE_URL}/info`; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the blockchain utility file
find . -name "blockchain.ts" -o -name "blockchain.js" | head -5Repository: pillarwallet/x
Length of output: 249
🏁 Script executed:
# Also check the client file mentioned
find . -path "*/perps/lib/hyperliquid/client.ts" -o -path "*/perps/lib/hyperliquid/client.js"Repository: pillarwallet/x
Length of output: 100
🏁 Script executed:
# List files in utils directory to understand structure
ls -la src/utils/ 2>/dev/null | head -20Repository: pillarwallet/x
Length of output: 883
🏁 Script executed:
# Read the blockchain.ts file to understand isTestnet
cat -n src/utils/blockchain.ts | head -100Repository: pillarwallet/x
Length of output: 3751
🏁 Script executed:
# Read the client.ts file to see the usage
cat -n src/apps/perps/lib/hyperliquid/client.ts | head -50Repository: pillarwallet/x
Length of output: 2011
🏁 Script executed:
# Search for imports of the hyperliquid client
rg "from.*hyperliquid/client|import.*client.*hyperliquid" --type ts --type tsxRepository: pillarwallet/x
Length of output: 85
🏁 Script executed:
# Also check if there are any SSR-related imports or server-side code
rg "hyperliquid" --type ts --type tsx | grep -i "server\|ssr\|node"Repository: pillarwallet/x
Length of output: 85
🏁 Script executed:
# Check the package.json to see what kind of build/test setup is used
cat -n package.json | head -50Repository: pillarwallet/x
Length of output: 2517
🏁 Script executed:
# Search for imports of hyperliquid/client (without tsx file type)
rg "from.*hyperliquid/client|import.*client.*hyperliquid"Repository: pillarwallet/x
Length of output: 1151
🏁 Script executed:
# Also search more broadly for hyperliquid imports to see usage
rg "hyperliquid/client"Repository: pillarwallet/x
Length of output: 1569
🏁 Script executed:
# Check if there are any test files for this module
find . -path "*/hyperliquid/**" -name "*.test.*" -o -path "*/hyperliquid/**" -name "*.spec.*"Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
# Check if there are test files for the perps app
find . -path "*/perps/**" -name "*.test.*" -o -path "*/perps/**" -name "*.spec.*" | head -20Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
# Check if perps/index.tsx is tested anywhere
rg "perps" --glob="*.test.*" --glob="*.spec.*" | head -20Repository: pillarwallet/x
Length of output: 417
🏁 Script executed:
# Look for vitest config to understand test setup
find . -name "vitest.config.*" -o -name "vite.config.*" | xargs cat 2>/dev/null | head -80Repository: pillarwallet/x
Length of output: 1121
🏁 Script executed:
# Check the test setup file
cat -n src/test-utils/setupTests.ts | head -50Repository: pillarwallet/x
Length of output: 1948
🏁 Script executed:
# Also check if localStorage is mocked or available in tests
rg "localStorage" src/test-utils/ --context 2Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
# Look for any imports of perps components in the main test setup or globally
rg "perps" src/test-utils/Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
# Check if jsdom is being used and if localStorage is available
cat -n src/test-utils/setupTests.ts | tail -50Repository: pillarwallet/x
Length of output: 2492
🏁 Script executed:
# Also verify whether isTestnet is actually used in any test context
rg "isTestnet" --type tsRepository: pillarwallet/x
Length of output: 6571
🏁 Script executed:
# Let me verify the jsdom environment one more time and check if localStorage is actually available in the test setup
cat -n src/test-utils/setupTests.ts | grep -A5 -B5 "localStorage\|window\|global"Repository: pillarwallet/x
Length of output: 2567
Refactor isTestnet to avoid module-load localStorage access for SSR/Node.js compatibility
isTestnet is evaluated at import time via an IIFE that calls localStorage.getItem(), which will crash if this client is imported in Node.js, SSR, or test environments that don't provide window or localStorage. Although the current jsdom test setup provides localStorage, this pattern is fragile and will break if:
- Code is executed server-side or in Node.js CLI scripts
- Test environment switches from jsdom to Node
- Module is imported before DOM initialization
Refactor isTestnet in src/utils/blockchain.ts into a runtime function guarded by typeof window !== 'undefined', or ensure this client module is only imported client-side via dynamic imports.
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/client.ts around lines 5 - 11, The
import-time evaluation of isTestnet (used to build BASE_URL, EXCHANGE_URL,
INFO_URL) reads localStorage and can crash in SSR/Node; change isTestnet from an
IIFE/constant to a runtime function (e.g., isTestnet()) in
src/utils/blockchain.ts that guards access with typeof window !== 'undefined' &&
window.localStorage, then update this module to compute
BASE_URL/EXCHANGE_URL/INFO_URL at runtime (call isTestnet() inside the functions
or factory that use these URLs) so no localStorage access happens during module
import.
| export async function getOpenOrders( | ||
| address: string, | ||
| symbol?: string | ||
| ): Promise<HyperliquidOrder[]> { | ||
| try { | ||
| const response = await axios.post( | ||
| INFO_URL, | ||
| { | ||
| type: 'openOrders', | ||
| user: address, | ||
| }, | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| const orders = response.data || []; | ||
|
|
||
| // Filter by symbol if provided | ||
| if (symbol) { | ||
| return orders.filter((order: any) => order.coin === symbol); | ||
| } | ||
|
|
||
| return orders; | ||
| } catch (error: any) { | ||
| console.error('Open orders error:', error.response?.data || error.message); | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| export async function getFrontendOpenOrders( | ||
| address: string | ||
| ): Promise<HyperliquidOrder[]> { | ||
| try { | ||
| const response = await axios.post( | ||
| INFO_URL, | ||
| { | ||
| type: 'frontendOpenOrders', | ||
| user: address, | ||
| }, | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
| return response.data || []; | ||
| } catch (error: any) { | ||
| console.error('Frontend open orders error:', error.response?.data || error.message); | ||
| return []; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the specific file
find . -type f -name "client.ts" | grep -E "hyperliquid|perps"Repository: pillarwallet/x
Length of output: 100
🏁 Script executed:
# Let's also search for the file more broadly
fd "client.ts" | head -20Repository: pillarwallet/x
Length of output: 98
🏁 Script executed:
# Check repository structure
git ls-files | grep -E "hyperliquid|perps" | head -30Repository: pillarwallet/x
Length of output: 1395
🏁 Script executed:
# Read the specific file at the lines mentioned in the review
cat -n src/apps/perps/lib/hyperliquid/client.ts | sed -n '90,150p'Repository: pillarwallet/x
Length of output: 1741
🏁 Script executed:
# Also check the full file to see type definitions and imports
wc -l src/apps/perps/lib/hyperliquid/client.tsRepository: pillarwallet/x
Length of output: 102
🏁 Script executed:
# Check the beginning of the file for imports and type definitions
cat -n src/apps/perps/lib/hyperliquid/client.ts | head -40Repository: pillarwallet/x
Length of output: 1660
🏁 Script executed:
# Search for HyperliquidOrder type definition
rg "type HyperliquidOrder|interface HyperliquidOrder" src/apps/perps/Repository: pillarwallet/x
Length of output: 133
🏁 Script executed:
# Look for other API calls in the file to understand response patterns
rg "response\.data" src/apps/perps/lib/hyperliquid/client.ts -A 2 -B 2Repository: pillarwallet/x
Length of output: 1307
🏁 Script executed:
# Check the HyperliquidOrder type definition
cat -n src/apps/perps/lib/hyperliquid/types.ts | grep -A 10 "export interface HyperliquidOrder"Repository: pillarwallet/x
Length of output: 459
🏁 Script executed:
# Look for Hyperliquid API documentation or comments about response structure
rg "openOrders|frontendOpenOrders" src/apps/perps/ -B 2 -A 5Repository: pillarwallet/x
Length of output: 18165
🏁 Script executed:
# Check how other functions handle similar responses - look for defensive patterns
cat -n src/apps/perps/lib/hyperliquid/client.ts | sed -n '160,210p'Repository: pillarwallet/x
Length of output: 1460
🏁 Script executed:
# Check if there's any documentation or examples of what the Hyperliquid API returns
rg "openOrders|frontendOpenOrders" -B 5 -A 5 | head -60Repository: pillarwallet/x
Length of output: 3445
🏁 Script executed:
# Look at the actual usage of these functions to see if there are any guards elsewhere
grep -r "getOpenOrders\|getFrontendOpenOrders" src/apps/perps/ --include="*.ts" --include="*.tsx" -B 2 -A 3Repository: pillarwallet/x
Length of output: 6523
Add defensive Array.isArray() guards before array operations
getOpenOrders() calls .filter() on response.data without validating it's an array (line 112). Similarly, getFrontendOpenOrders() assumes response.data is an array when returning (line 142). While the Hyperliquid API currently returns arrays, defensive guards prevent runtime errors if the response structure changes or differs from expectations.
Proposed fix
- const orders = response.data || [];
+ const orders: HyperliquidOrder[] = Array.isArray(response.data) ? response.data : [];
// Filter by symbol if provided
if (symbol) {
return orders.filter((order: any) => order.coin === symbol);
}
return orders;- return response.data || [];
+ return Array.isArray(response.data) ? response.data : [];📝 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.
| export async function getOpenOrders( | |
| address: string, | |
| symbol?: string | |
| ): Promise<HyperliquidOrder[]> { | |
| try { | |
| const response = await axios.post( | |
| INFO_URL, | |
| { | |
| type: 'openOrders', | |
| user: address, | |
| }, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| const orders = response.data || []; | |
| // Filter by symbol if provided | |
| if (symbol) { | |
| return orders.filter((order: any) => order.coin === symbol); | |
| } | |
| return orders; | |
| } catch (error: any) { | |
| console.error('Open orders error:', error.response?.data || error.message); | |
| return []; | |
| } | |
| } | |
| export async function getFrontendOpenOrders( | |
| address: string | |
| ): Promise<HyperliquidOrder[]> { | |
| try { | |
| const response = await axios.post( | |
| INFO_URL, | |
| { | |
| type: 'frontendOpenOrders', | |
| user: address, | |
| }, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| return response.data || []; | |
| } catch (error: any) { | |
| console.error('Frontend open orders error:', error.response?.data || error.message); | |
| return []; | |
| } | |
| } | |
| export async function getOpenOrders( | |
| address: string, | |
| symbol?: string | |
| ): Promise<HyperliquidOrder[]> { | |
| try { | |
| const response = await axios.post( | |
| INFO_URL, | |
| { | |
| type: 'openOrders', | |
| user: address, | |
| }, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| const orders: HyperliquidOrder[] = Array.isArray(response.data) ? response.data : []; | |
| // Filter by symbol if provided | |
| if (symbol) { | |
| return orders.filter((order: any) => order.coin === symbol); | |
| } | |
| return orders; | |
| } catch (error: any) { | |
| console.error('Open orders error:', error.response?.data || error.message); | |
| return []; | |
| } | |
| } | |
| export async function getFrontendOpenOrders( | |
| address: string | |
| ): Promise<HyperliquidOrder[]> { | |
| try { | |
| const response = await axios.post( | |
| INFO_URL, | |
| { | |
| type: 'frontendOpenOrders', | |
| user: address, | |
| }, | |
| { | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| return Array.isArray(response.data) ? response.data : []; | |
| } catch (error: any) { | |
| console.error('Frontend open orders error:', error.response?.data || error.message); | |
| return []; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/client.ts around lines 94 - 147, Wrap and
validate response.data with Array.isArray before using array methods: in
getOpenOrders replace const orders = response.data || []; with something like
const orders = Array.isArray(response.data) ? response.data : []; and only call
orders.filter when orders is an array (so return [] if not); do the same in
getFrontendOpenOrders to return Array.isArray(response.data) ? response.data :
[]; this prevents runtime errors if the API returns non-array payloads.
| export async function getMarkPrice(symbol: string): Promise<number | null> { | ||
| try { | ||
| const response = await axios.post( | ||
| INFO_URL, | ||
| { | ||
| type: 'allMids', | ||
| }, | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (response.data && typeof response.data === 'object') { | ||
| const markPrice = response.data[symbol]; | ||
| return markPrice ? parseFloat(markPrice) : null; | ||
| } | ||
|
|
||
| return null; | ||
| } catch (error: any) { | ||
| console.error('Mark price error:', error.response?.data || error.message); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
getMarkPrice: avoid truthiness check and NaN leakage
return markPrice ? parseFloat(markPrice) : null; can mis-handle numeric 0 and can return NaN if parsing fails.
Proposed 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;
}🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/client.ts around lines 172 - 196, The
getMarkPrice function currently uses a truthiness check and parseFloat that
treats 0 as falsy and can return NaN; change the logic to explicitly check that
response.data[symbol] is not null or undefined (use markPrice !== undefined &&
markPrice !== null), convert it to a number safely (e.g., Number(markPrice) or
parseFloat), then validate the result with Number.isFinite (or !Number.isNaN)
and return the numeric value only when valid; otherwise return null. Ensure
references to markPrice, parseFloat/Number conversion, and the getMarkPrice
function are updated so 0 is preserved and invalid parses yield null.
| export async function getAllAssets(): Promise<AssetInfo[]> { | ||
| try { | ||
| const response = await axios.post( | ||
| INFO_URL, | ||
| { | ||
| type: 'meta', | ||
| }, | ||
| { | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (response.data && response.data.universe) { | ||
| return response.data.universe.map((asset: any, index: number) => ({ | ||
| id: index, | ||
| symbol: asset.name, | ||
| szDecimals: asset.szDecimals || 3, | ||
| maxLeverage: asset.maxLeverage || 50, | ||
| })); | ||
| } | ||
|
|
||
| return []; | ||
| } catch (error: any) { | ||
| console.error('Assets error:', error.response?.data || error.message); | ||
| return []; | ||
| } | ||
| } |
There was a problem hiding this comment.
getAllAssets: use ?? (don’t override valid 0) + validate universe array
asset.szDecimals || 3 and asset.maxLeverage || 50 will replace valid 0. Also, response.data.universe should be validated as an array before mapping.
Proposed 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,
}));
}🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/client.ts around lines 198 - 226, In
getAllAssets: avoid overriding valid zero values by replacing uses of
`asset.szDecimals || 3` and `asset.maxLeverage || 50` with the nullish
coalescing operator (`asset.szDecimals ?? 3`, `asset.maxLeverage ?? 50`), and
ensure `response.data.universe` is an array before mapping (use
`Array.isArray(response.data.universe)`); if it's not an array, return an empty
list or handle accordingly so mapping only runs on a validated array.
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.
| export function storeAgentWalletLocal( | ||
| masterAddress: string, | ||
| address: string, | ||
| privateKey: Hex, | ||
| approved?: boolean | ||
| ): void { | ||
| if (typeof window === 'undefined') return; | ||
|
|
||
| localStorage.setItem(getStorageKey(masterAddress, 'address'), address); | ||
| localStorage.setItem(getStorageKey(masterAddress, 'key'), privateKey); | ||
| if (approved !== undefined) { | ||
| localStorage.setItem( | ||
| getStorageKey(masterAddress, 'approved'), | ||
| String(approved) | ||
| ); | ||
| } | ||
| // Clear cache and session on new plaintext storage | ||
| clearSession(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, find the keystore.ts file and read it to see the full context
find . -name "keystore.ts" -type f | head -5Repository: pillarwallet/x
Length of output: 102
🏁 Script executed:
#!/bin/bash
# Check for usages of the legacy plaintext storage functions
rg -n --type=ts --type=tsx "storeAgentWalletLocal|getAgentWalletLocal" -C 3Repository: pillarwallet/x
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Look for encrypted storage alternatives and deprecation notices
rg -n --type=ts --type=tsx "encrypted|Encrypted|crypto|encryption" src/apps/perps/lib/hyperliquid/ -C 2Repository: pillarwallet/x
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Check for any deprecation comments or warnings in the keystore file
rg -n --type=ts --type=tsx "@deprecated|DEPRECATED|Legacy|LEGACY" src/apps/perps/lib/hyperliquid/keystore.ts -C 3Repository: pillarwallet/x
Length of output: 85
🏁 Script executed:
#!/bin/bash
# Read the keystore.ts file to see the full context
wc -l ./src/apps/perps/lib/hyperliquid/keystore.tsRepository: pillarwallet/x
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Read the keystore.ts file
cat -n ./src/apps/perps/lib/hyperliquid/keystore.tsRepository: pillarwallet/x
Length of output: 17579
🏁 Script executed:
#!/bin/bash
# Search for usages of legacy functions without type flag
rg -n "storeAgentWalletLocal|getAgentWalletLocal" -C 3Repository: pillarwallet/x
Length of output: 2084
🏁 Script executed:
#!/bin/bash
# Look for encryption/deprecated markers in keystore
rg -n "deprecated|Deprecated|LEGACY|Legacy|encrypted|Encrypted" ./src/apps/perps/lib/hyperliquid/keystore.ts -C 2Repository: pillarwallet/x
Length of output: 4727
🏁 Script executed:
#!/bin/bash
# Search for calls to storeAgentWalletLocal outside keystore.ts
rg "storeAgentWalletLocal" --type ts | grep -v "keystore.ts"Repository: pillarwallet/x
Length of output: 40
🏁 Script executed:
#!/bin/bash
# Also check for getAgentWalletLocal calls outside keystore.ts
rg "getAgentWalletLocal" --type ts | grep -v "keystore.ts"Repository: pillarwallet/x
Length of output: 40
Legacy plaintext storage still accessible—confirm migration is complete before removal.
The storeAgentWalletLocal function (line 132) stores private keys in plaintext in localStorage, creating XSS vulnerability. While already marked deprecated (line 121, 343), these functions remain exported and accessible as fallback paths in unlockAgentWallet (line 88) and getAgentWallet (line 382). No external callers were found in the codebase, suggesting migration to encrypted storage may be complete.
Verify migration status:
- If legacy plaintext data still exists in user localStorage: continue supporting fallback but add explicit migration warnings
- If all data has been migrated to encrypted storage: remove these functions entirely to eliminate the security surface
The encrypted alternative (storeAgentWalletEncrypted with PIN-based encryption) is already the primary path and actively clears legacy plaintext on upgrade (line 49-50).
🧰 Tools
🪛 ast-grep (0.40.4)
[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)
| // 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.
| 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) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Unsafe JSON parsing could throw unhandled exception.
Line 259 uses JSON.parse with a non-null assertion operator without wrapping in try-catch. If GLOBAL_ACCOUNT_KEY contains corrupted JSON data, this will throw an unhandled exception and crash the function.
🛡️ Proposed fix: Add try-catch for safe parsing
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)
- );
+ const newAddress = localStorage.getItem(GLOBAL_IMPORTED_ADDRESS_KEY);
+ if (newAddress) return newAddress;
+
+ const legacyData = localStorage.getItem(GLOBAL_ACCOUNT_KEY);
+ if (legacyData) {
+ try {
+ return JSON.parse(legacyData).accountAddress;
+ } catch {
+ return null;
+ }
+ }
+
+ return 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.
| 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) | |
| ); | |
| } | |
| export function getImportedAccountAddress(): string | null { | |
| if (typeof window === 'undefined') return null; | |
| const newAddress = localStorage.getItem(GLOBAL_IMPORTED_ADDRESS_KEY); | |
| if (newAddress) return newAddress; | |
| const legacyData = localStorage.getItem(GLOBAL_ACCOUNT_KEY); | |
| if (legacyData) { | |
| try { | |
| return JSON.parse(legacyData).accountAddress; | |
| } catch { | |
| return null; | |
| } | |
| } | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/keystore.ts around lines 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.
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.
| if ( | ||
| useBuilderFee && | ||
| (error?.message?.includes('Builder fee has not been approved') || | ||
| error?.response?.data?.includes('Builder fee has not been approved')) | ||
| ) { | ||
| // If useBuilderFee is true, it means signingAddress === masterAddress | ||
|
|
||
| // Check if we are using an imported account that effectively IS the master | ||
| const imported = getImportedAccount(); | ||
| if (imported && imported.accountAddress.toLowerCase() === masterAddress?.toLowerCase()) { | ||
| try { | ||
| toast.info('Approving Builder Fee...', { | ||
| id: toastId, | ||
| description: 'One-time approval required for trading.', | ||
| }); | ||
|
|
||
| await approveBuilderFeeSDK( | ||
| imported.privateKey, | ||
| BUILDER_ADDRESS, | ||
| BUILDER_FEE_APPROVAL | ||
| ); | ||
|
|
||
| toast.success('Builder Fee Approved!', { | ||
| id: toastId, | ||
| description: 'Please try placing your order again.', | ||
| duration: 5000, | ||
| }); | ||
| return; // Exit without showing generic error | ||
| } catch (approvalError: any) { | ||
| console.error('Failed to auto-approve builder fee:', approvalError); | ||
| toast.error('Failed to approve builder fee', { | ||
| id: toastId, | ||
| description: approvalError.message, | ||
| }); | ||
| return; | ||
| } | ||
| } else { | ||
| // We are using Agent (or Master Wallet via some other means) but don't have the private key to approve. | ||
| toast.error('Builder Fee Not Approved', { | ||
| id: toastId, | ||
| description: 'Please go to Settings > Perps Account and approve PillarX.', | ||
| duration: 5000 | ||
| }); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Unreachable auto-approval code for imported accounts.
The builder fee auto-approval logic at lines 659-686 can never execute due to a logical contradiction:
- Line 652 checks
if (useBuilderFee && ...) - Line 516 defines
useBuilderFee = !imported && (signingAddress === masterAddress) - Line 659 checks
if (imported && ...)
Since we're inside a block where useBuilderFee is true, the condition !imported must be true, which means imported must be false. Therefore, the check at line 659 if (imported && ...) can never be true.
This means imported accounts will never get auto-approval and will always fall through to the generic error message at line 699.
🔧 Suggested approach
The logic needs to be restructured. Consider checking for the builder fee error first (before checking useBuilderFee), then determining the appropriate action based on whether an imported account is in use:
// Handle "Builder fee has not been approved"
- // We only attempt to auto-approve if we are using an Imported Account AND it matches the signing address.
- // If we are using Agent (fallback), we cannot auto-approve because we don't have the Master key.
if (
- useBuilderFee &&
(error?.message?.includes('Builder fee has not been approved') ||
error?.response?.data?.includes('Builder fee has not been approved'))
) {
- // If useBuilderFee is true, it means signingAddress === masterAddress
-
- // Check if we are using an imported account that effectively IS the master
const imported = getImportedAccount();
if (imported && imported.accountAddress.toLowerCase() === masterAddress?.toLowerCase()) {
+ // Imported account matches master - can auto-approve
try {
toast.info('Approving Builder Fee...', {
id: toastId,
description: 'One-time approval required for trading.',
});
await approveBuilderFeeSDK(
imported.privateKey,
BUILDER_ADDRESS,
BUILDER_FEE_APPROVAL
);
toast.success('Builder Fee Approved!', {
id: toastId,
description: 'Please try placing your order again.',
duration: 5000,
});
return;
} catch (approvalError: any) {
console.error('Failed to auto-approve builder fee:', approvalError);
toast.error('Failed to approve builder fee', {
id: toastId,
description: approvalError.message,
});
return;
}
} else {
- // We are using Agent (or Master Wallet via some other means) but don't have the private key to approve.
+ // 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;
}
}📝 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.
| if ( | |
| useBuilderFee && | |
| (error?.message?.includes('Builder fee has not been approved') || | |
| error?.response?.data?.includes('Builder fee has not been approved')) | |
| ) { | |
| // If useBuilderFee is true, it means signingAddress === masterAddress | |
| // Check if we are using an imported account that effectively IS the master | |
| const imported = getImportedAccount(); | |
| if (imported && imported.accountAddress.toLowerCase() === masterAddress?.toLowerCase()) { | |
| try { | |
| toast.info('Approving Builder Fee...', { | |
| id: toastId, | |
| description: 'One-time approval required for trading.', | |
| }); | |
| await approveBuilderFeeSDK( | |
| imported.privateKey, | |
| BUILDER_ADDRESS, | |
| BUILDER_FEE_APPROVAL | |
| ); | |
| toast.success('Builder Fee Approved!', { | |
| id: toastId, | |
| description: 'Please try placing your order again.', | |
| duration: 5000, | |
| }); | |
| return; // Exit without showing generic error | |
| } catch (approvalError: any) { | |
| console.error('Failed to auto-approve builder fee:', approvalError); | |
| toast.error('Failed to approve builder fee', { | |
| id: toastId, | |
| description: approvalError.message, | |
| }); | |
| return; | |
| } | |
| } else { | |
| // We are using Agent (or Master Wallet via some other means) but don't have the private key to approve. | |
| toast.error('Builder Fee Not Approved', { | |
| id: toastId, | |
| description: 'Please go to Settings > Perps Account and approve PillarX.', | |
| duration: 5000 | |
| }); | |
| return; | |
| } | |
| } | |
| if ( | |
| (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()) { | |
| try { | |
| toast.info('Approving Builder Fee...', { | |
| id: toastId, | |
| description: 'One-time approval required for trading.', | |
| }); | |
| await approveBuilderFeeSDK( | |
| imported.privateKey, | |
| BUILDER_ADDRESS, | |
| BUILDER_FEE_APPROVAL | |
| ); | |
| toast.success('Builder Fee Approved!', { | |
| id: toastId, | |
| description: 'Please try placing your order again.', | |
| duration: 5000, | |
| }); | |
| return; | |
| } catch (approvalError: any) { | |
| console.error('Failed to auto-approve builder fee:', approvalError); | |
| toast.error('Failed to approve builder fee', { | |
| id: toastId, | |
| description: approvalError.message, | |
| }); | |
| return; | |
| } | |
| } 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; | |
| } | |
| } |
🤖 Prompt for AI Agents
In @src/apps/perps/components/TradeForm.tsx around lines 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.
| export async function approveAgentSDK( | ||
| masterPrivateKey: Hex, | ||
| agentAddress: string, | ||
| agentName?: string | ||
| ): Promise<any> { | ||
| const transport = new HttpTransport(); | ||
| const client = new ExchangeClient({ wallet: masterPrivateKey, transport }); | ||
|
|
||
| console.log('[SDK] Approving agent:', agentAddress); | ||
| const response = await client.approveAgent({ agentAddress, agentName }); | ||
| console.log('[SDK] Approve response:', response); | ||
|
|
||
| return response; | ||
| } |
There was a problem hiding this comment.
Inconsistent private key usage in ExchangeClient initialization.
Line 183 passes the raw masterPrivateKey directly to ExchangeClient, while approveBuilderFeeSDK (lines 200-202) and getExchangeClientForAgent (lines 9-11) first convert the private key to an account using privateKeyToAccount. This inconsistency could lead to type errors or unexpected behavior.
Given the commit message mentions "fix sdk private key usage", this appears to be a lingering issue that should be addressed.
🔧 Proposed 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 });
console.log('[SDK] Approving agent:', agentAddress);
const response = await client.approveAgent({ agentAddress, agentName });
console.log('[SDK] Approve response:', response);
return response;
}📝 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.
| export async function approveAgentSDK( | |
| masterPrivateKey: Hex, | |
| agentAddress: string, | |
| agentName?: string | |
| ): Promise<any> { | |
| const transport = new HttpTransport(); | |
| const client = new ExchangeClient({ wallet: masterPrivateKey, transport }); | |
| console.log('[SDK] Approving agent:', agentAddress); | |
| const response = await client.approveAgent({ agentAddress, agentName }); | |
| console.log('[SDK] Approve response:', response); | |
| return response; | |
| } | |
| 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: account, transport }); | |
| console.log('[SDK] Approving agent:', agentAddress); | |
| const response = await client.approveAgent({ agentAddress, agentName }); | |
| console.log('[SDK] Approve response:', response); | |
| return response; | |
| } |
🤖 Prompt for AI Agents
In @src/apps/perps/lib/hyperliquid/sdk.ts around lines 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.
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.
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.