Skip to content

MTS Fee Calculation#486

Merged
vignesha22 merged 5 commits intostagingfrom
MTS_Fee_Calculation
Jan 7, 2026
Merged

MTS Fee Calculation#486
vignesha22 merged 5 commits intostagingfrom
MTS_Fee_Calculation

Conversation

@vignesha22
Copy link
Contributor

@vignesha22 vignesha22 commented Dec 22, 2025

Description

  • Showed the estimated fee calculation if its USDC for MTS paymaster on top up screen
  • Added some more gas as the gas limit was higher in recent transactions from bundler

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

  • New Features

    • Gasless Fee Estimate panel: shows estimated gas fee (token), total required (top-up + gas), and balance status; displayed only when gasless is supported and a fee asset is selected.
  • Improvements

    • Centralized token-aware balance validation for gasless top-ups; validation runs before proceeding and adjusts checks when fee token matches top-up token.
    • Improved error messaging for gasless vs native flows.
    • Updated (higher) gas estimates for top-up operations.
    • Approval flow now accounts for gasless balance validation and potential fee-token approval needs.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

📝 Walkthrough

Walkthrough

Adds token-aware gasless fee validation and a Gasless Fee Estimate UI to the TopUp flow; increases TopUp-related gas consumption constants in the gasless service. (49 words)

Changes

Cohort / File(s) Summary
Top-up flow & UI
src/apps/pulse/components/Onboarding/TopUpScreen.tsx
Adds validateGaslessFeeBalance and replaces inline checks with it; computes top-up amount in token units, reserves gas when fee token equals top-up token, surfaces tailored insufficient-balance errors; integrates validation into the flow and adds a Gasless Fee Estimate panel showing estimated gas, total needed, and balance.
Gas consumption updates
src/services/gasless.ts
Bumps TopUp-related GasConsumptions: topup_install_modules 500000→610000, topup_install_modules_arb 700000→810000, topup_deposit 500000→610000, topup_deposit_arb 700000→810000, topup_swap 1500000→1610000, topup_swap_arb 1700000→1810000. No other logic changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User as User
    participant UI as TopUpScreen
    participant GasSvc as GaslessService
    participant Validator as validateGaslessFeeBalance
    participant Panel as FeeEstimatePanel

    User->>UI: open TopUp
    UI->>GasSvc: request gas estimate (fetch chain gas price)
    GasSvc-->>UI: return gasUnits + tokenConversion (estimated gas cost)
    UI->>Validator: validateGaslessFeeBalance(topupAmount, feeToken, balances, gasEstimate)
    alt feeToken == topUpToken
        Validator->>Validator: totalNeeded = topupAmount + gasCostInToken
        Validator->>Validator: check balance >= totalNeeded
    else
        Validator->>Validator: compute gasCostInToken
        Validator->>Validator: check feeTokenBalance >= gasCostInToken
    end
    Validator-->>UI: validation result (ok / error + message)
    alt ok
        UI->>Panel: render estimated gas, total needed, current balance
        Panel-->>User: display estimates
        User->>UI: confirm -> proceed (approval/data generation)
    else
        UI-->>User: show error (prevent progression)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Possibly related PRs

Suggested reviewers

  • RanaBug

Poem

🐰 I hopped through gas and token trails,
Counted carrots, weighed the scales,
A tiny panel shows the fee,
Top‑up and gas sit close with me,
Hooray — the balance sings and sails!

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'MTS Fee Calculation' is concise and directly related to the main change: displaying estimated fee calculations for MTS paymaster on the top-up screen.
Description check ✅ Passed The description covers all required sections with adequate detail: it explains the changes made, provides testing evidence including a local test and transaction reference, and specifies the change type as a bug fix.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 22, 2025

Deploying pillarx-debug with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0bb637f
Status: ✅  Deploy successful!
Preview URL: https://1ab6afbf.pillarx-debug.pages.dev
Branch Preview URL: https://mts-fee-calculation.pillarx-debug.pages.dev

View logs

