Skip to content

Conversation

sidmorizon
Copy link
Contributor

@sidmorizon sidmorizon commented Sep 27, 2025

Summary by CodeRabbit

  • Refactor
    • Streamlined internal credential cleanup and deletion flow by adopting object-based parameters across services and callers.
    • Introduced a brief (1s) delay during orphaned credential cleanup to improve consistency when accounts are removed or reindexed.
    • Normalized identifiers before processing to reduce edge-case mismatches.
    • Updated all internal call sites to match the new parameter shape.
    • No user-facing changes or settings impacted.

This PR provides comprehensive improvements to the HyperLiquid perpetuals trading functionality, including account selection enhancements, error handling improvements, UI/UX refinements, and agent credential cleanup optimizations.

Changes

Account Selection & Management

  • Enhanced debug logging: Added comprehensive logging throughout the account selection process for better debugging
  • Auto-selection: Automatically select perps account when ETH accounts are added to wallet via event bus
  • Loading UX: Improved loading timeout from 0ms to 300ms for better user experience
  • Error handling: Enhanced error logging and handling in account selection flow

Agent Credential Cleanup

  • Parameter refactoring: Improved shouldDeleteCredential method with object parameters for better readability
  • Deletion logic: Added delay and logic to prevent wallet-level deletion when account-specific deletion is requested
  • Parameter optimization: Removed unnecessary walletId parameter when deleting specific accounts
  • Code cleanup: Better parameter passing in cleanup methods

UI/UX Improvements

  • Conditional deposit button: Hide deposit button when no perps account is selected for cleaner interface
  • Debug button redesign: Replace text-based debug button with cleaner icon-based button
  • Error toast cleanup: Remove redundant error toasts to prevent duplicate error messages

Error Handling & Code Quality

  • Error decorators: Add @toastIfError decorator to enableTrading method for consistent error handling
  • Type consistency: Fix atom setter type consistency in disposeExchangeClients
  • Error flow refactoring: Refactor invalid agent error handling to use service method instead of direct atom manipulation
  • Console log cleanup: Remove unnecessary console logs in subscription service

Technical Details

Files Modified

  • ServiceAccount.ts: Agent credential cleanup improvements
  • ServiceHyperliquid.ts: Account selection and error handling enhancements
  • ServiceHyperliquidSubscription.ts: Console log cleanup
  • withToast.ts: Error handling refactoring
  • PerpsGlobalEffects.tsx: Auto-selection logic and event handling
  • PerpsHeaderRight.tsx: UI improvements and conditional rendering
  • DepositWithdrawModal.tsx: Error handling cleanup

Test Plan

  • Test account selection flow when switching between accounts
  • Verify auto-selection works when adding new ETH accounts
  • Test deposit button visibility based on account selection state
  • Verify error handling shows appropriate toast messages without duplicates
  • Test agent credential cleanup when deleting accounts vs wallets
  • Test debug functionality in development environment
  • Verify proper error propagation in deposit/withdraw flows

🤖 Generated with Claude Code

sidmorizon and others added 3 commits September 27, 2025 09:46
- Add debug logging and error handling in perps account selection
- Adjust loading timeout delay from 0ms to 300ms for better UX
- Add auto-select perps account when ETH accounts are added to wallet
- Hide deposit button when no perps account is selected
- Replace debug button text with icon for cleaner UI
- Remove unnecessary console logs in subscription service

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Add @toastIfError decorator to enableTrading method for better error handling
- Fix atom setter type consistency in disposeExchangeClients
- Refactor invalid agent error handling to use service method instead of direct atom manipulation
- Remove redundant error toast in DepositWithdrawModal to avoid duplicate error messages

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
- Refactor shouldDeleteCredential method to use object parameters for better readability
- Add delay and logic to prevent wallet-level deletion when account-specific deletion is requested
- Fix parameter passing in cleanupOrphanedHyperLiquidAgentCredentials calls
- Remove unnecessary walletId parameter when deleting specific accounts

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

coderabbitai bot commented Sep 27, 2025

Walkthrough

Refactors two functions to accept parameter objects instead of multiple params. Updates internal call sites accordingly. Adds a one-second delay and walletId normalization in cleanupOrphanedHyperLiquidAgentCredentials when indexedAccountId or accountId are present. Reconstructs deletedInfo for downstream logic.

Changes

Cohort / File(s) Summary of Changes
ServiceAccount refactor
packages/kit-bg/src/services/ServiceAccount/ServiceAccount.ts
- shouldDeleteCredential now takes { addressRecord, deletedInfo }
- cleanupOrphanedHyperLiquidAgentCredentials now takes { walletId, indexedAccountId, accountId } and reconstructs deletedInfo internally
- Adds 1s delay and walletId normalization when indexedAccountId or accountId provided
- Updated internal callers to new object-arg style; removeAccount passes accountId/indexedAccountId instead of walletId

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Caller
  participant ServiceAccount
  participant CredentialStore as Credential Logic

  Caller->>ServiceAccount: cleanupOrphanedHyperLiquidAgentCredentials({ walletId?, indexedAccountId?, accountId? })
  alt indexedAccountId or accountId provided
    ServiceAccount->>ServiceAccount: wait 1s
    ServiceAccount->>ServiceAccount: normalize walletId
    ServiceAccount->>ServiceAccount: reconstruct deletedInfo
  else
    ServiceAccount->>ServiceAccount: use provided walletId/deletedInfo
  end
  ServiceAccount->>CredentialStore: shouldDeleteCredential({ addressRecord, deletedInfo })
  CredentialStore-->>ServiceAccount: decision
  ServiceAccount->>CredentialStore: delete or skip
  CredentialStore-->>ServiceAccount: result
  ServiceAccount-->>Caller: completion
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title mentions both HyperLiquid improvements and credential cleanup but it is overly broad and uses “comprehensive,” which is vague; a concise title should highlight the most impactful change and clearly signal what was fixed at a glance. Please refine the title to remove generic terms and focus on the central change, for example “fix(perp): add debug logging, auto-select perps account, hide deposit button and refactor agent credential cleanup.”
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/perps-agent-remove

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Cache: Disabled due to data retention organization setting

Knowledge base: Disabled due to data retention organization setting

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 8440258 and 05bf3b6.

📒 Files selected for processing (1)
  • packages/kit-bg/src/services/ServiceAccount/ServiceAccount.ts (3 hunks)
⏰ 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: unittest (20.x)
  • GitHub Check: lint (20.x)

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

@revan-zhang
Copy link
Contributor

revan-zhang commented Sep 27, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

@sidmorizon sidmorizon enabled auto-merge (squash) September 27, 2025 07:40
@sidmorizon sidmorizon changed the title fix(perp): comprehensive HyperLiquid improvements and agent credential cleanup fix: perps agent credential cleanup logic Sep 27, 2025
@sidmorizon sidmorizon merged commit bacbd06 into x Sep 27, 2025
11 checks passed
@sidmorizon sidmorizon deleted the fix/perps-agent-remove branch September 27, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants