> For the complete documentation index, see [llms.txt](https://audits.solidgrp.io/metafish/llms.txt). Markdown versions of documentation pages are available by appending `.md` to page URLs; this page is available as [Markdown](https://audits.solidgrp.io/metafish/audit-results/findings.md).

# Findings

## **Issue 01**

| **Type**           | **Severity** | **Location**    | **Status**                                    |
| ------------------ | ------------ | --------------- | --------------------------------------------- |
| Owner Capabilities | High         | `lock` `unlock` | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

An owner can gain ownership of the contract even if he calls renounceOwnership function.

This can be achieved by performing the following steps:

The owner of the contract can call lock() to lock the contract (the lock function saves the previous owner into a variable)

After the locking period has passed the owner of the contract can call unlock() and regain the ownership.

The owner of the contract can then call the renounceOwnership function. Now, the contract allegedly has no owner (users can verify it by looking for the renounceOwnership transaction and making sure that the owner is set to the zero address)

**Recommendation:**

Our recommendation Is to remove the lock and unlock function if not needed

## Issue 02&#x20;

| **Type**      | **Severity**  | **Location**       | **Status**                                    |
| ------------- | ------------- | ------------------ | --------------------------------------------- |
| Logical Issue | Informational | `swapETHForTokens` | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

The swap transaction will be executed immediately, no need to set time-out.&#x20;

```
 function swapETHForTokens(uint256 amount) private {
        // generate the uniswap pair path of token -> weth
        address[] memory path = new address[](2);
        path[0] = uniswapV2Router.WETH();
        path[1] = address(this);

      // make the swap
        uniswapV2Router.swapExactETHForTokensSupportingFeeOnTransferTokens{value: amount}(
            0, // accept any amount of Tokens
            path,
            deadAddress, // Burn address
            block.timestamp.add(300)
        );
        
        emit SwapETHForTokens(amount, path);
    }
```

## **Issue 03**

| **Type**         | **Severity**  | **Location** | **Status**                                    |
| ---------------- | ------------- | ------------ | --------------------------------------------- |
| Gas Optimization | Informational | `_approve`   | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

approve is being called every transaction on the same tokens and for the same spender (the router). &#x20;

**Recommendation**

In order to reduce gas costs, `approve` could be called once (with max int), and then check if it is needed again using `allowance`.

## **Issue 04**

| **Type**      | **Severity** | **Location** | **Status**                                    |
| ------------- | ------------ | ------------ | --------------------------------------------- |
| Volatile Code | High         | `_transfer`  | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

`_transfer` may call internally to `swapExactTokensForETHSupportingFeeOnTransferTokens` , `swapExactETHForTokensSupportingFeeOnTransferTokensSince` these functions can be called during `_transfer`, it may cause `_transfer` to fail unnecessarily.

**Recommendation**

`_transfer` should always work, and shouldn't fail if  `swapExactTokensForETHSupportingFeeOnTransferTokens` or `addLiquidityETH` or any other router related fails in order to make sure the **token will always be tradable.**

## **Issue 05**&#x20;

| **Type**      | **Severity** | **Location** | **Status**                                      |
| ------------- | ------------ | ------------ | ----------------------------------------------- |
| Best Practice | Medium       | `receive`    | <mark style="color:yellow;">Acknowledged</mark> |

```
receive() external payable {}
```

**Description**

There is a receive function in the contract, which means any address can send BNB to the contract. The problem is that there is no way to recover BNB that were mistakenly sent to the contract.

**Recommendation**

&#x20;In order to prevent the contract from receiving BNB from investors, which will result in a loss of funds, our recommendation is to only accept ETH from “whitelisted” addresses  (for example the router address should be whitelisted).\
The receive function will revert if the address is not whitelisted.

## **Issue 06**

| **Type**           | **Severity** | **Location**                                | **Status**                                    |
| ------------------ | ------------ | ------------------------------------------- | --------------------------------------------- |
| Owner Capabilities | High         | `setLiquidityFeePercent`,`setTaxFeePercent` | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

The owner of the contract can set the liquidity fee and tax fee by calling `setLiquidityFeePercent` and `setTaxFeePercent` to any value he desires. If the owner sets the fee to 100% the token will be untradeable.&#x20;

**Recommendation**

Consider adding an upper limit to the set functions.&#x20;

## Issue 07

| **Type**           | **Severity** | **Location**     | **Status**                                    |
| ------------------ | ------------ | ---------------- | --------------------------------------------- |
| Owner Capabilities | High         | `setMaxTxAmount` | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

The owner of the contract can make the token untradable by calling `setMaxTxAmount` with 0.

**Recommendation**

Consider preventing it in the code by adding a require statement in the set function.

## Issue 08

| **Type**      | **Severity** | **Location**     | **Status**                                    |
| ------------- | ------------ | ---------------- | --------------------------------------------- |
| Logical Issue | High         | `_takeLiquidity` | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

This function is taking fees from the transfer amount. When taking fee from the transfer amount it is best practice to emit events, so investors would know that fees were deducted.&#x20;

**Recommendation**

Add Transfer event when taking fees.

## Issue 09

| **Type**       | **Severity**  | **Location** | **Status**                                    |
| -------------- | ------------- | ------------ | --------------------------------------------- |
| Lack of events | Informational | All          | ✔️ <mark style="color:green;">Resolved</mark> |

**Description**

set functions don't emit events.

**Recommendation**

Consider emitting events when changing the state of the contract.&#x20;