# NAFTA
## Info
- Auditor: mohamed moualim
- Commit hash: ...
git repository (commit hash)
## Disclaimers
## Issues
## ***Missing address verification***
## [Low]
**Description**
Certain functions lack a safety check in the address, the address-type arguments should include a zero-address test, otherwise, the contract's functionality may become inaccessible.
**file**
* `contracts/Nafta.sol` (L43:L46,L55:57,L69:103,L111:L138,L365)
**Recommendation**
We recommend that you make sure the addresses provided in the arguments are different from the address(0).
### Owner Can Renounce Ownership
### [Low]
**Description**
Typically, the account that deploys the contract is also its owner. Consequently, the owner is able to engage in certain privileged activities in his own name. In smart contracts, the renounceOwnership function is used to renounce ownership, which means that if the contract's ownership has never been transferred, it will never have an Owner, rendering some owner-exclusive functionality unavailable.
**File(s)**
* `contracts/Nafta.sol` (L12)
**Recommendation**
We recommend that you prevent the owner from calling renounceOwnership without first transferring ownership to a different address. Additionally, if you decide to use a multi-signature wallet, then the execution of the renounceOwnership will require for at least two or more users to be confirmed. Alternatively, you can disable Renounce Ownership functionality by overriding it.
### Race Condition
### [Medium]
**Description**
The _ilpFee _devFee variables have setters. If the user checks the value of one of these variables, then performs a transaction, and the owner updates the fees, the order of the transaction might overturn and the user's transaction in this case will be executed with the new variables without him knowing about it.
**File(s)**
* `contracts/Nafta.sol` (L342)
**Recommendation**
We recommend adding the fees in the arguments of the transfer function and add require statements in order to verify that the values that are provided in the arguments are the same as the ones that are stored in the smart contract.
### Reset Earning before cheking transfer
### [High]
**Description**
The transfer can failed and the user will lost his earning, after reseting the earning the condition require doesn't prevent failure.
**files**
* `contracts/Nafta.sol` (L172)
**Recommendation**
Check if the transfer have been done succesfully if not recover the earning.
function withdrawEarnings() external override {
uint256 transferAmount = earnings[msg.sender];
// Verify that earnings > 0
require(transferAmount > 0, "No earnings to withdraw");
// Reset earnings before transfer
earnings[msg.sender] = 0;
Succed = WETH9.transfer(msg.sender, transferAmount);
if (! Succed){earning[msg.sender] = transferAmount;}
// Push the WETH9 earnings to msg.sender
require(Succed, "WETH9 transfer failed");
// Emit withdraw event
emit WithdrawEarnings(transferAmount, msg.sender);
}
### Floating Pragma
### [Low]
**Description**
The contract makes use of the floating-point pragma 0.8.4. Contracts should be deployed using the same compiler version. Locking the pragma helps ensure that contracts will not unintentionally be deployed using another pragma, which in some cases may be an obsolete version, that may introduce issues to the contract system.
**File(s)**
* `contracts/Nafta.sol` (L1)
**Recommendation**
Consider locking the pragma version. It is advised that floating pragma should not be used in production. Both truffle-config.js and hardhat.config.js support locking the pragma version.
### The feepool are centralized
### [Medium]
**Description**
The owner can change the poolfee in accordance with what the community agreed.
**file**
* `contracts/Nafta.sol` (L342)
**Recommendation**
We recommend that you prevent the owner from calling renounceOwnership without first transferring ownership to a different address. Additionally, if you decide to use a multi-signature wallet, then the execution of the renounceOwnership will require for at least two or more users to be confirmed. Alternatively, you can disable Renounce Ownership functionality by overriding it.