Conversation
Deploying hyperterminal with
|
| Latest commit: |
f5796ad
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b1fe253c.hyperterminal.pages.dev |
| Branch Preview URL: | https://update-across.hyperterminal.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an "Across Bridge" feature, enabling users to deposit funds from external chains like Ethereum, Arbitrum, and Base into their Hyperliquid account via the Across protocol. The changes include a new AcrossBridgeModal component, updates to the global modal store, and the addition of a "Bridge" button in the top navigation. Feedback focuses on improving the robustness of the bridge modal, specifically addressing missing error handling for dynamic imports that could leave the UI in a permanent loading state, a bug where 0% fees are incorrectly hidden, and the need to restrict the bridge button's visibility to mainnet environments.
| import("@across-protocol/app-sdk").then(async ({ createAcrossClient }) => { | ||
| const client = createAcrossClient({ | ||
| integratorId: INTEGRATOR_ID, | ||
| chains: [mainnet, arbitrum, base], | ||
| }); | ||
| if (cancelled) return; | ||
| setAcrossClient(client); | ||
| try { | ||
| const tokens = await client.getSwapTokens({ chainId: arbitrum.id }); | ||
| if (cancelled) return; | ||
| const popular = tokens.filter((tok) => POPULAR_SYMBOLS.has(tok.symbol)); | ||
| setSourceTokens(popular); | ||
| if (popular.length > 0) setSelectedTokenAddress(popular[0].address); | ||
| setView("form"); | ||
| } catch { | ||
| if (cancelled) return; | ||
| setError(t`Unable to load bridge routes. Please try again.`); | ||
| setView("error"); | ||
| } | ||
| }); | ||
| return () => { | ||
| cancelled = true; | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
The dynamic import and initial data fetching logic lacks comprehensive error handling. If the import() fails (e.g., due to network issues), the component will remain in a permanent "loading" state because the .then() block is never reached. Additionally, the "Retry" button in the error view (line 246) only resets the view state without re-attempting this initialization logic, leaving the user stuck if the initial load failed. Consider moving the initialization into a named function that can be called both on mount and by the retry handler, and ensure the import promise has a .catch() block.
| } | ||
|
|
||
| const expectedOutput = deposit ? formatUnits(BigInt(deposit.expectedOutputAmount), USDC_PERPS.decimals) : ""; | ||
| const feePct = deposit?.fees?.total?.pct ? Number(deposit.fees.total.pct) / 1e16 : null; |
There was a problem hiding this comment.
The condition deposit?.fees?.total?.pct will evaluate to false if the percentage is exactly "0", causing feePct to be null and hiding the fee information in the UI (line 433). It is better to check for the existence of the property (e.g., using !== undefined) so that a 0% fee is correctly displayed to the user.
| const feePct = deposit?.fees?.total?.pct ? Number(deposit.fees.total.pct) / 1e16 : null; | |
| const feePct = deposit?.fees?.total?.pct !== undefined ? Number(deposit.fees.total.pct) / 1e16 : null; |
| <Button | ||
| variant="outlined" | ||
| onClick={openAcrossBridge} | ||
| className="h-6 px-2 text-xs font-medium rounded-xs bg-fill-100 border border-border-300 text-text-950 hover:border-border-500 transition-colors inline-flex items-center gap-1 shadow-xs" | ||
| > | ||
| <ArrowsLeftRightIcon className="size-4" /> | ||
| <Trans>Bridge</Trans> | ||
| </Button> |
There was a problem hiding this comment.
The "Bridge" button is currently visible on testnet, but the AcrossBridgeModal is configured with mainnet chains (Ethereum, Arbitrum, Base) and a specific destination chain ID. It should likely be hidden when isTestnet is true, similar to the Deposit button, to prevent users from attempting to bridge funds in the wrong environment.
{!isTestnet && (
<Button
variant="outlined"
onClick={openAcrossBridge}
className="h-6 px-2 text-xs font-medium rounded-xs bg-fill-100 border border-border-300 text-text-950 hover:border-border-500 transition-colors inline-flex items-center gap-1 shadow-xs"
>
<ArrowsLeftRightIcon className="size-4" />
<Trans>Bridge</Trans>
</Button>
)}
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a new bridge feature using the Across Protocol SDK, allowing users to deposit funds into their Hyperliquid account from other chains. Key changes include the addition of the @across-protocol/app-sdk dependency, the implementation of the AcrossBridgeModal component, and the integration of a 'Bridge' button in the top navigation. Feedback focuses on improving the robustness of the bridge flow by handling potential parsing errors in parseUnits and ensuring the refundAddress defaults to the connected wallet to prevent loss of funds in case of bridge failure.
| outputToken: USDC_PERPS.address, | ||
| originChainId: selectedChainId, | ||
| destinationChainId: HYPER_CORE_CHAIN_ID, | ||
| amount: parseUnits(amount, tokenDecimals), |
There was a problem hiding this comment.
The parseUnits function from viem will throw an error if the amount string contains more decimal places than the tokenDecimals allows (e.g., if a user pastes a high-precision value). Since this is directly tied to user input, it should be wrapped in a try-catch block or validated beforehand to prevent the application from crashing.
try {
const response = await fetchCounterfactual({
inputToken: selectedTokenAddress,
outputToken: USDC_PERPS.address,
originChainId: selectedChainId,
destinationChainId: HYPER_CORE_CHAIN_ID,
amount: parseUnits(amount, tokenDecimals),
recipient: finalRecipient,
refundAddress: recipientAddress || finalRecipient,
});
setDeposit(response);
setView("address");
} catch (err) {
setError(err instanceof Error ? err.message : t`Failed to generate deposit address`);
setView("error");
}
| destinationChainId: HYPER_CORE_CHAIN_ID, | ||
| amount: parseUnits(amount, tokenDecimals), | ||
| recipient: finalRecipient, | ||
| refundAddress: finalRecipient, |
There was a problem hiding this comment.
The refundAddress is currently set to finalRecipient, which might be an override address provided by the user. According to Across Protocol best practices, the refundAddress should be an address controlled by the sender on the origin chain. If the user is bridging to a third-party address (like an exchange deposit) and the bridge fails, setting the refund to that third-party address could result in lost funds. It is safer to default the refund address to the connected wallet address (recipientAddress).
| refundAddress: finalRecipient, | |
| refundAddress: recipientAddress || finalRecipient, |
1 participant
No description provided.