From b6c1a7e0914b8b15a08672d00491cb31e77a766d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1s=20Migone?= <tomas@edgeandnode.com>
Date: Thu, 14 Sep 2023 15:54:26 -0300
Subject: [PATCH] fix: add additional checks to check() (0xM L-04)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Tomás Migone <tomas@edgeandnode.com>
---
 contracts/RecurringPayments.sol               | 17 ++++++++++--
 .../payment-types/common.test.ts              | 26 ++++++++++++++++---
 2 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/contracts/RecurringPayments.sol b/contracts/RecurringPayments.sol
index 68f0b0d..1b82fe8 100644
--- a/contracts/RecurringPayments.sol
+++ b/contracts/RecurringPayments.sol
@@ -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 {
@@ -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);
     }
 
diff --git a/test/recurring-payments/payment-types/common.test.ts b/test/recurring-payments/payment-types/common.test.ts
index 58d747e..3326a35 100644
--- a/test/recurring-payments/payment-types/common.test.ts
+++ b/test/recurring-payments/payment-types/common.test.ts
@@ -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 () {
@@ -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')
         })
       })
     })