feat: ERC20RecurringPaymentProxy batch schedule v0.2.0 - #1763
Conversation
Greptile SummaryThe PR introduces ERC20RecurringPaymentProxy v0.2.0 with signed batch schedules, explicit due times, multi-leg settlement, persistent schedule state, subscriber admission and cancellation, hardened token handling, and ERC-1271 signatures.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains. No blocking failure remains. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
A[Signed batch permit] --> B[Validate signature and schedule]
B --> C{Caller authorized?}
C -->|Relayer role| D[Check due, cancellation, order, replay]
C -->|Admitted subscriber| D
D --> E[Pull exact payer total]
E --> F[Settle initial or recurring legs]
F --> G[Pay relayer fee]
G --> H[Assert baseline balance restored]
H --> I[Emit PaymentTriggered]
Reviews (5): Last reviewed commit: "fix(recurring): unique refs, hash-once, ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
❌ Echidna Fuzzing ResultsMode: ( test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
|
| Severity | Count | Status |
|---|---|---|
| ✅ High | 0 | Pass |
| ✅ Medium | 0 | Pass |
| 🔵 Low | 0 | Info |
| ℹ️ Informational | 0 | Info |
📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.
LeoSlrRf
changed the title
|
| Severity | Count | Status |
|---|---|---|
| ✅ High | 0 | Pass |
| ✅ Medium | 0 | Pass |
| 🔵 Low | 0 | Info |
| ℹ️ Informational | 0 | Info |
📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.
❌ Echidna Fuzzing ResultsMode: ( test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
❌ Echidna Fuzzing ResultsMode: ( test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
|
| Severity | Count | Status |
|---|---|---|
| ✅ High | 0 | Pass |
| ✅ Medium | 0 | Pass |
| 🔵 Low | 0 | Info |
| ℹ️ Informational | 0 | Info |
📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.
❌ Echidna Fuzzing ResultsMode: ( test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
|
| Severity | Count | Status |
|---|---|---|
| ✅ High | 0 | Pass |
| ✅ Medium | 0 | Pass |
| 🔵 Low | 0 | Info |
| ℹ️ Informational | 0 | Info |
📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.
rodrigopavezi
left a comment
rodrigopavezi
left a comment
There was a problem hiding this comment.
great job!!
Let's only deploy to mainnet chains we support after we test everything same chain direct recurring payment on testnet. we never know how it will behave with the rest of the stack and the UX.
Cheers
rodrigopavezi
left a comment
rodrigopavezi
left a comment
There was a problem hiding this comment.
Strong PR. _pullExact is the right shape — a balanceOf delta rather than just a require, so it catches silent-fail and fee-on-transfer under-delivery, which a return-value check alone would miss. The post-settlement UnexpectedBalance assertion is a good addition, and the test "cannot settle an unfunded subscriber from a residual proxy balance" closes a vector that is reachable once the subscriber can self-trigger. Digest parity against ethers _TypedDataEncoder covers the nested-array hashing, which was the most likely thing to go quietly wrong.
Four concrete defects inline. Two other things worth raising here rather than on a line:
1. The single-fee entry point is gone — is that intended?
triggerRecurringPayment and the SchedulePermit struct are removed, so 0.2.0 is a new deployment alongside 0.1.0's seven live ones. The shaped decision was explicitly additive: keep the single entry point beside the batch one, because existing schedules are bound to the old address by their signed permits and cannot be migrated. Dropping it means 0.1.0 monthly schedules keep draining on the old address for up to 21 years while detection, the reconciler and the dashboard handle two addresses and two ABIs.
That may be the right call if few enough 0.1.0 schedules are live — but it is a decision worth making out loud rather than absorbing.
2. paymentReference uniqueness makes a subgraph fix blocking.
Nothing enforces distinct references across legs, so two legs sharing one emit two identical-reference TransferWithReferenceAndFee events in a single transaction. The subgraph builds entity ids without a log index, so those collide. Either enforce uniqueness in _assertLegs, or land the subgraph log-index fix before this deploys — it moves from latent to blocking with this change.
Nit on the description: it says scheduleId is "excluded from the EIP-712 digest", but it is in the digest (see inline). Worth correcting, since auditors read the description.
Worth writing down somewhere: the schedule key changes when any signed term changes, which the tests confirm is intentional. The consequence is that an amended schedule gets a virgin bitmap and a virgin cancelled flag — so cancelling schedule A does not cancel an amended variant B, and re-signing with a changed amount resets paid cycles. Fine if the engine always treats amend as cancel-plus-new-scheduleId, but that assumption should be explicit rather than inferred.
❌ Echidna Fuzzing ResultsMode: ( test sequences) Property Test Results
📄 Full report and corpus available in workflow artifacts. ℹ️ About Echidna FuzzingEchidna is a property-based fuzzer that generates random sequences of transactions Properties tested:
|
|
Thanks for the review — replies are on the inline threads as well. R1 (testnet before mainnet): Agreed. We will only deploy to supported mainnet chains after same-chain direct recurring payment is tested on testnet. R2a (single-fee entry point removed): Not a code change. Discussed and agreed — R2b (unique paymentReference): Fixed — see the inline reply on R2c (scheduleId in the digest): Fixed — PR description corrected; see the inline reply. R2d (amend = new key / virgin cancelled): Fixed. Documented on the |
|
| Severity | Count | Status |
|---|---|---|
| ✅ High | 0 | Pass |
| ✅ Medium | 0 | Pass |
| 🔵 Low | 0 | Info |
| ℹ️ Informational | 0 | Info |
📄 Full report available in workflow artifacts.
🔍 View detailed findings in the Security tab.
2 participants
ERC20RecurringPaymentProxyv0.2.0 — Batch schedule permit with multi-leg paymentsWhat changed
Batch permit model replaces the single-recipient permit
SchedulePermitis replaced bySchedulePermitBatch, which carries two typed arrays ofLegstructs (initialLegsandrecurringLegs). EachLegholds a recipient, amount, and an 8-byte payment reference. Index 1 settlesinitialLegswhen present; all subsequent indices settlerecurringLegs. Up to 8 legs per array (MAX_LEGS = 8) are allowed. Payment references must be unique within each array.Explicit due-time array instead of
firstPayment + periodSecondsThe permit now carries a
uint32[] dueTimesarray (one entry per cycle) that must be strictly increasing. This replaces the previousfirstPayment/periodSecondsarithmetic and allows irregular schedules.scheduleIdanchors replay state independently of nonce/deadlineA
bytes32 scheduleIdfield is committed into both the schedule key and the EIP-712 digest.nonceanddeadlineare in the digest but excluded from the key, so re-signing with a new nonce or deadline does not change the schedule key and bitmap/cancellation state persist across re-signs. A zeroscheduleIdis rejected. Changing any other signed term produces a new key with a virgin bitmap and cancelled flag.Ownableremoved; all admin actions gated byDEFAULT_ADMIN_ROLEpause,unpause,setFeeProxy,grantRelayer, andrevokeRelayernow requireDEFAULT_ADMIN_ROLEinstead ofOwnable.grantRelayer/revokeRelayerreplace the single-slotsetRelayer. Multiple relayers may holdRELAYER_ROLEsimultaneously and compete for therelayerFee.Subscriber self-trigger via
admitCycles/revokeCyclesRelayers can grant (
admitCycles) or clear (revokeCycles) a bitmask of cycle indices on a schedule key. A subscriber may calltriggerRecurringPaymentBatchdirectly only for admitted cycles. Relayers bypass the admission check entirely.Hardened token-pull path
_pullExactverifies the contract's balance increased by exactly the requested amount, rejecting fee-on-transfer tokens and silent-failtransferFromimplementations._approveFeeProxyand_payRelayeralso check return values. A post-settlement balance assertion (UnexpectedBalance) ensures nothing leaks.cancelScheduleBatchlets subscribers halt a scheduleSubscribers can mark a schedule key as cancelled; subsequent trigger attempts revert with
Cancelled.rescueTokensfor stuck balancesAdmins can recover any ERC-20 token accidentally left in the contract.
EIP-1271 smart-account signatures
ECDSA.recoveris replaced bySignatureChecker.isValidSignatureNow, enabling contract-wallet subscribers.FeeProxyUpdatedevent onsetFeeProxyThe old and new proxy addresses are emitted when the fee proxy is updated.
New test contracts
ERC20SilentFail— returnsfalseon a failedtransferFrominstead of reverting.ERC20FeeOnTransfer— under-delivers by 1 token on everytransferFrom.ERC20FailTransfer—transfer()always returnsfalse.ERC20BlockRecipient— reverts transfers to a configurable blocked address.MockERC1271— minimal ERC-1271 wallet for smart-account signature tests.ABI artifact
src/lib/artifacts/ERC20RecurringPaymentProxy/0.2.0.jsonis added with the full v0.2.0 ABI.