Conversation
chore: change token identification to use an identifier instead of tokenIndex
# Conflicts: # package.json # pnpm-lock.yaml
|
The latest updates on your projects. Learn more about Vercel for GitHub.
5 Skipped Deployments
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request substantially refactors the token management system from index-based to key-based lookup (chain-symbol-address), introduces a new swap-bridge feature with Interchain Account support, restructures navigation and form components, adds AWS S3 font distribution, and applies sweeping UI/styling updates across the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as TransferForm
participant Swap as SwapBridge Engine
participant Origin as Origin Chain
participant Router as Universal Router
participant ICA as ICA Commitment
participant Dest as Destination Chain
participant Relay as Relayer Service
User->>UI: Select origin token, destination, amount
UI->>Swap: executeSwapBridge(params)
Swap->>Origin: Switch chain context
Swap->>Router: Quote swap (origin→bridge token)
Router-->>Swap: Swap output amount
Swap->>Dest: Quote bridge fee + ICA fee
Dest-->>Swap: Fee amounts
Swap->>Swap: Build ICA commitment (approve+transferRemote)
Swap->>Origin: Check token approval needed
alt Approval Required
Swap->>Router: Approve bridge token spend
Router-->>Swap: Approval confirmed
end
Swap->>Router: Simulate swap+bridge tx with fee buffer
Swap->>Router: Send transaction (swap→bridge→ICA)
Router-->>Swap: TX receipt
Swap->>Relay: Post ICA commitment payload
Relay-->>Swap: Commitment stored
Swap-->>UI: Success (txHash)
UI-->>User: Show confirmation
Note over Dest: ICA executor processes commit/reveal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/features/transfer/useTokenTransfer.ts (1)
359-376:⚠️ Potential issue | 🟠 Major
originis not accessible in thecatchblock — pre-existing scoping issue now affects swap-bridge path tooThis is like finding an onion with extra layers, aye? The
const origindeclarations (line 177 in the swap-bridgeif-block, line 239 in the try body) are block-scoped and not visible inside thecatch. If an error is thrown after theoriginassignment, the reference at line 371 (getChainDisplayName(multiProvider, origin)) would throw aReferenceError, masking the real error.This was pre-existing, but the new swap-bridge path makes it more likely to be hit. Consider hoisting
originwithletbefore thetryblock.🐛 Proposed fix
+ let origin: string | undefined; try { const originToken = routeOverrideToken || getTokenByKey(warpCore.tokens, originTokenKey); const destinationToken = getTokenByKey(warpCore.tokens, destinationTokenKey); if (!originToken || !destinationToken) throw new Error('No token route found between chains'); ... if (routeType === 'swap-bridge') { - const origin = originToken.chainName; + origin = originToken.chainName; ... } - const origin = originToken.chainName; + origin = originToken.chainName; ... } catch (error: any) { ... - toast.error( - `Transaction timed out, ${getChainDisplayName(multiProvider, origin)} may be busy...` - ); + toast.error( + `Transaction timed out, ${origin ? getChainDisplayName(multiProvider, origin) : 'the chain'} may be busy...` + ); ... }src/features/chains/ChainConnectionWarning.tsx (1)
1-7:⚠️ Potential issue | 🔴 Critical
ChainNametype is used but not imported — this'll cause a compilation error, donkey.Lines 13-14 reference
ChainNamein the component props, but it's nowhere in the import block. Likely needs to come from@hyperlane-xyz/sdk.🐛 Proposed fix
-import { ChainMetadata, isRpcHealthy } from '@hyperlane-xyz/sdk'; +import { ChainMetadata, ChainName, isRpcHealthy } from '@hyperlane-xyz/sdk';#!/bin/bash # Verify how ChainName is typically imported in this codebase rg -n "import.*ChainName" --type=ts --type=tsx -g '!node_modules' | head -20Also applies to: 13-14
src/features/transfer/TransferFeeModal.tsx (1)
22-73:⚠️ Potential issue | 🟡 MinorSkeletons won't show during initial load when
feesis null.Now look — the skeleton placeholders are nested inside conditionals like
fees?.localQuote && fees.localQuote.amount > 0n. Whenfeesisnull(initial loading state), none of these blocks render at all, so the user sees an empty modal with just the "Read more" link. If the intent is to show loading indicators while fees are being fetched, the skeletons need to be rendered independently of thefeeschecks.Something like:
- {fees?.localQuote && fees.localQuote.amount > 0n && ( + {(isLoading || (fees?.localQuote && fees.localQuote.amount > 0n)) && (Or restructure so that when
isLoading && !fees, a generic loading skeleton is displayed.src/features/transfer/fees.ts (1)
140-151:⚠️ Potential issue | 🟠 MajorPass
originRouteTokeninstead oforiginTokentotryGetDefaultOriginToken.The code explicitly calls
findRouteTokento resolve the correct token after deduplication (lines 125-127), then buildstokensWithSameCollateralAddressesusing that resolvedoriginRouteToken(lines 128-130). However, the lookup at line 142 passes the originaloriginTokeninstead. Since the function uses the token'scollateralAddressOrDenomand chain name for config lookups, passing a deduplicated token that differs from the one used to build the candidates array will cause the default route matching to fail.
🤖 Fix all issues with AI agents
In `@src/features/chains/ChainFilterPanel.tsx`:
- Around line 1-9: The prop type ChainName used in the ChainFilterPanelProps
(for selectedChain and onSelectChain) is not imported; add the missing named
import for ChainName (e.g. import { ChainName } from '<module where ChainName is
defined>') at the top of ChainFilterPanel.tsx so the ChainFilterPanelProps type
compiles; ensure the import is a named import and matches the module that
exports ChainName.
In `@src/features/swap/hooks/useIcaAddress.ts`:
- Around line 16-17: The queryKey for useQuery currently includes the class
instance icaApp which is unstable; remove icaApp from the queryKey and use a
stable primitive instead (e.g., a unique id/hash from the object or simply omit
it and rely on the existing enabled guard). Update the useQuery call that builds
queryKey=['icaAddress', icaApp, userAddress, originChainName,
destinationChainName] to exclude icaApp or replace it with icaApp?.id or a
deterministic configHash, and ensure the enabled option uses Boolean(icaApp) so
the query only runs when the instance is available; keep function names like
useIcaAddress and useQuery unchanged.
In `@src/features/swap/hooks/useIcaTransaction.ts`:
- Around line 186-190: The code in useIcaTransaction sets status to 'complete'
unconditionally even when publicClient is undefined, so change the post-submit
flow to reflect unverifiable transactions: after attempting to wait for receipt
using publicClient.waitForTransactionReceipt({ hash, confirmations: 1 }), only
setStatus('complete') if publicClient existed and the receipt was successfully
awaited; if publicClient is undefined (or the await throws/no receipt),
setStatus('confirming') or 'submitted' instead (or add a new 'submitted' state)
and ensure any error path keeps status as 'confirming' so users know the tx is
not confirmed; update references to setStatus and any UI consumers expecting
'complete' accordingly.
- Around line 83-103: The code currently defaults msgFee to 0n after calling
destinationPublicClient.readContract(quoteTransferRemote) which can silently
allow sending zero native value; after the for-loop in useIcaTransaction where
msgFee and tokenPull are computed, add a guard that detects msgFee === 0n (and
optionally quotes.length > 0) and either throw a descriptive error or emit a
diagnostic log before proceeding (include function/variable names like
quoteTransferRemote, msgFee, tokenPull, destConfig.icaBridgeRoute,
destinationPublicClient.readContract) so the caller sees that no native fee was
quoted and the transaction should not send value 0. Ensure the thrown error
message includes the original quote response context to aid debugging.
In `@src/features/swap/hooks/useInterchainAccount.ts`:
- Around line 18-25: The catch is silently swallowing errors in
useInterchainAccount when InterchainAccount.fromAddressesMap fails; update the
catch to log the error (including context like addressesMap and
multiProtocolProvider.metadata) via console.error or process logger and then
rethrow (or return null only for expected/handled cases), referencing the
InterchainAccount.fromAddressesMap call and the MultiProvider construction (new
MultiProvider(multiProtocolProvider.metadata)) so failures are visible for
debugging.
In `@src/features/swap/swapConfig.ts`:
- Around line 83-110: The SWAP_CHAIN_CONFIGS object contains mixed-case
(checksummed) EVM addresses; update the hardcoded fields (universalRouter,
quoterV2, icaBridgeRoute, wrappedNative, etc.) to be lowercase and ensure any
registry-derived values returned by functions like
requireIcaRouter(DEMO_ORIGIN_CHAIN) / requireIcaRouter(DEMO_DESTINATION_CHAIN)
are normalized (call .toLowerCase() on their result or within the function)
before assignment so all EVM addresses in SWAP_CHAIN_CONFIGS are lowercase while
preserving case-sensitive non-EVM values.
In `@src/features/tokens/hooks.ts`:
- Around line 29-35: findTokenByChainSymbol currently uses
chainSymbol.split('-') which breaks when chain names contain hyphens; change the
parsing to find the last dash and split there (use
chainSymbol.lastIndexOf('-')), return undefined if no dash, set chainName =
chainSymbol.slice(0, lastDash) and symbol = chainSymbol.slice(lastDash + 1),
then perform the same comparison (t.chainName === chainName &&
t.symbol.toLowerCase() === symbol.toLowerCase()) so tokens with hyphenated chain
names (e.g., "arbitrum-nova-USDC") are handled correctly.
In `@src/features/tokens/ImportTokenButton.tsx`:
- Around line 20-24: In the catch block in ImportTokenButton.tsx (the block
referencing USER_REJECTED_ERROR), stop logging all errors at debug level: when
the error is NOT a user-rejection (i.e., if
!errorDetails.includes(USER_REJECTED_ERROR)) call logger.error(error) (so it
surfaces to Sentry) and show toast.error(errorDetails) as you already do; only
for user-rejected cases keep the quieter path (logger.debug or no toast). Leave
the USER_REJECTED_ERROR check, the toast.error call for non-rejections, and
ensure you do not suppress or swallow unexpected errors by keeping logger.error
for those cases.
In `@src/features/transfer/TransferTokenForm.tsx`:
- Around line 1018-1043: getRouteType duplicates the route-classification logic
in useTransferRoute; extract the pure decision logic into a single shared helper
(e.g., determineTransferRoute or computeRouteType) that accepts the same inputs
(tokens, collateralGroups, values or the minimal inputs: originToken,
destinationToken, collateralGroups) and returns TransferRouteType, then update
both useTransferRoute and getRouteType to call that helper; ensure the helper
uses the same calls already present (getTokenByKey, checkTokenHasRoute,
isSwapSupported, getSwappableAddress, isDemoSwapBridgePath) and keep public
signatures compatible with TransferFormValues/TransferRouteType so callers need
only replace direct logic with a single call to the new function.
- Around line 114-123: In validate (used with TransferFormValues) after you
detect routeType === 'swap-bridge', in addition to checking values.amount ensure
there is a valid recipient by verifying either values.recipient is
present/non-empty or the derived ICA/destination wallet is available (the logic
that derives ICA address or checks connected destination wallet should be
consulted here); if neither exists return a validation error (e.g. { recipient:
'Recipient required' }) so the form cannot submit with an empty/undefined
recipient. Use getRouteType, values.amount and values.recipient/ICA-derivation
check to locate where to add this check.
In `@src/features/transfer/types.ts`:
- Around line 17-19: The new enum values PostingCommitment, SigningSwapBridge,
and ConfirmingSwapBridge were added but not handled downstream; update
getTransferStatusLabel() to return human-readable labels for those three values,
and add the new statuses to the appropriate status sets (either
SentTransferStatuses and/or FinalTransferStatuses) so finality logic recognizes
swap-bridge flows—specifically ensure ConfirmingSwapBridge is treated as final
(or Sent if you have a separate in-flight concept) so
TransfersDetailsModal.isFinal, STATUSES_WITH_ICON (used by SideBarMenu), the
wallet reconnection check in utils.ts, and failUnconfirmedTransfers() in
store.ts behave correctly; if you decide PostingCommitment should remain
in-flight, include it in SentTransferStatuses but not FinalTransferStatuses, and
ensure SigningSwapBridge is classified consistently with its lifecycle.
In `@src/features/transfer/useSwapBridgeTransfer.ts`:
- Around line 393-414: The call to shareCallsWithPrivateRelayer in the
onStatusChange(TransferStatus.PostingCommitment) block can hang indefinitely;
modify this call to enforce a timeout and surface a clear recovery error: create
an AbortController (or use a timeout wrapper) and pass its signal into
shareCallsWithPrivateRelayer if the function supports it, or wrap the promise in
a Promise.race with a timeout that aborts/throws after a configurable duration
(e.g., 10–30s); on timeout, throw a descriptive error that includes context
(commitment hash, originDomain via buildPostCallsPayload and
COMMITMENTS_SERVICE_URL) and use toErrorMessage for consistency so callers can
present a specific recovery message instead of hanging.
- Around line 107-135: The parameter publicClient in checkAndApprove is typed as
any, losing type safety for publicClient.readContract and
publicClient.waitForTransactionReceipt; change the publicClient parameter type
to viem's PublicClient (import { PublicClient } from 'viem' or your project's
viem wrapper) and update the function signature async function
checkAndApprove(walletClient: WalletClient, publicClient: PublicClient,
tokenAddress: Address, spender: Address, amount: bigint): Promise<void>, leaving
calls to publicClient.readContract and publicClient.waitForTransactionReceipt
as-is so TypeScript can validate their return types and args (ensure
createPublicClient is used where the client is created so the type lines up).
🟡 Minor comments (19)
src/consts/links.ts-3-3 (1)
3-3:⚠️ Potential issue | 🟡 MinorThis explorer URL looks like a temporary branch preview deployment.
hyperlane-explorer-git-pb-offsite-swap-abacus-works.vercel.appis a Vercel git-branch preview URL. Once that branch gets merged or deleted, this link will go stale faster than an onion left in the sun. If this is intentional for a demo, consider leaving a comment noting it's temporary; otherwise, swap it for the stable production explorer URL..env.example-8-8 (1)
8-8:⚠️ Potential issue | 🟡 MinorMissing trailing newline at end of file.
Some tools and POSIX standards expect a final newline. Static analysis flagged this too. Small thing, but easy to fix.
Proposed fix
AWS_REGION=us-east-1 +src/consts/warpRouteWhitelist.ts-11-16 (1)
11-16:⚠️ Potential issue | 🟡 MinorSubstring matching with
'form'will unintentionally exclude routes containing that substring.The filter at line 125 of warpCoreConfig.ts uses
.includes()for case-insensitive matching, so the blacklist entry'form'will match any route ID containing "form" as a substring — think 'reform', 'transform', 'platform', 'perform', etc. If you're just trying to block Form chain routes, you'd want to be more specific, like'form/'or match against the chain name separately rather than the route ID.src/components/input/TextField.tsx-32-33 (1)
32-33:⚠️ Potential issue | 🟡 MinorRemoving padding and font-size from the shared default breaks two out of three consumers.
Stripping
mt-1.5 px-2.5 py-2.5 text-smfromdefaultClassNamemeans everyTextFieldandTextInputloses its padding and font sizing by default. Checking the actual usages:
SelectOrInputTokenIds.tsx: passes onlyclassName="w-full"— now renders with zero paddingTransferTokenForm.tsx: passestext-xlbut no padding classes — loses all default spacingSearchInput.tsx: passesall:py-2 all:text-sm all:border-gray-300— this one's coveredSo two of the three consumers will render with missing padding and visually inconsistent sizing unless they're updated too. That's the kind of change that ripples through everywhere else silently.
package.json-125-126 (1)
125-126:⚠️ Potential issue | 🟡 MinorPrebuild font-fetching could break CI if credentials are missing.
The
prebuildscript runsnode scripts/fetch-fonts.mjsbefore every build. If this script requires AWS S3 credentials (implied by the@aws-sdk/client-s3devDependency) and they're not configured in the build environment, the build will fail. Consider making this step graceful — the CSS already notes the fonts are "optional" and fall back to system fonts.src/features/transfer/maxAmount.ts-42-49 (1)
42-49:⚠️ Potential issue | 🟡 MinorRecipient fallback to origin sender address may be invalid for cross-protocol transfers.
When
formRecipientandconnectedDestAddressare both empty, the fallback isaddress— the sender's origin chain address. For same-protocol transfers (EVM→EVM) this is fine, but for cross-protocol transfers (e.g., EVM→Cosmos, EVM→Solana), the origin address is not a valid recipient on the destination chain. This could cause the fee estimation to fail or return incorrect results.If this is intentionally a best-effort estimate for UI purposes, consider adding a brief comment explaining the rationale. Otherwise, returning
undefined(skip max amount calculation) when no valid destination recipient exists might be safer, like staying in your own swamp when you don't know the way.scripts/fetch-fonts.mjs-52-55 (1)
52-55:⚠️ Potential issue | 🟡 Minor
response.Bodycould beundefined— guard before streamingThe S3
GetObjectCommandresponse can have an undefinedBody(e.g., if the key doesn't exist and no error is thrown, or in certain SDK edge cases). Calling.transformToWebStream()onundefinedwill throw an unhandledTypeErrorthat the outer catch will swallow with a vague message.🛡️ Proposed fix
const response = await s3.send(command); + if (!response.Body) { + throw new Error(`S3 returned empty body for ${fontFile}`); + } const writeStream = createWriteStream(outputPath); await pipeline(Readable.fromWeb(response.Body.transformToWebStream()), writeStream);tailwind.config.js-124-130 (1)
124-130:⚠️ Potential issue | 🟡 Minor
accent-glowanderror-glowshould both useaccent.100per design system v2Right now
accent-glowuses a hardcodedrgba(154, 13, 255, 0.35)(a primary purple) whileerror-glowcorrectly referencestheme('colors.accent.100'). These should use the same color — the idea being you distinguish error states via theerror-gradientbackground, not the glow color. Also, using a hardcoded rgba instead of a theme token makes the accent-glow drift if the palette ever changes.🎨 Proposed fix
boxShadow: ({ theme }) => ({ - 'accent-glow': `inset 0 0 20px 0 rgba(154, 13, 255, 0.35)`, + 'accent-glow': `inset 0 0 20px 0 ${theme('colors.accent.100')}59`, 'error-glow': `inset 2px 2px 13px 2px ${theme('colors.accent.100')}`,Based on learnings: "In v2 of the design system, define both accent-glow and error-glow box shadows to use the same color (accent.100). Distinguish error states using the error-gradient background instead of changing the glow color."
src/features/transfer/useTokenTransfer.ts-173-233 (1)
173-233:⚠️ Potential issue | 🟡 Minor
senderfrom wagmi may be checksummed — should be lowercased for EVMAt line 179,
walletClient.account?.addressreturns a checksummed (mixed-case) EVM address. Per your coding guidelines, EVM addresses should be lowercase. Thissenderis stored in the transfer record at line 189 and potentially used downstream. Same goes fororiginSwapAddressif it derives from checksummed SDK values.🛡️ Proposed fix
const sender = walletClient.account?.address; if (!sender) throw new Error('No active account found'); + const normalizedSender = sender.toLowerCase(); addTransfer({ timestamp: new Date().getTime(), status: TransferStatus.Preparing, origin, destination, originTokenAddressOrDenom: originToken.addressOrDenom, destTokenAddressOrDenom: destinationToken.addressOrDenom, - sender, + sender: normalizedSender, recipient, amount, });As per coding guidelines: "Only lowercase EVM addresses; Solana and Cosmos addresses are case-sensitive."
src/features/transfer/RecipientConfirmationModal.tsx-29-29 (1)
29-29:⚠️ Potential issue | 🟡 MinorEmpty recipient could show a blank confirmation bubble.
If both
values.recipientandconnectedDestAddressare falsy,recipientbecomes''and line 41 renders an empty styled<p>. The modal asks "Is this address correct?" while showing... nothing. Might want a guard or at least a fallback display, so the user isn't staring into the void.Also applies to: 41-41
src/features/tokens/ImportTokenButton.tsx-10-11 (1)
10-11:⚠️ Potential issue | 🟡 MinorType alignment check:
Tokenprop should matchuseAddTokenparameter type.The
useAddTokenhook declares its parameter asIToken, but the component'stokenprop is typed asToken. While both come from the@hyperlane-xyz/sdklibrary and the code runs fine (suggesting structural compatibility), aligning the prop type with what the hook expects—eitherITokenor verifying thatTokenextendsIToken—would make the types consistent throughout the feature.src/components/nav/Header.tsx-48-68 (1)
48-68:⚠️ Potential issue | 🟡 MinorAbsolute-positioned nav and wallet button may overlap the centered logo on mid-size screens.
The nav (
sm:absolute sm:left-6) and wallet button (sm:absolute sm:right-6) use absolute positioning, while the logo is flow-centered. On screens around thesmbreakpoint (~640px), these could collide with the logo. Consider testing at intermediate viewport widths or adding amin-width/lg-only gate for the nav.src/features/wallet/WalletDropdown.tsx-53-58 (1)
53-58:⚠️ Potential issue | 🟡 MinorEVM addresses should be lowercased before saving.
The
onSaveRecipientcallback passes the address straight through fromRecipientAddressModalwithout lowercasing it. TheRecipientAddressModalonly trims the input. Per the coding guidelines, EVM addresses must be lowercase. This could cause comparison mismatches downstream (e.g., line 65 whererecipientis compared toconnectedAddress).🔧 Suggested fix
const onSaveRecipient = useCallback( (address: string) => { - onRecipientChange?.(address); + onRecipientChange?.(address.toLowerCase()); }, [onRecipientChange], );As per coding guidelines: "Only lowercase EVM addresses; Solana and Cosmos addresses are case-sensitive." Since the protocol is known here, you could conditionally lowercase only for EVM, but if this component is EVM-only for now, a simple
.toLowerCase()does the job.src/features/tokens/TokenListPanel.tsx-44-50 (1)
44-50:⚠️ Potential issue | 🟡 Minor
aria-labelprop is silently dropped bySearchInput.Looking at
SearchInputinsrc/components/input/SearchInput.tsx, its interface only acceptsinputRef,value,onChange, andplaceholder— there's noaria-labelpassthrough. Soaria-label="Search tokens"here does nothing. The input relies solely onplaceholderfor its accessible name, which isn't ideal since placeholder text disappears on focus in some screen readers.Either add
aria-labelsupport toSearchInputor use theplaceholderas the sole accessible hint (current behavior).src/features/swap/hooks/useIcaTransaction.ts-101-101 (1)
101-101:⚠️ Potential issue | 🟡 MinorESLint: unused caught
errorvariable — prefix with underscore.Static analysis flags this. Quick fix to keep the linter happy.
Proposed fix
- } catch (error) { + } catch (_error) {src/features/wallet/RecipientAddressModal.tsx-24-36 (1)
24-36:⚠️ Potential issue | 🟡 MinorEVM addresses should be lowercased before saving.
When
protocolis Ethereum,onSave(trimmedAddress)should lowercase the address. For Solana/Cosmos it must be preserved as-is. Right now user-pasted checksummed addresses get stored in mixed case.As per coding guidelines: "Only lowercase EVM addresses; Solana and Cosmos addresses are case-sensitive."
Proposed fix
setError(''); - onSave(trimmedAddress); + onSave(protocol === ProtocolType.Ethereum ? trimmedAddress.toLowerCase() : trimmedAddress); close();src/features/chains/ChainFilterPanel.tsx-36-41 (1)
36-41:⚠️ Potential issue | 🟡 Minor
aria-labelis not part ofSearchInput's prop interface.Looking at the
SearchInputcomponent insrc/components/input/SearchInput.tsx, it accepts{ inputRef, value, onChange, placeholder }— noaria-label. This extra prop will be flagged by TypeScript as an invalid prop. If you want the aria attribute on the underlying<input>, you'd need to pass it through or extendSearchInput's props.src/features/transfer/TransfersDetailsModal.tsx-203-218 (1)
203-218:⚠️ Potential issue | 🟡 MinorDead
isFailedcheck in the loading branch — this ogre's nose can smell unreachable code.On line 207,
isFailed ? 'text-red-600' : 'text-gray-600'will never evaluate to'text-red-600'because this entire block is inside the!isFinalbranch (line 173's else). SinceisFinal = isSent || isFailed(line 104), being in!isFinalmeansisFailedis alwaysfalsehere.If the intention is to show a red error state for failed transfers in this view, the condition needs restructuring. If it's just leftover, simplify to
'text-gray-600'.Proposed simplification
- <div - className={`mt-5 text-center text-sm ${isFailed ? 'text-red-600' : 'text-gray-600'}`} - > + <div className="mt-5 text-center text-sm text-gray-600">src/features/tokens/TokenSelectField.tsx-159-162 (1)
159-162:⚠️ Potential issue | 🟡 MinorTailwind class typo:
cursor pointershould becursor-pointer.Missing the hyphen on line 161. Without it, Tailwind won't apply the pointer cursor on enabled state. Small thing, but it'll make the token button not show a hand cursor on hover.
🔧 Fix
const styles = { base: 'w-full py-2 flex items-center justify-between transition-all rounded-xl px-1.5 border duration-150 border-gray-400/25 shadow-sm group', - enabled: 'hover:bg-gray-50 cursor pointer', + enabled: 'hover:bg-gray-50 cursor-pointer', disabled: 'cursor-not-allowed opacity-60', };
| import { ChevronIcon } from '@hyperlane-xyz/widgets'; | ||
| import { SearchInput } from '../../components/input/SearchInput'; | ||
| import { ChainList } from './ChainList'; | ||
|
|
||
| interface ChainFilterPanelProps { | ||
| searchQuery: string; | ||
| onSearchChange: (s: string) => void; | ||
| selectedChain: ChainName | null; | ||
| onSelectChain: (chain: ChainName | null) => void; |
There was a problem hiding this comment.
ChainName type is used but never imported — this won't compile, donkey.
Line 8 references ChainName in the selectedChain prop type, but there's no import for it. This'll blow up at build time faster than an onion in a volcano.
🐛 Proposed fix
-import { ChevronIcon } from '@hyperlane-xyz/widgets';
+import { ChainName } from '@hyperlane-xyz/sdk';
+import { ChevronIcon } from '@hyperlane-xyz/widgets';📝 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.
| import { ChevronIcon } from '@hyperlane-xyz/widgets'; | |
| import { SearchInput } from '../../components/input/SearchInput'; | |
| import { ChainList } from './ChainList'; | |
| interface ChainFilterPanelProps { | |
| searchQuery: string; | |
| onSearchChange: (s: string) => void; | |
| selectedChain: ChainName | null; | |
| onSelectChain: (chain: ChainName | null) => void; | |
| import { ChainName } from '@hyperlane-xyz/sdk'; | |
| import { ChevronIcon } from '@hyperlane-xyz/widgets'; | |
| import { SearchInput } from '../../components/input/SearchInput'; | |
| import { ChainList } from './ChainList'; | |
| interface ChainFilterPanelProps { | |
| searchQuery: string; | |
| onSearchChange: (s: string) => void; | |
| selectedChain: ChainName | null; | |
| onSelectChain: (chain: ChainName | null) => void; |
🤖 Prompt for AI Agents
In `@src/features/chains/ChainFilterPanel.tsx` around lines 1 - 9, The prop type
ChainName used in the ChainFilterPanelProps (for selectedChain and
onSelectChain) is not imported; add the missing named import for ChainName (e.g.
import { ChainName } from '<module where ChainName is defined>') at the top of
ChainFilterPanel.tsx so the ChainFilterPanelProps type compiles; ensure the
import is a named import and matches the module that exports ChainName.
| const { data: icaAddress = null, isLoading, isError, refetch } = useQuery({ | ||
| queryKey: ['icaAddress', icaApp, userAddress, originChainName, destinationChainName] as const, |
There was a problem hiding this comment.
Putting an SDK object instance in the queryKey is like tossing a donkey into a pond — things get unstable fast.
icaApp is a class instance; React Query uses deep structural comparison for keys, but complex objects with methods/prototypes won't serialize or compare reliably. This can cause the query to re-fire on every render if the icaApp reference isn't stable (which it likely isn't if created in a hook).
Use a stable primitive identifier instead — for instance, a boolean or a hash of the config that produced it.
🧅 Proposed fix
const { data: icaAddress = null, isLoading, isError, refetch } = useQuery({
- queryKey: ['icaAddress', icaApp, userAddress, originChainName, destinationChainName] as const,
+ queryKey: ['icaAddress', userAddress, originChainName, destinationChainName] as const,
queryFn: async (): Promise<string | null> => {The enabled guard already prevents execution when icaApp is null, so it doesn't need to be in the key for correctness.
📝 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 { data: icaAddress = null, isLoading, isError, refetch } = useQuery({ | |
| queryKey: ['icaAddress', icaApp, userAddress, originChainName, destinationChainName] as const, | |
| const { data: icaAddress = null, isLoading, isError, refetch } = useQuery({ | |
| queryKey: ['icaAddress', userAddress, originChainName, destinationChainName] as const, |
🤖 Prompt for AI Agents
In `@src/features/swap/hooks/useIcaAddress.ts` around lines 16 - 17, The queryKey
for useQuery currently includes the class instance icaApp which is unstable;
remove icaApp from the queryKey and use a stable primitive instead (e.g., a
unique id/hash from the object or simply omit it and rely on the existing
enabled guard). Update the useQuery call that builds queryKey=['icaAddress',
icaApp, userAddress, originChainName, destinationChainName] to exclude icaApp or
replace it with icaApp?.id or a deterministic configHash, and ensure the enabled
option uses Boolean(icaApp) so the query only runs when the instance is
available; keep function names like useIcaAddress and useQuery unchanged.
| try { | ||
| // InterchainAccount requires EVM MultiProvider, not MultiProtocolProvider. | ||
| // Build one from the same chain metadata the store already holds. | ||
| const evmMultiProvider = new MultiProvider(multiProtocolProvider.metadata); | ||
| return InterchainAccount.fromAddressesMap(addressesMap, evmMultiProvider); | ||
| } catch { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Silent catch swallows errors — at least log it, will ya?
Right now if InterchainAccount.fromAddressesMap throws, you get null back with zero indication of what went wrong. That's the kind of thing that'll have you wandering the swamp for hours debugging.
🔧 Proposed fix
try {
// InterchainAccount requires EVM MultiProvider, not MultiProtocolProvider.
// Build one from the same chain metadata the store already holds.
const evmMultiProvider = new MultiProvider(multiProtocolProvider.metadata);
return InterchainAccount.fromAddressesMap(addressesMap, evmMultiProvider);
- } catch {
+ } catch (error) {
+ console.error('Failed to create InterchainAccount app', error);
return null;
}As per coding guidelines, "For unexpected issues (invalid state, broken invariants): fail loudly with throw or console.error. NEVER add silent fallbacks for unexpected issues."
📝 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.
| try { | |
| // InterchainAccount requires EVM MultiProvider, not MultiProtocolProvider. | |
| // Build one from the same chain metadata the store already holds. | |
| const evmMultiProvider = new MultiProvider(multiProtocolProvider.metadata); | |
| return InterchainAccount.fromAddressesMap(addressesMap, evmMultiProvider); | |
| } catch { | |
| return null; | |
| } | |
| try { | |
| // InterchainAccount requires EVM MultiProvider, not MultiProtocolProvider. | |
| // Build one from the same chain metadata the store already holds. | |
| const evmMultiProvider = new MultiProvider(multiProtocolProvider.metadata); | |
| return InterchainAccount.fromAddressesMap(addressesMap, evmMultiProvider); | |
| } catch (error) { | |
| console.error('Failed to create InterchainAccount app', error); | |
| return null; | |
| } |
🤖 Prompt for AI Agents
In `@src/features/swap/hooks/useInterchainAccount.ts` around lines 18 - 25, The
catch is silently swallowing errors in useInterchainAccount when
InterchainAccount.fromAddressesMap fails; update the catch to log the error
(including context like addressesMap and multiProtocolProvider.metadata) via
console.error or process logger and then rethrow (or return null only for
expected/handled cases), referencing the InterchainAccount.fromAddressesMap call
and the MultiProvider construction (new
MultiProvider(multiProtocolProvider.metadata)) so failures are visible for
debugging.
| const validate = async (values: TransferFormValues) => { | ||
| const routeType = getRouteType(warpCore, tokens, collateralGroups, values); | ||
|
|
||
| // Skip full warp validation for swap-bridge routes | ||
| if (routeType === 'swap-bridge') { | ||
| if (!values.amount || parseFloat(values.amount) <= 0) { | ||
| return { amount: 'Invalid amount' }; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Swap-bridge validation skips recipient check — this could let users submit with no recipient.
Right now, for swap-bridge routes, the only validation is that amount is positive. There's no check that a recipient (or at minimum a connected wallet on the destination chain) exists. If icaAddress derivation fails and the user hasn't entered a manual recipient, this could lead to a transaction with an empty or undefined recipient downstream.
At least validate that the connected destination wallet or ICA address is available before allowing submission.
🤖 Prompt for AI Agents
In `@src/features/transfer/TransferTokenForm.tsx` around lines 114 - 123, In
validate (used with TransferFormValues) after you detect routeType ===
'swap-bridge', in addition to checking values.amount ensure there is a valid
recipient by verifying either values.recipient is present/non-empty or the
derived ICA/destination wallet is available (the logic that derives ICA address
or checks connected destination wallet should be consulted here); if neither
exists return a validation error (e.g. { recipient: 'Recipient required' }) so
the form cannot submit with an empty/undefined recipient. Use getRouteType,
values.amount and values.recipient/ICA-derivation check to locate where to add
this check.
| function getRouteType( | ||
| _warpCore: WarpCore, | ||
| tokens: Token[], | ||
| collateralGroups: Map<string, Token[]>, | ||
| values: TransferFormValues, | ||
| ): TransferRouteType { | ||
| const originToken = getTokenByKey(tokens, values.originTokenKey); | ||
| const destinationToken = getTokenByKey(tokens, values.destinationTokenKey); | ||
| if (!originToken || !destinationToken) return 'unavailable'; | ||
| if (checkTokenHasRoute(originToken, destinationToken, collateralGroups)) return 'warp'; | ||
| if (isSwapSupported(originToken.chainName, destinationToken.chainName)) { | ||
| const destinationTokenAddress = | ||
| getSwappableAddress(destinationToken) ?? destinationToken.addressOrDenom; | ||
| if ( | ||
| isDemoSwapBridgePath({ | ||
| originChainName: originToken.chainName, | ||
| destinationChainName: destinationToken.chainName, | ||
| destinationTokenAddress, | ||
| destinationRouteAddress: destinationToken.addressOrDenom, | ||
| }) | ||
| ) { | ||
| return 'swap-bridge'; | ||
| } | ||
| } | ||
| return 'unavailable'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
getRouteType duplicates useTransferRoute logic.
This standalone function replicates the same route-classification logic that useTransferRoute (from ./useTransferRoute) already implements. According to the external snippet, useTransferRoute does the same origin/destination token checks, checkTokenHasRoute, isSwapSupported, and isDemoSwapBridgePath calls.
Having two copies means any future route-type change needs updating in both places. Consider extracting the pure logic into a shared function that both useTransferRoute and getRouteType (used in validate) can call.
#!/bin/bash
# Verify that useTransferRoute has the same logic as getRouteType
ast-grep --pattern 'export function useTransferRoute($$$) {
$$$
}'🤖 Prompt for AI Agents
In `@src/features/transfer/TransferTokenForm.tsx` around lines 1018 - 1043,
getRouteType duplicates the route-classification logic in useTransferRoute;
extract the pure decision logic into a single shared helper (e.g.,
determineTransferRoute or computeRouteType) that accepts the same inputs
(tokens, collateralGroups, values or the minimal inputs: originToken,
destinationToken, collateralGroups) and returns TransferRouteType, then update
both useTransferRoute and getRouteType to call that helper; ensure the helper
uses the same calls already present (getTokenByKey, checkTokenHasRoute,
isSwapSupported, getSwappableAddress, isDemoSwapBridgePath) and keep public
signatures compatible with TransferFormValues/TransferRouteType so callers need
only replace direct logic with a single call to the new function.
| PostingCommitment = 'posting-commitment', | ||
| SigningSwapBridge = 'signing-swap-bridge', | ||
| ConfirmingSwapBridge = 'confirming-swap-bridge', |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'SentTransferStatuses|FinalTransferStatuses' --type=ts --type=tsx -C3Repository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 111
🏁 Script executed:
# Also check if the new statuses are referenced elsewhere
rg -n 'PostingCommitment|SigningSwapBridge|ConfirmingSwapBridge' --type=ts --type=tsx -C2Repository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 111
🏁 Script executed:
# Look at the types.ts file to see the full status definitions
fd -n 'types.ts' src/features/transfer | head -1 | xargs cat -nRepository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 314
🏁 Script executed:
rg -n 'SentTransferStatuses|FinalTransferStatuses' -C3Repository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 3166
🏁 Script executed:
rg -n 'PostingCommitment|SigningSwapBridge|ConfirmingSwapBridge' -C2Repository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 2501
🏁 Script executed:
cat -n src/features/transfer/types.tsRepository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 1637
🏁 Script executed:
sed -n '6,42p' src/features/transfer/utils.tsRepository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 1912
🏁 Script executed:
# Check if isTransferSent or SentTransferStatuses is used elsewhere for polling/retry logic
rg -n 'isTransferSent' --context=5Repository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 2614
🏁 Script executed:
# Check how isFinal is used in TransfersDetailsModal and whether there's rendering logic affected
sed -n '102,130p' src/features/transfer/TransfersDetailsModal.tsxRepository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 1092
🏁 Script executed:
# Check if there are other uses of these status arrays that might be affected
rg -n 'getIconByTransferStatus' --context=10Repository: hyperlane-xyz/hyperlane-warp-ui-template
Length of output: 8164
Add status labels and finality classification for new swap-bridge statuses.
The three new statuses are missing from critical downstream handling:
-
Missing in
getTransferStatusLabel()— the UI will show blank ("...") forPostingCommitment,SigningSwapBridge, andConfirmingSwapBridge. -
Not in
SentTransferStatusesorFinalTransferStatuses— this breaks multiple flows:- In
TransfersDetailsModal,isFinalonly becomes true forConfirmedTransferorFailed. Swap-bridge transfers won't display the completion header (date + status icon). - In
SideBarMenu, only statuses inSTATUSES_WITH_ICONshow icons; swap-bridge transfers will show a spinner forever. - The wallet reconnection check (line 13 in
utils.ts) will show "Please connect wallet" even after the swap-bridge transaction is confirmed on-chain. - In
store.ts,failUnconfirmedTransfers()won't mark incomplete swap-bridge transfers as failed if the wallet disconnects.
- In
Add appropriate status labels in getTransferStatusLabel() and classify ConfirmingSwapBridge (and possibly PostingCommitment) in either SentTransferStatuses or FinalTransferStatuses, or handle them with dedicated logic.
🤖 Prompt for AI Agents
In `@src/features/transfer/types.ts` around lines 17 - 19, The new enum values
PostingCommitment, SigningSwapBridge, and ConfirmingSwapBridge were added but
not handled downstream; update getTransferStatusLabel() to return human-readable
labels for those three values, and add the new statuses to the appropriate
status sets (either SentTransferStatuses and/or FinalTransferStatuses) so
finality logic recognizes swap-bridge flows—specifically ensure
ConfirmingSwapBridge is treated as final (or Sent if you have a separate
in-flight concept) so TransfersDetailsModal.isFinal, STATUSES_WITH_ICON (used by
SideBarMenu), the wallet reconnection check in utils.ts, and
failUnconfirmedTransfers() in store.ts behave correctly; if you decide
PostingCommitment should remain in-flight, include it in SentTransferStatuses
but not FinalTransferStatuses, and ensure SigningSwapBridge is classified
consistently with its lifecycle.
| async function checkAndApprove( | ||
| walletClient: WalletClient, | ||
| publicClient: any, | ||
| tokenAddress: Address, | ||
| spender: Address, | ||
| amount: bigint, | ||
| ): Promise<void> { | ||
| const owner = walletClient.account?.address; | ||
| if (!owner) throw new Error('Wallet not connected'); | ||
|
|
||
| const currentAllowance = await publicClient.readContract({ | ||
| address: tokenAddress, | ||
| abi: erc20Abi, | ||
| functionName: 'allowance', | ||
| args: [owner, spender], | ||
| }); | ||
|
|
||
| if (currentAllowance < amount) { | ||
| const approveHash = await walletClient.writeContract({ | ||
| account: owner, | ||
| address: tokenAddress, | ||
| abi: erc20Abi, | ||
| functionName: 'approve', | ||
| args: [spender, maxUint256], | ||
| chain: null, | ||
| }); | ||
| await publicClient.waitForTransactionReceipt({ hash: approveHash, confirmations: 1 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
publicClient: any loses all type safety — get that outta me swamp.
The publicClient parameter on line 109 is typed as any, which means readContract and waitForTransactionReceipt calls have no compile-time checking. Since you're creating the client via createPublicClient from viem, you can use the proper return type.
Proposed fix
+import { type PublicClient } from 'viem';
+
async function checkAndApprove(
walletClient: WalletClient,
- publicClient: any,
+ publicClient: PublicClient,
tokenAddress: Address,
spender: Address,
amount: bigint,
): Promise<void> {🤖 Prompt for AI Agents
In `@src/features/transfer/useSwapBridgeTransfer.ts` around lines 107 - 135, The
parameter publicClient in checkAndApprove is typed as any, losing type safety
for publicClient.readContract and publicClient.waitForTransactionReceipt; change
the publicClient parameter type to viem's PublicClient (import { PublicClient }
from 'viem' or your project's viem wrapper) and update the function signature
async function checkAndApprove(walletClient: WalletClient, publicClient:
PublicClient, tokenAddress: Address, spender: Address, amount: bigint):
Promise<void>, leaving calls to publicClient.readContract and
publicClient.waitForTransactionReceipt as-is so TypeScript can validate their
return types and args (ensure createPublicClient is used where the client is
created so the type lines up).
| onStatusChange(TransferStatus.PostingCommitment); | ||
| try { | ||
| const payload = buildPostCallsPayload({ | ||
| calls: commitmentPayload.normalizedCalls, | ||
| relayers: [], | ||
| salt, | ||
| commitmentDispatchTx: hash, | ||
| originDomain: originConfig.domainId, | ||
| }); | ||
| const relayerResponse = await shareCallsWithPrivateRelayer( | ||
| COMMITMENTS_SERVICE_URL, | ||
| payload, | ||
| ); | ||
|
|
||
| if (!relayerResponse.ok) { | ||
| throw new Error('Relayer rejected commitment payload.'); | ||
| } | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Failed to post ICA call commitment to relayer service. Without this, reveal cannot execute destination calls. Root cause: ${toErrorMessage(error)}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
No timeout on relayer service call — could hang indefinitely.
The shareCallsWithPrivateRelayer call has no visible timeout. If the relayer service is down or unresponsive, this would leave the user stuck at the "Posting Commitment" state with no way to recover. The transaction is already on-chain at this point, so a hung post-commitment call means the user has paid fees but the reveal can't execute.
Consider adding a timeout or at minimum surfacing a more specific recovery message.
🤖 Prompt for AI Agents
In `@src/features/transfer/useSwapBridgeTransfer.ts` around lines 393 - 414, The
call to shareCallsWithPrivateRelayer in the
onStatusChange(TransferStatus.PostingCommitment) block can hang indefinitely;
modify this call to enforce a timeout and surface a clear recovery error: create
an AbortController (or use a timeout wrapper) and pass its signal into
shareCallsWithPrivateRelayer if the function supports it, or wrap the promise in
a Promise.race with a timeout that aborts/throws after a configurable duration
(e.g., 10–30s); on timeout, throw a descriptive error that includes context
(commitment hash, originDomain via buildPostCallsPayload and
COMMITMENTS_SERVICE_URL) and use toErrorMessage for consistency so callers can
present a specific recovery message instead of hanging.
Removed duplicate getIcaCommitRevealFee implementation that duplicated the SDK's getIcaFee. All call sites now import directly from @hyperlane-xyz/sdk. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extracted COMMITMENTS_SERVICE_URL, randomSalt, and toErrorMessage into src/features/swap/utils.ts. Both useIcaTransaction and useSwapBridgeTransfer now import from the shared module instead of defining their own copies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Consolidated split getIcaFee import into single SDK import block. Prettier auto-formatted import ordering in affected files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
3 participants
Summary
Continuation of #939 — Warp UI integration for Metaswaps demo (Optimism→Base USDC with ICA commit-reveal).
Related
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style
Improvements