Skip to content

PRO-3886-MTS Support for Gas Tank Top Up#484

Merged
vignesha22 merged 3 commits intostagingfrom
PRO-3886-MTS_Support_On_TopUpGasTank
Dec 22, 2025
Merged

PRO-3886-MTS Support for Gas Tank Top Up#484
vignesha22 merged 3 commits intostagingfrom
PRO-3886-MTS_Support_On_TopUpGasTank

Conversation

@vignesha22
Copy link
Contributor

@vignesha22 vignesha22 commented Dec 18, 2025

Description

  • If MTS is supported for the selected token in top up then it will pay using that token

How Has This Been Tested?

  • Tested Locally

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 top-up flows: users can pay gas with supported tokens via paymaster support; integrated approval step, token-based gas estimates, and improved transaction tracking.
    • UI updates: shows token-based gas estimate, spinner while estimating, and native-gas fallback.
  • Utilities

    • Added gas-cost calculator and expanded gas-consumption entries for more accurate top-up estimates across chains.
  • Chores

    • Wiring, validation and fallback improvements for more reliable gasless handling.

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 18, 2025

Walkthrough

Adds gasless/paymaster support to Pulse top-up: new FeeAsset types and props, gas cost calculation, approval calldata generation, paymaster injection into batch sends, executeTopUp returning { userOperationHash }, and UI updates to show token-based gas estimates.

Changes

