From 2c6a218281738e99291e02bdc6f19d0aa5012f7c Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 21 Jun 2023 11:03:41 +0200 Subject: [PATCH 1/4] voucher can be expired after validUntilDate --- .../protocol/facets/ExchangeHandlerFacet.sol | 2 +- test/protocol/DisputeHandlerTest.js | 18 +++++++-------- test/protocol/ExchangeHandlerTest.js | 23 +++++++++++++++++-- test/protocol/FundsHandlerTest.js | 10 ++++---- test/protocol/clients/BosonVoucherTest.js | 4 ++-- test/util/utils.js | 6 +++-- 6 files changed, 43 insertions(+), 20 deletions(-) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 53c051fa0..ed2f56792 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -382,7 +382,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/protocol/DisputeHandlerTest.js b/test/protocol/DisputeHandlerTest.js index 4e6dac311..13ed373d1 100644 --- a/test/protocol/DisputeHandlerTest.js +++ b/test/protocol/DisputeHandlerTest.js @@ -487,7 +487,7 @@ describe("IBosonDisputeHandler", function () { block = await ethers.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( @@ -1079,7 +1079,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( @@ -1639,7 +1639,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( @@ -1671,7 +1671,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)) @@ -1681,7 +1681,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); @@ -1729,7 +1729,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]); @@ -1931,7 +1931,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( @@ -2083,7 +2083,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); @@ -2336,7 +2336,7 @@ describe("IBosonDisputeHandler", function () { block = await ethers.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 ccf8e0b8d..c3c419668 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -1979,6 +1979,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 + ); }); }); }); @@ -2013,6 +2021,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 @@ -2060,7 +2079,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( @@ -3581,7 +3600,7 @@ describe("IBosonExchangeHandler", function () { block = await ethers.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 9212ed4cf..671954841 100644 --- a/test/protocol/FundsHandlerTest.js +++ b/test/protocol/FundsHandlerTest.js @@ -2823,7 +2823,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 () { @@ -2936,7 +2936,7 @@ describe("IBosonFundsHandler", function () { disputedDate = block.timestamp.toString(); timeout = ethers.BigNumber.from(disputedDate).add(resolutionPeriod).toString(); - await setNextBlockTimestamp(Number(timeout)); + await setNextBlockTimestamp(Number(timeout) + 1); }); it("should emit a FundsReleased event", async function () { @@ -3825,7 +3825,7 @@ describe("IBosonFundsHandler", function () { block = await ethers.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 () { @@ -3927,7 +3927,9 @@ describe("IBosonFundsHandler", function () { block = await ethers.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 b4b9e9894..79298c951 100644 --- a/test/protocol/clients/BosonVoucherTest.js +++ b/test/protocol/clients/BosonVoucherTest.js @@ -1071,8 +1071,8 @@ describe("IBosonVoucher", function () { }); it("Should be 0 if offer is expired", async function () { - // Skip to after offer expiry - await setNextBlockTimestamp(ethers.BigNumber.from(offerDates.validUntil).add(1).toHexString()); + // Skip to offer expiry + await setNextBlockTimestamp(ethers.BigNumber.from(offerDates.validUntil).toHexString(), 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 0447b5562..287094b8f 100644 --- a/test/util/utils.js +++ b/test/util/utils.js @@ -117,11 +117,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) { From a9022c53cdf5170c35319e411b27bb421aa09130 Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 21 Jun 2023 11:59:28 +0200 Subject: [PATCH 2/4] fix integration test --- test/integration/02-Upgraded-facet.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/02-Upgraded-facet.js b/test/integration/02-Upgraded-facet.js index 3f887cb56..d0a63717e 100644 --- a/test/integration/02-Upgraded-facet.js +++ b/test/integration/02-Upgraded-facet.js @@ -626,7 +626,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)) From fb63e1856d021088278ffab19f712093dfa8e0cd Mon Sep 17 00:00:00 2001 From: zajck Date: Wed, 21 Jun 2023 12:43:25 +0200 Subject: [PATCH 3/4] change inequalities --- contracts/protocol/clients/voucher/BosonVoucher.sol | 6 +++--- contracts/protocol/facets/ExchangeHandlerFacet.sol | 2 +- test/protocol/ExchangeHandlerTest.js | 4 ++-- test/protocol/clients/BosonVoucherTest.js | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/contracts/protocol/clients/voucher/BosonVoucher.sol b/contracts/protocol/clients/voucher/BosonVoucher.sol index 3ea7bab82..7d45755ca 100644 --- a/contracts/protocol/clients/voucher/BosonVoucher.sol +++ b/contracts/protocol/clients/voucher/BosonVoucher.sol @@ -226,7 +226,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable // Make sure that offer is not expired or voided (Offer memory offer, OfferDates memory offerDates) = getBosonOffer(_offerId); - require(!offer.voided && (offerDates.validUntil > block.timestamp), OFFER_EXPIRED_OR_VOIDED); + require(!offer.voided && (block.timestamp <= offerDates.validUntil), OFFER_EXPIRED_OR_VOIDED); // Get the first token to mint uint256 start = range.start + range.minted; @@ -281,7 +281,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable // Make sure that offer is either expired or voided (Offer memory offer, OfferDates memory offerDates) = getBosonOffer(_offerId); - require(offer.voided || (offerDates.validUntil <= block.timestamp), OFFER_STILL_VALID); + require(offer.voided || (block.timestamp > offerDates.validUntil), OFFER_STILL_VALID); // Get max amount that can be burned in a single transaction address protocolDiamond = IClientExternalAddresses(BeaconClientLib._beacon()).getProtocolAddress(); @@ -328,7 +328,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable function getAvailablePreMints(uint256 _offerId) external view returns (uint256 count) { // If offer is expired or voided, return 0 (Offer memory offer, OfferDates memory offerDates) = getBosonOffer(_offerId); - if (offer.voided || (offerDates.validUntil <= block.timestamp)) { + if (offer.voided || (block.timestamp > offerDates.validUntil)) { return 0; } diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index ed2f56792..8b49ad580 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -168,7 +168,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { OfferDates storage offerDates = fetchOfferDates(_offerId); require(block.timestamp >= offerDates.validFrom, OFFER_NOT_AVAILABLE); require(!_offer.voided, OFFER_HAS_BEEN_VOIDED); - require(block.timestamp < offerDates.validUntil, OFFER_HAS_EXPIRED); + require(block.timestamp <= offerDates.validUntil, OFFER_HAS_EXPIRED); if (!_isPreminted) { // For non-preminted offers, quantityAvailable must be greater than zero, since it gets decremented diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index c3c419668..340186ff5 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -709,7 +709,7 @@ describe("IBosonExchangeHandler", function () { it("offer has expired", async function () { // Go past offer expiration date - await setNextBlockTimestamp(Number(offerDates.validUntil)); + await setNextBlockTimestamp(Number(offerDates.validUntil) + 1); // Attempt to commit to the expired offer, expecting revert await expect( @@ -961,7 +961,7 @@ describe("IBosonExchangeHandler", function () { it("offer has expired", async function () { // Go past offer expiration date - await setNextBlockTimestamp(Number(offerDates.validUntil)); + await setNextBlockTimestamp(Number(offerDates.validUntil) + 1); // Attempt to commit to the expired offer, expecting revert await expect( diff --git a/test/protocol/clients/BosonVoucherTest.js b/test/protocol/clients/BosonVoucherTest.js index 79298c951..054c26bfa 100644 --- a/test/protocol/clients/BosonVoucherTest.js +++ b/test/protocol/clients/BosonVoucherTest.js @@ -1072,7 +1072,7 @@ describe("IBosonVoucher", function () { it("Should be 0 if offer is expired", async function () { // Skip to offer expiry - await setNextBlockTimestamp(ethers.BigNumber.from(offerDates.validUntil).toHexString(), true); + await setNextBlockTimestamp(ethers.BigNumber.from(offerDates.validUntil).add(1).toHexString(), true); // Get available premints from contract let availablePremints = await bosonVoucher.getAvailablePreMints(offerId); @@ -1747,7 +1747,7 @@ describe("IBosonVoucher", function () { it("Transfer preminted voucher, where offer has expired", async function () { // Skip past offer expiry - await setNextBlockTimestamp(ethers.BigNumber.from(offerValid).toHexString()); + await setNextBlockTimestamp(ethers.BigNumber.from(offerValid).add(1).toHexString()); // Transfer should fail, since protocol reverts await expect( From 2a461390822528a54aa5cc846d7a7e0028d7cc14 Mon Sep 17 00:00:00 2001 From: zajck Date: Fri, 7 Jul 2023 08:00:40 +0200 Subject: [PATCH 4/4] fix unit test --- test/protocol/ExchangeHandlerTest.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 9a19bce6a..2dac961cb 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -2070,7 +2070,7 @@ describe("IBosonExchangeHandler", function () { it("offer has expired", async function () { // Go past offer expiration date - await setNextBlockTimestamp(Number(offerDates.validUntil)); + await setNextBlockTimestamp(Number(offerDates.validUntil) + 1); // Attempt to commit to the expired offer, expecting revert await expect(