Iskra - Audit for Token (Part 1)

The contract `GameToken` is importing and inheriting the `Ownable2Step` contract, however, the `onlyOwner` modifier is never used in the contract.

The contract `Vesting.sol` is using a solidity version greater than 0.8.0; this version has built-in checks in the compiler preventing overflow/underflow of arithmetic operators. In the function `prepare()`, the varaible `_totalLocked` is computed as follow: ```solidity=88 initialVestingAmount = _amount; initialUnlockedAmount = _initialUnlocked; uint256 _totalLocked = initialVestingAmount - initialUnlockedAmount; ``` meaning that `_totalLocked = _amount - _initialUnlocked`, hence any underflow is prevented by the following check: ```solidity=74 require( _amount >= _initialUnlocked, "Vesting: `_initialUnlocked` is greater than `_amount`" ); ```

When transferring standard ERC20 deflationary tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user stakes 100 deflationary tokens (with a 10% transaction fee) in the `Vesting` contract, only 90 tokens actually arrive in the contract. However, the user can still withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction.

One or more parameters with `memory` data location are never modified in their functions and those functions are never called internally within the contract. Thus, their data location can be changed to `calldata` to avoid the gas consumption copying from calldata to memory. ```solidity=14 function setURI(string memory newuri) public { ``` `setURI` has memory location parameters: `newuri`. --- ```solidity=18 function mint( ``` `mint` has memory location parameters: `data`. --- ```solidity=27 function mintBatch( ``` `mintBatch` has memory location parameters: `ids`, `values`, `data`. --- ```solidity=44 function burnBatch( ``` `burnBatch` has memory location parameters: `ids`, `values`. --- ```solidity=60 function setURI(uint256 tokenId, string memory tokenURI_) public onlyOwner { ``` `setURI` has memory location parameters: `tokenURI_`. --- ```solidity=64 function setBaseURI(string memory baseURI) public onlyOwner { ``` `setBaseURI` has memory location parameters: `baseURI`. --- ```solidity=121 function mint( ``` `mint` has memory location parameters: `data`. --- ```solidity=130 function mintBatch( ``` `mintBatch` has memory location parameters: `ids`, `amounts`, `data`. --- ```solidity=115 function setTokenURI(uint256 tokenId, string memory _tokenURI) ``` `setTokenURI` has memory location parameters: `_tokenURI`.

In the contract `Vesting.sol`, the function `revoke()` should have a check preventing `_reclaimer == address(this)`. \ If the `_reclaimer` parameter is mistakenly set as the address of the contract, all tokens will stay in `Vesting`, and since after the transfer the `VestingStatus` is set as `REVOKED` it will be impossible to retrieve the tokens.

Addresses should be checked before assignment or external call to make sure they are not zero addresses. ```solidity=187 beneficiary = _newBeneficiary; ``` - `_newBeneficiary` is not zero-checked before being used.

The return value of the transfer()/transferFrom() call is not checked. ```solidity=93 token.transferFrom( _distributor, address(this), initialVestingAmount * 10**18 ); ``` --- ```solidity=146 token.transfer(_reclaimer, _amount); ``` --- ```solidity=169 token.transfer(beneficiary, _amount * 10**18); ```

In the contract `Vesting.sol`, the function `prepare()` performs the following checks: ```solidity=71 require(_duration > 0, "Vesting: `_duration` is 0"); // otherwise, claimer can claim all amount after first unlock period require(_amount > 0, "Vesting: `_amount` is 0"); ``` ```solidity=79 require( _amount >= _duration, "Vesting: _amount must be greater than _duration" ); ``` Since `_duration > 0` is enforced by the first **require** statement and `_amount >= _duration` is enforced by the third one, the `_amount > 0` is implied and: ```solidity=74 require(_amount > 0, "Vesting: `_amount` is 0"); ``` is redundant.

One or more logic contracts do not protect their initializers. An attacker can call the initializer and assume ownership of the logic contract, whereby she can perform privileged operations that trick unsuspecting users into believing that she is the owner of the upgradeable contract. ```solidity=10 contract Vesting is OwnableUpgradeable { ``` - `Vesting` is an upgradeable contract that does not protect its initializer. ```solidity=47 function initialize() public initializer { ``` - `initialize` is an unprotected initializer function.