feat(synapse-interface): Adds Hyperliquid bridge & deposit support #3461
Conversation
abtestingalpha
requested review from
bigboydiamonds and
aureliusbtc
as code owners
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/synapse-interface/components/HyperliquidDepositInfo.tsxOops! Something went wrong! :( ESLint: 8.57.1 Error: Failed to load parser '@babel/eslint-parser' declared in 'packages/synapse-interface/.eslintrc.js': Cannot find module '@babel/core/package.json'
WalkthroughThis pull request introduces several enhancements to the Synapse interface, primarily focused on the Hyperliquid platform. It adds new components for managing deposits and displaying deposit information, modifies existing components to accommodate Hyperliquid functionality, and updates localization files to include translations for new user interface strings. Additionally, it introduces new constants and modifies state management within various components to improve the handling of transactions related to Hyperliquid. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Deploying sanguine-fe with
|
| Latest commit: |
91798b5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f888faf1.sanguine-fe.pages.dev |
| Branch Preview URL: | https://fe-hyperliquid.sanguine-fe.pages.dev |
Deploying sanguine with
|
| Latest commit: |
91798b5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8f3e1524.sanguine.pages.dev |
| Branch Preview URL: | https://fe-hyperliquid.sanguine.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (7)
packages/synapse-interface/messages/fr.json (1)
21-22: Consider using "en cours" suffix for consistencyWhile the translations are accurate, consider updating the "Depositing" translation to "Dépôt en cours" to maintain consistency with other action states in the file (e.g., "Bridging" → "Transfert en cours", "Approving" → "Approbation en cours").
"Bridge": { "Deposit {symbol}": "Dépôt {symbol}", - "Depositing": "Dépôt", + "Depositing": "Dépôt en cours",packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx (2)
103-109: Provide user feedback on approval errorsCurrently, if an error occurs during the approval process, the user is not notified. Consider displaying an error message to inform the user about the issue.
You can modify the
catchblock to include user feedback:} catch (error) { console.error('Approval error:', error) + // Display error message to user + // Example: toast.error('Failed to approve the token. Please try again.') } finally { setIsApproving(false) }
116-150: Provide user feedback on deposit errorsIn the event of an error during the deposit process, only a console error is logged. Consider notifying the user about the error to enhance the user experience.
You can modify the
catchblock to inform the user:} catch (error) { console.error('Deposit error:', error) + // Display error message to user + // Example: toast.error('Failed to complete the deposit. Please try again.') } finally { setIsDepositing(false) }packages/synapse-interface/components/HyperliquidDepositInfo.tsx (2)
6-10: Add TypeScript type definitions for component propsTo improve type safety and code maintainability, consider adding explicit type definitions for the component props.
Define a type for the props and specify it in the component definition:
interface HyperliquidDepositInfoProps { fromChainId: number; isOnArbitrum: boolean; hasDepositedOnHyperliquid: boolean; } export const HyperliquidDepositInfo = ({ fromChainId, isOnArbitrum, hasDepositedOnHyperliquid, }: HyperliquidDepositInfoProps) => { // component code... }
17-17: Update thealttext to accurately describe the imageThe
alttext currently says "Switch Network", which may not accurately describe the image being displayed. Updating it will improve accessibility and provide better context for users using screen readers.For example:
- alt="Switch Network" + alt="Hyperliquid Chain Icon"packages/synapse-interface/constants/existingBridgeRoutes.ts (1)
1-2: Import specific lodash functions to reduce bundle sizeInstead of importing the entire lodash library, import only the required function to optimize bundle size. This helps reduce the application’s overall size and improves performance.
Change the import statement and update the code:
- import _ from 'lodash' + import { mapValues } from 'lodash' ... - return _.mapValues(routes, (innerList, key) => { + return mapValues(routes, (innerList, key) => {Also applies to: 55-55
packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts (1)
Line range hint
69-74: Consider adding Hyperliquid-specific validation testsThe bridge state validation logic has special handling for Hyperliquid, but there are no visible tests ensuring this behavior works correctly.
Would you like me to help create test cases for Hyperliquid bridge validation scenarios?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/synapse-interface/assets/chains/hyperliquid.svgis excluded by!**/*.svg
📒 Files selected for processing (17)
packages/synapse-interface/components/HyperliquidDepositInfo.tsx(1 hunks)packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx(1 hunks)packages/synapse-interface/components/StateManagedBridge/OutputContainer.tsx(4 hunks)packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts(2 hunks)packages/synapse-interface/components/_Transaction/_Transaction.tsx(2 hunks)packages/synapse-interface/components/_Transaction/_Transactions.tsx(2 hunks)packages/synapse-interface/constants/chains/master.tsx(2 hunks)packages/synapse-interface/constants/existingBridgeRoutes.ts(2 hunks)packages/synapse-interface/messages/ar.json(1 hunks)packages/synapse-interface/messages/en-US.json(1 hunks)packages/synapse-interface/messages/es.json(1 hunks)packages/synapse-interface/messages/fr.json(1 hunks)packages/synapse-interface/messages/jp.json(1 hunks)packages/synapse-interface/messages/tr.json(1 hunks)packages/synapse-interface/messages/zh-CN.json(1 hunks)packages/synapse-interface/pages/state-managed-bridge/index.tsx(12 hunks)packages/synapse-interface/slices/bridgeQuote/thunks.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx
[error] 213-221: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (12)
packages/synapse-interface/messages/jp.json (1)
21-22: LGTM! Translations are accurate.
Both "デポジット {symbol}" and "入金" are commonly used in Japanese financial contexts and correctly convey the intended meaning.
packages/synapse-interface/messages/ar.json (1)
21-22: LGTM! Translations are accurate.
Both "إيداع {symbol}" and "الإيداع" are the correct Arabic terms for deposit-related actions in financial contexts.
packages/synapse-interface/messages/en-US.json (2)
21-22: LGTM: New deposit-related strings are clear and consistent.
The new strings follow the existing pattern and maintain proper placeholder usage.
Line range hint 2-2: Improved consistency in punctuation.
The addition of periods at the end of these messages improves consistency across the interface.
Also applies to: 3-3, 4-4
packages/synapse-interface/messages/tr.json (1)
21-22: LGTM: Turkish translations are accurate and natural.
The translations maintain proper placeholder usage and use correct Turkish terminology.
packages/synapse-interface/messages/es.json (1)
21-22: LGTM: Spanish translations are accurate and natural.
The translations maintain proper placeholder usage and use correct Spanish terminology with proper accentuation.
packages/synapse-interface/components/_Transaction/_Transactions.tsx (1)
65-69: Verify the transaction status handling for Hyperliquid
The code forces all Hyperliquid transactions to show as 'completed' regardless of their actual status. This could potentially hide failed or pending transactions from users.
Let's check if there's any special handling for Hyperliquid transaction statuses elsewhere:
✅ Verification successful
Hyperliquid status handling is intentionally set to 'completed'
Based on the codebase analysis, this is an intentional design choice. The evidence shows that:
- Hyperliquid transactions are handled through a special flow using
HyperliquidTransactionButtonandHyperliquidDepositInfocomponents - The transactions are actually deposits to a specific Hyperliquid deposit address (
0x2Df1c51E09aECF9cacB7bc98cB1742757f163dF7) on Arbitrum - The UI treats Hyperliquid as a special destination with ID 998, and all transactions are routed through Arbitrum first
- Once the deposit is confirmed on Arbitrum, it's considered "completed" since there's no way to track the subsequent status on Hyperliquid directly
This implementation is correct as the transaction status refers to the deposit on Arbitrum, which is the actual on-chain transaction being performed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Hyperliquid transaction status handling
rg -l "HYPERLIQUID.*status|status.*HYPERLIQUID" --type ts --type tsx
Length of output: 98
Script:
#!/bin/bash
# Search for Hyperliquid transaction status handling with correct file extensions
rg -l "HYPERLIQUID.*status|status.*HYPERLIQUID"
# Also search for general Hyperliquid-related code to understand the context
rg -A 5 "HYPERLIQUID"
# Look for transaction status handling patterns
rg -A 5 "destinationChain.*status|status.*destinationChain"
Length of output: 20736
packages/synapse-interface/components/StateManagedBridge/OutputContainer.tsx (2)
57-61: Verify amount display logic for Arbitrum to Hyperliquid bridge
When bridging from Arbitrum to Hyperliquid, the input amount is shown directly instead of the calculated output amount. This differs from other bridge paths.
Let's check if this is consistent with other Hyperliquid-related amount handling:
✅ Verification successful
Special handling for Arbitrum to Hyperliquid bridge path is intentional
The different amount display logic for Arbitrum to Hyperliquid bridge is a deliberate design choice, consistently implemented across the codebase. The search results show this special case is handled in multiple places:
OutputContainer.tsx: Shows input amount instead of calculated outputHyperliquidDepositButton.tsx: Uses a specific deposit address and custom transaction flowuseBridgeValidations.ts: Contains special validation logic for this path- Multiple checks in
state-managed-bridge/index.tsxconfirming this is an intentional architectural decision
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Hyperliquid amount handling
rg -A 5 "amount.*HYPERLIQUID|HYPERLIQUID.*amount" --type ts --type tsx
Length of output: 100
Script:
#!/bin/bash
# Search for Hyperliquid amount handling with correct file extensions
rg -A 5 "amount.*HYPERLIQUID|HYPERLIQUID.*amount" -t typescript -t tsx
# Search for any Hyperliquid-related logic in bridge components
rg -A 10 "HYPERLIQUID" packages/synapse-interface/components/StateManagedBridge/
# Look for other instances of fromChainId/toChainId comparisons
ast-grep --pattern 'fromChainId === $_ && toChainId === $_'
Length of output: 11803
48-50: Verify destination address handling for Hyperliquid
The destination address input is hidden when bridging to Hyperliquid. Please confirm:
- Is this intentional?
- How are funds directed to the correct destination on Hyperliquid?
Let's check how destination addresses are handled for Hyperliquid:
✅ Verification successful
Destination address handling for Hyperliquid is correctly implemented
The code is working as intended. For Hyperliquid bridges:
- Funds are sent to a fixed deposit address (
0x2Df1c51E09aECF9cacB7bc98cB1742757f163dF7) - The destination address input is intentionally hidden as it's not needed
- The bridge flow is handled through a specialized
HyperliquidTransactionButtoncomponent that manages the deposit process
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Hyperliquid destination address handling
rg -A 5 "destinationAddress.*HYPERLIQUID|HYPERLIQUID.*destinationAddress" --type ts --type tsx
Length of output: 124
Script:
#!/bin/bash
# Search for Hyperliquid destination address handling with correct file extensions
rg -A 5 "destinationAddress.*HYPERLIQUID|HYPERLIQUID.*destinationAddress"
# Search for general Hyperliquid-related code to understand the context
rg -A 5 "HYPERLIQUID"
# Look for destination address handling patterns
ast-grep --pattern 'DestinationAddressInput'
Length of output: 21198
Script:
#!/bin/bash
# Let's check the Hyperliquid deposit flow
ast-grep --pattern 'HyperliquidTransactionButton'
# Check for any deposit address validation or handling
rg -A 5 "HYPERLIQUID_DEPOSIT_ADDRESS"
Length of output: 2013
packages/synapse-interface/components/StateManagedBridge/hooks/useBridgeValidations.ts (1)
70-70: Review Hyperliquid validation strategy
The code replaces HYPERLIQUID.id with ARBITRUM.id in the bridge state validation. This could mask Hyperliquid-specific validation issues and lead to incorrect quote comparisons.
Let's check for other Hyperliquid-specific validations:
packages/synapse-interface/components/_Transaction/_Transaction.tsx (1)
189-197: LGTM: Explorer link handling for Hyperliquid
The condition correctly prevents displaying the explorer link for Hyperliquid transactions, which is appropriate since Hyperliquid may use a different transaction tracking mechanism.
packages/synapse-interface/constants/chains/master.tsx (1)
623-623: Clarify environment configuration
The comment indicates this is Hyperliquid Testnet, but there's no clear separation between testnet and mainnet configurations. Consider adding environment indicators or separate configurations.
| @@ -18,6 +18,8 @@ | |||
| "Please select Destination network": "请选择目标网络", | |||
| "Please select an Origin token": "请选择来源代币", | |||
| "Bridge {symbol}": "桥接 {symbol}", | |||
| "Deposit {symbol}": "订金 {symbol}", | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a more accurate translation for "Deposit {symbol}"
The current translation "订金 {symbol}" implies a down payment or reservation fee. Consider using "存入 {symbol}" which better represents the action of depositing funds.
- "Deposit {symbol}": "订金 {symbol}",
+ "Deposit {symbol}": "存入 {symbol}",📝 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.
| "Deposit {symbol}": "订金 {symbol}", | |
| "Deposit {symbol}": "存入 {symbol}", |
| <> | ||
| <div className="flex flex-col w-full"> | ||
| <TransactionButton | ||
| {...buttonProperties} | ||
| disabled={isButtonDisabled} | ||
| chainId={fromChainId} | ||
| /> | ||
| </div> | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary Fragment to simplify JSX structure
The Fragment (<> and </>) wrapping the <div> is redundant since it contains only one child. Removing it will simplify the JSX structure without affecting functionality.
Apply the following diff to remove the unnecessary Fragment:
return (
buttonProperties && (
- <>
<div className="flex flex-col w-full">
<TransactionButton
{...buttonProperties}
disabled={isButtonDisabled}
chainId={fromChainId}
/>
</div>
- </>
)
)Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 213-221: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
| @@ -154,7 +168,7 @@ const StateManagedBridge = () => { | |||
| fetchBridgeQuote({ | |||
| synapseSDK, | |||
| fromChainId, | |||
| toChainId, | |||
| toChainId: toChainId === HYPERLIQUID.id ? 42161 : toChainId, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace hardcoded chain ID '42161' with 'ARBITRUM.id' constant
To improve code clarity and maintainability, replace the hardcoded chain ID 42161 with the constant ARBITRUM.id. This ensures consistency and makes it easier to update chain IDs in the future.
Apply the following changes:
- Line 171:
toChainId: toChainId === HYPERLIQUID.id ? 42161 : toChainId, + toChainId: toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainId, - Line 296:
destinationChainId: - toChainId === HYPERLIQUID.id ? 42161 : toChainId, + toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainId,
- Line 316:
destinationChain: - CHAINS_BY_ID[toChainId === HYPERLIQUID.id ? 42161 : toChainId], + CHAINS_BY_ID[toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainId],
- Line 341:
fromChainId, - toChainId === HYPERLIQUID.id ? 42161 : toChainId, + toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainId,
- Line 379:
destinationChainId: - toChainId === HYPERLIQUID.id ? 42161 : toChainId, + toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainId,
Also applies to: 296-296, 316-316, 341-341, 379-379
| if ( | ||
| !( | ||
| originQuery && | ||
| maxAmountOut && | ||
| destQuery && | ||
| feeAmount && | ||
| toChainId !== HYPERLIQUID.id | ||
| ) | ||
| ) { |
There was a problem hiding this comment.
💡 Codebase verification
Based on the gathered evidence, I can now provide a final response:
Bypassing quote validation for Hyperliquid requires additional validation logic
The code shows that Hyperliquid (chain ID 998) is treated as a special case, but there's no evidence of alternative validation logic or safety checks specific to Hyperliquid quotes. The only special handling found is in the bridge UI (state-managed-bridge/index.tsx) where it forces Arbitrum as the source chain, but this doesn't address the quote validation bypass.
- The quote validation being skipped (
toChainId !== HYPERLIQUID.id) removes important safety checks for:- Origin query validation
- Maximum amount validation
- Destination query validation
- Fee amount validation
🔗 Analysis chain
Verify the safety of bypassing quote validation for Hyperliquid
The condition toChainId !== HYPERLIQUID.id bypasses the standard quote validation checks for Hyperliquid. This could be a security risk if not properly handled elsewhere.
Let me gather more context about the Hyperliquid handling in the codebase.
Let me check for any special handling or validation logic around Hyperliquid quotes.
Let me check one final aspect to understand the complete flow of Hyperliquid bridge quotes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any additional validation specific to Hyperliquid
rg -A 5 "HYPERLIQUID.id.*validation|validation.*HYPERLIQUID.id"
# Look for any safety checks or handlers specific to Hyperliquid
ast-grep --pattern 'if ($_ === HYPERLIQUID.id) {
$$$
}'
Length of output: 772
Script:
#!/bin/bash
# Search for Hyperliquid-related code in thunks.ts
rg -A 10 "HYPERLIQUID" packages/synapse-interface/slices/bridgeQuote/thunks.ts
# Look for quote validation logic
ast-grep --pattern 'fetchBridgeQuote($$$) {
$$$
}'
# Search for any Hyperliquid-specific error handling
rg -A 5 "error.*HYPERLIQUID|HYPERLIQUID.*error"
Length of output: 893
Script:
#!/bin/bash
# Search for the complete fetchBridgeQuote function implementation
rg -B 10 -A 20 "fetchBridgeQuote.*createAsyncThunk" packages/synapse-interface/slices/bridgeQuote/thunks.ts
# Look for any Hyperliquid-specific quote or validation logic
rg -l "HYPERLIQUID" packages/synapse-interface --type ts | xargs rg -A 10 "quote.*HYPERLIQUID|HYPERLIQUID.*quote"
# Check for any Hyperliquid-specific configuration or constants
rg -A 5 "HYPERLIQUID.*=|export.*HYPERLIQUID" packages/synapse-interface/constants/
Length of output: 1884
Script:
#!/bin/bash
# Search for the complete bridge quote validation logic
rg -B 5 -A 15 "if \(.*originQuery.*maxAmountOut.*destQuery" packages/synapse-interface/slices/bridgeQuote/
# Look for any special Hyperliquid bridge handling
rg -B 5 -A 10 "bridge.*HYPERLIQUID|HYPERLIQUID.*bridge" packages/synapse-interface/
# Check for any Hyperliquid-specific route or path handling
ast-grep --pattern 'if ($_ === HYPERLIQUID.id) {
$$$
return $$$
}'
Length of output: 267
Bundle ReportChanges will decrease total bundle size by 2.62MB (-7.36%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (1)
171-171: Consider refactoring repeated ternary operation into a helper functionThe ternary operation
toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainIdis used multiple times (lines 171, 296, 316, 341, 379). To improve code maintainability and reduce duplication, consider extracting this logic into a helper function, such asgetDestinationChainId(toChainId).Here's how you might define the helper function:
const getDestinationChainId = (toChainId) => toChainId === HYPERLIQUID.id ? ARBITRUM.id : toChainId;Then replace the ternary operations with calls to
getDestinationChainId(toChainId).Also applies to: 296-296, 316-316, 341-341, 379-379
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx (1)
99-110: Provide user feedback on approval and deposit errorsIn the
handleApproveandhandleDepositfunctions, errors are caught and logged but no feedback is provided to the user. Consider displaying error messages to inform users about any issues during the approval or deposit process, enhancing the user experience.For example, you can use a notification or toast component to display the error message:
import toast from 'react-hot-toast'; // In handleApprove catch block catch (error) { console.error('Approval error:', error); toast.error('Approval failed. Please try again.'); } // In handleDeposit catch block catch (error) { console.error('Deposit error:', error); toast.error('Deposit failed. Please try again.'); }Also applies to: 112-151
packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx (1)
63-63: LocalizebannerContentfor internationalization supportThe
bannerContentprop is currently hardcoded in English. To support internationalization, consider using thetfunction fromuseTranslationsto localize the banner content.For example, you can import
useTranslationsand updatebannerContentas:import { useTranslations } from 'next-intl'; const t = useTranslations('AnnouncementBanner'); // ... <AnnouncementBanner bannerId="2024-12-11-hyperliquid" bannerContent={t('Synapse now supports Hyperliquid. Bridge and deposit to your Hyperliquid account now!')} startDate={new Date('2024-12-11T18:45:09+00:00')} endDate={new Date('2025-01-10T18:45:09+00:00')} />Ensure that the translation string is added to your localization files.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx(1 hunks)packages/synapse-interface/components/layouts/LandingPageWrapper/index.tsx(1 hunks)packages/synapse-interface/messages/zh-CN.json(1 hunks)packages/synapse-interface/pages/state-managed-bridge/index.tsx(12 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/synapse-interface/messages/zh-CN.json
🧰 Additional context used
📓 Learnings (1)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (1)
Learnt from: abtestingalpha
PR: synapsecns/sanguine#3461
File: packages/synapse-interface/pages/state-managed-bridge/index.tsx:438-438
Timestamp: 2024-12-13T18:55:49.060Z
Learning: In `packages/synapse-interface/pages/state-managed-bridge/index.tsx`, the `switchChain` function from `@wagmi/core` can be called with `wagmiConfig` and an object containing `{ chainId: ARBITRUM.id }` as arguments.
🪛 Biome (1.9.4)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx
[error] 213-221: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (4)
packages/synapse-interface/pages/state-managed-bridge/index.tsx (3)
12-12: Import of switchChain is appropriate
The import of switchChain from @wagmi/core is correctly added to handle chain switching functionality.
76-81: Approved: Imports for Hyperliquid integration
The imports of ARBITRUM, HYPERLIQUID, HyperliquidTransactionButton, USDC, CheckCircleIcon, Image, and HyperliquidDepositInfo are appropriate for the added Hyperliquid functionality.
100-102: State variable hasDepositedOnHyperliquid initialized correctly
The new state variable hasDepositedOnHyperliquid is correctly initialized to false using useState.
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx (1)
213-221: Remove unnecessary Fragment to simplify JSX structure
The <Fragment> wrapping the <div> is redundant since it contains only one child. Removing it will simplify the JSX structure without affecting functionality.
Apply the following diff to remove the unnecessary Fragment:
return (
buttonProperties && (
- <>
<div className="flex flex-col w-full">
<TransactionButton
{...buttonProperties}
disabled={isButtonDisabled}
chainId={fromChainId}
/>
</div>
- </>
)
)🧰 Tools
🪛 Biome (1.9.4)
[error] 213-221: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
packages/synapse-interface/components/HyperliquidDepositInfo.tsx (1)
10-27: Refactor repetitive JSX into reusable componentsThe JSX code for rendering the steps is repeated with minor differences across multiple blocks. Consider refactoring these into reusable components or functions to improve maintainability and reduce duplication.
Also applies to: 33-50, 56-73
| height="32" | ||
| rx="16" | ||
| stroke="#565058" | ||
| stroke-width="2" |
There was a problem hiding this comment.
Use camelCase for SVG attribute 'strokeWidth' in JSX
In JSX, SVG attributes should be camelCase. Replace stroke-width with strokeWidth to ensure proper rendering without warnings or errors.
Apply this diff to correct the attribute names:
<rect
x="1"
y="1"
width="32"
height="32"
rx="16"
stroke="#565058"
- stroke-width="2"
+ strokeWidth="2"
/>
...
- <circle cx="17" cy="17" r="16" stroke="#565058" stroke-width="2" />
+ <circle cx="17" cy="17" r="16" stroke="#565058" strokeWidth="2" />Also applies to: 100-100
| export const HyperliquidDepositInfo = ({ | ||
| fromChainId, | ||
| isOnArbitrum, | ||
| hasDepositedOnHyperliquid, | ||
| }) => { | ||
| if (fromChainId !== ARBITRUM.id) { | ||
| return ( | ||
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | ||
| <div className="flex justify-between"> | ||
| <div> | ||
| <div className="text-[#97FCE4] mb-2">Step 1</div> | ||
| <div className="flex items-center space-x-2"> | ||
| <GreenStep1Circle /> | ||
| <div>Bridge (Arbitrum)</div> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <div className="mb-2 text-white/65">Step 2</div> | ||
| <div className="flex items-center space-x-2"> | ||
| <GrayStep2Circle /> | ||
| <div className="text-white/65">Deposit (Hyperliquid)</div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| if (hasDepositedOnHyperliquid) { | ||
| return ( | ||
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | ||
| <div className="flex justify-between"> | ||
| <div> | ||
| <div className="mb-2 text-white/65">Step 1</div> | ||
| <div className="flex items-center space-x-2"> | ||
| <CompletedCheckMarkCircle /> | ||
| <div className="text-white/65">Bridge (Arbitrum)</div> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <div className="mb-2 text-white/65">Step 2</div> | ||
| <div className="flex items-center space-x-2"> | ||
| <CompletedCheckMarkCircle /> | ||
| <div className="text-white/65">Deposit (Hyperliquid)</div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
|
|
||
| if (fromChainId === ARBITRUM.id && isOnArbitrum) { | ||
| return ( | ||
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | ||
| <div className="flex justify-between"> | ||
| <div> | ||
| <div className="mb-2 text-white/65">Step 1</div> | ||
| <div className="flex items-center space-x-2"> | ||
| <CompletedCheckMarkCircle /> | ||
| <div className="text-white/65">Bridge (Arbitrum)</div> | ||
| </div> | ||
| </div> | ||
| <div> | ||
| <div className="mb-2 text-[#97FCE4]">Step 2</div> | ||
| <div className="flex items-center space-x-2"> | ||
| <GreenStep2Circle /> | ||
| <div className="text-[#97FCE4]">Deposit (Hyperliquid)</div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure all code paths return a value in 'HyperliquidDepositInfo' component
The HyperliquidDepositInfo function may not return any value if none of the conditions are met. This could lead to rendering issues or unexpected behavior. Ensure that all possible code paths return a React element.
Consider adding a default return statement at the end of the function:
}
+ return null; // or return a default component or message
}📝 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 const HyperliquidDepositInfo = ({ | |
| fromChainId, | |
| isOnArbitrum, | |
| hasDepositedOnHyperliquid, | |
| }) => { | |
| if (fromChainId !== ARBITRUM.id) { | |
| return ( | |
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | |
| <div className="flex justify-between"> | |
| <div> | |
| <div className="text-[#97FCE4] mb-2">Step 1</div> | |
| <div className="flex items-center space-x-2"> | |
| <GreenStep1Circle /> | |
| <div>Bridge (Arbitrum)</div> | |
| </div> | |
| </div> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 2</div> | |
| <div className="flex items-center space-x-2"> | |
| <GrayStep2Circle /> | |
| <div className="text-white/65">Deposit (Hyperliquid)</div> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } | |
| if (hasDepositedOnHyperliquid) { | |
| return ( | |
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | |
| <div className="flex justify-between"> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 1</div> | |
| <div className="flex items-center space-x-2"> | |
| <CompletedCheckMarkCircle /> | |
| <div className="text-white/65">Bridge (Arbitrum)</div> | |
| </div> | |
| </div> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 2</div> | |
| <div className="flex items-center space-x-2"> | |
| <CompletedCheckMarkCircle /> | |
| <div className="text-white/65">Deposit (Hyperliquid)</div> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } | |
| if (fromChainId === ARBITRUM.id && isOnArbitrum) { | |
| return ( | |
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | |
| <div className="flex justify-between"> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 1</div> | |
| <div className="flex items-center space-x-2"> | |
| <CompletedCheckMarkCircle /> | |
| <div className="text-white/65">Bridge (Arbitrum)</div> | |
| </div> | |
| </div> | |
| <div> | |
| <div className="mb-2 text-[#97FCE4]">Step 2</div> | |
| <div className="flex items-center space-x-2"> | |
| <GreenStep2Circle /> | |
| <div className="text-[#97FCE4]">Deposit (Hyperliquid)</div> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } | |
| } | |
| export const HyperliquidDepositInfo = ({ | |
| fromChainId, | |
| isOnArbitrum, | |
| hasDepositedOnHyperliquid, | |
| }) => { | |
| if (fromChainId !== ARBITRUM.id) { | |
| return ( | |
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | |
| <div className="flex justify-between"> | |
| <div> | |
| <div className="text-[#97FCE4] mb-2">Step 1</div> | |
| <div className="flex items-center space-x-2"> | |
| <GreenStep1Circle /> | |
| <div>Bridge (Arbitrum)</div> | |
| </div> | |
| </div> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 2</div> | |
| <div className="flex items-center space-x-2"> | |
| <GrayStep2Circle /> | |
| <div className="text-white/65">Deposit (Hyperliquid)</div> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } | |
| if (hasDepositedOnHyperliquid) { | |
| return ( | |
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | |
| <div className="flex justify-between"> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 1</div> | |
| <div className="flex items-center space-x-2"> | |
| <CompletedCheckMarkCircle /> | |
| <div className="text-white/65">Bridge (Arbitrum)</div> | |
| </div> | |
| </div> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 2</div> | |
| <div className="flex items-center space-x-2"> | |
| <CompletedCheckMarkCircle /> | |
| <div className="text-white/65">Deposit (Hyperliquid)</div> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } | |
| if (fromChainId === ARBITRUM.id && isOnArbitrum) { | |
| return ( | |
| <div className="flex flex-col p-2 mb-2 space-y-1 text-sm "> | |
| <div className="flex justify-between"> | |
| <div> | |
| <div className="mb-2 text-white/65">Step 1</div> | |
| <div className="flex items-center space-x-2"> | |
| <CompletedCheckMarkCircle /> | |
| <div className="text-white/65">Bridge (Arbitrum)</div> | |
| </div> | |
| </div> | |
| <div> | |
| <div className="mb-2 text-[#97FCE4]">Step 2</div> | |
| <div className="flex items-center space-x-2"> | |
| <GreenStep2Circle /> | |
| <div className="text-[#97FCE4]">Deposit (Hyperliquid)</div> | |
| </div> | |
| </div> | |
| </div> | |
| </div> | |
| ) | |
| } | |
| return null; // or return a default component or message | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx (1)
151-155: Consider adding more context to the minimum deposit messageThe current message only shows the minimum amount but doesn't explain why it exists or what it means for the user.
- label: `${HYPERLIQUID_MINIMUM_DEPOSIT} USDC Minimum`, + label: `Minimum deposit of ${HYPERLIQUID_MINIMUM_DEPOSIT} USDC required for Hyperliquid`,packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx (1)
27-27: Consider moving sensitive addresses to environment variablesHardcoding deposit addresses directly in the source code can make it difficult to update and poses security risks if the repository is public.
Consider moving
HYPERLIQUID_DEPOSIT_ADDRESSto environment variables or a secure configuration management system.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx(4 hunks)packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx(1 hunks)packages/synapse-interface/constants/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/synapse-interface/constants/index.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/synapse-interface/components/StateManagedBridge/BridgeTransactionButton.tsx
[error] 64-66: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx
[error] 215-223: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (1)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx (1)
215-223: Remove unnecessary Fragment wrapper
The Fragment wrapper is redundant since it contains only one child element.
🧰 Tools
🪛 Biome (1.9.4)
[error] 215-223: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
| const hasHyperliquidMinDeposit = | ||
| toChainId === HYPERLIQUID.id | ||
| ? Number(debouncedFromValue) > HYPERLIQUID_MINIMUM_DEPOSIT | ||
| ? true | ||
| : false | ||
| : true |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the nested ternary operator
The nested ternary operator can be simplified to improve readability.
- const hasHyperliquidMinDeposit =
- toChainId === HYPERLIQUID.id
- ? Number(debouncedFromValue) > HYPERLIQUID_MINIMUM_DEPOSIT
- ? true
- : false
- : true
+ const hasHyperliquidMinDeposit =
+ toChainId !== HYPERLIQUID.id ||
+ Number(debouncedFromValue) > HYPERLIQUID_MINIMUM_DEPOSIT📝 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 hasHyperliquidMinDeposit = | |
| toChainId === HYPERLIQUID.id | |
| ? Number(debouncedFromValue) > HYPERLIQUID_MINIMUM_DEPOSIT | |
| ? true | |
| : false | |
| : true | |
| const hasHyperliquidMinDeposit = | |
| toChainId !== HYPERLIQUID.id || | |
| Number(debouncedFromValue) > HYPERLIQUID_MINIMUM_DEPOSIT |
🧰 Tools
🪛 Biome (1.9.4)
[error] 64-66: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
| const approve = async (address: Address, amount: bigint) => { | ||
| const { request } = await simulateContract(wagmiConfig, { | ||
| chainId: ARBITRUM.id, | ||
| address: USDC.addresses[ARBITRUM.id], | ||
| abi: erc20Abi, | ||
| functionName: 'approve', | ||
| args: [address, amount], | ||
| }) | ||
|
|
||
| const hash = await writeContract(wagmiConfig, request) | ||
|
|
||
| const txReceipt = await waitForTransactionReceipt(wagmiConfig, { hash }) | ||
|
|
||
| return txReceipt | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling in approve function
The approve function should include proper error handling and validation.
const approve = async (address: Address, amount: bigint) => {
+ if (!address || amount <= 0n) {
+ throw new Error('Invalid approve parameters')
+ }
+
const { request } = await simulateContract(wagmiConfig, {
chainId: ARBITRUM.id,
address: USDC.addresses[ARBITRUM.id],
abi: erc20Abi,
functionName: 'approve',
args: [address, amount],
})
const hash = await writeContract(wagmiConfig, request)
const txReceipt = await waitForTransactionReceipt(wagmiConfig, { hash })
return txReceipt
}📝 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 approve = async (address: Address, amount: bigint) => { | |
| const { request } = await simulateContract(wagmiConfig, { | |
| chainId: ARBITRUM.id, | |
| address: USDC.addresses[ARBITRUM.id], | |
| abi: erc20Abi, | |
| functionName: 'approve', | |
| args: [address, amount], | |
| }) | |
| const hash = await writeContract(wagmiConfig, request) | |
| const txReceipt = await waitForTransactionReceipt(wagmiConfig, { hash }) | |
| return txReceipt | |
| } | |
| const approve = async (address: Address, amount: bigint) => { | |
| if (!address || amount <= 0n) { | |
| throw new Error('Invalid approve parameters') | |
| } | |
| const { request } = await simulateContract(wagmiConfig, { | |
| chainId: ARBITRUM.id, | |
| address: USDC.addresses[ARBITRUM.id], | |
| abi: erc20Abi, | |
| functionName: 'approve', | |
| args: [address, amount], | |
| }) | |
| const hash = await writeContract(wagmiConfig, request) | |
| const txReceipt = await waitForTransactionReceipt(wagmiConfig, { hash }) | |
| return txReceipt | |
| } |
| const handleDeposit = async () => { | ||
| setIsDepositing(true) | ||
| const currentTimestamp: number = getUnixTimeMinutesFromNow(0) | ||
| try { | ||
| const txReceipt = await deposit(amount) | ||
|
|
||
| setHasDepositedOnHyperliquid(true) | ||
| setIsApproved(false) | ||
| segmentAnalyticsEvent(`[Hyperliquid Deposit]`, { | ||
| inputAmount: debouncedFromValue, | ||
| }) | ||
| dispatch( | ||
| fetchAndStoreSingleNetworkPortfolioBalances({ | ||
| address, | ||
| chainId: ARBITRUM.id, | ||
| }) | ||
| ) | ||
| dispatch( | ||
| addPendingBridgeTransaction({ | ||
| id: currentTimestamp, | ||
| originChain: ARBITRUM, | ||
| originToken: fromToken, | ||
| originValue: debouncedFromValue, | ||
| destinationChain: HYPERLIQUID, | ||
| destinationToken: undefined, | ||
| transactionHash: txReceipt.transactionHash, | ||
| timestamp: undefined, | ||
| isSubmitted: false, | ||
| estimatedTime: undefined, | ||
| bridgeModuleName: undefined, | ||
| destinationAddress: undefined, | ||
| routerAddress: undefined, | ||
| }) | ||
| ) | ||
| } catch (error) { | ||
| console.error('Deposit error:', error) | ||
| } finally { | ||
| setIsDepositing(false) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance transaction error handling and user feedback
The deposit handler could benefit from more detailed error handling and user feedback.
const handleDeposit = async () => {
setIsDepositing(true)
const currentTimestamp: number = getUnixTimeMinutesFromNow(0)
try {
const txReceipt = await deposit(amount)
setHasDepositedOnHyperliquid(true)
setIsApproved(false)
segmentAnalyticsEvent(`[Hyperliquid Deposit]`, {
inputAmount: debouncedFromValue,
})
// ... dispatch calls ...
} catch (error) {
- console.error('Deposit error:', error)
+ console.error('Deposit error:', error)
+ segmentAnalyticsEvent(`[Hyperliquid Deposit Error]`, {
+ inputAmount: debouncedFromValue,
+ error: error.message
+ })
+ throw new Error('Failed to complete deposit. Please try again.')
} finally {
setIsDepositing(false)
}
}📝 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 handleDeposit = async () => { | |
| setIsDepositing(true) | |
| const currentTimestamp: number = getUnixTimeMinutesFromNow(0) | |
| try { | |
| const txReceipt = await deposit(amount) | |
| setHasDepositedOnHyperliquid(true) | |
| setIsApproved(false) | |
| segmentAnalyticsEvent(`[Hyperliquid Deposit]`, { | |
| inputAmount: debouncedFromValue, | |
| }) | |
| dispatch( | |
| fetchAndStoreSingleNetworkPortfolioBalances({ | |
| address, | |
| chainId: ARBITRUM.id, | |
| }) | |
| ) | |
| dispatch( | |
| addPendingBridgeTransaction({ | |
| id: currentTimestamp, | |
| originChain: ARBITRUM, | |
| originToken: fromToken, | |
| originValue: debouncedFromValue, | |
| destinationChain: HYPERLIQUID, | |
| destinationToken: undefined, | |
| transactionHash: txReceipt.transactionHash, | |
| timestamp: undefined, | |
| isSubmitted: false, | |
| estimatedTime: undefined, | |
| bridgeModuleName: undefined, | |
| destinationAddress: undefined, | |
| routerAddress: undefined, | |
| }) | |
| ) | |
| } catch (error) { | |
| console.error('Deposit error:', error) | |
| } finally { | |
| setIsDepositing(false) | |
| } | |
| } | |
| const handleDeposit = async () => { | |
| setIsDepositing(true) | |
| const currentTimestamp: number = getUnixTimeMinutesFromNow(0) | |
| try { | |
| const txReceipt = await deposit(amount) | |
| setHasDepositedOnHyperliquid(true) | |
| setIsApproved(false) | |
| segmentAnalyticsEvent(`[Hyperliquid Deposit]`, { | |
| inputAmount: debouncedFromValue, | |
| }) | |
| dispatch( | |
| fetchAndStoreSingleNetworkPortfolioBalances({ | |
| address, | |
| chainId: ARBITRUM.id, | |
| }) | |
| ) | |
| dispatch( | |
| addPendingBridgeTransaction({ | |
| id: currentTimestamp, | |
| originChain: ARBITRUM, | |
| originToken: fromToken, | |
| originValue: debouncedFromValue, | |
| destinationChain: HYPERLIQUID, | |
| destinationToken: undefined, | |
| transactionHash: txReceipt.transactionHash, | |
| timestamp: undefined, | |
| isSubmitted: false, | |
| estimatedTime: undefined, | |
| bridgeModuleName: undefined, | |
| destinationAddress: undefined, | |
| routerAddress: undefined, | |
| }) | |
| ) | |
| } catch (error) { | |
| console.error('Deposit error:', error) | |
| segmentAnalyticsEvent(`[Hyperliquid Deposit Error]`, { | |
| inputAmount: debouncedFromValue, | |
| error: error.message | |
| }) | |
| throw new Error('Failed to complete deposit. Please try again.') | |
| } finally { | |
| setIsDepositing(false) | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx
[error] 176-184: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
🔇 Additional comments (2)
packages/synapse-interface/components/StateManagedBridge/HyperliquidDepositButton.tsx (2)
176-184: Remove unnecessary Fragment wrapper
The Fragment (<> and </>) wrapping the <div> is redundant since it contains only one child.
🧰 Tools
🪛 Biome (1.9.4)
[error] 176-184: Avoid using unnecessary Fragment.
A fragment is redundant if it contains only one child, or if it is the child of a html element, and is not a keyed fragment.
Unsafe fix: Remove the Fragment
(lint/complexity/noUselessFragments)
83-121: 🛠️ Refactor suggestion
Improve transaction handling and metadata completeness
The handleDeposit function needs better error handling and complete transaction metadata.
const handleDeposit = async () => {
setIsDepositing(true)
const currentTimestamp: number = getUnixTimeMinutesFromNow(0)
try {
+ if (!address) {
+ throw new Error('Wallet not connected')
+ }
+
const txReceipt = await deposit(amount)
setHasDepositedOnHyperliquid(true)
segmentAnalyticsEvent(`[Hyperliquid Deposit]`, {
inputAmount: debouncedFromValue,
+ transactionHash: txReceipt.transactionHash,
})
dispatch(
addPendingBridgeTransaction({
id: currentTimestamp,
originChain: ARBITRUM,
originToken: fromToken,
originValue: debouncedFromValue,
destinationChain: HYPERLIQUID,
destinationToken: undefined,
transactionHash: txReceipt.transactionHash,
- timestamp: undefined,
+ timestamp: currentTimestamp,
- isSubmitted: false,
+ isSubmitted: true,
- estimatedTime: undefined,
- bridgeModuleName: undefined,
- destinationAddress: undefined,
- routerAddress: undefined,
+ estimatedTime: 300, // 5 minutes
+ bridgeModuleName: 'hyperliquid',
+ destinationAddress: HYPERLIQUID_DEPOSIT_ADDRESS,
+ routerAddress: USDC.addresses[ARBITRUM.id],
})
)
} catch (error) {
- console.error('Deposit error:', error)
+ const errorMessage = error instanceof Error ? error.message : 'Unknown error'
+ console.error('Deposit error:', errorMessage)
+ segmentAnalyticsEvent(`[Hyperliquid Deposit Error]`, {
+ inputAmount: debouncedFromValue,
+ error: errorMessage,
+ })
+ throw new Error(`Failed to complete deposit: ${errorMessage}`)
} finally {
setIsDepositing(false)
}
}Likely invalid or redundant comment.
| const depositingMinimumAmount = | ||
| Number(debouncedFromValue) >= HYPERLIQUID_MINIMUM_DEPOSIT |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add maximum transaction limit validation
While minimum deposit validation is implemented, there should also be a maximum transaction limit for security purposes.
+const HYPERLIQUID_MAXIMUM_DEPOSIT = 100000 // Example: 100k USDC
+
const depositingMinimumAmount =
Number(debouncedFromValue) >= HYPERLIQUID_MINIMUM_DEPOSIT
+
+const depositingUnderMaximum =
+ Number(debouncedFromValue) <= HYPERLIQUID_MAXIMUM_DEPOSIT
const isButtonDisabled =
isTyping ||
isDepositing ||
!depositingMinimumAmount ||
+ !depositingUnderMaximum ||
isWalletPending ||
!hasValidInput ||
(isConnected && !hasSufficientBalance)Also applies to: 133-139
| import { getUnixTimeMinutesFromNow } from '@/utils/time' | ||
| import { HYPERLIQUID_MINIMUM_DEPOSIT } from '@/constants' | ||
|
|
||
| const HYPERLIQUID_DEPOSIT_ADDRESS = '0x2Df1c51E09aECF9cacB7bc98cB1742757f163dF7' |
There was a problem hiding this comment.
Move sensitive deposit address to environment configuration
The Hyperliquid deposit address should not be hardcoded in the source code. This makes it difficult to manage across different environments and poses security risks.
Consider:
- Moving this to environment variables or a secure configuration management system
- Adding address checksum validation
- Including a comment documenting the address purpose and ownership
-const HYPERLIQUID_DEPOSIT_ADDRESS = '0x2Df1c51E09aECF9cacB7bc98cB1742757f163dF7'
+const HYPERLIQUID_DEPOSIT_ADDRESS = process.env.NEXT_PUBLIC_HYPERLIQUID_DEPOSIT_ADDRESS
+if (!HYPERLIQUID_DEPOSIT_ADDRESS || !isAddress(HYPERLIQUID_DEPOSIT_ADDRESS)) {
+ throw new Error('Invalid Hyperliquid deposit address configuration')
+}Committable suggestion skipped: line range outside the PR's diff.
| const deposit = async (amount: bigint) => { | ||
| try { | ||
| const { request } = await simulateContract(wagmiConfig, { | ||
| chainId: ARBITRUM.id, | ||
| address: USDC.addresses[ARBITRUM.id], | ||
| abi: erc20Abi, | ||
| functionName: 'transfer', | ||
| args: [HYPERLIQUID_DEPOSIT_ADDRESS, amount], | ||
| }) | ||
|
|
||
| const hash = await writeContract(wagmiConfig, request) | ||
|
|
||
| const txReceipt = await waitForTransactionReceipt(wagmiConfig, { hash }) | ||
|
|
||
| return txReceipt | ||
| } catch (error) { | ||
| console.error('Confirmation error:', error) | ||
| throw error | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance deposit function with proper validation and error handling
The deposit function needs additional safeguards for this critical financial operation.
const deposit = async (amount: bigint) => {
+ if (!amount || amount <= 0n) {
+ throw new Error('Invalid deposit amount')
+ }
+
try {
const { request } = await simulateContract(wagmiConfig, {
chainId: ARBITRUM.id,
address: USDC.addresses[ARBITRUM.id],
abi: erc20Abi,
functionName: 'transfer',
args: [HYPERLIQUID_DEPOSIT_ADDRESS, amount],
})
const hash = await writeContract(wagmiConfig, request)
+ const timeout = setTimeout(() => {
+ throw new Error('Transaction confirmation timeout')
+ }, 300000) // 5 minutes timeout
+
const txReceipt = await waitForTransactionReceipt(wagmiConfig, { hash })
+ clearTimeout(timeout)
return txReceipt
} catch (error) {
- console.error('Confirmation error:', error)
+ const errorMessage = error instanceof Error ? error.message : 'Unknown error'
+ console.error('Deposit transaction failed:', errorMessage)
+ throw new Error(`Deposit failed: ${errorMessage}`)
}
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
packages/synapse-interface/components/HyperliquidDepositInfo.tsx (2)
8-76: Refactor duplicate JSX structure into a reusable componentThe component has similar JSX structure repeated three times. Consider extracting it into a reusable component:
interface StepProps { step1: { active: boolean; completed: boolean; }; step2: { active: boolean; completed: boolean; }; } const Steps: React.FC<StepProps> = ({ step1, step2 }) => ( <div className="flex flex-col p-2 mb-2 space-y-1 text-sm"> <div className="flex justify-between"> <div> <div className={`mb-2 ${step1.active ? 'text-[#97FCE4]' : 'text-white/65'}`}> Step 1 </div> <div className="flex items-center space-x-2"> {step1.completed ? <CompletedCheckMarkCircle /> : <GreenStep1Circle />} <div className={step1.active ? 'text-[#97FCE4]' : 'text-white/65'}> Bridge (Arbitrum) </div> </div> </div> <div> <div className={`mb-2 ${step2.active ? 'text-[#97FCE4]' : 'text-white/65'}`}> Step 2 </div> <div className="flex items-center space-x-2"> {step2.completed ? ( <CompletedCheckMarkCircle /> ) : step2.active ? ( <GreenStep2Circle /> ) : ( <GrayStep2Circle /> )} <div className={step2.active ? 'text-[#97FCE4]' : 'text-white/65'}> Deposit (Hyperliquid) </div> </div> </div> </div> </div> );
13-13: Extract color values into constantsConsider extracting hardcoded colors into theme constants for better maintainability:
const COLORS = { primary: '#97FCE4', disabled: '#565058', text: { white65: 'rgba(255, 255, 255, 0.65)', }, } as const;Also applies to: 66-66, 93-93, 120-120, 146-146, 172-172
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cspell.json(1 hunks)docs/bridge/blog-posts/2024-12-12-fastbridgev2-post.md(1 hunks)packages/synapse-interface/components/HyperliquidDepositInfo.tsx(1 hunks)packages/synapse-interface/components/StateManagedBridge/BridgeQuoteResetTimer.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/synapse-interface/components/StateManagedBridge/BridgeQuoteResetTimer.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- cspell.json
🔇 Additional comments (3)
docs/bridge/blog-posts/2024-12-12-fastbridgev2-post.md (1)
90-90: Spelling correction approved, but let's verify the compatibility claim.
The spelling correction from "backwards combatible" to "backwards compatible" is correct.
Let's verify the backward compatibility claim by checking for any breaking changes in the contract interfaces:
✅ Verification successful
Based on the search results, I can now provide a definitive response about the backward compatibility claim. The evidence shows that:
- IFastBridgeV2 inherits from IFastBridge, maintaining the original interface
- FastBridgeV2 includes explicit backward compatibility for decoding requests
- The interfaces maintain structural compatibility with no breaking changes to the original functions
Backward compatibility claim verified
The statement "The Synapse Intent Network is backwards compatible with the original Fastbridge Contracts" is accurate. The codebase shows explicit backward compatibility support through interface inheritance and compatibility layers for request decoding.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for contract interface changes that might affect backward compatibility
# Look for interface definitions and contract upgrades
rg -A 10 'interface IFastBridge|contract FastBridge'
# Look for potential breaking changes in function signatures
ast-grep --pattern 'function $FUNC_NAME($$$) {
$$$
}'
Length of output: 65681
Script:
#!/bin/bash
# Let's search for any breaking changes in the interface or implementation
# Search for FastBridge contract files
fd -e sol -e json "FastBridge|fastbridge" --exec echo "=== {} ===" \;
# Search for any interface changes or version upgrades
rg -i "version|upgrade|breaking|compat" --type md
Length of output: 71656
Script:
#!/bin/bash
# Let's search for interface compatibility between FastBridge versions
# Search for interface definitions and changes between versions
rg -A 5 "interface IFastBridge" ./packages/contracts-rfq/contracts/interfaces/
# Look for any breaking changes or compatibility notes in FastBridgeV2
rg -i "break|compat|upgrade" ./packages/contracts-rfq/contracts/FastBridgeV2.sol
# Check for any migration or upgrade documentation
fd -e md "migration|upgrade" --exec cat {} \;
Length of output: 2069
packages/synapse-interface/components/HyperliquidDepositInfo.tsx (2)
94-94: Use camelCase for SVG attribute 'strokeWidth' in JSX
In JSX, SVG attributes should be camelCase. Replace stroke-width with strokeWidth to ensure proper rendering without warnings or errors.
Also applies to: 100-100
8-76:
Add a default return statement
The component should handle all possible states:
}
+ return null; // Or a loading/error state component
}Likely invalid or redundant comment.
| export const HyperliquidDepositInfo = ({ | ||
| fromChainId, | ||
| isOnArbitrum, | ||
| hasDepositedOnHyperliquid, | ||
| }) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add TypeScript types to component props
Adding TypeScript types will improve type safety and developer experience:
-export const HyperliquidDepositInfo = ({
+interface HyperliquidDepositInfoProps {
+ fromChainId: number;
+ isOnArbitrum: boolean;
+ hasDepositedOnHyperliquid: boolean;
+}
+
+export const HyperliquidDepositInfo: React.FC<HyperliquidDepositInfoProps> = ({
fromChainId,
isOnArbitrum,
hasDepositedOnHyperliquid,
}) => {📝 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 const HyperliquidDepositInfo = ({ | |
| fromChainId, | |
| isOnArbitrum, | |
| hasDepositedOnHyperliquid, | |
| }) => { | |
| interface HyperliquidDepositInfoProps { | |
| fromChainId: number; | |
| isOnArbitrum: boolean; | |
| hasDepositedOnHyperliquid: boolean; | |
| } | |
| export const HyperliquidDepositInfo: React.FC<HyperliquidDepositInfoProps> = ({ | |
| fromChainId, | |
| isOnArbitrum, | |
| hasDepositedOnHyperliquid, | |
| }) => { |
| height="34" | ||
| viewBox="0 0 34 34" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <rect | ||
| x="1" | ||
| y="1" | ||
| width="32" | ||
| height="32" | ||
| rx="16" | ||
| stroke="#565058" | ||
| strokeWidth="2" | ||
| /> | ||
| <path | ||
| d="M13.7112 23.204C13.7112 23.06 13.6952 22.996 13.6312 22.996L13.2632 23.172C13.2632 23.092 13.2152 23.044 13.1352 23.012L13.0072 22.996C12.8952 22.996 12.8472 23.012 12.6872 23.108C12.6392 23.012 12.5752 22.9 12.5272 22.804C12.1112 22.004 11.6792 20.804 11.4872 20.276C11.3912 20.004 11.2952 19.444 11.1832 18.596C11.3112 18.676 11.4072 18.708 11.4552 18.708C11.5192 18.708 11.5992 18.596 11.6632 18.372C11.6952 18.42 11.7592 18.436 11.8392 18.436C11.8872 18.436 11.9512 18.42 11.9832 18.372L12.2392 17.988L12.5272 18.084H12.5432C12.5752 18.084 12.6232 18.036 12.7032 17.988C12.7832 17.94 12.8472 17.908 12.8952 17.908L12.9432 17.924C13.1992 18.052 13.3752 18.276 13.4552 18.628C13.6472 19.444 13.8232 19.844 14.0312 19.844C14.2072 19.844 14.4472 19.636 14.7032 19.236C14.9592 18.836 15.2152 18.292 15.5032 17.636C15.5192 17.764 15.5352 17.828 15.5672 17.828C15.6632 17.828 15.9032 17.268 16.4952 16.324C17.3752 14.9 19.5512 12.164 20.1112 11.78C20.5272 11.492 20.8472 11.22 21.0712 10.98C21.0392 11.14 21.0072 11.252 21.0072 11.3C21.0072 11.348 21.0392 11.364 21.0712 11.364L21.5192 11.14V11.204C21.5192 11.284 21.5352 11.332 21.5832 11.332C21.6472 11.332 21.9032 11.076 21.9352 10.98L21.9032 11.204L22.4472 10.884L22.3192 11.172C22.4792 11.06 22.6072 10.996 22.6872 10.996C22.7672 10.996 22.8152 11.124 22.8152 11.204C22.8152 11.332 22.7032 11.508 22.5272 11.732C22.3352 11.988 21.8552 12.484 20.4152 14.132C19.7912 14.836 17.0232 18.596 16.4952 19.492L15.5032 21.172C15.0712 21.892 14.7992 22.356 14.6552 22.532C14.5112 22.708 14.3352 22.884 14.1272 23.044L13.9832 22.964L13.8552 23.044L13.7112 23.204Z" | ||
| fill="#97FCE4" | ||
| /> | ||
| <circle cx="17" cy="17" r="16" stroke="#565058" strokeWidth="2" /> | ||
| </svg> | ||
| ) | ||
| } | ||
|
|
||
| const GreenStep1Circle = () => { | ||
| return ( | ||
| <svg | ||
| width="34" | ||
| height="34" | ||
| viewBox="0 0 34 34" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <rect | ||
| x="1" | ||
| y="1" | ||
| width="32" | ||
| height="32" | ||
| rx="16" | ||
| stroke="#97FCE4" | ||
| strokeWidth="2" | ||
| /> | ||
| <path | ||
| d="M17.2346 22.5V13.124C16.4666 13.828 15.6346 14.18 14.5786 14.372V13.076C15.6986 12.836 16.6746 12.26 17.4586 11.3H18.6106V22.5H17.2346Z" | ||
| fill="#FCFCFD" | ||
| /> | ||
| </svg> | ||
| ) | ||
| } | ||
|
|
||
| const GreenStep2Circle = () => { | ||
| return ( | ||
| <svg | ||
| width="34" | ||
| height="34" | ||
| viewBox="0 0 34 34" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <rect | ||
| x="1" | ||
| y="1" | ||
| width="32" | ||
| height="32" | ||
| rx="16" | ||
| stroke="#97FCE4" | ||
| strokeWidth="2" | ||
| /> | ||
| <path | ||
| d="M13.5034 22.5L13.4874 21.236L17.6954 17.604C19.0074 16.468 19.5674 15.54 19.5674 14.532C19.5674 13.284 18.7354 12.468 17.4234 12.468C15.8554 12.468 14.9754 13.572 15.1354 15.236L13.7274 15.14C13.5514 12.788 15.0874 11.172 17.4234 11.172C19.6314 11.172 21.0714 12.5 21.0714 14.532C21.0714 15.892 20.3194 17.14 18.5914 18.596L15.5514 21.188H21.0714V22.5H13.5034Z" | ||
| fill="#97FCE4" | ||
| /> | ||
| </svg> | ||
| ) | ||
| } | ||
|
|
||
| const GrayStep2Circle = () => { | ||
| return ( | ||
| <svg | ||
| width="34" | ||
| height="34" | ||
| viewBox="0 0 34 34" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <rect | ||
| x="1" | ||
| y="1" | ||
| width="32" | ||
| height="32" | ||
| rx="16" | ||
| stroke="#565058" | ||
| strokeWidth="2" | ||
| /> | ||
| <path | ||
| d="M13.5034 22.5L13.4874 21.236L17.6954 17.604C19.0074 16.468 19.5674 15.54 19.5674 14.532C19.5674 13.284 18.7354 12.468 17.4234 12.468C15.8554 12.468 14.9754 13.572 15.1354 15.236L13.7274 15.14C13.5514 12.788 15.0874 11.172 17.4234 11.172C19.6314 11.172 21.0714 12.5 21.0714 14.532C21.0714 15.892 20.3194 17.14 18.5914 18.596L15.5514 21.188H21.0714V22.5H13.5034Z" | ||
| fill="#C0BCC2" | ||
| /> | ||
| </svg> | ||
| ) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes to SVG components
SVG components should include proper accessibility attributes:
const CompletedCheckMarkCircle = () => {
return (
<svg
+ role="img"
+ aria-label="Step completed"
width="34"
height="34"Apply similar changes to other SVG components with appropriate aria-labels.
Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
New Features
HyperliquidDepositInfocomponent for displaying deposit information.HyperliquidTransactionButtonfor managing deposits to Hyperliquid.StateManagedBridgecomponent to support bridging to Hyperliquid.Bug Fixes
Localization
8108035: synapse-interface preview link
19dcf44: synapse-interface preview link
2a370e5: synapse-interface preview link
edd1ce3: synapse-interface preview link
613222d: synapse-interface preview link
c3f3baf: synapse-interface preview link
ce8468e: synapse-interface preview link
98d327b: synapse-interface preview link
2e05662: synapse-interface preview link
673cb78: docs preview link
673cb78: synapse-interface preview link