Skip to content

fix/paymaster-batch-cleanup#408

Merged
RanaBug merged 1 commit intostagingfrom
fix/paymaster-batch-cleanup
Sep 24, 2025
Merged

fix/paymaster-batch-cleanup#408
RanaBug merged 1 commit intostagingfrom
fix/paymaster-batch-cleanup

Conversation

@RanaBug
Copy link
Collaborator

@RanaBug RanaBug commented Sep 24, 2025

Description

  • Fix for batches cleanup if gasless transaction (batch) fails

How Has This Been Tested?

  • Existing Unit Tests and Manual Testing

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

  • Bug Fixes
    • Internal paymaster batches are no longer shown in batch lists or counted, ensuring accurate batch indicators and cleaner views.
    • Automatic cleanup of failed or stale paymaster batches across multiple error scenarios prevents phantom batches and stuck states.
    • Improved grouping logic in the batches tab for more consistent display.
    • More reliable send flow in the tokens tab by proactively removing conflicting paymaster batches before creation and after errors, reducing confusion and preventing unintended reuse.

@RanaBug RanaBug requested a review from IAmKio September 24, 2025 13:30
@RanaBug RanaBug self-assigned this Sep 24, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

Adds explicit handling for an internal paymaster batch named "paymaster-batch": it is filtered out from grouping/counting/rendering in batch views and provider logic, and proactively cleaned up in SendModalTokensTabView during creation and on multiple error paths in the paymaster flow. No exported interface changes.

Changes

Cohort / File(s) Summary
SendModal Batches View Filtering
src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx
Skips batches named "paymaster-batch" when grouping batches by chainId, in addition to existing "pulse-sell" skip.
SendModal Tokens View Paymaster Cleanup
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx
Proactively removes any existing "paymaster-batch" before creating a new one; adds cleanup of "paymaster-batch" on multiple error paths (estimation, send, missing UserOp hash, other failures) in non-payload paymaster flow.
Global Batch Provider Filtering
src/providers/GlobalTransactionsBatchProvider.tsx
Excludes "paymaster-batch" from batch counts alongside "pulse-sell"; updates comments to document internal status and exclusion rationale.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant UI as SendModalTokensTabView
  participant BM as BatchManager
  participant EST as Estimator
  participant TX as Sender

  U->>UI: Initiate paymaster send (non-payload)
  UI->>BM: Delete batch "paymaster-batch" (pre-clean)
  UI->>BM: Create batch "paymaster-batch"
  UI->>EST: Estimate "paymaster-batch"
  alt Estimation fails
    EST-->>UI: Error
    UI->>BM: Delete batch "paymaster-batch" (cleanup)
    UI-->>U: Show error
  else Estimation succeeds
    UI->>TX: Send "paymaster-batch"
    alt Send fails or missing UserOp hash
      TX-->>UI: Error
      UI->>BM: Delete batch "paymaster-batch" (cleanup)
      UI-->>U: Show error
    else Send succeeds
      TX-->>UI: Success
      UI-->>U: Confirm sent
    end
  end

  note over UI,BM: "paymaster-batch" is internal and excluded from UI lists/counts
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • userOp states fix #322 — Also adjusts SendModalTokensTabView to reset send-related state and clean up stale userOp/paymaster batch artifacts.
  • PRO-3185-Gas_Abstraction #308 — Introduces paymaster/gasless flows and batch creation; current PR adds guards and cleanup for the "paymaster-batch" within that flow.

Suggested reviewers

  • IAmKio
  • vignesha22

Poem

A bunny tidies batches, hop by hop,
Sweeps “paymaster-batch” before it flops.
If errors sprout, it whisks away,
So shiny queues can start their day.
Two ears up—clean flows dispatch!
No ghosts remain, not even a batch. 🐇✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title "fix/paymaster-batch-cleanup" succinctly and accurately captures the primary change—fixing paymaster batch cleanup—and aligns with the changes in batching and cleanup logic across the touched files. It is concise and clear enough for a reviewer scanning PR history to understand the main purpose.
Description Check ✅ Passed The PR description follows the repository template and includes a Description, How Has This Been Tested, and Types of changes with the bugfix box checked, and it notes both unit and manual testing. It is concise and covers the required sections, though it would be more helpful if it listed affected files, specific test steps or unit test names, and any reproduction or failure details.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/paymaster-batch-cleanup

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

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5032e5a
Status: ✅  Deploy successful!
Preview URL: https://5a967e8e.x-e62.pages.dev
Branch Preview URL: https://fix-paymaster-batch-cleanup.x-e62.pages.dev

View logs

@RanaBug RanaBug merged commit a430b17 into staging Sep 24, 2025
5 of 6 checks passed
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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (2)

1598-1638: Missing cleanup on cost-warning early return leaves stale paymaster-batch

If the “cost higher than amount” warning triggers, we return without removing the internal batch. This leaves a stale paymaster-batch until the next send attempt (where it’s cleared at the start). Clean it here too.