@github-actions github-actions bot temporarily deployed to Preview (MTS_Fee_Calculation) December 22, 2025 16:42 Inactive
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Dec 22, 2025

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 0bb637f
Status: ✅  Deploy successful!
Preview URL: https://3fa60b6f.x-e62.pages.dev
Branch Preview URL: https://mts-fee-calculation.x-e62.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)

420-430: Helpful debug logging for fee estimation.

The detailed logging will aid in debugging gasless fee calculations. Consider gating this behind a debug flag or environment variable in production to reduce console noise.


432-465: Duplicated balance validation logic.

This validation logic (lines 432-465) is duplicated in validateGaslessFeeBalance (lines 619-661). Consider extracting the shared calculation into a helper function to ensure consistency and reduce maintenance burden.

🔎 Proposed refactor: Extract shared calculation
+// Helper to calculate balance requirements for gasless transactions
+const calculateGaslessBalanceRequirements = (
+  userBalance: number,
+  tokenPrice: number,
+  topUpAmountUSD: number,
+  estimatedGasInToken: number,
+  isFeeSameAsToken: boolean
+) => {
+  const topUpAmountInTokens = tokenPrice > 0 ? topUpAmountUSD / tokenPrice : 0;
+  const availableBalanceForFee = isFeeSameAsToken
+    ? userBalance - topUpAmountInTokens
+    : userBalance;
+  const totalNeededInTokens = topUpAmountInTokens + estimatedGasInToken;
+  const hasSufficientBalance = availableBalanceForFee >= estimatedGasInToken;
+  
+  return { topUpAmountInTokens, availableBalanceForFee, totalNeededInTokens, hasSufficientBalance };
+};

Then use this helper in both generateApprovalData and validateGaslessFeeBalance.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5487f1 and af8bbc8.

📒 Files selected for processing (2)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
  • src/services/gasless.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
⏰ 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). (4)
  • GitHub Check: unit-tests
  • GitHub Check: lint
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (3)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)

614-661: Well-documented validation function with clear logic.

The JSDoc comment clearly explains the validation formula. The function correctly handles both scenarios: when fee token matches topup token and when they differ.

Note: The function has a side effect (calling setError) while returning a boolean. This is acceptable for the current use case but could be refactored for better testability by returning an error message or null instead.


698-701: Correct integration of gasless balance validation.

The validation is properly placed before navigating to preview, preventing users from proceeding with insufficient balance.

src/services/gasless.ts (1)

22-27: The gas consumption values are internally consistent and well-documented. All Arbitrum variants correctly maintain the +200,000 gas offset from their mainnet counterparts:

  • topup_install_modules_arb (810,000) = topup_install_modules (610,000) + 200,000 ✓
  • topup_deposit_arb (810,000) = topup_deposit (610,000) + 200,000 ✓
  • topup_swap_arb (1,810,000) = topup_swap (1,610,000) + 200,000 ✓

The comment on line 23 accurately reflects the calculation. All constants are properly used throughout the codebase in the calculateTopUpGasCost function without conflicts.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

♻️ Duplicate comments (2)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)

1016-1020: Unit mismatch: adding USD amount to token amount.

amount is in USD while estimatedGasCostInToken is in token units. This works coincidentally for USDC (≈$1) but would be incorrect for non-stablecoin tokens if gasless support is extended.

The validation logic in validateGaslessFeeBalance correctly converts USD to token units—apply the same conversion here for consistency.


1026-1028: Balance comparison uses same unit mismatch.

The balance check adds parseFloat(amount) (USD) to parseFloat(estimatedGasCostInToken) (token units). Convert amount to token units using tokenPrice as done in validateGaslessFeeBalance.

🧹 Nitpick comments (2)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)

604-652: Duplicated validation logic with generateApprovalData.

The logic in validateGaslessFeeBalance (lines 609-649) duplicates the balance validation in generateApprovalData (lines 420-454):

  • USD to token conversion
  • Fee token same-as-topup check
  • Available balance calculation
  • Error message construction

This duplication creates maintenance risk if one is updated but not the other.

