Skip to content

fix: KEEP-394 clamp Pimlico fees to EIP-1559 invariant#1093

Merged
suisuss merged 1 commit intostagingfrom
fix/KEEP-394-pimlico-fee-clamp
May 3, 2026
Merged

fix: KEEP-394 clamp Pimlico fees to EIP-1559 invariant#1093
suisuss merged 1 commit intostagingfrom
fix/KEEP-394-pimlico-fee-clamp

Conversation

@suisuss
Copy link
Copy Markdown

@suisuss suisuss commented May 2, 2026

Summary

Pimlico's getUserOperationGasPrice().fast was observed on Sepolia returning a pair where maxPriorityFeePerGas > maxFeePerGas, violating the EIP-1559 invariant required by ERC-4337 bundlers.

lib/web3/sponsored-client.ts was passing the result of getUserOperationGasPrice().fast directly into estimateFeesPerGas without enforcing the invariant. This PR adds clampSponsoredFees (pure helper, exported for unit testing) that lifts maxFeePerGas to maxPriorityFeePerGas when violated, and wires it through sponsored-client construction.

The on-chain cost paid is still baseFee + tip, so the only effect of the clamp is that the cap moves up; spend is unchanged in normal conditions.

Mirrors the KEEP-384 fix in lib/web3/gas-strategy.ts that addressed the same class of bug for the direct-signing path.

Linear

KEEP-394

Environments

  • staging: NEXT_PUBLIC_GAS_SPONSORSHIP_ENABLED=true - was affected, this PR fixes
  • prod: NEXT_PUBLIC_GAS_SPONSORSHIP_ENABLED=false - sponsored path is gated off, no behavioral change

Test plan

  • pnpm vitest run tests/unit/sponsored-fee-clamp.test.ts - 6/6 pass (invalid pair lifted, valid preserved, equal preserved, zero pair valid, both wei boundary cases)
  • pnpm vitest run tests/unit/sponsored-client.test.ts - 2/2 pass (clamp is wired through createSponsoredClient.estimateFeesPerGas; valid pair passes through)
  • pnpm check - clean on changed files
  • pnpm type-check - clean
  • Manual verification on staging: run an approve + Uniswap V3 swap workflow on Sepolia, confirm no invariant-violation failure
  • Confirm no regression on chains where Pimlico returns valid pairs (Base, Optimism)

Investigation notes

  • Pimlico Alto (source) exposes a floor-max-priority-fee-per-gas CLI option (src/cli/config/options.ts) that uses maxBigInt(floor, priority) to enforce a priority minimum (src/handlers/gasPriceManager.ts:138-148). The 100,000,000 wei value observed in production matches 0.1 gwei exactly, consistent with such a floor being configured on Pimlico's hosted bundler, though we have not confirmed the deployed config.
  • Current Alto source enforces the EIP-1559 invariant at the end of bumpTheGasPrice (gasPriceManager.ts:153: maxFeePerGas: maxBigInt(maxFeePerGas, maxPriorityFeePerGas)). The production observation either pre-dates that line or hits a path that bypasses it. Either way, defensive clamping on our side is required because we do not control Pimlico's deployed version.
  • Sampled 512 recent Sepolia blocks (~100 minutes of history) via public RPC: base fees sat in the 10-40 wei range, p50 = 22 wei. The 2,463,760 wei maxFeePerGas figure that has appeared in incident logs is Pimlico's buffered value, not the chain's raw base fee.

suisuss added a commit that referenced this pull request May 3, 2026
Three review follow-ups for PR #1093:

1. Extract clampSponsoredFees to lib/web3/sponsored-fee-clamp.ts so the
   helper can be unit-tested without mocking 16 modules. The unit test
   now imports directly with zero mocks.

2. Replace tests/unit/sponsored-client.test.ts with an integration
   smoke test that exercises createSponsoredClient end-to-end:
   captures the args passed to createSmartAccountClient, pulls the
   estimateFeesPerGas callback back out, and asserts the clamp is
   actually invoked. Guards against accidental regressions where the
   call to clampSponsoredFees is removed.

3. Tighten the helper's docstring: drop the attribution to "Pimlico's
   priority floor" (which conflated Pimlico's behavior with our chain
   config). Describe the failure mode in terms of base-fee dynamics
   only.

Adds two boundary cases to the helper test (off-by-one above/below).

Net coverage: 9 tests across 2 files, mock count drops from 16 in the
unit test path to 0.
@suisuss suisuss force-pushed the fix/KEEP-394-pimlico-fee-clamp branch from e83ebb5 to 39d4b5c Compare May 3, 2026 01:21
@suisuss suisuss force-pushed the fix/KEEP-394-pimlico-fee-clamp branch from 39d4b5c to 44f2321 Compare May 3, 2026 02:32
Pimlico's getUserOperationGasPrice().fast has been observed on Sepolia
returning a pair where maxPriorityFeePerGas > maxFeePerGas, which
violates the EIP-1559 invariant required by ERC-4337 bundlers.

Add clampSponsoredFees() (pure helper, exported for unit testing) that
lifts maxFeePerGas to maxPriorityFeePerGas when the invariant is
violated, and apply it at sponsored-client construction. The on-chain
cost paid is still baseFee + tip, so the only effect is that the cap
moves up; spend is unchanged in normal conditions.

Mirrors the KEEP-384 fix in lib/web3/gas-strategy.ts that addressed the
same class of bug for the direct-signing path.

Tests cover the helper directly (invalid pair, valid pair, equality,
zero, and the wei boundary cases) plus a smoke test that asserts the
clamp is wired through createSponsoredClient's estimateFeesPerGas.
@suisuss suisuss force-pushed the fix/KEEP-394-pimlico-fee-clamp branch from 44f2321 to 63d5034 Compare May 3, 2026 02:43
@suisuss suisuss merged commit b2c04b3 into staging May 3, 2026
33 checks passed
@suisuss suisuss deleted the fix/KEEP-394-pimlico-fee-clamp branch May 3, 2026 02:46
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 3, 2026

🧹 PR Environment Cleaned Up

The PR environment has been successfully deleted.

Deleted Resources:

  • Namespace: pr-1093
  • All Helm releases (Keeperhub, Scheduler, Event services)
  • PostgreSQL Database (including data)
  • LocalStack, Redis
  • All associated secrets and configs

All resources have been cleaned up and will no longer incur costs.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 3, 2026

ℹ️ No PR Environment to Clean Up

No PR environment was found for this PR. This is expected if:

  • The PR never had the deploy-pr-environment label
  • The environment was already cleaned up
  • The deployment never completed successfully

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.

1 participant