Apply this diff:

         if (
           !ignoreSafetyWarning &&
           amountInFiat &&
           costAsFiat > amountInFiat
         ) {
           setSafetyWarningMessage(
             t`warning.transactionSafety.costHigherThanAmount`
           );
           setIsSending(false);
           transactionDebugLog(
             'Batch cost warning: estimated cost higher than amount'
           );
-          return;
+          try {
+            kit.batch({ batchName }).remove();
+          } catch (e) {
+            console.warn('Failed to clear paymaster batch after cost warning', e);
+          }
+          return;
         }

1979-2002: Ensure paymaster-batch is cleared on any thrown error in onSend

If an exception occurs anywhere in the paymaster path, we currently don’t remove the internal batch in this catch. Add a best-effort cleanup here to cover unforeseen throw paths.

Apply this diff:

     } catch (error: unknown) {
+      // Best-effort cleanup for internal paymaster batch on any thrown error
+      if (isPaymaster) {
+        try {
+          kit.batch({ batchName: 'paymaster-batch' }).remove();
+        } catch (e) {
+          console.warn('Failed to clear paymaster batch in catch()', e);
+        }
+      }
       Sentry.captureException(error, {
         tags: {
           component: 'send_flow',
           action: 'send_error',
           sendId,
         },
🧹 Nitpick comments (6)
src/providers/GlobalTransactionsBatchProvider.tsx (1)

79-85: Exclude internal ‘paymaster-batch’ from counts; consider centralizing the sentinel name

Filtering out paymaster-batch here is correct and aligned with the PR goal. To avoid string drift across files, consider centralizing the sentinel in a shared constant (e.g., PAYMASTER_BATCH_NAME) or a small helper isInternalBatch(name) used here and in views.

src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (1)

729-736: Also exclude ‘paymaster-batch’ in the render-time filter for defense-in-depth

You already skip it when grouping, but adding the check here prevents future regressions if the grouping prefilter changes.

Apply this diff:

           .filter(
             ({ batchName }) =>
               batchName &&
               batchName !== 'batch-undefined' &&
-              !batchName.includes('pulse-sell')
+              !batchName.includes('pulse-sell') &&
+              batchName !== 'paymaster-batch'
           )
src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (4)

1516-1521: Good: proactive pre-clean before creating a new paymaster batch

This avoids stale batches. Wrap remove() in a try/catch to make cleanup best-effort and avoid bubbling an exception that would skip the send flow.

Apply this diff:

-        if (existingBatches[batchName]) {
-          kit.batch({ batchName }).remove();
-        }
+        if (existingBatches[batchName]) {
+          try {
+            kit.batch({ batchName }).remove();
+          } catch (e) {
+            console.warn('Failed to clear existing paymaster batch', e);
+          }
+        }

1728-1746: Optional: clear paymaster-batch after a successful send once userOpHash is obtained

Since paymaster-batch is internal and hidden, removing it after successful submission prevents accumulation in state until the next action. It doesn’t affect status polling.

Apply this diff:

         startUserOpPolling({
           userOpHash,
           chainIdForTxHash,
           recipientAddress: recipient,
           amountSent: Number(amount),
           sendId,
         });
+        // Clear internal batch after successful submission
+        try {
+          kit.batch({ batchName }).remove();
+        } catch (e) {
+          console.warn('Failed to clear paymaster batch after successful send', e);
+        }

1523-1531: Guard against empty approveData before adding the approval transaction

If approveData hasn’t been computed yet, the approval tx will be invalid. Add a precondition and surface a user-friendly error.

Apply this diff:

         // 1. Approval transaction
+        if (!approveData) {
+          Sentry.captureMessage('Approval data not ready for paymaster batch', {
+            level: 'warning',
+            tags: { component: 'send_flow', action: 'missing_approve_data', sendId },
+          });
+          handleError(t`error.unableToPrepareApproval`);
+          return;
+        }
         kit
           .transaction({
             chainId: selectedAsset.chainId,
             to: selectedFeeAsset.token,
             value: '0',
             data: approveData,
           })

1516-1521: Cross-file constant for ‘paymaster-batch’

The sentinel name appears in multiple files. Extract a shared PAYMASTER_BATCH_NAME = 'paymaster-batch' (e.g., in a constants module) and replace string literals to prevent drift.

Also applies to: 1569-1571, 1657-1659, 1704-1706

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 38f6cf3 and 5032e5a.

📒 Files selected for processing (3)
  • src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (1 hunks)
  • src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (4 hunks)
  • src/providers/GlobalTransactionsBatchProvider.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
🔇 Additional comments (4)
src/components/BottomMenuModal/SendModal/SendModalBatchesTabView.tsx (1)

83-85: LGTM: hide internal ‘paymaster-batch’ in grouping

Skipping paymaster-batch at the grouping stage matches the intent to keep it internal.

src/components/BottomMenuModal/SendModal/SendModalTokensTabView.tsx (3)

1569-1571: Good: cleanup on estimation failure

Clearing the failed paymaster batch here is correct.


1657-1659: Good: cleanup on send failure

Clearing the failed paymaster batch after sendBatches error is correct.


1704-1706: Good: cleanup when userOpHash is missing

This prevents leftover internal batches on this error path.

@coderabbitai coderabbitai bot mentioned this pull request Oct 1, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Dec 4, 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