PRO-3185-Gas Tank Paymaster #388
Conversation
Deploying x with
|
| Latest commit: |
b476b64
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://715b81a8.x-e62.pages.dev |
| Branch Preview URL: | https://pro-3185-gas-tank-paymaster.x-e62.pages.dev |
WalkthroughAdds a new Gas Tank app: UI components (balance, history, top-up), LiFi swap orchestration, paymaster integration, balance/history hooks, Redux slice and typed hooks, many utilities/types, Sentry observability, search/portfolio UI with tests, Send modal integration, and PillarX dashboard tile. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant UG as UniversalGasTank
participant HM as TopUpModal
participant OF as useOffer
participant LiFi as LiFi API
participant PM as Paymaster API
participant GB as useGasTankBalance
U->>UG: Click "Top up"
UG->>HM: open modal
HM->>GB: ensure portfolio & balances loaded
U->>HM: select token & amount
alt token ≠ USDC
HM->>OF: getBestOffer()
OF->>LiFi: getRoutes
LiFi-->>OF: routes
OF-->>HM: offer
HM->>OF: getStepTransactions(route...)
OF-->>HM: StepTransaction[]
end
HM->>PM: request deposit txs for USDC (if applicable)
PM-->>HM: deposit StepTransaction[]
HM->>UG: trigger batch send
UG->>GB: refetch balance
GB->>PM: GET /getGasTankBalance?sender=wallet
PM-->>GB: per-chain balances
GB-->>UG: update UI
sequenceDiagram
autonumber
participant U as User
participant SM as SendModalTokensTabView
participant Svc as services.getGasTankBalance
participant PM as Paymaster API
U->>SM: Open Send modal
SM->>Svc: fetch Gas Tank balance
Svc-->>SM: totalBalance
alt totalBalance > 0
SM->>SM: enable/select "Gas Tank Paymaster"
SM->>PM: use /gasTankPaymaster URL for batches
else
SM->>SM: default to Native Token fees
end
U->>SM: send
SM->>PM: submit via chosen paymaster path
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (2)
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: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (3)
160-185: Fix: remove unused gasTankBalance to satisfy ESLint and pipelinegasTankBalance is never read and breaks CI.
- const gasTankPaymasterUrl = `${paymasterUrl}/gasTankPaymaster`; - const [isPaymaster, setIsPaymaster] = React.useState<boolean>(true); + const gasTankPaymasterUrl = `${paymasterUrl}/gasTankPaymaster`; + const [isPaymaster, setIsPaymaster] = React.useState<boolean>(true); @@ - const [selectedFeeType, setSelectedFeeType] = - React.useState<string>('Native Token'); - const [gasTankBalance, setGasTankBalance] = React.useState<number>(0); + const [selectedFeeType, setSelectedFeeType] = + React.useState<string>('Native Token');Also remove setGasTankBalance below (see next comment).
232-264: Do not mutate feeTypeOptions; build a fresh, ordered arraypush/reverse on a shared array inside effects leads to duplicates and nondeterministic ordering across renders, and it also conflicts with selectedFeeType defaults.
- getGasTankBalance(accountAddress ?? '').then((res) => { - feeTypeOptions.push({ - id: 'GasTankPaymaster', - title: 'Gas Tank Paymaster', - type: 'token', - value: '', - }); - if (res) { - setGasTankBalance(res); - if (res > 0) { - feeTypeOptions.reverse(); - setFeeType(feeTypeOptions); - setIsPaymaster(true); - setPaymasterContext({ - mode: 'gasTankPaymaster', - }); - setSelectedFeeType('Gas Tank Paymaster'); - } else { - setIsPaymaster(false); - setPaymasterContext(null); - setSelectedFeeType('Native Token'); - } - } else { - setIsPaymaster(false); - setPaymasterContext(null); - setSelectedFeeType('Native Token'); - } - }); + getGasTankBalance(accountAddress ?? '').then((res) => { + const hasGasTank = typeof res === 'number' && res > 0; + // Always include Gasless and Native Token; optionally include Gas Tank + const options = [ + { id: 'Native Token', title: 'Native Token', type: 'token', value: '' }, + { id: 'Gasless', title: 'Gasless', type: 'token', value: '' }, + ...(hasGasTank + ? [{ id: 'GasTankPaymaster', title: 'Gas Tank Paymaster', type: 'token', value: '' }] + : []), + ]; + setFeeType(options); + + if (hasGasTank) { + setIsPaymaster(true); + setPaymasterContext({ mode: 'gasTankPaymaster' }); + setSelectedFeeType('Gas Tank Paymaster'); + } else { + setIsPaymaster(false); + setPaymasterContext(null); + setSelectedFeeType('Native Token'); + } + });This removes mutation, duplication, and defines an explicit order.
1429-1436: Use Gas Tank paymaster URL when fee type is Gas Tank in batches payloads tooThe transaction payload path already switches URLs; the batches payload path should mirror it.
- paymaster={ - isPaymaster - ? { - url: `${paymasterUrl}${queryString}`, - context: paymasterContext, - } - : undefined - } + paymaster={ + isPaymaster + ? { + url: `${selectedFeeType === 'Gasless' ? paymasterUrl : gasTankPaymasterUrl}${queryString}`, + context: paymasterContext, + } + : undefined + }
🧹 Nitpick comments (34)
src/apps/gas-tank/manifest.json (1)
4-8: Add description to translations for i18n parity.Include a localized description alongside the title to keep UX consistent across locales.
Apply this diff:
"translations": { "en": { - "title": "Gas Tank by PillarX" + "title": "Gas Tank by PillarX", + "description": "Universal Gas Tank for PillarX" } }src/apps/gas-tank/styles/gasTank.css (2)
2-6: Ensure the.gas-tank-appclass is actually applied to a container.Currently this only styles elements with
.gas-tank-app. If not applied in the rendered tree, the dark theme won’t take effect.If not already, add the class on the top-level wrapper (see suggested change in src/apps/gas-tank/index.tsx). After adding, confirm the background/text colors render as expected across breakpoints.
9-13: Avoid spacing conflicts with styled-components.There are responsive paddings both here and in the Wrapper styled-component. This can cause double-padding if both apply. Prefer one source of truth for spacing.
src/apps/gas-tank/utils/converters.ts (1)
1-4: Handle scientific notation when inspecting decimals.Numbers like 0.00000001 stringify as "1e-8", making the current string-split logic fail. Use a fixed representation when checking the decimal prefix.
Apply this diff:
-export const hasThreeZerosAfterDecimal = (num: number): boolean => { - const decimalPart = num.toString().split('.')[1] || ''; - return decimalPart.startsWith('000'); -}; +export const hasThreeZerosAfterDecimal = (num: number): boolean => { + // Use fixed representation to avoid scientific notation pitfalls (e.g., 1e-8). + const [, decimalPart = ''] = Number(num).toFixed(8).split('.'); + return decimalPart.startsWith('000'); +};src/apps/gas-tank/index.tsx (2)
10-16: Apply the.gas-tank-appclass to activate global styles.Ensures the dark theme and responsive overrides in gasTank.css actually apply.
Apply this diff:
return ( - <Wrapper> + <Wrapper className="gas-tank-app"> <GasTank /> </Wrapper> );
1-1: Avoid disabling eslint; define Wrapper before use instead.Remove the file-wide disable and move the styled-component above the App to satisfy the rule and improve readability.
Apply this diff:
-/* eslint-disable @typescript-eslint/no-use-before-define */ import styled from 'styled-components'; // styles import './styles/gasTank.css'; // components import GasTank from './components/GasTank'; +const Wrapper = styled.div` + display: flex; + width: 100%; + margin: 0 auto; + flex-direction: column; + max-width: 1248px; + padding: 32px; + + @media (min-width: 1024px) { + padding: 52px 62px; + } + + @media (max-width: 1024px) { + padding: 52px 32px; + } + + @media (max-width: 768px) { + padding: 32px 16px; + } +`; export const App = () => { return ( - <Wrapper> + <Wrapper className="gas-tank-app"> <GasTank /> </Wrapper> ); }; -const Wrapper = styled.div` - display: flex; - width: 100%; - margin: 0 auto; - flex-direction: column; - max-width: 1248px; - padding: 32px; - - @media (min-width: 1024px) { - padding: 52px 62px; - } - - @media (max-width: 1024px) { - padding: 52px 32px; - } - - @media (max-width: 768px) { - padding: 32px 16px; - } -`;Also applies to: 18-37
src/services/gasless.ts (2)
84-84: Typo: “cumalative” → “cumulative”.Minor comment typo fix for clarity.
If you keep the comment, correct the spelling:
- // Get cumalative balance for all chains + // Get cumulative balance for all chains
14-17: AlignChainBalance.chainIdtype across implementationsThe
ChainBalanceinterface is in active use (in both the gasless service and the GasTank hook), so removing it isn’t safe. To avoid type mismatches with other parts of the codebase (e.g.Paymasters.chainId: number), update thechainIdfield to accept both numbers and strings.Locations to update:
- src/services/gasless.ts →
interface ChainBalance(near top of file)- src/apps/gas-tank/hooks/useGasTankBalance.tsx →
interface ChainBalance(at file start)Suggested diff for both definitions:
interface ChainBalance { - chainId: string; + chainId: number | string; balance: string; }src/apps/gas-tank/reducer/gasTankSlice.ts (2)
7-13: Rename state type from SwapState to GasTankState for clarity.The current name suggests a different domain and can be confusing when reading the store.
Apply this diff:
-export type SwapState = { +export type GasTankState = { walletPortfolio: PortfolioData | undefined; }; -const initialState: SwapState = { +const initialState: GasTankState = { walletPortfolio: undefined, };
15-30: Consider exporting memoized selectors with reselect.Providing
selectWalletPortfolio(and others later) improves encapsulation and avoids repetitive state shape knowledge across the app.I can add a small selectors module (e.g., src/apps/gas-tank/reducer/selectors.ts) using
createSelector. Want me to draft it?src/apps/gas-tank/components/GasTankHistory.tsx (3)
101-103: Derive positivity from the entry type, not from string prefixes.Relying on string prefixes is brittle and hides data inconsistencies.
Apply this diff:
- <AmountCell isPositive={entry.amount.startsWith('+')}> + <AmountCell isPositive={entry.type === 'Top-up'}> {entry.amount} </AmountCell>
33-46: Fix sample data inconsistencies between Type and Amount sign.Top-ups should be positive; spends should be negative.
Apply this diff:
{ id: '3', date: 'Apr 27, 16:03', type: 'Top-up', - amount: '-$110 (gas)', + amount: '+$110.00', token: { symbol: 'USDC', value: '119', icon: '🔵' }, }, { id: '4', date: 'Apr 27, 16:45', type: 'Spend', - amount: '+$47.50', + amount: '-$47.50 (gas)', token: { symbol: 'OP', value: '30.84', icon: '🔴' }, },
77-79: Date parsing is locale-dependent; consider storing ISO timestamps and formatting for display.new Date('Apr 27, 16:45') is ambiguous and can parse differently across environments. Prefer ISO timestamps (e.g., 2025-04-27T16:45:00Z) in data and a formatter (Intl.DateTimeFormat) in the UI. Also, the header suggests sorting, but it’s static—consider removing arrows or wiring sort handlers later.
Would you like me to update the mock data to ISO strings and add a simple formatter and optional sort handlers?
Also applies to: 89-95, 148-176
src/apps/gas-tank/hooks/useGasTankBalance.tsx (3)
41-50: Encode query param and be resilient to trailing slashes.Encode the address to be safe in URLs.
Apply this diff:
- const response = await fetch( - `${paymasterUrl}/getGasTankBalance?sender=${walletAddress}`, + const response = await fetch( + `${paymasterUrl.replace(/\/+$/, '')}/getGasTankBalance?sender=${encodeURIComponent(String(walletAddress))}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, } );
83-87: Optional: abort in-flight requests on unmount or address change to prevent stale updates.Using AbortController avoids setting state from outdated requests when the address changes rapidly.
I can provide a minimal AbortController integration if you want to harden this hook against race conditions.
1-98: Consider centralizing this fetch in a service and reusing error/logging conventions.AI summary mentions a getGasTankBalance service in src/services/gasless.ts. Aligning to that avoids duplication and keeps logging/metrics consistent.
Do you already have src/services/gasless.ts in this PR? If yes, I can refactor this hook to use it.
src/apps/gas-tank/utils/types.tsx (3)
10-18: Prefer string/bigint amounts to avoid floating-point precision lossUsing number for token amounts is error-prone (JS floats). Recommend switching to string (decimal) for human-readable values or bigint for base units to preserve precision end-to-end.
Apply this diff to make the most error-prone fields stringly typed:
export type SwapType = { - fromAmount: number; + fromAmount: string; // decimal string or base-units string fromTokenAddress: string; fromChainId: number; fromTokenDecimals: number; slippage?: number; fromAccountAddress?: string; provider?: BridgingProvider; }; export type SwapOffer = { - tokenAmountToReceive: number; + tokenAmountToReceive: string; // decimal string or base-units string offer: Route; };If your downstream code already expects number, we can add a narrow adapter at boundaries instead. Let me know if you want me to draft it.
Also applies to: 20-23
30-39: Narrow StepTransaction.type to known EIP-2718/EIP-1559 variantstype?: number | string is too broad. Recommend explicit union for type safety.
export type StepTransaction = { to?: string; data?: Hex; value?: bigint; gasLimit?: bigint; gasPrice?: bigint; chainId?: number; - type?: number | string; + // 0 = legacy, 1 = access list, 2 = EIP-1559; some libs use hex-literals + type?: 0 | 1 | 2 | '0x0' | '0x1' | '0x2'; transactionType?: StepType; };
1-8: Nit: this file has no JSX; consider renaming to .tsKeeps the repo consistent and improves editor tooling.
src/apps/gas-tank/components/UniversalGasTank.tsx (4)
58-61: Guard against undefined/NaN balance before calling toFixedIf totalBalance is ever undefined or NaN, this will throw. Safer to coalesce to 0.
- return <Balance>${totalBalance.toFixed(2)}</Balance>; + return <Balance>${(Number.isFinite(totalBalance) ? totalBalance : 0).toFixed(2)}</Balance>;
33-37: Add accessible labels to the refresh buttonImprove a11y: screen readers don’t read emoji meaningfully.
- {!isBalanceLoading && !balanceError && ( - <RefreshButton onClick={refetch} title="Refresh balance"> + {!isBalanceLoading && !balanceError && ( + <RefreshButton + onClick={refetch} + title="Refresh balance" + aria-label="Refresh balance" + > 🔄 </RefreshButton> )}
63-64: Add accessible labels to the “Top up” buttonA small a11y improvement.
- <TopUpButton onClick={handleTopUp}>Top up</TopUpButton> + <TopUpButton onClick={handleTopUp} aria-label="Top up Gas Tank">Top up</TopUpButton>
69-73: Hard-coded Total Spend value likely a placeholderIf this is a temporary stub, consider either:
- Hide until the endpoint is ready, or
- Add a TODO comment with a link to the work item to prevent it shipping inadvertently.
src/apps/gas-tank/utils/wrappedTokens.ts (1)
8-21: Gnosis/XDAI exclusion acknowledged; verify chain coverage is sufficientThe XDAI exclusion matches prior learnings. Consider whether you also need entries for additional chains you support in the Gas Tank (e.g., Avalanche, zkSync) to avoid inconsistent wrapping.
Would you like me to open a follow-up adding other mainnets you support with their wrapped native tokens?
src/apps/gas-tank/components/TopUpModal.tsx (2)
174-201: DRY: factor out repeated BigInt normalizationYou repeat value normalization. Extract a small helper to reduce duplication and potential inconsistency.
+// helper near the top of the file +const normalizeValueToWeiString = (value: unknown): string => { + if (typeof value === 'bigint') return value.toString(); + if (value === undefined || value === null || value === '0') return '0'; + try { + return BigNumber.from(value as string).toString(); + } catch { + return '0'; + } +};Then replace the repeated blocks with:
- const value = tx.value || '0'; - // Handle bigint conversion properly - let bigIntValue: bigint; - if (typeof value === 'bigint') { - bigIntValue = value; - } else if (value) { - bigIntValue = BigNumber.from(value).toBigInt(); - } else { - bigIntValue = BigInt(0); - } - const integerValue = formatEther(bigIntValue); + const integerValue = formatEther(BigInt(normalizeValueToWeiString(tx.value)));Also applies to: 201-226
156-166: Paymaster API call: add basic error detail and defensive parsingImprove debuggability and resilience.
- const response = await fetch( + const response = await fetch( `${paymasterUrl}/getTransactionForDeposit?chainId=${chainNameToChainIdTokensData(selectedToken.blockchain)}&amount=${receiveSwapAmount}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, } ); - if (!response.ok) { - throw new Error('Failed to get deposit transaction'); - } + if (!response.ok) { + const text = await response.text().catch(() => ''); + throw new Error(`Failed to get deposit transaction: ${response.status} ${text}`); + } - const transactionData = await response.json(); + const transactionData = await response.json().catch(() => ({}));Also applies to: 171-174
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
1314-1332: Remove stray console log and tighten fee type switchingconsole.log left in production and branching is slightly inconsistent; remove the log and keep logic concise.
- const handleOnChangeFeeAsset = (value: SelectOption) => { - console.log('handleOnChangeFeeAsset', value, selectedFeeType); + const handleOnChangeFeeAsset = (value: SelectOption) => { setSelectedFeeType(value.title); if (value.title === 'Gasless') { setPaymasterContext({ mode: 'commonerc20', token: selectedFeeAsset?.token, }); setIsPaymaster(true); } else if (value.title === 'Gas Tank Paymaster') { setPaymasterContext({ mode: 'gasTankPaymaster', }); setIsPaymaster(true); } else { setPaymasterContext(null); setIsPaymaster(false); } };
1397-1409: Ensure the fee-type Select reflects selectedFeeType; force remount with keyYour Select stores internal selectedId only on mount via defaultSelectedId. When selectedFeeType changes programmatically, the UI may not update. Force remount keyed by selectedFeeType.
- <Select + <Select + key={selectedFeeType} type="token" onChange={handleOnChangeFeeAsset} options={feeType} isLoadingOptions={ feeAssetOptions.length === 0 && selectedFeeType !== 'Gas Tank Paymaster' } defaultSelectedId={feeType[0].id} />Apply similarly to the non-payload path:
- <Select + <Select + key={selectedFeeType} type="token" onChange={handleOnChangeFeeAsset} options={feeType} isLoadingOptions={ feeAssetOptions.length === 0 && selectedFeeType !== 'Gas Tank Paymaster' } defaultSelectedId={feeType[0].id} />Also applies to: 1539-1551
src/apps/gas-tank/hooks/useOffer.tsx (4)
1-51: Missing environment configuration for USDC addresses on additional chains.The
USDC_ADDRESSESobject only covers 3 chains (Polygon, Arbitrum, Optimism) but the app appears to support more chains based on the blockchain utilities. Consider adding USDC addresses for other supported chains (Ethereum mainnet, BSC, Base, Gnosis) to ensure the swap functionality works across all networks.const USDC_ADDRESSES: { [chainId: number]: string } = { + 1: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48', // Ethereum Mainnet + 56: '0x8AC76a51cc950d9822D68b83fE1Ad97B32Cd580d', // BSC + 100: '0xDDAfbb505ad214D7b80b1f830fcCc89B60fb7A83', // Gnosis (USDC.e) + 8453: '0x833589fCD6eDb6E08f4c7C32D4f71b54bdA02913', // Base // Add more chains as needed };
148-150: Consider making the platform fee configurable.The 1% platform fee is hardcoded. Consider making this value configurable through environment variables or configuration files to allow for easier adjustments without code changes.
+ const platformFeePercentage = import.meta.env.VITE_SWAP_FEE_PERCENTAGE || 1; const fromAmountInWei = parseUnits(String(fromAmount), fromTokenDecimals); - const feeDeduction = fromAmountInWei / BigInt(100); // 1% fee + const feeDeduction = fromAmountInWei * BigInt(platformFeePercentage) / BigInt(100); const fromAmountFeeDeducted = fromAmountInWei - feeDeduction;
463-473: Improve bigint conversion logic for type safety.The current bigint conversion logic is repetitive and could be simplified with a helper function. Also, consider handling potential parsing errors.
Add a helper function at the top of the file:
+const safeToBigInt = (value: unknown, defaultValue = BigInt(0)): bigint => { + if (typeof value === 'bigint') return value; + if (value == null) return defaultValue; + try { + return BigInt(String(value)); + } catch { + console.warn('Failed to convert value to bigint:', value); + return defaultValue; + } +};Then use it:
- const valueBigInt = - typeof value === 'bigint' ? value : BigInt(String(value || 0)); - const gasLimitBigInt = - typeof gasLimit === 'bigint' - ? gasLimit - : BigInt(String(gasLimit || 0)); - const gasPriceBigInt = - typeof gasPrice === 'bigint' - ? gasPrice - : BigInt(String(gasPrice || 0)); + const valueBigInt = safeToBigInt(value); + const gasLimitBigInt = safeToBigInt(gasLimit); + const gasPriceBigInt = safeToBigInt(gasPrice);
61-118: Remove unusedgetNativeFeeForERC20in gas-tank/hooks
- Location:
src/apps/gas-tank/hooks/useOffer.tsx(around lines 61–118).- This function is never called in this file or elsewhere in the codebase.
- Note: a nearly identical implementation lives in
src/apps/the-exchange/hooks/useOffer.tsx. If you need this logic in both apps, extract it into a shared utility; otherwise, delete the unused function to eliminate dead code.src/apps/gas-tank/utils/blockchain.ts (1)
109-121: Normalize chain name comparisons in chainNameToChainIdTokensDataThe
chainNameToChainIdTokensDataimplementation insrc/services/tokensData.tsonly matches exact, case-sensitive names (e.g."Ethereum","Polygon", etc.) and returns0for any variation or different casing:• src/services/tokensData.ts (lines ~234–243):
export const chainNameToChainIdTokensData = (chain: string|undefined) => { switch (chain) { case 'Ethereum': return 1; case 'Polygon': return 137; … default: return 0; } };• src/apps/gas-tank/utils/blockchain.ts (lines 109–121) and other callers rely on this for matching
t.blockchain.If
t.blockchainever differs in casing (e.g.'ethereum','ETHEREUM', etc.), the function will fall back to0, causing incorrect chain-ID comparisons and token lookups.Recommendation: normalize the incoming
chainstring before the switch—e.g.:- export const chainNameToChainIdTokensData = (chain: string|undefined) => { + export const chainNameToChainIdTokensData = (chain?: string) => { + const key = chain?.trim().toLowerCase(); switch (chain) { - case 'Ethereum': return 1; + case 'ethereum': return 1; - case 'Polygon': return 137; + case 'polygon': return 137; … - default: return 0; + default: return 0; } };This ensures any casing or leading/trailing whitespace is handled consistently across all consumers.
src/apps/gas-tank/utils/sentry.ts (1)
100-101: Improve operation message formatting.The message formatting uses string manipulation that could be simplified and made more robust.
Sentry.captureMessage( - `${operationType.charAt(0).toUpperCase() + operationType.slice(1)} operation: ${operation}`, + `${operationType[0]?.toUpperCase()}${operationType.slice(1)} operation: ${operation}`, 'info' );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
src/apps/gas-tank/icon.pngis excluded by!**/*.png
📒 Files selected for processing (18)
src/apps/gas-tank/components/GasTank.tsx(1 hunks)src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)src/apps/gas-tank/components/UniversalGasTank.tsx(1 hunks)src/apps/gas-tank/hooks/useGasTankBalance.tsx(1 hunks)src/apps/gas-tank/hooks/useOffer.tsx(1 hunks)src/apps/gas-tank/hooks/useReducerHooks.tsx(1 hunks)src/apps/gas-tank/index.tsx(1 hunks)src/apps/gas-tank/manifest.json(1 hunks)src/apps/gas-tank/reducer/gasTankSlice.ts(1 hunks)src/apps/gas-tank/styles/gasTank.css(1 hunks)src/apps/gas-tank/utils/blockchain.ts(1 hunks)src/apps/gas-tank/utils/converters.ts(1 hunks)src/apps/gas-tank/utils/sentry.ts(1 hunks)src/apps/gas-tank/utils/types.tsx(1 hunks)src/apps/gas-tank/utils/wrappedTokens.ts(1 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(13 hunks)src/services/gasless.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-23T14:44:33.911Z
Learnt from: RanaBug
PR: pillarwallet/x#315
File: src/apps/the-exchange/utils/wrappedTokens.ts:6-20
Timestamp: 2025-05-23T14:44:33.911Z
Learning: XDAI (Gnosis Chain) is intentionally excluded from the WRAPPED_NATIVE_TOKEN_ADDRESSES mapping in the exchange app's wrappedTokens utility.
Applied to files:
src/apps/gas-tank/utils/wrappedTokens.ts
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/gas-tank/utils/converters.ts
🧬 Code Graph Analysis (5)
src/apps/gas-tank/hooks/useReducerHooks.tsx (1)
src/store.ts (2)
AppDispatch(100-100)RootState(98-98)
src/apps/gas-tank/components/TopUpModal.tsx (2)
src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/apps/gas-tank/utils/converters.ts (1)
formatTokenAmount(6-11)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
src/services/gasless.ts (1)
getGasTankBalance(63-93)src/components/Form/Select/index.tsx (1)
Select(75-146)
src/apps/gas-tank/utils/blockchain.ts (3)
src/apps/gas-tank/utils/types.tsx (1)
StepTransaction(30-39)src/services/tokensData.ts (1)
Token(19-29)src/apps/gas-tank/utils/wrappedTokens.ts (1)
isNativeToken(30-31)
src/apps/gas-tank/hooks/useOffer.tsx (5)
src/apps/gas-tank/utils/blockchain.ts (3)
processEth(18-24)getNativeBalanceFromPortfolio(129-141)toWei(124-126)src/apps/gas-tank/utils/types.tsx (3)
SwapType(10-18)SwapOffer(20-23)StepTransaction(30-39)src/apps/gas-tank/utils/wrappedTokens.ts (3)
getWrappedTokenAddressIfNative(33-44)isWrappedToken(23-28)isNativeToken(30-31)src/apps/deposit/utils/blockchain.tsx (1)
getNetworkViem(60-79)src/services/tokensData.ts (1)
Token(19-29)
🪛 GitHub Actions: vignesha22 has committed new code! 🚀 Checking...
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx
[error] 184-184: ESLint: 'gasTankBalance' is assigned a value but never used. (no-unused-vars)
🔇 Additional comments (14)
src/apps/gas-tank/manifest.json (1)
1-9: Manifest schema is consistent across apps; no extra keys neededThe
gas-tank/manifest.jsononly definestitle,description, andtranslations, which matches every other app manifest insrc/apps(aside from the outlierfaq-app’s optionalsupportkey). There’s no precedent for top-levelid,slug,entry, oriconfields in existing manifests—so you can safely ignore the previous suggestion to add them.Likely an incorrect or invalid review comment.
src/apps/gas-tank/utils/converters.ts (1)
6-11: Stabilize formatTokenAmount return type & guard invalid numbersUnify the return value to always be a string and handle
undefined,NaN, orInfinityinputs:-export const formatTokenAmount = (amount?: number) => { - if (amount === undefined) return 0; - return hasThreeZerosAfterDecimal(amount) - ? amount.toFixed(8) - : amount.toFixed(4); -}; +export const formatTokenAmount = (amount?: number): string => { + if (amount === undefined || !Number.isFinite(amount)) return '0'; + return hasThreeZerosAfterDecimal(amount) + ? amount.toFixed(8) + : amount.toFixed(4); +};• All current usages (in TopUpModal.tsx, ExchangeOffer.tsx, and ExchangeAction.tsx) render the returned value directly in JSX, so switching from
number|stringto alwaysstringis safe.
• No downstream arithmetic is performed on the output—it's purely for display.src/apps/gas-tank/hooks/useReducerHooks.tsx (1)
1-6: React-Redux v9.1.2 detected: withTypes-based hooks are supported, no fallback needed.
TypeduseAppDispatch/useAppSelectorcan remain as-is.src/apps/gas-tank/components/GasTank.tsx (1)
8-19: Clean composition and responsive layout.Straightforward container that composes the left and right sections; styles look consistent and responsive. No issues spotted.
src/apps/gas-tank/reducer/gasTankSlice.ts (1)
1-33: Ignore reducer export recommendation — slice isn’t wired into the storeWe searched
src/store.tsand found:
- No
importofgasTankSlice(default or otherwise)- No
gasTank:key in anycombineReducersorconfigureStorecall- No references to
gasTankSlice.reducerSince the slice’s default export isn’t used by the store at all, switching it to
.reducerwould have no effect. If you intend to add this slice to your root reducer, import and attachgasTankSlice.reducerexplicitly; otherwise, you can safely disregard this suggestion.Likely an incorrect or invalid review comment.
src/apps/gas-tank/utils/wrappedTokens.ts (2)
1-5: Normalize input to lowercase once at call sites; your set already contains lowercase entriesYour isNativeToken lowercases input before has(), which is correct. Ensure callers don’t pre-normalize inconsistently; otherwise you’re good here.
33-44: getWrappedTokenAddressIfNative: returns original address if mapping missingThis fallback is sensible and safe. LGTM.
src/apps/gas-tank/components/TopUpModal.tsx (2)
119-144: Verify “value” unit for addToBatch/EtherspotTransactionWe currently convert
tx.value(wei) to an ether string viaformatEtherbefore callingaddToBatch. IfaddToBatch/EtherspotTransactionexpects a wei‐basedBigNumberishor string, this conversion will under-send by 1e18×.• Confirm in the EtherspotTransactionKit docs or type definitions whether the
valueprop should be passed in wei.
• If wei is required, change each use ofconst integerValue = formatEther(bigIntValue);to
const integerValue = bigIntValue.toString();in
– src/apps/gas-tank/components/TopUpModal.tsx lines 119–144 (swap batch)
– src/apps/gas-tank/components/TopUpModal.tsx lines 171–226 (top-up loops)
– src/apps/gas-tank/components/TopUpModal.tsx lines 215–221 (single transaction)
100-117: Verify paymaster amount unitsWe’ve confirmed that getBestOffer internally converts your
Number(amount)into base units viaparseUnits, and getStepTransactions uses the route’s ownfromAmount(already in base units) for transaction values. No change is needed there.However, the paymaster API call at TopUpModal.tsx (around lines 125–132) passes
receiveSwapAmount(a human-readable decimal string) directly in the query:const response = await fetch( `${paymasterUrl}/getTransactionForDeposit?chainId=${chainId}&amount=${receiveSwapAmount}`, { … } );Most paymaster endpoints expect the amount in the smallest unit (e.g. USDC’s 6-decimals base unit). Please confirm the
/getTransactionForDepositAPI’s expected unit. If it requires base units, convert before fetching:- const response = await fetch( - `${paymasterUrl}/getTransactionForDeposit?chainId=${chainId}&amount=${receiveSwapAmount}`, - { … } - ); + import { parseUnits } from 'viem'; + + const usdcBaseAmount = parseUnits(receiveSwapAmount, 6).toString(); + const response = await fetch( + `${paymasterUrl}/getTransactionForDeposit?chainId=${chainId}&amount=${usdcBaseAmount}`, + { … } + );src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (4)
1379-1390: Approval transaction should only render for ERC-20 paymaster modeYou already guard with selectedFeeType === 'Gasless', which is correct. Looks good after the URL fix.
Also applies to: 1605-1616
221-231: Avoid proceeding when walletPortfolio is absent; you already guard, LGTMThe early return prevents unnecessary work and flicker. Good.
1595-1601: Switching paymaster URL based on selectedFeeType is correctMirrors the transaction payload branch. After the URL and Select fixes, this path should be robust.
184-184: ESLint failure: ‘gasTankBalance’ unusedSee earlier fix to remove state. This will resolve the pipeline error:
[pipeline] ESLint: 'gasTankBalance' is assigned a value but never used. (no-unused-vars)
src/apps/gas-tank/utils/blockchain.ts (1)
75-92: Unsafe array access in ERC20 transfer decoding.The code accesses
decoded.args[1]without proper bounds checking beyond just checking if the array exists. This could cause runtime errors if the decoded args array has fewer than 2 elements.const decoded = decodeFunctionData({ abi: erc20Abi, data: tx.data || '0x', }); if ( decoded.args && Array.isArray(decoded.args) && - decoded.args.length > 1 && + decoded.args.length >= 2 && typeof decoded.args[1] === 'bigint' ) { return formatUnits(decoded.args[1], decimals); } + console.warn('Unexpected ERC20 transfer args structure:', decoded.args); + return '0';Likely an incorrect or invalid review comment.
| // Extract chain balances | ||
| const balances: ChainBalance[] = Object.values(data.balance || {}); | ||
| setChainBalances(balances); | ||
|
|
||
| // Calculate total balance by summing all chain balances | ||
| const total = balances.reduce((sum, chainBalance) => { | ||
| const balance = parseFloat(chainBalance.balance) || 0; | ||
| return sum + balance; | ||
| }, 0); | ||
|
|
||
| setTotalBalance(total); | ||
| } catch (err) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Support both object-map and array-of-objects response shapes.
Some backends return a map of chainId → string/number, others return ChainBalance objects. Handle both.
Apply this diff:
- // Extract chain balances
- const balances: ChainBalance[] = Object.values(data.balance || {});
- setChainBalances(balances);
-
- // Calculate total balance by summing all chain balances
- const total = balances.reduce((sum, chainBalance) => {
- const balance = parseFloat(chainBalance.balance) || 0;
- return sum + balance;
- }, 0);
+ // Extract chain balances (supports { [chainId]: number|string|{ chainId, balance } })
+ const entries = Object.entries(data.balance || {});
+ const balances: ChainBalance[] = entries.map(([cid, val]) => {
+ if (val && typeof val === 'object' && 'balance' in (val as any)) {
+ const v = val as Partial<ChainBalance> & { balance: unknown };
+ return {
+ chainId: v.chainId ?? cid,
+ balance: String(v.balance ?? '0'),
+ };
+ }
+ return { chainId: cid, balance: String(val ?? '0') };
+ });
+ setChainBalances(balances);
+
+ // Calculate total balance by summing all chain balances
+ const total = balances.reduce((sum, cb) => sum + (parseFloat(cb.balance) || 0), 0);📝 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.
| // Extract chain balances | |
| const balances: ChainBalance[] = Object.values(data.balance || {}); | |
| setChainBalances(balances); | |
| // Calculate total balance by summing all chain balances | |
| const total = balances.reduce((sum, chainBalance) => { | |
| const balance = parseFloat(chainBalance.balance) || 0; | |
| return sum + balance; | |
| }, 0); | |
| setTotalBalance(total); | |
| } catch (err) { | |
| // Extract chain balances (supports { [chainId]: number|string|{ chainId, balance } }) | |
| const entries = Object.entries(data.balance || {}); | |
| const balances: ChainBalance[] = entries.map(([cid, val]) => { | |
| if (val && typeof val === 'object' && 'balance' in (val as any)) { | |
| const v = val as Partial<ChainBalance> & { balance: unknown }; | |
| return { | |
| chainId: v.chainId ?? cid, | |
| balance: String(v.balance ?? '0'), | |
| }; | |
| } | |
| return { chainId: cid, balance: String(val ?? '0') }; | |
| }); | |
| setChainBalances(balances); | |
| // Calculate total balance by summing all chain balances | |
| const total = balances.reduce((sum, cb) => sum + (parseFloat(cb.balance) || 0), 0); | |
| setTotalBalance(total); |
🤖 Prompt for AI Agents
In src/apps/gas-tank/hooks/useGasTankBalance.tsx around lines 58 to 69, the code
only handles an array of ChainBalance objects; update it to support both
response shapes (an object map of chainId → string|number and an array of
ChainBalance). Normalize data.balance into a ChainBalance[]: if it's an array
use it directly, if it's an object map convert Object.entries(...) into [{
chainId, balance: String(value) }]. Then compute total by iterating the
normalized array, parsing each balance to a number with parseFloat or
Number(...) and defaulting to 0 for invalid values, call setChainBalances with
the normalized array, and setTotalBalance with the computed sum.
| export const useWalletAddressForLogging = () => { | ||
| const walletAddress = useWalletAddress(); | ||
| return walletAddress || 'unknown_wallet_address'; | ||
| }; |
There was a problem hiding this comment.
Duplicate export of useWalletAddressForLogging.
The function useWalletAddressForLogging is exported twice (lines 196 and at the implied end of the public exports list from the AI summary). This appears to be a duplicate that should be removed.
Remove the duplicate function definition. Keep only one instance of this function.
🤖 Prompt for AI Agents
In src/apps/gas-tank/utils/sentry.ts around lines 196 to 199 there is a
duplicate export/definition of useWalletAddressForLogging; remove the redundant
copy so only one implementation remains. Keep the preferred implementation
(ensure it still uses useWalletAddress and returns walletAddress ||
'unknown_wallet_address'), remove the other duplicate declaration and any
duplicate export entry, and update any export list or index references so the
function is exported exactly once.
| export const startExchangeTransaction = ( | ||
| operation: string, | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| _data: Record<string, unknown> = {}, | ||
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | ||
| _walletAddress?: string | ||
| ) => { | ||
| return Sentry.startSpan( | ||
| { | ||
| name: `exchange.${operation}`, | ||
| op: 'exchange.operation', | ||
| }, | ||
| (span) => { | ||
| // Note: Span API has changed in Sentry v10 | ||
| // Properties are set via the span context instead | ||
| return span; | ||
| } | ||
| ); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ineffective transaction monitoring implementation.
The startExchangeTransaction function creates a Sentry span but doesn't use the provided _data and _walletAddress parameters (they're prefixed with underscore indicating they're unused). The span also doesn't set any meaningful attributes or tags, making it less useful for monitoring.
export const startExchangeTransaction = (
operation: string,
- // eslint-disable-next-line @typescript-eslint/no-unused-vars
- _data: Record<string, unknown> = {},
- // eslint-disable-next-line @typescript-eslint/no-unused-vars
- _walletAddress?: string
+ data: Record<string, unknown> = {},
+ walletAddress?: string
) => {
return Sentry.startSpan(
{
name: `exchange.${operation}`,
op: 'exchange.operation',
+ attributes: {
+ 'wallet.address': walletAddress || fallbackWalletAddressForLogging(),
+ 'operation.type': operation,
+ ...Object.entries(data).reduce((acc, [key, value]) => {
+ acc[`exchange.${key}`] = String(value);
+ return acc;
+ }, {} as Record<string, string>)
+ }
},
(span) => {
- // Note: Span API has changed in Sentry v10
- // Properties are set via the span context instead
return span;
}
);
};📝 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 startExchangeTransaction = ( | |
| operation: string, | |
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |
| _data: Record<string, unknown> = {}, | |
| // eslint-disable-next-line @typescript-eslint/no-unused-vars | |
| _walletAddress?: string | |
| ) => { | |
| return Sentry.startSpan( | |
| { | |
| name: `exchange.${operation}`, | |
| op: 'exchange.operation', | |
| }, | |
| (span) => { | |
| // Note: Span API has changed in Sentry v10 | |
| // Properties are set via the span context instead | |
| return span; | |
| } | |
| ); | |
| }; | |
| export const startExchangeTransaction = ( | |
| operation: string, | |
| data: Record<string, unknown> = {}, | |
| walletAddress?: string | |
| ) => { | |
| return Sentry.startSpan( | |
| { | |
| name: `exchange.${operation}`, | |
| op: 'exchange.operation', | |
| attributes: { | |
| 'wallet.address': walletAddress || fallbackWalletAddressForLogging(), | |
| 'operation.type': operation, | |
| ...Object.entries(data).reduce((acc, [key, value]) => { | |
| acc[`exchange.${key}`] = String(value); | |
| return acc; | |
| }, {} as Record<string, string>) | |
| } | |
| }, | |
| (span) => { | |
| return span; | |
| } | |
| ); | |
| }; |
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx
Outdated
Show resolved
Hide resolved
| export const getGasTankBalance = async ( | ||
| walletAddress: string | ||
| ): Promise<number | null> => { | ||
| try { | ||
| const response = await fetch( | ||
| `http://localhost:5000/pillarx-staging/us-central1/paymaster/getGasTankBalance?sender=${walletAddress}`, | ||
| { | ||
| method: 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to fetch gas tank balance: ${response.status}`); | ||
| } | ||
|
|
||
| const data = await response.json(); | ||
| const balances: ChainBalance[] = Object.values(data.balance || {}); | ||
| // Get cumalative balance for all chains | ||
| const total = balances.reduce((sum, chainBalance) => { | ||
| const balance = parseFloat(chainBalance.balance) || 0; | ||
| return sum + balance; | ||
| }, 0); | ||
| return total; | ||
| } catch (error) { | ||
| console.error('Error fetching gas tank balance:', error); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Hard-coded localhost URL; use env base URL and encode params.
A localhost/staging URL will break outside dev and can leak environments. Use the same env base used elsewhere, encode the sender, and make JSON parsing robust.
Apply this diff:
export const getGasTankBalance = async (
walletAddress: string
): Promise<number | null> => {
try {
- const response = await fetch(
- `http://localhost:5000/pillarx-staging/us-central1/paymaster/getGasTankBalance?sender=${walletAddress}`,
- {
- method: 'GET',
- headers: {
- 'Content-Type': 'application/json',
- },
- }
- );
+ if (!walletAddress) {
+ throw new Error('walletAddress is required');
+ }
+ const baseUrl = import.meta.env.VITE_PAYMASTER_URL;
+ if (!baseUrl) {
+ throw new Error('Missing VITE_PAYMASTER_URL');
+ }
+ const url = `${baseUrl}/getGasTankBalance?sender=${encodeURIComponent(walletAddress)}`;
+ const response = await fetch(url, { method: 'GET' });
if (!response.ok) {
throw new Error(`Failed to fetch gas tank balance: ${response.status}`);
}
const data = await response.json();
- const balances: ChainBalance[] = Object.values(data.balance || {});
- // Get cumalative balance for all chains
- const total = balances.reduce((sum, chainBalance) => {
- const balance = parseFloat(chainBalance.balance) || 0;
- return sum + balance;
- }, 0);
+ const balanceMap = (data?.balance ?? {}) as Record<string, unknown>;
+ // Get cumulative balance across all chains
+ const total = Object.values(balanceMap).reduce((sum, v) => {
+ const balanceStr =
+ v && typeof v === 'object' && 'balance' in (v as any)
+ ? String((v as any).balance)
+ : String(v);
+ const n = Number.parseFloat(balanceStr);
+ return sum + (Number.isFinite(n) ? n : 0);
+ }, 0);
return total;
} catch (error) {
console.error('Error fetching gas tank balance:', error);
return null;
}
};📝 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 getGasTankBalance = async ( | |
| walletAddress: string | |
| ): Promise<number | null> => { | |
| try { | |
| const response = await fetch( | |
| `http://localhost:5000/pillarx-staging/us-central1/paymaster/getGasTankBalance?sender=${walletAddress}`, | |
| { | |
| method: 'GET', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch gas tank balance: ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| const balances: ChainBalance[] = Object.values(data.balance || {}); | |
| // Get cumalative balance for all chains | |
| const total = balances.reduce((sum, chainBalance) => { | |
| const balance = parseFloat(chainBalance.balance) || 0; | |
| return sum + balance; | |
| }, 0); | |
| return total; | |
| } catch (error) { | |
| console.error('Error fetching gas tank balance:', error); | |
| return null; | |
| } | |
| }; | |
| export const getGasTankBalance = async ( | |
| walletAddress: string | |
| ): Promise<number | null> => { | |
| try { | |
| if (!walletAddress) { | |
| throw new Error('walletAddress is required'); | |
| } | |
| const baseUrl = import.meta.env.VITE_PAYMASTER_URL; | |
| if (!baseUrl) { | |
| throw new Error('Missing VITE_PAYMASTER_URL'); | |
| } | |
| const url = `${baseUrl}/getGasTankBalance?sender=${encodeURIComponent(walletAddress)}`; | |
| const response = await fetch(url, { method: 'GET' }); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch gas tank balance: ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| const balanceMap = (data?.balance ?? {}) as Record<string, unknown>; | |
| // Get cumulative balance across all chains | |
| const total = Object.values(balanceMap).reduce((sum, v) => { | |
| const balanceStr = | |
| v && typeof v === 'object' && 'balance' in (v as any) | |
| ? String((v as any).balance) | |
| : String(v); | |
| const n = Number.parseFloat(balanceStr); | |
| return sum + (Number.isFinite(n) ? n : 0); | |
| }, 0); | |
| return total; | |
| } catch (error) { | |
| console.error('Error fetching gas tank balance:', error); | |
| return null; | |
| } | |
| }; |
|
|
||
| const GasTankHistory = () => { | ||
| const [historyData] = useState<HistoryEntry[]>([ | ||
| { |
There was a problem hiding this comment.
Is this supposed to be live data?
There was a problem hiding this comment.
yeah so I am building that data from indexer my work is done there now building a api on x-firebase to get this array
| <HeaderCell>Date ▲</HeaderCell> | ||
| <HeaderCell>Type ▼</HeaderCell> | ||
| <HeaderCell>Amount ▼</HeaderCell> | ||
| <HeaderCell>Token ▼</HeaderCell> |
There was a problem hiding this comment.
remember to check the AI Agent output as the column sort buttons are text character arrows here but should sort
There was a problem hiding this comment.
yeah one thing at a time I saw it and thought I will change this to sorted once I get all the data
| if (isBalanceLoading) { | ||
| return ( | ||
| <LoadingBalance> | ||
| <CircularProgress size={24} style={{ color: '#8b5cf6' }} /> |
There was a problem hiding this comment.
Can we move this into a style class
| 1: '0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2', // WETH | ||
| // Polygon | ||
| 137: '0x0d500B1d8E8eF31E21C99d1Db9A6444d3ADf1270', // WMATIC | ||
| // Optimism | ||
| 10: '0x4200000000000000000000000000000000000006', // WETH | ||
| // Arbitrum | ||
| 42161: '0x82aF49447D8a07e3bd95BD0d56f35241523fBab1', // WETH | ||
| // Base | ||
| 8453: '0x4200000000000000000000000000000000000006', // WETH | ||
| // BNB | ||
| 56: '0xbb4CdB9CBd36B01bD1cBaEBF2De08d9173bc095c', // WBNB |
There was a problem hiding this comment.
Double check these values please
There was a problem hiding this comment.
These values were copied from the-exchange app
| }; | ||
|
|
||
| const handleOnChangeFeeAsset = (value: SelectOption) => { | ||
| console.log('handleOnChangeFeeAsset', value, selectedFeeType); |
There was a problem hiding this comment.
Use transactionDebugLog here
src/services/gasless.ts
Outdated
| ): Promise<number | null> => { | ||
| try { | ||
| const response = await fetch( | ||
| `http://localhost:5000/pillarx-staging/us-central1/paymaster/getGasTankBalance?sender=${walletAddress}`, |
There was a problem hiding this comment.
Can we replace this with a configurable URL please
| }, 0); | ||
| return total; | ||
| } catch (error) { | ||
| console.error('Error fetching gas tank balance:', error); |
There was a problem hiding this comment.
Sentry logging here please
| <HeaderCell>Date ▲</HeaderCell> | ||
| <HeaderCell>Type ▼</HeaderCell> | ||
| <HeaderCell>Amount ▼</HeaderCell> | ||
| <HeaderCell>Token ▼</HeaderCell> |
There was a problem hiding this comment.
I'm assuming the same as previously mentioned - this is supposed to be sortable
github-actions
bot
requested a deployment
to
Preview (PRO-3185-Gas-Tank-Paymaster)
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (10)
src/apps/gas-tank/components/TopUpModal.tsx (7)
77-78: Add Sentry error logging as requestedAs requested by the reviewer, please add Sentry error capture here for better monitoring.
if (walletPortfolioDataError) { - console.error(walletPortfolioDataError); + console.error(walletPortfolioDataError); + logExchangeError( + walletPortfolioDataError instanceof Error + ? walletPortfolioDataError + : 'Failed to load wallet portfolio', + { + walletAddress, + error: walletPortfolioDataError + }, + { component: 'TopUpModal', action: 'portfolio_load_error' } + ); }Also add the import at the top:
+import { logExchangeError } from '../utils/sentry';
119-121: Add Sentry logging for missing swap offersAs requested, add Sentry logging when no best offer is found, and ensure the user experience is handled gracefully.
if (!bestOffer) { setErrorMsg('No best offer found for the swap. Please try a different token or amount.'); - console.warn('No best offer found for swap'); + logExchangeEvent( + 'No best offer found for swap', + 'warning', + { + fromToken: selectedToken.contract, + fromAmount: amount, + chainId: chainNameToChainIdTokensData(selectedToken.blockchain), + tokenSymbol: selectedToken.symbol + }, + { component: 'TopUpModal', action: 'swap_offer_not_found' } + ); return; }Also add the import if not already present:
+import { logExchangeEvent, logExchangeError } from '../utils/sentry';
158-165: Add Sentry capture for swap errorsAs requested, add Sentry error capture when swap transactions fail.
} catch (swapError) { console.error('Error getting swap transactions:', swapError); - console.warn( - 'Failed to get swap route. Please try a different token or amount.' - ); + logExchangeError( + swapError instanceof Error ? swapError : 'Failed to get swap route', + { + fromToken: selectedToken.contract, + fromAmount: amount, + chainId: chainNameToChainIdTokensData(selectedToken.blockchain), + tokenSymbol: selectedToken.symbol + }, + { component: 'TopUpModal', action: 'swap_transaction_error' } + ); setErrorMsg('Failed to get swap route. Please try a different token or amount.'); setIsProcessing(false); return;
179-185: Handle paymaster API errors properlyThe error handling needs proper user recovery flow as mentioned in the review comments. The current implementation sets error message but doesn't properly throw or handle the error in the calling context.
if (!response.ok) { const errorText = await response.text(); - console.error('Error fetching transaction data:', errorText); + logExchangeError( + `Paymaster API error: ${errorText}`, + { + chainId: chainNameToChainIdTokensData(selectedToken.blockchain), + amount: receiveSwapAmount, + statusCode: response.status, + errorText + }, + { component: 'TopUpModal', action: 'paymaster_api_error' } + ); setErrorMsg('Failed to fetch transaction data. Please try again with different token or amount.'); setIsProcessing(false); return; }
188-188: Use transaction debug logger instead of console.logAs requested by the reviewer, use the proper transaction debug logger.
First, add the import:
+import { useTransactionDebugLogger } from '../../../hooks/useTransactionDebugLogger';Then in the component:
+const { transactionDebugLog } = useTransactionDebugLogger();Replace the console.log:
-console.log('Transaction data:', transactionData); +transactionDebugLog('Transaction data received', transactionData);
248-251: Capture errors with Sentry and inform the userThe generic error handling needs Sentry capture and proper user notification.
} catch (error) { console.error('Error processing top-up:', error); - console.warn('Failed to process top-up. Please try again.'); + logExchangeError( + error instanceof Error ? error : 'Failed to process top-up', + { + selectedToken: selectedToken?.contract, + amount, + chainId: selectedToken ? chainNameToChainIdTokensData(selectedToken.blockchain) : undefined + }, + { component: 'TopUpModal', action: 'topup_process_error' } + ); + setErrorMsg('Failed to process top-up. Please try again later.'); } finally {
283-324: Fix token list loading check - empty array is truthyThe condition
if (!portfolioTokens)will never render the loading state since[]is truthy. Use the query flags instead.{(() => { - if (!portfolioTokens) { + if (!isWalletPortfolioDataSuccess && !walletPortfolioDataError) { return ( <LoadingContainer> <CircularProgress size={24} /> <span>Loading wallet tokens...</span> </LoadingContainer> ); } if (walletPortfolioDataError) { return ( <ErrorMessage>Failed to load wallet tokens</ErrorMessage> ); } return ( <TokenList> {portfolioTokens.map((token) => (src/apps/gas-tank/utils/sentry.ts (3)
13-18: Fallback wallet address always returns placeholder valueThe
fallbackWalletAddressForLoggingfunction always returns 'unknown_wallet_address' regardless of context, providing no useful debugging information. Consider implementing a proper fallback mechanism or removing this function entirely.Either remove the fallback function and make wallet address required:
-export const fallbackWalletAddressForLogging = (): string => { - // This is a utility function that should be called from within React components - // The actual wallet address should be passed as a parameter or obtained via hook - // For now, return unknown as this is a fallback utility - return 'unknown_wallet_address'; -};Or implement a proper context-based fallback:
+let globalWalletAddress: string | null = null; + +export const setGlobalWalletAddress = (address: string) => { + globalWalletAddress = address; +}; + export const fallbackWalletAddressForLogging = (): string => { - return 'unknown_wallet_address'; + return globalWalletAddress || 'unknown_wallet_address'; };
205-223: Unused parameters in startExchangeTransactionThe
_dataand_walletAddressparameters are prefixed with underscore indicating they're unused, making the span less useful for monitoring.export const startExchangeTransaction = ( operation: string, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _data: Record<string, unknown> = {}, - // eslint-disable-next-line @typescript-eslint/no-unused-vars - _walletAddress?: string + data: Record<string, unknown> = {}, + walletAddress?: string ) => { return Sentry.startSpan( { name: `exchange.${operation}`, op: 'exchange.operation', + attributes: { + 'wallet.address': walletAddress || fallbackWalletAddressForLogging(), + 'operation.type': operation, + ...Object.entries(data).reduce((acc, [key, value]) => { + acc[`exchange.${key}`] = String(value); + return acc; + }, {} as Record<string, string>) + } }, (span) => { - // Note: Span API has changed in Sentry v10 - // Properties are set via the span context instead return span; } ); };
177-177: Inconsistent app_module tagThe
logPerformanceMetricfunction uses 'the-exchange' as the app_module while all other functions use 'gasTank'. This should be consistent.-scope.setTag('app_module', 'the-exchange'); +scope.setTag('app_module', 'gasTank');
🧹 Nitpick comments (5)
src/apps/gas-tank/components/TopUpModal.tsx (2)
95-98: Use contract address for USDC validation instead of symbolThe current check uses the contract address which is correct. However, the error message should be clearer about which chains are supported.
Consider improving the error message to specify supported chains:
-setErrorMsg('Gas Tank is not supported on the selected token\'s chain.'); +const supportedChainNames = Object.entries(USDC_ADDRESSES) + .filter(([_, address]) => address !== undefined) + .map(([chainId]) => { + // Convert chainId back to chain name for display + const chain = supportedChains.find(c => c.id === Number(chainId)); + return chain?.name || chainId; + }) + .join(', '); +setErrorMsg(`Gas Tank is only supported on: ${supportedChainNames}`);
365-365: Consider using contract address comparison for USDC checkWhile checking by symbol works, it would be more reliable to check by contract address to ensure it's actually USDC and not another token with the same symbol.
-if (selectedToken?.symbol?.toUpperCase() === 'USDC') { +if (selectedToken?.contract === USDC_ADDRESSES[chainNameToChainIdTokensData(selectedToken.blockchain)]) { return 'Add to Gas Tank'; }src/apps/gas-tank/index.tsx (2)
66-67: Avoid using 'any' type - use proper typing for wallet clientThe use of
anytype bypasses TypeScript's type safety. Consider defining a proper type or interface.-// eslint-disable-next-line @typescript-eslint/no-explicit-any -getWalletClient: async () => provider as any, +getWalletClient: async () => provider as unknown as WalletClient,You may need to import the proper type from LiFi SDK or define an appropriate interface based on what LiFi expects.
176-177: Repeated use of 'any' typeAnother instance where proper typing would be better than using
any.-// eslint-disable-next-line @typescript-eslint/no-explicit-any -return provider as any; +return provider as unknown as WalletClient;src/apps/gas-tank/utils/sentry.ts (1)
196-199: Consider removing the duplicate hook definitionThe
useWalletAddressForLogginghook appears to be a wrapper that doesn't add much value over usinguseWalletAddressdirectly. Consider removing it to reduce code duplication.Either remove this hook and use
useWalletAddressdirectly throughout the codebase, or enhance it to provide additional functionality such as caching or validation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)src/apps/gas-tank/index.tsx(1 hunks)src/apps/gas-tank/utils/sentry.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
PR: pillarwallet/x#374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/gas-tank/index.tsx
🧬 Code graph analysis (2)
src/apps/gas-tank/components/TopUpModal.tsx (5)
src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/services/tokensData.ts (2)
PortfolioToken(31-34)chainNameToChainIdTokensData(234-255)src/types/api.ts (1)
PortfolioData(731-740)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/converters.ts (1)
formatTokenAmount(6-11)
src/apps/gas-tank/index.tsx (2)
src/apps/gas-tank/utils/sentry.ts (3)
initSentryForGasTank(5-8)logExchangeEvent(21-46)addExchangeBreadcrumb(226-244)src/utils/blockchain.ts (1)
supportedChains(139-148)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
src/apps/gas-tank/components/TopUpModal.tsx (1)
48-48: Use of string type for amount is appropriate given the contextBased on the previous discussion, using string for the amount is correct since it will be sent to x-firebase to get transaction data. The string type also aligns with the numeric input validation in the component.
| }, | ||
| }), | ||
| ], | ||
| apiKey: import.meta.env.VITE_LIFI_API_KEY, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Validate environment variable availability
The LiFi API key is critical for the app to function. Add validation to ensure it exists.
+const lifiApiKey = import.meta.env.VITE_LIFI_API_KEY;
+if (!lifiApiKey) {
+ const error = new Error('VITE_LIFI_API_KEY environment variable is not set');
+ console.error(error);
+ logExchangeEvent(
+ 'Missing LiFi API key',
+ 'error',
+ { walletAddress },
+ { component: 'App', action: 'missing_env_var' }
+ );
+ throw error;
+}
createConfig({
integrator: 'PillarX',
providers: [
// ... provider config
],
- apiKey: import.meta.env.VITE_LIFI_API_KEY,
+ apiKey: lifiApiKey,
});📝 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.
| apiKey: import.meta.env.VITE_LIFI_API_KEY, | |
| // Validate that the LiFi API key is provided | |
| const lifiApiKey = import.meta.env.VITE_LIFI_API_KEY; | |
| if (!lifiApiKey) { | |
| const error = new Error('VITE_LIFI_API_KEY environment variable is not set'); | |
| console.error(error); | |
| logExchangeEvent( | |
| 'Missing LiFi API key', | |
| 'error', | |
| { walletAddress }, | |
| { component: 'App', action: 'missing_env_var' } | |
| ); | |
| throw error; | |
| } | |
| createConfig({ | |
| integrator: 'PillarX', | |
| providers: [ | |
| // ... provider config | |
| ], | |
| apiKey: lifiApiKey, | |
| }); |
🤖 Prompt for AI Agents
In src/apps/gas-tank/index.tsx around line 202, the LiFi API key is read
directly from import.meta.env.VITE_LIFI_API_KEY without validation; add a check
that the variable is defined and non-empty at startup and throw or log a clear
error (and halt initialization) if missing, so the app fails fast and provides a
helpful message; update initialization to read the env value into a const,
validate it, and only pass it into the apiKey field when present.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (7)
src/apps/gas-tank/hooks/useOffer.tsx (7)
73-94: Instrument with Sentry breadcrumbs on LiFi route operationsPer prior review guidance, add Sentry logging. Record both the route request and any errors for
getNativeFeeForERC20to aid debugging.Apply this diff:
const feeRouteRequest: RoutesRequest = { fromChainId: chainId, @@ }; + addExchangeBreadcrumb('gas-tank:getNativeFeeForERC20:request', { + chainId, + tokenAddress, + feeAmount, + slippage, + }); const result = await getRoutes(feeRouteRequest); @@ - console.error('Failed to get native fee estimation via LiFi:', e); + addExchangeBreadcrumb('gas-tank:getNativeFeeForERC20:error', { + chainId, + tokenAddress, + feeAmount, + error: String(e), + }); + console.error('Failed to get native fee estimation via LiFi:', e);Also applies to: 115-117
199-205: Improve error telemetry in getBestOffer with Sentry breadcrumbSurface this failure with Sentry to correlate user sessions and parameters.
Apply this diff:
- console.error( - 'Sorry, an error occurred while trying to fetch the best swap offer. Please try again.', - e - ); + addExchangeBreadcrumb('gas-tank:getBestOffer:error', { + fromTokenAddress, + fromChainId, + fromTokenDecimals, + slippage, + error: String(e), + }); + console.error( + 'Sorry, an error occurred while trying to fetch the best swap offer. Please try again.', + e + );
485-487: Also capture step transaction failures with SentryAdd a breadcrumb with route metadata and the failing step for better triage.
Apply this diff:
- } catch (error) { - console.error('Failed to get step transactions:', error); + } catch (error) { + addExchangeBreadcrumb('gas-tank:getStepTransactions:error', { + routeFromChainId: route.fromChainId, + routeToChainId: route.toChainId, + firstStepType: route.steps?.[0]?.type, + error: String(error), + }); + console.error('Failed to get step transactions:', error); throw error; // Re-throw so the UI can handle it }
277-281: Incorrect hardcoded decimals for fromAmount conversionThis assumes 6 decimals (USDC) for
fromAmount, butroute.fromTokencan be any token. Use the actual token decimals.Apply this diff:
- const fromAmountBigInt = parseUnits( - String(fromAmount), - 6 // Assuming USDC has 6 decimals - ); + const fromAmountBigInt = parseUnits( + String(fromAmount), + route.fromToken.decimals ?? 18 + );
308-315: Reinstate native balance check and replace console.log with transactionDebugLog
- The balance check is commented out; we should validate sufficient native balance when the swap spends native.
- Replace console logging with the standardized
transactionDebugLog.- Compare base units consistently (native uses 18 decimals; route amounts are already base units).
Apply this diff:
- console.log('totalNativeRequired', fromAmountBigInt); - // // Calculate total required - // const totalNativeRequired = fromAmountBigInt; - // if (userNativeBalance < totalNativeRequired) { - // throw new Error( - // 'Insufficient native token balance to cover swap and fee.' - // ); - // } + // For native token swaps, verify sufficient balance + transactionDebugLog('gas-tank:getStepTransactions:native-balance-check', { + fromChainId: fromTokenChainId, + routeFromAmount: route.fromAmount, + userNativeBalance: userNativeBalance.toString(), + }); + if (isNativeToken(route.fromToken.address)) { + const totalNativeRequired = BigInt(route.fromAmount); + if (userNativeBalance < totalNativeRequired) { + throw new Error('Insufficient native token balance to cover swap amount.'); + } + }And extend the existing Sentry utils import to include the logger:
-import { - addExchangeBreadcrumb, - startExchangeTransaction, -} from '../utils/sentry'; +import { + addExchangeBreadcrumb, + startExchangeTransaction, + transactionDebugLog, +} from '../utils/sentry';
360-369: Validate approval address before allowance check; avoid passing empty spenderCurrently
spender: step.estimate.approvalAddress || ''can degrade into an empty string, skipping allowance checks and later causing approval to fail. Validate first, then callisAllowanceSetwith a proper address.Apply this diff:
- const isAllowance = isTokenNative - ? undefined // Native tokens never require approval - : // eslint-disable-next-line no-await-in-loop - await isAllowanceSet({ - owner: fromAccount, - spender: step.estimate.approvalAddress || '', - tokenAddress: step.action.fromToken.address, - chainId: step.action.fromChainId, - }); + if (!isTokenNative && !step.estimate.approvalAddress) { + throw new Error('Approval address is undefined for non-native token'); + } + const isAllowance = isTokenNative + ? undefined // Native tokens never require approval + : // eslint-disable-next-line no-await-in-loop + await isAllowanceSet({ + owner: fromAccount, + spender: step.estimate.approvalAddress as `0x${string}`, + tokenAddress: step.action.fromToken.address, + chainId: step.action.fromChainId, + });
370-376: Compare allowance as BigInt, not formatted stringsString comparisons after
formatUnitscan be wrong. Compare base units directly.Apply this diff:
- const isEnoughAllowance = isAllowance - ? formatUnits(isAllowance, step.action.fromToken.decimals) >= - formatUnits( - BigInt(step.action.fromAmount), - step.action.fromToken.decimals - ) - : undefined; + const isEnoughAllowance = isAllowance + ? isAllowance >= BigInt(step.action.fromAmount) + : undefined;
🧹 Nitpick comments (8)
src/apps/gas-tank/hooks/useOffer.tsx (8)
45-50: USDC mapping: add guards and centralize; risk of undefined toTokenAddress on unsupported chainsIf
fromChainIdisn’t inUSDC_ADDRESSES,toTokenAddressbecomesundefined, which will break the typedRoutesRequestand/or cause runtime errors. Also, product asked to “add all that PillarX supports even if the service doesn't,” so this table should live in a shared constants module and be exhaustive.Action:
- Add a runtime guard in getBestOffer (see diff below in Lines 151-153).
- Consider moving this mapping to a shared constants file used by both Gas Tank and the-exchange.
- Add all PillarX-supported chains and verify addresses/decimals.
Do you want me to open a follow-up PR to centralize stablecoin addresses and add a validation test?
236-240: Avoid creating a new viem client per allowance call; memoize per chainCreating a
publicClienton every allowance check is wasteful. Memoize bychainIdor reuse an existing shared client.Suggestion:
- Extract a
getPublicClient(chainId)helper in a shared module that caches clients.- Replace this call site with the cached client.
52-118: Dead code: getNativeFeeForERC20 is unusedThe helper is not returned from the hook and is unused in this file. Keep the codebase lean: either export and use it where needed, or remove it to avoid drift.
If it’s required for the Gas Tank fee path later, I can wire it into getStepTransactions similarly to the-exchange implementation.
41-44: Imported Sentry helpers aren’t used yetYou import
startExchangeTransactionbut never call it. Either use it (e.g., mark the beginning of transaction orchestration) or remove the import.Do you intend to start a Sentry transaction around
getStepTransactions? If yes, I can provide a minimal wrapper.
151-170: Bridge/exchange allowlists are very restrictive; confirm product intentYou’re allowing only
relaybridge andopenocean,kyberswapexchanges. This may reduce route availability and success rates on some chains. Confirm this constraint with product, or consider making it configurable.
188-194: Offer selection logic is reasonableUsing
reduceonroutesandprocessEthwith the target token decimals is fine. Consider returning the index alongside the offer for easier diagnostics, but this is optional.
492-496: Export surface: consider exposing getNativeFeeForERC20 or removing itRight now the hook only returns
getBestOfferandgetStepTransactions. If the native-fee estimation will be needed by the UI/TopUpModal, export it now; otherwise remove it.
151-153: Unit tests: add coverage for unsupported-chain guard and decimals handlingOnce you add the guard and decimals fix, add tests to ensure:
- getBestOffer returns
undefinedwhenUSDC_ADDRESSES[fromChainId]is missing.- getStepTransactions uses
route.fromToken.decimalswhen calculatingfromAmountBigInt.I can scaffold a small viem-mocked test to validate these behaviors if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/apps/gas-tank/hooks/useOffer.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/gas-tank/hooks/useOffer.tsx
🧬 Code graph analysis (1)
src/apps/gas-tank/hooks/useOffer.tsx (5)
src/apps/gas-tank/utils/blockchain.ts (3)
processEth(18-24)getNativeBalanceFromPortfolio(129-141)toWei(124-126)src/apps/gas-tank/utils/types.tsx (3)
SwapType(10-18)SwapOffer(20-23)StepTransaction(30-39)src/apps/gas-tank/utils/wrappedTokens.ts (3)
getWrappedTokenAddressIfNative(33-44)isWrappedToken(23-28)isNativeToken(30-31)src/apps/deposit/utils/blockchain.tsx (1)
getNetworkViem(60-79)src/services/tokensData.ts (1)
Token(19-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (4)
src/apps/gas-tank/hooks/useOffer.tsx (4)
208-254: Allowance check: return type and zero-address early exit are goodThe early exit for zero address and returning
undefinedto signal “no approval required” reads cleanly. Once spender validation is added (see above), this helper looks solid.
120-206: Fee deduction via BigInt is correct and consistentConverting to base units and applying integer math for the 1% fee is a good approach and avoids float precision issues.
298-306: Balance retrieval and parsing looks fineUsing
getNativeBalanceFromPortfolioandtoWeiwith 18 decimals for native tokens is consistent with your utils.
454-483: Transaction request normalization to BigInt is carefulThe guard for missing
toand normalization ofvalue,gasLimit, andgasPricetobigintis solid.
src/apps/gas-tank/hooks/useOffer.tsx
Outdated
| const fromAmountInWei = parseUnits(String(fromAmount), fromTokenDecimals); | ||
| const feeDeduction = fromAmountInWei / BigInt(100); // 1% fee | ||
| const fromAmountFeeDeducted = fromAmountInWei - feeDeduction; | ||
| const toTokenAddress = USDC_ADDRESSES[fromChainId]; | ||
| const toTokenDecimals = 6; // USDC has 6 decimals | ||
| /** |
There was a problem hiding this comment.
Guard missing USDC on chain and avoid hardcoding decimals
Two issues:
- If
USDC_ADDRESSES[fromChainId]is undefined, the request is invalid. toTokenDecimalsis hardcoded to 6. USDC is indeed 6 on major chains, but it’s safer to derive decimals from token metadata (if available) or encode the mapping alongside the address.
Apply this diff to add a guard and keep 6 as a documented fallback:
- const toTokenAddress = USDC_ADDRESSES[fromChainId];
- const toTokenDecimals = 6; // USDC has 6 decimals
+ const toTokenAddress = USDC_ADDRESSES[fromChainId];
+ if (!toTokenAddress) {
+ console.error(`USDC address not configured for chain ${fromChainId}`);
+ return undefined;
+ }
+ // USDC typically has 6 decimals; keep as fallback, prefer metadata when available
+ const toTokenDecimals = 6;📝 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 fromAmountInWei = parseUnits(String(fromAmount), fromTokenDecimals); | |
| const feeDeduction = fromAmountInWei / BigInt(100); // 1% fee | |
| const fromAmountFeeDeducted = fromAmountInWei - feeDeduction; | |
| const toTokenAddress = USDC_ADDRESSES[fromChainId]; | |
| const toTokenDecimals = 6; // USDC has 6 decimals | |
| /** | |
| const fromAmountInWei = parseUnits(String(fromAmount), fromTokenDecimals); | |
| const feeDeduction = fromAmountInWei / BigInt(100); // 1% fee | |
| const fromAmountFeeDeducted = fromAmountInWei - feeDeduction; | |
| const toTokenAddress = USDC_ADDRESSES[fromChainId]; | |
| if (!toTokenAddress) { | |
| console.error(`USDC address not configured for chain ${fromChainId}`); | |
| return undefined; | |
| } | |
| // USDC typically has 6 decimals; keep as fallback, prefer metadata when available | |
| const toTokenDecimals = 6; | |
| /** |
🤖 Prompt for AI Agents
In src/apps/gas-tank/hooks/useOffer.tsx around lines 148 to 153, the code
assumes USDC_ADDRESSES[fromChainId] exists and hardcodes toTokenDecimals=6; add
a guard that throws/returns an explicit error when USDC_ADDRESSES[fromChainId]
is undefined to prevent invalid requests, and replace the hardcoded decimals
with a lookup that tries to read token metadata (e.g., a TOKEN_DECIMALS map or
token contract call) falling back to 6 with an inline comment documenting the
fallback.
| const isWrapRequired = isWrappedToken( | ||
| route.fromToken.address, | ||
| route.fromToken.chainId | ||
| ); | ||
|
|
||
| // Convert fromAmount (number) to BigInt using token decimals | ||
| const fromAmountBigInt = parseUnits( | ||
| String(fromAmount), | ||
| 6 // Assuming USDC has 6 decimals | ||
| ); | ||
|
|
||
| /** | ||
| * Step 2: Fee calculation and validation | ||
| * Calculate 1% platform fee and validate fee receiver address | ||
| */ | ||
| const feeReceiver = import.meta.env.VITE_SWAP_FEE_RECEIVER; | ||
|
|
||
| // Validate fee receiver address | ||
| if (!feeReceiver) { | ||
| throw new Error('Fee receiver address is not configured'); | ||
| } | ||
|
|
||
| /** | ||
| * Step 3: Balance checks | ||
| * Verify user has sufficient balance for swap and fees | ||
| */ | ||
| let userNativeBalance = BigInt(0); | ||
| try { | ||
| // Get native balance from portfolio | ||
| const nativeBalance = | ||
| getNativeBalanceFromPortfolio(userPortfolio, fromTokenChainId) || '0'; | ||
| userNativeBalance = toWei(nativeBalance, 18); | ||
| } catch (e) { | ||
| throw new Error('Unable to fetch balances for swap.'); | ||
| } | ||
|
|
||
| console.log('totalNativeRequired', fromAmountBigInt); | ||
| // // Calculate total required | ||
| // const totalNativeRequired = fromAmountBigInt; | ||
| // if (userNativeBalance < totalNativeRequired) { | ||
| // throw new Error( | ||
| // 'Insufficient native token balance to cover swap and fee.' | ||
| // ); | ||
| // } | ||
|
|
||
| /** | ||
| * Step 4: Wrap transaction (if required) | ||
| * If the route requires wrapped tokens but user has native tokens, | ||
| * add a wrapping transaction | ||
| */ | ||
| if (isWrapRequired) { | ||
| const wrapCalldata = encodeFunctionData({ | ||
| abi: [ | ||
| { | ||
| name: 'deposit', | ||
| type: 'function', | ||
| stateMutability: 'payable', | ||
| inputs: [], | ||
| outputs: [], | ||
| }, | ||
| ], | ||
| functionName: 'deposit', | ||
| }); | ||
|
|
||
| stepTransactions.push({ | ||
| to: route.fromToken.address, // wrapped token address | ||
| data: wrapCalldata, | ||
| value: BigInt(route.fromAmount), | ||
| chainId: route.fromChainId, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Wrapping logic is inverted and risks double-wrapping; rely on LiFi steps instead
isWrapRequired is set to isWrappedToken(route.fromToken.address, ...) and then a manual deposit() is added if true. If route.fromToken is already the wrapped token (e.g., WETH), you typically do NOT need to wrap again; LiFi steps also include wrap/unwrapped actions when needed. This can lead to duplicate wrapping or incorrect behavior.
Recommendation: remove the manual wrap step and rely on the LiFi-provided steps (from getStepTransaction) which already account for wrapping.
Apply this diff to remove the manual wrap block:
- const isWrapRequired = isWrappedToken(
- route.fromToken.address,
- route.fromToken.chainId
- );
- // Convert fromAmount (number) to BigInt using token decimals
+ // Convert fromAmount (number) to BigInt using token decimals
const fromAmountBigInt = parseUnits(
String(fromAmount),
- 6 // Assuming USDC has 6 decimals
+ 6 // replaced below; see next comment for correct decimals
);
- /**
- * Step 4: Wrap transaction (if required)
- * If the route requires wrapped tokens but user has native tokens,
- * add a wrapping transaction
- */
- if (isWrapRequired) {
- const wrapCalldata = encodeFunctionData({
- abi: [
- {
- name: 'deposit',
- type: 'function',
- stateMutability: 'payable',
- inputs: [],
- outputs: [],
- },
- ],
- functionName: 'deposit',
- });
-
- stepTransactions.push({
- to: route.fromToken.address, // wrapped token address
- data: wrapCalldata,
- value: BigInt(route.fromAmount),
- chainId: route.fromChainId,
- });
- }📝 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 isWrapRequired = isWrappedToken( | |
| route.fromToken.address, | |
| route.fromToken.chainId | |
| ); | |
| // Convert fromAmount (number) to BigInt using token decimals | |
| const fromAmountBigInt = parseUnits( | |
| String(fromAmount), | |
| 6 // Assuming USDC has 6 decimals | |
| ); | |
| /** | |
| * Step 2: Fee calculation and validation | |
| * Calculate 1% platform fee and validate fee receiver address | |
| */ | |
| const feeReceiver = import.meta.env.VITE_SWAP_FEE_RECEIVER; | |
| // Validate fee receiver address | |
| if (!feeReceiver) { | |
| throw new Error('Fee receiver address is not configured'); | |
| } | |
| /** | |
| * Step 3: Balance checks | |
| * Verify user has sufficient balance for swap and fees | |
| */ | |
| let userNativeBalance = BigInt(0); | |
| try { | |
| // Get native balance from portfolio | |
| const nativeBalance = | |
| getNativeBalanceFromPortfolio(userPortfolio, fromTokenChainId) || '0'; | |
| userNativeBalance = toWei(nativeBalance, 18); | |
| } catch (e) { | |
| throw new Error('Unable to fetch balances for swap.'); | |
| } | |
| console.log('totalNativeRequired', fromAmountBigInt); | |
| // // Calculate total required | |
| // const totalNativeRequired = fromAmountBigInt; | |
| // if (userNativeBalance < totalNativeRequired) { | |
| // throw new Error( | |
| // 'Insufficient native token balance to cover swap and fee.' | |
| // ); | |
| // } | |
| /** | |
| * Step 4: Wrap transaction (if required) | |
| * If the route requires wrapped tokens but user has native tokens, | |
| * add a wrapping transaction | |
| */ | |
| if (isWrapRequired) { | |
| const wrapCalldata = encodeFunctionData({ | |
| abi: [ | |
| { | |
| name: 'deposit', | |
| type: 'function', | |
| stateMutability: 'payable', | |
| inputs: [], | |
| outputs: [], | |
| }, | |
| ], | |
| functionName: 'deposit', | |
| }); | |
| stepTransactions.push({ | |
| to: route.fromToken.address, // wrapped token address | |
| data: wrapCalldata, | |
| value: BigInt(route.fromAmount), | |
| chainId: route.fromChainId, | |
| }); | |
| } | |
| // Convert fromAmount (number) to BigInt using token decimals | |
| const fromAmountBigInt = parseUnits( | |
| String(fromAmount), | |
| 6 // replaced below; see next comment for correct decimals | |
| ); | |
| /** | |
| * Step 2: Fee calculation and validation | |
| * Calculate 1% platform fee and validate fee receiver address | |
| */ | |
| const feeReceiver = import.meta.env.VITE_SWAP_FEE_RECEIVER; |
github-actions
bot
temporarily deployed
to
Preview (PRO-3185-Gas-Tank-Paymaster)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/providers/AllowedAppsProvider.tsx (1)
56-62: Gate Gas Tank behind testnet/flag and de-duplicate IDs (avoid mutating arrays).Blindly pushing "gas-tank" can bypass server allow-listing and may introduce duplicates if the API already returns it. Prefer a set-based merge and only force-enable on testnets (or behind a feature flag), while keeping immutability.
Apply this diff:
- const allowedAppIds = - data?.map((app: ApiAllowedApp) => app.appId) || []; - - // Add gas-tank app for development - allowedAppIds.push('gas-tank'); - - setAllowed(allowedAppIds); + const apiIds = data.map((app: ApiAllowedApp) => app.appId); + // Only force-enable Gas Tank on testnets; de-dupe to avoid duplicates if API already allows it. + const nextAllowed = Array.from( + new Set([...apiIds, ...(isTestnet ? ['gas-tank'] : [])]), + ); + setAllowed(nextAllowed);Optional (if you need to toggle on mainnets or staging as well), you can replace the testnet check with a feature flag. Use the convention your bundler supports:
const shouldForceEnableGasTank = isTestnet || process.env.NEXT_PUBLIC_ENABLE_GAS_TANK === 'true' || process.env.REACT_APP_ENABLE_GAS_TANK === 'true' || (typeof import.meta !== 'undefined' && import.meta.env?.VITE_ENABLE_GAS_TANK === 'true'); const nextAllowed = Array.from( new Set([...apiIds, ...(shouldForceEnableGasTank ? ['gas-tank'] : [])]), );Follow-up: Do you want Gas Tank to remain visible when the API returns an empty list or fails? If yes, we should also adjust the early-return branch (Lines 52–55) to still compute a fallback allow-list; I can provide a targeted patch.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/providers/AllowedAppsProvider.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint
- GitHub Check: unit-tests
- GitHub Check: build
github-actions
bot
temporarily deployed
to
Preview (PRO-3185-Gas-Tank-Paymaster)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
src/apps/gas-tank/components/TopUpModal.tsx (2)
83-87: Add Sentry logging where failures occur (repeat ask)Multiple error branches only console.*; add Sentry to capture failures for observability.
+import { logExchangeEvent } from '../utils/sentry'; @@ if (!isWalletPortfolioDataSuccess || walletPortfolioDataError) { if (walletPortfolioDataError) { console.error(walletPortfolioDataError); + logExchangeEvent('Wallet portfolio fetch failed', 'error', { walletAddress, walletPortfolioDataError }, { component: 'TopUpModal', action: 'portfolio_fetch_error' }); } @@ } catch (swapError) { console.error('Error getting swap transactions:', swapError); + logExchangeEvent('Swap route fetch failed', 'error', { swapError, amount, token: selectedToken?.symbol }, { component: 'TopUpModal', action: 'swap_route_error' }); @@ if (!response.ok) { const errorText = await response.text(); console.error('Error fetching transaction data:', errorText); + logExchangeEvent('Paymaster transaction fetch failed', 'error', { status: response.status, errorText }, { component: 'TopUpModal', action: 'paymaster_fetch_error' }); @@ console.log('Transaction data:', transactionData); + logExchangeEvent('Paymaster transaction data received', 'info', { count: Array.isArray(transactionData?.result) ? transactionData.result.length : 1 }, { component: 'TopUpModal', action: 'paymaster_fetch_success' }); @@ } catch (error) { console.error('Error processing top-up:', error); console.warn('Failed to process top-up. Please try again.'); + logExchangeEvent('Top-up failed', 'error', { error }, { component: 'TopUpModal', action: 'topup_failed' });Also applies to: 165-171, 186-192, 195-195, 255-258
291-335: Token list loading check won’t trigger; empty arrays are truthy (repeat)if (!portfolioTokens) never shows loader. Use query flags or explicit length check.
Apply this diff:
- if (!portfolioTokens) { + if (!isWalletPortfolioDataSuccess && !walletPortfolioDataError) { return ( <LoadingContainer> <CircularProgress size={24} /> <span>Loading wallet tokens...</span> </LoadingContainer> ); }Optionally render an empty state when portfolioTokens.length === 0 and success.
#!/bin/bash # Check RTK Query flags available from the wallet portfolio hook rg -n "useGetWalletPortfolioQuery" src | head -n 20src/apps/gas-tank/index.tsx (1)
62-66: Fail fast if VITE_LIFI_API_KEY is missing (repeat)LiFi config silently proceeds with an undefined API key. Validate it and log with Sentry to aid ops.
- createConfig({ + const lifiApiKey = import.meta.env.VITE_LIFI_API_KEY; + if (!lifiApiKey) { + const error = new Error('VITE_LIFI_API_KEY environment variable is not set'); + console.error(error); + logExchangeEvent( + 'Missing LiFi API key', + 'error', + { walletAddress }, + { component: 'App', action: 'missing_env_var' } + ); + throw error; + } + createConfig({ integrator: 'PillarX', @@ - apiKey: import.meta.env.VITE_LIFI_API_KEY, + apiKey: lifiApiKey, });Also applies to: 200-203
🧹 Nitpick comments (6)
src/apps/gas-tank/components/TopUpModal.tsx (5)
175-184: Encode query params and guard chainId computationUse encodeURIComponent for amount; compute chainId once. Helps avoid malformed URLs and repeated mapping calls.
- const response = await fetch( - `${paymasterUrl}/getTransactionForDeposit?chainId=${chainNameToChainIdTokensData(selectedToken.blockchain)}&amount=${receiveSwapAmount}`, + const chainId = chainNameToChainIdTokensData(selectedToken.blockchain); + const response = await fetch( + `${paymasterUrl}/getTransactionForDeposit?chainId=${chainId}&amount=${encodeURIComponent(receiveSwapAmount)}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, } );
175-192: Networking ergonomics
- Add Accept: application/json header.
- Surface response.status in the error message (and Sentry).
- Consider retry/backoff or specific handling for 429/5xx.
const response = await fetch( @@ { method: 'GET', headers: { 'Content-Type': 'application/json', + 'Accept': 'application/json', }, } ); @@ - if (!response.ok) { + if (!response.ok) { const errorText = await response.text(); console.error('Error fetching transaction data:', errorText); - setErrorMsg('Failed to fetch transaction data. Please try again with different token or amount.'); + setErrorMsg(`Failed to fetch transaction data (HTTP ${response.status}). Please try again with a different token or amount.`);
59-62: Ensure selector points to correct slice or remove it if unusedThe
useAppSelectorin TopUpModal.tsx is currently targetingstate.swap.walletPortfolio, but the slice is named “gasTank” and exportswalletPortfoliofromgasTankSlice(see lines 7–8 in gasTankSlice.ts). Moreover, thewalletPortfoliovariable defined on line 59 of TopUpModal.tsx isn’t referenced elsewhere in this component.• If you need to read the persisted portfolio from Redux, change the selector to:
- const walletPortfolio = useAppSelector( - (state) => state.swap.walletPortfolio as PortfolioData | undefined - ); + const walletPortfolio = useAppSelector( + (state) => state.gasTank.walletPortfolio as PortfolioData | undefined + );• If you’re not using Redux state here (you already track portfolio via local
portfolioTokens), remove lines 59–62 entirely to eliminate dead code.
37-42: Import the sharedUSDC_ADDRESSESfromuseOfferand remove the local duplicateThe
USDC_ADDRESSESmapping is already exported fromsrc/apps/gas-tank/hooks/useOffer.tsx. Defining it again inTopUpModal.tsxintroduces unnecessary duplication and the risk of address drift (e.g. forgetting to add a new chain in one of the copies).• File:
src/apps/gas-tank/components/TopUpModal.tsx
– Remove the localconst USDC_ADDRESSESblock (lines 37–42).
– Update the import at the top of the file to pull in the shared constant.Apply this diff:
--- a/src/apps/gas-tank/components/TopUpModal.tsx +++ b/src/apps/gas-tank/components/TopUpModal.tsx @@ - import useOffer from '../hooks/useOffer'; + import useOffer, { USDC_ADDRESSES } from '../hooks/useOffer'; @@ - const USDC_ADDRESSES: { [chainId: number]: string } = { - 137: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359', // Polygon - 42161: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831', // Arbitrum - 10: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85', // Optimism - // Add more chains as needed - };With this change, all references to
USDC_ADDRESSESinTopUpModal.tsxwill correctly use the single source of truth fromuseOffer.tsx, eliminating duplication.
102-106: Chain-name mapping is fragile—consider storing chainId in PortfolioToken
- In
src/services/tokensData.ts,convertPortfolioAPIResponseToTokenonly sets theblockchain(name) field, not the numericchainId, so downstream code must re-derive it by string lookup.- Relying on
chainNameToChainIdTokensData(selectedToken.blockchain)is brittle (exact name matches only) and can mask supported chains when names differ (e.g. “Optimistic” vs. “Optimism”).- Optional refactor: extend
PortfolioTokento include achainIdproperty (populated from the rawcontract.chainId) and use that directly in all consumers.- At minimum, prevent double lookup and guard against missing mappings in
TopUpModal.tsx:- if (USDC_ADDRESSES[chainNameToChainIdTokensData(selectedToken.blockchain)] === undefined) { + const chainId = chainNameToChainIdTokensData(selectedToken.blockchain); + if (!chainId || USDC_ADDRESSES[chainId] === undefined) { setErrorMsg("Gas Tank is not supported on the selected token's chain."); return; }src/apps/gas-tank/index.tsx (1)
116-153: Add warning when provider lacks EIP-1193 request supportIf providerWithRequest.request is missing, switchChain is a no-op; add explicit telemetry.
- if (providerWithRequest.request) { + if (providerWithRequest.request) { try { /** * Attempt to switch to the target chain @@ } - } + } else { + logExchangeEvent( + 'Provider does not support EIP-1193 request', + 'warning', + { walletAddress, chainId }, + { component: 'App', action: 'chain_switch_unsupported' } + ); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)src/apps/gas-tank/index.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
PR: pillarwallet/x#374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/gas-tank/index.tsx
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
🧬 Code graph analysis (2)
src/apps/gas-tank/index.tsx (2)
src/apps/gas-tank/utils/sentry.ts (3)
initSentryForGasTank(5-8)logExchangeEvent(21-46)addExchangeBreadcrumb(226-244)src/utils/blockchain.ts (1)
supportedChains(139-148)
src/apps/gas-tank/components/TopUpModal.tsx (6)
src/apps/gas-tank/hooks/useOffer.tsx (1)
USDC_ADDRESSES(45-50)src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/services/tokensData.ts (2)
PortfolioToken(31-34)chainNameToChainIdTokensData(234-255)src/types/api.ts (1)
PortfolioData(731-740)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/converters.ts (1)
formatTokenAmount(6-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (1)
src/apps/gas-tank/index.tsx (1)
74-170: Chain switching flow is robust and well-instrumentedValidation against supportedChains, add-then-switch (4902), success/error logging, and returning the configured provider are all correct.
| const bestOffer = await getBestOffer({ | ||
| fromTokenAddress: selectedToken.contract, | ||
| fromAmount: Number(amount), | ||
| fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain), | ||
| fromTokenDecimals: selectedToken.decimals, | ||
| slippage: 0.03, | ||
| }); | ||
| if (!bestOffer) { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Avoid Number(amount) for token math; eliminate floating-point precision loss
Using Number with token amounts (especially 18 decimals) is unsafe. Prefer strings + decimals or integer smallest units.
- Change getBestOffer signature to accept amount as string and decimals, or pass smallest units.
Example:
- const bestOffer = await getBestOffer({
- fromTokenAddress: selectedToken.contract,
- fromAmount: Number(amount),
- fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain),
- fromTokenDecimals: selectedToken.decimals,
- slippage: 0.03,
- });
+ const bestOffer = await getBestOffer({
+ fromTokenAddress: selectedToken.contract,
+ fromAmount: amount, // string
+ fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain),
+ fromTokenDecimals: selectedToken.decimals,
+ slippage: 0.03,
+ });🏁 Script executed:
#!/bin/bash
# Inspect getBestOffer signature types to confirm accepted types for fromAmount
rg -n -C4 "getBestOffer\s*\(" src/apps/gas-tank/hooks/useOffer.tsxLength of output: 68
🏁 Script executed:
#!/bin/bash
# Locate getBestOffer definitions and inspect its signature across the codebase
rg -n "getBestOffer" -C5 --type ts --type tsx .Length of output: 75
🏁 Script executed:
#!/bin/bash
# Find the import of getBestOffer in TopUpModal.tsx
rg -n "getBestOffer" -C3 src/apps/gas-tank/components/TopUpModal.tsx
# Show TopUpModal.tsx import section for context
sed -n '1,50p' src/apps/gas-tank/components/TopUpModal.tsxLength of output: 2657
🏁 Script executed:
#!/bin/bash
# List hook files in the gas-tank/hooks directory
echo "Listing hook files:"
ls src/apps/gas-tank/hooks
# Locate and display the beginning of the useOffer file(s)
for f in src/apps/gas-tank/hooks/useOffer.*; do
echo
echo "===== Inspecting $f ====="
sed -n '1,200p' "$f"
doneLength of output: 6228
🏁 Script executed:
#!/bin/bash
# List files in the utils directory to locate the types file
ls src/apps/gas-tank/utils
# Display the beginning of the types file to inspect SwapType definition
sed -n '1,200p' src/apps/gas-tank/utils/types.tsLength of output: 218
🏁 Script executed:
#!/bin/bash
# Locate SwapType definition in types.tsx
grep -R "SwapType" -n src/apps/gas-tank/utils/types.tsx || true
# Display the contents of types.tsx around SwapType
sed -n '1,200p' src/apps/gas-tank/utils/types.tsxLength of output: 990
Mandate refactor: switch fromAmount to string to eliminate floating-point precision loss
The getBestOffer hook currently defines its SwapType.fromAmount as a number, and callers (e.g. in TopUpModal.tsx) invoke Number(amount) to match that signature. However, JavaScript number cannot reliably represent token amounts with 18 decimals—this risks subtle rounding errors. Internally, getBestOffer already uses parseUnits(String(fromAmount), fromTokenDecimals) to convert the value to wei, so it can and should accept the raw string input directly.
Locations to update:
- src/apps/gas-tank/utils/types.tsx
ChangeSwapType.fromAmountfromnumbertostring. - src/apps/gas-tank/hooks/useOffer.tsx
Update thegetBestOffersignature to acceptfromAmount: stringand remove the unnecessaryString(fromAmount)wrapper whereparseUnitsis called. - src/apps/gas-tank/components/TopUpModal.tsx
Pass theamountstring directly (noNumber(...)) intogetBestOffer.
Suggested diff snippets:
--- a/src/apps/gas-tank/utils/types.tsx
+++ b/src/apps/gas-tank/utils/types.tsx
@@
-export type SwapType = {
- fromAmount: number;
+export type SwapType = {
+ fromAmount: string;
fromTokenAddress: string;
fromChainId: number;
fromTokenDecimals: number;
slippage?: number;--- a/src/apps/gas-tank/hooks/useOffer.tsx
+++ b/src/apps/gas-tank/hooks/useOffer.tsx
@@
-const getBestOffer = async ({
- fromAmount,
+const getBestOffer = async ({
+ fromAmount, // now a string
fromTokenAddress,
fromChainId,
fromTokenDecimals,
slippage = 0.03,
}: SwapType): Promise<SwapOffer | undefined> => {
// ...
- const fromAmountInWei = parseUnits(String(fromAmount), fromTokenDecimals);
+ const fromAmountInWei = parseUnits(fromAmount, fromTokenDecimals);
const feeDeduction = fromAmountInWei / BigInt(100);
// ...--- a/src/apps/gas-tank/components/TopUpModal.tsx
+++ b/src/apps/gas-tank/components/TopUpModal.tsx
@@
- const bestOffer = await getBestOffer({
- fromTokenAddress: selectedToken.contract,
- fromAmount: Number(amount),
+ const bestOffer = await getBestOffer({
+ fromTokenAddress: selectedToken.contract,
+ fromAmount: amount, // pass string directly
fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain),
fromTokenDecimals: selectedToken.decimals,
slippage: 0.03,This refactor ensures full precision throughout the swap workflow and aligns types with actual parsing logic.
📝 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 bestOffer = await getBestOffer({ | |
| fromTokenAddress: selectedToken.contract, | |
| fromAmount: Number(amount), | |
| fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain), | |
| fromTokenDecimals: selectedToken.decimals, | |
| slippage: 0.03, | |
| }); | |
| if (!bestOffer) { | |
| const bestOffer = await getBestOffer({ | |
| fromTokenAddress: selectedToken.contract, | |
| fromAmount: amount, // pass string directly | |
| fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain), | |
| fromTokenDecimals: selectedToken.decimals, | |
| slippage: 0.03, | |
| }); |
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/TopUpModal.tsx (lines 118-125) and related
files, change the SwapType.fromAmount type to string in
src/apps/gas-tank/utils/types.tsx, update getBestOffer signature in
src/apps/gas-tank/hooks/useOffer.tsx to accept fromAmount: string and remove any
String(fromAmount) wrappers where parseUnits is called, and in TopUpModal.tsx
pass the amount string directly (remove Number(amount)) when calling
getBestOffer so the flow preserves full precision.
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
235-272: Only add “Gas Tank Paymaster” when balance > 0; avoid mutating the shared options array across async callsPushing into
feeTypeOptionsunconditionally and then conditionally callingsetFeeTypecan surface “Gas Tank Paymaster” even when the balance is 0/null due to promise race and shared array mutation. Compose a fresh array and callsetFeeTypeonce.Apply:
- setFetchingBalances(true); - getGasTankBalance(accountAddress ?? '').then((res) => { - feeTypeOptions.push({ - id: 'Gas Tank Paymaster', - title: 'Gas Tank Paymaster', - type: 'token', - value: '', - }); - if (res) { - setGasTankBalance(res); - if (res > 0) { - setFeeType(feeTypeOptions); - setIsPaymaster(true); - setPaymasterContext({ - mode: 'gasTankPaymaster', - }); - setSelectedFeeType('Gas Tank Paymaster'); - setFetchingBalances(false); - } else { - setIsPaymaster(false); - setPaymasterContext(null); - setSelectedFeeType('Native Token'); - setDefaultSelectedFeeTypeId('Native Token'); - setFetchingBalances(false); - } - } else { - setIsPaymaster(false); - setFetchingBalances(false); - setPaymasterContext(null); - setSelectedFeeType('Native Token'); - setDefaultSelectedFeeTypeId('Native Token'); - } - }); + setFetchingBalances(true); + if (!accountAddress) { + setFetchingBalances(false); + return; + } + getGasTankBalance(accountAddress).then((res) => { + const nextFeeType = [...feeTypeOptions]; + if (res !== null && res > 0) { + nextFeeType.push({ + id: 'Gas Tank Paymaster', + title: 'Gas Tank Paymaster', + type: 'token', + value: '', + }); + setGasTankBalance(res); + setIsPaymaster(true); + setPaymasterContext({ mode: 'gasTankPaymaster' }); + setSelectedFeeType('Gas Tank Paymaster'); + } else { + setIsPaymaster(false); + setPaymasterContext(null); + setSelectedFeeType('Native Token'); + setDefaultSelectedFeeTypeId('Native Token'); + } + setFeeType(nextFeeType); + setFetchingBalances(false); + });
♻️ Duplicate comments (5)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
320-348: Compose “Gasless” once; don’t push then set twiceThis double set causes UI flicker and risks duplication with concurrent mutations. Build a new array and set once.
- if (feeOptions && feeOptions.length > 0 && feeOptions[0]) { - setFeeType(feeTypeOptions); + if (feeOptions && feeOptions.length > 0 && feeOptions[0]) { setFeeAssetOptions(feeOptions); // get Skandha gas price getGasPrice(selectedAsset.chainId).then((res) => { setGasPrice(res); }); setSelectedFeeAsset({ token: feeOptions[0].asset.contract, decimals: feeOptions[0].asset.decimals, tokenPrice: feeOptions[0].asset.price?.toString(), balance: feeOptions[0].value?.toString(), }); - feeTypeOptions.push({ - id: 'Gasless', - title: 'Gasless', - type: 'token', - value: '', - }); + const nextFeeType = [ + ...feeTypeOptions, + { + id: 'Gasless', + title: 'Gasless', + type: 'token', + value: '', + }, + ]; setSelectedPaymasterAddress(feeOptions[0].id.split('-')[2]); if (selectedFeeType === 'Native Token') { setPaymasterContext({ mode: 'commonerc20', token: feeOptions[0].asset.contract, }); setIsPaymaster(true); setDefaultSelectedFeeTypeId('Gasless'); } - setFeeType(feeTypeOptions); + setFeeType(nextFeeType);
1322-1336: Replace console.log with transactionDebugLog (and remove eslint disable reliance)Use the app’s debug logger for consistent observability and to avoid shipping console noise.
- console.log('handleOnChangeFeeAsset', value, selectedFeeType); + transactionDebugLog('handleOnChangeFeeAsset', { value, selectedFeeType });src/apps/gas-tank/hooks/useGasTankBalance.tsx (1)
66-76: Normalize API shape: support object-map and array formatsBackend may return either a map or an array. Normalize before summing. (This echoes a prior suggestion.)
- // Extract chain balances - const balances: ChainBalance[] = Object.values(data.balance || {}); - setChainBalances(balances); - - // Calculate total balance by summing all chain balances - const total = balances.reduce((sum, chainBalance) => { - const balance = parseFloat(chainBalance.balance) || 0; - return sum + balance; - }, 0); + // Normalize { [chainId]: val } or ChainBalance[] into ChainBalance[] + const raw = (data as any)?.balance ?? []; + const balances: ChainBalance[] = Array.isArray(raw) + ? raw.map((v: any) => ({ + chainId: String(v?.chainId ?? ''), + balance: String(v?.balance ?? '0'), + })) + : Object.entries(raw).map(([cid, val]: [string, any]) => + val && typeof val === 'object' && 'balance' in val + ? { chainId: String(val.chainId ?? cid), balance: String(val.balance ?? '0') } + : { chainId: cid, balance: String(val ?? '0') } + ); + setChainBalances(balances); + + const total = balances.reduce((sum, cb) => sum + (parseFloat(cb.balance) || 0), 0);src/apps/gas-tank/components/TopUpModal.tsx (2)
388-393: Disable CTA when invalid amount or missing Paymaster URLKeep UI and handleTopUp validations consistent. (Previously suggested.)
- <TopUpButton - onClick={handleTopUp} - disabled={ - !selectedToken || !amount || isProcessing - } - > + {(() => { + const amt = Number(amount); + const isAmountValid = + !!selectedToken && + Number.isFinite(amt) && + amt > 0 && + amt <= (selectedToken?.balance ?? 0); + return ( + <TopUpButton + onClick={handleTopUp} + disabled={!selectedToken || !isAmountValid || isProcessing || !paymasterUrl} + > + ); + })()}
325-332: Loading check won’t trigger; empty arrays are truthyUse query flags or length. (Previously noted.)
- if (!portfolioTokens) { + if (!isWalletPortfolioDataSuccess && !walletPortfolioDataError) { return ( <LoadingContainer> <CircularProgress size={24} /> <span>Loading wallet tokens...</span> </LoadingContainer> ); }
🧹 Nitpick comments (13)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (4)
160-188: Initialize isPaymaster to false to avoid initial UI flicker; align defaultsStarting with
truebriefly renders Paymaster UI until balance/paymasters resolve. Prefer a conservative default.- const [isPaymaster, setIsPaymaster] = React.useState<boolean>(true); + const [isPaymaster, setIsPaymaster] = React.useState<boolean>(false); ... - React.useState<string>('Gas Tank Paymaster'); + React.useState<string>('Native Token');
235-361: Guard async state updates and add cancellation to prevent stale updatesThese two concurrent async branches can update state after unmount or after
selectedAssetchanges. Add anabortedflag.useEffect(() => { if (!walletPortfolio) return; const tokens = convertPortfolioAPIResponseToToken(walletPortfolio); if (!selectedAsset) return; + let aborted = false; ... - getGasTankBalance(accountAddress).then((res) => { + getGasTankBalance(accountAddress).then((res) => { + if (aborted) return; ... }); ... - getAllGaslessPaymasters(selectedAsset.chainId, tokens).then( + getAllGaslessPaymasters(selectedAsset.chainId, tokens).then( (paymasterObject) => { + if (aborted) return; ... } ); // eslint-disable-next-line react-hooks/exhaustive-deps }, [selectedAsset, walletPortfolio]);And return cleanup:
- }, [selectedAsset, walletPortfolio]); + return () => { + aborted = true; + }; + }, [selectedAsset, walletPortfolio]);
224-234: Prefer constants for fee type ids/titles; avoid stringly-typed comparisonsReduces typo/translation drift across the file.
Example:
const FEE_NATIVE = { id: 'Native Token', title: 'Native Token' } as const; const FEE_GASLESS = { id: 'Gasless', title: 'Gasless' } as const; const FEE_GAS_TANK = { id: 'Gas Tank Paymaster', title: 'Gas Tank Paymaster' } as const;
364-387: Minor consistency: use either ethers.utils or utils consistentlyYou import both
utilsand later useethers.utils. Pick one for readability.AGENT.md (3)
15-17: Add “gas-tank” to Active apps list to reflect this PRKeep docs in sync with code; the Gas Tank app is now part of src/apps.
- - **Active apps**: pillarx-app, the-exchange, token-atlas, deposit, leaderboard + - **Active apps**: pillarx-app, the-exchange, token-atlas, deposit, leaderboard, gas-tank
26-26: Wording nit: “2 space tabs” → “2 spaces”Clarify indentation guidance.
- - **Quotes**: Single quotes, trailing commas (ES5), 80 char width, 2 space tabs + - **Quotes**: Single quotes, trailing commas (ES5), 80 char width, 2 spaces
14-21: Document required envs for Gas TankReaders need VITE_PAYMASTER_URL to run the new app locally.
## Architecture & Structure @@ - **Blockchain**: Ethers v5, Viem, multiple wallet connectors (Privy, Reown/WalletConnect) + +## Environment +- Gas Tank: set `VITE_PAYMASTER_URL` to the Paymaster service base URL.src/apps/gas-tank/hooks/useGasTankBalance.tsx (2)
49-58: Optional: add AbortController and safe URL buildingPrevents state updates from stale requests and properly encodes params.
- const response = await fetch( - `${paymasterUrl}/getGasTankBalance?sender=${walletAddress}`, - { + const controller = new AbortController(); + const url = new URL('/getGasTankBalance', paymasterUrl); + url.searchParams.set('sender', walletAddress); + const response = await fetch(url.toString(), { method: 'GET', headers: { 'Content-Type': 'application/json', }, - } - ); + signal: controller.signal, + });Consider aborting in a cleanup if you move fetch here; or keep as-is if effect-scoped.
92-95: Comment mismatchEffect also reruns when Paymaster URL changes, not just wallet address.
- // Initial fetch and when wallet address changes + // Initial fetch and when wallet address or Paymaster URL changessrc/apps/gas-tank/components/TopUpModal.tsx (4)
136-142: Avoid Number(amount) for token amounts; prefer strings to preserve precisionIf possible, pass amount as string through getBestOffer/useOffer and parse with parseUnits internally.
- const bestOffer = await getBestOffer({ + const bestOffer = await getBestOffer({ fromTokenAddress: selectedToken.contract, - fromAmount: Number(amount), + fromAmount: amount, // string to avoid FP precision loss fromChainId: chainNameToChainIdTokensData(selectedToken.blockchain), fromTokenDecimals: selectedToken.decimals, slippage: 0.03, });If getBestOffer currently requires number, I can provide a small PR to change its type to string and update parseUnits usage.
150-156: Mirror the same precision-safe change when fetching step transactionsKeep amount as string if useOffer supports it; otherwise, at least avoid double conversions.
- const swapTransactions = await getStepTransactions( + const swapTransactions = await getStepTransactions( bestOffer.offer, walletAddress, portfolioTokens, - Number(amount), + amount, // prefer string );
39-44: USDC chains map is incomplete; verify Base and othersConsider adding Base (8453) and any chains you plan to support, or derive from a single source to avoid drift.
Would you like me to wire this to a constants file shared across apps to ensure one source of truth?
301-309: Type mismatch risk: formatTokenAmount returns stringformatTokenAmount uses toFixed and returns a string; confirm its TypeScript return type is string. If it’s typed as number, update it to avoid downstream confusion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
AGENT.md(1 hunks)src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)src/apps/gas-tank/components/UniversalGasTank.tsx(1 hunks)src/apps/gas-tank/hooks/useGasTankBalance.tsx(1 hunks)src/apps/gas-tank/hooks/useOffer.tsx(1 hunks)src/apps/gas-tank/styles/gasTank.css(1 hunks)src/apps/gas-tank/utils/sentry.ts(1 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(13 hunks)src/services/gasless.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/apps/gas-tank/hooks/useOffer.tsx
- src/apps/gas-tank/styles/gasTank.css
- src/apps/gas-tank/utils/sentry.ts
- src/apps/gas-tank/components/UniversalGasTank.tsx
- src/services/gasless.ts
- src/apps/gas-tank/components/GasTankHistory.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
🧬 Code graph analysis (3)
src/apps/gas-tank/components/TopUpModal.tsx (6)
src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/services/tokensData.ts (2)
PortfolioToken(31-34)chainNameToChainIdTokensData(234-255)src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/sentry.ts (2)
logExchangeError(52-81)logExchangeEvent(24-49)src/apps/gas-tank/utils/converters.ts (1)
formatTokenAmount(6-11)
src/apps/gas-tank/hooks/useGasTankBalance.tsx (1)
src/apps/gas-tank/utils/sentry.ts (1)
logExchangeError(52-81)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)
src/services/gasless.ts (1)
getGasTankBalance(63-93)src/components/Form/Select/index.tsx (1)
Select(75-146)
🪛 LanguageTool
AGENT.md
[grammar] ~14-~14: There might be a mistake here.
Context: ...th Prettier ## Architecture & Structure - Multi-app platform: Core PillarX with ...
(QB_NEW_EN)
[grammar] ~15-~15: There might be a mistake here.
Context: ... Core PillarX with multiple sub-apps in src/apps/ - Active apps: pillarx-app, the-exchange...
(QB_NEW_EN)
[grammar] ~16-~16: There might be a mistake here.
Context: ...hange, token-atlas, deposit, leaderboard - Testing: Vitest + React Testing Librar...
(QB_NEW_EN)
[grammar] ~17-~17: There might be a mistake here.
Context: ...s in __tests__/ or test/ directories - State: Redux Toolkit (src/store.ts) ...
(QB_NEW_EN)
[grammar] ~18-~18: There might be a mistake here.
Context: ...tore.ts`) + React Query for server state - Styling: Tailwind CSS + styled-compone...
(QB_NEW_EN)
[grammar] ~19-~19: There might be a mistake here.
Context: ...-components + Material-UI Joy components - Blockchain: Ethers v5, Viem, multiple ...
(QB_NEW_EN)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/apps/gas-tank/hooks/useGasTankBalance.tsx (1)
32-45: Good guardrails for missing address and Paymaster URLEarly returns reset state and surface config errors. Looks solid.
src/apps/gas-tank/components/TopUpModal.tsx (1)
14-15: Chain name mapping: “Optimistic” vs “Optimism”tokensData’s chainNameToChainIdTokensData switches on 'Optimistic'. Confirm portfolio token.chain names; mismatches will return 0 and mark chains unsupported.
I can open a follow-up to either:
- add 'Optimism' to chainNameToChainIdTokensData, or
- map by chainId directly where available.
| // Add swap transactions to batch | ||
| swapTransactions.forEach((tx, index) => { | ||
| const value = tx.value || '0'; | ||
| // Handle bigint conversion properly | ||
| let bigIntValue: bigint; | ||
| if (typeof value === 'bigint') { | ||
| // If value is already a native bigint, use it directly | ||
| bigIntValue = value; | ||
| } else if (value) { | ||
| // If value exists but is not a bigint, convert it | ||
| bigIntValue = BigNumber.from(value).toBigInt(); | ||
| } else { | ||
| // If value is undefined/null, use 0 | ||
| bigIntValue = BigInt(0); | ||
| } | ||
|
|
||
| const integerValue = formatEther(bigIntValue); | ||
| if (!tx.to || !isAddress(tx.to)) { | ||
| setErrorMsg('Invalid transaction target for swap route. Please try again.'); | ||
| logExchangeEvent('Invalid tx.to in swap step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to' }); | ||
| setIsProcessing(false); | ||
| return; | ||
| } | ||
| addToBatch({ | ||
| title: `Swap to USDC ${index + 1}/${swapTransactions.length}`, | ||
| description: `Convert ${amount} ${selectedToken.symbol} to USDC for Gas Tank`, | ||
| to: tx.to, | ||
| value: integerValue, | ||
| data: tx.data, | ||
| chainId: chainNameToChainIdTokensData(selectedToken.blockchain), | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Bug: forEach return doesn’t abort handleTopUp; and value must be wei
Returning inside forEach doesn’t stop outer flow. Also, passing ETH-formatted strings to value underfunds by 1e18. Replace with a for loop and pass wei string.
- // Add swap transactions to batch
- swapTransactions.forEach((tx, index) => {
+ // Add swap transactions to batch
+ for (let index = 0; index < swapTransactions.length; index += 1) {
+ const tx = swapTransactions[index];
const value = tx.value || '0';
// Handle bigint conversion properly
let bigIntValue: bigint;
if (typeof value === 'bigint') {
// If value is already a native bigint, use it directly
bigIntValue = value;
} else if (value) {
// If value exists but is not a bigint, convert it
bigIntValue = BigNumber.from(value).toBigInt();
} else {
// If value is undefined/null, use 0
bigIntValue = BigInt(0);
}
-
- const integerValue = formatEther(bigIntValue);
if (!tx.to || !isAddress(tx.to)) {
setErrorMsg('Invalid transaction target for swap route. Please try again.');
logExchangeEvent('Invalid tx.to in swap step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to' });
setIsProcessing(false);
- return;
+ return; // abort handleTopUp
}
addToBatch({
title: `Swap to USDC ${index + 1}/${swapTransactions.length}`,
description: `Convert ${amount} ${selectedToken.symbol} to USDC for Gas Tank`,
to: tx.to,
- value: integerValue,
+ value: bigIntValue.toString(),
data: tx.data,
chainId: chainNameToChainIdTokensData(selectedToken.blockchain),
});
- });
+ }📝 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.
| // Add swap transactions to batch | |
| swapTransactions.forEach((tx, index) => { | |
| const value = tx.value || '0'; | |
| // Handle bigint conversion properly | |
| let bigIntValue: bigint; | |
| if (typeof value === 'bigint') { | |
| // If value is already a native bigint, use it directly | |
| bigIntValue = value; | |
| } else if (value) { | |
| // If value exists but is not a bigint, convert it | |
| bigIntValue = BigNumber.from(value).toBigInt(); | |
| } else { | |
| // If value is undefined/null, use 0 | |
| bigIntValue = BigInt(0); | |
| } | |
| const integerValue = formatEther(bigIntValue); | |
| if (!tx.to || !isAddress(tx.to)) { | |
| setErrorMsg('Invalid transaction target for swap route. Please try again.'); | |
| logExchangeEvent('Invalid tx.to in swap step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to' }); | |
| setIsProcessing(false); | |
| return; | |
| } | |
| addToBatch({ | |
| title: `Swap to USDC ${index + 1}/${swapTransactions.length}`, | |
| description: `Convert ${amount} ${selectedToken.symbol} to USDC for Gas Tank`, | |
| to: tx.to, | |
| value: integerValue, | |
| data: tx.data, | |
| chainId: chainNameToChainIdTokensData(selectedToken.blockchain), | |
| }); | |
| }); | |
| // Add swap transactions to batch | |
| for (let index = 0; index < swapTransactions.length; index += 1) { | |
| const tx = swapTransactions[index]; | |
| const value = tx.value || '0'; | |
| // Handle bigint conversion properly | |
| let bigIntValue: bigint; | |
| if (typeof value === 'bigint') { | |
| // If value is already a native bigint, use it directly | |
| bigIntValue = value; | |
| } else if (value) { | |
| // If value exists but is not a bigint, convert it | |
| bigIntValue = BigNumber.from(value).toBigInt(); | |
| } else { | |
| // If value is undefined/null, use 0 | |
| bigIntValue = BigInt(0); | |
| } | |
| // Validate recipient and abort entire handleTopUp if invalid | |
| if (!tx.to || !isAddress(tx.to)) { | |
| setErrorMsg('Invalid transaction target for swap route. Please try again.'); | |
| logExchangeEvent( | |
| 'Invalid tx.to in swap step', | |
| 'error', | |
| { tx }, | |
| { component: 'TopUpModal', action: 'invalid_tx_to' } | |
| ); | |
| setIsProcessing(false); | |
| return; // abort handleTopUp | |
| } | |
| addToBatch({ | |
| title: `Swap to USDC ${index + 1}/${swapTransactions.length}`, | |
| description: `Convert ${amount} ${selectedToken.symbol} to USDC for Gas Tank`, | |
| to: tx.to, | |
| // Pass raw wei as a string rather than an ETH-formatted decimal | |
| value: bigIntValue.toString(), | |
| data: tx.data, | |
| chainId: chainNameToChainIdTokensData(selectedToken.blockchain), | |
| }); | |
| } |
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/TopUpModal.tsx around lines 157 to 188: replace
the swapTransactions.forEach with a normal for (let
i=0;i<swapTransactions.length;i++) loop so early return/abort (setErrorMsg,
setIsProcessing(false) and return) actually exits the parent handler; compute
the transaction value in wei (BigNumber) and pass its string (wei) to addToBatch
instead of an ETH-formatted string — handle native bigint or string by creating
a BigNumber with BigNumber.from(value) (or BigInt -> BigNumber) and use
.toString() to get wei, then use that as the value field; keep other validations
(isAddress) and log calls intact and return from the parent function on error.
| const integerValue = formatEther(bigIntValue); | ||
| addToBatch({ | ||
| title: `Gas Tank Top-up ${index + 1}/${transactionData.result.length}`, | ||
| description: `Add ${amount} ${selectedToken.symbol} to Gas Tank`, | ||
| to: tx.to, | ||
| value: integerValue, | ||
| data: tx.data, | ||
| chainId: chainNameToChainIdTokensData(selectedToken.blockchain), | ||
| }); |
There was a problem hiding this comment.
Bug: deposit batch uses ETH-formatted string for value
Use raw wei string; also validate recipient.
- const integerValue = formatEther(bigIntValue);
- addToBatch({
+ if (!tx.to || !isAddress(tx.to)) {
+ setErrorMsg('Invalid deposit transaction. Please try again later.');
+ logExchangeEvent('Invalid tx.to in deposit step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to_deposit' });
+ setIsProcessing(false);
+ return;
+ }
+ addToBatch({
title: `Gas Tank Top-up ${index + 1}/${transactionData.result.length}`,
description: `Add ${amount} ${selectedToken.symbol} to Gas Tank`,
to: tx.to,
- value: integerValue,
+ value: bigIntValue.toString(),
data: tx.data,
chainId: chainNameToChainIdTokensData(selectedToken.blockchain),
});Committable suggestion skipped: line range outside the PR's diff.
| <Select | ||
| type="token" | ||
| onChange={handleOnChangeFeeAsset} | ||
| options={feeType} | ||
| isLoadingOptions={feeAssetOptions.length === 0} | ||
| defaultSelectedId={feeType[0].id} | ||
| isLoadingOptions={feeAssetOptions.length === 0 && fetchingBalances} | ||
| defaultSelectedId={defaultSelectedFeeTypeId} | ||
| /> |
There was a problem hiding this comment.
Apply the same Select remount fix in the regular modal path
<Select
+ key={`feeType-select-regular-${defaultSelectedFeeTypeId}`}
type="token"
onChange={handleOnChangeFeeAsset}
options={feeType}
- isLoadingOptions={feeAssetOptions.length === 0 && fetchingBalances}
+ isLoadingOptions={feeAssetOptions.length === 0 && fetchingBalances}
defaultSelectedId={defaultSelectedFeeTypeId}
/>📝 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.
| <Select | |
| type="token" | |
| onChange={handleOnChangeFeeAsset} | |
| options={feeType} | |
| isLoadingOptions={feeAssetOptions.length === 0} | |
| defaultSelectedId={feeType[0].id} | |
| isLoadingOptions={feeAssetOptions.length === 0 && fetchingBalances} | |
| defaultSelectedId={defaultSelectedFeeTypeId} | |
| /> | |
| <Select | |
| key={`feeType-select-regular-${defaultSelectedFeeTypeId}`} | |
| type="token" | |
| onChange={handleOnChangeFeeAsset} | |
| options={feeType} | |
| isLoadingOptions={feeAssetOptions.length === 0 && fetchingBalances} | |
| defaultSelectedId={defaultSelectedFeeTypeId} | |
| /> |
🤖 Prompt for AI Agents
In src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx around
lines 1551 to 1557, the Select in the regular modal path needs the same remount
fix as the other modal path; add a key prop tied to the default/selected fee
type (e.g., key={String(defaultSelectedFeeTypeId)} or similar) so the Select
remarts when the default changes, or convert it to a fully controlled component
by passing a selectedId prop derived from state; ensure the key or controlled
prop uses the same identifier used in the other path so the component
reinitializes when the fee asset selection changes.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (8)
src/apps/gas-tank/components/GasTankHistory.tsx (8)
53-53: Encode the address and normalize base URL.Avoid malformed URLs and potential issues with special chars in the address.
- fetch(`${API_URL}/getGasTankHistory?sender=${walletAddress}`, { + fetch(`${API_URL.replace(/\/+$/, '')}/getGasTankHistory?sender=${encodeURIComponent(walletAddress)}`, {
2-2: Memoize sorting and fix numeric comparison for token value.Prevents unnecessary resorting on every render and ensures numeric sort for token values.
-import { useEffect, useState } from 'react'; +import { useEffect, useMemo, useState } from 'react';- const sortedHistory = [...historyData].sort((a, b) => { + const sortedHistory = useMemo(() => { + return [...historyData].sort((a, b) => { if (!sortKey) return 0; let valA, valB; switch (sortKey) { case 'id': valA = Number(a.id); valB = Number(b.id); break; case 'date': valA = new Date(a.date).getTime(); valB = new Date(b.date).getTime(); break; case 'type': valA = a.type; valB = b.type; break; case 'amount': valA = Number(a.amount.replace(/[^0-9.-]+/g, '')); valB = Number(b.amount.replace(/[^0-9.-]+/g, '')); break; case 'token': - valA = a.token.value; - valB = b.token.value; + valA = Number(a.token.value); + valB = Number(b.token.value); break; default: return 0; } if (valA < valB) return sortOrder === 'asc' ? -1 : 1; if (valA > valB) return sortOrder === 'asc' ? 1 : -1; return 0; - }); + }); + }, [historyData, sortKey, sortOrder]);Also applies to: 116-146
69-73: Avoid hard-coding USDC and 6 decimals unless guaranteed.If API may serve other tokens, derive symbol/decimals from payload and pass decimals into formatters. Otherwise confirm USDC-only.
If USDC-only is not guaranteed:
- Extend API to return token { symbol, decimals } per row.
- Replace 6 with returned decimals in formatAmount/formatTokenValue.
Also applies to: 241-251
192-193: Polish the error copy.Minor grammar tweak.
- Error has occurred while fetching. Please try after some time + An error occurred while fetching history. Please try again later.
166-186: Make column headers keyboard-accessible.Clickable divs aren’t focusable. Add role, tabIndex, and key handlers.
const handleSort = (key: SortKey) => { if (sortKey === key) { setSortOrder(sortOrder === 'asc' ? 'desc' : 'asc'); } else { setSortKey(key); setSortOrder('asc'); } }; + + const handleHeaderKey = (e: React.KeyboardEvent, key: SortKey) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + handleSort(key); + } + };- <HeaderCell onClick={() => handleSort('id')}> + <HeaderCell role="button" tabIndex={0} onClick={() => handleSort('id')} onKeyDown={(e) => handleHeaderKey(e, 'id')}> # <SortIcon>{getSortIcon('id')}</SortIcon> </HeaderCell> - <HeaderCell onClick={() => handleSort('date')}> + <HeaderCell role="button" tabIndex={0} onClick={() => handleSort('date')} onKeyDown={(e) => handleHeaderKey(e, 'date')}> Date <SortIcon>{getSortIcon('date')}</SortIcon> </HeaderCell> - <HeaderCell onClick={() => handleSort('type')}> + <HeaderCell role="button" tabIndex={0} onClick={() => handleSort('type')} onKeyDown={(e) => handleHeaderKey(e, 'type')}> Type <SortIcon>{getSortIcon('type')}</SortIcon> </HeaderCell> - <HeaderCell onClick={() => handleSort('amount')}> + <HeaderCell role="button" tabIndex={0} onClick={() => handleSort('amount')} onKeyDown={(e) => handleHeaderKey(e, 'amount')}> Amount <SortIcon>{getSortIcon('amount')}</SortIcon> </HeaderCell> - <HeaderCell onClick={() => handleSort('token')}> + <HeaderCell role="button" tabIndex={0} onClick={() => handleSort('token')} onKeyDown={(e) => handleHeaderKey(e, 'token')}> Token <SortIcon>{getSortIcon('token')}</SortIcon> </HeaderCell>Also applies to: 100-111
1-1: Avoid blanket eslint disable.Either move helpers above usage or locally disable per occurrence to keep rule coverage.
49-58: Optional: add request cancellation to prevent state updates after unmount.Abort on wallet changes/unmount; pass signal to fetch.
- const fetchHistory = () => { + const fetchHistory = (signal?: AbortSignal) => { if (!walletAddress) return; setLoading(true); setError(false); // Reset error before fetching - fetch(`${API_URL.replace(/\/+$/, '')}/getGasTankHistory?sender=${encodeURIComponent(walletAddress)}`, { + fetch(`${API_URL.replace(/\/+$/, '')}/getGasTankHistory?sender=${encodeURIComponent(walletAddress)}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, + signal, })- useEffect(() => { - fetchHistory(); + useEffect(() => { + const controller = new AbortController(); + fetchHistory(controller.signal); + return () => controller.abort(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [walletAddress]);Also applies to: 85-89
49-83: Consider extracting API I/O into a typed service and using SWR/React Query.You’ll get caching, retries, focus refetch, and better error states out of the box.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| interface HistoryEntry { | ||
| id: string; | ||
| date: string; | ||
| type: 'Top-up' | 'Spend'; | ||
| amount: string; | ||
| token: { | ||
| symbol: string; | ||
| value: string; | ||
| icon: string; | ||
| }; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Sort by a stable numeric timestamp, not a formatted date string.
Parsing localized strings is brittle and can misorder data. Persist the raw timestamp and sort on it.
interface HistoryEntry {
id: string;
date: string;
+ timestamp: number;
type: 'Top-up' | 'Spend';
amount: string;
token: { const entries: HistoryEntry[] = (data.history || []).map((item: any, idx: number) => {
const isDeposit = item.transactionType === 'Deposit';
return {
id: String(idx + 1), // Numeric id starting from 1
date: formatTimestamp(item.timestamp),
+ timestamp: Number(item.timestamp),
type: isDeposit ? 'Top-up' : 'Spend',
amount: formatAmount(item.amount, isDeposit),
token: { case 'date':
- valA = new Date(a.date).getTime();
- valB = new Date(b.date).getTime();
+ valA = a.timestamp;
+ valB = b.timestamp;
break;Also applies to: 62-76, 124-127
github-actions
bot
temporarily deployed
to
Preview (PRO-3185-Gas-Tank-Paymaster)
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
src/apps/gas-tank/components/GasTankHistory.tsx (1)
10-20: Sort by raw timestamp, not the formatted date string.Parsing the UI string ("Sep 4, 05:30 PM") is locale-sensitive and lossy (no year/seconds). Keep a numeric timestamp and sort on it. This also matches earlier feedback.
interface HistoryEntry { id: string; date: string; + timestamp: number; type: 'Top-up' | 'Spend'; amount: string; token: {const entries: HistoryEntry[] = (data.history || []).map((item: any, idx: number) => { const isDeposit = item.transactionType === 'Deposit'; return { id: String(idx + 1), // Numeric id starting from 1 date: formatTimestamp(item.timestamp), + timestamp: Number(item.timestamp), type: isDeposit ? 'Top-up' : 'Spend', amount: formatAmount(item.amount, isDeposit), token: {- case 'date': - valA = new Date(a.date).getTime(); - valB = new Date(b.date).getTime(); + case 'date': + valA = a.timestamp; + valB = b.timestamp; break;const entries: HistoryEntry[] = (data.history || []).map((item: any, idx: number) => { const isDeposit = item.transactionType === 'Deposit'; return { id: String(idx + 1), date: formatTimestamp(item.timestamp), + timestamp: Number(item.timestamp), type: isDeposit ? 'Top-up' : 'Spend', amount: formatAmount(item.amount, isDeposit), token: {Also applies to: 62-75, 124-127, 278-291
🧹 Nitpick comments (6)
src/apps/gas-tank/components/GasTankHistory.tsx (4)
49-83: Deduplicate fetching logic — use one hook for both places.Both the component and the exported hook implement near-identical fetch/map/error logic. Prefer a single hook (e.g., move
useGasTankHistoryto../hooks/useGasTankHistory) and have this component consume it to avoid drift.Happy to generate the refactor diff relocating the hook and wiring the component to it.
Also applies to: 266-299
188-197: Surface a retry action in the error state.You already have a header refresh; adding an inline Retry improves UX and parity with the balance card.
Proposed minimal change:
- Update the message to: “Sorry, we had an issue loading history.”
- Add a small “Retry” button calling
fetchHistory.
1-1: Limit the ESLint disable scope.Disabling
no-use-before-definefile-wide is heavy-handed. Either remove it (function declarations are hoisted) or disable only where needed.Can you confirm the rule triggers here without the global disable?
49-83: Optional: cancel stale requests on rapid refresh/address change.Add
AbortControllerto prevent race conditions and wasted work when users spam refresh or the address changes.I can provide a diff wiring
AbortControllerwith cleanup inuseEffect.Also applies to: 266-299
src/apps/gas-tank/components/UniversalGasTank.tsx (2)
100-104: Also refresh history after a successful top-up.Balance refreshes, but the history (used for “Total Spend”) may be stale.
<TopUpModal isOpen={showTopUpModal} onClose={() => setShowTopUpModal(false)} onSuccess={() => { setShowTopUpModal(false); - refetch(); + refetch(); + refetchHistory(); }} />
45-48: Add accessible labels to icon-only buttons.Improve a11y and testing selectors by adding
aria-label.- <RefreshButton onClick={refetch} title="Refresh balance"> + <RefreshButton onClick={refetch} title="Refresh balance" aria-label="Refresh balance"> 🔄 </RefreshButton>- <TopUpButton onClick={handleTopUp}>Top up</TopUpButton> + <TopUpButton onClick={handleTopUp} aria-label="Top up Gas Tank">Top up</TopUpButton>Also applies to: 74-74
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)src/apps/gas-tank/components/UniversalGasTank.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-20T09:14:16.888Z
Learnt from: RanaBug
PR: pillarwallet/x#374
File: src/apps/pillarx-app/index.tsx:12-12
Timestamp: 2025-08-20T09:14:16.888Z
Learning: In this codebase, Transaction Kit providers are set up at the container level (src/containers/Authorized.tsx), not at individual app component levels. App components like src/apps/pillarx-app/index.tsx are children that consume the context through the provider tree.
Applied to files:
src/apps/gas-tank/components/UniversalGasTank.tsx
🧬 Code graph analysis (1)
src/apps/gas-tank/components/UniversalGasTank.tsx (1)
src/apps/gas-tank/components/GasTankHistory.tsx (1)
useGasTankHistory(256-307)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/apps/gas-tank/components/GasTankHistory.tsx (1)
241-251: USDC-only assumption and 6 decimals — confirm API contract.Amounts and token metadata are hard-coded to USDC with 6 decimals. If multi-token support is planned, pull
symbolanddecimalsfrom the API and derive formatting accordingly.Would you like me to sketch a type and formatter that handle variable decimals/tokens?
Also applies to: 69-74
src/apps/gas-tank/components/UniversalGasTank.tsx (1)
69-69: GuardtoFixedontotalBalancetype.If
useGasTankBalance()ever returns a string/BigInt,toFixedwill throw.Is
totalBalanceguaranteed to be a number? If not, normalize in the hook.
| const fetchHistory = () => { | ||
| if (!walletAddress) return; | ||
| setLoading(true); | ||
| setError(false); // Reset error before fetching | ||
| fetch(`${API_URL}/getGasTankHistory?sender=${walletAddress}`, { |
There was a problem hiding this comment.
Avoid indefinite loading when walletAddress is undefined.
If there's no wallet, fetchHistory() returns early leaving loading stuck at true.
const fetchHistory = () => {
- if (!walletAddress) return;
+ if (!walletAddress) {
+ setHistoryData([]);
+ setError(false);
+ setLoading(false);
+ return;
+ }
setLoading(true);Repeat for the hook’s fetchHistory.
Also applies to: 266-270
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/GasTankHistory.tsx around lines 49-53 (and
similarly for the hook's fetchHistory around lines 266-270), the function
returns early when walletAddress is undefined but leaves loading stuck true;
modify both fetchHistory implementations to check walletAddress before setting
setLoading(true) (or if you must set loading first, ensure you call
setLoading(false) and optionally setError(true/false) before returning) so that
loading is always reset when aborting due to missing walletAddress.
| valA = a.token.value; | ||
| valB = b.token.value; | ||
| break; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Token column is sorted lexicographically; cast to number.
String comparison will misorder values ("100" < "9").
- case 'token':
- valA = a.token.value;
- valB = b.token.value;
+ case 'token':
+ valA = Number(a.token.value);
+ valB = Number(b.token.value);
break;📝 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.
| valA = a.token.value; | |
| valB = b.token.value; | |
| break; | |
| case 'token': | |
| valA = Number(a.token.value); | |
| valB = Number(b.token.value); | |
| break; |
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/GasTankHistory.tsx around lines 137 to 139, the
token column comparison assigns valA = a.token.value and valB = b.token.value
which compares strings lexicographically; coerce both values to numbers (e.g.,
using Number(...) or parseFloat(...)) before comparison so numeric sorting is
used, and handle NaN safely (fall back to 0 or string compare) to avoid
incorrect ordering.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/apps/gas-tank/components/TopUpModal.tsx (2)
509-517: Token list loading check won’t trigger; empty array is truthyUse query flags instead of checking array truthiness.
- if (!portfolioTokens) { + if (!isWalletPortfolioDataSuccess && !walletPortfolioDataError) { return ( <LoadingContainer> <CircularProgress size={24} /> <span>Loading wallet tokens...</span> </LoadingContainer> ); }
309-339: forEach return doesn’t abort; use a for-loop to bail out earlyReturning inside forEach won’t stop handleConfirmSwap. Replace with a for-loop so errors halt the flow.
- // Add swap transactions to batch - swapDetails.swapTransactions.forEach((tx, index) => { + // Add swap transactions to batch + for (let index = 0; index < swapDetails.swapTransactions.length; index += 1) { + const tx = swapDetails.swapTransactions[index]; const value = tx.value || '0'; // Handle bigint conversion properly let bigIntValue: bigint; if (typeof value === 'bigint') { // If value is already a native bigint, use it directly bigIntValue = value; } else if (value) { // If value exists but is not a bigint, convert it bigIntValue = BigNumber.from(value).toBigInt(); } else { // If value is undefined/null, use 0 bigIntValue = BigInt(0); } - - const integerValue = formatEther(bigIntValue); if (!tx.to || !isAddress(tx.to)) { setErrorMsg('Invalid transaction target for swap route. Please try again.'); logExchangeEvent('Invalid tx.to in swap step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to' }); setIsProcessing(false); - return; + return; // abort handleConfirmSwap } addToBatch({ title: `Swap to USDC ${index + 1}/${swapDetails.swapTransactions.length}`, description: `Convert ${amount} ${selectedToken.symbol} to USDC for Gas Tank`, to: tx.to, - value: integerValue, + value: bigIntValue.toString(), data: tx.data, chainId: chainNameToChainIdTokensData(selectedToken.blockchain), }); - }); + }
🧹 Nitpick comments (4)
src/apps/gas-tank/components/TopUpModal.tsx (4)
578-583: Disable CTA when invalid/missing prerequisites; compute isAmountValid onceGate the button on amount validity and paymaster URL; avoid duplicating checks.
@@ - const [swapAmountUsdPrice, setSwapAmountUsdPrice] = useState(0); + // Derived validation + const isAmountValid = (() => { + if (!selectedToken) return false; + const n = Number(amount); + return Number.isFinite(n) && n > 0 && n <= (selectedToken.balance ?? 0); + })(); @@ - const handleTopUp = async () => { - if (!selectedToken || !amount || !walletAddress) return; + const handleTopUp = async () => { + if (!selectedToken || !walletAddress) return; @@ - // Validate paymaster URL - if (!paymasterUrl) { + // Validate paymaster URL + if (!paymasterUrl) { setErrorMsg('Service unavailable. Please try again later.'); return; } @@ - // Validate amount - const n = Number(amount); - if (!Number.isFinite(n) || n <= 0 || n > (selectedToken.balance ?? 0)) { + // Validate amount + if (!isAmountValid) { setErrorMsg('Enter a valid amount within your balance.'); return; } @@ <TopUpButton onClick={handleTopUp} - disabled={ - !selectedToken || !amount || isProcessing - } + disabled={!selectedToken || !isAmountValid || isProcessing || !paymasterUrl} >Also applies to: 110-116, 123-131, 46-66
20-21: Avoid duplicating USDC_ADDRESSES; import the shared mapKeep a single source of truth by importing from the hook.
-import useOffer from '../hooks/useOffer'; +import useOffer, { USDC_ADDRESSES } from '../hooks/useOffer'; @@ -const USDC_ADDRESSES: { [chainId: number]: string } = { - 137: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359', // Polygon - 42161: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831', // Arbitrum - 10: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85', // Optimism - // Add more chains as needed -}; +// Use USDC_ADDRESSES from useOfferAlso applies to: 39-45
66-67: Remove unused USD price state and styled componentswapAmountUsdPrice/UsdPriceDisplay are unused; trim to reduce noise.
- const [swapAmountUsdPrice, setSwapAmountUsdPrice] = useState(0); @@ - let tokenUsdPrice = 0; - if (selectedToken) { - tokenUsdPrice = Number(value) * (selectedToken.price ?? 0); - } - setSwapAmountUsdPrice(tokenUsdPrice); + // USD preview computed inline below; no state needed @@ -const UsdPriceDisplay = styled.div` - background: #2a2a2a; - border: 1px solid #444; - border-radius: 8px; - padding: 12px 16px; - color: #9ca3af; - font-size: 16px; - white-space: nowrap; - min-width: 80px; - text-align: right; -`;Also applies to: 278-289, 789-799
188-196: Add fetch timeouts to avoid hanging UIGuard external calls with AbortController (e.g., 15s).
- const response = await fetch( + const controller1 = new AbortController(); + const t1 = setTimeout(() => controller1.abort(), 15000); + const response = await fetch( `${paymasterUrl}/getTransactionForDeposit?chainId=${chainNameToChainIdTokensData(selectedToken.blockchain)}&amount=${receiveSwapAmount}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, + signal: controller1.signal, } ); + clearTimeout(t1); @@ - const response = await fetch( + const controller2 = new AbortController(); + const t2 = setTimeout(() => controller2.abort(), 15000); + const response = await fetch( `${paymasterUrl}/getTransactionForDeposit?chainId=${chainNameToChainIdTokensData(selectedToken.blockchain)}&amount=${swapDetails.receiveAmount}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, + signal: controller2.signal, } ); + clearTimeout(t2);Also applies to: 342-350
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/apps/gas-tank/components/GasTankHistory.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
🧬 Code graph analysis (1)
src/apps/gas-tank/components/TopUpModal.tsx (6)
src/apps/gas-tank/hooks/useOffer.tsx (1)
USDC_ADDRESSES(46-51)src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/services/tokensData.ts (2)
PortfolioToken(31-34)chainNameToChainIdTokensData(234-255)src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/apps/gas-tank/utils/sentry.ts (2)
logExchangeError(52-81)logExchangeEvent(24-49)src/apps/gas-tank/utils/converters.ts (1)
formatTokenAmount(6-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/apps/gas-tank/components/TopUpModal.tsx (2)
145-149: Don’t passamountas a string
SwapType.fromAmountis typed asnumberin both gas-tank and exchange modules, and bothgetBestOfferandgetStepTransactionssignatures expect anumber, so removingNumber(amount)would break the existing types.Likely an incorrect or invalid review comment.
233-254: Add missing “Optimism” alias in chain-to-ID mapping
In src/services/tokensData.ts, extend the string→chainId switch to handle both “Optimistic” and “Optimism”:- case 'Optimistic': + case 'Optimistic': + case 'Optimism': return 10;Likely an incorrect or invalid review comment.
github-actions
bot
temporarily deployed
to
Preview (PRO-3185-Gas-Tank-Paymaster)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/apps/gas-tank/components/TopUpModal.tsx (1)
506-558: Loading/CTA logic: use query flags and strict amount/env validation for UXEmpty arrays are truthy; the loading check won’t trigger. Also disable the CTA unless amount is valid and the service is configured.
Apply:
@@ - {(() => { - if (!portfolioTokens) { + {(() => { + if (!isWalletPortfolioDataSuccess && !walletPortfolioDataError) { return ( <LoadingContainer> <CircularProgress size={24} /> <span>Loading wallet tokens...</span> </LoadingContainer> ); } if (walletPortfolioDataError) { return ( <ErrorMessage>Failed to load wallet tokens</ErrorMessage> ); } + if (isWalletPortfolioDataSuccess && portfolioTokens.length === 0) { + return <ErrorMessage>No tokens found</ErrorMessage>; + } return ( <TokenList>Add a computed validity guard (place near other hooks/state):
+ const isAmountValid = (() => { + if (!selectedToken) return false; + const n = Number(amount); + return Number.isFinite(n) && n > 0 && n <= (selectedToken.balance ?? 0); + })(); + const isServiceAvailable = Boolean(paymasterUrl);And wire to the CTA:
- <TopUpButton - onClick={handleTopUp} - disabled={ - !selectedToken || !amount || isProcessing - } - > + <TopUpButton + onClick={handleTopUp} + disabled={!selectedToken || !isAmountValid || isProcessing || !isServiceAvailable} + >Also applies to: 589-593
🧹 Nitpick comments (7)
src/apps/gas-tank/components/TopUpModal.tsx (7)
39-44: Avoid config drift: reuse exported USDC_ADDRESSES from hook instead of re-definingSingle source of truth prevents mismatches across files.
-import useOffer from '../hooks/useOffer'; +import useOffer, { USDC_ADDRESSES } from '../hooks/useOffer'; @@ -const USDC_ADDRESSES: { [chainId: number]: string } = { - 137: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359', // Polygon - 42161: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831', // Arbitrum - 10: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85', // Optimism - // Add more chains as needed -}; +// USDC_ADDRESSES imported from hook (single source of truth)Also applies to: 20-21
187-195: Safer URL building: use URL + URLSearchParams (and optional AbortController)Avoid manual string interpolation for query params.
- const response = await fetch( - `${paymasterUrl}/getTransactionForDeposit?chainId=${chainNameToChainIdTokensData(selectedToken.blockchain)}&amount=${receiveSwapAmount}`, - { - method: 'GET', - headers: { - 'Content-Type': 'application/json', - }, - } - ); + { + const url = new URL(`${paymasterUrl}/getTransactionForDeposit`); + url.search = new URLSearchParams({ + chainId: String(chainNameToChainIdTokensData(selectedToken.blockchain)), + amount: receiveSwapAmount, + }).toString(); + const response = await fetch(url.toString(), { + method: 'GET', + headers: { 'Content-Type': 'application/json' }, + }); + // ... keep existing handling + }Repeat the same pattern for the swap-confirm path.
Also applies to: 341-349
69-72: Remove unused selector to avoid re-renderswalletPortfolio is never used; drop it.
- const walletPortfolio = useAppSelector( - (state) => state.swap.walletPortfolio as PortfolioData | undefined - );
852-862: Remove unused styled component (UsdPriceDisplay)Dead code adds noise.
-const UsdPriceDisplay = styled.div` - background: #2a2a2a; - border: 1px solid #444; - border-radius: 8px; - padding: 12px 16px; - color: #9ca3af; - font-size: 16px; - white-space: nowrap; - min-width: 80px; - text-align: right; -`;
83-105: Effect logic: avoid clearing portfolio during initial loadingThe else-branch runs when isSuccess is false even before error is set, which may cause flicker. Consider gating the “clear” path strictly on error.
- if (!isWalletPortfolioDataSuccess || walletPortfolioDataError) { - if (walletPortfolioDataError) { + if (walletPortfolioDataError) { + { logExchangeError('Failed to fetch wallet portfolio', { "error": walletPortfolioDataError }, { component: 'TopUpModal', action: 'failed_to_fetch_wallet_portfolio' }); console.error(walletPortfolioDataError); - } - dispatch(setWalletPortfolio(undefined)); + dispatch(setWalletPortfolio(undefined)); + } }
301-306: Minor UX: when showing swap confirmation, also reset any previous errorMsgPrevents the CTA from showing an error while the confirmation is open.
setIsProcessing(true); setShowSwapConfirmation(false); + setErrorMsg(null);
291-299: MAX uses formatted balance; consider using raw balance string to avoid rounding surprisesIf available, prefer raw balance string (unformatted) for MAX.
- const getMaxAmount = () => { + const getMaxAmount = () => { if (!selectedToken) return '0'; - return formatTokenAmount(selectedToken.balance); + return String(selectedToken.balance ?? 0); };Note: consistent with team learnings on float tolerances; fine to defer if portfolio returns numbers only.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)src/apps/gas-tank/hooks/useGasTankBalance.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/apps/gas-tank/components/GasTankHistory.tsx
- src/apps/gas-tank/hooks/useGasTankBalance.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/gas-tank/components/TopUpModal.tsx
🧬 Code graph analysis (1)
src/apps/gas-tank/components/TopUpModal.tsx (7)
src/apps/gas-tank/hooks/useOffer.tsx (1)
USDC_ADDRESSES(46-51)src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/services/tokensData.ts (2)
PortfolioToken(31-34)chainNameToChainIdTokensData(234-255)src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/sentry.ts (2)
logExchangeError(52-81)logExchangeEvent(24-49)src/apps/gas-tank/utils/converters.ts (1)
formatTokenAmount(6-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/apps/gas-tank/components/TopUpModal.tsx (2)
264-267: Double-trigger check: do we need both setShowBatchSendModal(true) and showSend()?If showSend opens the batch modal, calling both might duplicate effects.
If both are needed, document why; otherwise remove the redundant call.
Also applies to: 418-421
145-149: Preserve precision: passamountas string and parse inside the hook
- In TopUpModal (lines 145–149, 157–163), replace all
Number(amount)withamount(string).- Change
fromAmounttostringinsrc/apps/gas-tank/utils/types.tsx.- In
useOffer(hook), callparseUnits(fromAmount, decimals)before invokinggetBestOffer/getStepTransactions.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/apps/gas-tank/components/GasTankHistory.tsx (3)
192-195: Token column sorts lexicographically; compare numerically.- case 'token': - valA = a.token.value; - valB = b.token.value; + case 'token': + valA = Number(a.token.value); + valB = Number(b.token.value); break;
88-91: Fix: loading can get stuck when walletAddress is undefined.Early return leaves
loadingtrue and UI spins forever. Reset state before returning.- const fetchHistory = useCallback(() => { - if (!walletAddress) return; + const fetchHistory = useCallback(() => { + if (!walletAddress) { + setHistoryData([]); + setError(false); + setLoading(false); + return; + } setLoading(true);- const fetchHistory = () => { - if (!walletAddress) return; + const fetchHistory = () => { + if (!walletAddress) { + setHistoryData([]); + setError(false); + setLoading(false); + return; + } setLoading(true);Also applies to: 324-327
98-99: Handle non-2xx responses before JSON parsing.Avoid opaque JSON errors and surface HTTP status/body.
- .then((res) => res.json()) + .then(async (res) => { + if (!res.ok) { + const body = await res.text().catch(() => ''); + throw new Error(`HTTP ${res.status} ${res.statusText}${body ? `: ${body}` : ''}`); + } + return res.json(); + })Also applies to: 334-335
🧹 Nitpick comments (7)
src/apps/gas-tank/components/GasTankHistory.tsx (7)
176-183: Sort by a stable numeric timestamp, not a formatted date string.Avoid locale parsing and string-format ambiguities.
- case 'date': - valA = new Date(a.date).getTime(); - valB = new Date(b.date).getTime(); + case 'date': + valA = a.timestamp; + valB = b.timestamp; break;Additionally, add
timestamp: number;toHistoryEntry(see separate diff).
21-32: Add timestamp to HistoryEntry to support reliable sorting.interface HistoryEntry { id: string; date: string; + timestamp: number; type: 'Top-up' | 'Spend'; amount: string; token: { symbol: string; value: string; icon: string; chainId: string; }; }
300-305: Fix amount formatting for small positives and unify decimals.Show "<0.01" only for 0 < x < 0.01 and always render two decimals otherwise.
-function formatAmount(amount: string, isDeposit: boolean): string { - let value = amount; - if (!isDeposit) value = Number(formatUnits(BigInt(amount), 6)).toFixed(2); - if (Number(value) <= 0) value = '<0.01'; - return `${isDeposit ? '+' : '-'}$${value}`; -} +function formatAmount(amount: string, isDeposit: boolean): string { + const num = isDeposit ? Number(amount) : Number(formatUnits(BigInt(amount), 6)); + const display = num > 0 && num < 0.01 ? '<0.01' : num.toFixed(2); + return `${isDeposit ? '+' : '-'}$${display}`; +}
47-49: Use explicit radix when parsing chainId.- const chainIdNum = parseInt(chainId); + const chainIdNum = parseInt(chainId, 10);
1-1: Avoid file-wide eslint disable; not needed for function declarations.Remove the rule or scope it to the exact line that violates it.
-/* eslint-disable @typescript-eslint/no-use-before-define */
88-99: Consider a data-fetching library to handle caching, retries, intervals, and cancellation.SWR/React Query can manage 30s polling, dedupe in-flight requests, and auto-cancel on unmount. It’ll also simplify error/loading states.
Also applies to: 324-367
256-257: Prefer a stable unique key for rows.If the API exposes a tx hash or event id, use that for
keyinstead of the 1-based index-derivedid, which can reorder across refetches and harm React reconciliation.
| fetch(`${API_URL}/getGasTankHistory?sender=0x70e8741c1758Ba32176B188286B8086956627B1c`, { | ||
| method: 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, |
There was a problem hiding this comment.
Replace hardcoded sender with connected wallet; URL-encode the address.
Requests ignore the active wallet and always query a fixed address. Use walletAddress and encodeURIComponent.
- fetch(`${API_URL}/getGasTankHistory?sender=0x70e8741c1758Ba32176B188286B8086956627B1c`, {
+ fetch(`${API_URL}/getGasTankHistory?sender=${encodeURIComponent(walletAddress!)}`, {
method: 'GET',
headers: {
'Content-Type': 'application/json',
},
})Also applies to: 328-333
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/GasTankHistory.tsx around lines 92-96 and
328-333 the fetch calls use a hardcoded sender address; replace that literal
with the connected wallet address variable (walletAddress) and wrap it with
encodeURIComponent before appending to the query string. Ensure you guard
against undefined walletAddress (e.g., skip fetch or handle error) and update
both locations to construct the URL like
`${API_URL}/getGasTankHistory?sender=${encodeURIComponent(walletAddress)}` so
the active wallet is used and the address is URL-encoded.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/apps/gas-tank/components/GasTankHistory.tsx (3)
88-93: Prevent indefinite loading when wallet is undefined; URL-encode the sender.If
walletAddressis falsy,loadingstaystrueand the UI spins forever. Also, encode the address in the querystring.const fetchHistory = useCallback(() => { - if (!walletAddress) return; - setLoading(true); - setError(false); // Reset error before fetching - fetch(`${API_URL}/getGasTankHistory?sender=${walletAddress}`, { + if (!walletAddress) { + setHistoryData([]); + setError(false); + setLoading(false); + return; + } + setLoading(true); + setError(false); // Reset error before fetching + const url = `${API_URL}/getGasTankHistory?sender=${encodeURIComponent(walletAddress)}`; + fetch(url, { method: 'GET', headers: { 'Content-Type': 'application/json', }, })const fetchHistory = () => { - if (!walletAddress) return; - setLoading(true); - setError(false); - fetch(`${API_URL}/getGasTankHistory?sender=${walletAddress}`, { + if (!walletAddress) { + setHistoryData([]); + setError(false); + setLoading(false); + return; + } + setLoading(true); + setError(false); + const url = `${API_URL}/getGasTankHistory?sender=${encodeURIComponent(walletAddress)}`; + fetch(url, { method: 'GET', headers: { 'Content-Type': 'application/json', }, })Also applies to: 324-329
98-99: Handle non-2xx HTTP responses before JSON parsing.Avoid opaque JSON errors; surface status and body.
- .then((res) => res.json()) + .then(async (res) => { + if (!res.ok) { + const body = await res.text().catch(() => ''); + throw new Error(`HTTP ${res.status} ${res.statusText}${body ? `: ${body}` : ''}`); + } + return res.json(); + })- .then((res) => res.json()) + .then(async (res) => { + if (!res.ok) { + const body = await res.text().catch(() => ''); + throw new Error(`HTTP ${res.status} ${res.statusText}${body ? `: ${body}` : ''}`); + } + return res.json(); + })Also applies to: 334-335
21-32: Sort by a stable numeric timestamp and compare token values numerically.Sorting on a localized date string and string token values can misorder rows.
interface HistoryEntry { id: string; date: string; + timestamp: number; type: 'Top-up' | 'Spend'; amount: string; token: {- date: formatTimestamp(item.timestamp), + date: formatTimestamp(item.timestamp), + timestamp: Number(item.timestamp) || 0,- date: formatTimestamp(item.timestamp), + date: formatTimestamp(item.timestamp), + timestamp: Number(item.timestamp) || 0,const sortedHistory = [...historyData].sort((a, b) => { if (!sortKey) return 0; let valA, valB; switch (sortKey) { case 'id': valA = Number(a.id); valB = Number(b.id); break; case 'date': - valA = new Date(a.date).getTime(); - valB = new Date(b.date).getTime(); + valA = a.timestamp; + valB = b.timestamp; break; case 'type': valA = a.type; valB = b.type; break; case 'amount': valA = Number(a.amount.replace(/[^0-9.-]+/g, '')); valB = Number(b.amount.replace(/[^0-9.-]+/g, '')); break; case 'token': - valA = a.token.value; - valB = b.token.value; + valA = Number(a.token.value) || 0; + valB = Number(b.token.value) || 0; break; default: return 0; }Also applies to: 172-195, 100-118, 336-353
🧹 Nitpick comments (4)
src/apps/gas-tank/components/GasTankHistory.tsx (4)
300-305: Normalize USD formatting and correct the “<0.01” threshold.Use 2dp consistently; show “<0.01” only for 0 < amount < 0.01.
function formatAmount(amount: string, isDeposit: boolean): string { - let value = amount; - if (!isDeposit) value = Number(formatUnits(BigInt(amount), 6)).toFixed(2); - if (Number(value) <= 0) value = '<0.01'; - return `${isDeposit ? '+' : '-'}$${value}`; + const num = isDeposit ? Number(amount) : Number(formatUnits(BigInt(amount), 6)); + if (Number.isFinite(num) && num > 0 && num < 0.01) { + return `${isDeposit ? '+' : '-'}$<0.01`; + } + const display = Number.isFinite(num) ? num.toFixed(2) : '0.00'; + return `${isDeposit ? '+' : '-'}$${display}`; }
2-2: Memoize sorting to avoid resorting on every render.Keeps renders cheap as state changes elsewhere occur.
-import { useEffect, useState, useCallback } from 'react'; +import { useEffect, useState, useCallback, useMemo } from 'react';- const sortedHistory = [...historyData].sort((a, b) => { + const sortedHistory = useMemo(() => [...historyData].sort((a, b) => { if (!sortKey) return 0; let valA, valB; switch (sortKey) { case 'id': valA = Number(a.id); valB = Number(b.id); break; case 'date': valA = a.timestamp; valB = b.timestamp; break; case 'type': valA = a.type; valB = b.type; break; case 'amount': valA = Number(a.amount.replace(/[^0-9.-]+/g, '')); valB = Number(b.amount.replace(/[^0-9.-]+/g, '')); break; case 'token': valA = Number(a.token.value) || 0; valB = Number(b.token.value) || 0; break; default: return 0; } if (valA < valB) return sortOrder === 'asc' ? -1 : 1; if (valA > valB) return sortOrder === 'asc' ? 1 : -1; return 0; - }); + }), [historyData, sortKey, sortOrder]);Also applies to: 172-203
47-49: PreferNumber(chainId)overparseInt(chainId)here.Covers both decimal and hex strings consistently.
- const chainIdNum = parseInt(chainId); + const chainIdNum = Number(chainId);
248-249: Polish copy: clearer error message.- Error has occurred while fetching. Please try after some time + An error occurred while fetching. Please try again later.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/apps/gas-tank/hooks/useOffer.tsx (3)
309-317: Remove debug console and dead validation block (or re‑enable properly).Keep logs via your debug logger and avoid commented code.
Apply:
- console.log('totalNativeRequired', fromAmountBigInt); - // // Calculate total required - // const totalNativeRequired = fromAmountBigInt; - // if (userNativeBalance < totalNativeRequired) { - // throw new Error( - // 'Insufficient native token balance to cover swap and fee.' - // ); - // } + // transactionDebugLog?.('gas-tank:totalNativeRequired', { fromAmountBigInt, userNativeBalance });Note: replace with your
transactionDebugLogutility if available.
371-377: Compare allowances as BigInt, not formatted strings.String comparison on formatted values is incorrect; compare raw base‑unit bigints.
Apply:
- const isEnoughAllowance = isAllowance - ? formatUnits(isAllowance, step.action.fromToken.decimals) >= - formatUnits( - BigInt(step.action.fromAmount), - step.action.fromToken.decimals - ) - : undefined; + const isEnoughAllowance = isAllowance + ? isAllowance >= BigInt(step.action.fromAmount) + : undefined;
268-343: Remove manual wrap; LiFi already handles wrap/unwrap in steps.Current check uses
isWrappedToken(...)which triggers wrap when token is already wrapped, risking double-wrapping or revert. Rely on LiFigetStepTransactionoutput instead.Apply:
- /** - * Step 4: Wrap transaction (if required) - * If the route requires wrapped tokens but user has native tokens, - * add a wrapping transaction - */ - if (isWrapRequired) { - const wrapCalldata = encodeFunctionData({ - abi: [ - { - name: 'deposit', - type: 'function', - stateMutability: 'payable', - inputs: [], - outputs: [], - }, - ], - functionName: 'deposit', - }); - - stepTransactions.push({ - to: route.fromToken.address, // wrapped token address - data: wrapCalldata, - value: BigInt(route.fromAmount), - chainId: route.fromChainId, - }); - }
🧹 Nitpick comments (4)
src/apps/gas-tank/hooks/useOffer.tsx (4)
361-369: Don’t call allowance check with empty spender.Passing
''risks an invalid RPC call; skip allowance lookup when no approval address is provided and fail fast before approval path.Apply:
- const isAllowance = isTokenNative - ? undefined // Native tokens never require approval - : // eslint-disable-next-line no-await-in-loop - await isAllowanceSet({ - owner: fromAccount, - spender: step.estimate.approvalAddress || '', - tokenAddress: step.action.fromToken.address, - chainId: step.action.fromChainId, - }); + const hasSpender = !!step.estimate.approvalAddress; + const isAllowance = isTokenNative || !hasSpender + ? undefined // Native or no spender => no allowance check + : // eslint-disable-next-line no-await-in-loop + await isAllowanceSet({ + owner: fromAccount, + spender: step.estimate.approvalAddress as `0x${string}`, + tokenAddress: step.action.fromToken.address, + chainId: step.action.fromChainId, + });And keep the existing guard below that throws if
!step.estimate.approvalAddressbefore adding approval.
286-293: Fee receiver is validated but never used; gate only when fee is applied.Either implement the 1% fee transfer step or defer the env check until a fee transaction is built. Throwing here blocks flows unnecessarily.
230-241: Memoize viem public clients per chain.Creating a client per call is wasteful. Cache by
chainIdinside the hook or a module‑level map.+ const clientCache = new Map<number, ReturnType<typeof createPublicClient>>(); try { - const publicClient = createPublicClient({ - chain: getNetworkViem(chainId), - transport: http(), - }); + const cached = clientCache.get(chainId); + const publicClient = cached ?? createPublicClient({ chain: getNetworkViem(chainId), transport: http() }); + if (!cached) clientCache.set(chainId, publicClient);
41-44: Unused imports/variables.
addExchangeBreadcrumb,startExchangeTransaction, andwalletAddressare unused. Remove or use them to add breadcrumbs at start ofgetBestOffer/getStepTransactions.Also applies to: 53-56
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/gas-tank/hooks/useOffer.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
PR: pillarwallet/x#351
File: src/apps/pulse/utils/intent.ts:44-53
Timestamp: 2025-08-12T07:42:24.656Z
Learning: In the Pulse app's intent utilities (src/apps/pulse/utils/intent.ts), the team has chosen to use floating-point arithmetic for token amount calculations despite potential precision issues, accepting JavaScript's decimal place limitations as a valid trade-off for their use case.
Applied to files:
src/apps/gas-tank/hooks/useOffer.tsx
🧬 Code graph analysis (1)
src/apps/gas-tank/hooks/useOffer.tsx (6)
src/apps/gas-tank/utils/blockchain.ts (3)
processEth(18-24)getNativeBalanceFromPortfolio(129-141)toWei(124-126)src/apps/gas-tank/utils/sentry.ts (1)
logExchangeError(52-81)src/apps/gas-tank/utils/types.tsx (3)
SwapType(10-18)SwapOffer(20-23)StepTransaction(30-39)src/apps/gas-tank/utils/wrappedTokens.ts (3)
getWrappedTokenAddressIfNative(33-44)isWrappedToken(23-28)isNativeToken(30-31)src/apps/deposit/utils/blockchain.tsx (1)
getNetworkViem(60-79)src/services/tokensData.ts (1)
Token(19-29)
🔇 Additional comments (4)
src/apps/gas-tank/hooks/useOffer.tsx (4)
434-445: Verify forcing toAddress = fromAddress is valid for all LiFi steps.Some steps (e.g., approvals) may not require
toAddress, and swaps might set a different recipient when bridging. Confirm with LiFi; otherwise, guard per step type.
273-281: Good: correct decimals conversion with fallback.Using
route.fromToken.decimalswith a sane fallback avoids precision bugs.
95-112: Best‑offer selection looks fine.Reduce over routes using parsed amounts is sound given
processEthbehavior.
144-146: Guard missing USDC address and avoid hardcoding decimals (derive from routes).Return early if USDC isn't configured for the chain and derive
toTokenDecimalsfrom the first route (fallback to 6).File: src/apps/gas-tank/hooks/useOffer.tsx — lines ~144-146 and ~169-186
Apply:
- const toTokenAddress = USDC_ADDRESSES[fromChainId]; - const toTokenDecimals = 6; // USDC has 6 decimals + const toTokenAddress = USDC_ADDRESSES[fromChainId]; + if (!toTokenAddress) { + logExchangeError('USDC address not configured for chain', { fromChainId }, { component: 'useOffer', action: 'usdc_address_missing' }); + return undefined; + }And after fetching routes:
- const allOffers = routes as Route[]; + const allOffers = routes as Route[]; + const toTokenDecimals = + allOffers[0]?.toToken?.decimals && allOffers[0].toToken.decimals > 0 + ? allOffers[0].toToken.decimals + : 6; // fallbackAlso verify the
USDC_ADDRESSESmapping covers every PillarX‑supported chain and extend it where missing.
github-actions
bot
requested a deployment
to
Preview (PRO-3185-Gas-Tank-Paymaster)
There was a problem hiding this comment.
Actionable comments posted: 17
♻️ Duplicate comments (2)
src/apps/gas-tank/components/TopUpModal.tsx (1)
316-347: Early return insideforEachdoesn’t abort the swap flowIn
handleConfirmSwap, thereturninside theforEachonly exits that iteration; the outer function continues, so after we hit an invalidtx.towe still call the paymaster API and show the send modal. Replace theforEachwith an abortable loop (for … ofor a classicfor) so you canreturnfromhandleConfirmSwapas soon as validation fails:- swapDetails.swapTransactions.forEach((tx, index) => { + for (let index = 0; index < swapDetails.swapTransactions.length; index += 1) { + const tx = swapDetails.swapTransactions[index]; const value = tx.value || '0'; … - if (!tx.to || !isAddress(tx.to)) { + if (!tx.to || !isAddress(tx.to)) { setErrorMsg('Invalid transaction target for swap route. Please try again.'); logExchangeEvent('Invalid tx.to in swap step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to' }); setIsProcessing(false); - return; + return; } addToBatch({ … }); - }); + }This ensures we truly abort the swap + deposit sequence when validation fails.
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (1)
1375-1387: Select still ignores updated defaults—add the key prop
defaultSelectedFeeTypeIdandselectedFeeTypenow change programmatically, but theSelectcomponent caches its internal state on first mount. This is the same issue flagged earlier: the visible selection becomes stale when we flip between “Gasless”, “Gas Tank Paymaster”, and “Native Token”. Please add thekeytied to the default (or convert the Select to a controlledselectedId) in both modal paths so it remounts on change, e.g.key={feeType-select-${defaultSelectedFeeTypeId}}.Also applies to: 1518-1526
🧹 Nitpick comments (12)
src/apps/gas-tank/components/Misc/Tooltip.tsx (1)
43-51: Consider simplifying the horizontal overflow logic.The horizontal overflow handling is correct but could be made more concise using Math.max/Math.min.
Optional refactor:
- // Adjust horizontal position if tooltip would overflow - if (idealLeft - tooltipWidth / 2 < minLeft) { - // Too far left - align to left edge - finalLeft = minLeft; - transform = 'translateX(0)'; - } else if (idealLeft + tooltipWidth / 2 > viewportWidth - margin) { - // Too far right - align to right edge - finalLeft = maxLeft; - transform = 'translateX(0)'; - } + // Adjust horizontal position if tooltip would overflow + const halfWidth = tooltipWidth / 2; + if (idealLeft - halfWidth < minLeft || idealLeft + halfWidth > viewportWidth - margin) { + finalLeft = Math.max(minLeft, Math.min(idealLeft, maxLeft)); + transform = 'translateX(0)'; + }src/apps/gas-tank/components/Misc/Close.tsx (2)
7-7: Consider using className for consistency.This component uses inline styles while other components in the same directory use className with Tailwind classes. Consider aligning with the project's styling approach for consistency.
1-15: Verify if both Close and CloseButton components are needed.Both
Close.tsxandCloseButton.tsxserve similar purposes but with different implementations (CloseButton includes Escape key handling and uses SVG, Close uses an imported icon without keyboard handling). Consider consolidating these into a single configurable component if they serve the same use case.src/apps/gas-tank/components/Search/Search.tsx (2)
86-97: Unstable query object in dependency array.The
queryobject returned byuseQuery()is recreated on every render, which will cause this useEffect to run on every render. Consider removing it from the dependencies since you're only reading it once on mount.Apply this diff:
useEffect(() => { inputRef.current?.focus(); const tokenAddress = query.get('asset'); // Only read asset parameter when in buy mode to prevent token address // from token-atlas showing in sell search if (isAddress(tokenAddress || '')) { setSearchText(tokenAddress!); } setSearchType(SearchType.MyHoldings); - }, [query, setSearchText]); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []);
108-138: Consider consolidating duplicate event listener logic.You have two separate useEffects setting up click-outside (lines 108-123) and ESC key (lines 126-138) handlers, both calling
handleClose. The Esc component (lines 266) already handles ESC internally. Consider removing the duplicate ESC useEffect.Apply this diff to remove the duplicate ESC handler:
- // ESC key to close functionality - useEffect(() => { - const handleKeyDown = (event: KeyboardEvent) => { - if (event.key === 'Escape') { - handleClose(); - } - }; - - document.addEventListener('keydown', handleKeyDown); - return () => { - document.removeEventListener('keydown', handleKeyDown); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []);src/apps/gas-tank/components/Search/ChainOverlay.tsx (1)
25-38: Fixed backdrop dimensions may not cover full screen.The backdrop has fixed dimensions (200x210 at lines 30-31), which may not cover the entire screen on all devices. Consider using full viewport dimensions.
Apply this diff:
<div style={{ position: 'fixed', top: 0, left: 0, - width: 200, - height: 210, + width: '100vw', + height: '100vh', zIndex: 1999, }}src/apps/gas-tank/utils/constants.ts (1)
17-26: Inconsistent enum member naming.The enum uses mixed naming conventions:
BNB_Smart_Chain_BEP20uses UPPER_SNAKE_CASE while others use PascalCase. Consider consistent PascalCase throughout.Apply this diff:
export enum MobulaChainNames { Ethereum = 'Ethereum', Polygon = 'Polygon', Base = 'Base', XDAI = 'XDAI', - BNB_Smart_Chain_BEP20 = 'BNB Smart Chain (BEP20)', + BnbSmartChain = 'BNB Smart Chain (BEP20)', Arbitrum = 'Arbitrum', Optimistic = 'Optimistic', All = 'All', }src/apps/gas-tank/types/tokens.ts (2)
12-21: Duplicate PayingToken type definition.The
PayingTokentype is duplicated in src/apps/gas-tank/types/types.ts (lines 26-35). Consider consolidating to a single source of truth.Remove the duplicate and import from the existing location:
+import { PayingToken } from './types'; + export type SelectedToken = { name: string; symbol: string; usdValue: string; logo: string; dailyPriceChange: number; chainId: number; decimals: number; address: string; }; -export type PayingToken = { - name: string; - symbol: string; - logo: string; - actualBal: string; - totalUsd: number; - totalRaw: string; - chainId: number; - address: string; -};
23-25: SearchType only supports MyHoldings.Unlike the Pulse app which has Trending, Fresh, TopGainers, and MyHoldings, Gas Tank only supports MyHoldings. This aligns with the simplified Search component that doesn't use search results. Consider adding a comment explaining this intentional simplification.
src/apps/gas-tank/components/Search/TokenList.tsx (1)
20-52: Unused sorting state and handler.The sorting state (lines 20-25) and
handleSortChangefunction (lines 27-52) are defined but never used. The component doesn't render any sort controls. Consider removing this dead code or adding the sort UI if sorting is needed.Based on learnings: Similar to how the Pulse app's TokenList conditionally shows sort controls for Trending/Fresh search types, if sorting isn't needed for Gas Tank's simplified MyHoldings-only view, remove the unused code.
src/apps/gas-tank/hooks/useGasTankBalance.tsx (2)
71-81: Response shape handling may need flexibility.The code only handles
data.balanceas an object-of-objects (line 72:Object.values(data.balance || {})). If the backend might return a different shape (e.g., array or object-map of primitives), this could break.Based on a past review comment, consider supporting multiple response shapes to improve resilience.
If you need to support multiple shapes, apply this diff:
- // Extract chain balances - const balances: ChainBalance[] = Object.values(data.balance || {}); + // Extract chain balances (supports both { [chainId]: ChainBalance } and array) + const rawBalance = data.balance || {}; + const balances: ChainBalance[] = Array.isArray(rawBalance) + ? rawBalance + : Object.entries(rawBalance).map(([chainId, val]) => { + if (val && typeof val === 'object' && 'balance' in val) { + return val as ChainBalance; + } + return { chainId, balance: String(val ?? '0') }; + }); setChainBalances(balances);
86-86: Consider structured Sentry context.Following up on IAmKio's request to "Please Sentry log this" - the logging is now in place. Consider using a more descriptive key than
"error"in the extra object for better Sentry organization.Apply this diff:
- logExchangeError(errorMessage, { "error": err }, { component: 'useGasTankBalance', action: 'failed_to_fetch_gas_tank_balance' }); + logExchangeError(errorMessage, { error: err, walletAddress, paymasterUrl }, { component: 'useGasTankBalance', action: 'failed_to_fetch_gas_tank_balance' });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (23)
package-lock.jsonis excluded by!**/package-lock.jsonsrc/apps/gas-tank/assets/arrow-down.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/close-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/confirmed-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/copy-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/esc-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/failed-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/gas-tank-icon.pngis excluded by!**/*.pngsrc/apps/gas-tank/assets/globe-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/moreinfo-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/new-tab.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/pending.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/refresh-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/seach-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/selected-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/setting-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/transaction-failed-details-icon.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/usd-coin-usdc-logo.pngis excluded by!**/*.pngsrc/apps/gas-tank/assets/wallet.svgis excluded by!**/*.svgsrc/apps/gas-tank/assets/warning.svgis excluded by!**/*.svgsrc/apps/gas-tank/components/Search/tests/__snapshots__/PortfolioTokenList.test.tsx.snapis excluded by!**/*.snapsrc/apps/gas-tank/components/Search/tests/__snapshots__/Search.test.tsx.snapis excluded by!**/*.snapsrc/apps/gas-tank/icon.pngis excluded by!**/*.png
📒 Files selected for processing (40)
package.json(2 hunks)src/apps/gas-tank/components/GasTank.tsx(1 hunks)src/apps/gas-tank/components/GasTankHistory.styles.ts(1 hunks)src/apps/gas-tank/components/GasTankHistory.tsx(1 hunks)src/apps/gas-tank/components/Misc/Close.tsx(1 hunks)src/apps/gas-tank/components/Misc/CloseButton.tsx(1 hunks)src/apps/gas-tank/components/Misc/Esc.tsx(1 hunks)src/apps/gas-tank/components/Misc/Refresh.tsx(1 hunks)src/apps/gas-tank/components/Misc/Settings.tsx(1 hunks)src/apps/gas-tank/components/Misc/Tooltip.tsx(1 hunks)src/apps/gas-tank/components/Price/TokenPrice.tsx(1 hunks)src/apps/gas-tank/components/Price/TokenPriceChange.tsx(1 hunks)src/apps/gas-tank/components/Search/ChainOverlay.tsx(1 hunks)src/apps/gas-tank/components/Search/ChainSelect.tsx(1 hunks)src/apps/gas-tank/components/Search/PortfolioTokenList.tsx(1 hunks)src/apps/gas-tank/components/Search/Search.tsx(1 hunks)src/apps/gas-tank/components/Search/Sort.tsx(1 hunks)src/apps/gas-tank/components/Search/TokenList.tsx(1 hunks)src/apps/gas-tank/components/Search/tests/PortfolioTokenList.test.tsx(1 hunks)src/apps/gas-tank/components/Search/tests/Search.test.tsx(1 hunks)src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)src/apps/gas-tank/components/UniversalGasTank.styles.ts(1 hunks)src/apps/gas-tank/components/UniversalGasTank.tsx(1 hunks)src/apps/gas-tank/components/shared.styles.ts(1 hunks)src/apps/gas-tank/constants/tokens.ts(1 hunks)src/apps/gas-tank/hooks/useGasTankBalance.tsx(1 hunks)src/apps/gas-tank/hooks/useTokenSearch.ts(1 hunks)src/apps/gas-tank/types/tokens.ts(1 hunks)src/apps/gas-tank/types/types.ts(1 hunks)src/apps/gas-tank/utils/constants.ts(1 hunks)src/apps/gas-tank/utils/number.ts(1 hunks)src/apps/gas-tank/utils/parseSearchData.ts(1 hunks)src/apps/gas-tank/utils/time.ts(1 hunks)src/apps/pillarx-app/components/GasTankPaymasterTile/GasTankPaymasterTile.tsx(1 hunks)src/apps/pillarx-app/index.tsx(2 hunks)src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx(13 hunks)src/types/api.ts(1 hunks)src/types/blockchain.ts(1 hunks)src/utils/blockchain.ts(13 hunks)src/utils/number.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/apps/gas-tank/components/GasTank.tsx
- src/apps/gas-tank/components/GasTankHistory.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
Applied to files:
src/apps/gas-tank/components/Search/Search.tsxsrc/apps/gas-tank/components/Search/TokenList.tsxsrc/apps/gas-tank/hooks/useTokenSearch.tssrc/apps/gas-tank/components/Search/PortfolioTokenList.tsx
🧬 Code graph analysis (22)
src/apps/gas-tank/components/Price/TokenPriceChange.tsx (1)
src/apps/pulse/components/Price/TokenPriceChange.tsx (2)
TokenPriceChange(5-57)TokenPriceChangeProps(1-3)
src/apps/gas-tank/constants/tokens.ts (1)
src/utils/blockchain.ts (1)
isGnosisEnabled(43-44)
src/apps/gas-tank/types/tokens.ts (1)
src/apps/gas-tank/types/types.ts (1)
PayingToken(27-36)
src/apps/gas-tank/components/Search/Search.tsx (10)
src/apps/gas-tank/types/tokens.ts (1)
SelectedToken(1-10)src/types/api.ts (2)
PortfolioData(732-741)Asset(18-25)src/apps/gas-tank/hooks/useTokenSearch.ts (1)
useTokenSearch(5-34)src/apps/gas-tank/utils/parseSearchData.ts (1)
Asset(15-28)src/services/tokensData.ts (2)
Token(19-29)chainNameToChainIdTokensData(234-255)src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)src/apps/gas-tank/components/Misc/Close.tsx (1)
Close(3-15)src/apps/gas-tank/components/Misc/Refresh.tsx (1)
Refresh(10-34)src/apps/gas-tank/components/Misc/Esc.tsx (1)
Esc(7-36)src/apps/pulse/components/Search/Search.tsx (1)
Search(64-451)
src/apps/pillarx-app/components/GasTankPaymasterTile/GasTankPaymasterTile.tsx (1)
src/apps/gas-tank/components/GasTankHistory.tsx (1)
useGasTankHistory(367-447)
src/apps/gas-tank/utils/constants.ts (1)
src/utils/blockchain.ts (3)
isGnosisEnabled(43-44)CompatibleChains(327-329)getChainName(275-294)
src/apps/gas-tank/components/Misc/Refresh.tsx (1)
src/apps/pulse/components/Misc/Refresh.tsx (1)
Refresh(10-35)
src/apps/gas-tank/utils/parseSearchData.ts (2)
src/types/api.ts (3)
Asset(18-25)TokenAssetResponse(542-561)PairResponse(563-649)src/apps/gas-tank/utils/constants.ts (1)
MOBULA_CHAIN_NAMES(13-15)
src/apps/gas-tank/components/Search/TokenList.tsx (7)
src/apps/gas-tank/utils/parseSearchData.ts (1)
Asset(15-28)src/utils/blockchain.ts (1)
getLogoForChainId(171-205)src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/apps/gas-tank/utils/number.ts (1)
formatBigNumber(1-15)src/apps/gas-tank/components/Price/TokenPrice.tsx (1)
TokenPrice(5-33)src/apps/gas-tank/components/Price/TokenPriceChange.tsx (1)
TokenPriceChange(5-56)src/apps/pulse/components/Search/TokenList.tsx (1)
TokenList(19-261)
src/apps/gas-tank/components/Search/ChainOverlay.tsx (3)
src/services/tokensData.ts (1)
chainNameToChainIdTokensData(234-255)src/utils/blockchain.ts (1)
getLogoForChainId(171-205)src/apps/pulse/components/Search/ChainOverlay.tsx (2)
ChainOverlay(15-111)ChainOverlayProps(7-13)
src/apps/gas-tank/hooks/useGasTankBalance.tsx (1)
src/apps/gas-tank/utils/sentry.ts (1)
logExchangeError(52-81)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (5)
src/services/tokensData.ts (3)
convertPortfolioAPIResponseToToken(97-121)Token(19-29)chainNameToChainIdTokensData(234-255)src/services/gasless.ts (2)
getGasTankBalance(63-93)getAllGaslessPaymasters(28-61)src/apps/gas-tank/utils/wrappedTokens.ts (1)
isNativeToken(30-31)src/types/index.ts (2)
TokenAssetSelectOption(41-46)SelectOption(57-63)src/components/Form/Select/index.tsx (1)
Select(75-146)
src/utils/blockchain.ts (1)
src/apps/the-exchange/utils/wrappedTokens.ts (1)
isNativeToken(30-31)
src/apps/gas-tank/components/UniversalGasTank.tsx (2)
src/apps/gas-tank/components/GasTankHistory.tsx (1)
useGasTankHistory(367-447)src/apps/gas-tank/components/UniversalGasTank.styles.ts (1)
S(27-365)
src/apps/gas-tank/components/TopUpModal.tsx (9)
src/apps/gas-tank/hooks/useReducerHooks.tsx (2)
useAppDispatch(5-5)useAppSelector(6-6)src/apps/gas-tank/types/tokens.ts (1)
SelectedToken(1-10)src/services/tokensData.ts (1)
PortfolioToken(31-34)src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/sentry.ts (2)
logExchangeError(52-81)logExchangeEvent(24-49)src/apps/gas-tank/components/Search/Search.tsx (1)
Search(49-324)src/apps/gas-tank/utils/constants.ts (1)
getChainName(51-70)src/utils/blockchain.ts (2)
getChainName(275-294)getLogoForChainId(171-205)
src/apps/gas-tank/types/types.ts (1)
src/apps/gas-tank/types/tokens.ts (1)
PayingToken(12-21)
src/apps/gas-tank/hooks/useTokenSearch.ts (1)
src/apps/gas-tank/utils/constants.ts (1)
getChainId(28-49)
src/apps/gas-tank/components/Search/PortfolioTokenList.tsx (5)
src/types/api.ts (1)
PortfolioData(732-741)src/services/tokensData.ts (2)
Token(19-29)chainNameToChainIdTokensData(234-255)src/apps/gas-tank/constants/tokens.ts (1)
STABLE_CURRENCIES(13-15)src/utils/blockchain.ts (1)
getLogoForChainId(171-205)src/utils/number.tsx (2)
formatExponentialSmallNumber(119-149)limitDigitsNumber(42-69)
src/apps/gas-tank/components/UniversalGasTank.styles.ts (2)
src/apps/gas-tank/components/shared.styles.ts (2)
colors(4-20)typography(23-39)src/apps/gas-tank/components/GasTankHistory.styles.ts (1)
S(10-289)
src/apps/gas-tank/components/Search/tests/PortfolioTokenList.test.tsx (1)
src/types/api.ts (1)
PortfolioData(732-741)
src/apps/gas-tank/components/Search/tests/Search.test.tsx (3)
src/types/api.ts (1)
PortfolioData(732-741)src/apps/gas-tank/hooks/useTokenSearch.ts (1)
useTokenSearch(5-34)src/apps/gas-tank/components/Search/Search.tsx (1)
Search(49-324)
src/apps/gas-tank/components/GasTankHistory.styles.ts (1)
src/apps/gas-tank/components/shared.styles.ts (2)
colors(4-20)typography(23-39)
🪛 Biome (2.1.2)
src/utils/number.tsx
[error] 1-1: Do not shadow the global "isNaN" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 1-1: Do not shadow the global "parseInt" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (16)
src/utils/number.tsx (1)
119-149: Use string-based trimming to avoid reintroducing exponential notation
Replace theparseFloat(fixed).toString()fallback with:const fixed = numValue.toFixed(maxDecimals); - const trimmed = parseFloat(fixed).toString(); - if (trimmed.includes('e-')) { - const [mantissa, exponent] = trimmed.split('e-'); - const exp = parseInt(exponent, 10); - const digits = mantissa.replace('.', ''); - const zeros = '0'.repeat(exp - 1); - return `0.${zeros}${digits}`; - } - return trimmed; + return fixed.replace(/(\.\d*?)0+$/, '$1').replace(/\.$/, '');Verify with edge cases (e.g. 0.00000000000000012, 1.23e-20) to confirm no
"e-"appears.package.json (2)
83-83: LGTM!The
react-loader-spinnerdependency is actively used in the new Refresh component and provides the TailSpin loader component.
70-70:polishedusage confirmed
polishedis imported insrc/apps/gas-tank/components/UniversalGasTank.styles.ts(darken), so this dependency addition is justified.src/types/api.ts (1)
560-560: LGTM!The
price_change_24hfield addition is consistent with existing patterns in this type and appropriately nullable to handle API data where this value may be unavailable.src/apps/gas-tank/components/Search/ChainSelect.tsx (1)
3-13: Verify the missing onClick handler.This button currently has no interactive behavior (no
onClickprop). Ensure this is intentional or if the click handler implementation is pending.src/types/blockchain.ts (1)
29-36: LGTM!The
TokenListTokeninterface is well-structured with appropriate field types and sensible optionality forlogoURI.src/apps/gas-tank/components/Misc/Esc.tsx (1)
1-36: LGTM!The component properly implements keyboard accessibility with Escape key handling and correctly cleans up the event listener on unmount.
src/apps/gas-tank/components/Misc/CloseButton.tsx (1)
1-46: LGTM!The component is well-implemented with proper keyboard accessibility (Escape key handling) and correct cleanup logic.
src/apps/gas-tank/components/Search/Search.tsx (2)
92-94: Non-null assertion after isAddress check is safe.The non-null assertion on
tokenAddress!is safe here sinceisAddressreturnstrueonly when the value is a valid address string.
61-64: searchData is fetched but never used.The
searchDatareturned fromuseTokenSearchis never used in this component. The component only rendersPortfolioTokenListwhich useswalletPortfolioData. Verify if search functionality is intentionally disabled or if this is incomplete.Based on learnings from the Pulse app's Search component (src/apps/pulse/components/Search/Search.tsx), search results should be rendered when
searchTextis present. Is search functionality intentionally disabled for Gas Tank?src/apps/gas-tank/components/Search/ChainOverlay.tsx (1)
15-111: LGTM! Component follows established pattern.This ChainOverlay component is nearly identical to the Pulse app's version (src/apps/pulse/components/Search/ChainOverlay.tsx), which promotes consistency across the codebase.
src/apps/gas-tank/hooks/useGasTankBalance.tsx (1)
37-95: LGTM! Guards and error handling look good.The fetchGasTankBalance callback properly guards against missing wallet address and paymaster URL, includes comprehensive error handling with Sentry logging, and resets state appropriately.
src/apps/gas-tank/components/Price/TokenPrice.tsx (1)
5-33: LGTM! Price formatting logic is clear and correct.The component handles edge cases well:
- Zero values return $0.00
- Values >= 0.01 show 5 decimals
- Very small values use compact subscript notation (e.g., $0.0₃1234)
The logic is easy to follow and the formatting choices are appropriate for displaying token prices.
src/apps/gas-tank/components/UniversalGasTank.styles.ts (1)
1-365: LGTM! Well-structured stylesheet with comprehensive loading states.The styles are well-organized with:
- Proper shimmer animation for skeleton loading states
- Responsive breakpoints for mobile (768px)
- Consistent use of shared color and typography tokens
- Loading variants for all UI components
The structure promotes maintainability and follows established patterns in the codebase.
src/apps/gas-tank/utils/constants.ts (1)
28-49: Default case behavior forAllis correct
The default branch returns a comma-separated list of all chain IDs, matching howchains === MobulaChainNames.Allis handled inparseSearchData.ts.src/apps/gas-tank/components/TopUpModal.tsx (1)
145-172: Fix swap input: amount passed to routing is in USD, not token unitsLine 155 takes the USD figure from the modal (
amount) and feeds it straight intogetBestOffer/getStepTransactionsas though it were a token quantity. Because we display$amounts and derive token quantity elsewhere by dividing byselectedToken.usdValue, any token whose USD price ≠ 1 will be over-swapped by orders of magnitude (e.g., requesting $10 of ETH tries to route 10 ETH). Convert the USD amount to the token amount before invoking the routing helpers and reuse that converted value for both the offer lookup and the step transactions. e.g.- let receiveSwapAmount = amount; + let receiveSwapAmount = amount; + const usdAmount = Number(amount); + const pricePerToken = Number(selectedToken.usdValue); + if (!Number.isFinite(pricePerToken) || pricePerToken <= 0) { + setErrorMsg('Price unavailable for selected token.'); + setIsProcessing(false); + return; + } + const tokenAmount = usdAmount / pricePerToken;- const bestOffer = await getBestOffer({ + const bestOffer = await getBestOffer({ fromTokenAddress: selectedToken.address, - fromAmount: Number(amount), + fromAmount: tokenAmount, fromChainId: selectedToken.chainId, fromTokenDecimals: selectedToken.decimals, slippage: 0.03, });- const swapTransactions = await getStepTransactions( + const swapTransactions = await getStepTransactions( bestOffer.offer, walletAddress, portfolioTokens, - Number(amount), + tokenAmount, );Apply the same fix in the swap-confirmation path so every downstream call uses the token-amount rather than the USD string.
Likely an incorrect or invalid review comment.
| import { TailSpin } from 'react-loader-spinner'; | ||
| import RefreshIcon from '../../assets/refresh-icon.svg'; | ||
|
|
||
| interface RefreshProps { | ||
| onClick?: () => void; | ||
| disabled?: boolean; | ||
| isLoading?: boolean; | ||
| } | ||
|
|
||
| export default function Refresh({ | ||
| onClick, | ||
| disabled = false, | ||
| isLoading = false, | ||
| }: RefreshProps) { | ||
| return ( | ||
| <button | ||
| className={`flex items-center justify-center w-[36px] h-[34px] bg-[#1E1D24] rounded-[8px] ${ | ||
| disabled || isLoading | ||
| ? 'cursor-not-allowed opacity-50' | ||
| : 'cursor-pointer' | ||
| }`} | ||
| type="button" | ||
| aria-label="Refresh" | ||
| onClick={onClick} | ||
| disabled={disabled || isLoading} | ||
| > | ||
| {isLoading ? ( | ||
| <TailSpin color="#FFFFFF" height={20} width={20} /> | ||
| ) : ( | ||
| <img src={RefreshIcon} alt="refresh-icon" /> | ||
| )} | ||
| </button> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated Refresh component to a shared location.
This component is identical to src/apps/pulse/components/Misc/Refresh.tsx. To follow DRY principles, extract this to a shared components directory (e.g., src/components/common/Refresh.tsx) and import it in both gas-tank and pulse apps.
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/Misc/Refresh.tsx lines 1-34: this Refresh
component is duplicated in src/apps/pulse/components/Misc/Refresh.tsx — extract
it to a shared location (e.g., src/components/common/Refresh.tsx), move or copy
the RefreshIcon asset into that shared folder (or update its import to a shared
assets path), export the component from the new shared file, update both
gas-tank and pulse imports to import Refresh from
'src/components/common/Refresh', delete the duplicate files in the app folders,
and run the TypeScript build/linters to fix any path or asset import issues.
| <button | ||
| className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]" | ||
| type="button" | ||
| aria-label="Save" | ||
| > | ||
| <img src={SettingsIcon} alt="settings-icon" /> | ||
| </button> |
There was a problem hiding this comment.
Button is inert and mislabeled for screen readers
This renders an icon-button that does nothing when clicked and announces “Save” to assistive tech, so users get a dead control with the wrong label. Please either remove the button until it’s wired up, or hook it to the intended settings action, and update the accessible name to match the purpose (e.g., “Open gas tank settings”). Example:
-export default function Settings() {
- return (
- <button
- className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]"
- type="button"
- aria-label="Save"
- >
- <img src={SettingsIcon} alt="settings-icon" />
- </button>
- );
-}
+interface SettingsProps {
+ onOpenSettings: () => void;
+}
+
+export default function Settings({ onOpenSettings }: SettingsProps) {
+ return (
+ <button
+ className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]"
+ type="button"
+ aria-label="Open gas tank settings"
+ onClick={onOpenSettings}
+ >
+ <img src={SettingsIcon} alt="Settings" />
+ </button>
+ );
+}📝 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.
| <button | |
| className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]" | |
| type="button" | |
| aria-label="Save" | |
| > | |
| <img src={SettingsIcon} alt="settings-icon" /> | |
| </button> | |
| interface SettingsProps { | |
| onOpenSettings: () => void; | |
| } | |
| export default function Settings({ onOpenSettings }: SettingsProps) { | |
| return ( | |
| <button | |
| className="flex items-center justify-center w-full h-full bg-[#121116] rounded-[10px]" | |
| type="button" | |
| aria-label="Open gas tank settings" | |
| onClick={onOpenSettings} | |
| > | |
| <img src={SettingsIcon} alt="Settings" /> | |
| </button> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/Misc/Settings.tsx around lines 5 to 11, the icon
button is inert and incorrectly labelled "Save" for screen readers; either wire
it to the real settings action or remove it. Change the aria-label to a
descriptive name like "Open gas tank settings" and add an onClick handler that
calls the component's/OpenSettings prop (or dispatches the settings-open action)
so the control actually opens settings; if there is no handler yet, remove the
button or render a non-interactive element (or set aria-hidden) until the
feature is implemented.
| if (idealTop < minTop) { | ||
| // Not enough space above - position below trigger | ||
| finalTop = triggerRect.bottom + 8; | ||
| transform = transform | ||
| .replace('translateX', 'translateX') | ||
| .replace('translateY', 'translateY'); | ||
| } |
There was a problem hiding this comment.
Fix the transform replacement logic.
Lines 57-59 replace translateX with translateX and translateY with translateY, which does nothing. When the tooltip flips below the trigger, the transform should likely be adjusted to account for the changed vertical positioning.
Apply this diff to fix the logic (or remove the no-op replacements):
// Adjust vertical position if tooltip would overflow above
if (idealTop < minTop) {
// Not enough space above - position below trigger
finalTop = triggerRect.bottom + 8;
- transform = transform
- .replace('translateX', 'translateX')
- .replace('translateY', 'translateY');
+ // Transform remains the same for below positioning
}Or if a vertical transform adjustment is needed when flipping:
if (idealTop < minTop) {
// Not enough space above - position below trigger
finalTop = triggerRect.bottom + 8;
- transform = transform
- .replace('translateX', 'translateX')
- .replace('translateY', 'translateY');
+ // Add any necessary vertical transform adjustment here
}📝 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.
| if (idealTop < minTop) { | |
| // Not enough space above - position below trigger | |
| finalTop = triggerRect.bottom + 8; | |
| transform = transform | |
| .replace('translateX', 'translateX') | |
| .replace('translateY', 'translateY'); | |
| } | |
| if (idealTop < minTop) { | |
| // Not enough space above - position below trigger | |
| finalTop = triggerRect.bottom + 8; | |
| // Transform remains the same for below positioning | |
| } |
| const handleMouseEnter = () => { | ||
| setIsVisible(true); | ||
| }; | ||
|
|
||
| const handleMouseLeave = () => { | ||
| setIsVisible(false); | ||
| setIsPositioned(false); | ||
| }; | ||
|
|
||
| useEffect(() => { | ||
| if (isVisible && tooltipRef.current) { | ||
| // Calculate position after tooltip is rendered | ||
| calculatePosition(); | ||
| } | ||
| }, [isVisible]); |
There was a problem hiding this comment.
Add window resize handling and consider touch device support.
The tooltip position is calculated once when it becomes visible but doesn't update if the window is resized. Additionally, touch devices won't trigger mouse events, making the tooltip inaccessible on mobile.
Consider adding resize handling:
useEffect(() => {
if (isVisible && tooltipRef.current) {
// Calculate position after tooltip is rendered
calculatePosition();
+
+ // Recalculate on window resize
+ const handleResize = () => calculatePosition();
+ window.addEventListener('resize', handleResize);
+ return () => window.removeEventListener('resize', handleResize);
}
}, [isVisible]);For touch support, add touch event handlers to the trigger element:
<div
ref={triggerRef}
onMouseEnter={handleMouseEnter}
onMouseLeave={handleMouseLeave}
+ onTouchStart={handleMouseEnter}
+ onTouchEnd={handleMouseLeave}
className="cursor-help"
>Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/Misc/Tooltip.tsx around lines 70-84, the tooltip
only calculates position when it becomes visible and relies on mouse events,
which breaks on window resize and touch devices; add a window resize listener
that calls calculatePosition whenever the tooltip is visible (register on show,
remove on hide and on unmount) and debounce if needed, and add touch handlers
(e.g., onTouchStart/onTouchEnd or pointer event equivalents) on the trigger to
toggle visibility and call calculatePosition so touch users can open/close the
tooltip; ensure you clean up all listeners and avoid duplicating mouse/touch
events (prefer pointer events if available) and continue to reset isPositioned
on hide as before.
| return ( | ||
| <div className="relative inline-block"> | ||
| <div | ||
| ref={triggerRef} | ||
| onMouseEnter={handleMouseEnter} | ||
| onMouseLeave={handleMouseLeave} | ||
| className="cursor-help" | ||
| > | ||
| {children} | ||
| </div> | ||
| {isVisible && ( | ||
| <div | ||
| ref={tooltipRef} | ||
| className="fixed z-50 w-max max-w-[220px] h-auto border border-white/5 bg-[#25232D] p-2 rounded" | ||
| style={{ | ||
| top: position.top, | ||
| left: position.left, | ||
| transform: position.transform, | ||
| opacity: isPositioned ? 1 : 0, | ||
| transition: 'opacity 0.1s ease-in-out', | ||
| }} | ||
| > | ||
| <div className="text-white text-[10px] font-normal leading-tight break-words"> | ||
| {content} | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Add accessibility attributes and keyboard support.
The tooltip is missing ARIA attributes and keyboard support, making it inaccessible to screen reader users and keyboard-only users.
Apply this diff to add basic accessibility:
+ const tooltipId = useRef(`tooltip-${Math.random().toString(36).substr(2, 9)}`);
+
return (
<div className="relative inline-block">
<div
ref={triggerRef}
onMouseEnter={handleMouseEnter}
onMouseLeave={handleMouseLeave}
+ onFocus={handleMouseEnter}
+ onBlur={handleMouseLeave}
+ aria-describedby={isVisible ? tooltipId.current : undefined}
+ tabIndex={0}
className="cursor-help"
>
{children}
</div>
{isVisible && (
<div
ref={tooltipRef}
+ id={tooltipId.current}
+ role="tooltip"
className="fixed z-50 w-max max-w-[220px] h-auto border border-white/5 bg-[#25232D] p-2 rounded"Note: Add useRef import at the top if using the tooltipId approach, or generate IDs using a different method.
📝 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.
| return ( | |
| <div className="relative inline-block"> | |
| <div | |
| ref={triggerRef} | |
| onMouseEnter={handleMouseEnter} | |
| onMouseLeave={handleMouseLeave} | |
| className="cursor-help" | |
| > | |
| {children} | |
| </div> | |
| {isVisible && ( | |
| <div | |
| ref={tooltipRef} | |
| className="fixed z-50 w-max max-w-[220px] h-auto border border-white/5 bg-[#25232D] p-2 rounded" | |
| style={{ | |
| top: position.top, | |
| left: position.left, | |
| transform: position.transform, | |
| opacity: isPositioned ? 1 : 0, | |
| transition: 'opacity 0.1s ease-in-out', | |
| }} | |
| > | |
| <div className="text-white text-[10px] font-normal leading-tight break-words"> | |
| {content} | |
| </div> | |
| </div> | |
| )} | |
| </div> | |
| ); | |
| // Add this above the return in your component body (and import useRef from 'react') | |
| const tooltipId = useRef(`tooltip-${Math.random().toString(36).substr(2, 9)}`); | |
| return ( | |
| <div className="relative inline-block"> | |
| <div | |
| ref={triggerRef} | |
| onMouseEnter={handleMouseEnter} | |
| onMouseLeave={handleMouseLeave} | |
| onFocus={handleMouseEnter} | |
| onBlur={handleMouseLeave} | |
| aria-describedby={isVisible ? tooltipId.current : undefined} | |
| tabIndex={0} | |
| className="cursor-help" | |
| > | |
| {children} | |
| </div> | |
| {isVisible && ( | |
| <div | |
| ref={tooltipRef} | |
| id={tooltipId.current} | |
| role="tooltip" | |
| className="fixed z-50 w-max max-w-[220px] h-auto border border-white/5 bg-[#25232D] p-2 rounded" | |
| style={{ | |
| top: position.top, | |
| left: position.left, | |
| transform: position.transform, | |
| opacity: isPositioned ? 1 : 0, | |
| transition: 'opacity 0.1s ease-in-out', | |
| }} | |
| > | |
| <div className="text-white text-[10px] font-normal leading-tight break-words"> | |
| {content} | |
| </div> | |
| </div> | |
| )} | |
| </div> | |
| ); |
| return ( | ||
| <TileContainer onClick={handleTileClick}> | ||
| <Header> | ||
| <IconImage src={gasTankIcon} alt="Gas Tank" /> | ||
| <Title>Gas Tank Paymaster</Title> | ||
| <ViewAllButton>View All →</ViewAllButton> | ||
| </Header> | ||
|
|
||
| <ContentContainer> | ||
| <LeftSection> | ||
| <BalanceSection> | ||
| {balanceError ? ( | ||
| <ErrorBalance> | ||
| <span>Error loading balance</span> | ||
| </ErrorBalance> | ||
| ) : isBalanceLoading ? ( | ||
| <LoadingBalance /> | ||
| ) : ( | ||
| <> | ||
| <BalanceAmount>${totalBalance.toFixed(2)}</BalanceAmount> | ||
| <NetworkLabel>On All Networks</NetworkLabel> | ||
| </> | ||
| )} | ||
| </BalanceSection> | ||
| </LeftSection> | ||
|
|
||
| <RightSection> | ||
| {isHistoryLoading ? ( | ||
| <LoadingContainer> | ||
| {Array.from({ length: 3 }).map((_, index) => ( | ||
| <LoadingRow key={index}> | ||
| <LoadingCell width="60px" /> | ||
| <LoadingCell width="80px" /> | ||
| <LoadingCell width="70px" /> | ||
| <LoadingCell width="90px" /> | ||
| <LoadingCell width="100px" /> | ||
| </LoadingRow> | ||
| ))} | ||
| </LoadingContainer> | ||
| ) : historyError ? ( | ||
| <ErrorContainer> | ||
| <ErrorText>Unable to load gas tank history</ErrorText> | ||
| </ErrorContainer> | ||
| ) : displayedHistory.length === 0 ? ( | ||
| <EmptyContainer> | ||
| <EmptyText>No transactions yet</EmptyText> | ||
| </EmptyContainer> | ||
| ) : ( | ||
| <TableContainer> | ||
| <TableHeader> | ||
| <HeaderCell width="10%">#</HeaderCell> | ||
| <HeaderCell width="25%">Date</HeaderCell> | ||
| <HeaderCell width="15%">Type</HeaderCell> | ||
| <HeaderCell width="20%">Amount</HeaderCell> | ||
| <HeaderCell width="30%">Token</HeaderCell> | ||
| </TableHeader> | ||
|
|
||
| <TableBody> | ||
| {displayedHistory.map((entry) => ( | ||
| <TableRow key={entry.id}> | ||
| <Cell width="10%">{entry.id}</Cell> | ||
| <Cell width="25%">{entry.date}</Cell> | ||
| <Cell width="15%"> | ||
| <TypeBadge $isDeposit={entry.type === 'Top-up'}> | ||
| {entry.type} | ||
| </TypeBadge> | ||
| </Cell> | ||
| <Cell width="20%"> | ||
| <Amount $isDeposit={entry.amount.startsWith('+')}> | ||
| {entry.amount} | ||
| </Amount> | ||
| </Cell> | ||
| <Cell width="30%"> | ||
| <TokenInfo> | ||
| <TokenIcon src={entry.token.icon} alt={entry.token.symbol} /> | ||
| <TokenDetails> | ||
| <TokenValue>{entry.token.value}</TokenValue> | ||
| <TokenSymbol>{entry.token.symbol}</TokenSymbol> | ||
| </TokenDetails> | ||
| </TokenInfo> | ||
| </Cell> | ||
| </TableRow> | ||
| ))} | ||
| </TableBody> | ||
| </TableContainer> | ||
| )} | ||
| </RightSection> | ||
| </ContentContainer> | ||
| </TileContainer> |
There was a problem hiding this comment.
Make the tile focusable & keyboard-activatable
The whole tile is wrapped in a div with an onClick, so keyboard users can’t focus or activate it. Please swap to a semantic control (button/link) or add role="button" plus tabIndex and keyboard event handling. Recommendation:
- <TileContainer onClick={handleTileClick}>
+ <TileContainer
+ as="button"
+ type="button"
+ onClick={handleTileClick}
+ >If you keep div, be sure to handle Enter/Space manually.
📝 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.
| return ( | |
| <TileContainer onClick={handleTileClick}> | |
| <Header> | |
| <IconImage src={gasTankIcon} alt="Gas Tank" /> | |
| <Title>Gas Tank Paymaster</Title> | |
| <ViewAllButton>View All →</ViewAllButton> | |
| </Header> | |
| <ContentContainer> | |
| <LeftSection> | |
| <BalanceSection> | |
| {balanceError ? ( | |
| <ErrorBalance> | |
| <span>Error loading balance</span> | |
| </ErrorBalance> | |
| ) : isBalanceLoading ? ( | |
| <LoadingBalance /> | |
| ) : ( | |
| <> | |
| <BalanceAmount>${totalBalance.toFixed(2)}</BalanceAmount> | |
| <NetworkLabel>On All Networks</NetworkLabel> | |
| </> | |
| )} | |
| </BalanceSection> | |
| </LeftSection> | |
| <RightSection> | |
| {isHistoryLoading ? ( | |
| <LoadingContainer> | |
| {Array.from({ length: 3 }).map((_, index) => ( | |
| <LoadingRow key={index}> | |
| <LoadingCell width="60px" /> | |
| <LoadingCell width="80px" /> | |
| <LoadingCell width="70px" /> | |
| <LoadingCell width="90px" /> | |
| <LoadingCell width="100px" /> | |
| </LoadingRow> | |
| ))} | |
| </LoadingContainer> | |
| ) : historyError ? ( | |
| <ErrorContainer> | |
| <ErrorText>Unable to load gas tank history</ErrorText> | |
| </ErrorContainer> | |
| ) : displayedHistory.length === 0 ? ( | |
| <EmptyContainer> | |
| <EmptyText>No transactions yet</EmptyText> | |
| </EmptyContainer> | |
| ) : ( | |
| <TableContainer> | |
| <TableHeader> | |
| <HeaderCell width="10%">#</HeaderCell> | |
| <HeaderCell width="25%">Date</HeaderCell> | |
| <HeaderCell width="15%">Type</HeaderCell> | |
| <HeaderCell width="20%">Amount</HeaderCell> | |
| <HeaderCell width="30%">Token</HeaderCell> | |
| </TableHeader> | |
| <TableBody> | |
| {displayedHistory.map((entry) => ( | |
| <TableRow key={entry.id}> | |
| <Cell width="10%">{entry.id}</Cell> | |
| <Cell width="25%">{entry.date}</Cell> | |
| <Cell width="15%"> | |
| <TypeBadge $isDeposit={entry.type === 'Top-up'}> | |
| {entry.type} | |
| </TypeBadge> | |
| </Cell> | |
| <Cell width="20%"> | |
| <Amount $isDeposit={entry.amount.startsWith('+')}> | |
| {entry.amount} | |
| </Amount> | |
| </Cell> | |
| <Cell width="30%"> | |
| <TokenInfo> | |
| <TokenIcon src={entry.token.icon} alt={entry.token.symbol} /> | |
| <TokenDetails> | |
| <TokenValue>{entry.token.value}</TokenValue> | |
| <TokenSymbol>{entry.token.symbol}</TokenSymbol> | |
| </TokenDetails> | |
| </TokenInfo> | |
| </Cell> | |
| </TableRow> | |
| ))} | |
| </TableBody> | |
| </TableContainer> | |
| )} | |
| </RightSection> | |
| </ContentContainer> | |
| </TileContainer> | |
| return ( | |
| <TileContainer | |
| as="button" | |
| type="button" | |
| onClick={handleTileClick} | |
| > | |
| <Header> | |
| <IconImage src={gasTankIcon} alt="Gas Tank" /> | |
| <Title>Gas Tank Paymaster</Title> | |
| <ViewAllButton>View All →</ViewAllButton> | |
| </Header> | |
| <ContentContainer> | |
| <LeftSection> | |
| <BalanceSection> | |
| {balanceError ? ( | |
| <ErrorBalance> | |
| <span>Error loading balance</span> | |
| </ErrorBalance> | |
| ) : isBalanceLoading ? ( | |
| <LoadingBalance /> | |
| ) : ( | |
| <> | |
| <BalanceAmount>${totalBalance.toFixed(2)}</BalanceAmount> | |
| <NetworkLabel>On All Networks</NetworkLabel> | |
| </> | |
| )} | |
| </BalanceSection> | |
| </LeftSection> | |
| <RightSection> | |
| {isHistoryLoading ? ( | |
| <LoadingContainer> | |
| {Array.from({ length: 3 }).map((_, index) => ( | |
| <LoadingRow key={index}> | |
| <LoadingCell width="60px" /> | |
| <LoadingCell width="80px" /> | |
| <LoadingCell width="70px" /> | |
| <LoadingCell width="90px" /> | |
| <LoadingCell width="100px" /> | |
| </LoadingRow> | |
| ))} | |
| </LoadingContainer> | |
| ) : historyError ? ( | |
| <ErrorContainer> | |
| <ErrorText>Unable to load gas tank history</ErrorText> | |
| </ErrorContainer> | |
| ) : displayedHistory.length === 0 ? ( | |
| <EmptyContainer> | |
| <EmptyText>No transactions yet</EmptyText> | |
| </EmptyContainer> | |
| ) : ( | |
| <TableContainer> | |
| <TableHeader> | |
| <HeaderCell width="10%">#</HeaderCell> | |
| <HeaderCell width="25%">Date</HeaderCell> | |
| <HeaderCell width="15%">Type</HeaderCell> | |
| <HeaderCell width="20%">Amount</HeaderCell> | |
| <HeaderCell width="30%">Token</HeaderCell> | |
| </TableHeader> | |
| <TableBody> | |
| {displayedHistory.map((entry) => ( | |
| <TableRow key={entry.id}> | |
| <Cell width="10%">{entry.id}</Cell> | |
| <Cell width="25%">{entry.date}</Cell> | |
| <Cell width="15%"> | |
| <TypeBadge $isDeposit={entry.type === 'Top-up'}> | |
| {entry.type} | |
| </TypeBadge> | |
| </Cell> | |
| <Cell width="20%"> | |
| <Amount $isDeposit={entry.amount.startsWith('+')}> | |
| {entry.amount} | |
| </Amount> | |
| </Cell> | |
| <Cell width="30%"> | |
| <TokenInfo> | |
| <TokenIcon src={entry.token.icon} alt={entry.token.symbol} /> | |
| <TokenDetails> | |
| <TokenValue>{entry.token.value}</TokenValue> | |
| <TokenSymbol>{entry.token.symbol}</TokenSymbol> | |
| </TokenDetails> | |
| </TokenInfo> | |
| </Cell> | |
| </TableRow> | |
| ))} | |
| </TableBody> | |
| </TableContainer> | |
| )} | |
| </RightSection> | |
| </ContentContainer> | |
| </TileContainer> |
🤖 Prompt for AI Agents
In src/apps/pillarx-app/components/GasTankPaymasterTile/GasTankPaymasterTile.tsx
around lines 52 to 140, the TileContainer is a non-focusable div with onClick
which prevents keyboard users from focusing/activating the tile; make it
accessible by either replacing TileContainer with a semantic interactive element
(button or anchor) or by keeping the div but adding role="button", tabIndex={0},
and an onKeyDown handler that triggers the same click behavior for Enter and
Space and prevents default on Space; ensure the element exposes an accessible
name (aria-label or readable text), preserve existing styles (use
styled-components polymorphism or styled.button) and move click handling logic
into a single handler used by both onClick and onKeyDown.
| setQueryString(`?chainId=${selectedAsset.chainId}`); | ||
| getAllGaslessPaymasters(selectedAsset.chainId, tokens).then( | ||
| (paymasterObject) => { | ||
| if (paymasterObject) { | ||
| const nativeToken = tokens.filter( | ||
| (token: Token) => | ||
| isNativeToken(token.contract) && | ||
| chainNameToChainIdTokensData(token.blockchain) === | ||
| selectedAsset.chainId | ||
| ); | ||
| if (nativeToken.length > 0) { | ||
| setNativeAssetPrice(nativeToken[0]?.price || 0); | ||
| } | ||
| const feeOptions = paymasterObject | ||
| .map( | ||
| (item: { | ||
| gasToken: string; | ||
| chainId: number; | ||
| epVersion: string; | ||
| paymasterAddress: string; | ||
| // eslint-disable-next-line consistent-return, array-callback-return | ||
| }) => { | ||
| const tokenData = tokens.find( | ||
| (token: Token) => | ||
| token.contract.toLowerCase() === item.gasToken.toLowerCase() | ||
| ); | ||
| if (tokenData) | ||
| return { | ||
| id: `${item.gasToken}-${item.chainId}-${item.paymasterAddress}-${tokenData.decimals}`, | ||
| type: 'token', | ||
| title: tokenData.name, | ||
| imageSrc: tokenData.logo, | ||
| chainId: chainNameToChainIdTokensData(tokenData.blockchain), | ||
| value: tokenData.balance, | ||
| price: tokenData.price, | ||
| asset: { | ||
| ...tokenData, | ||
| contract: item.gasToken, | ||
| decimals: tokenData.decimals, | ||
| }, | ||
| balance: tokenData.balance ?? 0, | ||
| } as TokenAssetSelectOption; | ||
| } | ||
| ) | ||
| .filter( | ||
| (value): value is TokenAssetSelectOption => value !== undefined | ||
|
|
||
| // Run both checks in parallel | ||
| Promise.all([ | ||
| getGasTankBalance(accountAddress ?? ''), | ||
| getAllGaslessPaymasters(selectedAsset.chainId, tokens) | ||
| ]).then(([resRaw, paymasterObject]) => { | ||
| const res = resRaw ?? 0; | ||
| setGasTankBalance(res); | ||
| setFetchingBalances(false); | ||
| setPaymasterContext(null); | ||
|
|
||
| // Add Gas Tank Paymaster if balance exists | ||
| if (res > 0) { | ||
| availableFeeTypes.push({ | ||
| id: 'Gas Tank Paymaster', | ||
| title: 'Gas Tank Paymaster', | ||
| type: 'token', | ||
| value: '' | ||
| }); | ||
| } | ||
| if (paymasterObject) { | ||
| const nativeToken = tokens.filter( | ||
| (token: Token) => | ||
| isNativeToken(token.contract) && | ||
| chainNameToChainIdTokensData(token.blockchain) === | ||
| selectedAsset.chainId | ||
| ); | ||
| if (nativeToken.length > 0) { | ||
| setNativeAssetPrice(nativeToken[0]?.price || 0); | ||
| } | ||
| const feeOptions = paymasterObject | ||
| .map((item) => { | ||
| const tokenData = tokens.find( | ||
| (token) => token.contract.toLowerCase() === item.gasToken.toLowerCase() | ||
| ); | ||
| if (feeOptions && feeOptions.length > 0 && feeOptions[0]) { | ||
| setFeeType(feeTypeOptions); | ||
| setFeeAssetOptions(feeOptions); | ||
| // get Skandha gas price | ||
| getGasPrice(selectedAsset.chainId).then((res) => { | ||
| setGasPrice(res); | ||
| }); | ||
| setSelectedFeeAsset({ | ||
| token: feeOptions[0].asset.contract, | ||
| decimals: feeOptions[0].asset.decimals, | ||
| tokenPrice: feeOptions[0].asset.price?.toString(), | ||
| balance: feeOptions[0].value?.toString(), | ||
| }); | ||
| setSelectedPaymasterAddress(feeOptions[0].id.split('-')[2]); | ||
| if (selectedFeeType === 'Gasless') { | ||
| setPaymasterContext({ | ||
| mode: 'commonerc20', | ||
| token: feeOptions[0].asset.contract, | ||
| }); | ||
| setIsPaymaster(true); | ||
| } | ||
| } else { | ||
| setIsPaymaster(false); | ||
| setPaymasterContext(null); | ||
| setFeeAssetOptions([]); | ||
| setFeeType([]); | ||
| } | ||
| if (tokenData) | ||
| return { | ||
| id: `${item.gasToken}-${item.chainId}-${item.paymasterAddress}-${tokenData.decimals}`, | ||
| type: 'token', | ||
| title: tokenData.name, | ||
| imageSrc: tokenData.logo, | ||
| chainId: chainNameToChainIdTokensData(tokenData.blockchain), | ||
| value: tokenData.balance, | ||
| price: tokenData.price, | ||
| asset: { | ||
| ...tokenData, | ||
| contract: item.gasToken, | ||
| decimals: tokenData.decimals, | ||
| }, | ||
| balance: tokenData.balance ?? 0, | ||
| } as TokenAssetSelectOption; | ||
| }) | ||
| .filter((value): value is TokenAssetSelectOption => value !== undefined); | ||
|
|
||
| if (feeOptions.length > 0) { | ||
| // Add Gasless option if we have gasless tokens | ||
| availableFeeTypes.push({ | ||
| id: 'Gasless', | ||
| title: 'Gasless', | ||
| type: 'token', | ||
| value: '' | ||
| }); | ||
| setFeeType(availableFeeTypes); | ||
| setFeeAssetOptions(feeOptions); | ||
| setSelectedFeeType('Gasless'); | ||
| setDefaultSelectedFeeTypeId('Gasless'); | ||
| setIsPaymaster(true); | ||
| } else { | ||
| setPaymasterContext(null); | ||
| setIsPaymaster(false); | ||
| setFeeType(availableFeeTypes); | ||
| setFeeAssetOptions([]); | ||
| setFeeType([]); | ||
| setSelectedFeeType('Native Token'); | ||
| setDefaultSelectedFeeTypeId('Native Token'); | ||
| setIsPaymaster(false); | ||
| } | ||
| } else { | ||
| setFeeType(availableFeeTypes); | ||
| setFeeAssetOptions([]); | ||
| setSelectedFeeType('Native Token'); | ||
| setDefaultSelectedFeeTypeId('Native Token'); | ||
| setIsPaymaster(false); | ||
| } | ||
| ); | ||
| }); |
There was a problem hiding this comment.
Gas Tank never becomes selectable when gasless paymasters are unavailable
When getAllGaslessPaymasters returns null or an empty array, we still push “Gas Tank Paymaster” into availableFeeTypes, but the branch immediately sets isPaymaster to false and the default fee type back to “Native Token”. Because the fee-type selector is gated on isPaymaster, the UI never renders the Gas Tank option and EtherspotBatches is invoked without a paymaster, so the new Gas Tank flow cannot be used at all unless a gasless paymaster also exists. Please keep isPaymaster true (and default the selection/context) whenever res > 0, even if there are no gasless paymasters. Suggested shape:
- setFeeType(availableFeeTypes);
- setFeeAssetOptions([]);
- setSelectedFeeType('Native Token');
- setDefaultSelectedFeeTypeId('Native Token');
- setIsPaymaster(false);
+ if (res > 0) {
+ setFeeType(availableFeeTypes);
+ setSelectedFeeType('Gas Tank Paymaster');
+ setDefaultSelectedFeeTypeId('Gas Tank Paymaster');
+ setIsPaymaster(true);
+ setPaymasterContext({ mode: 'gasTankPaymaster' });
+ } else {
+ setFeeType([{ id: 'Native Token', title: 'Native Token', type: 'token', value: '' }]);
+ setSelectedFeeType('Native Token');
+ setDefaultSelectedFeeTypeId('Native Token');
+ setIsPaymaster(false);
+ }This way the Gas Tank path stays available whenever balance exists.
Committable suggestion skipped: line range outside the PR's diff.
| const feeOptions = paymasterObject | ||
| .map((item) => { | ||
| const tokenData = tokens.find( | ||
| (token) => token.contract.toLowerCase() === item.gasToken.toLowerCase() | ||
| ); | ||
| if (feeOptions && feeOptions.length > 0 && feeOptions[0]) { | ||
| setFeeType(feeTypeOptions); | ||
| setFeeAssetOptions(feeOptions); | ||
| // get Skandha gas price | ||
| getGasPrice(selectedAsset.chainId).then((res) => { | ||
| setGasPrice(res); | ||
| }); | ||
| setSelectedFeeAsset({ | ||
| token: feeOptions[0].asset.contract, | ||
| decimals: feeOptions[0].asset.decimals, | ||
| tokenPrice: feeOptions[0].asset.price?.toString(), | ||
| balance: feeOptions[0].value?.toString(), | ||
| }); | ||
| setSelectedPaymasterAddress(feeOptions[0].id.split('-')[2]); | ||
| if (selectedFeeType === 'Gasless') { | ||
| setPaymasterContext({ | ||
| mode: 'commonerc20', | ||
| token: feeOptions[0].asset.contract, | ||
| }); | ||
| setIsPaymaster(true); | ||
| } | ||
| } else { | ||
| setIsPaymaster(false); | ||
| setPaymasterContext(null); | ||
| setFeeAssetOptions([]); | ||
| setFeeType([]); | ||
| } | ||
| if (tokenData) | ||
| return { | ||
| id: `${item.gasToken}-${item.chainId}-${item.paymasterAddress}-${tokenData.decimals}`, | ||
| type: 'token', | ||
| title: tokenData.name, | ||
| imageSrc: tokenData.logo, | ||
| chainId: chainNameToChainIdTokensData(tokenData.blockchain), | ||
| value: tokenData.balance, | ||
| price: tokenData.price, | ||
| asset: { | ||
| ...tokenData, | ||
| contract: item.gasToken, | ||
| decimals: tokenData.decimals, | ||
| }, | ||
| balance: tokenData.balance ?? 0, | ||
| } as TokenAssetSelectOption; | ||
| }) | ||
| .filter((value): value is TokenAssetSelectOption => value !== undefined); | ||
|
|
||
| if (feeOptions.length > 0) { | ||
| // Add Gasless option if we have gasless tokens | ||
| availableFeeTypes.push({ | ||
| id: 'Gasless', | ||
| title: 'Gasless', | ||
| type: 'token', | ||
| value: '' | ||
| }); | ||
| setFeeType(availableFeeTypes); | ||
| setFeeAssetOptions(feeOptions); | ||
| setSelectedFeeType('Gasless'); | ||
| setDefaultSelectedFeeTypeId('Gasless'); | ||
| setIsPaymaster(true); | ||
| } else { |
There was a problem hiding this comment.
Restore the default gasless fee-asset selection
We removed the code that seeded selectedFeeAsset, selectedPaymasterAddress, and paymasterContext from the first gasless option. As a result, when gasless paymasters exist, selectedFeeAsset stays undefined, paymasterContext remains null, and the approval transaction is never inserted—gasless sends now fail unless the user manually re-selects an asset. Re-instate the default assignment when feeOptions is non-empty:
if (feeOptions.length > 0) {
// Add Gasless option if we have gasless tokens
availableFeeTypes.push({
id: 'Gasless',
title: 'Gasless',
type: 'token',
value: ''
});
setFeeType(availableFeeTypes);
setFeeAssetOptions(feeOptions);
- setSelectedFeeType('Gasless');
- setDefaultSelectedFeeTypeId('Gasless');
- setIsPaymaster(true);
+ const [firstFeeOption] = feeOptions;
+ if (firstFeeOption) {
+ const [tokenAddress,, paymasterAddress, decimals] = firstFeeOption.id.split('-');
+ setSelectedFeeAsset({
+ token: tokenAddress,
+ decimals: Number(decimals),
+ tokenPrice: firstFeeOption.asset.price?.toString(),
+ balance: firstFeeOption.value?.toString(),
+ });
+ setSelectedPaymasterAddress(paymasterAddress);
+ setPaymasterContext({
+ mode: 'commonerc20',
+ token: tokenAddress,
+ });
+ }
+ setSelectedFeeType('Gasless');
+ setDefaultSelectedFeeTypeId('Gasless');
+ setIsPaymaster(true);Without this, the gasless flow is broken.
🤖 Prompt for AI Agents
In src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx around
lines 272-309, when feeOptions is non-empty the code adds a Gasless option but
no longer seeds selectedFeeAsset, selectedPaymasterAddress, or paymasterContext;
restore those defaults by taking the first gasless fee option (feeOptions[0])
and: set selectedFeeAsset to feeOptions[0].asset, set selectedPaymasterAddress
to the paymasterAddress that matches that asset (find the entry in
paymasterObject where gasToken equals the asset.contract), and set
paymasterContext accordingly (e.g., include the matched paymaster object or
relevant context used later for approval/tx insertion); keep existing setting of
fee type and flags. Ensure null/undefined checks and use the token/contract
match to populate the approval-related state so the approval transaction is
inserted for gasless sends.
| const amountString = | ||
| typeof amount === 'string' ? amount : amount.toString(); | ||
|
|
||
| if (isNativeToken(tokenAddress)) { | ||
| // Native token transfer | ||
| try { | ||
| const parsedValue = parseUnits(amountString, decimals); | ||
| // Ensure the parsed value is a valid bigint | ||
| if (typeof parsedValue !== 'bigint') { | ||
| throw new Error(`parseUnits returned invalid value: ${parsedValue}`); | ||
| } | ||
| return { | ||
| to: recipient, | ||
| value: parsedValue, | ||
| data: '0x', | ||
| }; | ||
| } catch (parseError) { | ||
| console.error( | ||
| 'parseUnits error:', | ||
| parseError, | ||
| 'amountString:', | ||
| amountString, | ||
| 'decimals:', | ||
| decimals | ||
| ); | ||
| throw new Error( | ||
| `Failed to parse units: ${parseError instanceof Error ? parseError.message : 'Unknown error'}` | ||
| ); | ||
| } | ||
| } | ||
| // ERC20 transfer | ||
| try { | ||
| const parsedAmount = parseUnits(amountString, decimals); | ||
| // Ensure the parsed amount is a valid bigint | ||
| if (typeof parsedAmount !== 'bigint') { | ||
| throw new Error(`parseUnits returned invalid value: ${parsedAmount}`); | ||
| } | ||
| return { | ||
| to: tokenAddress, | ||
| value: '0', | ||
| data: encodeFunctionData({ | ||
| abi: erc20Abi, | ||
| functionName: 'transfer', | ||
| args: [recipient as `0x${string}`, parsedAmount], | ||
| }), | ||
| }; | ||
| } catch (parseError) { | ||
| console.error( | ||
| 'parseUnits error:', | ||
| parseError, | ||
| 'amountString:', | ||
| amountString, | ||
| 'decimals:', | ||
| decimals | ||
| ); | ||
| throw new Error( | ||
| `Failed to parse units: ${parseError instanceof Error ? parseError.message : 'Unknown error'}` | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Failed to build transaction data: ${error instanceof Error ? error.message : 'Unknown error'}` | ||
| ); | ||
| } |
There was a problem hiding this comment.
Don’t re-scale bigint amounts in buildTransactionData
When callers pass a bigint that already represents base units (e.g. 1_000_000n for USDC), we convert it to a string and run parseUnits again. That multiplies by 10^decimals, turning 1 USDC into a million USDC. The same double-scaling happens for native transfers. The ERC-20 branch also returns value: '0', which breaks consumers expecting a bigint. Normalize once and reuse the bigint:
- const amountString =
- typeof amount === 'string' ? amount : amount.toString();
-
- if (isNativeToken(tokenAddress)) {
- try {
- const parsedValue = parseUnits(amountString, decimals);
- if (typeof parsedValue !== 'bigint') {
- throw new Error(`parseUnits returned invalid value: ${parsedValue}`);
- }
- return {
- to: recipient,
- value: parsedValue,
- data: '0x',
- };
- } catch (parseError) {
- …
- }
- }
- // ERC20 transfer
- try {
- const parsedAmount = parseUnits(amountString, decimals);
- if (typeof parsedAmount !== 'bigint') {
- throw new Error(`parseUnits returned invalid value: ${parsedAmount}`);
- }
- return {
- to: tokenAddress,
- value: '0',
- data: encodeFunctionData({
- abi: erc20Abi,
- functionName: 'transfer',
- args: [recipient as `0x${string}`, parsedAmount],
- }),
- };
- } catch (parseError) {
- …
- }
+ const amountString =
+ typeof amount === 'string' ? amount : amount.toString();
+
+ let parsedAmount: bigint;
+ if (typeof amount === 'bigint') {
+ parsedAmount = amount;
+ } else {
+ try {
+ parsedAmount = parseUnits(amountString, decimals);
+ } catch (parseError) {
+ …
+ }
+ }
+
+ if (isNativeToken(tokenAddress)) {
+ return {
+ to: recipient,
+ value: parsedAmount,
+ data: '0x',
+ };
+ }
+
+ return {
+ to: tokenAddress,
+ value: 0n,
+ data: encodeFunctionData({
+ abi: erc20Abi,
+ functionName: 'transfer',
+ args: [recipient as `0x${string}`, parsedAmount],
+ }),
+ };This keeps bigint inputs intact and makes the value field consistent.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/blockchain.ts around lines 435-498, the function currently always
calls parseUnits on amount after coercing to string, which double-scales values
already passed in as bigints and returns a string '0' for ERC-20 transfers;
change the logic to: if amount is a bigint, treat it as already expressed in
base units and use it directly (for native transfers set value to that bigint,
for ERC-20 use it as the transfer arg), otherwise convert non-bigint inputs
(number|string) to string and call parseUnits once; also make the ERC-20
transfer return value a bigint zero (0n) instead of the string '0' so consumers
get a consistent bigint type. Ensure error handling around parseUnits remains
for parsing paths and type checks keep verifying parsed values are bigint.
| @@ -1,4 +1,4 @@ | |||
| import { parseInt, parseInt as parseIntLodash } from 'lodash'; | |||
| import { isNaN, parseInt, parseInt as parseIntLodash } from 'lodash'; | |||
There was a problem hiding this comment.
Fix duplicate import and avoid shadowing global properties.
The import statement has critical issues:
parseIntis imported twice (once as itself and once asparseIntLodash)- Importing
isNaNandparseIntfrom lodash shadows the global properties, which can lead to confusion and subtle bugs
Apply this diff to fix the import:
-import { isNaN, parseInt, parseInt as parseIntLodash } from 'lodash';
+import { isNaN as isNaNLodash, parseInt as parseIntLodash } from 'lodash';Then update line 126 to use the renamed import:
- if (isNaN(numValue)) return '0';
+ if (isNaNLodash(numValue)) return '0';This addresses the Biome linter warnings and eliminates the duplicate import.
📝 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 { isNaN, parseInt, parseInt as parseIntLodash } from 'lodash'; | |
| // src/utils/number.tsx | |
| import { isNaN as isNaNLodash, parseInt as parseIntLodash } from 'lodash'; | |
| ... | |
| // original line 126: | |
| if (isNaNLodash(numValue)) return '0'; | |
| ... |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: Do not shadow the global "isNaN" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
[error] 1-1: Do not shadow the global "parseInt" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🤖 Prompt for AI Agents
In src/utils/number.tsx around line 1 (and update at line 126), the import
statement currently imports parseInt twice and shadows globals by importing
isNaN and parseInt from lodash; replace the lodash imports so you only import
the lodash functions with distinct names (e.g., import isNaNLodash and
parseIntLodash) or remove the lodash imports entirely if globals are sufficient,
and then update the usage at line 126 to call the renamed import
(parseIntLodash) instead of the duplicated parseInt; ensure there is no
duplicate import and no global shadowing.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (7)
src/apps/gas-tank/components/TopUpModal.tsx (7)
6-6: Critical: formatEther import still present despite past reviews flagging value underfundingPast reviews identified that
formatEtherconverts wei to a decimal ETH string (e.g., "0.1234"), butaddToBatch.valueexpects aBigNumberishin wei. UsingformatEtherfor transaction values underfunds by 10¹⁸ or causes validation failures. The import remains on line 6 and the function is used throughout the file (lines 247, 272, 341, 398, 423). This critical bug must be fixed.Remove the
formatEtherimport and replace all usages with.toString()on the bigint wei value:-import { formatEther, isAddress } from 'viem'; +import { isAddress } from 'viem';Then update all affected lines (247, 272, 341, 398, 423) to pass wei directly:
- const integerValue = formatEther(bigIntValue); addToBatch({ ... - value: integerValue, + value: bigIntValue.toString(), ... });Based on past review comments.
163-169: Pass amount as string to getBestOffer to preserve precisionSimilar to past reviews on this file, using
Number(swapAmount)can lose precision for token amounts with many decimals. Past reviews recommended passing amount as a string and updating thegetBestOffersignature to acceptfromAmount: string.const bestOffer = await getBestOffer({ fromTokenAddress: selectedToken.address, - fromAmount: Number(swapAmount), + fromAmount: swapAmount.toString(), fromChainId: selectedToken.chainId, fromTokenDecimals: selectedToken.decimals, slippage: 0.03, });Ensure the
SwapTypeinterface insrc/apps/gas-tank/utils/types.tsxdefinesfromAmount: string.Based on past review comments.
232-256: Critical: Replace forEach with for loop and remove formatEther usageThe
forEachon line 232 prevents early returns from abortinghandleTopUp, andformatEtheron line 247 converts wei to an ETH decimal string, which underfunds the transaction by 10¹⁸. Past reviews flagged both issues.- if (Array.isArray(transactionData.result)) { - transactionData.result.forEach((tx: {value?: string, to: string, data?: string}, index: number) => { + if (Array.isArray(transactionData.result)) { + for (let index = 0; index < transactionData.result.length; index += 1) { + const tx = transactionData.result[index] as { value?: string | bigint; to?: string; data?: string }; const value = tx.value || '0'; - // Handle bigint conversion properly - let bigIntValue: bigint; - if (typeof value === 'bigint') { - // If value is already a native bigint, use it directly - bigIntValue = value; - } else if (value) { - // If value exists but is not a bigint, convert it - bigIntValue = BigNumber.from(value).toBigInt(); - } else { - // If value is undefined/null, use 0 - bigIntValue = BigInt(0); - } - - const integerValue = formatEther(bigIntValue); + const wei: bigint = typeof value === 'bigint' ? value : BigInt(value); + if (!tx?.to || !isAddress(tx.to)) { + setErrorMsg('Invalid deposit transaction. Please try again later.'); + logExchangeEvent('Invalid tx.to in deposit step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to_deposit' }); + setIsProcessing(false); + return; + } addToBatch({ title: `Gas Tank Top-up ${index + 1}/${transactionData.result.length}`, description: `Add ${amount} ${selectedToken.symbol} to Gas Tank`, to: tx.to, - value: integerValue, + value: wei.toString(), data: tx.data, chainId: selectedToken.chainId, }); - }); + }Based on past review comments.
257-282: Critical: Remove formatEther from single transaction pathLine 272 uses
formatEther, which converts wei to an ETH decimal string. This underfunds the transaction by 10¹⁸. Also add address validation.} else { const value = transactionData.result.value || '0'; - // Handle bigint conversion properly - let bigIntValue: bigint; - if (typeof value === 'bigint') { - // If value is already a native bigint, use it directly - bigIntValue = value; - } else if (value) { - // If value exists but is not a bigint, convert it - bigIntValue = BigNumber.from(value).toBigInt(); - } else { - // If value is undefined/null, use 0 - bigIntValue = BigInt(0); - } - - const integerValue = formatEther(bigIntValue); - // Single transaction + const wei: bigint = typeof value === 'bigint' ? value : BigInt(value); + if (!transactionData.result.to || !isAddress(transactionData.result.to)) { + setErrorMsg('Invalid deposit transaction. Please try again later.'); + logExchangeEvent('Invalid single tx.to in deposit', 'error', { tx: transactionData.result }, { component: 'TopUpModal', action: 'invalid_tx_to_single' }); + setIsProcessing(false); + return; + } addToBatch({ title: 'Gas Tank Top-up', description: `Add ${amount} ${selectedToken.symbol} to Gas Tank`, to: transactionData.result.to, - value: integerValue, + value: wei.toString(), data: transactionData.result.data, chainId: selectedToken.chainId, });Based on past review comments.
326-356: Critical: Replace forEach with for loop and remove formatEther in swap batchThe
forEachon line 326 prevents early returns from abortinghandleConfirmSwap, andformatEtheron line 341 converts wei to an ETH decimal string, underfunding by 10¹⁸.- // Add swap transactions to batch - swapDetails.swapTransactions.forEach((tx, index) => { + // Add swap transactions to batch (abortable) + for (let index = 0; index < swapDetails.swapTransactions.length; index += 1) { + const tx = swapDetails.swapTransactions[index]; const value = tx.value || '0'; - // Handle bigint conversion properly - let bigIntValue: bigint; - if (typeof value === 'bigint') { - // If value is already a native bigint, use it directly - bigIntValue = value; - } else if (value) { - // If value exists but is not a bigint, convert it - bigIntValue = BigNumber.from(value).toBigInt(); - } else { - // If value is undefined/null, use 0 - bigIntValue = BigInt(0); - } - - const integerValue = formatEther(bigIntValue); + const wei: bigint = typeof value === 'bigint' ? value : BigInt(value); if (!tx.to || !isAddress(tx.to)) { setErrorMsg('Invalid transaction target for swap route. Please try again.'); logExchangeEvent('Invalid tx.to in swap step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to' }); setIsProcessing(false); return; } addToBatch({ title: `Swap to USDC ${index + 1}/${swapDetails.swapTransactions.length}`, description: `Convert ${amount} ${selectedToken.symbol} to USDC for Gas Tank`, to: tx.to, - value: integerValue, + value: wei.toString(), data: tx.data, chainId: selectedToken.chainId, }); - }); + }Based on past review comments.
383-407: Critical: Replace forEach with for loop and remove formatEther in deposit batchSame issues as above:
forEachon line 383 doesn't abort properly, andformatEtheron line 398 underfunds by 10¹⁸.- if (Array.isArray(transactionData.result)) { - transactionData.result.forEach((tx: {value?: string, to: string, data?: string}, index: number) => { + if (Array.isArray(transactionData.result)) { + for (let index = 0; index < transactionData.result.length; index += 1) { + const tx = transactionData.result[index] as { value?: string | bigint; to?: string; data?: string }; const value = tx.value || '0'; - // Handle bigint conversion properly - let bigIntValue: bigint; - if (typeof value === 'bigint') { - // If value is already a native bigint, use it directly - bigIntValue = value; - } else if (value) { - // If value exists but is not a bigint, convert it - bigIntValue = BigNumber.from(value).toBigInt(); - } else { - // If value is undefined/null, use 0 - bigIntValue = BigInt(0); - } - - const integerValue = formatEther(bigIntValue); + const wei: bigint = typeof value === 'bigint' ? value : BigInt(value); + if (!tx?.to || !isAddress(tx.to)) { + setErrorMsg('Invalid deposit transaction. Please try again later.'); + logExchangeEvent('Invalid tx.to in deposit step', 'error', { tx }, { component: 'TopUpModal', action: 'invalid_tx_to_deposit' }); + setIsProcessing(false); + return; + } addToBatch({ title: `Gas Tank Top-up ${index + 1}/${transactionData.result.length}`, description: `Add ${amount} ${selectedToken.symbol} to Gas Tank`, to: tx.to, - value: integerValue, + value: wei.toString(), data: tx.data, chainId: selectedToken.chainId, }); - }); + }Based on past review comments.
408-433: Critical: Remove formatEther from single deposit transaction pathLine 423 uses
formatEther, underfunding by 10¹⁸. Also add address validation.} else { const value = transactionData.result.value || '0'; - // Handle bigint conversion properly - let bigIntValue: bigint; - if (typeof value === 'bigint') { - // If value is already a native bigint, use it directly - bigIntValue = value; - } else if (value) { - // If value exists but is not a bigint, convert it - bigIntValue = BigNumber.from(value).toBigInt(); - } else { - // If value is undefined/null, use 0 - bigIntValue = BigInt(0); - } - - const integerValue = formatEther(bigIntValue); - // Single transaction + const wei: bigint = typeof value === 'bigint' ? value : BigInt(value); + if (!transactionData.result.to || !isAddress(transactionData.result.to)) { + setErrorMsg('Invalid deposit transaction. Please try again later.'); + logExchangeEvent('Invalid single tx.to in deposit', 'error', { tx: transactionData.result }, { component: 'TopUpModal', action: 'invalid_tx_to_single' }); + setIsProcessing(false); + return; + } addToBatch({ title: 'Gas Tank Top-up', description: `Add ${amount} ${selectedToken.symbol} to Gas Tank`, to: transactionData.result.to, - value: integerValue, + value: wei.toString(), data: transactionData.result.data, chainId: selectedToken.chainId, });Based on past review comments.
🧹 Nitpick comments (1)
src/apps/gas-tank/components/TopUpModal.tsx (1)
45-50: Eliminate USDC_ADDRESSES duplicationThis constant is duplicated from
src/apps/gas-tank/hooks/useOffer.tsx(lines 45-50). Import it fromuseOffer.tsxor move to a shared constants file to maintain a single source of truth.-const USDC_ADDRESSES: { [chainId: number]: string } = { - 137: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359', // Polygon - 42161: '0xaf88d065e77c8cC2239327C5EDb3A432268e5831', // Arbitrum - 10: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85', // Optimism - // Add more chains as needed -}; +import { USDC_ADDRESSES } from '../hooks/useOffer';Or export from a shared constants file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/gas-tank/components/TopUpModal.tsx (9)
src/apps/gas-tank/hooks/useOffer.tsx (1)
USDC_ADDRESSES(46-51)src/apps/gas-tank/types/tokens.ts (1)
SelectedToken(1-10)src/services/tokensData.ts (1)
PortfolioToken(31-34)src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/sentry.ts (2)
logExchangeError(52-81)logExchangeEvent(24-49)src/apps/gas-tank/utils/constants.ts (2)
getChainId(28-49)getChainName(51-70)src/apps/gas-tank/components/Search/Search.tsx (1)
Search(49-324)src/utils/blockchain.ts (2)
getChainName(275-294)getLogoForChainId(171-205)
| <DetailsSection> | ||
| <DetailRow> | ||
| <DetailLabel>Rate</DetailLabel> | ||
| <DetailValue>1 USD ≈ {selectedToken ? (1 / parseFloat(selectedToken.usdValue)).toFixed(4) : '1.08'} {selectedToken?.symbol || 'USDC'}</DetailValue> |
There was a problem hiding this comment.
Guard against division by zero in rate display
If selectedToken.usdValue is not parseable, 1 / parseFloat(...) will fail.
- <DetailValue>1 USD ≈ {selectedToken ? (1 / parseFloat(selectedToken.usdValue)).toFixed(4) : '1.08'} {selectedToken?.symbol || 'USDC'}</DetailValue>
+ <DetailValue>1 USD ≈ {selectedToken ? (1 / (parseFloat(selectedToken.usdValue) || 1)).toFixed(4) : '1.08'} {selectedToken?.symbol || 'USDC'}</DetailValue>📝 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.
| <DetailValue>1 USD ≈ {selectedToken ? (1 / parseFloat(selectedToken.usdValue)).toFixed(4) : '1.08'} {selectedToken?.symbol || 'USDC'}</DetailValue> | |
| <DetailValue> | |
| 1 USD ≈ {selectedToken | |
| ? (1 / (parseFloat(selectedToken.usdValue) || 1)).toFixed(4) | |
| : '1.08'}{' '} | |
| {selectedToken?.symbol || 'USDC'} | |
| </DetailValue> |
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/TopUpModal.tsx around line 603, the expression 1
/ parseFloat(selectedToken.usdValue) can throw or produce Infinity when usdValue
is not parseable or zero; update the code to parse the value first (e.g., const
rate = parseFloat(selectedToken?.usdValue ?? '') or Number(...)), check that
rate is a finite non-zero number (isFinite(rate) && rate !== 0), and only
compute (1 / rate).toFixed(4) when valid; otherwise fall back to the existing
default string ('1.08') or compute a safe fallback and keep using
selectedToken?.symbol || 'USDC'.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/apps/gas-tank/components/TopUpModal.tsx (1)
335-365: Abort the swap flow when validation fails (forEach return won’t exit the handler)Returning inside the
forEachonly stops that iteration;handleConfirmSwapstill continues, so after hitting the "invalid tx.to" branch we proceed to call the paymaster endpoint and show the send modal with a broken batch. Use an abortable loop so you canreturn;from the handler.- swapDetails.swapTransactions.forEach((tx, index) => { + for (let index = 0; index < swapDetails.swapTransactions.length; index += 1) { + const tx = swapDetails.swapTransactions[index]; @@ - return; + return; } addToBatch({ @@ - }); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
src/apps/gas-tank/components/TopUpModal.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/apps/gas-tank/components/TopUpModal.tsx (8)
src/apps/gas-tank/types/tokens.ts (1)
SelectedToken(1-10)src/services/tokensData.ts (1)
PortfolioToken(31-34)src/hooks/useTransactionDebugLogger.tsx (1)
useTransactionDebugLogger(1-15)src/apps/gas-tank/reducer/gasTankSlice.ts (1)
setWalletPortfolio(19-24)src/apps/gas-tank/utils/sentry.ts (2)
logExchangeError(52-81)logExchangeEvent(24-49)src/apps/gas-tank/utils/constants.ts (2)
getChainId(28-49)getChainName(51-70)src/apps/gas-tank/components/Search/Search.tsx (1)
Search(49-324)src/utils/blockchain.ts (2)
getChainName(275-294)getLogoForChainId(171-205)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| import { CircularProgress } from '@mui/material'; | ||
| import { useState, useEffect } from 'react'; | ||
| import { BigNumber } from 'ethers'; | ||
| import { formatEther, isAddress } from 'viem'; |
There was a problem hiding this comment.
Send wei to the batch instead of human-readable ETH strings
addToBatch.value must be a wei BigNumberish. Passing formatEther(...) turns it into a decimal ETH string, which underfunds every transaction by 10¹⁸ (or gets rejected). Feed the raw bigint/string value and drop the now-unused formatEther import. Please fix all occurrences in this file.
-import { formatEther, isAddress } from 'viem';
+import { isAddress } from 'viem';
@@
- const integerValue = formatEther(bigIntValue);
addToBatch({
@@
- value: integerValue,
+ value: bigIntValue.toString(),
@@
- const integerValue = formatEther(bigIntValue);
addToBatch({
@@
- value: integerValue,
+ value: bigIntValue.toString(),
@@
- const integerValue = formatEther(bigIntValue);
if (!tx.to || !isAddress(tx.to)) {
@@
- value: integerValue,
+ value: bigIntValue.toString(),
@@
- const integerValue = formatEther(bigIntValue);
addToBatch({
@@
- value: integerValue,
+ value: bigIntValue.toString(),
@@
- const integerValue = formatEther(bigIntValue);
addToBatch({
@@
- value: integerValue,
+ value: bigIntValue.toString(),Also applies to: 247-255, 272-279, 358-364, 409-414, 437-441
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/TopUpModal.tsx around lines 6, 247-255, 272-279,
358-364, 409-414 and 437-441: the code currently calls formatEther(...) and
passes human-readable ETH strings to addToBatch.value which must be a wei
BigNumberish; this underfunds/rejects transactions. Remove the formatEther
import and all formatEther(...) calls in these locations, instead pass the raw
wei value (string or bigint) directly into addToBatch.value (ensure it's a
BigNumberish type), and update any types/validation to accept the raw wei input;
also remove the unused import of formatEther from the top of the file.
| const handleMaxClick = () => { | ||
| setErrorMsg(null); | ||
| // Since we don't have balance info in SelectedToken, we'll just clear the amount | ||
| // In a real implementation, you'd need to fetch balance for the selected token | ||
| if (!selectedToken) return; | ||
| const portfolioToken = portfolioTokens.find( | ||
| (t) => | ||
| t.contract.toLowerCase() === selectedToken.address.toLowerCase() && | ||
| Number(getChainId(t.blockchain as MobulaChainNames)) === selectedToken.chainId | ||
| ); | ||
| if (portfolioToken && portfolioToken.balance) { | ||
| const maxUsdValue = (Number(portfolioToken.balance) * parseFloat(selectedToken.usdValue)).toString(); | ||
| setAmount(maxUsdValue); | ||
| } |
There was a problem hiding this comment.
MAX button leaves swapAmount at the old value
After hitting MAX we update amount but never recompute swapAmount, so handleTopUp bails on the !swapAmount guard and the CTA silently does nothing. Keep the USD amount and token amount in sync.
- if (portfolioToken && portfolioToken.balance) {
- const maxUsdValue = (Number(portfolioToken.balance) * parseFloat(selectedToken.usdValue)).toString();
- setAmount(maxUsdValue);
- }
+ if (portfolioToken && portfolioToken.balance) {
+ const balance = Number(portfolioToken.balance);
+ const usdPrice = parseFloat(selectedToken.usdValue);
+ if (!Number.isFinite(balance) || !Number.isFinite(usdPrice)) return;
+ setAmount((balance * usdPrice).toString());
+ setSwapAmount(balance);
+ }📝 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 handleMaxClick = () => { | |
| setErrorMsg(null); | |
| // Since we don't have balance info in SelectedToken, we'll just clear the amount | |
| // In a real implementation, you'd need to fetch balance for the selected token | |
| if (!selectedToken) return; | |
| const portfolioToken = portfolioTokens.find( | |
| (t) => | |
| t.contract.toLowerCase() === selectedToken.address.toLowerCase() && | |
| Number(getChainId(t.blockchain as MobulaChainNames)) === selectedToken.chainId | |
| ); | |
| if (portfolioToken && portfolioToken.balance) { | |
| const maxUsdValue = (Number(portfolioToken.balance) * parseFloat(selectedToken.usdValue)).toString(); | |
| setAmount(maxUsdValue); | |
| } | |
| const handleMaxClick = () => { | |
| setErrorMsg(null); | |
| // Since we don't have balance info in SelectedToken, we'll just clear the amount | |
| // In a real implementation, you'd need to fetch balance for the selected token | |
| if (!selectedToken) return; | |
| const portfolioToken = portfolioTokens.find( | |
| (t) => | |
| t.contract.toLowerCase() === selectedToken.address.toLowerCase() && | |
| Number(getChainId(t.blockchain as MobulaChainNames)) === selectedToken.chainId | |
| ); | |
| if (portfolioToken && portfolioToken.balance) { | |
| const balance = Number(portfolioToken.balance); | |
| const usdPrice = parseFloat(selectedToken.usdValue); | |
| if (!Number.isFinite(balance) || !Number.isFinite(usdPrice)) return; | |
| setAmount((balance * usdPrice).toString()); | |
| setSwapAmount(balance); | |
| } | |
| }; |
🤖 Prompt for AI Agents
In src/apps/gas-tank/components/TopUpModal.tsx around lines 311 to 324, the MAX
handler updates the USD amount state but never updates swapAmount, so
handleTopUp exits on the !swapAmount guard; after computing the maxUsdValue
setAmount call, also setSwapAmount to the corresponding token quantity (use
portfolioToken.balance as the token amount string, or compute it by dividing
maxUsdValue by selectedToken.usdValue if needed) so both USD amount and token
amount remain in sync when MAX is clicked.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
UI/UX Improvements
Integrations
Tests
Chores