From 3bdf43331060a204431b606fe30d702909117029 Mon Sep 17 00:00:00 2001 From: LeoSlrRf Date: Mon, 24 Aug 2026 13:12:59 +0200 Subject: [PATCH] fix(recurring): revert on a failed or short subscriber pull --- .../contracts/ERC20RecurringPaymentProxy.sol | 47 ++++++-- .../contracts/test/ERC20PullTestTokens.sol | 62 ++++++++++ .../ERC20RecurringPaymentProxy.test.ts | 108 ++++++++++++++++++ 3 files changed, 207 insertions(+), 10 deletions(-) create mode 100644 packages/smart-contracts/src/contracts/test/ERC20PullTestTokens.sol diff --git a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol index faaabc3cd..9d3f2b27b 100644 --- a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol +++ b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol @@ -25,6 +25,8 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran error ERC20RecurringPaymentProxy__NotDueYet(); error ERC20RecurringPaymentProxy__AlreadyPaid(); error ERC20RecurringPaymentProxy__ZeroAddress(); + error ERC20RecurringPaymentProxy__TransferFailed(); + error ERC20RecurringPaymentProxy__ShortPull(); bytes32 public constant RELAYER_ROLE = keccak256('RELAYER_ROLE'); @@ -170,6 +172,38 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran } } + function _pullExact( + IERC20 token, + address from, + uint256 amount + ) private { + uint256 balanceBefore = token.balanceOf(address(this)); + if (!token.safeTransferFrom(from, address(this), amount)) { + revert ERC20RecurringPaymentProxy__TransferFailed(); + } + if (token.balanceOf(address(this)) - balanceBefore != amount) { + revert ERC20RecurringPaymentProxy__ShortPull(); + } + } + + function _approveFeeProxy(IERC20 token, uint256 amount) private { + if (!token.safeApprove(address(erc20FeeProxy), 0)) { + revert ERC20RecurringPaymentProxy__TransferFailed(); + } + if (!token.safeApprove(address(erc20FeeProxy), amount)) { + revert ERC20RecurringPaymentProxy__TransferFailed(); + } + } + + function _payRelayer(IERC20 token, uint256 amount) private { + if (amount == 0) { + return; + } + if (!token.safeTransfer(msg.sender, amount)) { + revert ERC20RecurringPaymentProxy__TransferFailed(); + } + } + function _proxyTransfer(SchedulePermit calldata p, bytes calldata paymentReference) private { erc20FeeProxy.transferFromWithReferenceAndFee( p.token, @@ -213,17 +247,10 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran uint256 total = p.amount + p.feeAmount + p.relayerFee; IERC20 token = IERC20(p.token); - token.safeTransferFrom(p.subscriber, address(this), total); - - /* USDT-safe zero-approve then set allowance */ - token.safeApprove(address(erc20FeeProxy), 0); - token.safeApprove(address(erc20FeeProxy), p.amount + p.feeAmount); - + _pullExact(token, p.subscriber, total); + _approveFeeProxy(token, p.amount + p.feeAmount); _proxyTransfer(p, paymentReference); - - if (p.relayerFee != 0) { - token.safeTransfer(msg.sender, p.relayerFee); - } + _payRelayer(token, p.relayerFee); } function setRelayer(address oldRelayer, address newRelayer) external onlyOwner { diff --git a/packages/smart-contracts/src/contracts/test/ERC20PullTestTokens.sol b/packages/smart-contracts/src/contracts/test/ERC20PullTestTokens.sol new file mode 100644 index 000000000..0d790b7e4 --- /dev/null +++ b/packages/smart-contracts/src/contracts/test/ERC20PullTestTokens.sol @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import '@openzeppelin/contracts/token/ERC20/ERC20.sol'; + +/** + * @notice ERC-20 that returns false on a failed transferFrom instead of reverting. + */ +contract ERC20SilentFail is ERC20 { + constructor(uint256 initialSupply) ERC20('Silent Fail', 'SFL') { + _mint(msg.sender, initialSupply); + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public override returns (bool) { + uint256 currentAllowance = allowance(from, _msgSender()); + if (balanceOf(from) < amount || currentAllowance < amount) { + return false; + } + _transfer(from, to, amount); + _approve(from, _msgSender(), currentAllowance - amount); + return true; + } +} + +/** + * @notice ERC-20 that under-delivers on transferFrom (fee-on-transfer). + */ +contract ERC20FeeOnTransfer is ERC20 { + constructor(uint256 initialSupply) ERC20('Fee On Transfer', 'FOT') { + _mint(msg.sender, initialSupply); + } + + function transferFrom( + address from, + address to, + uint256 amount + ) public override returns (bool) { + require(amount > 1, 'ERC20FeeOnTransfer: amount'); + address spender = _msgSender(); + _spendAllowance(from, spender, amount); + _transfer(from, to, amount - 1); + _transfer(from, address(this), 1); + return true; + } +} + +/** + * @notice ERC-20 whose transfer() returns false so a relayer-fee payout can fail. + */ +contract ERC20FailTransfer is ERC20 { + constructor(uint256 initialSupply) ERC20('Fail Transfer', 'FLT') { + _mint(msg.sender, initialSupply); + } + + function transfer(address, uint256) public pure override returns (bool) { + return false; + } +} diff --git a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts index dec7e050c..c06ed977e 100644 --- a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts +++ b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts @@ -675,6 +675,114 @@ describe('ERC20RecurringPaymentProxy', () => { }); }); + describe('Pull assertions', () => { + const paymentReference = '0x1234567890abcdef'; + + it('reverts an under-funded pull, leaves the bitmap unset, and stays collectable after funding', async () => { + await testERC20.transfer(subscriberAddress, 50); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit(); + const signature = await createSignature(permit, subscriber); + const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.reverted; + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + + await testERC20.transfer(subscriberAddress, 500); + await erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.not.equal(0); + }); + + it('cannot settle an unfunded subscriber from a residual proxy balance', async () => { + const SilentFailFactory = await ethers.getContractFactory('ERC20SilentFail'); + const silentFail = await SilentFailFactory.deploy(1000); + await silentFail.deployed(); + + await silentFail.transfer(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit({ token: silentFail.address }); + const signature = await createSignature(permit, subscriber); + const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__TransferFailed'); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + expect(await silentFail.balanceOf(erc20RecurringPaymentProxy.address)).to.equal(500); + expect(await silentFail.balanceOf(recipientAddress)).to.equal(0); + }); + + it('reverts a fee-on-transfer token that under-delivers', async () => { + const FeeOnTransferFactory = await ethers.getContractFactory('ERC20FeeOnTransfer'); + const feeOnTransfer = await FeeOnTransferFactory.deploy(1000); + await feeOnTransfer.deployed(); + + await feeOnTransfer.transfer(subscriberAddress, 500); + await feeOnTransfer.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit({ token: feeOnTransfer.address }); + const signature = await createSignature(permit, subscriber); + const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__ShortPull'); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + }); + + it('reverts when the token returns false without reverting', async () => { + const SilentFailFactory = await ethers.getContractFactory('ERC20SilentFail'); + const silentFail = await SilentFailFactory.deploy(1000); + await silentFail.deployed(); + + await silentFail.transfer(subscriberAddress, 500); + // No approve: transferFrom returns false instead of reverting. + + const permit = createSchedulePermit({ token: silentFail.address }); + const signature = await createSignature(permit, subscriber); + const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__TransferFailed'); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + }); + + it('does not mark the cycle paid when the relayer-fee transfer fails', async () => { + const FailTransferFactory = await ethers.getContractFactory('ERC20FailTransfer'); + const failTransfer = await FailTransferFactory.deploy(1000); + await failTransfer.deployed(); + + await failTransfer.transfer(subscriberAddress, 500); + await failTransfer.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit({ token: failTransfer.address }); + const signature = await createSignature(permit, subscriber); + const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__TransferFailed'); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + expect(await failTransfer.balanceOf(recipientAddress)).to.equal(0); + }); + }); + describe('EIP-1271 signatures', () => { const paymentReference = '0x1234567890abcdef';