Cohort / File(s) Summary
Onboarding UI & preview
src/apps/pulse/components/Onboarding/TopUpScreen.tsx, src/apps/pulse/components/Onboarding/PreviewTopUp.tsx
Added gasless-related state/props (isGaslessSupported, selectedFeeAsset, approveData, paymasterAddress, estimatedGasCostInToken, gasPrice), effects to fetch/select paymaster and fee asset, approval calldata generation, propagate gasless data into preview, and conditional token-based gas fee UI.
Top-up execution hook
src/apps/pulse/hooks/useTopUp.ts
Added FeeAsset type, extended TopUpParams with gasless fields, changed executeTopUp return to `Promise<{ userOperationHash: string }
Gas cost calculation util
src/apps/pulse/utils/gasCalculation.ts
New calculateTopUpGasCost({ chainId, needsModuleInstall, needsSwap }) computing total gas using chain-specific GasConsumptions entries.
Gas consumption constants
src/services/gasless.ts
Added six top-up gas consumption entries: topup_install_modules, topup_install_modules_arb, topup_deposit, topup_deposit_arb, topup_swap, topup_swap_arb.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant User
    participant TopUpScreen
    participant PaymasterService
    participant useTopUp
    participant BatchService
    participant Blockchain

    User->>TopUpScreen: select token & amount
    TopUpScreen->>PaymasterService: fetch paymaster & fee asset
    PaymasterService-->>TopUpScreen: paymasterDetails + selectedFeeAsset
    TopUpScreen->>TopUpScreen: calculateTopUpGasCost() & generateApprovalData()
    TopUpScreen-->>User: show gas estimate (token or native)

    User->>TopUpScreen: confirm top-up
    TopUpScreen->>useTopUp: executeTopUp(params + gasless fields)

    alt Gasless flow
        useTopUp->>BatchService: add approval txn (ERC20 approve)
        useTopUp->>BatchService: add top-up txn
        useTopUp->>BatchService: inject paymasterDetails
    else Native flow
        useTopUp->>BatchService: add top-up txn
    end

    BatchService->>Blockchain: execute batch
    Blockchain-->>BatchService: { userOperationHash }
    BatchService-->>useTopUp: { userOperationHash }
    useTopUp-->>TopUpScreen: { userOperationHash }
    TopUpScreen-->>User: surface transaction hash/status
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Verify all call sites handle new executeTopUp return shape { userOperationHash }.
  • Review approval calldata generation and ERC20 approve amount logic in TopUpScreen.
  • Validate paymasterDetails formatting/normalization passed into batch send.
  • Inspect effect dependencies and async race conditions when fetching paymasters and computing gas.
  • Confirm fallbacks for non-gasless flows and batch cleanup on errors.

Possibly related PRs

Suggested reviewers

  • IAmKio
  • RanaBug

Poem

🐇 A rabbit scampers through the code so light,
Paymaster in pocket, gasless feels right.
Approve a token, then the top-up sings,
Batches hop onward with tiny wings,
Hooray — wallets top up through the night!

Pre-merge checks and finishing touches

❌ 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 clearly summarizes the main change: adding MTS (Meta Transaction Service) support for gas tank top-up payments using selected tokens.
Description check ✅ Passed The description covers the main feature goal and includes testing information, though specific testing details are minimal and change type is selected, meeting basic requirements.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch PRO-3886-MTS_Support_On_TopUpGasTank

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 18, 2025

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 55760eb
Status: ✅  Deploy successful!
Preview URL: https://9d308e65.x-e62.pages.dev
Branch Preview URL: https://pro-3886-mts-support-on-topu.x-e62.pages.dev

View logs

@cloudflare-workers-and-pages
Copy link

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

Deploying pillarx-debug with  Cloudflare Pages  Cloudflare Pages

Latest commit: 55760eb
Status: ✅  Deploy successful!
Preview URL: https://c7d79d6a.pillarx-debug.pages.dev
Branch Preview URL: https://pro-3886-mts-support-on-topu.pillarx-debug.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 (5)
src/apps/pulse/utils/gasCalculation.ts (1)

1-35: LGTM! Consider extracting the Arbitrum chain ID to a constant.

The function logic is correct and follows a clean pattern. For maintainability, consider extracting the Arbitrum chain ID 42161 to a shared constant, especially if it's used elsewhere in the codebase.

🔎 Optional: Extract magic number
+const ARBITRUM_CHAIN_ID = 42161;
+
 export const calculateTopUpGasCost = ({
   chainId,
   needsModuleInstall,
   needsSwap,
 }: {
   chainId: number;
   needsModuleInstall: boolean;
   needsSwap: boolean;
 }): number => {
-  const isArbitrum = chainId === 42161;
+  const isArbitrum = chainId === ARBITRUM_CHAIN_ID;
src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (1)

74-79: Avoid using any type for selectedFeeAsset.

Using any bypasses TypeScript's type checking. Consider defining a proper interface for the fee asset structure to improve type safety and enable better IDE support.

🔎 Suggested type definition
interface FeeAsset {
  id: string;
  type: string;
  title: string;
  imageSrc: string;
  chainId: number;
  value: string;
  price: string;
  balance: number;
  tokenPrice?: string;
  decimals: number;
  asset: {
    name: string;
    symbol: string;
    contract: string;
    decimals: number;
    balance: number;
    price?: number;
  };
  paymasterAddress: string;
}

Then update:

-  selectedFeeAsset?: any;
+  selectedFeeAsset?: FeeAsset;
src/apps/pulse/hooks/useTopUp.ts (1)

28-33: Avoid using any type for selectedFeeAsset.

Same concern as in PreviewTopUp.tsx. Define a shared interface for FeeAsset and import it in both files to ensure type safety and consistency.

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

76-83: Avoid using any type for selectedFeeAsset.

This is the third occurrence of any typing for this object. Consider creating a shared FeeAsset type definition in a common types file and importing it across all three files.


422-448: Missing ESLint exhaustive-deps comment.

The effect has intentionally omitted dependencies (isSelectedTokenUSDC, generateApprovalData). Add an ESLint disable comment to document this intentional omission, similar to other effects in this file.

🔎 Suggested fix
     // Generate approval data and calculate cost in token
     generateApprovalData(totalGasCost).catch(console.error);
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [isGaslessSupported, selectedFeeAsset, gasPrice, selectedToken]);
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0cd220 and e72fafe.

📒 Files selected for processing (5)
  • src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (5 hunks)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx (8 hunks)
  • src/apps/pulse/hooks/useTopUp.ts (6 hunks)
  • src/apps/pulse/utils/gasCalculation.ts (1 hunks)
  • src/services/gasless.ts (1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/hooks/useTopUp.ts
  • src/apps/pulse/utils/gasCalculation.ts
  • src/apps/pulse/components/Onboarding/PreviewTopUp.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
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧬 Code graph analysis (3)
src/apps/pulse/utils/gasCalculation.ts (1)
src/services/gasless.ts (1)
  • GasConsumptions (14-28)
src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (2)
src/apps/pulse/utils/number.ts (1)
  • truncateDecimals (55-68)
src/utils/number.tsx (2)
  • formatExponentialSmallNumber (119-149)
  • limitDigitsNumber (42-69)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)
src/services/gasless.ts (2)
  • getAllGaslessPaymasters (30-63)
  • Paymasters (7-12)
src/apps/pulse/utils/gasCalculation.ts (1)
  • calculateTopUpGasCost (3-35)
⏰ 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: unit-tests
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (8)
src/services/gasless.ts (1)

21-28: LGTM!

The new top-up gas cost constants follow the established naming pattern and maintain consistency with existing Arbitrum variant calculations.

src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (2)

251-263: LGTM!

The result handling correctly extracts userOperationHash from the new object structure, and the gas fee string calculation properly differentiates between gasless (token-based) and native gas payment paths.


756-769: LGTM!

The conditional rendering correctly displays gasless token-based fees when available and falls back to native gas display otherwise.

src/apps/pulse/hooks/useTopUp.ts (2)

207-229: LGTM!

The approval transaction is correctly placed first in the batch to ensure the paymaster can deduct fees before other transactions execute. The conditional checks are appropriate.


289-313: Verify paymaster context mode against service documentation.

The paymaster context uses mode 'commonerc20' which does not appear in standard ERC-20 paymaster API documentation (Etherspot Arka, Biconomy, Pimlico). Confirm this mode is correct for your paymaster service, or verify if it should be 'erc20' or another supported mode.

src/apps/pulse/components/Onboarding/TopUpScreen.tsx (3)

374-410: Gas cost calculation uses floating-point arithmetic.

The estimation converts through float operations before creating the approval amount. This aligns with the team's accepted approach for token calculations. Based on learnings, the team has accepted JavaScript's decimal place limitations for their use case.


649-667: LGTM!

The gasless-related props are correctly passed to PreviewTopUp, enabling the gasless transaction flow when supported.


569-573: LGTM!

Good defensive validation that prevents proceeding with gasless flow when approval data couldn't be generated (e.g., due to insufficient token balance for gas fees).

Comment on lines +435 to +444
// Check if modules need to be installed (need access to state from parent)
// For now, assume module installation may be needed
const needsModuleInstall = true; // This should come from parent props
const needsSwap = !isSelectedTokenUSDC();

const totalGasCost = calculateTopUpGasCost({
chainId: selectedToken.chainId,
needsModuleInstall,
needsSwap,
});
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 18, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Hardcoded needsModuleInstall = true may cause over-estimation.

The comment acknowledges this should come from parent props. With it hardcoded to true, users will always approve gas fees for module installation even when modules are already installed, potentially over-approving tokens to the paymaster.

Consider passing areModulesInstalled from PreviewTopUp or lifting the module installation check to this component.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@vignesha22 check here as well

Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just for gas estimation done from the preconfigured gas amounts stating to include that step of gas amount too so that even if that step isn't there the balance check would be valid

@vignesha22 vignesha22 requested review from IAmKio and RanaBug December 18, 2025 14:07
@github-actions github-actions bot requested a deployment to Preview (PRO-3886-MTS_Support_On_TopUpGasTank) December 18, 2025 14:07 Abandoned
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

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

447-456: Hardcoded needsModuleInstall = true causes gas over-estimation.

As noted in the comment, this value should be derived from actual module installation state. With it hardcoded to true, gas costs will always include module installation overhead even when modules are already installed.

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

39-53: Duplicate FeeAsset interface definition across files.

The FeeAsset interface is defined identically in TopUpScreen.tsx (lines 39-53), useTopUp.ts (lines 21-35), and PreviewTopUp.tsx (lines 63-77). This violates DRY and creates maintenance burden.

Consider extracting to a shared types file:
// src/apps/pulse/types/gasless.ts
export interface FeeAsset {
  decimals: number;
  balance: number;
  tokenPrice?: string;
  asset: {
    symbol: string;
    contract: string;
    name: string;
    decimals: number;
    balance: number;
    price?: number;
  };
  paymasterAddress?: string;
  [key: string]: unknown;
}

Then import it in all three files.

Also applies to: 63-77

src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (1)

692-715: Nested ternary reduces readability.

The conditional rendering logic works correctly but uses a nested ternary that requires an eslint-disable comment. Consider extracting to a helper function for better maintainability.

Extract to a render function:
+  const renderGasFeeContent = () => {
+    if (isEstimatingGas) {
+      return (
+        <div className="flex items-center">
+          <TailSpin color="#FFFFFF" height={12} width={12} />
+        </div>
+      );
+    }
+    if (showGaslessGasPrice) {
+      return (
+        <span className="text-white text-[13px] tracking-tight">
+          ≈ {truncateDecimals(estimatedGasCostInToken, 6)}{' '}
+          {selectedFeeAsset.asset.symbol}
+        </span>
+      );
+    }
+    return (
+      <span className="text-white text-[13px] tracking-tight">
+        {nativeGasFeeDisplay}
+      </span>
+    );
+  };
+
+  const gasFeeContent = renderGasFeeContent();
-  // eslint-disable-next-line no-nested-ternary
-  const gasFeeContent = isEstimatingGas ? (
-    ...
-  );
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e72fafe and ae65701.

📒 Files selected for processing (3)
  • src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (7 hunks)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx (8 hunks)
  • src/apps/pulse/hooks/useTopUp.ts (6 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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/PreviewTopUp.tsx
  • src/apps/pulse/hooks/useTopUp.ts
📚 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-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧬 Code graph analysis (3)
src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (2)
src/apps/pulse/utils/number.ts (1)
  • truncateDecimals (55-68)
src/utils/number.tsx (2)
  • formatExponentialSmallNumber (119-149)
  • limitDigitsNumber (42-69)
src/apps/pulse/hooks/useTopUp.ts (3)
src/apps/pulse/types/tokens.ts (1)
  • SelectedToken (1-10)
src/apps/pulse/hooks/useRelaySell.ts (1)
  • SellOffer (37-43)
src/services/tokensData.ts (1)
  • Token (19-29)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)
src/services/gasless.ts (2)
  • getAllGaslessPaymasters (30-63)
  • Paymasters (7-12)
src/apps/pulse/utils/gasCalculation.ts (1)
  • calculateTopUpGasCost (3-35)
⏰ 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 (11)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (3)

582-586: LGTM!

The validation correctly prevents proceeding with the top-up when gasless is enabled but approval data generation failed (likely due to insufficient balance). The error message is appropriately set in generateApprovalData.


673-678: LGTM!

The gasless-related props are properly threaded to the PreviewTopUp component, enabling the downstream component to handle gasless transaction execution.


264-346: No action needed—the dependency array is correctly scoped.

The getGasPrice function from useDeployWallet doesn't need to be in the dependency array since it's only called within the effect and isn't used as a dependency for optimizations or prop passing. The eslint-disable comment appropriately suppresses a false positive; adding getGasPrice to the array would cause unnecessary re-runs on every render because the function gets a new reference each time (it's not memoized). The effect is correctly keyed to selectedToken and portfolioTokens, which are the actual values that determine when fetching should occur.

src/apps/pulse/hooks/useTopUp.ts (4)

203-211: Guard condition updated to allow gasless-only batches.

The guard at line 206 now includes !approveData to allow proceeding when there are no top-up transactions but there is approval data for gasless. This correctly supports the scenario where only an approval transaction is needed.


223-245: LGTM!

The approval transaction is correctly inserted first in the batch, ensuring the paymaster has the necessary token allowance before executing subsequent transactions. The debug logging is helpful for troubleshooting.


305-323: Verify that paymasterDetails matches the paymaster API contract for the service at VITE_PAYMASTER_URL.

The code constructs paymasterDetails with mode: 'commonerc20' inside the context. Standard ERC-20 paymasters use paymasterContext: { token: address } without a mode field. Confirm that your paymaster service expects this custom mode field and commonerc20 value, or adjust to match your paymaster provider's API specification.


177-180: Return type changed from string | null to { userOperationHash: string } | null.

This is a breaking change to the executeTopUp API. Callers must now access result?.userOperationHash instead of using the result directly as a string. All usages have been updated accordingly.

src/apps/pulse/components/Onboarding/PreviewTopUp.tsx (4)

253-268: Correctly handles new executeTopUp return type.

The result is properly destructured to access userOperationHash (line 267), aligning with the updated return type from useTopUp.ts. The gasless props are correctly passed to the execution function.


273-279: LGTM!

The gas fee string construction correctly prioritizes gasless token display when available, falling back to native gas display. The formatting utilities are appropriately applied to each case.


693-694: LGTM!

The showGaslessGasPrice flag correctly requires all three conditions to be truthy, ensuring the gasless gas fee is only displayed when complete data is available.


108-113: LGTM!

The gasless props are correctly destructured with sensible defaults (isGaslessSupported = false), ensuring backward compatibility if the parent component doesn't provide these props.

@github-actions github-actions bot temporarily deployed to Preview (PRO-3886-MTS_Support_On_TopUpGasTank) December 18, 2025 14:57 Inactive
markOnboardingComplete: () => void;
// Gasless transaction support
isGaslessSupported?: boolean;
selectedFeeAsset?: FeeAsset | null;
Copy link
Collaborator

Choose a reason for hiding this comment

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

why null here since selectedFeeAsset can also be undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are the ones who is gonna set it so if it's null or undefined on preview screen it won't go through those checks are in place

setSelectedPaymasterAddress('');
}
} catch (err) {
console.error('Failed to fetch gasless paymasters:', err);
Copy link
Collaborator

Choose a reason for hiding this comment

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

shouldn't we informed the user as well on the UI of this error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If there is error on fetching then the selectedFeeAsset won't be populated and gasless flag would be false which makes the transaction go through with native token

}
}
} catch (err) {
console.error('Failed to generate approval data:', err);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Once again how do we inform the user about this?

Comment on lines +435 to +444
// Check if modules need to be installed (need access to state from parent)
// For now, assume module installation may be needed
const needsModuleInstall = true; // This should come from parent props
const needsSwap = !isSelectedTokenUSDC();

const totalGasCost = calculateTopUpGasCost({
chainId: selectedToken.chainId,
needsModuleInstall,
needsSwap,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

@vignesha22 check here as well

});

// Generate approval data and calculate cost in token
generateApprovalData(totalGasCost).catch(console.error);
Copy link
Collaborator

Choose a reason for hiding this comment

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

is that a correct way to genereateApprovslData? What about is fails, how is the user informed about that?

sellOffer: SellOffer | null; // Only needed for non-USDC tokens
userPortfolio?: Token[];
additionalTransactions?: Transactions[];
// Gasless transaction support
Copy link
Collaborator

Choose a reason for hiding this comment

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

We already have this type of interface somewhere else no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we have it in sendModalTabView not anywhere else

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

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

479-488: Verify hardcoded needsModuleInstall = true is intentional.

The comment acknowledges this should ideally come from parent props. The past review discussion indicates this ensures sufficient balance for gas fees even when modules are already installed. However, this conservative approach may lead to:

  1. Users being blocked from transactions when they have sufficient funds for the actual operation but not for the over-estimated gas
  2. Larger approval amounts to the paymaster than necessary

Consider accepting areModulesInstalled as a prop from PreviewTopUp or lifting the module check to this component for more accurate gas estimation.

Based on past review comments, this was previously flagged and discussed.

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

328-329: Consider logging when gas price fetch fails.

The gas price fetch silently fails if getGasPrice returns null/undefined. While the downstream effect at line 471 prevents progression when gasPrice is empty, adding a console.error here would help with debugging.

🔎 Suggested improvement
 const price = await getGasPrice(selectedToken.chainId);
-if (price) setGasPrice(price);
+if (price) {
+  setGasPrice(price);
+} else {
+  console.error('Failed to fetch gas price for chainId:', selectedToken.chainId);
+}

361-464: Consider refactoring for better separation of concerns.

The generateApprovalData function handles multiple responsibilities: fetching price data, calculating costs, validating balance, and generating approval calldata. While functional, splitting this into smaller focused functions would improve testability and maintainability.

Potential structure:

  • fetchNativePrice(chainId) - fetch and validate price
  • calculateGasCostInToken(gasCost, gasPrice, nativePrice, tokenPrice) - pure calculation
  • validateTokenBalance(required, available) - validation logic
  • generateApprovalCalldata(token, spender, amount) - calldata generation
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae65701 and 55760eb.

📒 Files selected for processing (1)
  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx (8 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 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
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 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/pulse/components/Onboarding/TopUpScreen.tsx
📚 Learning: 2025-11-04T14:34:00.373Z
Learnt from: RanaBug
Repo: pillarwallet/x PR: 445
File: src/containers/Authorized.tsx:63-77
Timestamp: 2025-11-04T14:34:00.373Z
Learning: In src/containers/Authorized.tsx, the tempKit useEffect intentionally excludes chainId from its dependency array because the kit is used by useWalletModeVerification to check wallet mode across all supported chains, regardless of the currently selected chainId. The chainId parameter is only used for initial kit configuration and does not affect the multi-chain verification results.

Applied to files:

  • src/apps/pulse/components/Onboarding/TopUpScreen.tsx
🧬 Code graph analysis (1)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (2)
src/services/gasless.ts (2)
  • getAllGaslessPaymasters (30-63)
  • Paymasters (7-12)
src/apps/pulse/utils/gasCalculation.ts (1)
  • calculateTopUpGasCost (3-35)
⏰ 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: unit-tests
  • GitHub Check: build
  • GitHub Check: Cloudflare Pages: pillarx-debug
🔇 Additional comments (7)
src/apps/pulse/components/Onboarding/TopUpScreen.tsx (7)

3-4: LGTM: Import additions support gasless functionality.

The new imports for ethers, viem, useDeployWallet, gasless services, and gas calculation utilities are appropriate for implementing gasless transaction support.

Also applies to: 21-21, 24-27, 37-37


39-53: LGTM: FeeAsset interface is well-structured.

The interface appropriately captures token metadata, balance, price, and paymaster information needed for gasless transactions.


86-95: LGTM: State variables and hook usage are appropriate.

The new state variables properly track gasless transaction state, and the useDeployWallet hook integration provides the necessary gas price data.

Also applies to: 103-103


378-406: LGTM: Response validation properly implemented.

The HTTP response validation and price data checks (lines 383-406) properly address the previous review concern about parsing JSON without status checks. The error handling now sets user-facing error messages and clears approval data on failure.

Based on past review comments, this issue has been resolved.


614-618: LGTM: Gasless validation properly implemented.

The validation ensures approval data exists when using gasless transactions, with appropriate error handling already set by generateApprovalData.


705-710: LGTM: Gasless props properly passed to PreviewTopUp.

All necessary gasless transaction data is appropriately threaded through to the preview component.


264-346: Configuration and type definitions are properly in place and aligned.

The implementation correctly handles paymaster API integration with proper defensive patterns:

  • VITE_PAYMASTER_URL is configured in .env.example and accessed via import.meta.env.VITE_PAYMASTER_URL
  • Paymasters type definition precisely matches the expected API response structure (gasToken, chainId, epVersion, paymasterAddress)
  • getAllGaslessPaymasters includes error handling that returns null on fetch failures, with TopUpScreen gracefully falling back to native token
  • The function filters results by chainId and token support, ensuring only valid paymasters are returned

No action required—the code already addresses deployment availability concerns through proper configuration and error handling.

@vignesha22 vignesha22 merged commit c5487f1 into staging Dec 22, 2025
7 of 8 checks passed
@vignesha22 vignesha22 deleted the PRO-3886-MTS_Support_On_TopUpGasTank branch December 22, 2025 14:12
@coderabbitai coderabbitai bot mentioned this pull request Dec 22, 2025
3 tasks
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