diff --git a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol index cf0cde08d..0c19b101a 100644 --- a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol +++ b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol @@ -31,6 +31,8 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran error ERC20RecurringPaymentProxy__TooManyLegs(); error ERC20RecurringPaymentProxy__EmptyLegs(); error ERC20RecurringPaymentProxy__ZeroAmount(); + error ERC20RecurringPaymentProxy__NotSubscriber(); + error ERC20RecurringPaymentProxy__Cancelled(); uint8 public constant MAX_LEGS = 8; @@ -60,6 +62,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran /* replay defence */ mapping(bytes32 => uint256) public triggeredPaymentsBitmap; mapping(bytes32 => uint8) public lastPaymentIndex; + mapping(bytes32 => bool) public cancelledSchedules; IERC20FeeProxy public erc20FeeProxy; @@ -223,6 +226,18 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran return _scheduleKeyFromBatch(p); } + function _assertSubscriber(address subscriber) private view { + if (msg.sender != subscriber) revert ERC20RecurringPaymentProxy__NotSubscriber(); + } + + function _assertNotCancelled(bytes32 scheduleKey) private view { + if (cancelledSchedules[scheduleKey]) revert ERC20RecurringPaymentProxy__Cancelled(); + } + + function _cancel(bytes32 scheduleKey) private { + cancelledSchedules[scheduleKey] = true; + } + function _assertUnpaid(bytes32 scheduleKey, uint8 index) private view { if (triggeredPaymentsBitmap[scheduleKey] & (1 << index) != 0) { revert ERC20RecurringPaymentProxy__AlreadyPaid(); @@ -354,6 +369,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran if (index > p.totalPayments) revert ERC20RecurringPaymentProxy__IndexOutOfBounds(); bytes32 scheduleKey = _scheduleKeyFromPermit(p); + _assertNotCancelled(scheduleKey); _assertOrder(scheduleKey, index, p.strictOrder); _assertUnpaid(scheduleKey, index); @@ -405,6 +421,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran _assertLegArrays(p); bytes32 scheduleKey = _scheduleKeyFromBatch(p); + _assertNotCancelled(scheduleKey); _assertOrder(scheduleKey, index, p.strictOrder); _assertUnpaid(scheduleKey, index); @@ -428,6 +445,28 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran _payRelayer(token, p.relayerFee); } + /** + * @notice Blocks further triggers for this single-fee schedule. + * @dev Does not revoke the subscriber's ERC-20 allowance to this contract. A relayer can + * still collect a due cycle if they include a trigger in the same block ahead of + * cancel. Also `approve` this proxy to 0 (or decrease) in the same wallet batch. + */ + function cancelSchedule(SchedulePermit calldata p) external { + _assertSubscriber(p.subscriber); + _cancel(_scheduleKeyFromPermit(p)); + } + + /** + * @notice Blocks further triggers for this batch schedule. + * @dev Does not revoke the subscriber's ERC-20 allowance to this contract. A relayer can + * still collect a due cycle if they include a trigger in the same block ahead of + * cancel. Also `approve` this proxy to 0 (or decrease) in the same wallet batch. + */ + function cancelScheduleBatch(SchedulePermitBatch calldata p) external { + _assertSubscriber(p.subscriber); + _cancel(_scheduleKeyFromBatch(p)); + } + function setRelayer(address oldRelayer, address newRelayer) external onlyOwner { if (newRelayer == address(0)) revert ERC20RecurringPaymentProxy__ZeroAddress(); _revokeRole(RELAYER_ROLE, oldRelayer); diff --git a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts index 730260ca8..b736a1e23 100644 --- a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts +++ b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts @@ -1360,6 +1360,101 @@ describe('ERC20RecurringPaymentProxy', () => { }); }); + describe('cancelSchedule', () => { + const paymentReference = '0x1234567890abcdef'; + const ref = (n: number) => ethers.utils.hexZeroPad(ethers.utils.hexlify(n), 8); + + const latestTs = async () => (await ethers.provider.getBlock('latest')).timestamp; + + const simpleBatch = async () => { + const now = await latestTs(); + return { + subscriber: subscriberAddress, + token: testERC20.address, + relayerFee: 0, + totalPayments: 1, + nonce: 0, + deadline: now + 86400, + strictOrder: false, + scheduleId: '0x0303030303030303030303030303030303030303030303030303030303030303', + dueTimes: [now], + initialLegs: [], + recurringLegs: [{ recipient: recipientAddress, amount: 10, paymentReference: ref(0x21) }], + }; + }; + + it('blocks the single-fee entry point after the subscriber cancels', async () => { + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const now = await latestTs(); + const permit = createSchedulePermit({ firstPayment: now, deadline: now + 86400 }); + const signature = await createSignature(permit, subscriber); + + await erc20RecurringPaymentProxy.connect(subscriber).cancelSchedule(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__Cancelled'); + }); + + it('blocks the batch entry point after the subscriber cancels', async () => { + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = await simpleBatch(); + const signature = await createBatchSignature(permit, subscriber); + await erc20RecurringPaymentProxy.connect(subscriber).cancelScheduleBatch(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPaymentBatch(permit, signature, 1), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__Cancelled'); + }); + + it('keeps a cancelled single-fee schedule cancelled after a deadline re-sign', async () => { + const now = await latestTs(); + const permit = createSchedulePermit({ firstPayment: now, deadline: now + 86400 }); + await erc20RecurringPaymentProxy.connect(subscriber).cancelSchedule(permit); + + const resigned = { ...permit, deadline: now + 86400 * 30 }; + const signature = await createSignature(resigned, subscriber); + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(resigned, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__Cancelled'); + }); + + it('reverts when a non-subscriber tries to cancel', async () => { + const permit = createSchedulePermit(); + await expect( + erc20RecurringPaymentProxy.connect(relayer).cancelSchedule(permit), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__NotSubscriber'); + await expect( + erc20RecurringPaymentProxy.connect(user).cancelScheduleBatch(await simpleBatch()), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__NotSubscriber'); + }); + + it("reverts when another subscriber tries to cancel someone else's schedule", async () => { + const permit = createSchedulePermit(); + const hijack = { ...permit, subscriber: userAddress }; + await expect(erc20RecurringPaymentProxy.connect(user).cancelSchedule(hijack)).to.not.be + .reverted; + expect( + await erc20RecurringPaymentProxy.cancelledSchedules( + await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit), + ), + ).to.equal(false); + }); + }); + describe('EIP-712 digest parity', () => { const ref = (n: number) => ethers.utils.hexZeroPad(ethers.utils.hexlify(n), 8);