Skip to content

fix(recurring): revert on a failed or short subscriber pull - #1753

Closed
LeoSlrRf wants to merge 1 commit into
feat/req-429-eip-1271from
feat/req-429-pull-assertions
Closed

fix(recurring): revert on a failed or short subscriber pull#1753
LeoSlrRf wants to merge 1 commit into
feat/req-429-eip-1271from
feat/req-429-pull-assertions

Conversation

@LeoSlrRf

@LeoSlrRf LeoSlrRf commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Harden token pull safety in ERC20RecurringPaymentProxy

Introduces two new custom errors and three private helper functions to make token transfers in ERC20RecurringPaymentProxy more robust against non-standard ERC-20 behaviour:

  • _pullExact — wraps safeTransferFrom and checks the contract's balance before and after the pull. Reverts with ERC20RecurringPaymentProxy__TransferFailed if the call returns false, and with ERC20RecurringPaymentProxy__ShortPull if fewer tokens than requested actually arrived (fee-on-transfer tokens).
  • _approveFeeProxy — performs the USDT-safe zero-then-set approval pattern, reverting with ERC20RecurringPaymentProxy__TransferFailed on either step failing.
  • _payRelayer — transfers the relayer fee to msg.sender, reverting with ERC20RecurringPaymentProxy__TransferFailed if the transfer returns false.

These helpers replace the inline transfer/approve/relayer-fee logic in _collectAndForward, ensuring that a failed or under-delivering token transfer always reverts the entire call and leaves the payment bitmap unset.

Three test token contracts are added to support the new test cases:

  • ERC20SilentFail — returns false instead of reverting on a failed transferFrom.
  • ERC20FeeOnTransfer — deducts a 1-unit fee on every transferFrom, simulating under-delivery.
  • ERC20FailTransfer — always returns false from transfer, allowing the relayer-fee failure path to be exercised.

A new Pull assertions test suite covers all four failure scenarios: an under-funded subscriber, a residual proxy balance that cannot substitute for a missing subscriber pull, a fee-on-transfer token, a silently failing transferFrom, and a failing relayer-fee payout.

@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR makes recurring ERC-20 settlement fail atomically when the subscriber pull is unsuccessful or under-delivers, an approval fails, or the relayer fee cannot be transferred.

  • Checks transfer and approval return values and verifies the proxy receives the exact requested amount.
  • Extracts pull, approval, and relayer-payment logic into focused helpers with custom errors.
  • Adds adversarial ERC-20 fixtures and rollback tests for insufficient funds, silent failures, fee-on-transfer shortfalls, residual balances, and failed relayer payouts.

Confidence Score: 5/5

The PR appears safe to merge, with the changed settlement path consistently reverting on failed or incomplete token operations.

The new checks preserve transaction atomicity, prevent residual proxy balances from funding an unsuccessful subscriber pull, and retain collectability after failures; the added tests exercise the principal changed token behaviors and rollback invariants.

Important Files Changed

Filename Overview
packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol Adds checked, exact token pulls and checked approval and relayer-payment helpers while preserving atomic rollback across the settlement transaction.
packages/smart-contracts/src/contracts/test/ERC20PullTestTokens.sol Adds focused adversarial token fixtures for false-returning transferFrom, fee-on-transfer under-delivery, and failed transfer behavior.
packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts Verifies failed or short pulls and failed relayer payouts revert without consuming the payment cycle or retaining partial settlement effects.

Sequence Diagram

sequenceDiagram
  participant R as Relayer
  participant P as Recurring Proxy
  participant T as ERC20 Token
  participant F as ERC20 Fee Proxy
  R->>P: triggerRecurringPayment(...)
  P->>P: Validate permit and mark cycle
  P->>T: balanceOf(proxy)
  P->>T: transferFrom(subscriber, proxy, total)
  P->>T: balanceOf(proxy)
  alt pull fails or received amount is short
    P-->>R: Revert entire transaction
  else exact pull succeeds
    P->>T: approve(feeProxy, 0)
    P->>T: approve(feeProxy, payment + fee)
    P->>F: transferFromWithReferenceAndFee(...)
    F->>T: Transfer recipient and fee amounts
    P->>T: transfer(relayer, relayerFee)
    alt approval or relayer transfer fails
      P-->>R: Revert all settlement effects
    else settlement succeeds
      P-->>R: Complete
    end
  end
Loading

Reviews (1): Last reviewed commit: "fix(recurring): revert on a failed or sh..." | Re-trigger Greptile

@LeoSlrRf LeoSlrRf closed this Aug 24, 2026
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