Resolve time conflict in redeemVoucher and expireVoucher (#695)

* voucher can be expired after validUntilDate

* fix integration test

---------

Co-authored-by: Mischa <mischa@mmt.me.uk>

contracts/protocol/facets/ExchangeHandlerFacet.sol

@@ -385,7 +385,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase {
         (Exchange storage exchange, Voucher storage voucher) = getValidExchange(_exchangeId, ExchangeState.Committed);
 
         // Make sure that the voucher has expired
-        require(block.timestamp >= voucher.validUntilDate, VOUCHER_STILL_VALID);
+        require(block.timestamp > voucher.validUntilDate, VOUCHER_STILL_VALID);
 
         // Finalize the exchange, burning the voucher
         finalizeExchange(exchange, ExchangeState.Canceled);

test/integration/02-Upgraded-facet.js

@@ -632,7 +632,7 @@ describe("[@skip-on-coverage] After facet upgrade, everything is still operation
           const escalatedDate = block.timestamp.toString();
 
           // Set time forward past the dispute escalation period
-          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
           // Expire the escalated dispute, testing for the event
           await expect(disputeHandler.connect(rando).expireEscalatedDispute(exchangeId))

test/protocol/DisputeHandlerTest.js

@@ -493,7 +493,7 @@ describe("IBosonDisputeHandler", function () {
           block = await provider.getBlock(blockNumber);
           escalatedDate = block.timestamp.toString();
 
-          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
           // Attempt to retract the dispute, expecting revert
           await expect(disputeHandler.connect(buyer).retractDispute(exchangeId)).to.revertedWith(
@@ -1082,7 +1082,7 @@ describe("IBosonDisputeHandler", function () {
 
           it("Dispute has expired", async function () {
             // Set time forward to the dispute expiration date
-            await setNextBlockTimestamp(Number(timeout));
+            await setNextBlockTimestamp(Number(timeout) + 1);
 
             // Attempt to resolve the dispute, expecting revert
             await expect(
@@ -1650,7 +1650,7 @@ describe("IBosonDisputeHandler", function () {
 
           it("Dispute escalation response period has elapsed", async function () {
             // Set time past escalation period
-            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
             // Attempt to decide the dispute, expecting revert
             await expect(
@@ -1682,7 +1682,7 @@ describe("IBosonDisputeHandler", function () {
 
         it("should emit a EscalatedDisputeExpired event", async function () {
           // Set time forward past the dispute escalation period
-          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
           // Expire the escalated dispute, testing for the event
           await expect(disputeHandler.connect(rando).expireEscalatedDispute(exchangeId))
@@ -1692,7 +1692,7 @@ describe("IBosonDisputeHandler", function () {
 
         it("should update state", async function () {
           // Set time forward past the dispute escalation period
-          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+          await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
           // Expire the dispute
           tx = await disputeHandler.connect(rando).expireEscalatedDispute(exchangeId);
@@ -1740,7 +1740,7 @@ describe("IBosonDisputeHandler", function () {
         context("💔 Revert Reasons", async function () {
           it("The disputes region of protocol is paused", async function () {
             // Set time forward past the dispute escalation period
-            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
             // Pause the disputes region of the protocol
             await pauseHandler.connect(pauser).pause([PausableRegion.Disputes]);
@@ -1942,7 +1942,7 @@ describe("IBosonDisputeHandler", function () {
 
           it("Dispute escalation response period has elapsed", async function () {
             // Set time forward past the dispute escalation period
-            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
             // Attempt to refuse the escalated dispute, expecting revert
             await expect(disputeHandler.connect(assistantDR).refuseEscalatedDispute(exchangeId)).to.revertedWith(
@@ -2094,7 +2094,7 @@ describe("IBosonDisputeHandler", function () {
 
         it("should return the expected dispute state if exchange id is valid and dispute has expired", async function () {
           // Set time forward to the dispute's timeout
-          await setNextBlockTimestamp(Number(timeout));
+          await setNextBlockTimestamp(Number(timeout) + 1);
 
           // Anyone calls expireDispute
           await disputeHandler.connect(rando).expireDispute(exchangeId);
@@ -2347,7 +2347,7 @@ describe("IBosonDisputeHandler", function () {
             block = await provider.getBlock(blockNumber);
             escalatedDate = block.timestamp.toString();
 
-            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod));
+            await setNextBlockTimestamp(Number(escalatedDate) + Number(escalationPeriod) + 1);
 
             // Expire dispute
             await disputeHandler.connect(rando).expireEscalatedDispute(exchangeId);

test/protocol/ExchangeHandlerTest.js

@@ -2038,6 +2038,14 @@ describe("IBosonExchangeHandler", function () {
           await expect(exchangeHandler.connect(rando).expireVoucher(exchange.id)).to.revertedWith(
             RevertReasons.VOUCHER_STILL_VALID
           );
+
+          // Set time forward past the last valid timestamp
+          await setNextBlockTimestamp(Number(voucherRedeemableFrom) + Number(voucherValid));
+
+          // Attempt to cancel the voucher, expecting revert
+          await expect(exchangeHandler.connect(rando).expireVoucher(exchange.id)).to.revertedWith(
+            RevertReasons.VOUCHER_STILL_VALID
+          );
         });
       });
     });
@@ -2072,6 +2080,17 @@ describe("IBosonExchangeHandler", function () {
         assert.equal(response, ExchangeState.Redeemed, "Exchange state is incorrect");
       });
 
+      it("It's possible to redeem at the the end of voucher validity period", async function () {
+        // Set time forward to the offer's validUntilDate
+        await setNextBlockTimestamp(Number(voucherRedeemableFrom) + Number(voucherValid));
+
+        // Redeem the voucher, expecting event
+        await expect(exchangeHandler.connect(buyer).redeemVoucher(exchange.id)).to.emit(
+          exchangeHandler,
+          "VoucherRedeemed"
+        );
+      });
+
       context("💔 Revert Reasons", async function () {
         it("The exchanges region of protocol is paused", async function () {
           // Pause the exchanges region of the protocol
@@ -2119,7 +2138,7 @@ describe("IBosonExchangeHandler", function () {
 
         it("current time is after to voucher's validUntilDate", async function () {
           // Set time forward past the voucher's validUntilDate
-          await setNextBlockTimestamp(Number(voucherRedeemableFrom) + Number(voucherValid) + Number(oneWeek));
+          await setNextBlockTimestamp(Number(voucherRedeemableFrom) + Number(voucherValid) + 1);
 
           // Attempt to redeem the voucher, expecting revert
           await expect(exchangeHandler.connect(buyer).redeemVoucher(exchange.id)).to.revertedWith(
@@ -4052,7 +4071,7 @@ describe("IBosonExchangeHandler", function () {
           block = await provider.getBlock(blockNumber);
           const escalatedDate = block.timestamp.toString();
 
-          await setNextBlockTimestamp(Number(escalatedDate) + Number(disputeResolver.escalationResponsePeriod));
+          await setNextBlockTimestamp(Number(escalatedDate) + Number(disputeResolver.escalationResponsePeriod) + 1);
 
           // Expire dispute
           await disputeHandler.connect(rando).expireEscalatedDispute(exchange.id);

test/protocol/FundsHandlerTest.js

@@ -2875,7 +2875,7 @@ describe("IBosonFundsHandler", function () {
             // protocol: protocolFee
             protocolPayoff = offerTokenProtocolFee;
 
-            await setNextBlockTimestamp(Number(timeout));
+            await setNextBlockTimestamp(Number(timeout) + 1);
           });
 
           it("should emit a FundsReleased event", async function () {
@@ -2993,7 +2993,7 @@ describe("IBosonFundsHandler", function () {
               disputedDate = block.timestamp.toString();
               timeout = BigInt(disputedDate) + resolutionPeriod.toString();
 
-              await setNextBlockTimestamp(Number(timeout));
+              await setNextBlockTimestamp(Number(timeout) + 1);
             });
 
             it("should emit a FundsReleased event", async function () {
@@ -3899,7 +3899,7 @@ describe("IBosonFundsHandler", function () {
               block = await provider.getBlock(blockNumber);
               escalatedDate = block.timestamp.toString();
 
-              await setNextBlockTimestamp(Number(escalatedDate) + Number(disputeResolver.escalationResponsePeriod));
+              await setNextBlockTimestamp(Number(escalatedDate) + Number(disputeResolver.escalationResponsePeriod) + 1);
             });
 
             it("should emit a FundsReleased event", async function () {
@@ -4001,7 +4001,9 @@ describe("IBosonFundsHandler", function () {
                 block = await provider.getBlock(blockNumber);
                 escalatedDate = block.timestamp.toString();
 
-                await setNextBlockTimestamp(Number(escalatedDate) + Number(disputeResolver.escalationResponsePeriod));
+                await setNextBlockTimestamp(
+                  Number(escalatedDate) + Number(disputeResolver.escalationResponsePeriod) + 1
+                );
               });
 
               it("should update state", async function () {

test/protocol/clients/BosonVoucherTest.js

@@ -950,7 +950,7 @@ describe("IBosonVoucher", function () {
 
       it("Should be 0 if offer is expired", async function () {
         // Skip to after offer expiry
-        await setNextBlockTimestamp(Number(BigInt(offerDates.validUntil) + 1n));
+        await setNextBlockTimestamp(Number(offerDates.validUntil), true);
 
         // Get available premints from contract
         let availablePremints = await bosonVoucher.getAvailablePreMints(offerId);

test/util/utils.js

@@ -132,11 +132,13 @@ function compareOfferStructs(returnedOffer) {
   return true;
 }
 
-async function setNextBlockTimestamp(timestamp) {
+async function setNextBlockTimestamp(timestamp, mine = false) {
   if (typeof timestamp == "string" && timestamp.startsWith("0x0") && timestamp.length > 3)
     timestamp = "0x" + timestamp.substring(3);
   await provider.send("evm_setNextBlockTimestamp", [timestamp]);
-  await provider.send("evm_mine", []);
+
+  // when testing static call, a block must be mined to get the correct timestamp
+  if (mine) await provider.send("evm_mine", []);
 }
 
 function getSignatureParameters(signature) {