From eed3f135170a905f87ae36c9c6490a2c6c577bdf Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Fri, 12 May 2023 14:54:53 -0300 Subject: [PATCH 01/31] Fix token gating --- contracts/domain/BosonTypes.sol | 1 + .../handlers/IBosonExchangeHandler.sol | 23 ++- contracts/protocol/bases/GroupBase.sol | 19 +- .../protocol/facets/ExchangeHandlerFacet.sol | 187 ++++++++++++------ contracts/protocol/libs/ProtocolLib.sol | 2 + scripts/config/revert-reasons.js | 2 + scripts/domain/Condition.js | 25 ++- test/protocol/ExchangeHandlerTest.js | 99 +++++----- test/util/mock.js | 5 +- 9 files changed, 231 insertions(+), 132 deletions(-) diff --git a/contracts/domain/BosonTypes.sol b/contracts/domain/BosonTypes.sol index c0285b79b..75a41ec77 100644 --- a/contracts/domain/BosonTypes.sol +++ b/contracts/domain/BosonTypes.sol @@ -174,6 +174,7 @@ contract BosonTypes { uint256 tokenId; uint256 threshold; uint256 maxCommits; + uint256 length; } struct Exchange { diff --git a/contracts/interfaces/handlers/IBosonExchangeHandler.sol b/contracts/interfaces/handlers/IBosonExchangeHandler.sol index fa769f6e2..ce8ceb376 100644 --- a/contracts/interfaces/handlers/IBosonExchangeHandler.sol +++ b/contracts/interfaces/handlers/IBosonExchangeHandler.sol @@ -43,6 +43,12 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I */ function commitToOffer(address payable _buyer, uint256 _offerId) external payable; + function commitToConditionalOffer( + address payable _buyer, + uint256 _offerId, + uint256 _tokenId + ) external payable; + /** * @notice Commits to a preminted offer (first step of an exchange). * @@ -64,7 +70,11 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I * @param _offerId - the id of the offer to commit to * @param _exchangeId - the id of the exchange */ - function commitToPreMintedOffer(address payable _buyer, uint256 _offerId, uint256 _exchangeId) external; + function commitToPreMintedOffer( + address payable _buyer, + uint256 _offerId, + uint256 _exchangeId + ) external; /** * @notice Completes an exchange. @@ -216,9 +226,14 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I * @return exchange - the exchange details. See {BosonTypes.Exchange} * @return voucher - the voucher details. See {BosonTypes.Voucher} */ - function getExchange( - uint256 _exchangeId - ) external view returns (bool exists, BosonTypes.Exchange memory exchange, BosonTypes.Voucher memory voucher); + function getExchange(uint256 _exchangeId) + external + view + returns ( + bool exists, + BosonTypes.Exchange memory exchange, + BosonTypes.Voucher memory voucher + ); /** * @notice Gets the state of a given exchange. diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 8cd4e5065..6903e2c32 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -94,15 +94,16 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { condition.tokenId = _condition.tokenId; condition.threshold = _condition.threshold; condition.maxCommits = _condition.maxCommits; + condition.length = _condition.length; } /** * @notice Validates that condition parameters make sense. * - * Reverts if: + * A invalid condition is one that: * - EvaluationMethod.None and has fields different from 0 * - EvaluationMethod.Threshold and token address or maxCommits is zero - * - EvaluationMethod.SpecificToken and token address or maxCommits is zero + * - EvaluationMethod.SpecificToken and token address or maxCommits is zero or tokenId it's present and length is zero * * @param _condition - fully populated condition struct * @return valid - validity of condition @@ -117,7 +118,10 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { } else if (_condition.method == EvaluationMethod.Threshold) { valid = (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0); } else if (_condition.method == EvaluationMethod.SpecificToken) { - valid = (_condition.tokenAddress != address(0) && _condition.threshold == 0 && _condition.maxCommits > 0); + valid = (_condition.tokenAddress != address(0) && + _condition.threshold == 0 && + _condition.maxCommits > 0 && + (_condition.tokenId != 0 ? _condition.length > 0 : _condition.length == 0)); } } @@ -191,10 +195,11 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return sellerId - the seller id * @return group - the group details */ - function preUpdateChecks( - uint256 _groupId, - uint256[] memory _offerIds - ) internal view returns (uint256 sellerId, Group storage group) { + function preUpdateChecks(uint256 _groupId, uint256[] memory _offerIds) + internal + view + returns (uint256 sellerId, Group storage group) + { // make sure that at least something will be updated require(_offerIds.length != 0, NOTHING_UPDATED); diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 20406cfe6..d2329a6df 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -72,10 +72,39 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) * @param _offerId - the id of the offer to commit to */ - function commitToOffer( + function commitToOffer(address payable _buyer, uint256 _offerId) + external + payable + override + exchangesNotPaused + buyersNotPaused + nonReentrant + { + // Make sure buyer address is not zero address + require(_buyer != address(0), INVALID_ADDRESS); + + // Get the offer + bool exists; + Offer storage offer; + (exists, offer) = fetchOffer(_offerId); + + // Make sure offer exists, is available, and isn't void, expired, or sold out + require(exists, NO_SUCH_OFFER); + + // For there to be a condition, there must be a group. + (exists, ) = getGroupIdByOffer(offer.id); + + // Make sure offer doesn't have a condition. If it does, use commitToConditionalOffer instead. + require(!exists, "Offer has a condition. Use commitToConditionalOffer instead."); + + commitToOfferInternal(_buyer, offer, 0, false); + } + + function commitToConditionalOffer( address payable _buyer, - uint256 _offerId - ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { + uint256 _offerId, + uint256 _tokenId + ) external payable { // Make sure buyer address is not zero address require(_buyer != address(0), INVALID_ADDRESS); @@ -87,6 +116,19 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Make sure offer exists, is available, and isn't void, expired, or sold out require(exists, NO_SUCH_OFFER); + uint256 groupId; + + // For there to be a condition, there must be a group. + (exists, groupId) = getGroupIdByOffer(offer.id); + + // Make sure the group exists + require(exists, NO_SUCH_GROUP); + + // Get the condition + Condition storage condition = fetchCondition(groupId); + + require(authorizeCommit(_buyer, condition, groupId, _tokenId), CANNOT_COMMIT); + commitToOfferInternal(_buyer, offer, 0, false); } @@ -126,6 +168,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { (bool exists, ) = fetchExchange(_exchangeId); require(!exists, EXCHANGE_ALREADY_EXISTS); + (exists, ) = getGroupIdByOffer(offer.id); + + require(!exists, "Offer has a condition. Use commitToConditionalOffer instead."); + commitToOfferInternal(_buyer, offer, _exchangeId, true); } @@ -177,9 +223,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { _exchangeId = protocolCounters().nextExchangeId++; } - // Authorize the buyer to commit if offer is in a conditional group - require(authorizeCommit(_buyer, _offer, _exchangeId), CANNOT_COMMIT); - // Fetch or create buyer uint256 buyerId = getValidBuyer(_buyer); @@ -503,10 +546,12 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _exchangeId - the id of the exchange * @param _newBuyer - the address of the new buyer */ - function onVoucherTransferred( - uint256 _exchangeId, - address payable _newBuyer - ) external override buyersNotPaused nonReentrant { + function onVoucherTransferred(uint256 _exchangeId, address payable _newBuyer) + external + override + buyersNotPaused + nonReentrant + { // Cache protocol lookups for reference ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); @@ -584,9 +629,16 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @return exchange - the exchange details. See {BosonTypes.Exchange} * @return voucher - the voucher details. See {BosonTypes.Voucher} */ - function getExchange( - uint256 _exchangeId - ) external view override returns (bool exists, Exchange memory exchange, Voucher memory voucher) { + function getExchange(uint256 _exchangeId) + external + view + override + returns ( + bool exists, + Exchange memory exchange, + Voucher memory voucher + ) + { (exists, exchange) = fetchExchange(_exchangeId); voucher = fetchVoucher(_exchangeId); } @@ -698,10 +750,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _exchange - the exchange for which twins should be transferred * @return shouldBurnVoucher - whether or not the voucher should be burned */ - function transferTwins( - Exchange storage _exchange, - Voucher storage _voucher - ) internal returns (bool shouldBurnVoucher) { + function transferTwins(Exchange storage _exchange, Voucher storage _voucher) + internal + returns (bool shouldBurnVoucher) + { // See if there is an associated bundle (bool exists, uint256 bundleId) = fetchBundleIdByOffer(_exchange.offerId); @@ -861,68 +913,72 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * - increment the count of commits to the group made by the buyer address * * Conditions are associated with offers via groups. One or more offers can be - * placed in a group and a single condition applied to the entire group. Thus: - * - If a buyer commits to one offer in a group with a condition, it counts + * placed in a group and a single _condition applied to the entire group. Thus: + * - If a buyer commits to one offer in a group with a _condition, it counts * against the buyer's allowable commits for the whole group. * - If the buyer has already committed the maximum number of times for the * group, the buyer can't commit again to any of its offers. * - * The buyer is allowed to commit if no group or condition is set for this offer. + * The buyer is allowed to commit if no group or _condition is set for this offer. * * @param _buyer buyer address - * @param _offer the offer - * @param exchangeId - the exchange id + * @param _condition - the _condition to check + * @param _groupId - the group id + * @param _tokenId - the token id. Valid only for SpecificToken evaluation method * - * @return bool - true if buyer is authorized to commit + * @return allow - true if buyer is authorized to commit */ - function authorizeCommit(address _buyer, Offer storage _offer, uint256 exchangeId) internal returns (bool) { + function authorizeCommit( + address _buyer, + Condition storage _condition, + uint256 _groupId, + uint256 _tokenId + ) internal returns (bool allow) { // Cache protocol lookups for reference ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); - // Allow by default - bool allow = true; + if (_condition.method == EvaluationMethod.SpecificToken) { + // How many times has this address committed to offers in the group? + uint256 commitCount = lookups.conditionalCommitsByTokenId[_groupId][_tokenId]; - // For there to be a condition, there must be a group. - (bool exists, uint256 groupId) = getGroupIdByOffer(_offer.id); - if (exists) { - // Get the condition - Condition storage condition = fetchCondition(groupId); - - // If a condition is set, investigate, otherwise all buyers are allowed - if (condition.method != EvaluationMethod.None) { - // How many times has this address committed to offers in the group? - uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][groupId]; - - // Evaluate condition if buyer hasn't exhausted their allowable commits, otherwise disallow - if (commitCount < condition.maxCommits) { - // Buyer is allowed if they meet the group's condition - allow = (condition.method == EvaluationMethod.Threshold) - ? holdsThreshold(_buyer, condition) - : holdsSpecificToken(_buyer, condition); - - if (allow) { - // Increment number of commits to the group for this address if they are allowed to commit - lookups.conditionalCommitsByAddress[_buyer][groupId] = ++commitCount; - // Store the condition to be returned afterward on getReceipt function - lookups.exchangeCondition[exchangeId] = condition; - } - } else { - // Buyer has exhausted their allowable commits - allow = false; - } + require(commitCount < _condition.maxCommits, "Max commits per token id reached"); + + // If _condition has a token id, check that the token id is in range, otherwise accept any token id + if (_condition.tokenId > 0) { + require( + _tokenId >= _condition.tokenId && _tokenId < _condition.tokenId + _condition.length, + "Token id not in range" + ); } - } - return allow; + allow = IERC721(_condition.tokenAddress).ownerOf(_tokenId) == _buyer; + + if (allow) { + // Increment number of commits to the group for this token id if they are allowed to commit + lookups.conditionalCommitsByTokenId[_groupId][_tokenId] = ++commitCount; + } + } else if (_condition.method == EvaluationMethod.Threshold) { + // How many times has this address committed to offers in the group? + uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId]; + + require(commitCount < _condition.maxCommits, "Max commits per address reached"); + + allow = holdsThreshold(_buyer, _condition); + + if (allow) { + // Increment number of commits to the group for this address if they are allowed to commit + lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount; + } + } } /** * @notice Checks if the buyer has the required balance of the conditional token. * * @param _buyer - address of potential buyer - * @param _condition - the condition to be evaluated + * @param _condition - the _condition to be evaluated * - * @return bool - true if buyer meets the condition + * @return bool - true if buyer meets the _condition */ function holdsThreshold(address _buyer, Condition storage _condition) internal view returns (bool) { uint256 balance; @@ -941,12 +997,17 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @notice Checks if the buyer own a specific non-fungible token id. * * @param _buyer - address of potential buyer - * @param _condition - the condition to be evaluated + * @param _tokenAddress - the address of the non-fungible token + * @param _tokenId - the id of the non-fungible token * - * @return bool - true if buyer meets the condition + * @return bool - true if buyer meets the _condition */ - function holdsSpecificToken(address _buyer, Condition storage _condition) internal view returns (bool) { - return (IERC721(_condition.tokenAddress).ownerOf(_condition.tokenId) == _buyer); + function holdsSpecificToken( + address _buyer, + address _tokenAddress, + uint256 _tokenId + ) internal view returns (bool) { + return (IERC721(_tokenAddress).ownerOf(_tokenId) == _buyer); } /** @@ -1021,7 +1082,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { receipt.twinReceipts = twinReceipts; } - // Fetch condition + // Fetch _condition (bool conditionExists, Condition storage condition) = fetchConditionByExchange(exchange.id); // Add condition to receipt if exists diff --git a/contracts/protocol/libs/ProtocolLib.sol b/contracts/protocol/libs/ProtocolLib.sol index dd3f18d02..63c0b8bae 100644 --- a/contracts/protocol/libs/ProtocolLib.sol +++ b/contracts/protocol/libs/ProtocolLib.sol @@ -182,6 +182,8 @@ library ProtocolLib { mapping(uint256 => BosonTypes.AuthToken) pendingAuthTokenUpdatesBySeller; // dispute resolver id => DisputeResolver mapping(uint256 => BosonTypes.DisputeResolver) pendingAddressUpdatesByDisputeResolver; + // groupId => tokenId => commit count (count how many times a token has been used as gate for this group) + mapping(uint256 => mapping(uint256 => uint256)) conditionalCommitsByTokenId; } // Incrementing id counters diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 7c52d2b2b..615fe59e5 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -57,6 +57,8 @@ exports.RevertReasons = { TOO_MANY_OFFERS: "Exceeded maximum offers in a single transaction", NOTHING_UPDATED: "Nothing updated", INVALID_CONDITION_PARAMETERS: "Invalid condition parameters", + MAX_COMMITS_ADDRESS_REACHED: "Max commits per address reached", + MAX_COMMITS_TOKEN_REACHED: "Max commits per token reached", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/scripts/domain/Condition.js b/scripts/domain/Condition.js index 21a4930af..63bab0395 100644 --- a/scripts/domain/Condition.js +++ b/scripts/domain/Condition.js @@ -16,16 +16,18 @@ class Condition { uint256 tokenId; uint256 threshold; uint256 maxCommits; + uint256 length; } */ - constructor(method, tokenType, tokenAddress, tokenId, threshold, maxCommits) { + constructor(method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length) { this.method = method; this.tokenType = tokenType; this.tokenAddress = tokenAddress; this.tokenId = tokenId; this.threshold = threshold; this.maxCommits = maxCommits; + this.length = length; } /** @@ -34,8 +36,8 @@ class Condition { * @returns {Condition} */ static fromObject(o) { - const { method, tokenType, tokenAddress, tokenId, threshold, maxCommits } = o; - return new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits); + const { method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length } = o; + return new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length); } /** @@ -44,10 +46,10 @@ class Condition { * @returns {*} */ static fromStruct(struct) { - let method, tokenType, tokenAddress, tokenId, threshold, maxCommits; + let method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length; // destructure struct - [method, tokenType, tokenAddress, tokenId, threshold, maxCommits] = struct; + [method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length] = struct; return Condition.fromObject({ method: parseInt(method), @@ -56,6 +58,7 @@ class Condition { tokenId: tokenId.toString(), threshold: threshold.toString(), maxCommits: maxCommits.toString(), + length: length.toString(), }); } @@ -76,6 +79,7 @@ class Condition { tmp.tokenId = tmp.tokenId.toString(); tmp.threshold = tmp.threshold.toString(); tmp.maxCommits = tmp.maxCommits.toString(); + tmp.length = tmp.length.toString(); return JSON.stringify(tmp); } @@ -146,6 +150,14 @@ class Condition { return bigNumberIsValid(this.maxCommits); } + /** + * Is this Condition instance's length field valid? + * @returns {boolean} + */ + lengthIsValid() { + return bigNumberIsValid(this.length); + } + /** * Is this Condition instance valid? * @returns {boolean} @@ -157,7 +169,8 @@ class Condition { this.tokenAddressIsValid() && this.tokenIdIsValid() && this.thresholdIsValid() && - this.maxCommitsIsValid() + this.maxCommitsIsValid() && + this.lengthIsValid() ); } } diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index a773ca04b..e617d96b9 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -985,7 +985,7 @@ describe("IBosonExchangeHandler", function () { }); }); - context("👉 commitToOffer() with condition", async function () { + context.only("👉 commitToOffer() with condition", async function () { context("✋ Threshold ERC20", async function () { beforeEach(async function () { // Required constructor params for Group @@ -1008,10 +1008,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met - await expect(exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price })).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("should allow buyer to commit up to the max times for the group", async function () { @@ -1022,7 +1021,7 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1031,7 +1030,7 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1041,13 +1040,15 @@ describe("IBosonExchangeHandler", function () { // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { - await exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }); + await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, 0, { value: price }); } // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) - ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); }); }); @@ -1079,10 +1080,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met - await expect(exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price })).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("should allow buyer to commit up to the max times for the group", async function () { @@ -1093,7 +1093,7 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1102,7 +1102,7 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1112,13 +1112,15 @@ describe("IBosonExchangeHandler", function () { // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { - await exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }); + await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, 0, { value: price }); } // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) - ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); }); }); @@ -1151,10 +1153,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met - await expect(exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price })).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("should allow buyer to commit up to the max times for the group", async function () { @@ -1165,7 +1166,7 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1174,7 +1175,7 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1184,22 +1185,26 @@ describe("IBosonExchangeHandler", function () { // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { - await exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }); + await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, 0, { value: price }); } // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) - ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); }); }); - context("✋ SpecificToken ERC721", async function () { + context.only("✋ SpecificToken ERC721", async function () { + let tokenId; beforeEach(async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; + tokenId = "12"; // Create Condition condition = mockCondition({ @@ -1207,8 +1212,9 @@ describe("IBosonExchangeHandler", function () { threshold: "0", maxCommits: "3", tokenType: TokenType.NonFungibleToken, - tokenId: "12", + tokenId, method: EvaluationMethod.SpecificToken, + length: "10", }); expect(condition.isValid()).to.be.true; @@ -1216,58 +1222,51 @@ describe("IBosonExchangeHandler", function () { group = new Group(groupId, seller.id, offerIds); expect(group.isValid()).is.true; await groupHandler.connect(assistant).createGroup(group, condition); - }); - it("should emit a BuyerCommitted event if user meets condition", async function () { // mint correct token for the buyer - await foreign721.connect(buyer).mint(condition.tokenId, "1"); + await foreign721.connect(buyer).mint(tokenId, "1"); + }); + it("should emit a BuyerCommitted event if user meets condition", async function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met - await expect(exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price })).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("should allow buyer to commit up to the max times for the group", async function () { - // mint correct token for the buyer - await foreign721.connect(buyer).mint(condition.tokenId, "1"); - // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); context("💔 Revert Reasons", async function () { + // @TODO it("token id does not exist", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.ERC721_NON_EXISTENT); }); it("buyer does not meet condition for commit", async function () { - // mint correct token but to another user - await foreign721.connect(rando).mint(condition.tokenId, "1"); - // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); it("buyer has exhausted allowable commits", async function () { - // mint correct token for the buyer - await foreign721.connect(buyer).mint(condition.tokenId, "1"); - // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { - await exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }); + await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }); } // Attempt to commit again after maximum commits has been reached diff --git a/test/util/mock.js b/test/util/mock.js index fdf913b81..96d57d27d 100644 --- a/test/util/mock.js +++ b/test/util/mock.js @@ -247,14 +247,15 @@ async function mockReceipt() { ); } -function mockCondition({ method, tokenType, tokenAddress, tokenId, threshold, maxCommits }) { +function mockCondition({ method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length }) { return new Condition( method ?? EvaluationMethod.Threshold, tokenType ?? TokenType.FungibleToken, tokenAddress ?? ethers.constants.AddressZero, tokenId ?? "0", threshold ?? "1", - maxCommits ?? "1" + maxCommits ?? "1", + length ?? "0" ); } From 5921565ef8c3af2cc77343b2d3373daa39322e7d Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 15 May 2023 09:58:42 -0300 Subject: [PATCH 02/31] ExchangeHandlerTest:commitToConditionalOffer --- .../handlers/IBosonExchangeHandler.sol | 25 ++----- .../handlers/IBosonGroupHandler.sol | 2 +- .../handlers/IBosonOrchestrationHandler.sol | 2 +- contracts/protocol/bases/GroupBase.sol | 9 +-- .../protocol/facets/ExchangeHandlerFacet.sol | 75 +++++++------------ scripts/config/revert-reasons.js | 3 +- test/protocol/ExchangeHandlerTest.js | 26 +++---- 7 files changed, 55 insertions(+), 87 deletions(-) diff --git a/contracts/interfaces/handlers/IBosonExchangeHandler.sol b/contracts/interfaces/handlers/IBosonExchangeHandler.sol index ce8ceb376..1840925d0 100644 --- a/contracts/interfaces/handlers/IBosonExchangeHandler.sol +++ b/contracts/interfaces/handlers/IBosonExchangeHandler.sol @@ -11,7 +11,7 @@ import { IBosonFundsLibEvents } from "../events/IBosonFundsEvents.sol"; * * @notice Handles exchanges associated with offers within the protocol. * - * The ERC-165 identifier for this interface is: 0xe300dfc1 + * The ERC-165 identifier for this interface is: 0xc0342297 */ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, IBosonTwinEvents { /** @@ -43,11 +43,7 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I */ function commitToOffer(address payable _buyer, uint256 _offerId) external payable; - function commitToConditionalOffer( - address payable _buyer, - uint256 _offerId, - uint256 _tokenId - ) external payable; + function commitToConditionalOffer(address payable _buyer, uint256 _offerId, uint256 _tokenId) external payable; /** * @notice Commits to a preminted offer (first step of an exchange). @@ -70,11 +66,7 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I * @param _offerId - the id of the offer to commit to * @param _exchangeId - the id of the exchange */ - function commitToPreMintedOffer( - address payable _buyer, - uint256 _offerId, - uint256 _exchangeId - ) external; + function commitToPreMintedOffer(address payable _buyer, uint256 _offerId, uint256 _exchangeId) external; /** * @notice Completes an exchange. @@ -226,14 +218,9 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I * @return exchange - the exchange details. See {BosonTypes.Exchange} * @return voucher - the voucher details. See {BosonTypes.Voucher} */ - function getExchange(uint256 _exchangeId) - external - view - returns ( - bool exists, - BosonTypes.Exchange memory exchange, - BosonTypes.Voucher memory voucher - ); + function getExchange( + uint256 _exchangeId + ) external view returns (bool exists, BosonTypes.Exchange memory exchange, BosonTypes.Voucher memory voucher); /** * @notice Gets the state of a given exchange. diff --git a/contracts/interfaces/handlers/IBosonGroupHandler.sol b/contracts/interfaces/handlers/IBosonGroupHandler.sol index f5edb3a25..df0b4d84e 100644 --- a/contracts/interfaces/handlers/IBosonGroupHandler.sol +++ b/contracts/interfaces/handlers/IBosonGroupHandler.sol @@ -9,7 +9,7 @@ import { IBosonGroupEvents } from "../events/IBosonGroupEvents.sol"; * * @notice Handles creation, voiding, and querying of groups within the protocol. * - * The ERC-165 identifier for this interface is: 0xe2bf2256 + * The ERC-165 identifier for this interface is: 0x1260850f */ interface IBosonGroupHandler is IBosonGroupEvents { /** diff --git a/contracts/interfaces/handlers/IBosonOrchestrationHandler.sol b/contracts/interfaces/handlers/IBosonOrchestrationHandler.sol index 7df227a0f..72cf954fa 100644 --- a/contracts/interfaces/handlers/IBosonOrchestrationHandler.sol +++ b/contracts/interfaces/handlers/IBosonOrchestrationHandler.sol @@ -13,7 +13,7 @@ import { IBosonBundleEvents } from "../events/IBosonBundleEvents.sol"; * * @notice Combines creation of multiple entities (accounts, offers, groups, twins, bundles) in a single transaction * - * The ERC-165 identifier for this interface is: 0x0c62d8e3 + * The ERC-165 identifier for this interface is: 0x4cf51479 */ interface IBosonOrchestrationHandler is IBosonAccountEvents, diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 6903e2c32..75a8de40c 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -195,11 +195,10 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return sellerId - the seller id * @return group - the group details */ - function preUpdateChecks(uint256 _groupId, uint256[] memory _offerIds) - internal - view - returns (uint256 sellerId, Group storage group) - { + function preUpdateChecks( + uint256 _groupId, + uint256[] memory _offerIds + ) internal view returns (uint256 sellerId, Group storage group) { // make sure that at least something will be updated require(_offerIds.length != 0, NOTHING_UPDATED); diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index d2329a6df..070b80e57 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -72,14 +72,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) * @param _offerId - the id of the offer to commit to */ - function commitToOffer(address payable _buyer, uint256 _offerId) - external - payable - override - exchangesNotPaused - buyersNotPaused - nonReentrant - { + function commitToOffer( + address payable _buyer, + uint256 _offerId + ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { // Make sure buyer address is not zero address require(_buyer != address(0), INVALID_ADDRESS); @@ -100,11 +96,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { commitToOfferInternal(_buyer, offer, 0, false); } - function commitToConditionalOffer( - address payable _buyer, - uint256 _offerId, - uint256 _tokenId - ) external payable { + function commitToConditionalOffer(address payable _buyer, uint256 _offerId, uint256 _tokenId) external payable { // Make sure buyer address is not zero address require(_buyer != address(0), INVALID_ADDRESS); @@ -127,6 +119,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); + require(condition.method != EvaluationMethod.None, "Group has no condition. Use commitToOffer instead"); + require(authorizeCommit(_buyer, condition, groupId, _tokenId), CANNOT_COMMIT); commitToOfferInternal(_buyer, offer, 0, false); @@ -546,12 +540,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _exchangeId - the id of the exchange * @param _newBuyer - the address of the new buyer */ - function onVoucherTransferred(uint256 _exchangeId, address payable _newBuyer) - external - override - buyersNotPaused - nonReentrant - { + function onVoucherTransferred( + uint256 _exchangeId, + address payable _newBuyer + ) external override buyersNotPaused nonReentrant { // Cache protocol lookups for reference ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); @@ -629,16 +621,9 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @return exchange - the exchange details. See {BosonTypes.Exchange} * @return voucher - the voucher details. See {BosonTypes.Voucher} */ - function getExchange(uint256 _exchangeId) - external - view - override - returns ( - bool exists, - Exchange memory exchange, - Voucher memory voucher - ) - { + function getExchange( + uint256 _exchangeId + ) external view override returns (bool exists, Exchange memory exchange, Voucher memory voucher) { (exists, exchange) = fetchExchange(_exchangeId); voucher = fetchVoucher(_exchangeId); } @@ -750,10 +735,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _exchange - the exchange for which twins should be transferred * @return shouldBurnVoucher - whether or not the voucher should be burned */ - function transferTwins(Exchange storage _exchange, Voucher storage _voucher) - internal - returns (bool shouldBurnVoucher) - { + function transferTwins( + Exchange storage _exchange, + Voucher storage _voucher + ) internal returns (bool shouldBurnVoucher) { // See if there is an associated bundle (bool exists, uint256 bundleId) = fetchBundleIdByOffer(_exchange.offerId); @@ -913,16 +898,16 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * - increment the count of commits to the group made by the buyer address * * Conditions are associated with offers via groups. One or more offers can be - * placed in a group and a single _condition applied to the entire group. Thus: - * - If a buyer commits to one offer in a group with a _condition, it counts + * placed in a group and a single condition applied to the entire group. Thus: + * - If a buyer commits to one offer in a group with a condition, it counts * against the buyer's allowable commits for the whole group. * - If the buyer has already committed the maximum number of times for the * group, the buyer can't commit again to any of its offers. * - * The buyer is allowed to commit if no group or _condition is set for this offer. + * The buyer is allowed to commit if no group or condition is set for this offer. * * @param _buyer buyer address - * @param _condition - the _condition to check + * @param _condition - the condition to check * @param _groupId - the group id * @param _tokenId - the token id. Valid only for SpecificToken evaluation method * @@ -943,7 +928,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { require(commitCount < _condition.maxCommits, "Max commits per token id reached"); - // If _condition has a token id, check that the token id is in range, otherwise accept any token id + // If condition has a token id, check that the token id is in range, otherwise accept any token id if (_condition.tokenId > 0) { require( _tokenId >= _condition.tokenId && _tokenId < _condition.tokenId + _condition.length, @@ -976,9 +961,9 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @notice Checks if the buyer has the required balance of the conditional token. * * @param _buyer - address of potential buyer - * @param _condition - the _condition to be evaluated + * @param _condition - the condition to be evaluated * - * @return bool - true if buyer meets the _condition + * @return bool - true if buyer meets the condition */ function holdsThreshold(address _buyer, Condition storage _condition) internal view returns (bool) { uint256 balance; @@ -1000,13 +985,9 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _tokenAddress - the address of the non-fungible token * @param _tokenId - the id of the non-fungible token * - * @return bool - true if buyer meets the _condition + * @return bool - true if buyer meets the condition */ - function holdsSpecificToken( - address _buyer, - address _tokenAddress, - uint256 _tokenId - ) internal view returns (bool) { + function holdsSpecificToken(address _buyer, address _tokenAddress, uint256 _tokenId) internal view returns (bool) { return (IERC721(_tokenAddress).ownerOf(_tokenId) == _buyer); } @@ -1082,7 +1063,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { receipt.twinReceipts = twinReceipts; } - // Fetch _condition + // Fetch condition (bool conditionExists, Condition storage condition) = fetchConditionByExchange(exchange.id); // Add condition to receipt if exists diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 615fe59e5..4534e02e0 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -58,7 +58,8 @@ exports.RevertReasons = { NOTHING_UPDATED: "Nothing updated", INVALID_CONDITION_PARAMETERS: "Invalid condition parameters", MAX_COMMITS_ADDRESS_REACHED: "Max commits per address reached", - MAX_COMMITS_TOKEN_REACHED: "Max commits per token reached", + MAX_COMMITS_TOKEN_REACHED: "Max commits per token id reached", + GROUP_HAS_NO_CONDITION: "Group has no condition. Use commitToOffer instead", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index e617d96b9..c1f86b4fc 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -985,7 +985,7 @@ describe("IBosonExchangeHandler", function () { }); }); - context.only("👉 commitToOffer() with condition", async function () { + context("👉 commitToConditionalOffer()", async function () { context("✋ Threshold ERC20", async function () { beforeEach(async function () { // Required constructor params for Group @@ -1198,7 +1198,7 @@ describe("IBosonExchangeHandler", function () { }); }); - context.only("✋ SpecificToken ERC721", async function () { + context("✋ SpecificToken ERC721", async function () { let tokenId; beforeEach(async function () { // Required constructor params for Group @@ -1246,8 +1246,8 @@ describe("IBosonExchangeHandler", function () { }); context("💔 Revert Reasons", async function () { - // @TODO it("token id does not exist", async function () { + tokenId = "13"; // Attempt to commit, expecting revert await expect( exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) @@ -1255,13 +1255,16 @@ describe("IBosonExchangeHandler", function () { }); it("buyer does not meet condition for commit", async function () { + // Send token to another user + await foreign721.connect(buyer).transferFrom(buyer.address, rando.address, tokenId); + // Attempt to commit, expecting revert await expect( exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); - it("buyer has exhausted allowable commits", async function () { + it("max commits per token id reached", async function () { // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { await exchangeHandler @@ -1271,8 +1274,8 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) - ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.MAX_COMMITS_TOKEN_REACHED); }); }); }); @@ -1293,13 +1296,10 @@ describe("IBosonExchangeHandler", function () { await groupHandler.connect(assistant).createGroup(group, condition); }); - it("should emit a BuyerCommitted event", async function () { - // Commit to offer. - // We're only concerned that the event is emitted, indicating the condition was met - await expect(exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price })).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + it("should revert when using this method", async function () { + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); }); }); }); From 31aec22698e865bc5b926fe2c7097875ef8dbaff Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 15 May 2023 16:11:14 -0300 Subject: [PATCH 03/31] Fix stack to deep and fix getReceipt unit tests --- .../protocol/facets/ExchangeHandlerFacet.sol | 37 ++++++++++++------- scripts/domain/Receipt.js | 2 +- test/protocol/ExchangeHandlerTest.js | 4 +- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 070b80e57..0f9a742b6 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -121,9 +121,14 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { require(condition.method != EvaluationMethod.None, "Group has no condition. Use commitToOffer instead"); - require(authorizeCommit(_buyer, condition, groupId, _tokenId), CANNOT_COMMIT); + bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId); + require(allow, CANNOT_COMMIT); - commitToOfferInternal(_buyer, offer, 0, false); + uint256 exchangeId = commitToOfferInternal(_buyer, offer, 0, false); + + if (allow) { + protocolLookups().exchangeCondition[exchangeId] = condition; + } } /** @@ -195,13 +200,14 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _offer - storage pointer to the offer * @param _exchangeId - the id of the exchange * @param _isPreminted - whether the offer is preminted + * @return _exchangeId - the id of the exchange */ function commitToOfferInternal( address payable _buyer, Offer storage _offer, uint256 _exchangeId, bool _isPreminted - ) internal { + ) internal returns (uint256) { uint256 _offerId = _offer.id; // Make sure offer is available, and isn't void, expired, or sold out OfferDates storage offerDates = fetchOfferDates(_offerId); @@ -236,9 +242,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Operate in a block to avoid "stack too deep" error { - // Cache protocol lookups for reference - ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); - // Determine the time after which the voucher can be redeemed uint256 startDate = (block.timestamp >= offerDates.voucherRedeemableFrom) ? block.timestamp @@ -248,27 +251,35 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { voucher.validUntilDate = (offerDates.voucherRedeemableUntil > 0) ? offerDates.voucherRedeemableUntil : startDate + fetchOfferDurations(_offerId).voucherValid; + } + // Operate in a block to avoid "stack too deep" error + { + // Cache protocol lookups for reference + ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); // Map the offerId to the exchangeId as one-to-many lookups.exchangeIdsByOffer[_offerId].push(_exchangeId); // Shouldn't decrement if offer is preminted or unlimited - if (!_isPreminted && _offer.quantityAvailable != type(uint256).max) { - // Decrement offer's quantity available - _offer.quantityAvailable--; - } - - // Issue voucher, unless it already exist (for preminted offers) - lookups.voucherCount[buyerId]++; if (!_isPreminted) { + if (_offer.quantityAvailable != type(uint256).max) { + // Decrement offer's quantity available + _offer.quantityAvailable--; + } + + // Issue voucher, unless it already exist (for preminted offers) IBosonVoucher bosonVoucher = IBosonVoucher(lookups.cloneAddress[_offer.sellerId]); uint256 tokenId = _exchangeId | (_offerId << 128); bosonVoucher.issueVoucher(tokenId, _buyer); } + + lookups.voucherCount[buyerId]++; } // Notify watchers of state change emit BuyerCommitted(_offerId, buyerId, _exchangeId, exchange, voucher, msgSender()); + + return _exchangeId; } /** diff --git a/scripts/domain/Receipt.js b/scripts/domain/Receipt.js index 50ac308c4..40a0607fc 100644 --- a/scripts/domain/Receipt.js +++ b/scripts/domain/Receipt.js @@ -70,7 +70,7 @@ class Receipt { this.agentId = agentId ?? "0"; this.exchangeToken = exchangeToken; this.finalizedDate = finalizedDate; - this.condition = condition ?? new Condition(0, 0, ethers.constants.AddressZero, "0", "0", "0"); + this.condition = condition ?? new Condition(0, 0, ethers.constants.AddressZero, "0", "0", "0", "0"); this.committedDate = committedDate; this.redeemedDate = redeemedDate; this.voucherExpired = voucherExpired; diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index c1f86b4fc..612e9d073 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -4268,7 +4268,9 @@ describe("IBosonExchangeHandler", function () { await foreign20.connect(buyer).mint(buyer.address, condition.threshold); // Commit to offer - let tx = await exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }); + let tx = await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, 0, { value: price }); // Decrease offer quantityAvailable offer.quantityAvailable = "9"; From d1ad44f1e910bf260495c26c3c7b6aba49608827 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 16 May 2023 12:30:11 -0300 Subject: [PATCH 04/31] Add length validation to GroupBase's validateCondition function and update GroupHandlerTest accordingly --- contracts/protocol/bases/GroupBase.sol | 19 +++++++++++++++---- test/protocol/GroupHandlerTest.js | 1 + 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 75a8de40c..34659b72f 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -109,19 +109,30 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return valid - validity of condition * */ - function validateCondition(Condition memory _condition) internal pure returns (bool valid) { + function validateCondition(Condition memory _condition) internal view returns (bool valid) { if (_condition.method == EvaluationMethod.None) { valid = (_condition.tokenAddress == address(0) && _condition.tokenId == 0 && _condition.threshold == 0 && - _condition.maxCommits == 0); + _condition.maxCommits == 0 && + _condition.length == 0); } else if (_condition.method == EvaluationMethod.Threshold) { - valid = (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0); + valid = (_condition.tokenAddress != address(0) && + _condition.maxCommits > 0 && + _condition.threshold > 0 && + _condition.length == 0); } else if (_condition.method == EvaluationMethod.SpecificToken) { valid = (_condition.tokenAddress != address(0) && _condition.threshold == 0 && _condition.maxCommits > 0 && - (_condition.tokenId != 0 ? _condition.length > 0 : _condition.length == 0)); + _condition.tokenId != 0 && // tokenId must be present + ( + _condition.tokenType == TokenType.NonFungibleToken + ? _condition.length > 0 // length must be > 0 for NFT + : _condition.tokenType == TokenType.MultiToken + ? _condition.length == 0 // length must be zero for MultiToken + : false // TokenType.FungibleToken is not supported + )); } } diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index d6a764df9..c3bd029e9 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -171,6 +171,7 @@ describe("IBosonGroupHandler", function () { tokenType: TokenType.MultiToken, tokenAddress: accounts[0].address, tokenId: "5150", + length: "0", }); expect(condition.isValid()).to.be.true; From 867681323b7fd5d9d1ca0de8355042ecf0bbe0a7 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 16 May 2023 15:50:21 -0300 Subject: [PATCH 05/31] Adding missing tests to ExchangeHandler --- contracts/domain/BosonConstants.sol | 2 + .../protocol/facets/ExchangeHandlerFacet.sol | 69 +++----- scripts/config/revert-reasons.js | 3 +- test/protocol/ExchangeHandlerTest.js | 148 +++++++++++++++++- 4 files changed, 170 insertions(+), 52 deletions(-) diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 3eee54ad8..3ae931471 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -100,6 +100,8 @@ string constant OFFER_NOT_IN_GROUP = "Offer not part of the group"; string constant TOO_MANY_OFFERS = "Exceeded maximum offers in a single transaction"; string constant NOTHING_UPDATED = "Nothing updated"; string constant INVALID_CONDITION_PARAMETERS = "Invalid condition parameters"; +string constant GROUP_HAS_NO_CONDITION = "Offer belongs to a group without a condition. Use commitToOffer instead"; +string constant GROUP_HAS_CONDITION = "Offer belongs to a group with a condition. Use commitToConditionalOffer instead"; // Revert Reasons: Exchange related string constant NO_SUCH_EXCHANGE = "No such exchange"; diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 0f9a742b6..ec9dc390d 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -76,42 +76,26 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { address payable _buyer, uint256 _offerId ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { - // Make sure buyer address is not zero address - require(_buyer != address(0), INVALID_ADDRESS); - - // Get the offer - bool exists; - Offer storage offer; - (exists, offer) = fetchOffer(_offerId); - - // Make sure offer exists, is available, and isn't void, expired, or sold out - require(exists, NO_SUCH_OFFER); + Offer storage offer = validateOffer(_buyer, _offerId); // For there to be a condition, there must be a group. - (exists, ) = getGroupIdByOffer(offer.id); + (bool exists, ) = getGroupIdByOffer(offer.id); // Make sure offer doesn't have a condition. If it does, use commitToConditionalOffer instead. - require(!exists, "Offer has a condition. Use commitToConditionalOffer instead."); + require(!exists, GROUP_HAS_CONDITION); commitToOfferInternal(_buyer, offer, 0, false); } - function commitToConditionalOffer(address payable _buyer, uint256 _offerId, uint256 _tokenId) external payable { - // Make sure buyer address is not zero address - require(_buyer != address(0), INVALID_ADDRESS); - - // Get the offer - bool exists; - Offer storage offer; - (exists, offer) = fetchOffer(_offerId); - - // Make sure offer exists, is available, and isn't void, expired, or sold out - require(exists, NO_SUCH_OFFER); - - uint256 groupId; + function commitToConditionalOffer( + address payable _buyer, + uint256 _offerId, + uint256 _tokenId + ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { + Offer storage offer = validateOffer(_buyer, _offerId); // For there to be a condition, there must be a group. - (exists, groupId) = getGroupIdByOffer(offer.id); + (bool exists, uint256 groupId) = getGroupIdByOffer(offer.id); // Make sure the group exists require(exists, NO_SUCH_GROUP); @@ -119,16 +103,26 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); - require(condition.method != EvaluationMethod.None, "Group has no condition. Use commitToOffer instead"); + require(condition.method != EvaluationMethod.None, GROUP_HAS_NO_CONDITION); bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId); require(allow, CANNOT_COMMIT); uint256 exchangeId = commitToOfferInternal(_buyer, offer, 0, false); - if (allow) { - protocolLookups().exchangeCondition[exchangeId] = condition; - } + protocolLookups().exchangeCondition[exchangeId] = condition; + } + + function validateOffer(address _buyer, uint256 _offerId) internal view returns (Offer storage offer) { + // Make sure buyer address is not zero address + require(_buyer != address(0), INVALID_ADDRESS); + + // Get the offer + bool exists; + (exists, offer) = fetchOffer(_offerId); + + // Make sure offer exists, is available, and isn't void, expired, or sold out + require(exists, NO_SUCH_OFFER); } /** @@ -169,7 +163,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { (exists, ) = getGroupIdByOffer(offer.id); - require(!exists, "Offer has a condition. Use commitToConditionalOffer instead."); + require(!exists, GROUP_HAS_CONDITION); commitToOfferInternal(_buyer, offer, _exchangeId, true); } @@ -989,19 +983,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { return balance >= _condition.threshold; } - /** - * @notice Checks if the buyer own a specific non-fungible token id. - * - * @param _buyer - address of potential buyer - * @param _tokenAddress - the address of the non-fungible token - * @param _tokenId - the id of the non-fungible token - * - * @return bool - true if buyer meets the condition - */ - function holdsSpecificToken(address _buyer, address _tokenAddress, uint256 _tokenId) internal view returns (bool) { - return (IERC721(_tokenAddress).ownerOf(_tokenId) == _buyer); - } - /** * @notice Gets exchange receipt. * diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 4534e02e0..5089125f3 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -59,7 +59,8 @@ exports.RevertReasons = { INVALID_CONDITION_PARAMETERS: "Invalid condition parameters", MAX_COMMITS_ADDRESS_REACHED: "Max commits per address reached", MAX_COMMITS_TOKEN_REACHED: "Max commits per token id reached", - GROUP_HAS_NO_CONDITION: "Group has no condition. Use commitToOffer instead", + GROUP_HAS_NO_CONDITION: "Offer belongs to a group without a condition. Use commitToOffer instead", + GROUP_HAS_CONDITION: "Offer belongs to a group with a condition. Use commitToConditionalOffer instead", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 612e9d073..ff64e61cd 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -730,6 +730,25 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); + + it("Offer belongs to a group with condition", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + // Create Condition + condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_CONDITION); + }); }); }); @@ -1280,26 +1299,141 @@ describe("IBosonExchangeHandler", function () { }); }); - context("✋ Group without condition", async function () { - beforeEach(async function () { + context("💔 Revert Reasons", async function () { + it("The exchanges region of protocol is paused", async function () { + // Pause the exchanges region of the protocol + await pauseHandler.connect(pauser).pause([PausableRegion.Exchanges]); + + // Attempt to create an exchange, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.REGION_PAUSED); + }); + + it("The buyers region of protocol is paused", async function () { + // Pause the buyers region of the protocol + await pauseHandler.connect(pauser).pause([PausableRegion.Buyers]); + + // Attempt to create a buyer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.REGION_PAUSED); + }); + + it("buyer address is the zero address", async function () { + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(ethers.constants.AddressZero, offerId, { value: price }) + ).to.revertedWith(RevertReasons.INVALID_ADDRESS); + }); + + it("offer id is invalid", async function () { + // An invalid offer id + offerId = "666"; + + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.NO_SUCH_OFFER); + }); + + it("offer is voided", async function () { + // Void the offer first + await offerHandler.connect(assistant).voidOffer(offerId); + + // Attempt to commit to the voided offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_HAS_BEEN_VOIDED); + }); + + it("offer is not yet available for commits", async function () { + // Create an offer with staring date in the future + // get current block timestamp + const block = await ethers.provider.getBlock("latest"); + const now = block.timestamp.toString(); + + // set validFrom date in the past + offerDates.validFrom = ethers.BigNumber.from(now) + .add(oneMonth * 6) + .toString(); // 6 months in the future + offerDates.validUntil = ethers.BigNumber.from(offerDates.validFrom).add(10).toString(); // just after the valid from so it succeeds. + + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + // Attempt to commit to the not availabe offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, ++offerId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_NOT_AVAILABLE); + }); + + it("offer has expired", async function () { + // Go past offer expiration date + await setNextBlockTimestamp(Number(offerDates.validUntil)); + + // Attempt to commit to the expired offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_HAS_EXPIRED); + }); + + it("offer sold", async function () { + // Create an offer with only 1 item + offer.quantityAvailable = "1"; + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + // Commit to offer, so it's not availble anymore + await exchangeHandler.connect(buyer).commitToOffer(buyer.address, ++offerId, { value: price }); + + // Attempt to commit to the sold out offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); + }); + + it("Offer belongs to a group with condition", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; // Create Condition - condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); + condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); expect(condition.isValid()).to.be.true; // Create Group group = new Group(groupId, seller.id, offerIds); expect(group.isValid()).is.true; await groupHandler.connect(assistant).createGroup(group, condition); - }); - it("should revert when using this method", async function () { await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) - ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_CONDITION); + }); + + context("✋ Group without condition", async function () { + beforeEach(async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + // Create Condition + condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + }); + + it("Should revert when using this method", async function () { + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); + }); }); }); }); From 6104cffdff03f82b0c81877eb58ae2d78d723a19 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 16 May 2023 15:50:21 -0300 Subject: [PATCH 06/31] Adding missing tests to ExchangeHandler --- contracts/domain/BosonConstants.sol | 4 + .../protocol/facets/ExchangeHandlerFacet.sol | 76 +++---- scripts/config/revert-reasons.js | 3 +- test/protocol/ExchangeHandlerTest.js | 190 +++++++++++++++++- 4 files changed, 220 insertions(+), 53 deletions(-) diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 3eee54ad8..fb5dce1d7 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -100,6 +100,10 @@ string constant OFFER_NOT_IN_GROUP = "Offer not part of the group"; string constant TOO_MANY_OFFERS = "Exceeded maximum offers in a single transaction"; string constant NOTHING_UPDATED = "Nothing updated"; string constant INVALID_CONDITION_PARAMETERS = "Invalid condition parameters"; +string constant GROUP_HAS_NO_CONDITION = "Offer belongs to a group without a condition. Use commitToOffer instead"; +string constant GROUP_HAS_CONDITION = "Offer belongs to a group with a condition. Use commitToConditionalOffer instead"; +string constant MAX_COMMITS_ADDRESS_REACHED = "Max commits per address reached"; +string constant MAX_COMMITS_TOKEN_REACHED = "Max commits per token id reached"; // Revert Reasons: Exchange related string constant NO_SUCH_EXCHANGE = "No such exchange"; diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 0f9a742b6..1daa7b3aa 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.9; +import "hardhat/console.sol"; import { IBosonExchangeHandler } from "../../interfaces/handlers/IBosonExchangeHandler.sol"; import { IBosonAccountHandler } from "../../interfaces/handlers/IBosonAccountHandler.sol"; import { IBosonVoucher } from "../../interfaces/clients/IBosonVoucher.sol"; @@ -76,42 +77,26 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { address payable _buyer, uint256 _offerId ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { - // Make sure buyer address is not zero address - require(_buyer != address(0), INVALID_ADDRESS); - - // Get the offer - bool exists; - Offer storage offer; - (exists, offer) = fetchOffer(_offerId); - - // Make sure offer exists, is available, and isn't void, expired, or sold out - require(exists, NO_SUCH_OFFER); + Offer storage offer = validateOffer(_buyer, _offerId); // For there to be a condition, there must be a group. - (exists, ) = getGroupIdByOffer(offer.id); + (bool exists, ) = getGroupIdByOffer(offer.id); // Make sure offer doesn't have a condition. If it does, use commitToConditionalOffer instead. - require(!exists, "Offer has a condition. Use commitToConditionalOffer instead."); + require(!exists, GROUP_HAS_CONDITION); commitToOfferInternal(_buyer, offer, 0, false); } - function commitToConditionalOffer(address payable _buyer, uint256 _offerId, uint256 _tokenId) external payable { - // Make sure buyer address is not zero address - require(_buyer != address(0), INVALID_ADDRESS); - - // Get the offer - bool exists; - Offer storage offer; - (exists, offer) = fetchOffer(_offerId); - - // Make sure offer exists, is available, and isn't void, expired, or sold out - require(exists, NO_SUCH_OFFER); - - uint256 groupId; + function commitToConditionalOffer( + address payable _buyer, + uint256 _offerId, + uint256 _tokenId + ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { + Offer storage offer = validateOffer(_buyer, _offerId); // For there to be a condition, there must be a group. - (exists, groupId) = getGroupIdByOffer(offer.id); + (bool exists, uint256 groupId) = getGroupIdByOffer(offer.id); // Make sure the group exists require(exists, NO_SUCH_GROUP); @@ -119,16 +104,24 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); - require(condition.method != EvaluationMethod.None, "Group has no condition. Use commitToOffer instead"); - bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId); require(allow, CANNOT_COMMIT); uint256 exchangeId = commitToOfferInternal(_buyer, offer, 0, false); - if (allow) { - protocolLookups().exchangeCondition[exchangeId] = condition; - } + protocolLookups().exchangeCondition[exchangeId] = condition; + } + + function validateOffer(address _buyer, uint256 _offerId) internal view returns (Offer storage offer) { + // Make sure buyer address is not zero address + require(_buyer != address(0), INVALID_ADDRESS); + + // Get the offer + bool exists; + (exists, offer) = fetchOffer(_offerId); + + // Make sure offer exists, is available, and isn't void, expired, or sold out + require(exists, NO_SUCH_OFFER); } /** @@ -169,7 +162,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { (exists, ) = getGroupIdByOffer(offer.id); - require(!exists, "Offer has a condition. Use commitToConditionalOffer instead."); + require(!exists, GROUP_HAS_CONDITION); commitToOfferInternal(_buyer, offer, _exchangeId, true); } @@ -937,7 +930,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // How many times has this address committed to offers in the group? uint256 commitCount = lookups.conditionalCommitsByTokenId[_groupId][_tokenId]; - require(commitCount < _condition.maxCommits, "Max commits per token id reached"); + require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); // If condition has a token id, check that the token id is in range, otherwise accept any token id if (_condition.tokenId > 0) { @@ -957,7 +950,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // How many times has this address committed to offers in the group? uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId]; - require(commitCount < _condition.maxCommits, "Max commits per address reached"); + require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED); allow = holdsThreshold(_buyer, _condition); @@ -965,6 +958,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Increment number of commits to the group for this address if they are allowed to commit lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount; } + } else { + revert(GROUP_HAS_NO_CONDITION); } } @@ -989,19 +984,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { return balance >= _condition.threshold; } - /** - * @notice Checks if the buyer own a specific non-fungible token id. - * - * @param _buyer - address of potential buyer - * @param _tokenAddress - the address of the non-fungible token - * @param _tokenId - the id of the non-fungible token - * - * @return bool - true if buyer meets the condition - */ - function holdsSpecificToken(address _buyer, address _tokenAddress, uint256 _tokenId) internal view returns (bool) { - return (IERC721(_tokenAddress).ownerOf(_tokenId) == _buyer); - } - /** * @notice Gets exchange receipt. * diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 4534e02e0..5089125f3 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -59,7 +59,8 @@ exports.RevertReasons = { INVALID_CONDITION_PARAMETERS: "Invalid condition parameters", MAX_COMMITS_ADDRESS_REACHED: "Max commits per address reached", MAX_COMMITS_TOKEN_REACHED: "Max commits per token id reached", - GROUP_HAS_NO_CONDITION: "Group has no condition. Use commitToOffer instead", + GROUP_HAS_NO_CONDITION: "Offer belongs to a group without a condition. Use commitToOffer instead", + GROUP_HAS_CONDITION: "Offer belongs to a group with a condition. Use commitToConditionalOffer instead", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 612e9d073..d0366f0dd 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -730,6 +730,25 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); + + it("Offer belongs to a group with condition", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + // Create Condition + condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_CONDITION); + }); }); }); @@ -982,6 +1001,26 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); + + it("Offer belongs to a group with condition", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + // Create Condition + condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + + // Attempt to commit to the offer with, expecting revert + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) + ).to.revertedWith(RevertReasons.GROUP_HAS_CONDITION); + }); }); }); @@ -1280,26 +1319,167 @@ describe("IBosonExchangeHandler", function () { }); }); - context("✋ Group without condition", async function () { + context("💔 Revert Reasons", async function () { + let tokenId; + beforeEach(async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; + tokenId = "12"; // Create Condition - condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); + condition = mockCondition({ + tokenAddress: foreign721.address, + threshold: "0", + maxCommits: "3", + tokenType: TokenType.NonFungibleToken, + tokenId, + method: EvaluationMethod.SpecificToken, + length: "10", + }); expect(condition.isValid()).to.be.true; // Create Group group = new Group(groupId, seller.id, offerIds); expect(group.isValid()).is.true; await groupHandler.connect(assistant).createGroup(group, condition); + + // mint correct token for the buyer + await foreign721.connect(buyer).mint(tokenId, "1"); }); - it("should revert when using this method", async function () { + it("The exchanges region of protocol is paused", async function () { + // Pause the exchanges region of the protocol + await pauseHandler.connect(pauser).pause([PausableRegion.Exchanges]); + + // Attempt to create an exchange, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) - ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.REGION_PAUSED); + }); + + it("The buyers region of protocol is paused", async function () { + // Pause the buyers region of the protocol + await pauseHandler.connect(pauser).pause([PausableRegion.Buyers]); + + // Attempt to create a buyer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.REGION_PAUSED); + }); + + it("buyer address is the zero address", async function () { + // Attempt to commit, expecting revert + await expect( + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(ethers.constants.AddressZero, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.INVALID_ADDRESS); + }); + + it("offer id is invalid", async function () { + // An invalid offer id + offerId = "666"; + + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.NO_SUCH_OFFER); + }); + + it("offer is voided", async function () { + // Void the offer first + await offerHandler.connect(assistant).voidOffer(offerId); + + // Attempt to commit to the voided offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_HAS_BEEN_VOIDED); + }); + + it("offer is not yet available for commits", async function () { + // Create an offer with staring date in the future + // get current block timestamp + const block = await ethers.provider.getBlock("latest"); + const now = block.timestamp.toString(); + + // set validFrom date in the past + offerDates.validFrom = ethers.BigNumber.from(now) + .add(oneMonth * 6) + .toString(); // 6 months in the future + offerDates.validUntil = ethers.BigNumber.from(offerDates.validFrom).add(10).toString(); // just after the valid from so it succeeds. + + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + // add offer to group + await groupHandler.connect(assistant).addOffersToGroup(groupId, [++offerId]); + + // Attempt to commit to the not availabe offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_NOT_AVAILABLE); + }); + + it("offer has expired", async function () { + // Go past offer expiration date + await setNextBlockTimestamp(Number(offerDates.validUntil)); + + // Attempt to commit to the expired offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_HAS_EXPIRED); + }); + + it("offer sold", async function () { + // Create an offer with only 1 item + offer.quantityAvailable = "1"; + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + // add offer to group + await groupHandler.connect(assistant).addOffersToGroup(groupId, [++offerId]); + + // Commit to offer, so it's not availble anymore + await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }); + + // Attempt to commit to the sold out offer, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); + }); + + context("✋ Group without condition", async function () { + beforeEach(async function () { + // Create a new offer + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + // Required constructor params for Group + groupId = "1"; + offerIds = [(++offerId).toString()]; + + // Create Condition + condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + }); + + it("Should revert when using this method", async function () { + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); + }); }); }); }); From 00b97fb8967a35f0cd0dec22366f50bdd040cb9c Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Wed, 17 May 2023 09:33:15 -0300 Subject: [PATCH 07/31] Fix validation function in GroupBase.sol and add new revert reason in ExchangeHandlerTest.js --- contracts/protocol/bases/GroupBase.sol | 12 +++-------- scripts/config/revert-reasons.js | 1 + test/protocol/ExchangeHandlerTest.js | 28 ++++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 34659b72f..54c3e17ac 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -109,7 +109,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return valid - validity of condition * */ - function validateCondition(Condition memory _condition) internal view returns (bool valid) { + function validateCondition(Condition memory _condition) internal pure returns (bool valid) { if (_condition.method == EvaluationMethod.None) { valid = (_condition.tokenAddress == address(0) && _condition.tokenId == 0 && @@ -125,14 +125,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { valid = (_condition.tokenAddress != address(0) && _condition.threshold == 0 && _condition.maxCommits > 0 && - _condition.tokenId != 0 && // tokenId must be present - ( - _condition.tokenType == TokenType.NonFungibleToken - ? _condition.length > 0 // length must be > 0 for NFT - : _condition.tokenType == TokenType.MultiToken - ? _condition.length == 0 // length must be zero for MultiToken - : false // TokenType.FungibleToken is not supported - )); + _condition.tokenType != TokenType.FungibleToken && // FungibleToken not allowed for SpecificToken + (_condition.tokenType == TokenType.MultiToken ? _condition.length == 0 : true)); // length must be zero for MultiToken } } diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 5089125f3..922652bc2 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -61,6 +61,7 @@ exports.RevertReasons = { MAX_COMMITS_TOKEN_REACHED: "Max commits per token id reached", GROUP_HAS_NO_CONDITION: "Offer belongs to a group without a condition. Use commitToOffer instead", GROUP_HAS_CONDITION: "Offer belongs to a group with a condition. Use commitToConditionalOffer instead", + TOKEN_ID_NOT_IN_CONDITION_RANGE: "Token id not in condition range", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index d0366f0dd..c379e193d 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -1284,6 +1284,26 @@ describe("IBosonExchangeHandler", function () { } }); + it("If tokenId is not specified, should allow any token from collection", async function () { + condition.tokenId = "0"; + condition.length = "0"; + + // remove offer from old group + await groupHandler.connect(assistant).removeOffersFromGroup(groupId, offerIds); + + // create new group with new condition + await groupHandler.connect(assistant).createGroup(group, condition); + + // mint any token for buyer + tokenId = "123"; + await foreign721.connect(buyer).mint(tokenId, "1"); + + // buyer can commit + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.not.reverted; + }); + context("💔 Revert Reasons", async function () { it("token id does not exist", async function () { tokenId = "13"; @@ -1316,6 +1336,14 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_TOKEN_REACHED); }); + + it("token id not in condition range", async function () { + tokenId = "666"; + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.TOKEN_ID_NOT_IN_CONDITION_RANGE); + }); }); }); From d0e1cf2218552dc5e3f4415ae4a332e3d322c74e Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Wed, 17 May 2023 11:45:48 -0300 Subject: [PATCH 08/31] Fix domains tests --- scripts/domain/Condition.js | 2 +- test/domain/ConditionTest.js | 42 ++++++++++++++++++++++++++++++------ test/domain/ReceiptTest.js | 2 +- test/util/mock.js | 2 +- 4 files changed, 39 insertions(+), 9 deletions(-) diff --git a/scripts/domain/Condition.js b/scripts/domain/Condition.js index 63bab0395..6780b41a2 100644 --- a/scripts/domain/Condition.js +++ b/scripts/domain/Condition.js @@ -88,7 +88,7 @@ class Condition { * @returns {string} */ toStruct() { - return [this.method, this.tokenType, this.tokenAddress, this.tokenId, this.threshold, this.maxCommits]; + return [this.method, this.tokenType, this.tokenAddress, this.tokenId, this.threshold, this.maxCommits, this.length]; } /** diff --git a/test/domain/ConditionTest.js b/test/domain/ConditionTest.js index 9f894086f..49bd6b250 100644 --- a/test/domain/ConditionTest.js +++ b/test/domain/ConditionTest.js @@ -11,7 +11,7 @@ const TokenType = require("../../scripts/domain/TokenType"); describe("Condition", function () { // Suite-wide scope let condition, object, promoted, clone, dehydrated, rehydrated, key, value, struct; - let accounts, method, tokenType, tokenAddress, tokenId, threshold, maxCommits; + let accounts, method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length; context("📋 Constructor", async function () { beforeEach(async function () { @@ -20,22 +20,24 @@ describe("Condition", function () { tokenAddress = accounts[1].address; // Required constructor params - method = EvaluationMethod.None; + method = EvaluationMethod.SpecificToken; tokenType = TokenType.MultiToken; tokenId = "1"; threshold = "1"; maxCommits = "3"; + length = "0"; }); it("Should allow creation of valid, fully populated Condition instance", async function () { // Create a valid condition - condition = new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits); + condition = new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length); expect(condition.methodIsValid()).is.true; expect(condition.tokenTypeIsValid()).is.true; expect(condition.tokenAddressIsValid()).is.true; expect(condition.tokenIdIsValid()).is.true; expect(condition.thresholdIsValid()).is.true; expect(condition.maxCommitsIsValid()).is.true; + expect(condition.lengthIsValid()).is.true; expect(condition.isValid()).is.true; }); }); @@ -46,7 +48,7 @@ describe("Condition", function () { method = EvaluationMethod.SpecificToken; // Create a valid condition, then set fields in tests directly - condition = new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits); + condition = new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length); expect(condition.isValid()).is.true; }); @@ -179,6 +181,33 @@ describe("Condition", function () { expect(condition.maxCommitsIsValid()).is.true; expect(condition.isValid()).is.true; }); + + it("If present, length must be the string representation of a BigNumber", async function () { + // Invalid field value + condition.length = "zedzdeadbaby"; + expect(condition.lengthIsValid()).is.false; + expect(condition.isValid()).is.false; + + // Invalid field value + condition.length = new Date(); + expect(condition.lengthIsValid()).is.false; + expect(condition.isValid()).is.false; + + // Invalid field value + condition.length = 12; + expect(condition.lengthIsValid()).is.false; + expect(condition.isValid()).is.false; + + // Valid field value + condition.length = "0"; + expect(condition.lengthIsValid()).is.true; + expect(condition.isValid()).is.true; + + // Valid field value + condition.length = "126"; + expect(condition.lengthIsValid()).is.true; + expect(condition.isValid()).is.true; + }); }); context("📋 Utility functions", async function () { @@ -187,7 +216,7 @@ describe("Condition", function () { method = EvaluationMethod.Threshold; // Create a valid condition, then set fields in tests directly - condition = new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits); + condition = new Condition(method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length); expect(condition.isValid()).is.true; // Get plain object @@ -198,10 +227,11 @@ describe("Condition", function () { tokenId, threshold, maxCommits, + length, }; // Struct representation - struct = [method, tokenType, tokenAddress, tokenId, threshold, maxCommits]; + struct = [method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length]; }); context("👉 Static", async function () { diff --git a/test/domain/ReceiptTest.js b/test/domain/ReceiptTest.js index 16eefd401..4d98f464f 100644 --- a/test/domain/ReceiptTest.js +++ b/test/domain/ReceiptTest.js @@ -339,7 +339,7 @@ describe("Receipt", function () { expect(receipt.isValid()).is.false; // Valid field value - receipt.condition = mockCondition(ethers.constants.AddressZero); + receipt.condition = mockCondition(); expect(receipt.conditionIsValid()).is.true; expect(receipt.isValid()).is.true; }); diff --git a/test/util/mock.js b/test/util/mock.js index 96d57d27d..4944e4358 100644 --- a/test/util/mock.js +++ b/test/util/mock.js @@ -247,7 +247,7 @@ async function mockReceipt() { ); } -function mockCondition({ method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length }) { +function mockCondition({ method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length } = {}) { return new Condition( method ?? EvaluationMethod.Threshold, tokenType ?? TokenType.FungibleToken, From e760ebad35f5bd290bb5f5b8de60648599ec927e Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Wed, 17 May 2023 11:58:31 -0300 Subject: [PATCH 09/31] Fix invalid condition parameter checks in GroupHandlerTes --- scripts/domain/Condition.js | 1 - test/protocol/GroupHandlerTest.js | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/scripts/domain/Condition.js b/scripts/domain/Condition.js index 6780b41a2..dda464dc1 100644 --- a/scripts/domain/Condition.js +++ b/scripts/domain/Condition.js @@ -19,7 +19,6 @@ class Condition { uint256 length; } */ - constructor(method, tokenType, tokenAddress, tokenId, threshold, maxCommits, length) { this.method = method; this.tokenType = tokenType; diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index c3bd029e9..a3e6bfdac 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -409,6 +409,15 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); + + it("Length is not zero", async function () { + condition.length = "5"; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); }); context("Condition 'Threshold' has invalid fields", async function () { @@ -447,6 +456,15 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); + + it("Condition 'Threshold' has non zero length", async function () { + condition.length = "5"; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); }); context("Condition 'SpecificToken' has invalid fields", async function () { From 382634eea497a529029fa39361e2de2bee00a099 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Wed, 17 May 2023 20:20:04 -0300 Subject: [PATCH 10/31] Allow condition with SpecificToken to be used for ERC1155 tokens --- contracts/protocol/bases/GroupBase.sol | 40 +++++-- contracts/protocol/bases/OfferBase.sol | 6 ++ contracts/protocol/bases/ProtocolBase.sol | 8 -- .../protocol/facets/ExchangeHandlerFacet.sol | 100 +++++++++++++----- .../protocol/facets/OfferHandlerFacet.sol | 12 +++ 5 files changed, 124 insertions(+), 42 deletions(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 54c3e17ac..3d6e0d20e 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -49,7 +49,13 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { for (uint256 i = 0; i < _group.offerIds.length; i++) { // make sure offer exists and belongs to the seller - getValidOffer(_group.offerIds[i]); + Offer storage offer = getValidOffer(_group.offerIds[i]); + + // Get seller, we assume seller exists if offer exists + (, Seller storage seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); // Offer should not belong to another group already (bool exist, ) = getGroupIdByOffer(_group.offerIds[i]); @@ -100,10 +106,14 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { /** * @notice Validates that condition parameters make sense. * - * A invalid condition is one that: - * - EvaluationMethod.None and has fields different from 0 - * - EvaluationMethod.Threshold and token address or maxCommits is zero - * - EvaluationMethod.SpecificToken and token address or maxCommits is zero or tokenId it's present and length is zero + * A invalid condition is one that fits any of the following criteria: + * - EvaluationMethod.None: any fields different from 0 + * - EvaluationMethod.Threshold: token address, maxCommits or threshold is zero and length is not zero + * - EvaluationMethod.SpecificToken: + - tokenType is FungibleToken + - tokenType is NonFungibleToken and threshold is not zero + - maxCommits is zero + - token address is zero * * @param _condition - fully populated condition struct * @return valid - validity of condition @@ -121,12 +131,16 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { _condition.maxCommits > 0 && _condition.threshold > 0 && _condition.length == 0); - } else if (_condition.method == EvaluationMethod.SpecificToken) { + } else { + // SpecificToken valid = (_condition.tokenAddress != address(0) && - _condition.threshold == 0 && _condition.maxCommits > 0 && - _condition.tokenType != TokenType.FungibleToken && // FungibleToken not allowed for SpecificToken - (_condition.tokenType == TokenType.MultiToken ? _condition.length == 0 : true)); // length must be zero for MultiToken + _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken + + // SpecificToken with NonFungibleToken should not have threshold + if (_condition.tokenType == TokenType.NonFungibleToken) { + valid = valid && _condition.threshold == 0; + } } } @@ -162,7 +176,13 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { for (uint256 i = 0; i < _offerIds.length; i++) { uint256 offerId = _offerIds[i]; // make sure offer exist and belong to the seller - getValidOffer(offerId); + Offer storage offer = getValidOffer(offerId); + + // Get seller, we assume seller exists if offer exists + (, Seller storage seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); // Offer should not belong to another group already (bool exist, ) = getGroupIdByOffer(offerId); diff --git a/contracts/protocol/bases/OfferBase.sol b/contracts/protocol/bases/OfferBase.sol index 1b5dc9bcd..ee50226dd 100644 --- a/contracts/protocol/bases/OfferBase.sol +++ b/contracts/protocol/bases/OfferBase.sol @@ -302,6 +302,12 @@ contract OfferBase is ProtocolBase, IBosonOfferEvents { // Get offer, make sure the caller is the assistant Offer storage offer = getValidOffer(_offerId); + // Get seller, we assume seller exists if offer exists + (, Seller storage seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); + // Prevent reservation of an empty range require(_length > 0, INVALID_RANGE_LENGTH); diff --git a/contracts/protocol/bases/ProtocolBase.sol b/contracts/protocol/bases/ProtocolBase.sol index f87a2023f..7686a775f 100644 --- a/contracts/protocol/bases/ProtocolBase.sol +++ b/contracts/protocol/bases/ProtocolBase.sol @@ -519,13 +519,11 @@ abstract contract ProtocolBase is PausableBase, ReentrancyGuardBase { * Reverts if: * - Offer does not exist * - Offer already voided - * - Caller is not the seller * * @param _offerId - the id of the offer to check */ function getValidOffer(uint256 _offerId) internal view returns (Offer storage offer) { bool exists; - Seller storage seller; // Get offer (exists, offer) = fetchOffer(_offerId); @@ -535,12 +533,6 @@ abstract contract ProtocolBase is PausableBase, ReentrancyGuardBase { // Offer must not already be voided require(!offer.voided, OFFER_HAS_BEEN_VOIDED); - - // Get seller, we assume seller exists if offer exists - (, seller, ) = fetchSeller(offer.sellerId); - - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); } /** diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index c00ac7502..57add7441 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -62,13 +62,13 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * - Offer's quantity available is zero * - Buyer address is zero * - Buyer account is inactive - * - Buyer is token-gated (conditional commit requirements not met or already used) * - Offer price is in native token and caller does not send enough * - Offer price is in some ERC20 token and caller also sends native currency * - Contract at token address does not support ERC20 function transferFrom * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) * - Received ERC20 token amount differs from the expected value * - Seller has less funds available than sellerDeposit + * - Offer belongs to a group with a condition * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) * @param _offerId - the id of the offer to commit to @@ -77,7 +77,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { address payable _buyer, uint256 _offerId ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { - Offer storage offer = validateOffer(_buyer, _offerId); + // Make sure buyer address is not zero address + require(_buyer != address(0), INVALID_ADDRESS); + + Offer storage offer = getValidOffer(_offerId); // For there to be a condition, there must be a group. (bool exists, ) = getGroupIdByOffer(offer.id); @@ -88,12 +91,42 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { commitToOfferInternal(_buyer, offer, 0, false); } + /** + * @notice Commits to an conditional offer (first step of an exchange). + * + * Emits a BuyerCommitted event if successful. + * Issues a voucher to the buyer address. + * + * Reverts if: + * - The exchanges region of protocol is paused + * - The buyers region of protocol is paused + * - OfferId is invalid + * - Offer has been voided + * - Offer has expired + * - Offer is not yet available for commits + * - Offer's quantity available is zero + * - Buyer address is zero + * - Buyer account is inactive + * - Conditional commit requirements not met or already used + * - Offer price is in native token and caller does not send enough + * - Offer price is in some ERC20 token and caller also sends native currency + * - Contract at token address does not support ERC20 function transferFrom + * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) + * - Received ERC20 token amount differs from the expected value + * - Seller has less funds available than sellerDeposit + * + * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) + * @param _offerId - the id of the offer to commit to + */ function commitToConditionalOffer( address payable _buyer, uint256 _offerId, uint256 _tokenId ) external payable override exchangesNotPaused buyersNotPaused nonReentrant { - Offer storage offer = validateOffer(_buyer, _offerId); + // Make sure buyer address is not zero address + require(_buyer != address(0), INVALID_ADDRESS); + + Offer storage offer = getValidOffer(_offerId); // For there to be a condition, there must be a group. (bool exists, uint256 groupId) = getGroupIdByOffer(offer.id); @@ -112,18 +145,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { protocolLookups().exchangeCondition[exchangeId] = condition; } - function validateOffer(address _buyer, uint256 _offerId) internal view returns (Offer storage offer) { - // Make sure buyer address is not zero address - require(_buyer != address(0), INVALID_ADDRESS); - - // Get the offer - bool exists; - (exists, offer) = fetchOffer(_offerId); - - // Make sure offer exists, is available, and isn't void, expired, or sold out - require(exists, NO_SUCH_OFFER); - } - /** * @notice Commits to a preminted offer (first step of an exchange). * @@ -139,6 +160,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * - Offer is not yet available for commits * - Buyer account is inactive * - Buyer is token-gated (conditional commit requirements not met or already used) + * - Buyer is token-gated and evaluation method is SpecificToken * - Seller has less funds available than sellerDeposit and price * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) @@ -150,8 +172,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { uint256 _offerId, uint256 _exchangeId ) external exchangesNotPaused buyersNotPaused nonReentrant { - // Fetch the offer info - (, Offer storage offer) = fetchOffer(_offerId); + Offer storage offer = getValidOffer(_offerId); // Make sure that the voucher was issued on the clone that is making a call require(msg.sender == protocolLookups().cloneAddress[offer.sellerId], ACCESS_DENIED); @@ -160,9 +181,19 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { (bool exists, ) = fetchExchange(_exchangeId); require(!exists, EXCHANGE_ALREADY_EXISTS); - (exists, ) = getGroupIdByOffer(offer.id); + uint256 groupId; + (exists, groupId) = getGroupIdByOffer(offer.id); - require(!exists, GROUP_HAS_CONDITION); + if (exists) { + // Get the condition + Condition storage condition = fetchCondition(groupId); + + // Make sure condition is not SpecificToken as it is not supported for preminted offers + require(condition.method != EvaluationMethod.SpecificToken, CANNOT_COMMIT); + + bool allow = authorizeCommit(_buyer, condition, groupId, 0); + require(allow, CANNOT_COMMIT); + } commitToOfferInternal(_buyer, offer, _exchangeId, true); } @@ -193,7 +224,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _offer - storage pointer to the offer * @param _exchangeId - the id of the exchange * @param _isPreminted - whether the offer is preminted - * @return _exchangeId - the id of the exchange */ function commitToOfferInternal( address payable _buyer, @@ -205,7 +235,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Make sure offer is available, and isn't void, expired, or sold out 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); if (!_isPreminted) { @@ -927,7 +956,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); if (_condition.method == EvaluationMethod.SpecificToken) { - // How many times has this address committed to offers in the group? + // How many times has this token id been used to commit to offers in the group? uint256 commitCount = lookups.conditionalCommitsByTokenId[_groupId][_tokenId]; require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); @@ -940,7 +969,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { ); } - allow = IERC721(_condition.tokenAddress).ownerOf(_tokenId) == _buyer; + allow = holdsSpecificToken(_buyer, _condition, _tokenId); if (allow) { // Increment number of commits to the group for this token id if they are allowed to commit @@ -959,7 +988,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount; } } else { - revert(GROUP_HAS_NO_CONDITION); + // No condition set, so allow the commit + allow = true; } } @@ -984,6 +1014,28 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { return balance >= _condition.threshold; } + /** + * @notice If token is ERC721, checks if the buyer owns the token. If token is ERC1155, checks if the buyer has the required balance, i.e at least the threshold. + * + * @param _buyer - address of potential buyer + * @param _condition - the condition to be evaluated + * @param _tokenId - the token id that buyer is supposed to own + * + * @return bool - true if buyer meets the condition + */ + function holdsSpecificToken( + address _buyer, + Condition storage _condition, + uint256 _tokenId + ) internal view returns (bool) { + if (_condition.tokenType == TokenType.MultiToken) { + return IERC1155(_condition.tokenAddress).balanceOf(_buyer, _tokenId) > _condition.threshold; + } else { + // no need to check if is NonFungible token there is no way to create a SpecifiedToken condition with a Fungible token + return (IERC721(_condition.tokenAddress).ownerOf(_tokenId) == _buyer); + } + } + /** * @notice Gets exchange receipt. * diff --git a/contracts/protocol/facets/OfferHandlerFacet.sol b/contracts/protocol/facets/OfferHandlerFacet.sol index 0b565fd3e..89b02b8c3 100644 --- a/contracts/protocol/facets/OfferHandlerFacet.sol +++ b/contracts/protocol/facets/OfferHandlerFacet.sol @@ -166,6 +166,12 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { // Get offer, make sure the caller is the assistant Offer storage offer = getValidOffer(_offerId); + // Get seller, we assume seller exists if offer exists + (, Seller storage seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); + // Void the offer offer.voided = true; @@ -216,6 +222,12 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { // Make sure the caller is the assistant, offer exists and is not voided Offer storage offer = getValidOffer(_offerId); + // Get seller, we assume seller exists if offer exists + (, Seller storage seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); + // Fetch the offer dates OfferDates storage offerDates = fetchOfferDates(_offerId); From b9f90701083b5eced000e8fc4510f39ff59155db Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 18 May 2023 14:51:22 -0300 Subject: [PATCH 11/31] Fix SnapshopGate tests --- .../example/SnapshotGate/SnapshotGate.sol | 4 +- contracts/protocol/bases/GroupBase.sol | 5 + .../protocol/facets/ExchangeHandlerFacet.sol | 13 ++- test/example/SnapshotGateTest.js | 15 ++- test/protocol/ExchangeHandlerTest.js | 103 ++++++++++++------ 5 files changed, 96 insertions(+), 44 deletions(-) diff --git a/contracts/example/SnapshotGate/SnapshotGate.sol b/contracts/example/SnapshotGate/SnapshotGate.sol index 0d3fadf6b..7b9461fc9 100644 --- a/contracts/example/SnapshotGate/SnapshotGate.sol +++ b/contracts/example/SnapshotGate/SnapshotGate.sol @@ -248,13 +248,13 @@ contract SnapshotGate is BosonTypes, Ownable, ERC721 { require(msg.value == offer.price, "Incorrect payment amount"); // Commit to the offer, passing the message value (native) - IBosonExchangeHandler(protocol).commitToOffer{ value: msg.value }(_buyer, _offerId); + IBosonExchangeHandler(protocol).commitToConditionalOffer{ value: msg.value }(_buyer, _offerId, _tokenId); } else { // Transfer the price into custody of this contract and approve protocol to transfer transferFundsToGateAndApproveProtocol(offer.exchangeToken, offer.price); // Commit to the offer on behalf of the buyer - IBosonExchangeHandler(protocol).commitToOffer(_buyer, _offerId); + IBosonExchangeHandler(protocol).commitToConditionalOffer(_buyer, _offerId, _tokenId); } // Remove the transaction details diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 3d6e0d20e..0210050e6 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -140,6 +140,11 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { // SpecificToken with NonFungibleToken should not have threshold if (_condition.tokenType == TokenType.NonFungibleToken) { valid = valid && _condition.threshold == 0; + + if (_condition.tokenId != 0) { + // SpecificToken with NonFungibleToken and tokenId should have length + valid = valid && _condition.length > 0; + } } } } diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 57add7441..969d60ae5 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.9; -import "hardhat/console.sol"; import { IBosonExchangeHandler } from "../../interfaces/handlers/IBosonExchangeHandler.sol"; import { IBosonAccountHandler } from "../../interfaces/handlers/IBosonAccountHandler.sol"; import { IBosonVoucher } from "../../interfaces/clients/IBosonVoucher.sol"; @@ -83,10 +82,14 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { Offer storage offer = getValidOffer(_offerId); // For there to be a condition, there must be a group. - (bool exists, ) = getGroupIdByOffer(offer.id); + (bool exists, uint256 groupId) = getGroupIdByOffer(offer.id); + if (exists) { + // Get the condition + Condition storage condition = fetchCondition(groupId); - // Make sure offer doesn't have a condition. If it does, use commitToConditionalOffer instead. - require(!exists, GROUP_HAS_CONDITION); + // Make sure group doesn't have a condition. If it does, use commitToConditionalOffer instead. + require(condition.method == EvaluationMethod.None, GROUP_HAS_CONDITION); + } commitToOfferInternal(_buyer, offer, 0, false); } @@ -224,6 +227,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _offer - storage pointer to the offer * @param _exchangeId - the id of the exchange * @param _isPreminted - whether the offer is preminted + * @return exchangeId - the id of the exchange */ function commitToOfferInternal( address payable _buyer, @@ -1011,6 +1015,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { } else { balance = IERC20(_condition.tokenAddress).balanceOf(_buyer); } + return balance >= _condition.threshold; } diff --git a/test/example/SnapshotGateTest.js b/test/example/SnapshotGateTest.js index 27ae42836..b143e8baa 100644 --- a/test/example/SnapshotGateTest.js +++ b/test/example/SnapshotGateTest.js @@ -2,6 +2,7 @@ const hre = require("hardhat"); const ethers = hre.ethers; const { expect } = require("chai"); +const { RevertReasons } = require("../../scripts/config/revert-reasons.js"); const Role = require("../../scripts/domain/Role"); const TokenType = require("../../scripts/domain/TokenType"); const Group = require("../../scripts/domain/Group"); @@ -63,6 +64,9 @@ describe("SnapshotGate", function () { let snapshot, snapshotTokenSupplies, snapshotTokenCount, holders, holderByAddress; beforeEach(async function () { + // Reset the accountId iterator + accountId.next(true); + // Make accounts available [ deployer, @@ -330,6 +334,7 @@ describe("SnapshotGate", function () { tokenType: TokenType.NonFungibleToken, tokenId: tokenId, method: EvaluationMethod.SpecificToken, + length: "1", }); expect(condition.isValid()).to.be.true; @@ -687,7 +692,7 @@ describe("SnapshotGate", function () { // Commit to the offer await expect( snapshotGate.connect(holder).commitToGatedOffer(entry.owner, offerId, entry.tokenId, { value: price }) - ).to.revertedWith("Condition specifies a different tokenId from the one given"); + ).to.revertedWith(RevertReasons.TOKEN_ID_NOT_IN_CONDITION_RANGE); }); it("offer is from another seller", async function () { @@ -870,7 +875,7 @@ describe("SnapshotGate", function () { // Relevant Boson Protocol methods context("📋 Protocol Methods", async function () { - context("👉 commitToOffer()", async function () { + context("👉 commitToConditionalOffer()", async function () { context("💔 Revert Reasons", async function () { it("buyer is in snapshot but attempts to commit directly on protocol", async function () { // Upload the snapshot @@ -889,9 +894,9 @@ describe("SnapshotGate", function () { let holder = holderByAddress[entry.owner]; // Check that holder cannot commit directly to the offer on the protocol itself - await expect(exchangeHandler.connect(holder).commitToOffer(holder.address, offerId)).to.revertedWith( - "Caller cannot commit" - ); + await expect( + exchangeHandler.connect(holder).commitToConditionalOffer(holder.address, offerId, entry.tokenId) + ).to.revertedWith("Caller cannot commit"); }); }); }); diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index c379e193d..5f56aba4d 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -891,6 +891,30 @@ describe("IBosonExchangeHandler", function () { ).to.emit(exchangeHandler, "BuyerCommitted"); }); + it("Offer belongs to a group with condition and is not type EvaluationMethod.SpecificToken", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + // Create Condition + condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + // mint enough tokens for the buyer + await foreign20.connect(buyer).mint(buyer.address, condition.threshold); + + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); + }); + context("💔 Revert Reasons", async function () { it("The exchanges region of protocol is paused", async function () { // Pause the exchanges region of the protocol @@ -1002,24 +1026,35 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); - it("Offer belongs to a group with condition", async function () { + it("Offer belongs to a group with condition and is type EvaluationMethod.SpecificToken", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; - // Create Condition - condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + condition = mockCondition({ + tokenAddress: foreign721.address, + threshold: "0", + maxCommits: "3", + tokenType: TokenType.NonFungibleToken, + tokenId: "1", + method: EvaluationMethod.SpecificToken, + length: "10", + }); + expect(condition.isValid()).to.be.true; // Create Group group = new Group(groupId, seller.id, offerIds); expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); - // Attempt to commit to the offer with, expecting revert + // mint enough tokens for the buyer + await foreign721.connect(buyer).mint(condition.tokenId, 1); + await expect( bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) - ).to.revertedWith(RevertReasons.GROUP_HAS_CONDITION); + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); }); }); @@ -1347,6 +1382,36 @@ describe("IBosonExchangeHandler", function () { }); }); + context("✋ Group without condition", async function () { + let tokenId = "0"; + beforeEach(async function () { + // Create a new offer + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + // Required constructor params for Group + groupId = "1"; + offerIds = [(++offerId).toString()]; + + // Create Condition + condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + }); + + it("should emit a BuyerCommitted event", async function () { + // Commit to offer. + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); + }); + }); + context("💔 Revert Reasons", async function () { let tokenId; @@ -1481,34 +1546,6 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); - - context("✋ Group without condition", async function () { - beforeEach(async function () { - // Create a new offer - await offerHandler - .connect(assistant) - .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); - - // Required constructor params for Group - groupId = "1"; - offerIds = [(++offerId).toString()]; - - // Create Condition - condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); - expect(condition.isValid()).to.be.true; - - // Create Group - group = new Group(groupId, seller.id, offerIds); - expect(group.isValid()).is.true; - await groupHandler.connect(assistant).createGroup(group, condition); - }); - - it("Should revert when using this method", async function () { - await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) - ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); - }); - }); }); }); From e09749bed4b07fb4a586202adeb2655b559795bb Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 18 May 2023 21:00:44 -0300 Subject: [PATCH 12/31] Add tests to validate 1155 with SpecificToken --- contracts/protocol/bases/BundleBase.sol | 6 + .../protocol/facets/ExchangeHandlerFacet.sol | 3 +- test/protocol/ExchangeHandlerTest.js | 171 ++++++++++++++++++ 3 files changed, 178 insertions(+), 2 deletions(-) diff --git a/contracts/protocol/bases/BundleBase.sol b/contracts/protocol/bases/BundleBase.sol index ec85a7e38..1e7b7c0c3 100644 --- a/contracts/protocol/bases/BundleBase.sol +++ b/contracts/protocol/bases/BundleBase.sol @@ -173,6 +173,12 @@ contract BundleBase is ProtocolBase, IBosonBundleEvents { // make sure all offers exist and belong to the seller Offer storage offer = getValidOffer(_offerId); + // Get seller, we assume seller exists if offer exists + (, Seller storage seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); + // Unchecked because we're handling overflow below unchecked { // Calculate the bundle offers total quantity available. diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 969d60ae5..8073fb635 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.9; - import { IBosonExchangeHandler } from "../../interfaces/handlers/IBosonExchangeHandler.sol"; import { IBosonAccountHandler } from "../../interfaces/handlers/IBosonAccountHandler.sol"; import { IBosonVoucher } from "../../interfaces/clients/IBosonVoucher.sol"; @@ -1034,7 +1033,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { uint256 _tokenId ) internal view returns (bool) { if (_condition.tokenType == TokenType.MultiToken) { - return IERC1155(_condition.tokenAddress).balanceOf(_buyer, _tokenId) > _condition.threshold; + return IERC1155(_condition.tokenAddress).balanceOf(_buyer, _tokenId) >= _condition.threshold; } else { // no need to check if is NonFungible token there is no way to create a SpecifiedToken condition with a Fungible token return (IERC721(_condition.tokenAddress).ownerOf(_tokenId) == _buyer); diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 5f56aba4d..409ccac70 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -636,6 +636,30 @@ describe("IBosonExchangeHandler", function () { ); }); + it("If group has no condition, buyers can commit using this method", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + // Create Condition + condition = mockCondition({ + method: EvaluationMethod.None, + tokenAddress: ethers.constants.AddressZero, + threshold: "0", + maxCommits: "0", + }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + + await expect( + exchangeHandler.connect(buyer).commitToOffer(buyer.address, offerId, { value: price }) + ).to.not.reverted; + }); + context("💔 Revert Reasons", async function () { it("The exchanges region of protocol is paused", async function () { // Pause the exchanges region of the protocol @@ -1056,6 +1080,33 @@ describe("IBosonExchangeHandler", function () { bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); + + it("buyer does not meet condition for commit", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + condition = mockCondition({ + tokenAddress: foreign721.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.NonFungibleToken, + tokenId: "1", + method: EvaluationMethod.Threshold, + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + }); }); }); @@ -1124,6 +1175,17 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); + + it("Group doesn't exist", async function () { + // Create a new offer + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, ++offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.NO_SUCH_GROUP); + }); }); }); @@ -1382,6 +1444,115 @@ describe("IBosonExchangeHandler", function () { }); }); + context("✋ SpecificToken ERC1155", async function () { + let tokenId; + beforeEach(async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + tokenId = "12"; + + // Create Condition + condition = mockCondition({ + tokenAddress: foreign1155.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.MultiToken, + tokenId, + method: EvaluationMethod.SpecificToken, + length: "10", + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + + // mint correct token for the buyer + await foreign1155.connect(buyer).mint(tokenId, "1"); + }); + + it("should emit a BuyerCommitted event if user meets condition", async function () { + // Commit to offer. + // We're only concerned that the event is emitted, indicating the condition was met + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); + }); + + it("should allow buyer to commit up to the max times for the group", async function () { + // Commit to offer the maximum number of times + for (let i = 0; i < Number(condition.maxCommits); i++) { + // We're only concerned that the event is emitted, indicating the commit was allowed + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.emit(exchangeHandler, "BuyerCommitted"); + } + }); + + it("If tokenId is not specified, should allow any token from collection", async function () { + condition.tokenId = "0"; + condition.length = "0"; + + // remove offer from old group + await groupHandler.connect(assistant).removeOffersFromGroup(groupId, offerIds); + + // create new group with new condition + await groupHandler.connect(assistant).createGroup(group, condition); + + // mint any token for buyer + tokenId = "123"; + await foreign1155.connect(buyer).mint(tokenId, "1"); + + // buyer can commit + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.not.reverted; + }); + + context("💔 Revert Reasons", async function () { + it("token id does not exist", async function () { + tokenId = "13"; + + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + }); + + it("buyer does not meet condition for commit", async function () { + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(rando).commitToConditionalOffer(rando.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + }); + + it("max commits per token id reached", async function () { + // Commit to offer the maximum number of times + for (let i = 0; i < Number(condition.maxCommits); i++) { + await exchangeHandler + .connect(buyer) + .commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }); + } + + // Attempt to commit again after maximum commits has been reached + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.MAX_COMMITS_TOKEN_REACHED); + }); + + it("token id not in condition range", async function () { + tokenId = "666"; + // Attempt to commit, expecting revert + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.TOKEN_ID_NOT_IN_CONDITION_RANGE); + }); + }); + }); + context("✋ Group without condition", async function () { let tokenId = "0"; beforeEach(async function () { From c50622d568f9a8dada638968b8a9d6e53af8ce60 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 18 May 2023 21:07:33 -0300 Subject: [PATCH 13/31] Add mising validation to group creation --- contracts/protocol/bases/GroupBase.sol | 13 +++++++++---- test/protocol/GroupHandlerTest.js | 11 ++++++++++- 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 0210050e6..353ef1247 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -110,10 +110,12 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * - EvaluationMethod.None: any fields different from 0 * - EvaluationMethod.Threshold: token address, maxCommits or threshold is zero and length is not zero * - EvaluationMethod.SpecificToken: - - tokenType is FungibleToken - - tokenType is NonFungibleToken and threshold is not zero - - maxCommits is zero - - token address is zero + * - tokenType is FungibleToken + * - tokenType is NonFungibleToken and threshold is not zero + * - tokenType is NonFunbileToken, tokenId is not zero and length is zero + * - tokenType is MultiToken and threshold is zero + * - maxCommits is zero + * - token address is zero * * @param _condition - fully populated condition struct * @return valid - validity of condition @@ -145,6 +147,9 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { // SpecificToken with NonFungibleToken and tokenId should have length valid = valid && _condition.length > 0; } + } else { + // SpecificToken with MultiToken should have threshold + valid = valid && _condition.threshold > 0; } } } diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index a3e6bfdac..ad8c61fbd 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -503,6 +503,15 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); + + it("Length is zero when tokenId is not zero", async function () { + condition.length = "0"; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); }); }); }); @@ -846,7 +855,7 @@ describe("IBosonGroupHandler", function () { condition = mockCondition({ tokenAddress: accounts[1].address, tokenId: "88775544", - threshold: "0", + threshold: "1", tokenType: TokenType.MultiToken, method: EvaluationMethod.SpecificToken, }); From 2b8f815069c8026fa653f4bf51d21becce9a9a38 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 22 May 2023 13:38:52 -0300 Subject: [PATCH 14/31] Review changes --- .../handlers/IBosonExchangeHandler.sol | 28 +++++++++ contracts/protocol/bases/BundleBase.sol | 8 +-- contracts/protocol/bases/GroupBase.sol | 30 +++------- contracts/protocol/bases/OfferBase.sol | 8 +-- contracts/protocol/bases/ProtocolBase.sol | 23 ++++++++ .../protocol/facets/ExchangeHandlerFacet.sol | 7 ++- .../protocol/facets/OfferHandlerFacet.sol | 18 +----- contracts/protocol/libs/ProtocolLib.sol | 2 +- test/protocol/ExchangeHandlerTest.js | 57 +++++++++---------- test/protocol/GroupHandlerTest.js | 1 + 10 files changed, 99 insertions(+), 83 deletions(-) diff --git a/contracts/interfaces/handlers/IBosonExchangeHandler.sol b/contracts/interfaces/handlers/IBosonExchangeHandler.sol index 1840925d0..14e54b7d0 100644 --- a/contracts/interfaces/handlers/IBosonExchangeHandler.sol +++ b/contracts/interfaces/handlers/IBosonExchangeHandler.sol @@ -43,6 +43,34 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I */ function commitToOffer(address payable _buyer, uint256 _offerId) external payable; + /** + * @notice Commits to an conditional offer (first step of an exchange). + * + * Emits a BuyerCommitted event if successful. + * Issues a voucher to the buyer address. + * + * Reverts if: + * - The exchanges region of protocol is paused + * - The buyers region of protocol is paused + * - OfferId is invalid + * - Offer has been voided + * - Offer has expired + * - Offer is not yet available for commits + * - Offer's quantity available is zero + * - Buyer address is zero + * - Buyer account is inactive + * - Conditional commit requirements not met or already used + * - Offer price is in native token and caller does not send enough + * - Offer price is in some ERC20 token and caller also sends native currency + * - Contract at token address does not support ERC20 function transferFrom + * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) + * - Received ERC20 token amount differs from the expected value + * - Seller has less funds available than sellerDeposit + * + * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) + * @param _offerId - the id of the offer to commit to + * @param _tokenId - the id of the token to use for the conditional commit + */ function commitToConditionalOffer(address payable _buyer, uint256 _offerId, uint256 _tokenId) external payable; /** diff --git a/contracts/protocol/bases/BundleBase.sol b/contracts/protocol/bases/BundleBase.sol index 1e7b7c0c3..298c3804c 100644 --- a/contracts/protocol/bases/BundleBase.sol +++ b/contracts/protocol/bases/BundleBase.sol @@ -171,13 +171,7 @@ contract BundleBase is ProtocolBase, IBosonBundleEvents { uint256 _offerId ) internal view returns (uint256 offersTotalQuantity) { // make sure all offers exist and belong to the seller - Offer storage offer = getValidOffer(_offerId); - - // Get seller, we assume seller exists if offer exists - (, Seller storage seller, ) = fetchSeller(offer.sellerId); - - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); + (Offer storage offer, ) = getValidOfferAndSeller(_offerId); // Unchecked because we're handling overflow below unchecked { diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 353ef1247..9980435da 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -49,13 +49,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { for (uint256 i = 0; i < _group.offerIds.length; i++) { // make sure offer exists and belongs to the seller - Offer storage offer = getValidOffer(_group.offerIds[i]); - - // Get seller, we assume seller exists if offer exists - (, Seller storage seller, ) = fetchSeller(offer.sellerId); - - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); + getValidOfferAndSeller(_group.offerIds[i]); // Offer should not belong to another group already (bool exist, ) = getGroupIdByOffer(_group.offerIds[i]); @@ -108,11 +102,11 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * * A invalid condition is one that fits any of the following criteria: * - EvaluationMethod.None: any fields different from 0 - * - EvaluationMethod.Threshold: token address, maxCommits or threshold is zero and length is not zero + * - EvaluationMethod.Threshold: token address, maxCommits or threshold is zero or length is not zero * - EvaluationMethod.SpecificToken: * - tokenType is FungibleToken * - tokenType is NonFungibleToken and threshold is not zero - * - tokenType is NonFunbileToken, tokenId is not zero and length is zero + * - tokenId is not zero and length is zero * - tokenType is MultiToken and threshold is zero * - maxCommits is zero * - token address is zero @@ -139,14 +133,14 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { _condition.maxCommits > 0 && _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken + if (_condition.tokenId != 0) { + // SpecificToken with tokenId should have length + valid = valid && _condition.length > 0; + } + // SpecificToken with NonFungibleToken should not have threshold if (_condition.tokenType == TokenType.NonFungibleToken) { valid = valid && _condition.threshold == 0; - - if (_condition.tokenId != 0) { - // SpecificToken with NonFungibleToken and tokenId should have length - valid = valid && _condition.length > 0; - } } else { // SpecificToken with MultiToken should have threshold valid = valid && _condition.threshold > 0; @@ -185,14 +179,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { for (uint256 i = 0; i < _offerIds.length; i++) { uint256 offerId = _offerIds[i]; - // make sure offer exist and belong to the seller - Offer storage offer = getValidOffer(offerId); - - // Get seller, we assume seller exists if offer exists - (, Seller storage seller, ) = fetchSeller(offer.sellerId); - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); + getValidOfferAndSeller(offerId); // Offer should not belong to another group already (bool exist, ) = getGroupIdByOffer(offerId); diff --git a/contracts/protocol/bases/OfferBase.sol b/contracts/protocol/bases/OfferBase.sol index ee50226dd..93fab6810 100644 --- a/contracts/protocol/bases/OfferBase.sol +++ b/contracts/protocol/bases/OfferBase.sol @@ -300,13 +300,7 @@ contract OfferBase is ProtocolBase, IBosonOfferEvents { address _to ) internal offersNotPaused exchangesNotPaused { // Get offer, make sure the caller is the assistant - Offer storage offer = getValidOffer(_offerId); - - // Get seller, we assume seller exists if offer exists - (, Seller storage seller, ) = fetchSeller(offer.sellerId); - - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); + (Offer storage offer, ) = getValidOfferAndSeller(_offerId); // Prevent reservation of an empty range require(_length > 0, INVALID_RANGE_LENGTH); diff --git a/contracts/protocol/bases/ProtocolBase.sol b/contracts/protocol/bases/ProtocolBase.sol index 7686a775f..e5ee5e6a8 100644 --- a/contracts/protocol/bases/ProtocolBase.sol +++ b/contracts/protocol/bases/ProtocolBase.sol @@ -535,6 +535,29 @@ abstract contract ProtocolBase is PausableBase, ReentrancyGuardBase { require(!offer.voided, OFFER_HAS_BEEN_VOIDED); } + /** + * @notice Gets offer and seller from protocol storage + * + * Reverts if: + * - Offer does not exist + * - Offer already voided + * - Seller assistant is not the caller + * + * @param _offerId - the id of the offer to check + */ + function getValidOfferAndSeller( + uint256 _offerId + ) internal view returns (Offer storage offer, Seller storage seller) { + // Get offer + offer = getValidOffer(_offerId); + + // Get seller, we assume seller exists if offer exists + (, seller, ) = fetchSeller(offer.sellerId); + + // Caller must be seller's assistant address + require(seller.assistant == msgSender(), NOT_ASSISTANT); + } + /** * @notice Gets the bundle id for a given offer id. * diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 8073fb635..06a0cf140 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -119,6 +119,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) * @param _offerId - the id of the offer to commit to + * @param _tokenId - the id of the token to use for the conditional commit */ function commitToConditionalOffer( address payable _buyer, @@ -139,6 +140,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); + require(condition.method != EvaluationMethod.None, GROUP_HAS_NO_CONDITION); + bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId); require(allow, CANNOT_COMMIT); @@ -960,7 +963,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { if (_condition.method == EvaluationMethod.SpecificToken) { // How many times has this token id been used to commit to offers in the group? - uint256 commitCount = lookups.conditionalCommitsByTokenId[_groupId][_tokenId]; + uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId]; require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); @@ -976,7 +979,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { if (allow) { // Increment number of commits to the group for this token id if they are allowed to commit - lookups.conditionalCommitsByTokenId[_groupId][_tokenId] = ++commitCount; + lookups.conditionalCommitsByTokenId[_tokenId][_groupId] = ++commitCount; } } else if (_condition.method == EvaluationMethod.Threshold) { // How many times has this address committed to offers in the group? diff --git a/contracts/protocol/facets/OfferHandlerFacet.sol b/contracts/protocol/facets/OfferHandlerFacet.sol index 89b02b8c3..b72a94169 100644 --- a/contracts/protocol/facets/OfferHandlerFacet.sol +++ b/contracts/protocol/facets/OfferHandlerFacet.sol @@ -163,14 +163,8 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { * @param _offerId - the id of the offer to void */ function voidOffer(uint256 _offerId) public override offersNotPaused nonReentrant { - // Get offer, make sure the caller is the assistant - Offer storage offer = getValidOffer(_offerId); - - // Get seller, we assume seller exists if offer exists - (, Seller storage seller, ) = fetchSeller(offer.sellerId); - - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); + // Get offer. Make sure caller is assistant + (Offer storage offer, ) = getValidOfferAndSeller(_offerId); // Void the offer offer.voided = true; @@ -220,13 +214,7 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { */ function extendOffer(uint256 _offerId, uint256 _validUntilDate) public override offersNotPaused nonReentrant { // Make sure the caller is the assistant, offer exists and is not voided - Offer storage offer = getValidOffer(_offerId); - - // Get seller, we assume seller exists if offer exists - (, Seller storage seller, ) = fetchSeller(offer.sellerId); - - // Caller must be seller's assistant address - require(seller.assistant == msgSender(), NOT_ASSISTANT); + (Offer storage offer, ) = getValidOfferAndSeller(_offerId); // Fetch the offer dates OfferDates storage offerDates = fetchOfferDates(_offerId); diff --git a/contracts/protocol/libs/ProtocolLib.sol b/contracts/protocol/libs/ProtocolLib.sol index 63c0b8bae..681509b7a 100644 --- a/contracts/protocol/libs/ProtocolLib.sol +++ b/contracts/protocol/libs/ProtocolLib.sol @@ -182,7 +182,7 @@ library ProtocolLib { mapping(uint256 => BosonTypes.AuthToken) pendingAuthTokenUpdatesBySeller; // dispute resolver id => DisputeResolver mapping(uint256 => BosonTypes.DisputeResolver) pendingAddressUpdatesByDisputeResolver; - // groupId => tokenId => commit count (count how many times a token has been used as gate for this group) + // tokenId => groupId => commit count (count how many times a token has been used as gate for this group) mapping(uint256 => mapping(uint256 => uint256)) conditionalCommitsByTokenId; } diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 409ccac70..90dd68a34 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -1553,36 +1553,6 @@ describe("IBosonExchangeHandler", function () { }); }); - context("✋ Group without condition", async function () { - let tokenId = "0"; - beforeEach(async function () { - // Create a new offer - await offerHandler - .connect(assistant) - .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); - - // Required constructor params for Group - groupId = "1"; - offerIds = [(++offerId).toString()]; - - // Create Condition - condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); - expect(condition.isValid()).to.be.true; - - // Create Group - group = new Group(groupId, seller.id, offerIds); - expect(group.isValid()).is.true; - await groupHandler.connect(assistant).createGroup(group, condition); - }); - - it("should emit a BuyerCommitted event", async function () { - // Commit to offer. - await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) - ).to.emit(exchangeHandler, "BuyerCommitted"); - }); - }); - context("💔 Revert Reasons", async function () { let tokenId; @@ -1717,6 +1687,33 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); + + it("Group without condition ", async function () { + let tokenId = "0"; + + // Create a new offer + await offerHandler + .connect(assistant) + .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); + + // Required constructor params for Group + groupId = "1"; + offerIds = [(++offerId).toString()]; + + // Create Condition + condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + await groupHandler.connect(assistant).createGroup(group, condition); + + // Commit to offer. + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); + }); }); }); diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index ad8c61fbd..e806888e9 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -858,6 +858,7 @@ describe("IBosonGroupHandler", function () { threshold: "1", tokenType: TokenType.MultiToken, method: EvaluationMethod.SpecificToken, + length: "1", }); expect(condition.isValid()).to.be.true; From bf001c898214c6c6c0e5cadc4e5bac511f37de67 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 23 May 2023 10:40:48 -0300 Subject: [PATCH 15/31] Increase MAX_SKIPPED --- .github/workflows/ci.yaml | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index ad75690bf..181834c75 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -12,8 +12,8 @@ jobs: runs-on: ubuntu-latest if: ${{ !github.event.pull_request.draft }} name: build - outputs: - test-chunks: ${{ steps['set-test-chunks'].outputs['test-chunks'] }} + outputs: + test-chunks: ${{ steps['set-test-chunks'].outputs['test-chunks'] }} test-chunk-ids: ${{ steps['set-test-chunk-ids'].outputs['test-chunk-ids'] }} steps: - name: Checkout repository @@ -72,8 +72,8 @@ jobs: run: npm run build - name: Contract Sizing run: npm run size - - name: Unit Tests - run: echo $CHUNKS | jq '.[${{ matrix.chunk }}] | .[] | @text' | xargs npx hardhat test + - name: Unit Tests + run: echo $CHUNKS | jq '.[${{ matrix.chunk }}] | .[] | @text' | xargs npx hardhat test env: CHUNKS: ${{ needs.build.outputs['test-chunks'] }} @@ -91,7 +91,7 @@ jobs: uses: actions/setup-node@v3.5.1 with: node-version: 16.14.x - cache: 'npm' + cache: "npm" - name: Install Dependencies run: npm install - name: Prepare Environment @@ -134,7 +134,7 @@ jobs: - name: Check Code Coverage shell: bash run: | - MAX_SKIPPED=1 # at this moment msgData() in BosonVoucher is not covered + MAX_SKIPPED=2 # at this moment msgData() in BosonVoucher is not covered { read TOTAL; read COVERED; read COVERAGE; } <<< $(jq '.total.lines.total, .total.lines.covered, .total.lines.pct' coverage/coverage-summary.json) SKIPPED=$(($TOTAL - $COVERED)) echo "solidity code coverage is '$COVERAGE'" @@ -166,19 +166,19 @@ jobs: if: failure() uses: andymckay/cancel-action@0.2 analyze: - needs: build - runs-on: ubuntu-latest - permissions: - contents: read - security-events: write - steps: + needs: build + runs-on: ubuntu-latest + permissions: + contents: read + security-events: write + steps: - name: Checkout repository uses: actions/checkout@v3.1.0 - name: Setup node uses: actions/setup-node@v3.5.1 with: node-version: 16.14.x - cache: 'npm' + cache: "npm" - name: Install Dependencies run: npm install - name: Prepare Environment @@ -197,5 +197,3 @@ jobs: uses: github/codeql-action/upload-sarif@v2 with: sarif_file: ${{ steps.slither.outputs.sarif }} - - From 934b5b94c2e770b5bea68b4cf7d3ed88e67bd52d Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 29 May 2023 15:24:48 -0300 Subject: [PATCH 16/31] Review changes --- contracts/protocol/bases/BundleBase.sol | 2 +- contracts/protocol/bases/GroupBase.sol | 4 ++-- contracts/protocol/bases/OfferBase.sol | 2 +- contracts/protocol/bases/ProtocolBase.sol | 7 +++---- contracts/protocol/facets/OfferHandlerFacet.sol | 4 ++-- 5 files changed, 9 insertions(+), 10 deletions(-) diff --git a/contracts/protocol/bases/BundleBase.sol b/contracts/protocol/bases/BundleBase.sol index 298c3804c..22c2baec6 100644 --- a/contracts/protocol/bases/BundleBase.sol +++ b/contracts/protocol/bases/BundleBase.sol @@ -171,7 +171,7 @@ contract BundleBase is ProtocolBase, IBosonBundleEvents { uint256 _offerId ) internal view returns (uint256 offersTotalQuantity) { // make sure all offers exist and belong to the seller - (Offer storage offer, ) = getValidOfferAndSeller(_offerId); + Offer storage offer = getValidOfferWithSellerCheck(_offerId); // Unchecked because we're handling overflow below unchecked { diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 9980435da..2cd360704 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -49,7 +49,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { for (uint256 i = 0; i < _group.offerIds.length; i++) { // make sure offer exists and belongs to the seller - getValidOfferAndSeller(_group.offerIds[i]); + getValidOfferWithSellerCheck(_group.offerIds[i]); // Offer should not belong to another group already (bool exist, ) = getGroupIdByOffer(_group.offerIds[i]); @@ -180,7 +180,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { for (uint256 i = 0; i < _offerIds.length; i++) { uint256 offerId = _offerIds[i]; - getValidOfferAndSeller(offerId); + getValidOfferWithSellerCheck(offerId); // Offer should not belong to another group already (bool exist, ) = getGroupIdByOffer(offerId); diff --git a/contracts/protocol/bases/OfferBase.sol b/contracts/protocol/bases/OfferBase.sol index 93fab6810..496828fe3 100644 --- a/contracts/protocol/bases/OfferBase.sol +++ b/contracts/protocol/bases/OfferBase.sol @@ -300,7 +300,7 @@ contract OfferBase is ProtocolBase, IBosonOfferEvents { address _to ) internal offersNotPaused exchangesNotPaused { // Get offer, make sure the caller is the assistant - (Offer storage offer, ) = getValidOfferAndSeller(_offerId); + Offer storage offer = getValidOfferWithSellerCheck(_offerId); // Prevent reservation of an empty range require(_length > 0, INVALID_RANGE_LENGTH); diff --git a/contracts/protocol/bases/ProtocolBase.sol b/contracts/protocol/bases/ProtocolBase.sol index e5ee5e6a8..3f6ae493d 100644 --- a/contracts/protocol/bases/ProtocolBase.sol +++ b/contracts/protocol/bases/ProtocolBase.sol @@ -544,15 +544,14 @@ abstract contract ProtocolBase is PausableBase, ReentrancyGuardBase { * - Seller assistant is not the caller * * @param _offerId - the id of the offer to check + * @return offer - the offer details. See {BosonTypes.Offer} */ - function getValidOfferAndSeller( - uint256 _offerId - ) internal view returns (Offer storage offer, Seller storage seller) { + function getValidOfferWithSellerCheck(uint256 _offerId) internal view returns (Offer storage offer) { // Get offer offer = getValidOffer(_offerId); // Get seller, we assume seller exists if offer exists - (, seller, ) = fetchSeller(offer.sellerId); + (, Seller storage seller, ) = fetchSeller(offer.sellerId); // Caller must be seller's assistant address require(seller.assistant == msgSender(), NOT_ASSISTANT); diff --git a/contracts/protocol/facets/OfferHandlerFacet.sol b/contracts/protocol/facets/OfferHandlerFacet.sol index b72a94169..679205dc3 100644 --- a/contracts/protocol/facets/OfferHandlerFacet.sol +++ b/contracts/protocol/facets/OfferHandlerFacet.sol @@ -164,7 +164,7 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { */ function voidOffer(uint256 _offerId) public override offersNotPaused nonReentrant { // Get offer. Make sure caller is assistant - (Offer storage offer, ) = getValidOfferAndSeller(_offerId); + Offer storage offer = getValidOfferWithSellerCheck(_offerId); // Void the offer offer.voided = true; @@ -214,7 +214,7 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { */ function extendOffer(uint256 _offerId, uint256 _validUntilDate) public override offersNotPaused nonReentrant { // Make sure the caller is the assistant, offer exists and is not voided - (Offer storage offer, ) = getValidOfferAndSeller(_offerId); + Offer storage offer = getValidOfferWithSellerCheck(_offerId); // Fetch the offer dates OfferDates storage offerDates = fetchOfferDates(_offerId); From e933c4a787e19aeec7adc4db5f6efe38d6de7d5a Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 30 May 2023 08:28:38 -0300 Subject: [PATCH 17/31] Update MAX_SKIPPED value in Check Code Coverage step of ci.yaml file. --- .github/workflows/ci.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 181834c75..390de758b 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -134,7 +134,7 @@ jobs: - name: Check Code Coverage shell: bash run: | - MAX_SKIPPED=2 # at this moment msgData() in BosonVoucher is not covered + MAX_SKIPPED=1 # at this moment msgData() in BosonVoucher is not covered { read TOTAL; read COVERED; read COVERAGE; } <<< $(jq '.total.lines.total, .total.lines.covered, .total.lines.pct' coverage/coverage-summary.json) SKIPPED=$(($TOTAL - $COVERED)) echo "solidity code coverage is '$COVERAGE'" From 8774854165d0672524057760bba0c8628106617e Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 30 May 2023 11:33:04 -0300 Subject: [PATCH 18/31] Removed unnecessary code in ExchangeHandlerFacet.sol --- contracts/protocol/facets/ExchangeHandlerFacet.sol | 3 --- 1 file changed, 3 deletions(-) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index bf5632e0b..53b0f5423 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -995,9 +995,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Increment number of commits to the group for this address if they are allowed to commit lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount; } - } else { - // No condition set, so allow the commit - allow = true; } } From 57376507111ef23494bd6daf3272c439fd46d529 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Wed, 31 May 2023 15:02:56 -0300 Subject: [PATCH 19/31] Fix per-address with ERC1155 condition --- contracts/domain/BosonConstants.sol | 1 + contracts/protocol/bases/GroupBase.sol | 3 +- .../protocol/facets/ExchangeHandlerFacet.sol | 30 ++++-- scripts/config/revert-reasons.js | 1 + test/protocol/ExchangeHandlerTest.js | 101 +++++++++++++++--- test/protocol/GroupHandlerTest.js | 11 +- test/protocol/OrchestrationHandlerTest.js | 26 ++++- 7 files changed, 144 insertions(+), 29 deletions(-) diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 41a26e281..26c8f44f2 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -105,6 +105,7 @@ string constant GROUP_HAS_CONDITION = "Offer belongs to a group with a condition string constant MAX_COMMITS_ADDRESS_REACHED = "Max commits per address reached"; string constant MAX_COMMITS_TOKEN_REACHED = "Max commits per token id reached"; string constant TOKEN_ID_NOT_IN_CONDITION_RANGE = "Token id not in condition range"; +string constant INVALID_TOKEN_ID = "ERC1155 requires non-zero tokenId; ERC721 and ERC20 require zero tokenId."; // Revert Reasons: Exchange related string constant NO_SUCH_EXCHANGE = "No such exchange"; diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 7f33f4af9..b85c387d7 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -126,7 +126,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { valid = (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0 && - _condition.length == 0); + _condition.length == 0 && + _condition.tokenId == 0); } else { // SpecificToken valid = (_condition.tokenAddress != address(0) && diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 53b0f5423..24db5cfcc 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -196,7 +196,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { Condition storage condition = fetchCondition(groupId); // Make sure condition is not SpecificToken as it is not supported for preminted offers - require(condition.method != EvaluationMethod.SpecificToken, CANNOT_COMMIT); + require( + condition.method == EvaluationMethod.Threshold && condition.tokenType != TokenType.MultiToken, + CANNOT_COMMIT + ); bool allow = authorizeCommit(_buyer, condition, groupId, 0); require(allow, CANNOT_COMMIT); @@ -950,7 +953,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * @param _buyer buyer address * @param _condition - the condition to check * @param _groupId - the group id - * @param _tokenId - the token id. Valid only for SpecificToken evaluation method + * @param _tokenId - the token id * * @return allow - true if buyer is authorized to commit */ @@ -964,11 +967,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); if (_condition.method == EvaluationMethod.SpecificToken) { - // How many times has this token id been used to commit to offers in the group? - uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId]; - - require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); - // If condition has a token id, check that the token id is in range, otherwise accept any token id if (_condition.tokenId > 0) { require( @@ -977,6 +975,11 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { ); } + // How many times has this token id been used to commit to offers in the group? + uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId]; + + require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); + allow = holdsSpecificToken(_buyer, _condition, _tokenId); if (allow) { @@ -984,12 +987,14 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { lookups.conditionalCommitsByTokenId[_tokenId][_groupId] = ++commitCount; } } else if (_condition.method == EvaluationMethod.Threshold) { + require(_condition.tokenType == TokenType.MultiToken ? _tokenId > 0 : _tokenId == 0, INVALID_TOKEN_ID); + // How many times has this address committed to offers in the group? uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId]; require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED); - allow = holdsThreshold(_buyer, _condition); + allow = holdsThreshold(_buyer, _condition, _tokenId); if (allow) { // Increment number of commits to the group for this address if they are allowed to commit @@ -1003,14 +1008,19 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * * @param _buyer - address of potential buyer * @param _condition - the condition to be evaluated + * @param _tokenId - the token id. Valid only for ERC1155 tokens. * * @return bool - true if buyer meets the condition */ - function holdsThreshold(address _buyer, Condition storage _condition) internal view returns (bool) { + function holdsThreshold( + address _buyer, + Condition storage _condition, + uint256 _tokenId + ) internal view returns (bool) { uint256 balance; if (_condition.tokenType == TokenType.MultiToken) { - balance = IERC1155(_condition.tokenAddress).balanceOf(_buyer, _condition.tokenId); + balance = IERC1155(_condition.tokenAddress).balanceOf(_buyer, _tokenId); } else if (_condition.tokenType == TokenType.NonFungibleToken) { balance = IERC721(_condition.tokenAddress).balanceOf(_buyer); } else { diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index aa58fb78f..4ec854d60 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -62,6 +62,7 @@ exports.RevertReasons = { GROUP_HAS_NO_CONDITION: "Offer belongs to a group without a condition. Use commitToOffer instead", GROUP_HAS_CONDITION: "Offer belongs to a group with a condition. Use commitToConditionalOffer instead", TOKEN_ID_NOT_IN_CONDITION_RANGE: "Token id not in condition range", + INVALID_TOKEN_ID: "ERC1155 requires non-zero tokenId; ERC721 and ERC20 require zero tokenId.", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 90dd68a34..313fc6186 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -915,7 +915,7 @@ describe("IBosonExchangeHandler", function () { ).to.emit(exchangeHandler, "BuyerCommitted"); }); - it("Offer belongs to a group with condition and is not type EvaluationMethod.SpecificToken", async function () { + it("Offer is part of a group that enforces per-address conditions and utilizes ERC20 tokens", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; @@ -939,6 +939,35 @@ describe("IBosonExchangeHandler", function () { ); }); + it("Offer is part of a group that enforces per-address conditions and utilizes ERC721 tokens", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + condition = mockCondition({ + tokenAddress: foreign721.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.NonFungibleToken, + method: EvaluationMethod.Threshold, + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + await foreign721.connect(buyer).mint("123", 1); + + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); + }); + context("💔 Revert Reasons", async function () { it("The exchanges region of protocol is paused", async function () { // Pause the exchanges region of the protocol @@ -1050,7 +1079,7 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); - it("Offer belongs to a group with condition and is type EvaluationMethod.SpecificToken", async function () { + it("Offer is part of a group that has a per-wallet condition", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; @@ -1081,6 +1110,32 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); + it("Offer is part of a group that has a per-address condition and token is ERC1155", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + condition = mockCondition({ + tokenAddress: foreign1155.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.MultiToken, + method: EvaluationMethod.Threshold, + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + }); + it("buyer does not meet condition for commit", async function () { // Required constructor params for Group groupId = "1"; @@ -1091,7 +1146,7 @@ describe("IBosonExchangeHandler", function () { threshold: "1", maxCommits: "3", tokenType: TokenType.NonFungibleToken, - tokenId: "1", + tokenId: "0", method: EvaluationMethod.Threshold, }); @@ -1186,6 +1241,11 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, ++offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.NO_SUCH_GROUP); }); + + it("Caller sends non-zero tokenId", async function () {}); + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 1, { value: price }) + ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); }); }); @@ -1258,10 +1318,16 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); + + it("Caller sends non-zero tokenId", async function () {}); + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 1, { value: price }) + ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); }); }); context("✋ Threshold ERC1155", async function () { + let tokenId; beforeEach(async function () { // Required constructor params for Group groupId = "1"; @@ -1273,36 +1339,40 @@ describe("IBosonExchangeHandler", function () { threshold: "20", maxCommits: "3", tokenType: TokenType.MultiToken, - tokenId: "1", + tokenId: "0", }); + expect(condition.isValid()).to.be.true; // Create Group group = new Group(groupId, seller.id, offerIds); expect(group.isValid()).is.true; await groupHandler.connect(assistant).createGroup(group, condition); + + // Set random token id + tokenId = 123; }); it("should emit a BuyerCommitted event if user meets condition", async function () { // mint enough tokens for the buyer - await foreign1155.connect(buyer).mint(condition.tokenId, condition.threshold); + await foreign1155.connect(buyer).mint(tokenId, condition.threshold); // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("should allow buyer to commit up to the max times for the group", async function () { // mint enough tokens for the buyer - await foreign1155.connect(buyer).mint(condition.tokenId, condition.threshold); + await foreign1155.connect(buyer).mint(tokenId, condition.threshold); // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1311,26 +1381,33 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); it("buyer has exhausted allowable commits", async function () { // mint enough tokens for the buyer - await foreign1155.connect(buyer).mint(condition.tokenId, condition.threshold); + await foreign1155.connect(buyer).mint(tokenId, condition.threshold); // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { await exchangeHandler .connect(buyer) - .commitToConditionalOffer(buyer.address, offerId, 0, { value: price }); + .commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }); } // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 0, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); + + it("Caller sends zero tokenId", async function () { + tokenId = 0; + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) + ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); + }); }); }); diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index e806888e9..304bcf2b3 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -170,8 +170,8 @@ describe("IBosonGroupHandler", function () { condition = mockCondition({ tokenType: TokenType.MultiToken, tokenAddress: accounts[0].address, - tokenId: "5150", length: "0", + method: EvaluationMethod.Threshold, }); expect(condition.isValid()).to.be.true; @@ -465,6 +465,15 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); + + it("Condition 'Threshold' has non zero tokenId", async function () { + condition.tokenId = "123"; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); }); context("Condition 'SpecificToken' has invalid fields", async function () { diff --git a/test/protocol/OrchestrationHandlerTest.js b/test/protocol/OrchestrationHandlerTest.js index f42978d28..95991671b 100644 --- a/test/protocol/OrchestrationHandlerTest.js +++ b/test/protocol/OrchestrationHandlerTest.js @@ -2199,7 +2199,11 @@ describe("IBosonOrchestrationHandler", function () { seller.id = "2"; // "1" is dispute resolver offerIds = ["1"]; - condition = mockCondition({ tokenAddress: other2.address, tokenType: TokenType.MultiToken, tokenId: "5150" }); + condition = mockCondition({ + tokenAddress: other2.address, + tokenType: TokenType.MultiToken, + method: EvaluationMethod.Threshold, + }); expect(condition.isValid()).to.be.true; group = new Group(nextGroupId, seller.id, offerIds); @@ -2879,7 +2883,7 @@ describe("IBosonOrchestrationHandler", function () { condition = mockCondition({ tokenType: TokenType.MultiToken, tokenAddress: other2.address, - tokenId: "5150", + method: EvaluationMethod.Threshold, maxCommits: "3", }); expect(condition.isValid()).to.be.true; @@ -4250,7 +4254,11 @@ describe("IBosonOrchestrationHandler", function () { // Required constructor params for Group offerIds = ["1"]; - condition = mockCondition({ tokenType: TokenType.MultiToken, tokenAddress: other2.address, tokenId: "5150" }); + condition = mockCondition({ + tokenType: TokenType.MultiToken, + tokenAddress: other2.address, + method: EvaluationMethod.Threshold, + }); expect(condition.isValid()).to.be.true; group = new Group(nextGroupId, seller.id, offerIds); @@ -5174,7 +5182,11 @@ describe("IBosonOrchestrationHandler", function () { // Required constructor params for Group offerIds = ["1"]; - condition = mockCondition({ tokenType: TokenType.MultiToken, tokenAddress: other2.address, tokenId: "5150" }); + condition = mockCondition({ + tokenType: TokenType.MultiToken, + tokenAddress: other2.address, + method: EvaluationMethod.Threshold, + }); expect(condition.isValid()).to.be.true; group = new Group(nextGroupId, seller.id, offerIds); @@ -6706,7 +6718,11 @@ describe("IBosonOrchestrationHandler", function () { offerIds = ["1"]; - condition = mockCondition({ tokenType: TokenType.MultiToken, tokenAddress: other2.address, tokenId: "5150" }); + condition = mockCondition({ + tokenType: TokenType.MultiToken, + tokenAddress: other2.address, + method: EvaluationMethod.Threshold, + }); expect(condition.isValid()).to.be.true; group = new Group(nextGroupId, seller.id, offerIds); From 9de5f992f5433e866797f8177a2fd6d5f2c33bb3 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 1 Jun 2023 15:51:43 -0300 Subject: [PATCH 20/31] Review changes --- contracts/protocol/bases/GroupBase.sol | 6 +----- .../protocol/facets/ExchangeHandlerFacet.sol | 17 ++++++++++------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index b85c387d7..fc04710bd 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -123,11 +123,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { _condition.maxCommits == 0 && _condition.length == 0); } else if (_condition.method == EvaluationMethod.Threshold) { - valid = (_condition.tokenAddress != address(0) && - _condition.maxCommits > 0 && - _condition.threshold > 0 && - _condition.length == 0 && - _condition.tokenId == 0); + valid = (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0); } else { // SpecificToken valid = (_condition.tokenAddress != address(0) && diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 24db5cfcc..577257bb9 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -197,7 +197,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Make sure condition is not SpecificToken as it is not supported for preminted offers require( - condition.method == EvaluationMethod.Threshold && condition.tokenType != TokenType.MultiToken, + condition.method != EvaluationMethod.SpecificToken && condition.tokenType != TokenType.MultiToken, CANNOT_COMMIT ); @@ -966,14 +966,15 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Cache protocol lookups for reference ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); + if (_condition.tokenId > 0) { + require( + _tokenId >= _condition.tokenId && _tokenId < _condition.tokenId + _condition.length, + TOKEN_ID_NOT_IN_CONDITION_RANGE + ); + } + if (_condition.method == EvaluationMethod.SpecificToken) { // If condition has a token id, check that the token id is in range, otherwise accept any token id - if (_condition.tokenId > 0) { - require( - _tokenId >= _condition.tokenId && _tokenId < _condition.tokenId + _condition.length, - TOKEN_ID_NOT_IN_CONDITION_RANGE - ); - } // How many times has this token id been used to commit to offers in the group? uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId]; @@ -1000,6 +1001,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Increment number of commits to the group for this address if they are allowed to commit lookups.conditionalCommitsByAddress[_buyer][_groupId] = ++commitCount; } + } else { + allow = true; } } From 1c820c9c48613904c387e709c53fe8165d839ab4 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Fri, 2 Jun 2023 08:21:22 -0300 Subject: [PATCH 21/31] Refactor ExchangeHandlerTest and GroupHandlerTest to improve code readability and remove redundant test cases. --- test/protocol/ExchangeHandlerTest.js | 103 +++++++++++++++++---------- test/protocol/GroupHandlerTest.js | 18 ----- 2 files changed, 65 insertions(+), 56 deletions(-) diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 313fc6186..fbcee64e0 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -915,57 +915,84 @@ describe("IBosonExchangeHandler", function () { ).to.emit(exchangeHandler, "BuyerCommitted"); }); - it("Offer is part of a group that enforces per-address conditions and utilizes ERC20 tokens", async function () { - // Required constructor params for Group - groupId = "1"; - offerIds = [offerId]; + context("Offer is part a group", async function () { + let groupId; + let offerIds; - // Create Condition - condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); - expect(condition.isValid()).to.be.true; + beforeEach(async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + }); - // Create Group - group = new Group(groupId, seller.id, offerIds); - expect(group.isValid()).is.true; + it("Offer is part of a group that enforces per-address conditions and utilizes ERC20 tokens", async function () { + // Create Condition + condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + expect(condition.isValid()).to.be.true; - await groupHandler.connect(assistant).createGroup(group, condition); + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; - // mint enough tokens for the buyer - await foreign20.connect(buyer).mint(buyer.address, condition.threshold); + await groupHandler.connect(assistant).createGroup(group, condition); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); - }); + // mint enough tokens for the buyer + await foreign20.connect(buyer).mint(buyer.address, condition.threshold); - it("Offer is part of a group that enforces per-address conditions and utilizes ERC721 tokens", async function () { - // Required constructor params for Group - groupId = "1"; - offerIds = [offerId]; + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); + }); - condition = mockCondition({ - tokenAddress: foreign721.address, - threshold: "1", - maxCommits: "3", - tokenType: TokenType.NonFungibleToken, - method: EvaluationMethod.Threshold, + it("Offer is part of a group that enforces per-address conditions and utilizes ERC721 tokens", async function () { + condition = mockCondition({ + tokenAddress: foreign721.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.NonFungibleToken, + method: EvaluationMethod.Threshold, + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + await foreign721.connect(buyer).mint("123", 1); + + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); }); - expect(condition.isValid()).to.be.true; + it("Offer is part of a group that has no condition", async function () { + condition = mockCondition({ + tokenAddress: ethers.constants.AddressZero, + threshold: "0", + maxCommits: "0", + tokenType: TokenType.FungibleToken, + method: EvaluationMethod.None, + }); - // Create Group - group = new Group(groupId, seller.id, offerIds); - expect(group.isValid()).is.true; + expect(condition.isValid()).to.be.true; - await groupHandler.connect(assistant).createGroup(group, condition); + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; - await foreign721.connect(buyer).mint("123", 1); + await groupHandler.connect(assistant).createGroup(group, condition); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await foreign721.connect(buyer).mint("123", 1); + + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); + }); }); context("💔 Revert Reasons", async function () { diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index 304bcf2b3..ae7e14b6a 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -456,24 +456,6 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); - - it("Condition 'Threshold' has non zero length", async function () { - condition.length = "5"; - - // Attempt to create the group, expecting revert - await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( - RevertReasons.INVALID_CONDITION_PARAMETERS - ); - }); - - it("Condition 'Threshold' has non zero tokenId", async function () { - condition.tokenId = "123"; - - // Attempt to create the group, expecting revert - await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( - RevertReasons.INVALID_CONDITION_PARAMETERS - ); - }); }); context("Condition 'SpecificToken' has invalid fields", async function () { From 2008253dc0b0151c3ddbc4e8f90a5009537dd840 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Fri, 2 Jun 2023 14:54:07 -0300 Subject: [PATCH 22/31] Allow for more use cases --- contracts/protocol/bases/GroupBase.sol | 68 +++++++++++++------ .../protocol/facets/ExchangeHandlerFacet.sol | 34 +++++----- test/protocol/ExchangeHandlerTest.js | 7 -- test/protocol/GroupHandlerTest.js | 34 +++++++++- 4 files changed, 99 insertions(+), 44 deletions(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index fc04710bd..1e2b011aa 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -1,6 +1,5 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.18; - import "./../../domain/BosonConstants.sol"; import { IBosonGroupEvents } from "../../interfaces/events/IBosonGroupEvents.sol"; import { ProtocolBase } from "./../bases/ProtocolBase.sol"; @@ -101,12 +100,18 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @notice Validates that condition parameters make sense. * * A invalid condition is one that fits any of the following criteria: - * - EvaluationMethod.None: any fields different from 0 - * - EvaluationMethod.Threshold: token address, maxCommits or threshold is zero or length is not zero + * - EvaluationMethod.None: any field different from zero + * - EvaluationMethod.Threshold: + -Token address, maxCommits, or threshold is zero. + * - TokenType is FungibleToken or NonFungibleToken and length and tokenId are not 0. + * - EvaluationMethod.Threshold: + * - token address, maxCommits or threshold is zero + * - tokenType is FungibleToken or NonFungibleToken and length and tokenId is not zero + * - tokenType is MultiToken and length is zero when tokenId is not zero or range overflow * - EvaluationMethod.SpecificToken: * - tokenType is FungibleToken * - tokenType is NonFungibleToken and threshold is not zero - * - tokenId is not zero and length is zero + * - tokenId is not zero and length is zero or range overflow * - tokenType is MultiToken and threshold is zero * - maxCommits is zero * - token address is zero @@ -115,32 +120,57 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return valid - validity of condition * */ - function validateCondition(Condition memory _condition) internal pure returns (bool valid) { + function validateCondition(Condition memory _condition) internal returns (bool valid) { if (_condition.method == EvaluationMethod.None) { valid = (_condition.tokenAddress == address(0) && _condition.tokenId == 0 && _condition.threshold == 0 && _condition.maxCommits == 0 && _condition.length == 0); - } else if (_condition.method == EvaluationMethod.Threshold) { - valid = (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0); } else { - // SpecificToken - valid = (_condition.tokenAddress != address(0) && - _condition.maxCommits > 0 && - _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken - if (_condition.tokenId != 0) { - // SpecificToken with tokenId should have length - valid = valid && _condition.length > 0; + if (_condition.length == 0) { + return false; + } + + // Create local copy so we can use assembly to check for overflow + uint256 tokenId = _condition.tokenId; + uint256 length = _condition.length; + uint256 sum; + assembly { + // Adding and checking for overflow in Assembly + let tmp := add(tokenId, sub(length, 1)) + if iszero(lt(tmp, tokenId)) { + sum := tmp + } + } + + if (sum == 0) { + return false; + } } - // SpecificToken with NonFungibleToken should not have threshold - if (_condition.tokenType == TokenType.NonFungibleToken) { - valid = valid && _condition.threshold == 0; + if (_condition.method == EvaluationMethod.Threshold) { + valid = (_condition.tokenAddress != address(0) && + _condition.maxCommits > 0 && + _condition.threshold > 0); + + if (_condition.tokenType != TokenType.MultiToken) { + // NonFungibleToken and FungibleToken should not have length and tokenId + valid = valid && _condition.length == 0 && _condition.tokenId == 0; + } } else { - // SpecificToken with MultiToken should have threshold - valid = valid && _condition.threshold > 0; + valid = (_condition.tokenAddress != address(0) && + _condition.maxCommits > 0 && + _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken + + // SpecificToken with NonFungibleToken should not have threshold + if (_condition.tokenType == TokenType.NonFungibleToken) { + valid = valid && _condition.threshold == 0; + } else { + // SpecificToken with MultiToken should have threshold + valid = valid && _condition.threshold > 0; + } } } } diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 577257bb9..93f379fff 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -144,6 +144,14 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { require(condition.method != EvaluationMethod.None, GROUP_HAS_NO_CONDITION); + if (condition.length > 1) { + // If condition has length > 1, check that the token id is in range + require( + _tokenId >= condition.tokenId && _tokenId < condition.tokenId + condition.length, + TOKEN_ID_NOT_IN_CONDITION_RANGE + ); + } + bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId); require(allow, CANNOT_COMMIT); @@ -195,11 +203,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); - // Make sure condition is not SpecificToken as it is not supported for preminted offers - require( - condition.method != EvaluationMethod.SpecificToken && condition.tokenType != TokenType.MultiToken, - CANNOT_COMMIT - ); + // Pre-minted vouchers cannot be used for token-gated offers which have a range condition since the caller (Boson Voucher) cannot specify the token id + if (condition.method != EvaluationMethod.None && condition.tokenType != TokenType.FungibleToken) { + require(condition.length == 1, CANNOT_COMMIT); + } bool allow = authorizeCommit(_buyer, condition, groupId, 0); require(allow, CANNOT_COMMIT); @@ -966,36 +973,29 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Cache protocol lookups for reference ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); - if (_condition.tokenId > 0) { - require( - _tokenId >= _condition.tokenId && _tokenId < _condition.tokenId + _condition.length, - TOKEN_ID_NOT_IN_CONDITION_RANGE - ); - } - if (_condition.method == EvaluationMethod.SpecificToken) { - // If condition has a token id, check that the token id is in range, otherwise accept any token id - // How many times has this token id been used to commit to offers in the group? uint256 commitCount = lookups.conditionalCommitsByTokenId[_tokenId][_groupId]; require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); - allow = holdsSpecificToken(_buyer, _condition, _tokenId); + allow = holdsSpecificToken(_buyer, _condition, _condition.length == 1 ? _condition.tokenId : _tokenId); if (allow) { // Increment number of commits to the group for this token id if they are allowed to commit lookups.conditionalCommitsByTokenId[_tokenId][_groupId] = ++commitCount; } } else if (_condition.method == EvaluationMethod.Threshold) { - require(_condition.tokenType == TokenType.MultiToken ? _tokenId > 0 : _tokenId == 0, INVALID_TOKEN_ID); + if (_condition.tokenType != TokenType.MultiToken) { + require(_tokenId == 0, INVALID_TOKEN_ID); + } // How many times has this address committed to offers in the group? uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId]; require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED); - allow = holdsThreshold(_buyer, _condition, _tokenId); + allow = holdsThreshold(_buyer, _condition, _condition.length == 1 ? _condition.tokenId : _tokenId); if (allow) { // Increment number of commits to the group for this address if they are allowed to commit diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index fbcee64e0..bd101b458 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -1428,13 +1428,6 @@ describe("IBosonExchangeHandler", function () { exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); - - it("Caller sends zero tokenId", async function () { - tokenId = 0; - await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, tokenId, { value: price }) - ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); - }); }); }); diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index ae7e14b6a..c2c0bad6a 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -456,6 +456,28 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); + + it("Condition 'Threshold' with MultiToken has non-zero tokenId and zero length", async function () { + condition.tokenType = TokenType.MultiToken; + condition.tokenId = "1"; + condition.length = "0"; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); + + it("Condition 'Threshold' with MultiToken has tokenId + length - 1 > max uint256", async function () { + condition.tokenType = TokenType.MultiToken; + condition.tokenId = 2; + condition.length = ethers.constants.MaxUint256; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); }); context("Condition 'SpecificToken' has invalid fields", async function () { @@ -477,7 +499,7 @@ describe("IBosonGroupHandler", function () { ); }); - it("Condition 'SpecificToken' has non zero threshold", async function () { + it("Condition 'SpecificToken' has non zero threshold when tokenType is NonFungibleToken", async function () { condition.threshold = "10"; // Attempt to create the group, expecting revert @@ -503,6 +525,16 @@ describe("IBosonGroupHandler", function () { RevertReasons.INVALID_CONDITION_PARAMETERS ); }); + + it("Condition 'SpecificToken' with MultiToken has zero threshold", async function () { + condition.tokenType = TokenType.MultiToken; + condition.threshold = "0"; + + // Attempt to create the group, expecting revert + await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( + RevertReasons.INVALID_CONDITION_PARAMETERS + ); + }); }); }); }); From 14ab5dd896ccaf738f8d3bb3ccd2f238a5dec6c7 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 5 Jun 2023 13:04:08 -0300 Subject: [PATCH 23/31] Fixing missing tests and adding natspec comments --- .../handlers/IBosonExchangeHandler.sol | 2 + .../protocol/facets/ExchangeHandlerFacet.sol | 11 +- test/example/SnapshotGateTest.js | 2 +- test/protocol/ExchangeHandlerTest.js | 114 +++++++++++++++--- 4 files changed, 109 insertions(+), 20 deletions(-) diff --git a/contracts/interfaces/handlers/IBosonExchangeHandler.sol b/contracts/interfaces/handlers/IBosonExchangeHandler.sol index 73c9fb795..2d027aad5 100644 --- a/contracts/interfaces/handlers/IBosonExchangeHandler.sol +++ b/contracts/interfaces/handlers/IBosonExchangeHandler.sol @@ -66,6 +66,7 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) * - Received ERC20 token amount differs from the expected value * - Seller has less funds available than sellerDeposit + * - Condition has a range and the token id is not within the range * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) * @param _offerId - the id of the offer to commit to @@ -88,6 +89,7 @@ interface IBosonExchangeHandler is IBosonExchangeEvents, IBosonFundsLibEvents, I * - Offer is not yet available for commits * - Buyer account is inactive * - Buyer is token-gated (conditional commit requirements not met or already used) + * - Buyer is token-gated and condition has a range. * - Seller has less funds available than sellerDeposit and price * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 93f379fff..bb00a52d4 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -1,6 +1,7 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.18; +import "hardhat/console.sol"; import { IBosonExchangeHandler } from "../../interfaces/handlers/IBosonExchangeHandler.sol"; import { IBosonAccountHandler } from "../../interfaces/handlers/IBosonAccountHandler.sol"; import { IBosonVoucher } from "../../interfaces/clients/IBosonVoucher.sol"; @@ -118,6 +119,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * - Calling transferFrom on token fails for some reason (e.g. protocol is not approved to transfer) * - Received ERC20 token amount differs from the expected value * - Seller has less funds available than sellerDeposit + * - Condition has a range and the token id is not within the range * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) * @param _offerId - the id of the offer to commit to @@ -175,7 +177,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { * - Offer is not yet available for commits * - Buyer account is inactive * - Buyer is token-gated (conditional commit requirements not met or already used) - * - Buyer is token-gated and evaluation method is SpecificToken + * - Buyer is token-gated and condition has a range. * - Seller has less funds available than sellerDeposit and price * * @param _buyer - the buyer's address (caller can commit on behalf of a buyer) @@ -203,8 +205,11 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); - // Pre-minted vouchers cannot be used for token-gated offers which have a range condition since the caller (Boson Voucher) cannot specify the token id - if (condition.method != EvaluationMethod.None && condition.tokenType != TokenType.FungibleToken) { + // If is a per-token condition or a per-address condition gated with a 1155 token, make sure the condition is not a range since caller (Boson Voucher) cannot specify the token id + if ( + condition.method == EvaluationMethod.SpecificToken || + (condition.method == EvaluationMethod.Threshold && condition.tokenType == TokenType.MultiToken) + ) { require(condition.length == 1, CANNOT_COMMIT); } diff --git a/test/example/SnapshotGateTest.js b/test/example/SnapshotGateTest.js index b143e8baa..df9adf841 100644 --- a/test/example/SnapshotGateTest.js +++ b/test/example/SnapshotGateTest.js @@ -334,7 +334,7 @@ describe("SnapshotGate", function () { tokenType: TokenType.NonFungibleToken, tokenId: tokenId, method: EvaluationMethod.SpecificToken, - length: "1", + length: "3", }); expect(condition.isValid()).to.be.true; diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index bd101b458..6edea6b61 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -970,6 +970,33 @@ describe("IBosonExchangeHandler", function () { ); }); + it("Offer is part of a group that enfoces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () { + condition = mockCondition({ + tokenAddress: foreign1155.address, + threshold: "2", + maxCommits: "3", + tokenType: TokenType.MultiToken, + method: EvaluationMethod.Threshold, + length: "1", + tokenId: "123", + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + await foreign1155.connect(buyer).mint(condition.tokenId, condition.threshold); + + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); + }); + it("Offer is part of a group that has no condition", async function () { condition = mockCondition({ tokenAddress: ethers.constants.AddressZero, @@ -993,6 +1020,36 @@ describe("IBosonExchangeHandler", function () { "BuyerCommitted" ); }); + + it("Offer is part of a group that has a per-wallet ERC721 condition", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + condition = mockCondition({ + tokenAddress: foreign721.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.NonFungibleToken, + method: EvaluationMethod.Threshold, + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + // mint enough tokens for the buyer + await foreign721.connect(buyer).mint(condition.tokenId, 1); + + await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId)).to.emit( + exchangeHandler, + "BuyerCommitted" + ); + }); }); context("💔 Revert Reasons", async function () { @@ -1106,19 +1163,44 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); - it("Offer is part of a group that has a per-wallet condition", async function () { + it("Offer is part of a group that has a per-address condition and token is ERC1155", async function () { + // Required constructor params for Group + groupId = "1"; + offerIds = [offerId]; + + condition = mockCondition({ + tokenAddress: foreign1155.address, + threshold: "1", + maxCommits: "3", + tokenType: TokenType.MultiToken, + method: EvaluationMethod.Threshold, + }); + + expect(condition.isValid()).to.be.true; + + // Create Group + group = new Group(groupId, seller.id, offerIds); + expect(group.isValid()).is.true; + + await groupHandler.connect(assistant).createGroup(group, condition); + + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + }); + + it("buyer does not meet condition for commit", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; condition = mockCondition({ tokenAddress: foreign721.address, - threshold: "0", + threshold: "1", maxCommits: "3", tokenType: TokenType.NonFungibleToken, - tokenId: "1", - method: EvaluationMethod.SpecificToken, - length: "10", + tokenId: "0", + method: EvaluationMethod.Threshold, }); expect(condition.isValid()).to.be.true; @@ -1129,25 +1211,24 @@ describe("IBosonExchangeHandler", function () { await groupHandler.connect(assistant).createGroup(group, condition); - // mint enough tokens for the buyer - await foreign721.connect(buyer).mint(condition.tokenId, 1); - await expect( bosonVoucher.connect(assistant).transferFrom(assistant.address, buyer.address, tokenId) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); - it("Offer is part of a group that has a per-address condition and token is ERC1155", async function () { + it("Offer is part of a group that has a per-wallet ERC1155 condition with length > 1", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; condition = mockCondition({ tokenAddress: foreign1155.address, - threshold: "1", + threshold: "2", maxCommits: "3", - tokenType: TokenType.MultiToken, - method: EvaluationMethod.Threshold, + tokenType: TokenType.MultiToken, // ERC1155 + tokenId: "1", + method: EvaluationMethod.Threshold, // per-wallet + length: "2", }); expect(condition.isValid()).to.be.true; @@ -1163,18 +1244,19 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); - it("buyer does not meet condition for commit", async function () { + it("Offer is part a group with a per-token condition with length > 1", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; condition = mockCondition({ tokenAddress: foreign721.address, - threshold: "1", + threshold: "0", maxCommits: "3", - tokenType: TokenType.NonFungibleToken, + tokenType: TokenType.NonFungibleToken, // ERC721 tokenId: "0", - method: EvaluationMethod.Threshold, + method: EvaluationMethod.SpecificToken, // per-wallet + length: "0", }); expect(condition.isValid()).to.be.true; From 374545ece360c81beb94ff7d000b711412790ac7 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 5 Jun 2023 15:25:14 -0300 Subject: [PATCH 24/31] Added Contracts CI badge to README.md file. --- README.md | 1 + contracts/protocol/bases/GroupBase.sol | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 203e2bf07..a7f9c1c9c 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@

Boson Protocol V2

[![Coverage Status](https://coveralls.io/repos/github/bosonprotocol/boson-protocol-contracts/badge.svg)](https://coveralls.io/github/bosonprotocol/boson-protocol-contracts) +[![Contracts CI](https://github.com/bosonprotocol/boson-protocol-contracts/actions/workflows/ci.yaml/badge.svg)](https://github.com/bosonprotocol/boson-protocol-contracts/actions/workflows/ci.yaml) ### Intro | [Audits](docs/audits.md) | [Setup](docs/setup.md) | [Tasks](docs/tasks.md) | [Architecture](docs/architecture.md) | [Domain Model](docs/domain.md) | [State Machines](docs/state-machines.md) | [Sequences](docs/sequences.md) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 1e2b011aa..251f61657 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -120,7 +120,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return valid - validity of condition * */ - function validateCondition(Condition memory _condition) internal returns (bool valid) { + function validateCondition(Condition memory _condition) internal pure returns (bool valid) { if (_condition.method == EvaluationMethod.None) { valid = (_condition.tokenAddress == address(0) && _condition.tokenId == 0 && From 2ee5ef41291a3afe00fea2a767c052bb8ccaca2d Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 5 Jun 2023 15:37:57 -0300 Subject: [PATCH 25/31] Remove log --- contracts/protocol/facets/ExchangeHandlerFacet.sol | 1 - 1 file changed, 1 deletion(-) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index bb00a52d4..684d03e9c 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: GPL-3.0-or-later pragma solidity 0.8.18; -import "hardhat/console.sol"; import { IBosonExchangeHandler } from "../../interfaces/handlers/IBosonExchangeHandler.sol"; import { IBosonAccountHandler } from "../../interfaces/handlers/IBosonAccountHandler.sol"; import { IBosonVoucher } from "../../interfaces/clients/IBosonVoucher.sol"; From 0e0f4a53311e4f28425e7bbac5a75c6d4bb825a8 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Mon, 5 Jun 2023 16:30:34 -0300 Subject: [PATCH 26/31] Add token ID validation for threshold evaluation method in ExchangeHandlerFacet.sol and update ExchangeHandlerTest.js to test for invalid token ID. --- contracts/protocol/facets/ExchangeHandlerFacet.sol | 8 ++++---- test/protocol/ExchangeHandlerTest.js | 9 +++++---- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 684d03e9c..540a08bb3 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -153,6 +153,10 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { ); } + if (condition.method == EvaluationMethod.Threshold && condition.tokenType != TokenType.MultiToken) { + require(_tokenId == 0, INVALID_TOKEN_ID); + } + bool allow = authorizeCommit(_buyer, condition, groupId, _tokenId); require(allow, CANNOT_COMMIT); @@ -990,10 +994,6 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { lookups.conditionalCommitsByTokenId[_tokenId][_groupId] = ++commitCount; } } else if (_condition.method == EvaluationMethod.Threshold) { - if (_condition.tokenType != TokenType.MultiToken) { - require(_tokenId == 0, INVALID_TOKEN_ID); - } - // How many times has this address committed to offers in the group? uint256 commitCount = lookups.conditionalCommitsByAddress[_buyer][_groupId]; diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 6edea6b61..fbc373cde 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -1428,10 +1428,11 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); - it("Caller sends non-zero tokenId", async function () {}); - await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 1, { value: price }) - ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); + it("Caller sends non-zero tokenId", async function () { + await expect( + exchangeHandler.connect(buyer).commitToConditionalOffer(buyer.address, offerId, 1, { value: price }) + ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); + }); }); }); From 16a05b560c242bb78cd291528e5b301c485d94f4 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 6 Jun 2023 11:02:06 -0300 Subject: [PATCH 27/31] Apply review requests --- contracts/domain/BosonConstants.sol | 2 +- contracts/protocol/bases/GroupBase.sol | 41 +++++++------------ .../protocol/facets/ExchangeHandlerFacet.sol | 15 ++++--- scripts/config/revert-reasons.js | 2 +- test/example/SnapshotGateTest.js | 2 +- test/protocol/ExchangeHandlerTest.js | 29 +++++-------- test/protocol/GroupHandlerTest.js | 2 +- 7 files changed, 40 insertions(+), 53 deletions(-) diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 26c8f44f2..0240146d4 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -105,7 +105,7 @@ string constant GROUP_HAS_CONDITION = "Offer belongs to a group with a condition string constant MAX_COMMITS_ADDRESS_REACHED = "Max commits per address reached"; string constant MAX_COMMITS_TOKEN_REACHED = "Max commits per token id reached"; string constant TOKEN_ID_NOT_IN_CONDITION_RANGE = "Token id not in condition range"; -string constant INVALID_TOKEN_ID = "ERC1155 requires non-zero tokenId; ERC721 and ERC20 require zero tokenId."; +string constant INVALID_TOKEN_ID = "ERC721 and ERC20 require zero tokenId"; // Revert Reasons: Exchange related string constant NO_SUCH_EXCHANGE = "No such exchange"; diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 251f61657..725057816 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -1,5 +1,6 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.18; + import "./../../domain/BosonConstants.sol"; import { IBosonGroupEvents } from "../../interfaces/events/IBosonGroupEvents.sol"; import { ProtocolBase } from "./../bases/ProtocolBase.sol"; @@ -120,7 +121,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return valid - validity of condition * */ - function validateCondition(Condition memory _condition) internal pure returns (bool valid) { + function validateCondition(Condition memory _condition) internal pure returns (bool) { + bool valid = true; if (_condition.method == EvaluationMethod.None) { valid = (_condition.tokenAddress == address(0) && _condition.tokenId == 0 && @@ -129,40 +131,25 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { _condition.length == 0); } else { if (_condition.tokenId != 0) { - if (_condition.length == 0) { - return false; - } - - // Create local copy so we can use assembly to check for overflow - uint256 tokenId = _condition.tokenId; - uint256 length = _condition.length; - uint256 sum; - assembly { - // Adding and checking for overflow in Assembly - let tmp := add(tokenId, sub(length, 1)) - if iszero(lt(tmp, tokenId)) { - sum := tmp - } - } - - if (sum == 0) { - return false; - } + valid = _condition.length != 0; + valid = valid && type(uint256).max - _condition.length >= _condition.tokenId; } if (_condition.method == EvaluationMethod.Threshold) { - valid = (_condition.tokenAddress != address(0) && - _condition.maxCommits > 0 && - _condition.threshold > 0); + valid = + valid && + (_condition.tokenAddress != address(0) && _condition.maxCommits > 0 && _condition.threshold > 0); if (_condition.tokenType != TokenType.MultiToken) { // NonFungibleToken and FungibleToken should not have length and tokenId valid = valid && _condition.length == 0 && _condition.tokenId == 0; } } else { - valid = (_condition.tokenAddress != address(0) && - _condition.maxCommits > 0 && - _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken + valid = + valid && + (_condition.tokenAddress != address(0) && + _condition.maxCommits > 0 && + _condition.tokenType != TokenType.FungibleToken); // FungibleToken not allowed for SpecificToken // SpecificToken with NonFungibleToken should not have threshold if (_condition.tokenType == TokenType.NonFungibleToken) { @@ -173,6 +160,8 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { } } } + + return valid; } /** diff --git a/contracts/protocol/facets/ExchangeHandlerFacet.sol b/contracts/protocol/facets/ExchangeHandlerFacet.sol index 540a08bb3..3d9ea8fa1 100644 --- a/contracts/protocol/facets/ExchangeHandlerFacet.sol +++ b/contracts/protocol/facets/ExchangeHandlerFacet.sol @@ -145,8 +145,8 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { require(condition.method != EvaluationMethod.None, GROUP_HAS_NO_CONDITION); - if (condition.length > 1) { - // If condition has length > 1, check that the token id is in range + if (condition.length >= 1) { + // If condition has length >= 1, check that the token id is in range require( _tokenId >= condition.tokenId && _tokenId < condition.tokenId + condition.length, TOKEN_ID_NOT_IN_CONDITION_RANGE @@ -208,15 +208,20 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { // Get the condition Condition storage condition = fetchCondition(groupId); + uint256 tokenId = 0; + // If is a per-token condition or a per-address condition gated with a 1155 token, make sure the condition is not a range since caller (Boson Voucher) cannot specify the token id if ( condition.method == EvaluationMethod.SpecificToken || (condition.method == EvaluationMethod.Threshold && condition.tokenType == TokenType.MultiToken) ) { require(condition.length == 1, CANNOT_COMMIT); + + // Uses token id from the condition + tokenId = condition.tokenId; } - bool allow = authorizeCommit(_buyer, condition, groupId, 0); + bool allow = authorizeCommit(_buyer, condition, groupId, tokenId); require(allow, CANNOT_COMMIT); } @@ -987,7 +992,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { require(commitCount < _condition.maxCommits, MAX_COMMITS_TOKEN_REACHED); - allow = holdsSpecificToken(_buyer, _condition, _condition.length == 1 ? _condition.tokenId : _tokenId); + allow = holdsSpecificToken(_buyer, _condition, _tokenId); if (allow) { // Increment number of commits to the group for this token id if they are allowed to commit @@ -999,7 +1004,7 @@ contract ExchangeHandlerFacet is IBosonExchangeHandler, BuyerBase, DisputeBase { require(commitCount < _condition.maxCommits, MAX_COMMITS_ADDRESS_REACHED); - allow = holdsThreshold(_buyer, _condition, _condition.length == 1 ? _condition.tokenId : _tokenId); + allow = holdsThreshold(_buyer, _condition, _tokenId); if (allow) { // Increment number of commits to the group for this address if they are allowed to commit diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 4ec854d60..f3e610cea 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -62,7 +62,7 @@ exports.RevertReasons = { GROUP_HAS_NO_CONDITION: "Offer belongs to a group without a condition. Use commitToOffer instead", GROUP_HAS_CONDITION: "Offer belongs to a group with a condition. Use commitToConditionalOffer instead", TOKEN_ID_NOT_IN_CONDITION_RANGE: "Token id not in condition range", - INVALID_TOKEN_ID: "ERC1155 requires non-zero tokenId; ERC721 and ERC20 require zero tokenId.", + INVALID_TOKEN_ID: "ERC721 and ERC20 require zero tokenId", // Account-related MUST_BE_ACTIVE: "Account must be active", diff --git a/test/example/SnapshotGateTest.js b/test/example/SnapshotGateTest.js index df9adf841..472eb9027 100644 --- a/test/example/SnapshotGateTest.js +++ b/test/example/SnapshotGateTest.js @@ -896,7 +896,7 @@ describe("SnapshotGate", function () { // Check that holder cannot commit directly to the offer on the protocol itself await expect( exchangeHandler.connect(holder).commitToConditionalOffer(holder.address, offerId, entry.tokenId) - ).to.revertedWith("Caller cannot commit"); + ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); }); }); diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index fbc373cde..8320afb27 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -915,7 +915,7 @@ describe("IBosonExchangeHandler", function () { ).to.emit(exchangeHandler, "BuyerCommitted"); }); - context("Offer is part a group", async function () { + context("Offer is part of a group", async function () { let groupId; let offerIds; @@ -970,7 +970,7 @@ describe("IBosonExchangeHandler", function () { ); }); - it("Offer is part of a group that enfoces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () { + it("Offer is part of a group that enforces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () { condition = mockCondition({ tokenAddress: foreign1155.address, threshold: "2", @@ -1021,17 +1021,18 @@ describe("IBosonExchangeHandler", function () { ); }); - it("Offer is part of a group that has a per-wallet ERC721 condition", async function () { + it("Offer is part of a group that enforces per-wallet conditions and range length == 1", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; condition = mockCondition({ tokenAddress: foreign721.address, - threshold: "1", + threshold: "0", maxCommits: "3", tokenType: TokenType.NonFungibleToken, - method: EvaluationMethod.Threshold, + method: EvaluationMethod.SpecificToken, + length: "1", }); expect(condition.isValid()).to.be.true; @@ -1163,7 +1164,7 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); - it("Offer is part of a group that has a per-address condition and token is ERC1155", async function () { + it("Offer is part of a group that has a per-address condition and token is ERC1155 but accets any token", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; @@ -1244,7 +1245,7 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); - it("Offer is part a group with a per-token condition with length > 1", async function () { + it("Offer is part of a group with a per-token condition with length > 1", async function () { // Required constructor params for Group groupId = "1"; offerIds = [offerId]; @@ -1255,7 +1256,7 @@ describe("IBosonExchangeHandler", function () { maxCommits: "3", tokenType: TokenType.NonFungibleToken, // ERC721 tokenId: "0", - method: EvaluationMethod.SpecificToken, // per-wallet + method: EvaluationMethod.SpecificToken, // per-token length: "0", }); @@ -1565,11 +1566,7 @@ describe("IBosonExchangeHandler", function () { condition.tokenId = "0"; condition.length = "0"; - // remove offer from old group - await groupHandler.connect(assistant).removeOffersFromGroup(groupId, offerIds); - - // create new group with new condition - await groupHandler.connect(assistant).createGroup(group, condition); + await groupHandler.connect(assistant).setGroupCondition(group.id, condition); // mint any token for buyer tokenId = "123"; @@ -1676,11 +1673,7 @@ describe("IBosonExchangeHandler", function () { condition.tokenId = "0"; condition.length = "0"; - // remove offer from old group - await groupHandler.connect(assistant).removeOffersFromGroup(groupId, offerIds); - - // create new group with new condition - await groupHandler.connect(assistant).createGroup(group, condition); + await groupHandler.connect(assistant).setGroupCondition(group.id, condition); // mint any token for buyer tokenId = "123"; diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index c2c0bad6a..664b39573 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -470,7 +470,7 @@ describe("IBosonGroupHandler", function () { it("Condition 'Threshold' with MultiToken has tokenId + length - 1 > max uint256", async function () { condition.tokenType = TokenType.MultiToken; - condition.tokenId = 2; + condition.tokenId = 1; condition.length = ethers.constants.MaxUint256; // Attempt to create the group, expecting revert From f286097f89c22c534ccd53822a421ac219cbc480 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Tue, 6 Jun 2023 18:03:38 -0300 Subject: [PATCH 28/31] Turn arg to calldata --- contracts/protocol/bases/GroupBase.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index 725057816..d8e27c72b 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -121,7 +121,7 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * @return valid - validity of condition * */ - function validateCondition(Condition memory _condition) internal pure returns (bool) { + function validateCondition(Condition calldata _condition) internal pure returns (bool) { bool valid = true; if (_condition.method == EvaluationMethod.None) { valid = (_condition.tokenAddress == address(0) && From 12595e317ed181a09b94d2fa065fb0cff748b1b2 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 6 Jul 2023 12:35:14 -0300 Subject: [PATCH 29/31] Tidy --- scripts/domain/Condition.js | 2 +- test/domain/ConditionTest.js | 2 +- test/domain/ReceiptTest.js | 610 +++++++++++++++++++++- test/protocol/ExchangeHandlerTest.js | 196 ++++--- test/protocol/OrchestrationHandlerTest.js | 1 - test/util/upgrade.js | 2 - 6 files changed, 744 insertions(+), 69 deletions(-) diff --git a/scripts/domain/Condition.js b/scripts/domain/Condition.js index 9a1513bb0..d8fbbb08c 100644 --- a/scripts/domain/Condition.js +++ b/scripts/domain/Condition.js @@ -162,7 +162,7 @@ class Condition { * @returns {boolean} */ isValid() { - this.methodIsValid() && + this.methodIsValid() && this.tokenTypeIsValid() && this.tokenAddressIsValid() && this.tokenIdIsValid() && diff --git a/test/domain/ConditionTest.js b/test/domain/ConditionTest.js index e28493831..b5388bc27 100644 --- a/test/domain/ConditionTest.js +++ b/test/domain/ConditionTest.js @@ -15,7 +15,7 @@ describe("Condition", function () { beforeEach(async function () { // Get a list of accounts - accounts = await ethers.getSigners(); + accounts = await getSigners(); tokenAddress = accounts[1].address; // Required constructor params diff --git a/test/domain/ReceiptTest.js b/test/domain/ReceiptTest.js index 9e5b2b870..1e78e421f 100644 --- a/test/domain/ReceiptTest.js +++ b/test/domain/ReceiptTest.js @@ -1 +1,609 @@ -const hre = require("hardhat"); const { getSigners, ZeroAddress } = hre.ethers; const { expect } = require("chai"); const Receipt = require("../../scripts/domain/Receipt.js"); const { mockReceipt, mockOffer, mockTwinReceipt, mockCondition } = require("../util/mock"); const DisputeState = require("../../scripts/domain/DisputeState"); /** * Test the Receipt domain entity */ describe("Receipt", function () { // Suite-wide scope let receipt, object, promoted, clone, dehydrated, rehydrated, key, value, struct, accounts; beforeEach(async function () { // Get a list of accounts accounts = await getSigners(); }); context("📋 Constructor", async function () { it("Should allow creation of valid, fully populated Receipt instance", async function () { receipt = await mockReceipt(); expect(receipt.isValid()).is.true; }); }); context("📋 Field validations", async function () { beforeEach(async function () { // Create a valid receipt, then set fields in tests directly receipt = await mockReceipt(); expect(receipt.exchangeIdIsValid()).is.true; expect(receipt.offerIdIsValid()).is.true; expect(receipt.buyerIdIsValid()).is.true; expect(receipt.sellerIdIsValid()).is.true; expect(receipt.priceIsValid()).is.true; expect(receipt.sellerDepositIsValid()).is.true; expect(receipt.buyerCancelPenaltyIsValid()).is.true; expect(receipt.offerFeesIsValid()).is.true; expect(receipt.agentIdIsValid()).is.true; expect(receipt.exchangeTokenIsValid()).is.true; expect(receipt.finalizedDateIsValid()).is.true; expect(receipt.conditionIsValid()).is.true; expect(receipt.committedDateIsValid()).is.true; expect(receipt.redeemedDateIsValid()).is.true; expect(receipt.voucherExpiredIsValid()).is.true; expect(receipt.disputeResolverIdIsValid()).is.true; expect(receipt.disputedDateIsValid()).is.true; expect(receipt.escalatedDateIsValid()).is.true; expect(receipt.disputeStateIsValid()).is.true; expect(receipt.twinReceiptsIsValid()).is.true; expect(receipt.isValid()).is.true; }); it("Always present, exchangeId must be the string representation of a non-zero BigNumber", async function () { // Invalid field value receipt.exchangeId = "zedzdeadbaby"; expect(receipt.exchangeIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.exchangeId = new Date(); expect(receipt.exchangeIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.exchangeId = "0"; expect(receipt.exchangeIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.exchangeId = "126"; expect(receipt.exchangeIdIsValid()).is.true; expect(receipt.isValid()).is.true; }); it("Always present, offerId must be the string representation of a non-zero BigNumber", async function () { // Invalid field value receipt.offerId = "zedzdeadbaby"; expect(receipt.offerIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.offerId = "0"; expect(receipt.offerIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.offerId = new Date(); expect(receipt.offerIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.offerId = "126"; expect(receipt.offerIdIsValid()).is.true; expect(receipt.isValid()).is.true; }); it("Always present, buyerId must be the string representation of a non-zero BigNumber", async function () { // Invalid field value receipt.buyerId = "zedzdeadbaby"; expect(receipt.buyerIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.buyerId = "0"; expect(receipt.buyerIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.buyerId = "126"; expect(receipt.buyerIdIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.buyerId = new Date(); expect(receipt.buyerIdIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, price must be the string representation of BigNumber", async function () { // Invalid field value receipt.price = "zedzdeadbaby"; expect(receipt.priceIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.price = "0"; expect(receipt.priceIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.price = "126"; expect(receipt.priceIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.price = new Date(); expect(receipt.priceIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, sellerDeposit must be the string representation of BigNumber", async function () { // Invalid field value receipt.sellerDeposit = "zedzdeadbaby"; expect(receipt.sellerDepositIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.sellerDeposit = "0"; expect(receipt.sellerDepositIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.sellerDeposit = "126"; expect(receipt.sellerDepositIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.sellerDeposit = new Date(); expect(receipt.sellerDepositIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, buyerCancelPenalty must be the string representation of BigNumber", async function () { // Invalid field value receipt.buyerCancelPenalty = "zedzdeadbaby"; expect(receipt.buyerCancelPenaltyIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.buyerCancelPenalty = "0"; expect(receipt.buyerCancelPenaltyIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.buyerCancelPenalty = "126"; expect(receipt.buyerCancelPenaltyIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.buyerCancelPenalty = new Date(); expect(receipt.buyerCancelPenaltyIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, offerFees must be a valid OfferFees instance", async function () { // Invalid field value receipt.offerFees = 12; expect(receipt.offerFeesIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.offerFees = "zedzdeadbaby"; expect(receipt.offerFeesIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.offerFees = true; expect(receipt.offerFeesIsValid()).is.false; expect(receipt.isValid()).is.false; const mo = await mockOffer(); // Valid field value receipt.offerFees = mo.offerFees; expect(receipt.offerFeesIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.offerFees = new Date(); expect(receipt.offerFeesIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("If present, agentId must be the string representation of BigNumber", async function () { // Invalid field value receipt.agentId = "zedzdeadbaby"; expect(receipt.agentIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.agentId = "0"; expect(receipt.priceIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.agentId = "126"; expect(receipt.agentIdIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.agentId = new Date(); expect(receipt.agentIdIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, exchangeToken must be a string representation of an EIP-55 compliant address", async function () { // Invalid field value receipt.exchangeToken = "0xASFADF"; expect(receipt.exchangeTokenIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.exchangeToken = "zedzdeadbaby"; expect(receipt.exchangeTokenIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.exchangeToken = accounts[0].address; expect(receipt.exchangeTokenIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.exchangeToken = "0xec2fd5bd6fc7b576dae82c0b9640969d8de501a2"; expect(receipt.exchangeTokenIsValid()).is.true; expect(receipt.isValid()).is.true; }); it("If present, finalizedDate must be the string representation of BigNumber", async function () { // Invalid field value receipt.finalizedDate = "zedzdeadbaby"; expect(receipt.finalizedDateIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.finalizedDate = "0"; expect(receipt.finalizedDateIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.finalizedDate = "126"; expect(receipt.finalizedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.finalizedDate = new Date(); expect(receipt.finalizedDateIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("If present, condition must be a valid Condition instance", async function () { // Invalid field value receipt.condition = "zedzdeadbaby"; expect(receipt.conditionIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.condition = "126"; expect(receipt.conditionIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.condition = mockCondition(); expect(receipt.conditionIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.condition = new Date(); expect(receipt.conditionIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, committedDate must be the string representation of BigNumber", async function () { // Invalid field value receipt.committedDate = "zedzdeadbaby"; expect(receipt.committedDateIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.committedDate = "126"; expect(receipt.committedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.committedDate = new Date(); expect(receipt.committedDateIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("If present, redeemedDate must be the string representation of BigNumber", async function () { // Invalid field value receipt.redeemedDate = "zedzdeadbaby"; expect(receipt.redeemedDateIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.redeemedDate = "0"; expect(receipt.redeemedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.redeemedDate = "126"; expect(receipt.redeemedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.redeemedDate = new Date(); expect(receipt.redeemedDateIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("Always present, voucherExpired must be a boolean", async function () { // Invalid field value receipt.voucherExpired = 12; expect(receipt.voucherExpiredIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.voucherExpired = "zedzdeadbaby"; expect(receipt.voucherExpiredIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.voucherExpired = false; expect(receipt.voucherExpiredIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.voucherExpired = true; expect(receipt.voucherExpiredIsValid()).is.true; expect(receipt.isValid()).is.true; }); it("If present, disputeResolverId must be the string representation of BigNumber", async function () { // Invalid field value receipt.disputeResolverId = "zedzdeadbaby"; expect(receipt.disputeResolverIdIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.disputeResolverId = "0"; expect(receipt.disputeResolverIdIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.disputeResolverId = "126"; expect(receipt.disputeResolverIdIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.disputeResolverId = new Date(); expect(receipt.disputeResolverIdIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("If present, disputedDate must be the string representation of BigNumber", async function () { // Invalid field value receipt.disputedDate = "zedzdeadbaby"; expect(receipt.disputedDateIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.disputedDate = "0"; expect(receipt.disputedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.disputedDate = "126"; expect(receipt.disputedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.disputedDate = new Date(); expect(receipt.disputedDateIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("If present, escalatedDate must be the string representation of BigNumber", async function () { // Invalid field value receipt.escalatedDate = "zedzdeadbaby"; expect(receipt.escalatedDateIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.escalatedDate = "0"; expect(receipt.escalatedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Valid field value receipt.escalatedDate = "126"; expect(receipt.escalatedDateIsValid()).is.true; expect(receipt.isValid()).is.true; // Invalid field value receipt.escalatedDate = new Date(); expect(receipt.escalatedDateIsValid()).is.false; expect(receipt.isValid()).is.false; }); it("If present, disputeState must be the string representation of a BigNumber", async function () { // Invalid field value receipt.disputeState = "zedzdeadbaby"; expect(receipt.disputeStateIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.disputeState = "0"; expect(receipt.disputeStateIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.disputeState = "126"; expect(receipt.disputeStateIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.disputeState = new Date(); expect(receipt.disputeStateIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.disputeState = DisputeState.Resolving; expect(receipt.disputeStateIsValid()).is.true; expect(receipt.isValid()).is.true; }); it("twinReceipt must be a valid TwinReceipt instance", async function () { // Invalid field value receipt.twinReceipts = 12; expect(receipt.twinReceiptsIsValid()).is.false; expect(receipt.isValid()).is.false; // Invalid field value receipt.twinReceipts = "zedzdeadbaby"; expect(receipt.twinReceiptsIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.twinReceipts = true; expect(receipt.twinReceiptsIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.twinReceipts = new Date(); expect(receipt.twinReceiptsIsValid()).is.false; expect(receipt.isValid()).is.false; // Valid field value receipt.twinReceipts = [mockTwinReceipt(ZeroAddress)]; expect(receipt.twinReceiptsIsValid()).is.true; expect(receipt.isValid()).is.true; }); context("📋 Utility functions", async function () { beforeEach(async function () { // Create a valid receipt then set fields in tests directly receipt = object = await mockReceipt(); expect(receipt.isValid()).is.true; const { exchangeId, offerId, buyerId, sellerId, price, sellerDeposit, buyerCancelPenalty, offerFees, agentId, exchangeToken, finalizedDate, condition, committedDate, redeemedDate, voucherExpired, disputeResolverId, disputedDate, escalatedDate, disputeState, twinReceipts, } = receipt; // Struct representation struct = [ exchangeId, offerId, buyerId, sellerId, price, sellerDeposit, buyerCancelPenalty, offerFees.toStruct(), agentId, exchangeToken, finalizedDate, condition.toStruct(), committedDate, redeemedDate, voucherExpired, disputeResolverId, disputedDate, escalatedDate, disputeState, twinReceipts.map((twinReceipt) => twinReceipt.toStruct()), ]; }); context("👉 Static", async function () { it("Receipt.fromObject() should return a Receipt instance with the same values as the given plain object", async function () { // Promote to instance promoted = Receipt.fromObject(object); // Is a Receipt instance expect(promoted instanceof Receipt).is.true; // Key values all match for ([key, value] of Object.entries(receipt)) { expect(JSON.stringify(promoted[key]) === JSON.stringify(value)).is.true; } }); it("Receipt.fromStruct() should return a Receipt instance from a struct representation", async function () { // Get an instance from the struct receipt = Receipt.fromStruct(struct); // Ensure it is valid expect(receipt.isValid()).to.be.true; }); }); context("👉 Instance", async function () { it("instance.toString() should return a JSON string representation of the Receipt instance", async function () { dehydrated = receipt.toString(); rehydrated = JSON.parse(dehydrated); for ([key, value] of Object.entries(receipt)) { expect(JSON.stringify(rehydrated[key]) === JSON.stringify(value)).is.true; } }); it("instance.clone() should return another Receipt instance with the same property values", async function () { // Get plain object clone = receipt.clone(); // Is an Receipt instance expect(clone instanceof Receipt).is.true; // Key values all match for ([key, value] of Object.entries(receipt)) { expect(JSON.stringify(clone[key]) === JSON.stringify(value)).is.true; } }); it("instance.toObject() should return a plain object representation of the Receipt instance", async function () { // Get plain object object = receipt.toObject(); // Not an Receipt instance expect(object instanceof Receipt).is.false; // Key values all match for ([key, value] of Object.entries(receipt)) { expect(JSON.stringify(object[key]) === JSON.stringify(value)).is.true; } }); it("instance.toStruct() should return a struct representation of the Receipt instance", async function () { // Get struct from receipt struct = receipt.toStruct(); // Marshal back to a receipt instance receipt = Receipt.fromStruct(struct); // Ensure it marshals back to a valid receipt expect(receipt.isValid()).to.be.true; }); }); }); }); }); \ No newline at end of file +const hre = require("hardhat"); +const { getSigners, ZeroAddress } = hre.ethers; +const { expect } = require("chai"); +const Receipt = require("../../scripts/domain/Receipt.js"); +const { mockReceipt, mockOffer, mockTwinReceipt, mockCondition } = require("../util/mock"); +const DisputeState = require("../../scripts/domain/DisputeState"); + +/** + * Test the Receipt domain entity + */ +describe("Receipt", function () { + // Suite-wide scope + let receipt, object, promoted, clone, dehydrated, rehydrated, key, value, struct, accounts; + + beforeEach(async function () { + // Get a list of accounts + accounts = await getSigners(); + }); + + context("📋 Constructor", async function () { + it("Should allow creation of valid, fully populated Receipt instance", async function () { + receipt = await mockReceipt(); + expect(receipt.isValid()).is.true; + }); + }); + + context("📋 Field validations", async function () { + beforeEach(async function () { + // Create a valid receipt, then set fields in tests directly + receipt = await mockReceipt(); + expect(receipt.exchangeIdIsValid()).is.true; + expect(receipt.offerIdIsValid()).is.true; + expect(receipt.buyerIdIsValid()).is.true; + expect(receipt.sellerIdIsValid()).is.true; + expect(receipt.priceIsValid()).is.true; + expect(receipt.sellerDepositIsValid()).is.true; + expect(receipt.buyerCancelPenaltyIsValid()).is.true; + expect(receipt.offerFeesIsValid()).is.true; + expect(receipt.agentIdIsValid()).is.true; + expect(receipt.exchangeTokenIsValid()).is.true; + expect(receipt.finalizedDateIsValid()).is.true; + expect(receipt.conditionIsValid()).is.true; + expect(receipt.committedDateIsValid()).is.true; + expect(receipt.redeemedDateIsValid()).is.true; + expect(receipt.voucherExpiredIsValid()).is.true; + expect(receipt.disputeResolverIdIsValid()).is.true; + expect(receipt.disputedDateIsValid()).is.true; + expect(receipt.escalatedDateIsValid()).is.true; + expect(receipt.disputeStateIsValid()).is.true; + expect(receipt.twinReceiptsIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + it("Always present, exchangeId must be the string representation of a non-zero BigNumber", async function () { + // Invalid field value + receipt.exchangeId = "zedzdeadbaby"; + expect(receipt.exchangeIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.exchangeId = new Date(); + expect(receipt.exchangeIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.exchangeId = "0"; + expect(receipt.exchangeIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.exchangeId = "126"; + expect(receipt.exchangeIdIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + it("Always present, offerId must be the string representation of a non-zero BigNumber", async function () { + // Invalid field value + receipt.offerId = "zedzdeadbaby"; + expect(receipt.offerIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.offerId = "0"; + expect(receipt.offerIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.offerId = new Date(); + expect(receipt.offerIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.offerId = "126"; + expect(receipt.offerIdIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + it("Always present, buyerId must be the string representation of a non-zero BigNumber", async function () { + // Invalid field value + receipt.buyerId = "zedzdeadbaby"; + expect(receipt.buyerIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.buyerId = "0"; + expect(receipt.buyerIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.buyerId = "126"; + expect(receipt.buyerIdIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.buyerId = new Date(); + expect(receipt.buyerIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, price must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.price = "zedzdeadbaby"; + expect(receipt.priceIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.price = "0"; + expect(receipt.priceIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.price = "126"; + expect(receipt.priceIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.price = new Date(); + expect(receipt.priceIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, sellerDeposit must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.sellerDeposit = "zedzdeadbaby"; + expect(receipt.sellerDepositIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.sellerDeposit = "0"; + expect(receipt.sellerDepositIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.sellerDeposit = "126"; + expect(receipt.sellerDepositIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.sellerDeposit = new Date(); + expect(receipt.sellerDepositIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, buyerCancelPenalty must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.buyerCancelPenalty = "zedzdeadbaby"; + expect(receipt.buyerCancelPenaltyIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.buyerCancelPenalty = "0"; + expect(receipt.buyerCancelPenaltyIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.buyerCancelPenalty = "126"; + expect(receipt.buyerCancelPenaltyIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.buyerCancelPenalty = new Date(); + expect(receipt.buyerCancelPenaltyIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, offerFees must be a valid OfferFees instance", async function () { + // Invalid field value + receipt.offerFees = 12; + expect(receipt.offerFeesIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.offerFees = "zedzdeadbaby"; + expect(receipt.offerFeesIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.offerFees = true; + expect(receipt.offerFeesIsValid()).is.false; + expect(receipt.isValid()).is.false; + + const mo = await mockOffer(); + // Valid field value + receipt.offerFees = mo.offerFees; + expect(receipt.offerFeesIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.offerFees = new Date(); + expect(receipt.offerFeesIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("If present, agentId must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.agentId = "zedzdeadbaby"; + expect(receipt.agentIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.agentId = "0"; + expect(receipt.priceIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.agentId = "126"; + expect(receipt.agentIdIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.agentId = new Date(); + expect(receipt.agentIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, exchangeToken must be a string representation of an EIP-55 compliant address", async function () { + // Invalid field value + receipt.exchangeToken = "0xASFADF"; + expect(receipt.exchangeTokenIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.exchangeToken = "zedzdeadbaby"; + expect(receipt.exchangeTokenIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.exchangeToken = accounts[0].address; + expect(receipt.exchangeTokenIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.exchangeToken = "0xec2fd5bd6fc7b576dae82c0b9640969d8de501a2"; + expect(receipt.exchangeTokenIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + it("If present, finalizedDate must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.finalizedDate = "zedzdeadbaby"; + expect(receipt.finalizedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.finalizedDate = "0"; + expect(receipt.finalizedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.finalizedDate = "126"; + expect(receipt.finalizedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.finalizedDate = new Date(); + expect(receipt.finalizedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("If present, condition must be a valid Condition instance", async function () { + // Invalid field value + receipt.condition = "zedzdeadbaby"; + expect(receipt.conditionIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.condition = "126"; + expect(receipt.conditionIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.condition = mockCondition(); + expect(receipt.conditionIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.condition = new Date(); + expect(receipt.conditionIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, committedDate must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.committedDate = "zedzdeadbaby"; + expect(receipt.committedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.committedDate = "126"; + expect(receipt.committedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.committedDate = new Date(); + expect(receipt.committedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("If present, redeemedDate must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.redeemedDate = "zedzdeadbaby"; + expect(receipt.redeemedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.redeemedDate = "0"; + expect(receipt.redeemedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.redeemedDate = "126"; + expect(receipt.redeemedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.redeemedDate = new Date(); + expect(receipt.redeemedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("Always present, voucherExpired must be a boolean", async function () { + // Invalid field value + receipt.voucherExpired = 12; + expect(receipt.voucherExpiredIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.voucherExpired = "zedzdeadbaby"; + expect(receipt.voucherExpiredIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.voucherExpired = false; + expect(receipt.voucherExpiredIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.voucherExpired = true; + expect(receipt.voucherExpiredIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + it("If present, disputeResolverId must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.disputeResolverId = "zedzdeadbaby"; + expect(receipt.disputeResolverIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.disputeResolverId = "0"; + expect(receipt.disputeResolverIdIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.disputeResolverId = "126"; + expect(receipt.disputeResolverIdIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.disputeResolverId = new Date(); + expect(receipt.disputeResolverIdIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("If present, disputedDate must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.disputedDate = "zedzdeadbaby"; + expect(receipt.disputedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.disputedDate = "0"; + expect(receipt.disputedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.disputedDate = "126"; + expect(receipt.disputedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.disputedDate = new Date(); + expect(receipt.disputedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("If present, escalatedDate must be the string representation of BigNumber", async function () { + // Invalid field value + receipt.escalatedDate = "zedzdeadbaby"; + expect(receipt.escalatedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.escalatedDate = "0"; + expect(receipt.escalatedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Valid field value + receipt.escalatedDate = "126"; + expect(receipt.escalatedDateIsValid()).is.true; + expect(receipt.isValid()).is.true; + + // Invalid field value + receipt.escalatedDate = new Date(); + expect(receipt.escalatedDateIsValid()).is.false; + expect(receipt.isValid()).is.false; + }); + + it("If present, disputeState must be the string representation of a BigNumber", async function () { + // Invalid field value + receipt.disputeState = "zedzdeadbaby"; + expect(receipt.disputeStateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.disputeState = "0"; + expect(receipt.disputeStateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.disputeState = "126"; + expect(receipt.disputeStateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.disputeState = new Date(); + expect(receipt.disputeStateIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.disputeState = DisputeState.Resolving; + expect(receipt.disputeStateIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + it("twinReceipt must be a valid TwinReceipt instance", async function () { + // Invalid field value + receipt.twinReceipts = 12; + expect(receipt.twinReceiptsIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Invalid field value + receipt.twinReceipts = "zedzdeadbaby"; + expect(receipt.twinReceiptsIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.twinReceipts = true; + expect(receipt.twinReceiptsIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.twinReceipts = new Date(); + expect(receipt.twinReceiptsIsValid()).is.false; + expect(receipt.isValid()).is.false; + + // Valid field value + receipt.twinReceipts = [mockTwinReceipt(ZeroAddress)]; + expect(receipt.twinReceiptsIsValid()).is.true; + expect(receipt.isValid()).is.true; + }); + + context("📋 Utility functions", async function () { + beforeEach(async function () { + // Create a valid receipt then set fields in tests directly + receipt = object = await mockReceipt(); + expect(receipt.isValid()).is.true; + + const { + exchangeId, + offerId, + buyerId, + sellerId, + price, + sellerDeposit, + buyerCancelPenalty, + offerFees, + agentId, + exchangeToken, + finalizedDate, + condition, + committedDate, + redeemedDate, + voucherExpired, + disputeResolverId, + disputedDate, + escalatedDate, + disputeState, + twinReceipts, + } = receipt; + + // Struct representation + struct = [ + exchangeId, + offerId, + buyerId, + sellerId, + price, + sellerDeposit, + buyerCancelPenalty, + offerFees.toStruct(), + agentId, + exchangeToken, + finalizedDate, + condition.toStruct(), + committedDate, + redeemedDate, + voucherExpired, + disputeResolverId, + disputedDate, + escalatedDate, + disputeState, + twinReceipts.map((twinReceipt) => twinReceipt.toStruct()), + ]; + }); + + context("👉 Static", async function () { + it("Receipt.fromObject() should return a Receipt instance with the same values as the given plain object", async function () { + // Promote to instance + promoted = Receipt.fromObject(object); + + // Is a Receipt instance + expect(promoted instanceof Receipt).is.true; + + // Key values all match + for ([key, value] of Object.entries(receipt)) { + expect(JSON.stringify(promoted[key]) === JSON.stringify(value)).is.true; + } + }); + + it("Receipt.fromStruct() should return a Receipt instance from a struct representation", async function () { + // Get an instance from the struct + receipt = Receipt.fromStruct(struct); + + // Ensure it is valid + expect(receipt.isValid()).to.be.true; + }); + }); + + context("👉 Instance", async function () { + it("instance.toString() should return a JSON string representation of the Receipt instance", async function () { + dehydrated = receipt.toString(); + rehydrated = JSON.parse(dehydrated); + + for ([key, value] of Object.entries(receipt)) { + expect(JSON.stringify(rehydrated[key]) === JSON.stringify(value)).is.true; + } + }); + + it("instance.clone() should return another Receipt instance with the same property values", async function () { + // Get plain object + clone = receipt.clone(); + + // Is an Receipt instance + expect(clone instanceof Receipt).is.true; + + // Key values all match + for ([key, value] of Object.entries(receipt)) { + expect(JSON.stringify(clone[key]) === JSON.stringify(value)).is.true; + } + }); + + it("instance.toObject() should return a plain object representation of the Receipt instance", async function () { + // Get plain object + object = receipt.toObject(); + + // Not an Receipt instance + expect(object instanceof Receipt).is.false; + + // Key values all match + for ([key, value] of Object.entries(receipt)) { + expect(JSON.stringify(object[key]) === JSON.stringify(value)).is.true; + } + }); + + it("instance.toStruct() should return a struct representation of the Receipt instance", async function () { + // Get struct from receipt + struct = receipt.toStruct(); + + // Marshal back to a receipt instance + receipt = Receipt.fromStruct(struct); + + // Ensure it marshals back to a valid receipt + expect(receipt.isValid()).to.be.true; + }); + }); + }); + }); +}); diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index 46c7d89e5..d4936ff9e 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -990,10 +990,9 @@ describe("IBosonExchangeHandler", function () { // mint enough tokens for the buyer await foreign20.connect(buyer).mint(await buyer.getAddress(), condition.threshold); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that enforces per-address conditions and utilizes ERC721 tokens", async function () { @@ -1015,10 +1014,9 @@ describe("IBosonExchangeHandler", function () { await foreign721.connect(buyer).mint("123", 1); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that enforces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () { @@ -1042,10 +1040,9 @@ describe("IBosonExchangeHandler", function () { await foreign1155.connect(buyer).mint(condition.tokenId, condition.threshold); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that has no condition", async function () { @@ -1066,10 +1063,9 @@ describe("IBosonExchangeHandler", function () { await foreign721.connect(buyer).mint("123", 1); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that enforces per-wallet conditions and range length == 1", async function () { @@ -1097,10 +1093,9 @@ describe("IBosonExchangeHandler", function () { // mint enough tokens for the buyer await foreign721.connect(buyer).mint(condition.tokenId, 1); - await expect(bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId)).to.emit( - exchangeHandler, - "BuyerCommitted" - ); + await expect( + bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + ).to.emit(exchangeHandler, "BuyerCommitted"); }); }); @@ -1362,7 +1357,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); }); @@ -1374,7 +1371,9 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1383,7 +1382,9 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1400,7 +1401,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); @@ -1411,13 +1414,17 @@ describe("IBosonExchangeHandler", function () { .createOffer(offer, offerDates, offerDurations, disputeResolverId, agentId); await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), ++offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), ++offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.NO_SUCH_GROUP); }); it("Caller sends non-zero tokenId", async function () {}); await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); }); }); @@ -1450,7 +1457,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); }); @@ -1462,7 +1471,9 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1471,7 +1482,9 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1486,17 +1499,21 @@ describe("IBosonExchangeHandler", function () { .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }); } - // Attempt to commit again after maximum commits has been reached + // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); it("Caller sends non-zero tokenId", async function () { await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) ).to.revertedWith(RevertReasons.INVALID_TOKEN_ID); - }); + }); }); }); @@ -1534,7 +1551,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); }); @@ -1546,7 +1565,9 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1555,7 +1576,9 @@ describe("IBosonExchangeHandler", function () { it("buyer does not meet condition for commit", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1572,7 +1595,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); }); @@ -1611,7 +1636,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); }); @@ -1620,7 +1647,9 @@ describe("IBosonExchangeHandler", function () { for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1637,7 +1666,9 @@ describe("IBosonExchangeHandler", function () { // buyer can commit await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.not.reverted; }); @@ -1646,7 +1677,9 @@ describe("IBosonExchangeHandler", function () { tokenId = "13"; // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.ERC721_NON_EXISTENT); }); @@ -1656,7 +1689,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1670,7 +1705,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_TOKEN_REACHED); }); @@ -1678,7 +1715,9 @@ describe("IBosonExchangeHandler", function () { tokenId = "666"; // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.TOKEN_ID_NOT_IN_CONDITION_RANGE); }); }); @@ -1718,20 +1757,20 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. // We're only concerned that the event is emitted, indicating the condition was met await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); }); - - - - it("should allow buyer to commit up to the max times for the group", async function () { // Commit to offer the maximum number of times for (let i = 0; i < Number(condition.maxCommits); i++) { // We're only concerned that the event is emitted, indicating the commit was allowed await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.emit(exchangeHandler, "BuyerCommitted"); } }); @@ -1748,7 +1787,9 @@ describe("IBosonExchangeHandler", function () { // buyer can commit await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.not.reverted; }); @@ -1758,7 +1799,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1779,7 +1822,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit again after maximum commits has been reached await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.MAX_COMMITS_TOKEN_REACHED); }); @@ -1787,7 +1832,9 @@ describe("IBosonExchangeHandler", function () { tokenId = "666"; // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.TOKEN_ID_NOT_IN_CONDITION_RANGE); }); }); @@ -1829,7 +1876,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to create an exchange, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.REGION_PAUSED); }); @@ -1839,7 +1888,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to create a buyer, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.REGION_PAUSED); }); @@ -1858,7 +1909,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.NO_SUCH_OFFER); }); @@ -1868,7 +1921,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit to the voided offer, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_HAS_BEEN_VOIDED); }); @@ -1893,7 +1948,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit to the not availabe offer, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_NOT_AVAILABLE); }); @@ -1903,7 +1960,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit to the expired offer, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_HAS_EXPIRED); }); @@ -1924,7 +1983,9 @@ describe("IBosonExchangeHandler", function () { // Attempt to commit to the sold out offer, expecting revert await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.OFFER_SOLD_OUT); }); @@ -1951,7 +2012,9 @@ describe("IBosonExchangeHandler", function () { // Commit to offer. await expect( - exchangeHandler.connect(buyer).commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) + exchangeHandler + .connect(buyer) + .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.GROUP_HAS_NO_CONDITION); }); }); @@ -3088,7 +3151,14 @@ describe("IBosonExchangeHandler", function () { await expect(tx) .to.emit(exchangeHandler, "TwinTransferFailed") - .withArgs(twin20.id, twin20.tokenAddress, exchange.id, twin20.tokenId, twin20.amount, await buyer.getAddress()); + .withArgs( + twin20.id, + twin20.tokenAddress, + exchange.id, + twin20.tokenId, + twin20.amount, + await buyer.getAddress() + ); // Get the exchange state [, response] = await exchangeHandler.connect(rando).getExchangeState(exchange.id); diff --git a/test/protocol/OrchestrationHandlerTest.js b/test/protocol/OrchestrationHandlerTest.js index 4ab5f6b25..390ed1ef8 100644 --- a/test/protocol/OrchestrationHandlerTest.js +++ b/test/protocol/OrchestrationHandlerTest.js @@ -6874,7 +6874,6 @@ describe("IBosonOrchestrationHandler", function () { condition = mockCondition({ tokenType: TokenType.MultiToken, tokenAddress: await other2.getAddress(), - tokenAddress: other2.address, method: EvaluationMethod.Threshold, }); expect(condition.isValid()).to.be.true; diff --git a/test/util/upgrade.js b/test/util/upgrade.js index babc637b0..ca5211c64 100644 --- a/test/util/upgrade.js +++ b/test/util/upgrade.js @@ -88,8 +88,6 @@ async function deploySuite(deployer, newVersion) { // checkout old version const { oldVersion: tag, deployScript: scriptsTag } = versionTags; - console.log("only", versionTags); - console.log(`Fetching tags`); shell.exec(`git fetch --force --tags origin`); console.log(`Checking out version ${tag}`); From 21b2c00c297b7bc7db260ab581c5be1c74937487 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 6 Jul 2023 13:18:58 -0300 Subject: [PATCH 30/31] Fix Condition:isValid method --- scripts/domain/Condition.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/domain/Condition.js b/scripts/domain/Condition.js index d8fbbb08c..225eaa0ba 100644 --- a/scripts/domain/Condition.js +++ b/scripts/domain/Condition.js @@ -162,13 +162,13 @@ class Condition { * @returns {boolean} */ isValid() { - this.methodIsValid() && - this.tokenTypeIsValid() && - this.tokenAddressIsValid() && - this.tokenIdIsValid() && - this.thresholdIsValid() && - this.maxCommitsIsValid() && - this.lengthIsValid(); + return(this.methodIsValid() && + this.tokenTypeIsValid() && + this.tokenAddressIsValid() && + this.tokenIdIsValid() && + this.thresholdIsValid() && + this.maxCommitsIsValid() && + this.lengthIsValid()); } } From 1cc0b3254bf51c7525f2615fc5d6934fa6d04a35 Mon Sep 17 00:00:00 2001 From: Ana Julia Date: Thu, 6 Jul 2023 13:48:18 -0300 Subject: [PATCH 31/31] Turn alterations on this PR compatible with ether.js 6 --- contracts/domain/BosonConstants.sol | 1 + .../protocol/clients/voucher/BosonVoucher.sol | 4 +- scripts/domain/Condition.js | 16 ++-- test/example/SnapshotGateTest.js | 1 + test/protocol/ExchangeHandlerTest.js | 80 +++++++++++-------- test/protocol/GroupHandlerTest.js | 4 +- 6 files changed, 62 insertions(+), 44 deletions(-) diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 30187ef63..be66e44e6 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -205,6 +205,7 @@ string constant ROYALTY_FEE_INVALID = "ERC2981: royalty fee exceeds protocol lim string constant NOT_COMMITTABLE = "Token not committable"; string constant INVALID_TO_ADDRESS = "Tokens can only be pre-mined to the contract or contract owner address"; string constant EXTERNAL_CALL_FAILED = "External call failed"; +string constant ERC721_INVALID_TOKEN_ID = "ERC721: invalid token ID"; // Meta Transactions - Structs bytes32 constant META_TRANSACTION_TYPEHASH = keccak256( diff --git a/contracts/protocol/clients/voucher/BosonVoucher.sol b/contracts/protocol/clients/voucher/BosonVoucher.sol index 3cad102b2..0aa3e02ce 100644 --- a/contracts/protocol/clients/voucher/BosonVoucher.sol +++ b/contracts/protocol/clients/voucher/BosonVoucher.sol @@ -373,7 +373,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable if (committable) return owner; // Otherwise revert - revert(INVALID_TOKEN_ID); + revert(ERC721_INVALID_TOKEN_ID); } } @@ -465,7 +465,7 @@ contract BosonVoucherBase is IBosonVoucher, BeaconClientBase, OwnableUpgradeable } } - require(exists, INVALID_TOKEN_ID); + require(exists, ERC721_INVALID_TOKEN_ID); return offer.metadataUri; } diff --git a/scripts/domain/Condition.js b/scripts/domain/Condition.js index 225eaa0ba..10f6dfb0c 100644 --- a/scripts/domain/Condition.js +++ b/scripts/domain/Condition.js @@ -162,13 +162,15 @@ class Condition { * @returns {boolean} */ isValid() { - return(this.methodIsValid() && - this.tokenTypeIsValid() && - this.tokenAddressIsValid() && - this.tokenIdIsValid() && - this.thresholdIsValid() && - this.maxCommitsIsValid() && - this.lengthIsValid()); + return ( + this.methodIsValid() && + this.tokenTypeIsValid() && + this.tokenAddressIsValid() && + this.tokenIdIsValid() && + this.thresholdIsValid() && + this.maxCommitsIsValid() && + this.lengthIsValid() + ); } } diff --git a/test/example/SnapshotGateTest.js b/test/example/SnapshotGateTest.js index cebb260b9..4dd50d853 100644 --- a/test/example/SnapshotGateTest.js +++ b/test/example/SnapshotGateTest.js @@ -356,6 +356,7 @@ describe("SnapshotGate", function () { method: EvaluationMethod.SpecificToken, length: "3", }); + expect(condition.isValid()).to.be.true; // Create Group diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index d4936ff9e..38cc28bb2 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -687,7 +687,7 @@ describe("IBosonExchangeHandler", function () { // Create Condition condition = mockCondition({ method: EvaluationMethod.None, - tokenAddress: ethers.constants.AddressZero, + tokenAddress: ZeroAddress, threshold: "0", maxCommits: "0", }); @@ -724,7 +724,7 @@ describe("IBosonExchangeHandler", function () { ).to.revertedWith(RevertReasons.REGION_PAUSED); }); - it("await buyer.getAddress() is the zero address", async function () { + it("buyer.address is the zero address", async function () { // Attempt to commit, expecting revert await expect( exchangeHandler.connect(buyer).commitToOffer(ZeroAddress, offerId, { value: price }) @@ -801,7 +801,7 @@ describe("IBosonExchangeHandler", function () { offerIds = [offerId]; // Create Condition - condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + condition = mockCondition({ tokenAddress: await foreign20.getAddress(), threshold: "50", maxCommits: "3" }); expect(condition.isValid()).to.be.true; // Create Group @@ -978,7 +978,7 @@ describe("IBosonExchangeHandler", function () { it("Offer is part of a group that enforces per-address conditions and utilizes ERC20 tokens", async function () { // Create Condition - condition = mockCondition({ tokenAddress: foreign20.address, threshold: "50", maxCommits: "3" }); + condition = mockCondition({ tokenAddress: await foreign20.getAddress(), threshold: "50", maxCommits: "3" }); expect(condition.isValid()).to.be.true; // Create Group @@ -991,13 +991,15 @@ describe("IBosonExchangeHandler", function () { await foreign20.connect(buyer).mint(await buyer.getAddress(), condition.threshold); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that enforces per-address conditions and utilizes ERC721 tokens", async function () { condition = mockCondition({ - tokenAddress: foreign721.address, + tokenAddress: await foreign721.getAddress(), threshold: "1", maxCommits: "3", tokenType: TokenType.NonFungibleToken, @@ -1015,13 +1017,15 @@ describe("IBosonExchangeHandler", function () { await foreign721.connect(buyer).mint("123", 1); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that enforces per-address conditions and utilizes ERC1155 tokens with range length == 1", async function () { condition = mockCondition({ - tokenAddress: foreign1155.address, + tokenAddress: await foreign1155.getAddress(), threshold: "2", maxCommits: "3", tokenType: TokenType.MultiToken, @@ -1041,13 +1045,15 @@ describe("IBosonExchangeHandler", function () { await foreign1155.connect(buyer).mint(condition.tokenId, condition.threshold); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.emit(exchangeHandler, "BuyerCommitted"); }); it("Offer is part of a group that has no condition", async function () { condition = mockCondition({ - tokenAddress: ethers.constants.AddressZero, + tokenAddress: ZeroAddress, threshold: "0", maxCommits: "0", tokenType: TokenType.FungibleToken, @@ -1064,7 +1070,9 @@ describe("IBosonExchangeHandler", function () { await foreign721.connect(buyer).mint("123", 1); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.emit(exchangeHandler, "BuyerCommitted"); }); @@ -1074,7 +1082,7 @@ describe("IBosonExchangeHandler", function () { offerIds = [offerId]; condition = mockCondition({ - tokenAddress: foreign721.address, + tokenAddress: await foreign721.getAddress(), threshold: "0", maxCommits: "3", tokenType: TokenType.NonFungibleToken, @@ -1094,7 +1102,9 @@ describe("IBosonExchangeHandler", function () { await foreign721.connect(buyer).mint(condition.tokenId, 1); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.emit(exchangeHandler, "BuyerCommitted"); }); }); @@ -1228,7 +1238,7 @@ describe("IBosonExchangeHandler", function () { offerIds = [offerId]; condition = mockCondition({ - tokenAddress: foreign1155.address, + tokenAddress: await foreign1155.getAddress(), threshold: "1", maxCommits: "3", tokenType: TokenType.MultiToken, @@ -1244,7 +1254,9 @@ describe("IBosonExchangeHandler", function () { await groupHandler.connect(assistant).createGroup(group, condition); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1254,7 +1266,7 @@ describe("IBosonExchangeHandler", function () { offerIds = [offerId]; condition = mockCondition({ - tokenAddress: foreign721.address, + tokenAddress: await foreign721.getAddress(), threshold: "1", maxCommits: "3", tokenType: TokenType.NonFungibleToken, @@ -1271,7 +1283,9 @@ describe("IBosonExchangeHandler", function () { await groupHandler.connect(assistant).createGroup(group, condition); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1281,7 +1295,7 @@ describe("IBosonExchangeHandler", function () { offerIds = [offerId]; condition = mockCondition({ - tokenAddress: foreign1155.address, + tokenAddress: await foreign1155.getAddress(), threshold: "2", maxCommits: "3", tokenType: TokenType.MultiToken, // ERC1155 @@ -1299,7 +1313,9 @@ describe("IBosonExchangeHandler", function () { await groupHandler.connect(assistant).createGroup(group, condition); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); @@ -1309,7 +1325,7 @@ describe("IBosonExchangeHandler", function () { offerIds = [offerId]; condition = mockCondition({ - tokenAddress: foreign721.address, + tokenAddress: await foreign721.getAddress(), threshold: "0", maxCommits: "3", tokenType: TokenType.NonFungibleToken, // ERC721 @@ -1327,7 +1343,9 @@ describe("IBosonExchangeHandler", function () { await groupHandler.connect(assistant).createGroup(group, condition); await expect( - bosonVoucher.connect(assistant).transferFrom(assistant.address, await buyer.getAddress(), tokenId) + bosonVoucher + .connect(assistant) + .transferFrom(await assistant.getAddress(), await buyer.getAddress(), tokenId) ).to.revertedWith(RevertReasons.CANNOT_COMMIT); }); }); @@ -1503,8 +1521,8 @@ describe("IBosonExchangeHandler", function () { await expect( exchangeHandler .connect(buyer) - .commitToConditionalOffer(await buyer.getAddress(), offerId, 1, { value: price }) - ).to.revertedWith(RevertReasons.CANNOT_COMMIT); + .commitToConditionalOffer(await buyer.getAddress(), offerId, 0, { value: price }) + ).to.revertedWith(RevertReasons.MAX_COMMITS_ADDRESS_REACHED); }); it("Caller sends non-zero tokenId", async function () { @@ -1680,7 +1698,7 @@ describe("IBosonExchangeHandler", function () { exchangeHandler .connect(buyer) .commitToConditionalOffer(await buyer.getAddress(), offerId, tokenId, { value: price }) - ).to.revertedWith(RevertReasons.ERC721_NON_EXISTENT); + ).to.revertedWith(RevertReasons.ERC721_INVALID_TOKEN_ID); }); it("buyer does not meet condition for commit", async function () { @@ -1733,7 +1751,7 @@ describe("IBosonExchangeHandler", function () { // Create Condition condition = mockCondition({ - tokenAddress: foreign1155.address, + tokenAddress: await foreign1155.getAddress(), threshold: "1", maxCommits: "3", tokenType: TokenType.MultiToken, @@ -1851,7 +1869,7 @@ describe("IBosonExchangeHandler", function () { // Create Condition condition = mockCondition({ - tokenAddress: foreign721.address, + tokenAddress: await foreign721.getAddress(), threshold: "0", maxCommits: "3", tokenType: TokenType.NonFungibleToken, @@ -1897,9 +1915,7 @@ describe("IBosonExchangeHandler", function () { it("await buyer.getAddress() is the zero address", async function () { // Attempt to commit, expecting revert await expect( - exchangeHandler - .connect(buyer) - .commitToConditionalOffer(ethers.constants.AddressZero, offerId, tokenId, { value: price }) + exchangeHandler.connect(buyer).commitToConditionalOffer(ZeroAddress, offerId, tokenId, { value: price }) ).to.revertedWith(RevertReasons.INVALID_ADDRESS); }); @@ -1934,10 +1950,8 @@ describe("IBosonExchangeHandler", function () { const now = block.timestamp.toString(); // set validFrom date in the past - offerDates.validFrom = ethers.BigNumber.from(now) - .add(oneMonth * 6) - .toString(); // 6 months in the future - offerDates.validUntil = ethers.BigNumber.from(offerDates.validFrom).add(10).toString(); // just after the valid from so it succeeds. + offerDates.validFrom = (BigInt(now) + BigInt(oneMonth) * 6n).toString(); // 6 months in the future + offerDates.validUntil = (BigInt(offerDates.validFrom) + 10n).toString(); // just after the valid from so it succeeds. await offerHandler .connect(assistant) diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index aaae653c0..3c44c560e 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -1,5 +1,5 @@ const { ethers } = require("hardhat"); -const { ZeroAddress, getSigners, parseUnits, getContractFactory } = ethers; +const { ZeroAddress, getSigners, parseUnits, getContractFactory, MaxUint256 } = ethers; const { assert, expect } = require("chai"); const Group = require("../../scripts/domain/Group"); @@ -483,7 +483,7 @@ describe("IBosonGroupHandler", function () { it("Condition 'Threshold' with MultiToken has tokenId + length - 1 > max uint256", async function () { condition.tokenType = TokenType.MultiToken; condition.tokenId = 1; - condition.length = ethers.constants.MaxUint256; + condition.length = MaxUint256; // Attempt to create the group, expecting revert await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith(