Skip to content

Commit

Permalink
fix: add additional checks to check() (0xM L-04)
Browse files Browse the repository at this point in the history
Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
  • Loading branch information
tmigone committed Sep 18, 2023
1 parent 3bf1454 commit b6c1a7e
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
17 changes: 15 additions & 2 deletions contracts/RecurringPayments.sol
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ contract RecurringPayments is IRecurringPayments, GelatoManager, Rescuable {
* result in the recurring payment being automatically cancelled.
* @dev The function will revert if the gas price is too high (threshold defined by `maxGasPrice`).
* @dev This function is meant to be called by Gelato Network task runners but can be called permissionlessly.
* @dev Since Gelato runners will only call this function if `check(user)` returns true, most of the validations don't happen here.
* @param user User address
*/
function execute(address user) external {
Expand Down Expand Up @@ -410,11 +411,23 @@ contract RecurringPayments is IRecurringPayments, GelatoManager, Rescuable {
* @return execPayload Calldata indicating the function and parameters to execute a recurring payment (`execute(user)`).
*/
function check(address user) external view returns (bool canExec, bytes memory execPayload) {
RecurringPayment storage recurringPayment = _getRecurringPaymentOrRevert(user);
RecurringPayment storage recurringPayment = recurringPayments[user];
if (recurringPayment.taskId == bytes32(0)) return (false, execPayload);

// Check if the recurring payment can be executed
canExec = _canExecute(recurringPayment.lastExecutedAt, recurringPayment.executionInterval);
execPayload = abi.encodeCall(this.execute, (user));
if (!canExec) return (false, execPayload);

// Additional checks
// Check the the RP contract has enough allowance to pay for the RP
uint256 allowance = IERC20(recurringPayment.paymentType.tokenAddress).allowance(user, address(this));
if (allowance < recurringPayment.recurringAmount) return (false, execPayload);

// Check that the user has enough balance to pay for the RP
uint256 balance = IERC20(recurringPayment.paymentType.tokenAddress).balanceOf(user);
if (balance < recurringPayment.recurringAmount) return (false, execPayload);

execPayload = abi.encodeCall(this.execute, (user));
return (canExec, execPayload);
}

Expand Down
26 changes: 22 additions & 4 deletions test/recurring-payments/payment-types/common.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -439,11 +439,12 @@ describe('RecurringPayments: payment types', () => {
await token.connect(user1.signer).approve(recurringPayments.address, oneMillion)
})

it('should revert if user has no recurring payment', async function () {
it('should not allow execution if user has no recurring payment', async function () {
await recurringPayments.connect(user1.signer)['cancel()']()

const tx = recurringPayments.connect(me.signer).check(user1.address)
await expect(tx).to.be.revertedWithCustomError(recurringPayments, 'NoRecurringPaymentFound')
const [canExec, execPayload] = await recurringPayments.connect(me.signer).check(user1.address)
expect(canExec).to.be.false
expect(execPayload).to.eq('0x')
})

it('should allow execution when executionInterval has passed', async function () {
Expand All @@ -457,7 +458,24 @@ describe('RecurringPayments: payment types', () => {
it('should not allow execution when executionInterval has not passed', async function () {
const [canExec, execPayload] = await recurringPayments.connect(me.signer).check(user1.address)
expect(canExec).to.be.false
expect(execPayload).to.eq(buildCheckExecPayload(user1.address))
expect(execPayload).to.eq('0x')
})

it('should not allow execution if user allowance is not enough', async function () {
await token.connect(user1.signer).approve(recurringPayments.address, zero)

const [canExec, execPayload] = await recurringPayments.connect(me.signer).check(user1.address)
expect(canExec).to.be.false
expect(execPayload).to.eq('0x')
})

it('should not allow execution if user balance is not enough', async function () {
const userBalance = await token.balanceOf(user1.address)
await token.connect(user1.signer).transfer(governor.address, userBalance)

const [canExec, execPayload] = await recurringPayments.connect(me.signer).check(user1.address)
expect(canExec).to.be.false
expect(execPayload).to.eq('0x')
})
})
})
Expand Down

0 comments on commit b6c1a7e

Please sign in to comment.