🔎 Suggested refactor: Extract shared validation helper
+interface GaslessFeeValidation {
+  topUpAmountInTokens: number;
+  availableBalanceForFee: number;
+  estimatedGasInToken: number;
+  isFeeSameAsToken: boolean;
+  isValid: boolean;
+}
+
+const calculateGaslessFeeRequirements = (
+  amount: string,
+  selectedFeeAsset: FeeAsset,
+  selectedTokenAddress: string,
+  estimatedGasCostInToken: string
+): GaslessFeeValidation => {
+  const userBalance = selectedFeeAsset.balance ?? 0;
+  const tokenPrice = parseFloat(selectedFeeAsset.tokenPrice || '0') || 1;
+  const estimatedGasInToken = parseFloat(estimatedGasCostInToken);
+  const topUpAmountUSD = parseFloat(amount) || 0;
+  const topUpAmountInTokens = tokenPrice > 0 ? topUpAmountUSD / tokenPrice : 0;
+
+  const isFeeSameAsToken =
+    selectedFeeAsset.asset.contract.toLowerCase() ===
+    selectedTokenAddress.toLowerCase();
+
+  const availableBalanceForFee = isFeeSameAsToken
+    ? userBalance - topUpAmountInTokens
+    : userBalance;
+
+  return {
+    topUpAmountInTokens,
+    availableBalanceForFee,
+    estimatedGasInToken,
+    isFeeSameAsToken,
+    isValid: availableBalanceForFee >= estimatedGasInToken,
+  };
+};

Then use this helper in both generateApprovalData and validateGaslessFeeBalance.


504-506: Hardcoded needsModuleInstall flag.

The comment notes this should come from parent props. If module installation status is known at runtime, consider passing it down to avoid overestimating gas costs.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between af8bbc8 and 87c91cf.

📒 Files selected for processing (1)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
⏰ 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: build
  • GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (2)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)

420-454: Validation logic is sound, but consider extracting to avoid duplication.

The balance validation correctly:

  • Converts USD to token units using tokenPrice
  • Handles the case when fee token equals topup token
  • Provides clear, actionable error messages

This same logic is duplicated in validateGaslessFeeBalance (lines 609-649). See earlier comment about extracting a shared helper.


689-692: Validation gate before preview is appropriate.

Calling validateGaslessFeeBalance before proceeding to preview ensures users see balance errors early. The early return pattern is clean.

@github-actions github-actions bot temporarily deployed to Preview (MTS_Fee_Calculation) January 5, 2026 15:11 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI Agents 🤖
In @src/apps/pulse/components/Onboarding/TopUpScreen.tsx:
- Around line 1001-1048: The UI can display "NaN" when amount is an empty
string; update the calculations inside the Onboarding TopUpScreen JSX (the IIFE
that computes tokenPrice, topUpAmountInTokens, gasInTokens, totalNeededInTokens,
hasEnoughBalance) to guard parseFloat(amount) and
parseFloat(estimatedGasCostInToken) by treating NaN as 0 (e.g., use
(parseFloat(amount) || 0) and (parseFloat(estimatedGasCostInToken) || 0)), keep
the existing tokenPrice fallback, and ensure hasEnoughBalance compares against
the sanitized totalNeededInTokens so the displayed values never render NaN.
🧹 Nitpick comments (2)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)

420-455: Unit conversion is now correct, but consider extracting duplicate validation logic.

The balance validation properly converts USD to token units before comparison, which resolves the unit mismatch issue. However, this validation logic is duplicated in the validateGaslessFeeBalance function (lines 609-652).

💡 Consider extracting shared validation logic

The balance validation logic appears in both generateApprovalData and validateGaslessFeeBalance. Consider extracting it into a shared helper function to reduce duplication and improve maintainability:

const calculateBalanceRequirements = (
  topUpAmountUSD: number,
  tokenPrice: number,
  gasEstimate: number,
  isFeeSameAsToken: boolean,
  userBalance: number
) => {
  const topUpInTokens = tokenPrice > 0 ? topUpAmountUSD / tokenPrice : 0;
  const availableForFee = isFeeSameAsToken 
    ? userBalance - topUpInTokens 
    : userBalance;
  const totalNeeded = topUpInTokens + gasEstimate;
  const isInsufficient = availableForFee < gasEstimate;
  
  return { topUpInTokens, availableForFee, totalNeeded, isInsufficient };
};

