Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly upgrades the application's wallet management by integrating Privy for a more streamlined and robust authentication experience. It also enhances Hyperliquid trading capabilities by implementing a 'Unified Account' system, allowing for more flexible collateral usage. Concurrently, the project undergoes a rebranding to 'HyperOdd', reflecting a fresh identity across the platform. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![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 successfully integrates Privy for authentication, replacing the custom wallet connection logic. This is a significant refactoring that simplifies wallet management and introduces new login methods like email and social logins. The changes are consistent across the application, and the introduction of a usePrivyWagmiSync hook to handle state synchronization between Privy and wagmi is a great addition.
I've identified a few areas for improvement:
- A hardcoded chain ID in the Vite configuration for SSR could pose future maintenance challenges.
- A component seems to have become dead code after the refactoring.
- The new unified account logic for available trading balance appears to be missing a case for
builderPerpmarkets.
Overall, this is a solid implementation of a major new feature. Addressing these points will enhance the robustness and maintainability of the new authentication system.
| if (market?.kind === "perp") { | ||
| const subscriptionHasBalance = fromSubscription && (fromSubscription[0] > 0 || fromSubscription[1] > 0); | ||
| if (!subscriptionHasBalance) { | ||
| const unified = getUnifiedStablecoinAvailable(spotBalances); | ||
| if (unified > 0) return [unified, unified]; | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic to fall back to the unified stablecoin balance for availableToTrade only considers market?.kind === "perp". However, builderPerp markets also use spot collateral for unified accounts. This logic should probably be extended to include builderPerp markets to ensure consistent behavior for all perpetual-style markets under the unified account model.
| if (market?.kind === "perp") { | |
| const subscriptionHasBalance = fromSubscription && (fromSubscription[0] > 0 || fromSubscription[1] > 0); | |
| if (!subscriptionHasBalance) { | |
| const unified = getUnifiedStablecoinAvailable(spotBalances); | |
| if (unified > 0) return [unified, unified]; | |
| } | |
| } | |
| if (market?.kind === "perp" || market?.kind === "builderPerp") { | |
| const subscriptionHasBalance = fromSubscription && (fromSubscription[0] > 0 || fromSubscription[1] > 0); | |
| if (!subscriptionHasBalance) { | |
| const unified = getUnifiedStablecoinAvailable(spotBalances); | |
| if (unified > 0) return [unified, unified]; | |
| } | |
| } |
| import { useLogin } from "@privy-io/react-auth"; | ||
| import { useEffect } from "react"; | ||
|
|
||
| interface Props { | ||
| open: boolean; | ||
| onOpenChange: (open: boolean) => void; | ||
| } | ||
|
|
||
| export function WalletDialog({ open, onOpenChange }: Props) { | ||
| const connectors = useConnectors(); | ||
| const { mutateAsync: connectAsync, isPending, error } = useConnect(); | ||
| const [connectingId, setConnectingId] = useState<string | null>(null); | ||
| const [showHelp, setShowHelp] = useState(false); | ||
| const [lastUsedWallet] = useState(() => getLastUsedWallet()); | ||
| const [customAddress, setCustomAddress] = useState(""); | ||
| const [customAddressError, setCustomAddressError] = useState<string | null>(null); | ||
|
|
||
| const { mockConnectors, regularConnectors } = useMemo(() => { | ||
| const mocks: Connector[] = []; | ||
| const regular: Connector[] = []; | ||
|
|
||
| for (const connector of connectors) { | ||
| if (isMockConnector(connector)) { | ||
| mocks.push(connector); | ||
| } else { | ||
| regular.push(connector); | ||
| } | ||
| } | ||
|
|
||
| return { mockConnectors: mocks, regularConnectors: regular }; | ||
| }, [connectors]); | ||
|
|
||
| const availableConnectors = useMemo(() => { | ||
| const sortByPriority = (a: Connector, b: Connector) => { | ||
| if (lastUsedWallet) { | ||
| if (a.id === lastUsedWallet) return -1; | ||
| if (b.id === lastUsedWallet) return 1; | ||
| } | ||
| const priorityA = getWalletInfo(a).priority ?? 50; | ||
| const priorityB = getWalletInfo(b).priority ?? 50; | ||
| return priorityA - priorityB; | ||
| }; | ||
|
|
||
| const popular = regularConnectors.filter((c) => getWalletInfo(c).popular).sort(sortByPriority); | ||
| const other = regularConnectors.filter((c) => !getWalletInfo(c).popular).sort(sortByPriority); | ||
| return { popular, other, all: regularConnectors }; | ||
| }, [regularConnectors, lastUsedWallet]); | ||
|
|
||
| const handleConnect = async (connector: Connector) => { | ||
| setConnectingId(connector.uid); | ||
| setLastUsedWallet(connector.id); | ||
| try { | ||
| await connectAsync({ connector }); | ||
| onOpenChange(false); | ||
| } finally { | ||
| setConnectingId(null); | ||
| } | ||
| }; | ||
|
|
||
| const handleCustomAddressConnect = async () => { | ||
| const trimmed = customAddress.trim(); | ||
| if (!trimmed) { | ||
| setCustomAddressError("Please enter an address"); | ||
| return; | ||
| } | ||
| if (!isAddress(trimmed)) { | ||
| setCustomAddressError("Invalid Ethereum address"); | ||
| return; | ||
| } | ||
| setCustomAddressError(null); | ||
|
|
||
| const mockWalletIndex = MOCK_WALLETS.findIndex((w) => w.address.toLowerCase() === trimmed.toLowerCase()); | ||
|
|
||
| if (mockWalletIndex !== -1 && mockConnectors[mockWalletIndex]) { | ||
| handleConnect(mockConnectors[mockWalletIndex]); | ||
| } else { | ||
| const customMockConnector = mock({ | ||
| accounts: [trimmed as Address], | ||
| features: { reconnect: true }, | ||
| }); | ||
| setConnectingId("custom-mock"); | ||
| try { | ||
| await connectAsync({ connector: customMockConnector }); | ||
| onOpenChange(false); | ||
| } catch { | ||
| setCustomAddressError("Failed to connect with custom address"); | ||
| } finally { | ||
| setConnectingId(null); | ||
| } | ||
| const { login } = useLogin({ | ||
| onComplete: () => onOpenChange(false), | ||
| onError: () => onOpenChange(false), | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| if (open) { | ||
| login(); | ||
| } | ||
| }; | ||
|
|
||
| const hasConnectors = availableConnectors.all.length > 0 || mockConnectors.length > 0; | ||
|
|
||
| return ( | ||
| <Dialog open={open} onOpenChange={onOpenChange}> | ||
| <DialogContent className="sm:max-w-md gap-0 p-0 overflow-hidden"> | ||
| <div className="p-6 pb-4 border-b border-border-200/50"> | ||
| <DialogHeader className="space-y-2"> | ||
| <DialogTitle className="flex items-center gap-2 text-lg"> | ||
| <WalletIcon className="size-5 text-primary-default" /> | ||
| <Trans>Connect Wallet</Trans> | ||
| </DialogTitle> | ||
| <DialogDescription> | ||
| <Trans>Connect your wallet to start trading on Hyperliquid</Trans> | ||
| </DialogDescription> | ||
| </DialogHeader> | ||
| </div> | ||
|
|
||
| <div className="p-4 space-y-4 max-h-[60vh] overflow-y-auto"> | ||
| {availableConnectors.popular.length > 0 && ( | ||
| <div className="space-y-2"> | ||
| <p className="text-xs font-medium text-text-600 uppercase tracking-wider px-1"> | ||
| <Trans>Popular</Trans> | ||
| </p> | ||
| <div className="space-y-2"> | ||
| {availableConnectors.popular.map((connector) => { | ||
| const walletInfo = getWalletInfo(connector); | ||
| const Icon = walletInfo.icon; | ||
| const isConnecting = connectingId === connector.uid; | ||
|
|
||
| return ( | ||
| <Button | ||
| key={connector.uid} | ||
| variant="text" | ||
| size="none" | ||
| onClick={() => handleConnect(connector)} | ||
| disabled={isPending} | ||
| className={cn( | ||
| "w-full gap-3 p-3 rounded-lg border", | ||
| "bg-surface-base hover:bg-surface-analysis/50 hover:border-primary-default/30", | ||
| "group focus:ring-2 focus:ring-primary-default/50", | ||
| )} | ||
| > | ||
| <div className="size-10 rounded-lg overflow-hidden flex-shrink-0 shadow-sm"> | ||
| <Icon className="size-full" /> | ||
| </div> | ||
| <div className="flex-1 text-left min-w-0"> | ||
| <p className="font-medium text-sm group-hover:text-primary-default transition-colors"> | ||
| {connector.name} | ||
| </p> | ||
| <p className="text-xs text-text-600 truncate">{walletInfo.description}</p> | ||
| </div> | ||
| {isConnecting ? ( | ||
| <SpinnerGapIcon className="size-4 animate-spin text-primary-default flex-shrink-0" /> | ||
| ) : ( | ||
| <div className="size-4 rounded-full border border-border-200 group-hover:border-primary-default/50 flex-shrink-0 transition-colors" /> | ||
| )} | ||
| </Button> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {availableConnectors.other.length > 0 && ( | ||
| <div className="space-y-2"> | ||
| <p className="text-xs font-medium text-text-600 uppercase tracking-wider px-1"> | ||
| <Trans>Other Options</Trans> | ||
| </p> | ||
| <div className="space-y-2"> | ||
| {availableConnectors.other.map((connector) => { | ||
| const walletInfo = getWalletInfo(connector); | ||
| const Icon = walletInfo.icon; | ||
| const isConnecting = connectingId === connector.uid; | ||
|
|
||
| return ( | ||
| <Button | ||
| key={connector.uid} | ||
| variant="text" | ||
| size="none" | ||
| onClick={() => handleConnect(connector)} | ||
| disabled={isPending} | ||
| className={cn( | ||
| "w-full gap-3 p-3 rounded-lg border", | ||
| "bg-surface-base hover:bg-surface-analysis/50 hover:border-primary-default/30", | ||
| "group focus:ring-2 focus:ring-primary-default/50", | ||
| )} | ||
| > | ||
| <div className="size-10 rounded-lg overflow-hidden flex-shrink-0 shadow-sm"> | ||
| <Icon className="size-full" /> | ||
| </div> | ||
| <div className="flex-1 text-left min-w-0"> | ||
| <p className="font-medium text-sm group-hover:text-primary-default transition-colors"> | ||
| {connector.name} | ||
| </p> | ||
| <p className="text-xs text-text-600 truncate">{walletInfo.description}</p> | ||
| </div> | ||
| {isConnecting ? ( | ||
| <SpinnerGapIcon className="size-4 animate-spin text-primary-default flex-shrink-0" /> | ||
| ) : ( | ||
| <div className="size-4 rounded-full border border-border-200 group-hover:border-primary-default/50 flex-shrink-0 transition-colors" /> | ||
| )} | ||
| </Button> | ||
| ); | ||
| })} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {mockConnectors.length > 0 && ( | ||
| <div className="space-y-2"> | ||
| <p className="text-xs font-medium text-warning-700 uppercase tracking-wider px-1"> | ||
| <Trans>Mock Wallet (Testing)</Trans> | ||
| </p> | ||
| <div className="space-y-2"> | ||
| {mockConnectors.map((connector, index) => { | ||
| const config = MOCK_WALLETS[index]; | ||
| const isConnecting = connectingId === connector.uid; | ||
|
|
||
| return ( | ||
| <Button | ||
| key={connector.uid} | ||
| variant="text" | ||
| size="none" | ||
| onClick={() => handleConnect(connector)} | ||
| disabled={isPending} | ||
| className={cn( | ||
| "w-full gap-3 p-3 rounded-lg border border-warning-700/30", | ||
| "bg-warning-700/5 hover:bg-warning-700/10 hover:border-warning-700/50", | ||
| "group focus:ring-2 focus:ring-warning-700/50", | ||
| )} | ||
| > | ||
| <div className="size-10 rounded-lg overflow-hidden flex-shrink-0 shadow-sm bg-warning-700/20 flex items-center justify-center"> | ||
| <FlaskIcon className="size-5 text-warning-700" /> | ||
| </div> | ||
| <div className="flex-1 text-left min-w-0"> | ||
| <p className="font-medium text-sm group-hover:text-warning-700 transition-colors"> | ||
| {config?.name ?? connector.name} | ||
| </p> | ||
| <p className="text-xs text-text-600 truncate font-mono">{config?.address ?? "Mock wallet"}</p> | ||
| </div> | ||
| {isConnecting ? ( | ||
| <SpinnerGapIcon className="size-4 animate-spin text-warning-700 flex-shrink-0" /> | ||
| ) : ( | ||
| <div className="size-4 rounded-full border border-warning-700/50 flex-shrink-0 transition-colors" /> | ||
| )} | ||
| </Button> | ||
| ); | ||
| })} | ||
| </div> | ||
| <div className="pt-2 space-y-2"> | ||
| <div className="flex gap-2"> | ||
| <Input | ||
| placeholder="0x..." | ||
| value={customAddress} | ||
| onChange={(e) => { | ||
| setCustomAddress(e.target.value); | ||
| setCustomAddressError(null); | ||
| }} | ||
| className="font-mono text-xs" | ||
| /> | ||
| <Button | ||
| variant="outlined" | ||
| size="sm" | ||
| onClick={handleCustomAddressConnect} | ||
| disabled={isPending} | ||
| className="shrink-0" | ||
| > | ||
| <Trans>Connect</Trans> | ||
| </Button> | ||
| </div> | ||
| {customAddressError && <p className="text-xs text-error-700 px-1">{customAddressError}</p>} | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {!hasConnectors && ( | ||
| <div className="py-8 text-center space-y-3"> | ||
| <div className="size-12 rounded-full bg-surface-analysis flex items-center justify-center mx-auto"> | ||
| <WarningCircleIcon className="size-6 text-text-600" /> | ||
| </div> | ||
| <div> | ||
| <p className="text-sm font-medium"> | ||
| <Trans>No wallets found</Trans> | ||
| </p> | ||
| <p className="text-xs text-text-600 mt-1"> | ||
| <Trans>Install a wallet extension to continue</Trans> | ||
| </p> | ||
| </div> | ||
| </div> | ||
| )} | ||
|
|
||
| {error && ( | ||
| <div className="flex items-start gap-2 p-3 rounded-xs bg-error-700/10 border border-error-700/20"> | ||
| <WarningCircleIcon className="size-4 text-error-700 shrink-0 mt-0.5" /> | ||
| <p className="text-xs text-error-700">{error.message}</p> | ||
| </div> | ||
| )} | ||
| </div> | ||
|
|
||
| <div className="border-t border-border-200/50 bg-surface-analysis"> | ||
| <Button | ||
| variant="text" | ||
| size="none" | ||
| onClick={() => setShowHelp(!showHelp)} | ||
| className="w-full justify-between p-4 text-sm text-text-600 hover:text-text-950 hover:bg-transparent" | ||
| > | ||
| <span className="flex items-center gap-2"> | ||
| <QuestionIcon className="size-4" /> | ||
| <Trans>New to wallets?</Trans> | ||
| </span> | ||
| <span className={cn("transition-transform", showHelp && "rotate-180")}> | ||
| <svg width="12" height="12" viewBox="0 0 12 12" fill="none" aria-hidden="true"> | ||
| <path | ||
| d="M2.5 4.5L6 8L9.5 4.5" | ||
| stroke="currentColor" | ||
| strokeWidth="1.5" | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| /> | ||
| </svg> | ||
| </span> | ||
| </Button> | ||
| }, [open, login]); | ||
|
|
||
| {showHelp && ( | ||
| <div className="px-4 pb-4 space-y-3 animate-in slide-in-from-top-2 duration-200"> | ||
| <div className="flex items-start gap-3 text-xs"> | ||
| <ShieldIcon className="size-4 text-market-up-600 flex-shrink-0 mt-0.5" /> | ||
| <div> | ||
| <p className="font-medium text-text-950"> | ||
| <Trans>Secure & Private</Trans> | ||
| </p> | ||
| <p className="text-text-600 mt-0.5"> | ||
| <Trans>Only you control your funds. No email or password required.</Trans> | ||
| </p> | ||
| </div> | ||
| </div> | ||
| <div className="flex items-start gap-3 text-xs"> | ||
| <WalletIcon className="size-4 text-primary-default flex-shrink-0 mt-0.5" /> | ||
| <div> | ||
| <p className="font-medium text-text-950"> | ||
| <Trans>What is a wallet?</Trans> | ||
| </p> | ||
| <p className="text-text-600 mt-0.5"> | ||
| <Trans>A crypto wallet lets you store and manage your digital assets securely.</Trans> | ||
| </p> | ||
| </div> | ||
| </div> | ||
| <Button variant="outlined" size="sm" className="w-full mt-2" asChild> | ||
| <a href="https://ethereum.org/en/wallets/" target="_blank" rel="noopener noreferrer"> | ||
| <Trans>Learn more</Trans> | ||
| <ArrowSquareOutIcon className="size-3 ml-1.5" /> | ||
| </a> | ||
| </Button> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </DialogContent> | ||
| </Dialog> | ||
| ); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
This component has been refactored to be a headless component that triggers the Privy login modal. However, all usages of this WalletDialog component seem to have been removed from the codebase in this pull request and replaced with direct calls to the login function from useLogin. If this component is no longer used anywhere, it should be removed to avoid dead code. If it is still used, its name is now misleading, as it no longer renders a dialog. Consider renaming it to something like PrivyLoginTrigger to better reflect its new purpose.
| export const useSwitchChain = () => ({ switchChain: () => {}, chains: [], status: 'idle', error: null }); | ||
| export const useWalletClient = () => ({ data: null, isLoading: false, error: null }); | ||
| export const usePublicClient = () => undefined; | ||
| export const useChainId = () => 42161; |
There was a problem hiding this comment.
The chain ID 42161 is hardcoded in the SSR stub for wagmi's useChainId hook. While this is correct for Arbitrum, which is the only supported chain currently, this could become a maintenance issue if the application adds support for other chains in the future. The server-side rendering will always assume Arbitrum, which might lead to incorrect behavior (e.g., for generating block explorer links) if chain-dependent logic is ever rendered on the server. Consider defining this as a constant at the top of the file to make it more explicit and easier to update when new chains are added.
Deploying hyperterminal with
|
| Latest commit: |
95e2b95
|
| Status: | ⚡️ Build in progress... |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
2 participants
No description provided.