fix(rp): init rp last execution time at current time (0xM L-03) (#41)

Signed-off-by: Tomás Migone <[email protected]>
edgeandnode · Sep 19, 2023 · 90ef626 · 90ef626
1 parent 7911422
commit 90ef626
Show file tree

Hide file tree

Showing 5 changed files with 6 additions and 32 deletions.
diff --git a/contracts/RecurringPayments.sol b/contracts/RecurringPayments.sol
@@ -193,7 +193,7 @@ contract RecurringPayments is IRecurringPayments, GelatoManager, Rescuable {
  );
 
  // Save recurring payment
- recurringPayments[user] = RecurringPayment(id, recurringAmount, block.timestamp, 0, paymentType);
+ recurringPayments[user] = RecurringPayment(id, recurringAmount, block.timestamp, block.timestamp, paymentType);
 
  // Create account if payment type requires it
  if (paymentType.requiresAccountCreation) {
@@ -251,7 +251,7 @@ contract RecurringPayments is IRecurringPayments, GelatoManager, Rescuable {
  // If user is calling we allow early execution and don't automatically cancel even if expiration time has passed
  if (user != msg.sender) {
  // Cancel the recurring payment if it has failed for long enough
- if (_canCancel(recurringPayment.createdAt, recurringPayment.lastExecutedAt)) {
+ if (_canCancel(recurringPayment.lastExecutedAt)) {
  _cancel(user, true);
  return;
  }
@@ -454,9 +454,8 @@ contract RecurringPayments is IRecurringPayments, GelatoManager, Rescuable {
  * @param lastExecutedAt Timestamp the recurring payment was last executed
  * @return True if the recurring payment can be cancelled
  */
- function _canCancel(uint256 createdAt, uint256 lastExecutedAt) private view returns (bool) {
- uint256 lastExecutedOrCreatedAt = lastExecutedAt == 0 ? createdAt : lastExecutedAt;
- return block.timestamp >= BokkyPooBahsDateTimeLibrary.addMonths(lastExecutedOrCreatedAt, expirationInterval);
+ function _canCancel(uint256 lastExecutedAt) private view returns (bool) {
+ return block.timestamp >= BokkyPooBahsDateTimeLibrary.addMonths(lastExecutedAt, expirationInterval);
  }
 
  /**

diff --git a/test/recurring-payments/helpers.ts b/test/recurring-payments/helpers.ts
@@ -54,7 +54,7 @@ export async function createRP(
 
  expect(recurringPayment.recurringAmount).to.equal(recurringAmount)
  expect(recurringPayment.createdAt).to.equal(receiptTimestamp)
- expect(recurringPayment.lastExecutedAt).to.equal(0)
+ expect(recurringPayment.lastExecutedAt).to.equal(receiptTimestamp)
  expect(recurringPayment.paymentType.id).to.equal(paymentType.id)
  expect(recurringPayment.paymentType.name).to.equal(paymentType.name)
  expect(recurringPayment.paymentType.contractAddress).to.equal(paymentType.contractAddress)

diff --git a/test/recurring-payments/payment-types/billing.test.ts b/test/recurring-payments/payment-types/billing.test.ts
@@ -149,9 +149,6 @@ describe('RecurringPayments: payment types', () => {
  })
 
  it('should allow execution by any party if executionInterval has passed', async function () {
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  const recurringPayment = await recurringPayments.recurringPayments(user1.address)
 
  // Time travel to next execution time and execute it a few times

diff --git a/test/recurring-payments/payment-types/common.test.ts b/test/recurring-payments/payment-types/common.test.ts
@@ -303,9 +303,6 @@ describe('RecurringPayments: payment types', () => {
  const recurringAmount = testPaymentType.recurringAmount ?? oneHundred
  await token.connect(user1.signer).approve(recurringPayments.address, recurringAmount.mul(times + 1))
 
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  // Time travel to next execution time and execute it a few times
  for (let index = 0; index < times; index++) {
  await time.increaseTo(await recurringPayments.getNextExecutionTime(user1.address))
@@ -317,9 +314,6 @@ describe('RecurringPayments: payment types', () => {
  const recurringAmount = testPaymentType.recurringAmount ?? oneHundred
  await token.connect(user1.signer).approve(recurringPayments.address, recurringAmount)
 
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  const tx = recurringPayments.connect(me.signer).execute(user1.address)
  await expect(tx).to.be.revertedWithCustomError(recurringPayments, 'RecurringPaymentInCooldown')
  })
@@ -328,18 +322,13 @@ describe('RecurringPayments: payment types', () => {
  const recurringAmount = testPaymentType.recurringAmount ?? oneHundred
  await token.connect(user1.signer).approve(recurringPayments.address, recurringAmount.mul(2))
 
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
  await executeRP(user1, user1.address, recurringPayments, token)
  })
 
  it('should cancel the recurring payment if expiration time has passed', async function () {
  const recurringAmount = testPaymentType.recurringAmount ?? oneHundred
  await token.connect(user1.signer).approve(recurringPayments.address, recurringAmount)
 
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  const recurringPayment = await recurringPayments.recurringPayments(user1.address)
 
  await time.increaseTo(await recurringPayments.getExpirationTime(user1.address))
@@ -356,9 +345,6 @@ describe('RecurringPayments: payment types', () => {
  const recurringAmount = testPaymentType.recurringAmount ?? oneHundred
  await token.connect(user1.signer).approve(recurringPayments.address, recurringAmount.mul(2))
 
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  await time.increaseTo(await recurringPayments.getExpirationTime(user1.address))
  try {
  await executeRP(user1, user1.address, recurringPayments, token)
@@ -394,19 +380,14 @@ describe('RecurringPayments: payment types', () => {
  })
 
  it('should allow execution when executionInterval has passed', async function () {
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
- await time.increaseTo(await recurringPayments.getExpirationTime(user1.address))
+ await time.increaseTo(await recurringPayments.getNextExecutionTime(user1.address))
 
  const [canExec, execPayload] = await recurringPayments.connect(me.signer).check(user1.address)
  expect(canExec).to.be.true
  expect(execPayload).to.eq(buildCheckExecPayload(user1.address))
  })
 
  it('should not allow execution when executionInterval has not passed', async function () {
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  const [canExec, execPayload] = await recurringPayments.connect(me.signer).check(user1.address)
  expect(canExec).to.be.false
  expect(execPayload).to.eq(buildCheckExecPayload(user1.address))

diff --git a/test/recurring-payments/payment-types/subscriptions.test.ts b/test/recurring-payments/payment-types/subscriptions.test.ts
@@ -211,9 +211,6 @@ describe('RecurringPayments: payment types', () => {
  })
 
  it('should allow execution by any party if executionInterval has passed', async function () {
- // Execute once to set lastExecutedAt to a non-zero value
- await executeRP(me, user1.address, recurringPayments, token)
-
  const recurringPayment = await recurringPayments.recurringPayments(user1.address)
 
  // Time travel to next execution time and execute it a few times