diff --git a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol index 9d3f2b27b..17d52686e 100644 --- a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol +++ b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol @@ -27,6 +27,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran error ERC20RecurringPaymentProxy__ZeroAddress(); error ERC20RecurringPaymentProxy__TransferFailed(); error ERC20RecurringPaymentProxy__ShortPull(); + error ERC20RecurringPaymentProxy__ZeroScheduleId(); bytes32 public constant RELAYER_ROLE = keccak256('RELAYER_ROLE'); @@ -172,6 +173,78 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran } } + function _scheduleKeyFromPermit(SchedulePermit calldata p) private pure returns (bytes32) { + return + keccak256( + abi.encode( + p.subscriber, + p.token, + p.recipient, + p.feeAddress, + p.amount, + p.feeAmount, + p.relayerFee, + p.periodSeconds, + p.firstPayment, + p.totalPayments, + p.strictOrder + ) + ); + } + + function scheduleKeyFromPermit(SchedulePermit calldata p) public pure returns (bytes32) { + return _scheduleKeyFromPermit(p); + } + + function _scheduleKeyFromBatch(SchedulePermitBatch calldata p) private pure returns (bytes32) { + if (p.scheduleId == bytes32(0)) revert ERC20RecurringPaymentProxy__ZeroScheduleId(); + return + keccak256( + abi.encode( + p.subscriber, + p.scheduleId, + p.token, + p.relayerFee, + p.totalPayments, + p.strictOrder, + _hashUint32Array(p.dueTimes), + _hashLegs(p.initialLegs), + _hashLegs(p.recurringLegs) + ) + ); + } + + function scheduleKeyFromBatch(SchedulePermitBatch calldata p) public pure returns (bytes32) { + return _scheduleKeyFromBatch(p); + } + + function _assertUnpaid(bytes32 scheduleKey, uint8 index) private view { + if (triggeredPaymentsBitmap[scheduleKey] & (1 << index) != 0) { + revert ERC20RecurringPaymentProxy__AlreadyPaid(); + } + } + + function _assertOrder( + bytes32 scheduleKey, + uint8 index, + bool strictOrder + ) private view { + if (strictOrder && index != lastPaymentIndex[scheduleKey] + 1) { + revert ERC20RecurringPaymentProxy__PaymentOutOfOrder(); + } + } + + function _markPaid( + bytes32 scheduleKey, + uint8 index, + bool strictOrder + ) private { + triggeredPaymentsBitmap[scheduleKey] |= (1 << index); + if (strictOrder) { + lastPaymentIndex[scheduleKey] = index; + } + } + function _pullExact( IERC20 token, address from, @@ -226,24 +299,17 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran _assertSigner(p.subscriber, digest, signature); if (block.timestamp > p.deadline) revert ERC20RecurringPaymentProxy__SignatureExpired(); + if (index == 0) revert ERC20RecurringPaymentProxy__IndexOutOfBounds(); if (index >= 256) revert ERC20RecurringPaymentProxy__IndexTooLarge(); - - if (p.strictOrder) { - if (index != lastPaymentIndex[digest] + 1) - revert ERC20RecurringPaymentProxy__PaymentOutOfOrder(); - lastPaymentIndex[digest] = index; - } - if (index > p.totalPayments) revert ERC20RecurringPaymentProxy__IndexOutOfBounds(); + bytes32 scheduleKey = _scheduleKeyFromPermit(p); + _assertOrder(scheduleKey, index, p.strictOrder); + _assertUnpaid(scheduleKey, index); + uint256 execTime = uint256(p.firstPayment) + uint256(index - 1) * p.periodSeconds; if (block.timestamp < execTime) revert ERC20RecurringPaymentProxy__NotDueYet(); - uint256 mask = 1 << index; - uint256 word = triggeredPaymentsBitmap[digest]; - if (word & mask != 0) revert ERC20RecurringPaymentProxy__AlreadyPaid(); - triggeredPaymentsBitmap[digest] = word | mask; - uint256 total = p.amount + p.feeAmount + p.relayerFee; IERC20 token = IERC20(p.token); @@ -251,6 +317,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran _approveFeeProxy(token, p.amount + p.feeAmount); _proxyTransfer(p, paymentReference); _payRelayer(token, p.relayerFee); + _markPaid(scheduleKey, index, p.strictOrder); } function setRelayer(address oldRelayer, address newRelayer) external onlyOwner { diff --git a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts index c06ed977e..79b7925ce 100644 --- a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts +++ b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts @@ -684,20 +684,20 @@ describe('ERC20RecurringPaymentProxy', () => { const permit = createSchedulePermit(); const signature = await createSignature(permit, subscriber); - const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit); await expect( erc20RecurringPaymentProxy .connect(relayer) .triggerRecurringPayment(permit, signature, 1, paymentReference), ).to.be.reverted; - expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).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); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.not.equal(0); }); it('cannot settle an unfunded subscriber from a residual proxy balance', async () => { @@ -709,14 +709,14 @@ describe('ERC20RecurringPaymentProxy', () => { const permit = createSchedulePermit({ token: silentFail.address }); const signature = await createSignature(permit, subscriber); - const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(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 erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0); expect(await silentFail.balanceOf(erc20RecurringPaymentProxy.address)).to.equal(500); expect(await silentFail.balanceOf(recipientAddress)).to.equal(0); }); @@ -731,14 +731,14 @@ describe('ERC20RecurringPaymentProxy', () => { const permit = createSchedulePermit({ token: feeOnTransfer.address }); const signature = await createSignature(permit, subscriber); - const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit); await expect( erc20RecurringPaymentProxy .connect(relayer) .triggerRecurringPayment(permit, signature, 1, paymentReference), ).to.be.revertedWith('ERC20RecurringPaymentProxy__ShortPull'); - expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(digest)).to.equal(0); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0); }); it('reverts when the token returns false without reverting', async () => { @@ -751,14 +751,14 @@ describe('ERC20RecurringPaymentProxy', () => { const permit = createSchedulePermit({ token: silentFail.address }); const signature = await createSignature(permit, subscriber); - const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(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 erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0); }); it('does not mark the cycle paid when the relayer-fee transfer fails', async () => { @@ -771,18 +771,137 @@ describe('ERC20RecurringPaymentProxy', () => { const permit = createSchedulePermit({ token: failTransfer.address }); const signature = await createSignature(permit, subscriber); - const digest = await erc20RecurringPaymentProxy.hashSchedule(permit); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(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 erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0); expect(await failTransfer.balanceOf(recipientAddress)).to.equal(0); }); }); + describe('Schedule key replay', () => { + const paymentReference = '0x1234567890abcdef'; + + it('re-signing with a new nonce or deadline does not reset paid indices', async () => { + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit(); + const signature = await createSignature(permit, subscriber); + await erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference); + + const resigned = { ...permit, nonce: 1, deadline: permit.deadline + 86400 }; + const resignedSignature = await createSignature(resigned, subscriber); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit); + + expect(await erc20RecurringPaymentProxy.scheduleKeyFromPermit(resigned)).to.equal( + scheduleKey, + ); + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(resigned, resignedSignature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__AlreadyPaid'); + }); + + it('rejects index 0', async () => { + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit(); + const signature = await createSignature(permit, subscriber); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 0, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__IndexOutOfBounds'); + }); + + it('keeps the batch schedule key stable across nonce and deadline re-sign', async () => { + const permit = { + subscriber: subscriberAddress, + token: testERC20.address, + relayerFee: 1, + totalPayments: 1, + nonce: 0, + deadline: Math.floor(Date.now() / 1000) + 86400, + strictOrder: false, + scheduleId: '0x0101010101010101010101010101010101010101010101010101010101010101', + dueTimes: [Math.floor(Date.now() / 1000)], + initialLegs: [], + recurringLegs: [], + }; + const resigned = { ...permit, nonce: 9, deadline: permit.deadline + 1 }; + expect(await erc20RecurringPaymentProxy.scheduleKeyFromBatch(permit)).to.equal( + await erc20RecurringPaymentProxy.scheduleKeyFromBatch(resigned), + ); + }); + + it('changes the batch schedule key when signed terms change', async () => { + const permit = { + subscriber: subscriberAddress, + token: testERC20.address, + relayerFee: 1, + totalPayments: 1, + nonce: 0, + deadline: Math.floor(Date.now() / 1000) + 86400, + strictOrder: false, + scheduleId: '0x0101010101010101010101010101010101010101010101010101010101010101', + dueTimes: [Math.floor(Date.now() / 1000)], + initialLegs: [], + recurringLegs: [], + }; + const key = await erc20RecurringPaymentProxy.scheduleKeyFromBatch(permit); + expect( + await erc20RecurringPaymentProxy.scheduleKeyFromBatch({ + ...permit, + token: ethers.constants.AddressZero, + }), + ).to.not.equal(key); + expect( + await erc20RecurringPaymentProxy.scheduleKeyFromBatch({ ...permit, strictOrder: true }), + ).to.not.equal(key); + expect( + await erc20RecurringPaymentProxy.scheduleKeyFromBatch({ + ...permit, + recurringLegs: [ + { + recipient: recipientAddress, + amount: 1, + paymentReference: ethers.utils.hexZeroPad('0x01', 32), + }, + ], + }), + ).to.not.equal(key); + }); + + it('rejects a zero batch scheduleId', async () => { + const permit = { + subscriber: subscriberAddress, + token: testERC20.address, + relayerFee: 0, + totalPayments: 1, + nonce: 0, + deadline: Math.floor(Date.now() / 1000) + 86400, + strictOrder: false, + scheduleId: ethers.constants.HashZero, + dueTimes: [Math.floor(Date.now() / 1000)], + initialLegs: [], + recurringLegs: [], + }; + await expect(erc20RecurringPaymentProxy.scheduleKeyFromBatch(permit)).to.be.revertedWith( + 'ERC20RecurringPaymentProxy__ZeroScheduleId', + ); + }); + }); + describe('EIP-1271 signatures', () => { const paymentReference = '0x1234567890abcdef';