diff --git a/contracts/domain/BosonConstants.sol b/contracts/domain/BosonConstants.sol index 083e75f3b..537fed017 100644 --- a/contracts/domain/BosonConstants.sol +++ b/contracts/domain/BosonConstants.sol @@ -58,13 +58,13 @@ string constant NO_SUCH_AGENT = "No such agent"; string constant WALLET_OWNS_VOUCHERS = "Wallet address owns vouchers"; string constant NO_SUCH_DISPUTE_RESOLVER = "No such dispute resolver"; string constant INVALID_ESCALATION_PERIOD = "Invalid escalation period"; -string constant INVALID_AMOUNT_DISPUTE_RESOLVER_FEES = "Dispute resolver fees are not present or exceed maximum dispute resolver fees in a single transaction"; +string constant INEXISTENT_DISPUTE_RESOLVER_FEES = "Dispute resolver fees are not present"; string constant DUPLICATE_DISPUTE_RESOLVER_FEES = "Duplicate dispute resolver fee"; string constant FEE_AMOUNT_NOT_YET_SUPPORTED = "Non-zero dispute resolver fees not yet supported"; string constant DISPUTE_RESOLVER_FEE_NOT_FOUND = "Dispute resolver fee not found"; string constant SELLER_ALREADY_APPROVED = "Seller id is approved already"; string constant SELLER_NOT_APPROVED = "Seller id is not approved"; -string constant INVALID_AMOUNT_ALLOWED_SELLERS = "Allowed sellers are not present or exceed maximum allowed sellers in a single transaction"; +string constant INEXISTENT_ALLOWED_SELLERS_LIST = "Allowed sellers are not present"; string constant INVALID_AUTH_TOKEN_TYPE = "Invalid AuthTokenType"; string constant ADMIN_OR_AUTH_TOKEN = "An admin address or an auth token is required"; string constant AUTH_TOKEN_MUST_BE_UNIQUE = "Auth token cannot be assigned to another entity of the same type"; @@ -97,7 +97,6 @@ string constant AGENT_FEE_AMOUNT_TOO_HIGH = "Sum of agent fee amount and protoco // Revert Reasons: Group related string constant NO_SUCH_GROUP = "No such group"; 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"; @@ -108,7 +107,6 @@ string constant VOUCHER_NOT_REDEEMABLE = "Voucher not yet valid or already expir string constant VOUCHER_EXTENSION_NOT_VALID = "Proposed date is not later than the current one"; string constant VOUCHER_STILL_VALID = "Voucher still valid"; string constant VOUCHER_HAS_EXPIRED = "Voucher has expired"; -string constant TOO_MANY_EXCHANGES = "Exceeded maximum exchanges in a single transaction"; string constant EXCHANGE_IS_NOT_IN_A_FINAL_STATE = "Exchange is not in a final state"; string constant EXCHANGE_ALREADY_EXISTS = "Exchange already exists"; string constant INVALID_RANGE_LENGTH = "Range length is too large or zero"; @@ -129,7 +127,6 @@ string constant INVALID_TOKEN_ADDRESS = "Token address is a contract that doesn' string constant NO_SUCH_BUNDLE = "No such bundle"; string constant TWIN_NOT_IN_BUNDLE = "Twin not part of the bundle"; string constant OFFER_NOT_IN_BUNDLE = "Offer not part of the bundle"; -string constant TOO_MANY_TWINS = "Exceeded maximum twins in a single transaction"; string constant BUNDLE_OFFER_MUST_BE_UNIQUE = "Offer must be unique to a bundle"; string constant BUNDLE_TWIN_MUST_BE_UNIQUE = "Twin must be unique to a bundle"; string constant EXCHANGE_FOR_BUNDLED_OFFERS_EXISTS = "Exchange for the bundled offers exists"; @@ -141,7 +138,6 @@ string constant NATIVE_WRONG_ADDRESS = "Native token address must be 0"; string constant NATIVE_WRONG_AMOUNT = "Transferred value must match amount"; string constant TOKEN_NAME_UNSPECIFIED = "Token name unspecified"; string constant NATIVE_CURRENCY = "Native currency"; -string constant TOO_MANY_TOKENS = "Too many tokens"; string constant TOKEN_AMOUNT_MISMATCH = "Number of amounts should match number of tokens"; string constant NOTHING_TO_WITHDRAW = "Nothing to withdraw"; string constant NOT_AUTHORIZED = "Not authorized to withdraw"; @@ -164,7 +160,6 @@ string constant DISPUTE_HAS_EXPIRED = "Dispute has expired"; string constant INVALID_BUYER_PERCENT = "Invalid buyer percent"; string constant DISPUTE_STILL_VALID = "Dispute still valid"; string constant INVALID_DISPUTE_TIMEOUT = "Invalid dispute timeout"; -string constant TOO_MANY_DISPUTES = "Exceeded maximum disputes in a single transaction"; string constant ESCALATION_NOT_ALLOWED = "Disputes without dispute resolver cannot be escalated"; // Revert Reasons: Config related @@ -187,7 +182,6 @@ string constant OFFER_RANGE_ALREADY_RESERVED = "Offer id already associated with string constant INVALID_RANGE_START = "Range start too low"; string constant INVALID_AMOUNT_TO_MINT = "Amount to mint is greater than remaining un-minted in range"; string constant NO_SILENT_MINT_ALLOWED = "Only owner's mappings can be updated without event"; -string constant TOO_MANY_TO_MINT = "Exceeded maximum amount to mint in a single transaction"; string constant OFFER_EXPIRED_OR_VOIDED = "Offer expired or voided"; string constant OFFER_STILL_VALID = "Offer still valid"; string constant NOTHING_TO_BURN = "Nothing to burn"; diff --git a/contracts/mock/MockExchangeHandlerFacet.sol b/contracts/mock/MockExchangeHandlerFacet.sol index be077dca9..bdfc48fe3 100644 --- a/contracts/mock/MockExchangeHandlerFacet.sol +++ b/contracts/mock/MockExchangeHandlerFacet.sol @@ -100,7 +100,6 @@ contract MockExchangeHandlerFacet is BuyerBase, DisputeBase { * * Reverts if: * - The exchanges region of protocol is paused - * - Number of exchanges exceeds maximum allowed number per batch * - For any exchange: * - Exchange does not exist * - Exchange is not in Redeemed state @@ -109,9 +108,6 @@ contract MockExchangeHandlerFacet is BuyerBase, DisputeBase { * @param _exchangeIds - the array of exchanges ids */ function completeExchangeBatch(uint256[] calldata _exchangeIds) external exchangesNotPaused { - // limit maximum number of exchanges to avoid running into block gas limit in a loop - require(_exchangeIds.length <= protocolLimits().maxExchangesPerBatch, TOO_MANY_EXCHANGES); - for (uint256 i = 0; i < _exchangeIds.length; i++) { // complete the exchange completeExchange(_exchangeIds[i]); diff --git a/contracts/protocol/bases/GroupBase.sol b/contracts/protocol/bases/GroupBase.sol index a3834e5c2..c1c86ed53 100644 --- a/contracts/protocol/bases/GroupBase.sol +++ b/contracts/protocol/bases/GroupBase.sol @@ -22,7 +22,6 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * - Any of offers belongs to different seller * - Any of offers does not exist * - Offer exists in a different group - * - Number of offers exceeds maximum allowed number per group * * @param _group - the fully populated struct with group id set to 0x0 * @param _condition - the fully populated condition struct @@ -38,9 +37,6 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { (bool exists, uint256 sellerId) = getSellerIdByAssistant(sender); require(exists, NOT_ASSISTANT); - // limit maximum number of offers to avoid running into block gas limit in a loop - require(_group.offerIds.length <= protocolLimits().maxOffersPerGroup, TOO_MANY_OFFERS); - // condition must be valid require(validateCondition(_condition), INVALID_CONDITION_PARAMETERS); @@ -129,7 +125,6 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * Reverts if: * - Caller is not the seller * - Offer ids param is an empty list - * - Current number of offers plus number of offers added exceeds maximum allowed number per group * - Group does not exist * - Any of offers belongs to different seller * - Any of offers does not exist @@ -146,10 +141,6 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { // check if group can be updated (uint256 sellerId, Group storage group) = preUpdateChecks(_groupId, _offerIds); - // limit maximum number of total offers to avoid running into block gas limit in a loop - // and make sure total number of offers in group does not exceed max - require(group.offerIds.length + _offerIds.length <= protocolLimits().maxOffersPerGroup, TOO_MANY_OFFERS); - for (uint256 i = 0; i < _offerIds.length; i++) { uint256 offerId = _offerIds[i]; // make sure offer exist and belong to the seller @@ -183,7 +174,6 @@ contract GroupBase is ProtocolBase, IBosonGroupEvents { * Reverts if: * - Caller is not the seller * - Offer ids param is an empty list - * - Number of offers exceeds maximum allowed number per group * - Group does not exist * * @param _groupId - the id of the group to be updated diff --git a/contracts/protocol/facets/ConfigHandlerFacet.sol b/contracts/protocol/facets/ConfigHandlerFacet.sol index 92630491b..5fafc1991 100644 --- a/contracts/protocol/facets/ConfigHandlerFacet.sol +++ b/contracts/protocol/facets/ConfigHandlerFacet.sol @@ -38,6 +38,7 @@ contract ConfigHandlerFacet is IBosonConfigHandler, ProtocolBase { setBeaconProxyAddress(_addresses.beaconProxy); setProtocolFeePercentage(_fees.percentage); setProtocolFeeFlatBoson(_fees.flatBoson); + setMaxEscalationResponsePeriod(_limits.maxEscalationResponsePeriod); setBuyerEscalationDepositPercentage(_fees.buyerEscalationDepositPercentage); setMaxTotalOfferFeePercentage(_limits.maxTotalOfferFeePercentage); setMaxRoyaltyPecentage(_limits.maxRoyaltyPecentage); diff --git a/contracts/protocol/facets/DisputeHandlerFacet.sol b/contracts/protocol/facets/DisputeHandlerFacet.sol index 74d7c8076..65759c65c 100644 --- a/contracts/protocol/facets/DisputeHandlerFacet.sol +++ b/contracts/protocol/facets/DisputeHandlerFacet.sol @@ -188,7 +188,6 @@ contract DisputeHandlerFacet is DisputeBase, IBosonDisputeHandler { * * Reverts if: * - The disputes region of protocol is paused - * - Number of disputes exceeds maximum allowed number per batch * - For any dispute: * - Exchange does not exist * - Exchange is not in a Disputed state @@ -198,9 +197,6 @@ contract DisputeHandlerFacet is DisputeBase, IBosonDisputeHandler { * @param _exchangeIds - the array of ids of the associated exchanges */ function expireDisputeBatch(uint256[] calldata _exchangeIds) external override { - // limit maximum number of disputes to avoid running into block gas limit in a loop - require(_exchangeIds.length <= protocolLimits().maxDisputesPerBatch, TOO_MANY_DISPUTES); - for (uint256 i = 0; i < _exchangeIds.length; i++) { // create offer and update structs values to represent true state expireDispute(_exchangeIds[i]); diff --git a/contracts/protocol/facets/DisputeResolverHandlerFacet.sol b/contracts/protocol/facets/DisputeResolverHandlerFacet.sol index d8ecb2c87..574ba2208 100644 --- a/contracts/protocol/facets/DisputeResolverHandlerFacet.sol +++ b/contracts/protocol/facets/DisputeResolverHandlerFacet.sol @@ -34,7 +34,6 @@ contract DisputeResolverHandlerFacet is IBosonAccountEvents, ProtocolBase { * - Any address is zero address * - Any address is not unique to this dispute resolver * - EscalationResponsePeriod is invalid - * - Number of seller ids in _sellerAllowList array exceeds max * - Some seller does not exist * - Some seller id is duplicated * - DisputeResolver is not active (if active == false) @@ -93,15 +92,6 @@ contract DisputeResolverHandlerFacet is IBosonAccountEvents, ProtocolBase { // Cache protocol limits for reference ProtocolLib.ProtocolLimits storage limits = protocolLimits(); - // Make sure the gas block limit is not hit - require(_sellerAllowList.length <= limits.maxAllowedSellers, INVALID_AMOUNT_ALLOWED_SELLERS); - - // The number of fees cannot exceed the maximum number of dispute resolver fees to avoid running into block gas limit in a loop - require( - _disputeResolverFees.length <= limits.maxFeesPerDisputeResolver, - INVALID_AMOUNT_DISPUTE_RESOLVER_FEES - ); - // Escalation period must be greater than zero and less than or equal to the max allowed require( _disputeResolver.escalationResponsePeriod > 0 && @@ -412,12 +402,8 @@ contract DisputeResolverHandlerFacet is IBosonAccountEvents, ProtocolBase { // Check that msg.sender is the admin address for this dispute resolver require(disputeResolver.admin == sender, NOT_ADMIN); - // At least one fee must be specified and the number of fees cannot exceed the maximum number of dispute resolver fees to avoid running into block gas limit in a loop - require( - _disputeResolverFees.length > 0 && - _disputeResolverFees.length <= protocolLimits().maxFeesPerDisputeResolver, - INVALID_AMOUNT_DISPUTE_RESOLVER_FEES - ); + // At least one fee must be specified + require(_disputeResolverFees.length > 0, INEXISTENT_DISPUTE_RESOLVER_FEES); // Set dispute resolver fees. Must loop because calldata structs cannot be converted to storage structs for (uint256 i = 0; i < _disputeResolverFees.length; i++) { @@ -476,11 +462,8 @@ contract DisputeResolverHandlerFacet is IBosonAccountEvents, ProtocolBase { // Check that msg.sender is the admin address for this dispute resolver require(disputeResolver.admin == sender, NOT_ADMIN); - // At least one fee must be specified and the number of fees cannot exceed the maximum number of dispute resolver fees to avoid running into block gas limit in a loop - require( - _feeTokenAddresses.length > 0 && _feeTokenAddresses.length <= protocolLimits().maxFeesPerDisputeResolver, - INVALID_AMOUNT_DISPUTE_RESOLVER_FEES - ); + // At least one fee must be specified and + require(_feeTokenAddresses.length > 0, INEXISTENT_DISPUTE_RESOLVER_FEES); // Set dispute resolver fees. Must loop because calldata structs cannot be converted to storage structs for (uint256 i = 0; i < _feeTokenAddresses.length; i++) { @@ -530,11 +513,8 @@ contract DisputeResolverHandlerFacet is IBosonAccountEvents, ProtocolBase { uint256 _disputeResolverId, uint256[] calldata _sellerAllowList ) external disputeResolversNotPaused nonReentrant { - // At least one seller id must be specified and the number of ids cannot exceed the maximum number of seller ids to avoid running into block gas limit in a loop - require( - _sellerAllowList.length > 0 && _sellerAllowList.length <= protocolLimits().maxAllowedSellers, - INVALID_AMOUNT_ALLOWED_SELLERS - ); + // At least one seller id must be specified + require(_sellerAllowList.length > 0, INEXISTENT_ALLOWED_SELLERS_LIST); bool exists; DisputeResolver storage disputeResolver; @@ -578,11 +558,8 @@ contract DisputeResolverHandlerFacet is IBosonAccountEvents, ProtocolBase { // Cache protocol lookups for reference ProtocolLib.ProtocolLookups storage lookups = protocolLookups(); - // At least one seller id must be specified and the number of ids cannot exceed the maximum number of seller ids to avoid running into block gas limit in a loop - require( - _sellerAllowList.length > 0 && _sellerAllowList.length <= protocolLimits().maxAllowedSellers, - INVALID_AMOUNT_ALLOWED_SELLERS - ); + // At least one seller id must be specified + require(_sellerAllowList.length > 0, INEXISTENT_ALLOWED_SELLERS_LIST); bool exists; DisputeResolver storage disputeResolver; diff --git a/contracts/protocol/facets/FundsHandlerFacet.sol b/contracts/protocol/facets/FundsHandlerFacet.sol index 99e972497..0a39f427f 100644 --- a/contracts/protocol/facets/FundsHandlerFacet.sol +++ b/contracts/protocol/facets/FundsHandlerFacet.sol @@ -199,7 +199,6 @@ contract FundsHandlerFacet is IBosonFundsHandler, ProtocolBase { * Reverts if: * - Caller is not associated with the entity id * - Token list length does not match amount list length - * - Token list length exceeds the maximum allowed number of tokens * - Caller tries to withdraw more that they have in available funds * - There is nothing to withdraw * - Transfer of funds is not successful @@ -221,10 +220,6 @@ contract FundsHandlerFacet is IBosonFundsHandler, ProtocolBase { // Make sure that the data is complete require(_tokenList.length == _tokenAmounts.length, TOKEN_AMOUNT_MISMATCH); - // Limit maximum number of tokens to avoid running into block gas limit in a loop - uint256 maxTokensPerWithdrawal = protocolLimits().maxTokensPerWithdrawal; - require(_tokenList.length <= maxTokensPerWithdrawal, TOO_MANY_TOKENS); - // Two possible options: withdraw all, or withdraw only specified tokens and amounts if (_tokenList.length == 0) { // Withdraw everything diff --git a/contracts/protocol/facets/OfferHandlerFacet.sol b/contracts/protocol/facets/OfferHandlerFacet.sol index 17699c11a..b42fda838 100644 --- a/contracts/protocol/facets/OfferHandlerFacet.sol +++ b/contracts/protocol/facets/OfferHandlerFacet.sol @@ -70,7 +70,6 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { * * Reverts if: * - The offers region of protocol is paused - * - Number of offers exceeds maximum allowed number per batch * - Number of elements in offers, offerDates and offerDurations do not match * - For any offer: * - Caller is not an assistant @@ -106,8 +105,6 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { uint256[] calldata _disputeResolverIds, uint256[] calldata _agentIds ) external override offersNotPaused nonReentrant { - // Limit maximum number of offers to avoid running into block gas limit in a loop - require(_offers.length <= protocolLimits().maxOffersPerBatch, TOO_MANY_OFFERS); // Number of offer dates structs, offer durations structs and _disputeResolverIds must match the number of offers require( _offers.length == _offerDates.length && @@ -182,7 +179,6 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { * * Reverts if, for any offer: * - The offers region of protocol is paused - * - Number of offers exceeds maximum allowed number per batch * - Offer id is invalid * - Caller is not the assistant of the offer * - Offer has already been voided @@ -190,8 +186,6 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { * @param _offerIds - list of ids of offers to void */ function voidOfferBatch(uint256[] calldata _offerIds) external override offersNotPaused { - // limit maximum number of offers to avoid running into block gas limit in a loop - require(_offerIds.length <= protocolLimits().maxOffersPerBatch, TOO_MANY_OFFERS); for (uint256 i = 0; i < _offerIds.length; i++) { voidOffer(_offerIds[i]); } @@ -241,7 +235,6 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { * * Reverts if: * - The offers region of protocol is paused - * - Number of offers exceeds maximum allowed number per batch * - For any of the offers: * - Offer does not exist * - Caller is not the assistant of the offer @@ -252,8 +245,6 @@ contract OfferHandlerFacet is IBosonOfferHandler, OfferBase { * @param _validUntilDate - new valid until date */ function extendOfferBatch(uint256[] calldata _offerIds, uint256 _validUntilDate) external override offersNotPaused { - // Limit maximum number of offers to avoid running into block gas limit in a loop - require(_offerIds.length <= protocolLimits().maxOffersPerBatch, TOO_MANY_OFFERS); for (uint256 i = 0; i < _offerIds.length; i++) { extendOffer(_offerIds[i], _validUntilDate); } diff --git a/docs/limit-estimation.md b/docs/limit-estimation.md deleted file mode 100644 index 7099167bf..000000000 --- a/docs/limit-estimation.md +++ /dev/null @@ -1,138 +0,0 @@ -[![banner](images/banner.png)](https://bosonprotocol.io) - -

