Affyn Land

In `PrivateLandSale()`, after a private land is minted to `target` address the value stored in `soldLands[]` is not updated to `true` allowing the same `hexID` to be reused.

Boolean constants can be used directly and do not need to be compared to true or false. ```solidity=2861 require(soldLands[hexID] == false, "Selected HexID is already sold"); ``` --- ```solidity=2877 require(soldLands[hexID] == false, "Selected HexID is already sold"); ``` --- ```solidity=2898 require(soldLands[hexIDs[i]] == false, "One of selected HexIDs is already sold"); ```

There should always be events emitted in the sensitive functions that are controlled by centralization roles. The functions in which it is necessary to emit events are given in the finding locations. --- Also, in the `ModifyReleaseTime()` function, the name of the emitted event `DepositedRevoked` is not relevant to the purpose of the function. It is intended to change the timelock release time value and not to revoke the user's deposits.

The function named `setWhitelistMerkleRoots()` is used to change the value of `whitelistedMerkleRoot` and only a single root is passed as input.

The functions which are never called internally within the contract should have external visibility.

**require** can be used to check for conditions and throw an exception if the condition is not met. It is better to provide a string message containing details about the error that will be passed back to the caller.

The contracts `AccessPassContract.sol` and `LandAndFlagContract.sol` are missing a SPDX license identifier.

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=728 _beneficiary = beneficiary_; ``` - `beneficiary_` is not zero-checked before being used. --- ```solidity=730 _admin = admin; ``` - `admin` is not zero-checked before being used. --- ```solidity=2650 teamAddr = payable(_teamAddr); ``` - `_teamAddr` is not zero-checked before being used.

Within the codebase the word `category` is used as input to different functions in order to access mappings or emit events. The same word is used to indicate both the different categories of lands and flags. In particular, the possible flag categories are 0,1,2 or 3; therefore LAND_FLAG_COST[category] has values only for category values in the range [0;3]. Assume, however, the following scenario: - The `CreateNewCitySale()` function is called for the first time, passing as input appropriate arrays with length = 5. Therefore merkleRoots.length = overallLandQuota.length = presaleLandQuotas.length = 5. - The 3 require statements placed at lines 2944, 2945, 2946 succeed and the value of `currentCitySaleIndex` is incremented to 1 - In the 5 iterations of each for loop, calls to the functions `SetHexRarityMerkleRoot()`, `SetCitySaleQuota()`, `SetCityPreSaleQuota()` occur by passing as the second parameter destined for `category` the value of loop index i. Specifically in the last iteration of the loops the index i is equal to 4. - This means making the following assignments: ```solidity PRESALE_LAND_SALE_LIMITS[1][4] = share; // line 2932 LAND_SALE_LIMITS[1][4] = dimension; // line 2938 saleHexRarity[1][4] = merkleRoot; // line 2973 ``` - Calling the `GetFlagBalanceAndLimits()` function with category = 4 as parameter will result in accessing LAND_FLAG_COST in a position that can never be set. On the other hand, accessing with a category value in the range [0;3] in order to have a valid LAND_FLAG_COST value will result in never accessing all LAND_SALE_LIMITS positions greater than index 3.

State variables that never change should be declared as `constant` to save gas. In `AccessPassContract.sol`: - `LOCKUP_DURATION` - `AAP_PRICE` - `EAP_PRICE` In `LandAndFlagContract.sol`: - `membershipContract` - `COLLECTION_NAME` - `TICKER`