Then both functions can use this helper for consistency.


506-506: Hardcoded needsModuleInstall should be made dynamic.

The TODO comment indicates this value should come from parent props to accurately reflect whether module installation is needed. Currently hardcoding true may result in overestimated gas costs for users who already have modules installed.

Do you want me to open a new issue to track passing the actual module installation state from the parent component? This would improve gas estimation accuracy for returning users.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 87c91cf and 2626678.

📒 Files selected for processing (1)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
⏰ 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). (4)
  • GitHub Check: lint
  • GitHub Check: Cloudflare Pages: pillarx-debug
  • GitHub Check: unit-tests
  • GitHub Check: build
🔇 Additional comments (3)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (3)

327-327: Good documentation improvement.

The updated comment clarifies that actual chain gas prices are being fetched for accurate fee estimation, which aligns well with the PR's goal of improving MTS fee calculation accuracy.


604-652: Well-structured validation function.

The validateGaslessFeeBalance function correctly:

  • Converts USD amounts to token units for accurate comparison
  • Handles the case where the fee token is the same as the top-up token
  • Provides clear, informative error messages distinguishing between scenarios
  • Returns early if gasless is not supported

The implementation properly addresses the unit mismatch issue and provides good defensive validation before transaction submission.


689-692: Excellent defensive validation before proceeding.

The validation check ensures users cannot proceed to the preview screen without sufficient balance for both the top-up and gas fees, providing good UX by catching issues early.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/apps/pulse/components/Onboarding/TopUpScreen.tsx:
- Around line 1005-1048: The UI always sums top-up + gas into
totalNeededInTokens which is incorrect when the fee token differs from the
top-up token; update the TopUpScreen display logic to mirror
validateGaslessFeeBalance by computing isFeeSameAsToken (compare
selectedFeeAsset.asset.address or symbol with the top-up token identifier) and
set totalNeededInTokens = gasInTokens + (isFeeSameAsToken ? topUpAmountInTokens
: 0); then compute hasEnoughBalance = selectedFeeAsset.balance >=
totalNeededInTokens, ensure parseFloat fallbacks (already used for tokenPrice,
amount, estimatedGasCostInToken) to avoid NaN, and update the displayed "Total
needed" and balance color logic accordingly.
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2626678 and 6cfeb4d.

