From f14d95259700a5aeb5da664ff7c96b6e4e7518e6 Mon Sep 17 00:00:00 2001 From: Klemen <64400885+zajck@users.noreply.github.com> Date: Thu, 6 Jul 2023 14:23:14 +0200 Subject: [PATCH] Resolve time conflict in redeemVoucher and expireVoucher (#695) * voucher can be expired after validUntilDate * fix integration test --------- Co-authored-by: Mischa --- .../protocol/facets/ExchangeHandlerFacet.sol | 2 +- test/integration/02-Upgraded-facet.js | 2 +- test/protocol/DisputeHandlerTest.js | 18 +++++++-------- test/protocol/ExchangeHandlerTest.js | 23 +++++++++++++++++-- test/protocol/FundsHandlerTest.js | 10 ++++---- test/protocol/clients/BosonVoucherTest.js | 2 +- test/util/utils.js | 6 +++-- 7 files changed, 43 insertions(+), 20 deletions(-) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index b86b3c77a..44855b29d 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/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); diff --git a/test/integration/02-Upgraded-facet.js b/test/integration/02-Upgraded-facet.js index cd8e9357f..4f72735dc 100644 --- a/test/integration/02-Upgraded-facet.js +++ b/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)) diff --git a/test/protocol/DisputeHandlerTest.js b/test/protocol/DisputeHandlerTest.js index 16b53c529..4d5d6511e 100644 --- a/test/protocol/DisputeHandlerTest.js +++ b/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); diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index d88f6f1d9..203a952d0 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/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); diff --git a/test/protocol/FundsHandlerTest.js b/test/protocol/FundsHandlerTest.js index 9bcf55105..591fa4a9e 100644 --- a/test/protocol/FundsHandlerTest.js +++ b/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 () { diff --git a/test/protocol/clients/BosonVoucherTest.js b/test/protocol/clients/BosonVoucherTest.js index 45f190369..4c9cf9103 100644 --- a/test/protocol/clients/BosonVoucherTest.js +++ b/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); diff --git a/test/util/utils.js b/test/util/utils.js index 6efc39419..eb369608e 100644 --- a/test/util/utils.js +++ b/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) {