feat(migration): top up the TFH paymaster's ERC-20 allowance (staging only) - #433
Conversation
The multi-token paymaster (WLD + USDC) charges for gas in tokens, so it needs
an allowance from the Safe. A token is topped up when the Safe holds some of it
*and* its allowance has fallen below half the target — the paymaster spends the
allowance down, so this runs again as it drains.
WLD 100 * 10^18, tops up below 50
USDC 30 * 10^6, tops up below 15
Both conditions are per-token: a WLD-only holder gets WLD approved and USDC
left alone. Approvals are for the exact target, never uint256::max.
**Staging and sandbox only.** `TfhPaymasterApprovalMigration::new` returns
`None` on production, so the migration cannot be constructed, registered, or
submitted there. The environment defaults to production when the config is
uninitialized, so an unknown environment declines too — covered by a test that
runs in a fresh process, since the config is a process-wide `OnceLock`.
Adds `BatchErc20Approval`, which `BatchPermit2Approval` could not serve: that
one hardcodes the Permit2 spender and a max amount. Adds
`TransactionTypeId::TfhPaymasterApprove = 141`, appended so no existing value
moves.
The Anvil test drives the whole rule against a WorldChain fork, including the
paymaster impersonating itself to `transferFrom` the allowance below half and
the top-up restoring it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
`BatchPermit2Approval` was a strict special case of `BatchErc20Approval` — the same `MultiSend` bundling and an identical `Is4337Encodable` impl, differing only in a hardcoded Permit2 spender, a hardcoded `U256::MAX`, and a hardcoded nonce class. It is gone; the Permit2 migration passes those three as arguments. Its call data and nonce are unchanged, which the existing byte-exact calldata tests and the Anvil integration test both hold to. Also, per review: - The environment gate moves from `TfhPaymasterApprovalMigration::new` — which returned `Option<Self>` — to the single registration site in `WalletMigrationController::new`. Two tests replace the constructor's: staging registers three migrations including the paymaster approval, production registers two and omits it, checked in a fresh process because the config is a process-wide `OnceLock`. - The targets are readable literals via `uint!` instead of `from_limbs`. - `PaymasterToken` and the `word` helper are gone; the token table is a tuple array and the two words are decoded inline behind one guard. - `observe` now refuses a short Multicall3 array. It would otherwise drop a token silently and read as "no gap", recording a success that never happened. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
USDC is the token with the odd consumption behaviour: Circle's `FiatToken` decrements allowance even from `type(uint256).max`, where a standard ERC-20 skips the decrement there. That split is why the Permit2 migration carries a USDC-only tolerance band — it approves `MAX`, so it lands exactly on it. This migration approves finite amounts, below which every ERC-20 decrements alike, so it needs no special case. The test now drains USDC as well as WLD and asserts the exact decrement and the top-up, so that is demonstrated rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b30b8f6023
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| pub const fn new(safe_account: Arc<SafeSmartAccount>) -> Self { | ||
| Self { safe_account } |
There was a problem hiding this comment.
Reject production in the migration constructor
In a production-configured process, this publicly exported constructor still creates a fully runnable migration, and calling reconcile() directly can grant the staging paymaster permission to transfer up to 100 WLD or 30 USDC from a production Safe. The controller’s registration check protects only that particular call path; it does not uphold the module’s stated staging/sandbox-only invariant for Rust callers or future registration sites. Enforce the environment restriction here (for example, by returning None outside staging/sandbox) or fail closed again before submission.
Useful? React with 👍 / 👎.
What
A wallet migration that keeps the TFH multi-token paymaster's ERC-20 allowance topped up, so it can charge for gas in WLD or USDC.
Important
Staging and sandbox only.
TfhPaymasterApprovalMigration::newreturnsNoneon production, so the migration cannot be constructed there — let alone registered, observed, or submitted. No record for it is ever written on production.The rule
A token is topped up when both hold:
Both conditions are per token: a Safe holding only WLD gets WLD approved and USDC left untouched.
Approvals are for the exact target, never
uint256::max. The paymaster spends the allowance down, and because the framework re-observes on every launch, the migration comes back and tops it up once it crosses below half. Convergence is not a terminal state here — it is the steady state between top-ups.Paymaster address
0xBF09Bc530dc29623c3cE171A4D9bf03edafE763c— the multi-token (WLD + USDC) deployment, from temporal-appse37f93a, corroborated acrossworld-app-helper-contractsandcrypto-apps. The previous0x161F475B…is drained with its stake unlocked and can no longer sponsor.How production is excluded
The gate is in the constructor rather than a runtime check, so there is no path that reaches a submission:
current_environment_or_default()returnsProductionwhen the config is uninitialized, so an unknown environment also declines.WalletMigrationController::newonly pushes the migration when the constructor yields one.test_production_never_constructscovers both refusals — uninitialized, then explicitlyProduction. It runs in a fresh child process, because the config is a process-wideOnceLockand the test binary is configured as staging for every other test.New shared pieces
BatchErc20Approval(contracts/erc20.rs) — batchedapprove(spender, amount)viaMultiSend, with the spender, the per-token amounts, and the nonce's transaction class all chosen by the caller.BatchPermit2Approvalcould not serve: it hardcodes the Permit2 spender andU256::MAX. It is left untouched rather than refactored onto this, to keep a shipped path out of the diff.TransactionTypeId::TfhPaymasterApprove = 141— appended, so no existing value moves. The enum's docs call out that downstream systems depend on the exact numbers.TFH_PAYMASTER_ADDRESS, andErc20::encode_balance_of.Observation
One
eth_call_batchedper pass —balanceOfandallowancefor each token, four calls in one Multicall3 round trip.reconcilereuses the value, so a healthy launch reads the chain once, and the gap is a local rather than a field.Tests
Unit (5): the id, both targets against
100e18/30e6, both halves as the trigger, staging constructs, and the production refusal in a fresh process.Anvil, against a WorldChain fork (
test_tfh_paymaster_approval_migration_full_flow), walking the rule end to end:transferFroms 10 WLD → 90 is still above half, so nothing happens.Step 4/5 drains the allowance the way production does, rather than writing the slot directly.
Also verified:
cargo clippy --all-targets --all-featuresclean, 408 lib tests, the three pre-existing migration integration tests, and a--no-default-featuresbuild.Note
Medium Risk
Submits on-chain ERC-20 approvals from user Safes on staging/sandbox; production is gated off, but mistaken environment config or allowance logic could approve the paymaster for real token balances in non-prod wallets.
Overview
Adds
TfhPaymasterApprovalMigration, which observes WLD/USDC balances and paymaster allowances on World Chain and submits batched ERC-20 approvals when the Safe holds a token and allowance drops below half the target (100 WLD / 30 USDC, finite amounts—not max).WalletMigrationControlleronly registers this migration in staging and sandbox; production and uninitialized config keep the existing two migrations.Introduces
BatchErc20Approval(MultiSend batchedapprovewith caller-chosen spender, amounts, andTransactionTypeId). The Permit2 wallet migration now uses it instead of removedBatchPermit2Approval; the paymaster migration usesTfhPaymasterApproveandTFH_PAYMASTER_ADDRESS. AddsErc20::encode_balance_of, unit/controller tests (including a child-process check that production omits the migration), and an Anvil fork integration test for the full top-up/drain cycle.Reviewed by Cursor Bugbot for commit b30b8f6. Bugbot is set up for automated code reviews on this repo. Configure here.