⛔ Files ignored due to path filters (2)
  • src/apps/pillarx-app/components/TokenMarketDataRow/tests/__snapshots__/LeftColumnTokenMarketDataRow.test.tsx.snap is excluded by !**/*.snap
  • src/apps/pillarx-app/components/TokensWithMarketDataTile/test/__snapshots__/TokensWithMarketDataTile.test.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (1)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
⏰ 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). (4)
  • GitHub Check: unit-tests
  • GitHub Check: lint
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (3)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (3)

327-327: LGTM - Clearer comment.

The updated comment better describes the purpose of fetching the gas price for accurate fee estimation.


604-652: Guard against NaN when amount is empty.

If amount is an empty string, parseFloat(amount) at line 620 returns NaN, which would flow through the calculations. While this might not cause immediate errors (NaN comparisons typically return false, so validation would fail gracefully), it's cleaner to explicitly handle this case.

🛡️ Proposed fix for NaN handling
   // Convert topup amount (USD) to token units
   const topUpAmountUSD = parseFloat(amount) || 0;
   const topUpAmountInTokens =
-    tokenPrice > 0 ? topUpAmountUSD / tokenPrice : 0;
+    tokenPrice > 0 ? topUpAmountUSD / tokenPrice : 0;

Actually, the current code already handles this reasonably at line 620 with || 0. No change needed here—the validation logic is safe.


689-691: LGTM - Proper validation before proceeding.

The gasless fee balance validation is correctly placed before showing the preview screen, ensuring users cannot proceed without sufficient balance.

@github-actions github-actions bot temporarily deployed to Preview (MTS_Fee_Calculation) January 7, 2026 12:23 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/apps/pulse/components/Onboarding/TopUpScreen.tsx:
- Around line 1005-1049: The UI always adds topUpAmountInTokens to gasInTokens
when computing totalNeededInTokens and hasEnoughBalance, but that only applies
when the fee token equals the top-up token; mirror the validation logic (see
validateGaslessFeeBalance and generateApprovalData which use isFeeSameAsToken).
Fix by computing totalNeededInTokens = (isFeeSameAsToken ? topUpAmountInTokens +
gasInTokens : gasInTokens) and use that value for the displayed "Total needed"
and for hasEnoughBalance (selectedFeeAsset.balance >= totalNeededInTokens), so
the displayed amount and balance color match the actual validation logic.
🧹 Nitpick comments (1)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (1)

420-455: Consider extracting duplicated balance validation logic.

The balance validation logic here (lines 420-455) is nearly identical to the logic in validateGaslessFeeBalance (lines 615-649). Both functions:

  • Calculate topUpAmountInTokens from USD amount
  • Check isFeeSameAsToken
  • Calculate availableBalanceForFee
  • Validate balance and set identical error messages

This duplication makes maintenance harder and increases the risk of the two implementations diverging.

♻️ Suggested refactor to eliminate duplication

Extract the common logic into a helper function that both can use:

// Helper to calculate balance requirements for gasless transactions
const calculateGaslessBalanceRequirements = (
  selectedFeeAsset: FeeAsset,
  selectedToken: SelectedToken | null,
  amount: string,
  estimatedGasCostInToken: string
) => {
  const userBalance = selectedFeeAsset.balance ?? 0;
  const tokenPrice = parseFloat(selectedFeeAsset.tokenPrice || '0') || 1;
  const estimatedGasInToken = parseFloat(estimatedGasCostInToken) || 0;
  
  const topUpAmountUSD = parseFloat(amount) || 0;
  const topUpAmountInTokens = tokenPrice > 0 ? topUpAmountUSD / tokenPrice : 0;
  
  const isFeeSameAsToken =
    selectedFeeAsset.asset.contract.toLowerCase() ===
    selectedToken?.address.toLowerCase();
  
  let availableBalanceForFee = userBalance;
  if (isFeeSameAsToken) {
    availableBalanceForFee = userBalance - topUpAmountInTokens;
  }
  
  const totalNeededInTokens = topUpAmountInTokens + estimatedGasInToken;
  const hasSufficientBalance = availableBalanceForFee >= estimatedGasInToken;
  
  return {
    userBalance,
    topUpAmountInTokens,
    estimatedGasInToken,
    totalNeededInTokens,
    isFeeSameAsToken,
    hasSufficientBalance
  };
};

Then both generateApprovalData and validateGaslessFeeBalance can call this helper and format their error messages accordingly.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6cfeb4d and 0bb637f.

📒 Files selected for processing (1)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-12T07:42:24.656Z
Learnt from: IAmKio
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
⏰ 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). (4)
  • GitHub Check: unit-tests
  • GitHub Check: lint
  • GitHub Check: Cloudflare Pages: pillarx-debug
  • GitHub Check: build
🔇 Additional comments (3)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (3)

327-327: LGTM: Clear comment for gas price fetching.

The comment accurately describes the purpose of fetching the gas price from the chain for fee estimation.


604-652: Good centralization of validation logic.

The validateGaslessFeeBalance function properly centralizes the gasless balance validation with correct logic for handling the case when fee token equals topup token (lines 625-633). The JSDoc is clear and the function signature is clean.

Note: The duplication between this function and generateApprovalData is addressed in the previous comment.


689-692: LGTM: Proper validation before proceeding with topup.

The call to validateGaslessFeeBalance correctly validates the balance before proceeding, with appropriate early return if validation fails.

@vignesha22 vignesha22 merged commit 6703f36 into staging Jan 7, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants