ValiFi - Audit 2

In the contract `Allowlistable` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and add or remove addresses from the `notAllowedList` mapping.  --- In the contract `Seller` the role `admin` has authority over the functions shown in the diagram below. Any compromise to the `admin` account may allow the hacker to take advantage of this authority and change the `admin`, change the `Relayer`, change the swap thresholds, and call the function `run()`. In addition, they can drain the contract by withdrawing all the funds in both ETH and any ERC20 tokens present.  In the contract `Seller` the role `relayer` has authority over the functions shown in the diagram below. Any compromise to the `relayer` account may allow the hacker to take advantage of this authority and call the `run()` function and drain the contract by withdrawing all the funds in both ETH and any ERC20 tokens present.  --- In the contract `TradeRestrictions` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and change the maximum buy and sell values, as well as the time that must elapse between trades.  --- In the contract `ValiFiToken` the role `_owner` has authority over the functions shown in the diagram below. Any compromise to the `_owner` account may allow the hacker to take advantage of this authority and, for example, change the base price values of the token and the maximum range it can have, change the initial liquidity values for BUSD and VALI, change the relayer and treasury addresses, pause the contract, and drain the contract by withdrawing all the funds in ETH, VALI, and any ERC20 tokens present.  Furthermore the contract owner has the following additional privileges: - Buy and sell while the contract is paused - Buy and sell during price stabilization and during the period when the price is out of bounds. Allowing them to sell at any price. - Buy and sell without amount and time limits.

`Allowlistable.sol`, `MinimalForwarderUpgradeable.sol`, `TradeRestrictions.sol` and `ValiFIToken.sol` are upgradeable contracts. The owner can upgrade the contract without the community's commitment. If an attacker compromises the account, they can change the implementation of the contract and drain tokens from the contract. In addition, the attacker can change the implementation of each of the functions so that any user that interacts with the contract could accidentally give approval for the attacker to take tokens from their wallet.

All of the `VALI` tokens are sent to the contract deployer when deploying the contract. This could be a centralization risk as the deployer can distribute `VALI` tokens without obtaining the consensus of the community.

In `Seller.sol` it is not checked if the address `_pair` passed to the constructor is the zero address. --- In `Seller.sol` it is not checked if the address `to` passed to `withdrawEthFromContract()` is the zero address. --- In `Seller.sol` within `setSwapSettings()` there are no upperbound and lowerbound checks on `_thresholdPercent` and `_thresholdDivisor`. --- In `ValiFiToken.sol` it is not checked if the addresses `_relayerWallet` and `_trustedForwarder` passed to `initialize()` are the zero address. --- In `ValiFIToken.sol` within `setRangeAndBasePrice()` there are no upperbound and lowerbound checks on `_range` and `_basePrice`. --- In `ValiFIToken.sol` within `setInitialLiquidity()` there are no upperbound and lowerbound checks on `_initialBusdLiquidity` and `_initialValifiLiquidity`. Furthermore `_initialValifiLiquidity` can't exceed `MAXTOTALSUPPLY` and both `_initialValifiLiquidity` and `_initialBusdLiquidity` should be greater than zero. --- In `Seller.sol` within `setAdmin()` is not checked if the address `_newAdmin` is the zero address. --- In `Seller.sol` within `setRelayer()` is not checked if the address `_newRelayer` is the zero address.

There are scenarios where the stabilization process via `stabilizePrice()` will not function as intended and require team intervention in order for users to buy or sell `ValiFi`. This occurs when `initialValifiLiquidity` or `initialBusdLiquidity` is less than or equal to the value of `reserve0` or `reserve1` (respectively). In that case, `_sellValifi()` or `_buyValifiBack()` will never be executed so that `stabilizePrice()` will not be able to stabilize the price. In addition, if the balances of USDC and ValiFi for the contract ValiFiToken are not large enough to stabilize the price, then it will also require team intervention in order for users to buy or sell `ValiFi` Considering these issues are infrequent and can be resolved with proper actions by the `ValiFi` team, we give this a minor severity.

The contract is serving as the underlying entity to interact with third party AMM protocols and Sentinels. The scope of this audit treats 3rd party entities as black boxes and assumes their functional correctness. However, in the real world, 3rd parties can be compromised and this may lead to lost/stolen assets or services to be interupted. In addition, upgrades of 3rd parties can possibly create severe impacts, such as increasing fees of 3rd parties, migrating to new LP pools, breaking functionality, etc.

The word `stabalizing` is used instead of `stabilizing`. In `Seller.sol`, there is a pragma that is unnecessary. In ValiFIToken.sol, `checkIfBuyBackNeeded` is mention in the comment for `_isPriceOutOfBounds()` but is not a function in the contract.

The return value of the transfer()/transferFrom() call is not checked.

`block.timestamp` is used for comparison in `ValiFIToken.sol`, which can be risky since timestamp can be influenced by miners. That means the miner creating the block can manipulate the `block.timestamp`, to some degree, and change the outcome of the transaction.

In the constructor of `Seller.sol`, the error message should be "Seller: Invalid relayer" instead of "Seller: Invalid pair". --- Inside `Seller.sol` within `setRelayer()`, the error message should be "Admin only" instead of "Admin or relayer only".