Boson Protocol V2

- -### [Intro](../README.md) | [Audits](audits.md) | [Setup](setup.md) | [Tasks](tasks.md) | [Architecture](architecture.md) | [Domain Model](domain.md) | [State Machines](state-machines.md) | [Sequences](sequences.md) - -# Protocol limit estimation - -Certain actions in the protocol require looping over dynamic size arrays. To avoid hitting the block gas limit, special protocol limits were introduced which revert the transaction before the loop even starts. Values for these limits are then determined through estimation process, described here. - -## Identifying the limits - -First we identify which limits we use at the moment and which functions (or chain of functions) use them. The goal is to identify the external functions which can be called during the estimation. - -### List of limits - -| limit | used in | remarks | -| :---- | :------ | :------ | -| maxExchangesPerBatch | **completeExchangeBatch** | | -| maxOffersPerGroup | createGroupInternal -> **createGroup** | | -| maxOffersPerGroup | createGroupInternal -> **createOfferWithCondition** | not a problem, always length 1 | -| maxOffersPerGroup | preUpdateChecks -> addOffersToGroupInternal -> **addOffersToGroup** | | -| maxOffersPerGroup | preUpdateChecks -> addOffersToGroupInternal -> **createOfferAddToGroup** | not a problem, always length 1 | -| maxOffersPerGroup | preUpdateChecks -> **removeOffersFromGroup** | | -| maxOffersPerBundle | createBundleInternal -> **createBundle** | | -| maxOffersPerBundle | createBundleInternal -> **createTwinAndBundleAfterOffer** | not a problem, always length 1 | -| maxTwinsPerBundle | createBundleInternal -> **createBundle** | | -| maxTwinsPerBundle | createBundleInternal -> **createTwinAndBundleAfterOffer** | not a problem, always length 1 | -| maxOffersPerBatch | **createOfferBatch** | | -| maxOffersPerBatch | **voidOfferBatch** | | -| maxOffersPerBatch | **extendOfferBatch** | | -| maxTokensPerWithdrawal | withdrawFundsInternal -> **withdrawFunds** | | -| maxTokensPerWithdrawal | withdrawFundsInternal -> **withdrawProtocolFees** | | -| maxFeesPerDisputeResolver | **createDisputeResolver** | | -| maxFeesPerDisputeResolver | **addFeesToDisputeResolver** | | -| maxFeesPerDisputeResolver | **removeFeesFromDisputeResolver** | | -| maxDisputesPerBatch | **expireDisputeBatch** | | -| maxAllowedSellers | **createDisputeResolver** | | -| maxAllowedSellers | **addSellersToAllowList** | | -| maxAllowedSellers | **removeSellersFromAllowList** | | - -## Estimation config - -Config file is placed in `scripts/config/limit-estimation.js`. It has the following fields: -- `blockGasLimit`: block gas limit against which you want to make the estimate -- `safeGasLimitPercent`: percent of total gas block limit that you consider safe for a transaction to actually be included in the block. For example if `blockGasLimit` is `30M` and you don't want your transaction to exceed `15M`, set `safeGasLimitPercent` to `50`. -- `maxArrayLength`: maximum length of the array used during the estimation. This value is typically smaller than actual limits calculated at the end. Increasing this value makes estimation more precise, however it also takes more time. Improvement in the estimate is increasing slower than run time, so setting this to `100` should be more than enough. If you want to speed up the process, setting this to `10` will still give you very good results. -- `limits`: list of limits you want to estimate. Each limit is an object with fields: - - `name`: name of the limit - - `methods`: object of pairs `"methodName":"handlerName"` where `methodName` is the name of the external function that uses the limit and `handlerName` is the name of the handler where this function is implemented. Example for limit `maxOffersPerGroup`: - ``` - { - name: "maxOffersPerGroup", - methods: { - createGroup: "IBosonGroupHandler", - addOffersToGroup: "IBosonGroupHandler", - removeOffersFromGroup: "IBosonGroupHandler", - }, - }, - ``` - -## Setting up the environment - -For each of the limits you must prepare an evironment, before it can be tested. For example, before `maxOffersPerGroup` can be tested, protocol contracts must be deployed and enough offers must be created so the limit can actually be tested. A similar setup is needed for all other methods. - -This is done in file `scripts/util/estimate-limits.js`. Each of the limits must have a setup function which accepts `maxArrayLength`, prepares the environment and returns the invocation details that can be then used when invoking the `methods` during the estimation. - -Invocation details contain -- `account`: account that calls the method (important if access is restricted) -- `args`: array of arguments that needs to be passed into method -- `arrayIndex`: index that tells which parameter's length should be varied during the estimation -- `structField`: if array is part of a struct, specify the field name - -The returned object must be in form `{ methodName_1: invocationDetails_1, methodName_2: invocationDetails_2, ..., methodName_n: invocationDetails_2}` with details for all methods specified in estimation config. - -## Running the script - -Scrip is run by calling - -```npm run estimate-limits``` - -During the estimation it outputs the information about the method it is estimating. At the end it stores the estimation details into two files: -- `logs/limit_estimates.json` Data in JSON format -- `logs/limit_estimates.md` Data in MD table - -## Results - -The results for parameters -- `blockGasLimit`: `30,000,000` (current ethereum mainnet block gas limit) -- `safeGasLimitPercent`: `60` - -| limit | max value | safe value | -| :-- | --: | --: | -|maxExchangesPerBatch | 557 | 333| -|maxOffersPerGroup | 388 | 232| -|maxOffersPerBundle | 508 | 303| -|maxTwinsPerBundle | 510 | 304| -|maxOffersPerBatch | 51 | 31| -|maxTokensPerWithdrawal | 491 | 293| -|maxFeesPerDisputeResolver | 305 | 181| -|maxDisputesPerBatch | 302 | 181| -|maxAllowedSellers | 597 | 352| - -`max value` is determined based on `blockGasLimit`, while safe value also applies `safeGasLimitPercent`. - -### Gas spent for different sizes of arrays -| # | maxExchangesPerBatch | maxOffersPerGroup | maxOffersPerGroup | maxOffersPerGroup | maxOffersPerBundle | maxTwinsPerBundle | maxOffersPerBatch | maxOffersPerBatch | maxOffersPerBatch | maxTokensPerWithdrawal | maxTokensPerWithdrawal | maxFeesPerDisputeResolver | maxFeesPerDisputeResolver | maxFeesPerDisputeResolver | maxDisputesPerBatch | maxAllowedSellers | maxAllowedSellers | maxAllowedSellers | -|--| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| ---:| -| | completeExchangeBatch | createGroup | addOffersToGroup | removeOffersFromGroup | createBundle | createBundle | createOfferBatch | voidOfferBatch | extendOfferBatch | withdrawFunds | withdrawProtocolFees | createDisputeResolver | addFeesToDisputeResolver | removeFeesFromDisputeResolver | expireDisputeBatch | createDisputeResolver | addSellersToAllowList | removeSellersFromAllowList | -| 1 | 184357 | 216179 | 166813 | 339950 | 266339 | 266339 | 645410 | 76369 | 63841 | 98246 | 124263 | 456727 | 168014 | 99325 | 223643 | 720355 | 120611 | 76720 | -| 2 | 237945 | 292824 | 243899 | 362917 | 324373 | 324237 | 1220883 | 111203 | 85650 | 144880 | 193424 | 552740 | 264455 | 145349 | 322061 | 769463 | 169117 | 99602 | -| 3 | 290692 | 369287 | 319811 | 385956 | 382685 | 382414 | 1796358 | 145208 | 107442 | 191521 | 262643 | 649449 | 360736 | 191089 | 420849 | 817790 | 217760 | 122522 | -| 4 | 344271 | 446010 | 396802 | 409687 | 440789 | 440369 | 2371848 | 179853 | 129223 | 238345 | 330958 | 745985 | 457657 | 236552 | 519034 | 866852 | 266659 | 145744 | -| 5 | 397506 | 522438 | 473499 | 432743 | 498656 | 498102 | 2947330 | 213982 | 150777 | 284285 | 399900 | 842313 | 553655 | 282583 | 617217 | 915916 | 315181 | 168453 | -| 6 | 450933 | 599145 | 549901 | 455724 | 556830 | 556142 | 3522863 | 248993 | 172476 | 330931 | 468759 | 939354 | 650340 | 327624 | 716072 | 964978 | 364020 | 191603 | -| 7 | 504557 | 675705 | 626011 | 478855 | 615481 | 614658 | 4098400 | 282740 | 194322 | 377398 | 538021 | 1038389 | 746851 | 373526 | 814350 | 1015980 | 412672 | 214102 | -| 8 | 557350 | 752116 | 703168 | 502121 | 673490 | 672532 | 4673951 | 317493 | 216313 | 423687 | 606770 | 1135642 | 843190 | 418787 | 912629 | 1065136 | 462015 | 237182 | -| 9 | 610704 | 829174 | 779581 | 525383 | 731386 | 730295 | 5289900 | 351745 | 237643 | 470697 | 675733 | 1232882 | 940232 | 464621 | 1013746 | 1114293 | 510385 | 260262 | -| 10 | 664157 | 905366 | 855847 | 548648 | 789928 | 788700 | 5869922 | 386681 | 259853 | 516720 | 744561 | 1325047 | 1039257 | 510454 | 1112302 | 1163448 | 559101 | 283342 | -| 20 | 1199674 | 1672031 | 1622868 | 780097 | 1371424 | 1368854 | 11585194 | 730484 | 476329 | 981216 | 1435505 | 2293894 | 2003996 | 966566 | 2097899 | 1648704 | 1050125 | 511799 | -| 30 | 1731595 | 2440464 | 2391564 | 1012387 | 1955241 | 1951326 | 17350904 | 1078743 | 693892 | 1446838 | 2126474 | 3256561 | 2967075 | 1424971 | 3083565 | 2138411 | 1535807 | 740419 | -| 40 | 2271836 | 3202792 | 3154246 | 1244680 | 2539116 | 2533856 | 23112733 | 1423827 | 911454 | 1913450 | 2817507 | 4248050 | 3934100 | 1882668 | 4069301 | 2628126 | 2025507 | 969085 | -| 50 | 2800719 | 3969835 | 3921550 | 1476974 | 3117065 | 3110472 | 28911628 | 1768963 | 1135500 | 2380130 | 3503226 | 5220856 | 4929416 | 2340381 | 5055106 | 3111881 | 2515218 | 1199241 | -| 60 | 3333793 | 4764194 | 4715895 | 1542105 | 3699938 | 3692002 | | 2114152 | 1353506 | 2841377 | 3983943 | 6192222 | 5902160 | 2409313 | 6040982 | 3600680 | 2999184 | 1220202 | -| 70 | 3866927 | 5535734 | 5487697 | 1607235 | 4307529 | 4298195 | | 2459395 | 1566348 | 3307259 | 4471569 | 7137316 | 6853941 | 2478233 | 7030675 | 4113038 | 3487977 | 1241164 | -| 80 | 4424168 | 6302543 | 6256137 | 1672369 | 4893873 | 4883188 | | 2804690 | 1783781 | 3773180 | 4953126 | 8125585 | 7799011 | 2547183 | 8046950 | 4604672 | 3976781 | 1262132 | -| 90 | 4960338 | 7052123 | 7005985 | 1737566 | 5480275 | 5468239 | | 3144491 | 2001293 | 4263556 | 5434728 | 9068993 | 8785898 | 2616114 | 9014128 | 5096317 | 4491310 | 1283105 | -| 100 | 5496569 | 7801814 | 7755932 | 1802826 | 6066735 | 6053349 | | 3489284 | 2218882 | 4732249 | 5916387 | 10051836 | 9729258 | 2685059 | 9977691 | 5587972 | 4982950 | 1304081 | -| **max** | **557** | **388** | **389** | **1733** | **508** | **510** | **51** | **868** | **1375** | **641** | **491** | **305** | **309** | **932** | **302** | **597** | **610** | **1906** | -| safe | 333 | 232 | 232 | 1031 | 303 | 304 | 31 | 520 | 824 | 384 | 293 | 181 | 185 | 557 | 181 | 352 | 365 | 1141 | - -## Methodology - -As seen from the results above, some limits are relatively high and to actually hit the limit, already the setup would take a lot of time (e.g. for making `>1700` offers to hit limit in `removeOffersFromGroup`). To get the estimates we therefore use the following approach: -- get the actual estimates for relatively small number of different array lengths -- given how gas is determined, there exist approximate linear relation, which can be written as `gasSpent = intrinsicGas + arrayLength*costPerLoop`. Intrinsic costs here contains all costs that are fixed regardless of the array size. -- use linear regression to estimate `intrinsicGas` and `costPerLoop` -- use these estimates to calculate the biggest `arrayLength` where `gasSpent <= blockGasLimit` which gives the maximum value. To get the safe value we find the biggest `arrayLength` where `gasSpent <= safeGasLimitPercent*blockGasLimit` diff --git a/scripts/config/revert-reasons.js b/scripts/config/revert-reasons.js index 4e3b70eda..496ba14f3 100644 --- a/scripts/config/revert-reasons.js +++ b/scripts/config/revert-reasons.js @@ -54,7 +54,6 @@ exports.RevertReasons = { // Group related NO_SUCH_GROUP: "No such group", OFFER_NOT_IN_GROUP: "Offer not part of the group", - TOO_MANY_OFFERS: "Exceeded maximum offers in a single transaction", NOTHING_UPDATED: "Nothing updated", INVALID_CONDITION_PARAMETERS: "Invalid condition parameters", @@ -76,8 +75,7 @@ exports.RevertReasons = { NOT_DISPUTE_RESOLVER_ASSISTANT: "Not dispute resolver's assistant address", NO_SUCH_DISPUTE_RESOLVER: "No such dispute resolver", INVALID_ESCALATION_PERIOD: "Invalid escalation period", - INVALID_AMOUNT_DISPUTE_RESOLVER_FEES: - "Dispute resolver fees are not present or exceed maximum dispute resolver fees in a single transaction", + INEXISTENT_DISPUTE_RESOLVER_FEES: "Dispute resolver fees are not present", DUPLICATE_DISPUTE_RESOLVER_FEES: "Duplicate dispute resolver fee", DISPUTE_RESOLVER_FEE_NOT_FOUND: "Dispute resolver fee not found", FEE_AMOUNT_NOT_YET_SUPPORTED: "Non-zero dispute resolver fees not yet supported", @@ -86,8 +84,7 @@ exports.RevertReasons = { AUTH_TOKEN_MUST_BE_UNIQUE: "Auth token cannot be assigned to another entity of the same type", SELLER_ALREADY_APPROVED: "Seller id is approved already", SELLER_NOT_APPROVED: "Seller id is not approved", - INVALID_AMOUNT_ALLOWED_SELLERS: - "Allowed sellers are not present or exceed maximum allowed sellers in a single transaction", + INEXISTENT_ALLOWED_SELLERS_LIST: "Allowed sellers are not present", INVALID_AGENT_FEE_PERCENTAGE: "Sum of agent fee percentage and protocol fee percentage should be <= max fee percentage limit", NO_PENDING_UPDATE_FOR_ACCOUNT: "No pending updates for the given account", @@ -109,7 +106,6 @@ exports.RevertReasons = { NO_SUCH_BUNDLE: "No such bundle", TWIN_NOT_IN_BUNDLE: "Twin not part of the bundle", OFFER_NOT_IN_BUNDLE: "Offer not part of the bundle", - TOO_MANY_TWINS: "Exceeded maximum twins in a single transaction", BUNDLE_OFFER_MUST_BE_UNIQUE: "Offer must be unique to a bundle", BUNDLE_TWIN_MUST_BE_UNIQUE: "Twin must be unique to a bundle", INSUFFICIENT_TWIN_SUPPLY_TO_COVER_BUNDLE_OFFERS: @@ -124,7 +120,6 @@ exports.RevertReasons = { VOUCHER_EXTENSION_NOT_VALID: "Proposed date is not later than the current one", VOUCHER_STILL_VALID: "Voucher still valid", VOUCHER_HAS_EXPIRED: "Voucher has expired", - TOO_MANY_EXCHANGES: "Exceeded maximum exchanges in a single transaction", EXCHANGE_IS_NOT_IN_A_FINAL_STATE: "Exchange is not in a final state", INVALID_RANGE_LENGTH: "Range length is too large or zero", EXCHANGE_ALREADY_EXISTS: "Exchange already exists", @@ -136,7 +131,6 @@ exports.RevertReasons = { INVALID_RANGE_START: "Range start too low", INVALID_AMOUNT_TO_MINT: "Amount to mint is greater than remaining un-minted in range", NO_SILENT_MINT_ALLOWED: "Only owner's mappings can be updated without event", - TOO_MANY_TO_MINT: "Exceeded maximum amount to mint in a single transaction", OFFER_EXPIRED_OR_VOIDED: "Offer expired or voided", OFFER_STILL_VALID: "Offer still valid", NOTHING_TO_BURN: "Nothing to burn", @@ -151,7 +145,6 @@ exports.RevertReasons = { INSUFFICIENT_VALUE_RECEIVED: "Insufficient value received", INSUFFICIENT_AVAILABLE_FUNDS: "Insufficient available funds", NATIVE_NOT_ALLOWED: "Transfer of native currency not allowed", - TOO_MANY_TOKENS: "Too many tokens", TOKEN_AMOUNT_MISMATCH: "Number of amounts should match number of tokens", NOTHING_TO_WITHDRAW: "Nothing to withdraw", NOT_AUTHORIZED: "Not authorized to withdraw", @@ -184,7 +177,6 @@ exports.RevertReasons = { INVALID_BUYER_PERCENT: "Invalid buyer percent", DISPUTE_STILL_VALID: "Dispute still valid", INVALID_DISPUTE_TIMEOUT: "Invalid dispute timeout", - TOO_MANY_DISPUTES: "Exceeded maximum disputes in a single transaction", ESCALATION_NOT_ALLOWED: "Disputes without dispute resolver cannot be escalated", // Config related diff --git a/test/protocol/BundleHandlerTest.js b/test/protocol/BundleHandlerTest.js index 5fab960e5..4e60fae92 100644 --- a/test/protocol/BundleHandlerTest.js +++ b/test/protocol/BundleHandlerTest.js @@ -485,16 +485,6 @@ describe("IBosonBundleHandler", function () { ); }); - it("Adding too many offers", async function () { - // Try to add the more than 100 offers - bundle.offerIds = [...Array(101).keys()]; - - // Attempt to create a bundle, expecting revert - await expect(bundleHandler.connect(assistant).createBundle(bundle)).to.revertedWith( - RevertReasons.TOO_MANY_OFFERS - ); - }); - it("Twin is duplicated", async function () { // Try to add the same twin twice bundle.twinIds = ["1", "1", "4"]; @@ -505,16 +495,6 @@ describe("IBosonBundleHandler", function () { ); }); - it("Adding too many twins", async function () { - // Try to add the more than 100 twins - bundle.twinIds = [...Array(101).keys()]; - - // Attempt to create a bundle, expecting revert - await expect(bundleHandler.connect(assistant).createBundle(bundle)).to.revertedWith( - RevertReasons.TOO_MANY_TWINS - ); - }); - it("Exchange already exists for the offerId in bundle", async function () { // Deposit seller funds so the commit will succeed await fundsHandler diff --git a/test/protocol/ConfigHandlerTest.js b/test/protocol/ConfigHandlerTest.js index 5b4a44b0b..4d3db493f 100644 --- a/test/protocol/ConfigHandlerTest.js +++ b/test/protocol/ConfigHandlerTest.js @@ -891,10 +891,6 @@ describe("IBosonConfigHandler", function () { maxEscalationResponsePeriod, "Invalid max escalatio response period" ); - expect(await configHandler.connect(rando).getMaxAllowedSellers()).to.equal( - maxAllowedSellers, - "Invalid max allowed sellers" - ); expect(await configHandler.connect(rando).getBuyerEscalationDepositPercentage()).to.equal( buyerEscalationDepositPercentage, "Invalid buyer escalation deposit" diff --git a/test/protocol/DisputeHandlerTest.js b/test/protocol/DisputeHandlerTest.js index 4e6dac311..999f8ba96 100644 --- a/test/protocol/DisputeHandlerTest.js +++ b/test/protocol/DisputeHandlerTest.js @@ -2508,16 +2508,6 @@ describe("IBosonDisputeHandler", function () { RevertReasons.INVALID_STATE ); }); - - it("Expiring too many disputes", async function () { - // Try to expire the more than 100 disputes - disputesToExpire = [...Array(101).keys()]; - - // Attempt to expire the disputes, expecting revert - await expect(disputeHandler.connect(rando).expireDisputeBatch(disputesToExpire)).to.revertedWith( - RevertReasons.TOO_MANY_DISPUTES - ); - }); }); }); }); diff --git a/test/protocol/DisputeResolverHandlerTest.js b/test/protocol/DisputeResolverHandlerTest.js index 40e25a6f5..ce99cf0f8 100644 --- a/test/protocol/DisputeResolverHandlerTest.js +++ b/test/protocol/DisputeResolverHandlerTest.js @@ -501,15 +501,6 @@ describe("DisputeResolverHandler", function () { ).to.revertedWith(RevertReasons.DISPUTE_RESOLVER_ADDRESS_MUST_BE_UNIQUE); }); - it("DisputeResolverFees above max", async function () { - await configHandler.setMaxFeesPerDisputeResolver(2); - - // Attempt to Create a DisputeResolver, expecting revert - await expect( - accountHandler.connect(admin).createDisputeResolver(disputeResolver, disputeResolverFees, sellerAllowList) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_DISPUTE_RESOLVER_FEES); - }); - it("EscalationResponsePeriod is invalid", async function () { await configHandler.setMaxEscalationResponsePeriod(oneWeek); @@ -541,15 +532,6 @@ describe("DisputeResolverHandler", function () { ).to.revertedWith(RevertReasons.DUPLICATE_DISPUTE_RESOLVER_FEES); }); - it("Number of seller ids above max", async function () { - sellerAllowList = new Array(101).fill("1"); - - // Attempt to Create a DisputeResolver, expecting revert - await expect( - accountHandler.connect(admin).createDisputeResolver(disputeResolver, disputeResolverFees, sellerAllowList) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_ALLOWED_SELLERS); - }); - it("Duplicate dispute resolver fees", async function () { //Create new sellerAllowList array sellerAllowList = ["3", "2", "8"]; @@ -1481,16 +1463,7 @@ describe("DisputeResolverHandler", function () { // Attempt to add fees to the dispute resolver, expecting revert await expect( accountHandler.connect(admin).addFeesToDisputeResolver(disputeResolver.id, disputeResolverFees) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_DISPUTE_RESOLVER_FEES); - }); - - it("DisputeResolverFees above max", async function () { - await configHandler.setMaxFeesPerDisputeResolver(2); - - // Attempt to add fees to the dispute resolver, expecting revert - await expect( - accountHandler.connect(admin).addFeesToDisputeResolver(disputeResolver.id, disputeResolverFees) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_DISPUTE_RESOLVER_FEES); + ).to.revertedWith(RevertReasons.INEXISTENT_DISPUTE_RESOLVER_FEES); }); it("Duplicate dispute resolver fees", async function () { @@ -1731,16 +1704,7 @@ describe("DisputeResolverHandler", function () { // Attempt to remove fees from the dispute resolver, expecting revert await expect( accountHandler.connect(admin).removeFeesFromDisputeResolver(disputeResolver.id, feeTokenAddressesToRemove) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_DISPUTE_RESOLVER_FEES); - }); - - it("DisputeResolverFees above max", async function () { - await configHandler.setMaxFeesPerDisputeResolver(2); - - // Attempt to remove fees from the dispute resolver, expecting revert - await expect( - accountHandler.connect(admin).removeFeesFromDisputeResolver(disputeResolver.id, feeTokenAddressesToRemove) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_DISPUTE_RESOLVER_FEES); + ).to.revertedWith(RevertReasons.INEXISTENT_DISPUTE_RESOLVER_FEES); }); it("DisputeResolverFee in array does not exist for Dispute Resolver", async function () { @@ -1866,16 +1830,7 @@ describe("DisputeResolverHandler", function () { // Attempt to add sellers to the allow list, expecting revert await expect( accountHandler.connect(admin).addSellersToAllowList(disputeResolver.id, allowedSellersToAdd) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_ALLOWED_SELLERS); - }); - - it("SellerAllowList above max", async function () { - allowedSellersToAdd = new Array(101).fill("1"); - - // Attempt to add sellers to the allow list, expecting revert - await expect( - accountHandler.connect(admin).addSellersToAllowList(disputeResolver.id, allowedSellersToAdd) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_ALLOWED_SELLERS); + ).to.revertedWith(RevertReasons.INEXISTENT_ALLOWED_SELLERS_LIST); }); it("Some seller does not exist", async function () { @@ -2070,16 +2025,7 @@ describe("DisputeResolverHandler", function () { // Attempt to remove sellers from the allowed list, expecting revert await expect( accountHandler.connect(admin).removeSellersFromAllowList(disputeResolver.id, allowedSellersToRemove) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_ALLOWED_SELLERS); - }); - - it("SellerAllowList above max", async function () { - allowedSellersToRemove = new Array(101).fill("1"); - - // Attempt to remove sellers from the allowed list, expecting revert - await expect( - accountHandler.connect(admin).removeSellersFromAllowList(disputeResolver.id, allowedSellersToRemove) - ).to.revertedWith(RevertReasons.INVALID_AMOUNT_ALLOWED_SELLERS); + ).to.revertedWith(RevertReasons.INEXISTENT_ALLOWED_SELLERS_LIST); }); it("Seller id is not approved", async function () { diff --git a/test/protocol/ExchangeHandlerTest.js b/test/protocol/ExchangeHandlerTest.js index ccf8e0b8d..177083b35 100644 --- a/test/protocol/ExchangeHandlerTest.js +++ b/test/protocol/ExchangeHandlerTest.js @@ -1624,16 +1624,6 @@ describe("IBosonExchangeHandler", function () { ); }); - it("Completing too many exchanges", async function () { - // Try to complete more than 100 exchanges - exchangesToComplete = [...Array(101).keys()]; - - // Attempt to complete the exchange, expecting revert - await expect(exchangeHandler.connect(rando).completeExchangeBatch(exchangesToComplete)).to.revertedWith( - RevertReasons.TOO_MANY_EXCHANGES - ); - }); - it("exchange id is invalid", async function () { // An invalid exchange id exchangeId = "666"; diff --git a/test/protocol/FundsHandlerTest.js b/test/protocol/FundsHandlerTest.js index 9212ed4cf..aa32011b6 100644 --- a/test/protocol/FundsHandlerTest.js +++ b/test/protocol/FundsHandlerTest.js @@ -635,73 +635,6 @@ describe("IBosonFundsHandler", function () { ); }); - it("if user has more different tokens than maximum number allowed to withdraw, only part of it is withdrawn", async function () { - // set maximum tokens per withdraw to 1 - await configHandler.connect(deployer).setMaxTokensPerWithdrawal("1"); - - // Read on chain state - sellersAvailableFunds = FundsList.fromStruct(await fundsHandler.getAvailableFunds(seller.id)); - const treasuryNativeBalanceBefore = await ethers.provider.getBalance(treasury.address); - const treasuryTokenBalanceBefore = await mockToken.balanceOf(treasury.address); - - // Chain state should match the expected available funds before the withdrawal - expectedSellerAvailableFunds = new FundsList([ - new Funds(mockToken.address, "Foreign20", sellerPayoff), - new Funds(ethers.constants.AddressZero, "Native currency", sellerPayoff), - ]); - expect(sellersAvailableFunds).to.eql( - expectedSellerAvailableFunds, - "Seller available funds mismatch before withdrawal" - ); - - // withdraw all funds - await fundsHandler.connect(assistant).withdrawFunds(seller.id, [], []); - - // Read on chain state - sellersAvailableFunds = FundsList.fromStruct(await fundsHandler.getAvailableFunds(seller.id)); - let treasuryNativeBalanceAfter = await ethers.provider.getBalance(treasury.address); - const treasuryTokenBalanceAfter = await mockToken.balanceOf(treasury.address); - - // Chain state should match the expected available funds after the withdrawal - // Funds available should still have the entries from above the threshold - expectedSellerAvailableFunds = new FundsList([ - new Funds(ethers.constants.AddressZero, "Native currency", sellerPayoff), - ]); - expect(sellersAvailableFunds).to.eql( - expectedSellerAvailableFunds, - "Seller available funds mismatch after first withdrawal" - ); - // Token balance is increased for sellerPayoff, while native currency balance remains the same - expect(treasuryNativeBalanceAfter).to.eql( - treasuryNativeBalanceBefore, - "Treasury native currency balance mismatch after first withdrawal" - ); - expect(treasuryTokenBalanceAfter).to.eql( - treasuryTokenBalanceBefore.add(sellerPayoff), - "Treasury token balance mismatch after first withdrawal" - ); - - // withdraw all funds again - await fundsHandler.connect(assistant).withdrawFunds(seller.id, [], []); - - // Read on chain state - sellersAvailableFunds = FundsList.fromStruct(await fundsHandler.getAvailableFunds(seller.id)); - treasuryNativeBalanceAfter = await ethers.provider.getBalance(treasury.address); - - // Chain state should match the expected available funds after the withdrawal - // Funds available should now be an empty list - expectedSellerAvailableFunds = new FundsList([]); - expect(sellersAvailableFunds).to.eql( - expectedSellerAvailableFunds, - "Seller available funds mismatch after second withdrawal" - ); - // Native currency balance is increased for the withdrawAmount - expect(treasuryNativeBalanceAfter).to.eql( - treasuryNativeBalanceBefore.add(sellerPayoff), - "Treasury native currency balance mismatch after second withdrawal" - ); - }); - it("It's possible to withdraw same toke twice if in total enough available funds", async function () { let reduction = ethers.utils.parseUnits("0.1", "ether").toString(); // Withdraw token @@ -881,16 +814,6 @@ describe("IBosonFundsHandler", function () { ).to.revertedWith(RevertReasons.TOKEN_AMOUNT_MISMATCH); }); - it("Caller wants to withdraw more different tokens than allowed", async function () { - tokenList = new Array(101).fill(ethers.constants.AddressZero); - tokenAmounts = new Array(101).fill("1"); - - // Attempt to withdraw the funds, expecting revert - await expect( - fundsHandler.connect(assistant).withdrawFunds(seller.id, tokenList, tokenAmounts) - ).to.revertedWith(RevertReasons.TOO_MANY_TOKENS); - }); - it("Caller tries to withdraw more than they have in the available funds", async function () { // Withdraw token tokenList = [mockToken.address]; @@ -1180,81 +1103,6 @@ describe("IBosonFundsHandler", function () { ); }); - it("if protocol has more different tokens than maximum number allowed to withdraw, only part of it is withdrawn", async function () { - // set maximum tokens per withdraw to 1 - await configHandler.connect(deployer).setMaxTokensPerWithdrawal("1"); - - // Read on chain state - protocolAvailableFunds = FundsList.fromStruct(await fundsHandler.getAvailableFunds(protocolId)); - let protocolTreasuryNativeBalanceBefore = await ethers.provider.getBalance(protocolTreasury.address); - const protocolTreasuryTokenBalanceBefore = await mockToken.balanceOf(protocolTreasury.address); - - // Chain state should match the expected available funds before the withdrawal - expectedProtocolAvailableFunds = new FundsList([ - new Funds(mockToken.address, "Foreign20", protocolPayoff), - new Funds(ethers.constants.AddressZero, "Native currency", protocolPayoff), - ]); - expect(protocolAvailableFunds).to.eql( - expectedProtocolAvailableFunds, - "Protocol available funds mismatch before withdrawal" - ); - - // withdraw all funds - let tx = await fundsHandler.connect(feeCollector).withdrawProtocolFees([], []); - - // calcualte tx costs - txReceipt = await tx.wait(); - txCost = tx.gasPrice.mul(txReceipt.gasUsed); - - // Read on chain state - protocolAvailableFunds = FundsList.fromStruct(await fundsHandler.getAvailableFunds(protocolId)); - let protocolTreasuryNativeBalanceAfter = await ethers.provider.getBalance(protocolTreasury.address); - const protocolTreasuryTokenBalanceAfter = await mockToken.balanceOf(protocolTreasury.address); - - // Chain state should match the expected available funds after the withdrawal - // Funds available should still have the entries from above the threshold - expectedProtocolAvailableFunds = new FundsList([ - new Funds(ethers.constants.AddressZero, "Native currency", protocolPayoff), - ]); - expect(protocolAvailableFunds).to.eql( - expectedProtocolAvailableFunds, - "Protocol available funds mismatch after first withdrawal" - ); - // Token balance is increased for protocolFee, while native currency balance is reduced only for tx costs - expect(protocolTreasuryNativeBalanceAfter).to.eql( - protocolTreasuryNativeBalanceBefore, - "Fee collector native currency balance mismatch after first withdrawal" - ); - expect(protocolTreasuryTokenBalanceAfter).to.eql( - protocolTreasuryTokenBalanceBefore.add(protocolPayoff), - "Fee collector token balance mismatch after first withdrawal" - ); - - // withdraw all funds again - tx = await fundsHandler.connect(feeCollector).withdrawProtocolFees([], []); - - // calcualte tx costs - txReceipt = await tx.wait(); - txCost = tx.gasPrice.mul(txReceipt.gasUsed); - - // Read on chain state - protocolAvailableFunds = FundsList.fromStruct(await fundsHandler.getAvailableFunds(protocolId)); - protocolTreasuryNativeBalanceAfter = await ethers.provider.getBalance(protocolTreasury.address); - - // Chain state should match the expected available funds after the withdrawal - // Funds available should now be an empty list - expectedProtocolAvailableFunds = new FundsList([]); - expect(protocolAvailableFunds).to.eql( - expectedProtocolAvailableFunds, - "Protocol available funds mismatch after second withdrawal" - ); - // Native currency balance is increased for the protocol fee - expect(protocolTreasuryNativeBalanceAfter).to.eql( - protocolTreasuryNativeBalanceBefore.add(offerTokenProtocolFee), - "Fee collector native currency balance mismatch after second withdrawal" - ); - }); - it("It's possible to withdraw same token twice if in total enough available funds", async function () { let reduction = ethers.utils.parseUnits("0.01", "ether").toString(); // Withdraw token @@ -1311,16 +1159,6 @@ describe("IBosonFundsHandler", function () { ).to.revertedWith(RevertReasons.TOKEN_AMOUNT_MISMATCH); }); - it("Caller wants to withdraw more different tokens than allowed", async function () { - tokenList = new Array(101).fill(ethers.constants.AddressZero); - tokenAmounts = new Array(101).fill("1"); - - // Attempt to withdraw the funds, expecting revert - await expect( - fundsHandler.connect(feeCollector).withdrawProtocolFees(tokenList, tokenAmounts) - ).to.revertedWith(RevertReasons.TOO_MANY_TOKENS); - }); - it("Caller tries to withdraw more than they have in the available funds", async function () { // Withdraw token tokenList = [mockToken.address]; diff --git a/test/protocol/GroupHandlerTest.js b/test/protocol/GroupHandlerTest.js index 466c6f017..2a2f50327 100644 --- a/test/protocol/GroupHandlerTest.js +++ b/test/protocol/GroupHandlerTest.js @@ -359,16 +359,6 @@ describe("IBosonGroupHandler", function () { ); }); - it("Adding too many offers", async function () { - // Try to add the more than 100 offers - group.offerIds = [...Array(101).keys()]; - - // Attempt to create a group, expecting revert - await expect(groupHandler.connect(assistant).createGroup(group, condition)).to.revertedWith( - RevertReasons.TOO_MANY_OFFERS - ); - }); - context("Condition 'None' has some values in other fields", async function () { beforeEach(async function () { condition = mockCondition({ method: EvaluationMethod.None, threshold: "0", maxCommits: "0" }); @@ -607,26 +597,6 @@ describe("IBosonGroupHandler", function () { ); }); - it("Number of offers to add exceeds max", async function () { - // Try to add the more than 100 offers - offerIdsToAdd = [...Array(101).keys()]; - - // Attempt to add offers to a group, expecting revert - await expect(groupHandler.connect(assistant).addOffersToGroup(group.id, offerIdsToAdd)).to.revertedWith( - RevertReasons.TOO_MANY_OFFERS - ); - }); - - it("Current number of offers plus number of offers to add exceeds max", async function () { - // Try to add offers to that total is more than 100. Group currently has 3. - offerIdsToAdd = [...Array(98).keys()]; - - // Attempt to add offers to a group, expecting revert - await expect(groupHandler.connect(assistant).addOffersToGroup(group.id, offerIdsToAdd)).to.revertedWith( - RevertReasons.TOO_MANY_OFFERS - ); - }); - it("Adding nothing", async function () { // Try to add nothing offerIdsToAdd = []; @@ -801,16 +771,6 @@ describe("IBosonGroupHandler", function () { ).to.revertedWith(RevertReasons.OFFER_NOT_IN_GROUP); }); - it("Removing too many offers", async function () { - // Try to remove the more than 100 offers - offerIdsToRemove = [...Array(101).keys()]; - - // Attempt to remove offers from the group, expecting revert - await expect( - groupHandler.connect(assistant).removeOffersFromGroup(group.id, offerIdsToRemove) - ).to.revertedWith(RevertReasons.TOO_MANY_OFFERS); - }); - it("Removing nothing", async function () { // Try to remove nothing offerIdsToRemove = []; diff --git a/test/protocol/OfferHandlerTest.js b/test/protocol/OfferHandlerTest.js index f182e0441..f5891ea10 100644 --- a/test/protocol/OfferHandlerTest.js +++ b/test/protocol/OfferHandlerTest.js @@ -2219,20 +2219,6 @@ describe("IBosonOfferHandler", function () { ).to.revertedWith(RevertReasons.OFFER_MUST_BE_ACTIVE); }); - it("Creating too many offers", async function () { - const gasLimit = 10000000; - - // Try to create the more than 100 offers - offers = new Array(101).fill(offer); - - // Attempt to create the offers, expecting revert - await expect( - offerHandler - .connect(assistant) - .createOfferBatch(offers, offerDatesList, offerDurationsList, disputeResolverIds, agentIds, { gasLimit }) - ).to.revertedWith(RevertReasons.TOO_MANY_OFFERS); - }); - it("Dispute valid duration is 0 for some offer", async function () { // Set dispute valid duration to 0 offerDurationsList[2].resolutionPeriod = "0"; @@ -2821,16 +2807,6 @@ describe("IBosonOfferHandler", function () { RevertReasons.OFFER_HAS_BEEN_VOIDED ); }); - - it("Voiding too many offers", async function () { - // Try to void the more than 100 offers - offersToVoid = [...Array(101).keys()]; - - // Attempt to void the offers, expecting revert - await expect(offerHandler.connect(assistant).voidOfferBatch(offersToVoid)).to.revertedWith( - RevertReasons.TOO_MANY_OFFERS - ); - }); }); }); @@ -2989,16 +2965,6 @@ describe("IBosonOfferHandler", function () { offerHandler.connect(assistant).extendOfferBatch(offersToExtend, newValidUntilDate) ).to.revertedWith(RevertReasons.OFFER_PERIOD_INVALID); }); - - it("Extending too many offers", async function () { - // Try to extend the more than 100 offers - offersToExtend = [...Array(101).keys()]; - - // Attempt to extend the offers, expecting revert - await expect( - offerHandler.connect(assistant).extendOfferBatch(offersToExtend, newValidUntilDate) - ).to.revertedWith(RevertReasons.TOO_MANY_OFFERS); - }); }); }); }); diff --git a/test/protocol/ProtocolInitializationHandlerTest.js b/test/protocol/ProtocolInitializationHandlerTest.js index 16ba4cb19..1b224ea58 100644 --- a/test/protocol/ProtocolInitializationHandlerTest.js +++ b/test/protocol/ProtocolInitializationHandlerTest.js @@ -457,20 +457,6 @@ describe("ProtocolInitializationHandler", async function () { ); }); - it("Should emit MaxPremintedVouchersChanged event", async function () { - // Make the cut, check the event - await expect( - diamondCutFacet.diamondCut( - [facetCut], - deployedProtocolInitializationHandlerFacet.address, - calldataProtocolInitialization, - await getFees(maxPriorityFeePerGas) - ) - ) - .to.emit(configHandler, "MaxPremintedVouchersChanged") - .withArgs(maxPremintedVouchers, deployer.address); - }); - it("Should update state", async function () { // Make the cut, check the event await diamondCutFacet.diamondCut( diff --git a/test/protocol/clients/BosonVoucherTest.js b/test/protocol/clients/BosonVoucherTest.js index b4b9e9894..08abb0717 100644 --- a/test/protocol/clients/BosonVoucherTest.js +++ b/test/protocol/clients/BosonVoucherTest.js @@ -1,4 +1,5 @@ const { ethers } = require("hardhat"); +const { assert, expect } = require("chai"); const DisputeResolutionTerms = require("../../../scripts/domain/DisputeResolutionTerms"); const { getInterfaceIds } = require("../../../scripts/config/supported-interfaces.js"); @@ -7,9 +8,6 @@ const { DisputeResolverFee } = require("../../../scripts/domain/DisputeResolverF const Range = require("../../../scripts/domain/Range"); const VoucherInitValues = require("../../../scripts/domain/VoucherInitValues"); const { Funds, FundsList } = require("../../../scripts/domain/Funds"); - -const { mockOffer, mockExchange, mockVoucher } = require("../../util/mock.js"); -const { assert, expect } = require("chai"); const { RevertReasons } = require("../../../scripts/config/revert-reasons"); const { mockDisputeResolver, @@ -18,6 +16,9 @@ const { mockAuthToken, mockBuyer, accountId, + mockVoucher, + mockExchange, + mockOffer, } = require("../../util/mock"); const { applyPercentage, @@ -275,7 +276,6 @@ describe("IBosonVoucher", function () { const mockProtocol = await deployMockProtocol(); const { offer, offerDates, offerDurations, offerFees } = await mockOffer(); const disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns("1000"); await mockProtocol.mock.getOffer.returns( true, offer, @@ -314,7 +314,7 @@ describe("IBosonVoucher", function () { const mockProtocol = await deployMockProtocol(); const { offer, offerDates, offerDurations, offerFees } = await mockOffer(); const disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns("1000"); + await mockProtocol.mock.getOffer.returns( true, offer, @@ -398,7 +398,6 @@ describe("IBosonVoucher", function () { mockProtocol = await deployMockProtocol(); ({ offer, offerDates, offerDurations, offerFees } = await mockOffer()); disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns("1000"); await mockProtocol.mock.getOffer.returns( true, offer, @@ -601,18 +600,6 @@ describe("IBosonVoucher", function () { ); }); - it("Too many to mint in a single transaction", async function () { - await mockProtocol.mock.getMaxPremintedVouchers.returns("100"); - - // Set invalid amount - amount = "101"; - - // Try to premint, it should fail - await expect(bosonVoucher.connect(assistant).preMint(offerId, amount)).to.be.revertedWith( - RevertReasons.TOO_MANY_TO_MINT - ); - }); - it("Offer already expired", async function () { // Skip to after offer expiration await setNextBlockTimestamp(ethers.BigNumber.from(offerDates.validUntil).add(1).toHexString()); @@ -647,16 +634,13 @@ describe("IBosonVoucher", function () { let offerId, start, length, amount; let mockProtocol; let offer, offerDates, offerDurations, offerFees, disputeResolutionTerms; - let maxPremintedVouchers; beforeEach(async function () { offerId = "5"; - maxPremintedVouchers = "10"; mockProtocol = await deployMockProtocol(); ({ offer, offerDates, offerDurations, offerFees } = await mockOffer()); disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns(maxPremintedVouchers); await mockProtocol.mock.getOffer .withArgs(offerId) .returns(true, offer, offerDates, offerDurations, disputeResolutionTerms, offerFees); @@ -790,7 +774,7 @@ describe("IBosonVoucher", function () { }); }); - it("Should burn all vouchers if there is less than MaxPremintedVouchers to burn", async function () { + it("Should burn all vouchers", async function () { // Burn tokens, test for event let tx = await bosonVoucher.connect(assistant).burnPremintedVouchers(offerId); @@ -810,64 +794,6 @@ describe("IBosonVoucher", function () { ); }); - it("Should burn only first MaxPremintedVouchers vouchers if there is more than MaxPremintedVouchers to burn", async function () { - // make offer not voided so premint is possible - offer.voided = false; - await mockProtocol.mock.getOffer - .withArgs(offerId) - .returns(true, offer, offerDates, offerDurations, disputeResolutionTerms, offerFees); - - // Mint another 10 vouchers, so that there are 15 in total - await bosonVoucher.connect(assistant).preMint(offerId, 10); - amount = `${Number(amount) + 10}`; - - // "void" the offer - offer.voided = true; - await mockProtocol.mock.getOffer - .withArgs(offerId) - .returns(true, offer, offerDates, offerDurations, disputeResolutionTerms, offerFees); - - // Burn tokens, test for event - let tx = await bosonVoucher.connect(assistant).burnPremintedVouchers(offerId); - - // Number of events emitted should be equal to maxPremintedVouchers - assert.equal((await tx.wait()).events.length, Number(maxPremintedVouchers), "Wrong number of events emitted"); - - // Last burned id should be updated - const tokenIdStart = deriveTokenId(offerId, start); - let lastBurnedId = tokenIdStart.add(maxPremintedVouchers - 1); - let range = new Range(tokenIdStart.toString(), length, amount, lastBurnedId.toString(), assistant.address); - let returnedRange = Range.fromStruct(await bosonVoucher.getRangeByOfferId(offerId)); - assert.equal(returnedRange.toString(), range.toString(), "Range mismatch"); - - // Second call should burn the difference - tx = await bosonVoucher.connect(assistant).burnPremintedVouchers(offerId); - - // Number of events emitted should be equal to amount - assert.equal( - (await tx.wait()).events.length, - Number(amount) - maxPremintedVouchers, - "Wrong number of events emitted" - ); - - // Last burned id should be updated - lastBurnedId = tokenIdStart.add(amount - 1); - range = new Range(tokenIdStart.toString(), length, amount, lastBurnedId.toString(), assistant.address); - returnedRange = Range.fromStruct(await bosonVoucher.getRangeByOfferId(offerId)); - assert.equal(returnedRange.toString(), range.toString(), "Range mismatch"); - - // All burned tokens should not have an owner - for (let i = 0; i < Number(amount); i++) { - let tokenId = tokenIdStart.add(i); - await expect(bosonVoucher.ownerOf(tokenId)).to.be.revertedWith(RevertReasons.ERC721_NON_EXISTENT); - } - - // Second call should revert since there's nothing to burn - await expect(bosonVoucher.connect(assistant).burnPremintedVouchers(offerId)).to.be.revertedWith( - RevertReasons.NOTHING_TO_BURN - ); - }); - it("Should skip all vouchers were already committed", async function () { let committedVouchers = [11, 14].map((tokenId) => deriveTokenId(offerId, tokenId).toString()); @@ -999,7 +925,6 @@ describe("IBosonVoucher", function () { mockProtocol = await deployMockProtocol(); ({ offer, offerDates, offerDurations, offerFees } = await mockOffer()); disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns("1000"); await mockProtocol.mock.getOffer.returns( true, offer, @@ -1033,9 +958,6 @@ describe("IBosonVoucher", function () { }); it("Range is fully minted", async function () { - // Adjust config value - await configHandler.connect(deployer).setMaxPremintedVouchers(length); - // Premint tokens await bosonVoucher.connect(assistant).preMint(offerId, length); @@ -1098,7 +1020,6 @@ describe("IBosonVoucher", function () { const mockProtocol = await deployMockProtocol(); const { offer, offerDates, offerDurations, offerFees } = await mockOffer(); const disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns("1000"); await mockProtocol.mock.getOffer.returns( true, offer, @@ -1168,7 +1089,6 @@ describe("IBosonVoucher", function () { mockProtocol = await deployMockProtocol(); ({ offer, offerDates, offerDurations, offerFees } = await mockOffer()); disputeResolutionTerms = new DisputeResolutionTerms("0", "0", "0", "0"); - await mockProtocol.mock.getMaxPremintedVouchers.returns("1000"); await mockProtocol.mock.getOffer.returns( true, offer, @@ -1228,8 +1148,6 @@ describe("IBosonVoucher", function () { // Add five more ranges // This tests more getPreMintStatus than ownerOf // Might even be put into integration tests - // Adjust config value - await configHandler.connect(deployer).setMaxPremintedVouchers("10000"); let previousOfferId = Number(offerId); let previousStartId = Number(start); let ranges = [new Range(Number(start), length, amount, "0")]; @@ -1895,7 +1813,7 @@ describe("IBosonVoucher", function () { ); expect(disputeResolver.isValid()).is.true; - //Create DisputeResolverFee array so offer creation will succeed + // Create DisputeResolverFee array so offer creation will succeed disputeResolverFees = [new DisputeResolverFee(ethers.constants.AddressZero, "Native", "0")]; // Make empty seller list, so every seller is allowed