Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
8e32dc7 to
f73679b
Compare
2b583a5 to
43c0281
Compare
43c0281 to
5d6ccb1
Compare
f73679b to
3fc7c62
Compare
|
@claude review |
|
Claude finished @paulbalaji's task in 1m 43s —— View job Review Complete
Submitted a consolidated PR review with 2 inline suggestions:
Overall the feature looks solid — good fail-closed design, proper decimal normalization, and clean test coverage. Nothing blocking. |
There was a problem hiding this comment.
Review Summary
Clean, well-structured feature. The fee parsing logic is sound — normalizing SentTransferRemote amounts back to native decimals via getWarpRouteAmountParts/getEffectiveDecimals is the right approach, and the fail-closed design (return null on any anomaly) is appropriate. The cursor bugbot issues from the earlier revision have been addressed.
Two minor items below worth considering; nothing blocking.
Nits
- The formatting-only changes (AppLayout, QuestionMarkIcon, MessageTable, WarpRouteVisualizationCard, Footer, Header, og.tsx, global.css) are fine but could've been a separate commit to keep the feature diff focused.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
5d6ccb1 to
8f62130
Compare
8f62130 to
6c1a3d0
Compare
6c1a3d0 to
2d1d657
Compare
2d1d657 to
26ee161
Compare
# This is the 1st commit message: feat: parse and display warp route fees from origin tx logs Parse ERC20 Transfer events and SentTransferRemote events from the origin transaction receipt to compute warp fees. Displays "Warp fee" and "Total sent" rows in the Warp Transfer Details card when fees > 0. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> # The commit message #2 will be skipped: # fix: address review issues in warp fee parsing # # - Fix decimals mismatch: normalize SentTransferRemote wireDecimals to # native token decimals before computing fee # - Switch useWarpFees to useQuery (fixes race condition, adds caching) # - Export and test actual parsing helpers instead of inline math # - Hoist interface objects to module scope # - Remove unused bridgeFeeRaw field and unnecessary types.ts # - Use formatAmountCompact for display # - Warn instead of silent fallback for missing decimals/negative fees # - Remove redundant isEvmChain check from hook # # Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
26ee161 to
8974943
Compare
|
Claude finished @paulbalaji's task in 9m 6s —— View job Review: Parse and display warp route fees
Submitted a PR review with 3 inline comments:
|
There was a problem hiding this comment.
Review Summary
Solid feature with fail-closed design and good test coverage. Two items need attention before merge.
1. Token address filtering regression - parseTotalErc20TransferredToRouter does not filter by log.address. The fix in adde1d7 appears lost during squash. Any ERC20 Transfer from sender to router gets summed regardless of which token contract emitted it.
2. Missing staleTime - Previously acknowledged but not applied. TX receipts are immutable; every remount triggers unnecessary RPC refetch.
See inline comments for details.
…malizeDecimals - parseTotalErc20TransferredToRouter now filters log.address to only match the expected token contract (collateralAddressOrDenom for collateral routes, routerAddress for synthetic) - Add staleTime: Infinity to useWarpFees since tx receipts are immutable - Export normalizeDecimals and add explicit test coverage Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis change adds warp fee calculation logic to the message flow. New modules compute bridge fees by parsing transaction receipts and extracting event data, a React hook queries these fees, and the details card displays the breakdown alongside existing message information. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as WarpTransferDetailsCard
participant Hook as useWarpFees Hook
participant Query as React Query
participant Fetch as fetchWarpFees
participant Provider as Origin Provider
participant Parser as Event Parser
UI->>Hook: useWarpFees(message, warpRouteDetails)
Hook->>Query: useQuery(fetchWarpFees, keys, options)
Query->>Fetch: fetchWarpFees(message, warpRouteDetails, multiProvider)
Fetch->>Provider: getTransactionReceipt(txHash)
Provider-->>Fetch: receipt with logs
Fetch->>Parser: parseSentTransferRemoteAmount(logs, router)
Parser-->>Fetch: sentAmount (wire decimals)
Fetch->>Parser: parseTotalErc20TransferredToRouter(logs, router, sender, token)
Parser-->>Fetch: totalTransferred (token decimals)
Fetch->>Fetch: normalizeDecimals(sentAmount, wireDecimals, tokenDecimals)
Fetch->>Fetch: Calculate bridgeFee = totalTransferred - sentAmount
Fetch-->>Query: WarpFeeBreakdown {bridgeFee, tokenSymbol, totalSent}
Query-->>Hook: data
Hook-->>UI: warpFees
UI->>UI: Render fee rows if warpFees exists
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/messages/warpFees/fetchWarpFees.ts`:
- Around line 55-76: The code currently treats the whole receipt as a single
warp send by calling parseSentTransferRemoteAmount(receipt.logs, routerAddress)
and then summing all ERC20 Transfer logs for the router/token; instead, locate
the specific SentTransferRemote log that belongs to this message (match by
message fields such as transferId/nonce/origin sender or by correlating
logIndex/sequence) and pass that identifier or log range into parsing so you
only sum Transfer events tied to that SentTransferRemote. Concretely: change
parseSentTransferRemoteAmount to accept message (or transferId/nonce) and return
the matching SentTransferRemote log (or its logIndex/identifier) rather than the
first match, and update parseTotalErc20TransferredToRouter to accept that
SentTransferRemote identifier/logIndex (or a filter function) so it only
aggregates Transfer events that are correlated to that specific
SentTransferRemote; apply the same per-message correlation where similar logic
exists in the 108-161 range.
In `@src/features/messages/warpFees/useWarpFees.ts`:
- Around line 12-18: The current useQuery call in useWarpFees (queryKey
['warpFees', message.id, warpRouteDetails?.originToken.addressOrDenom],
staleTime: Infinity) caches null when fetchWarpFees swallows errors, causing
permanent “no fees”; update fetchWarpFees to rethrow unexpected errors instead
of returning null so React Query treats failures as errors and will retry,
and/or remove/shorten staleTime: Infinity in the useQuery options and add
retry/retryDelay settings so transient RPC misses are retried on mount/focus;
locate function fetchWarpFees and the useQuery block in useWarpFees.ts and
implement the rethrow or query retry adjustments accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 98272492-0494-40ab-8482-e70c9a44c4cc
📒 Files selected for processing (4)
src/features/messages/cards/WarpTransferDetailsCard.tsxsrc/features/messages/warpFees/fetchWarpFees.test.tssrc/features/messages/warpFees/fetchWarpFees.tssrc/features/messages/warpFees/useWarpFees.ts
| // SentTransferRemote amount is in wireDecimals (max decimals across route) | ||
| const sentAmountWire = parseSentTransferRemoteAmount(receipt.logs, routerAddress); | ||
| if (!sentAmountWire) return null; | ||
|
|
||
| // For collateral routes the ERC20 token differs from the router; | ||
| // for synthetic routes the router IS the ERC20 token. | ||
| const tokenAddress = (warpRouteDetails.originToken as Record<string, unknown>) | ||
| .collateralAddressOrDenom as string | undefined; | ||
|
|
||
| // ERC20 Transfer amounts are in native token decimals | ||
| const totalTransferred = parseTotalErc20TransferredToRouter( | ||
| receipt.logs, | ||
| routerAddress, | ||
| message.origin.from, | ||
| tokenAddress || routerAddress, | ||
| ); | ||
| if (!totalTransferred || totalTransferred.isZero()) return null; | ||
|
|
||
| // Normalize sentAmount from wireDecimals to native decimals | ||
| const wireDecimals = warpRouteDetails.originToken.wireDecimals ?? decimals; | ||
| const sentAmount = normalizeDecimals(sentAmountWire, wireDecimals, decimals); | ||
|
|
There was a problem hiding this comment.
This treats the whole receipt as one warp send.
Line 56 grabs the first SentTransferRemote, while Lines 65-70 add every matching Transfer in the receipt. If one tx emits two warp sends through the same router/token, each message can wind up showing the wrong fee or the same duplicated fee. The parser needs to correlate logs to the specific message, not just the tx-wide router/sender/token tuple.
Also applies to: 108-161
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/messages/warpFees/fetchWarpFees.ts` around lines 55 - 76, The
code currently treats the whole receipt as a single warp send by calling
parseSentTransferRemoteAmount(receipt.logs, routerAddress) and then summing all
ERC20 Transfer logs for the router/token; instead, locate the specific
SentTransferRemote log that belongs to this message (match by message fields
such as transferId/nonce/origin sender or by correlating logIndex/sequence) and
pass that identifier or log range into parsing so you only sum Transfer events
tied to that SentTransferRemote. Concretely: change
parseSentTransferRemoteAmount to accept message (or transferId/nonce) and return
the matching SentTransferRemote log (or its logIndex/identifier) rather than the
first match, and update parseTotalErc20TransferredToRouter to accept that
SentTransferRemote identifier/logIndex (or a filter function) so it only
aggregates Transfer events that are correlated to that specific
SentTransferRemote; apply the same per-message correlation where similar logic
exists in the 108-161 range.
| const { data } = useQuery({ | ||
| // eslint-disable-next-line @tanstack/query/exhaustive-deps | ||
| queryKey: ['warpFees', message.id, warpRouteDetails?.originToken.addressOrDenom], | ||
| queryFn: () => fetchWarpFees(message, warpRouteDetails!, multiProvider), | ||
| enabled: !!warpRouteDetails, | ||
| staleTime: Infinity, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack React Query v5 docs: if a query function catches an error and returns null, is that treated as a successful cached result? With staleTime: Infinity, will that null stay fresh and skip automatic refetches/retries?
💡 Result:
Yes, if a query function catches an error and returns null (or any value), it is treated as a successful result by TanStack React Query v5. The library determines success solely by whether the queryFn promise resolves without throwing/rejecting. A resolved promise (with data=null) sets the query status to "success", caches the null value, and with staleTime: Infinity, that null will remain fresh. This prevents automatic refetches (like on mount, window focus, etc.) and skips retries (since no error occurred). To treat it as an error, the queryFn must throw. Null is a valid data value (unlike undefined, which is disallowed since v4). The cached null acts as any successful data for staleness checks.
Citations:
- 1: https://tanstack.com/query/v5/docs/framework/react/guides/query-functions
- 2: https://tanstack.com/query/v5/docs/framework/react/reference/useQuery
- 3: https://tanstack.com/query/v5/docs/framework/react/guides/important-defaults
- 4: Undefined not allowed as return value of queryFn since v4 TanStack/query#4964
🏁 Script executed:
find src -name "fetchWarpFees.ts" -o -name "useWarpFees.ts" | head -20Repository: hyperlane-xyz/hyperlane-explorer
Length of output: 169
🏁 Script executed:
cat -n src/features/messages/warpFees/useWarpFees.tsRepository: hyperlane-xyz/hyperlane-explorer
Length of output: 971
🏁 Script executed:
cat -n src/features/messages/warpFees/fetchWarpFees.tsRepository: hyperlane-xyz/hyperlane-explorer
Length of output: 6999
Layer some error retry logic to avoid caching transient RPC misses forever.
When fetchWarpFees catches an error and returns null, React Query treats that as a successful result. With staleTime: Infinity on line 17, that null sticks around for the entire session—even if the RPC was just temporarily unreachable. One brief blip becomes a permanent "no fees" state instead of retrying on the next mount or window focus.
The function logs the error (good), but swallowing it into a cached null violates the guideline: "Fail loudly with throw or console.error for unexpected issues; never add silent fallbacks." Either let unexpected errors throw so React Query retries them, or use a shorter staleTime with explicit retry logic.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/messages/warpFees/useWarpFees.ts` around lines 12 - 18, The
current useQuery call in useWarpFees (queryKey ['warpFees', message.id,
warpRouteDetails?.originToken.addressOrDenom], staleTime: Infinity) caches null
when fetchWarpFees swallows errors, causing permanent “no fees”; update
fetchWarpFees to rethrow unexpected errors instead of returning null so React
Query treats failures as errors and will retry, and/or remove/shorten staleTime:
Infinity in the useQuery options and add retry/retryDelay settings so transient
RPC misses are retried on mount/focus; locate function fetchWarpFees and the
useQuery block in useWarpFees.ts and implement the rethrow or query retry
adjustments accordingly.

Summary
Transferevents (total tokens sent to router) vsSentTransferRemoteevent (net bridged amount)Changes
src/features/messages/warpFees/types.tsWarpFeeBreakdowntypesrc/features/messages/warpFees/fetchWarpFees.tssrc/features/messages/warpFees/useWarpFees.tssrc/features/messages/warpFees/fetchWarpFees.test.tssrc/features/messages/cards/WarpTransferDetailsCard.tsxHow it works
SentTransferRemote(uint32,bytes32,uint256)to get net bridged amountTransfer(from,to,value)events from sender → router for total tokens pulledTest plan
pnpm run typecheck— cleanpnpm run lint— cleanpnpm run test— 3 new tests pass, no regressions🤖 Generated with Claude Code
Note
Medium Risk
Medium risk because it introduces new on-chain receipt fetching and log parsing that can fail silently or miscompute fees for certain routes/senders, impacting user-facing transfer details.
Overview
Warp transfer details now optionally show fees. The
WarpTransferDetailsCardcalls a newuseWarpFeeshook and, when available, displaysWarp feeandTotal sentrows.New fee parsing logic fetches the origin transaction receipt for EVM ERC20 routes and derives the fee by subtracting the router
SentTransferRemoteamount (normalized for decimals) from the total ERC20Transferamount sent from the tx sender to the router; unsupported cases (non-EVM, native tokens, missing decimals, unexpected logs) returnnull. Basic unit tests cover the log parsers and decimal normalization helpers.Written by Cursor Bugbot for commit c789ab8. This will update automatically on new commits. Configure here.