---
# System prepended metadata

title: Security Audit of `Narfex-fiat-factory` contracts.

---

# Security Audit of `Narfex-fiat-factory` contracts. 

## Conclusion

This audit was made by 
Auditor: Vladimir Smelov <vladimirfol@gmail.com>.
Date: 2023-05-04

TODO

## Scope

TODO

## Methodology

1. Blind audit. Understand the structure of the code without reading any docs.
2. Ask questions to developers.
3. Run static analyzers.
4. Find problems with:
- backdoors;
- bugs;
- math;
- potential leaking of funds;
- potential locking of the contract;
- validate arguments and events;
- others.

## Result

#### CRITICAL-1.

At
- NarfexLawyers.sol:123

```solidity=123
        if (getIsLawyer(_account)) return false;
```

should be "if (!getIsLawyer(_account)) return false;"

##### Status.
NEW


______

#### CRITICAL-2.

At
- NarfexP2pRouter.sol:50,63-64

```solidity=50
    function swapToETH(address to, address token, uint amount) external {
```
```solidity=63
        data.outAmount = 0;
        data.outAmountMin = 0;
```

minAmountOut is absolutely requiring otherwise sandwich attack.

##### Status.
NEW


______

#### CRITICAL-3.

At
- NarfexExchangerPool.sol:150

```solidity=150
            0,
```

sandwich attack

##### Status.
NEW


______

#### CRITICAL-4.

At
- NarfexExchangerRouter3.sol:68,257-268,272-280,354-357

```solidity=68
        uint transferFee;
```
```solidity=257
        if (data.isExactOut) {
            /// Increase output amount by outgoing token fee for calculations
            data.outAmount = B.transferFee > 0
                ? data.amount * (PERCENT_PRECISION + B.transferFee) / PERCENT_PRECISION
                : data.amount;
        } else {
            transferInAmount = data.amount;
            /// Decrease input amount for calculations
            data.inAmount = A.transferFee > 0
                ? data.amount * (PERCENT_PRECISION - A.transferFee) / PERCENT_PRECISION
                : data.amount;
        }
```
```solidity=272
        if (data.isExactOut) {
            /// Increase input amount by inbound token fee
            transferInAmount = A.transferFee > 0
                ? data.inAmount * (PERCENT_PRECISION + A.transferFee) / PERCENT_PRECISION
                : data.inAmount;
            require(data.inAmount <= data.inAmountMax, "Input amount is higher than maximum");
        } else {
            require(data.outAmount >= data.outAmountMin, "Output amount is lower than minimum");
        }
```
```solidity=354
                if (C.transferFee > 0) {
                    /// Increasing the output amount offsets the loss from the fee
                    data.outAmount = data.outAmount * (PERCENT_PRECISION + C.transferFee) / PERCENT_PRECISION;
                }
```

for deflationary reflective tokens (which are quite popular), it is not possible to set a clear transferFee, as it varies for each transfer and is generally unpredictable. Has this been examined in the context of transfer-fee tokens? <br /> Firstly, it is essential to consider that the PERCENT_PRECISION might differ among various tokens, for instance, a conceivable token could impose a fee as minute as 0.000000000000000000000001%. <br /> Secondly, a significant issue arises, as the equation x * (1+f) * (1-f) does not equal x; if a token deducts a fee of f% from the increased value of x*(1+f), the final amount sent to the pool would be insufficient for the swap; to address this, the following equation must be solved: y * (DENOMINATOR-f) / DENOMINATOR = x; y = x * DENOMINATOR / (DENOMINATOR-f); it is crucial to note that this is not equivalent to x * (DENOMINATOR+f) / DENOMINATOR, although the values are closely related.

##### Status.
NEW


______

#### CRITICAL-5.

At
- NarfexExchangerRouter3.sol:131,148-149

```solidity=131
        uint uintValue = oracle.getPrice(_token) * uint(_amount) / USDC_PRECISION;
```
```solidity=148
            uint priceA = A.addr == address(USDC) ? USDC_PRECISION : A.price;
            uint priceB = B.addr == address(USDC) ? USDC_PRECISION : B.price;
```

1 IDR = 0.000068 USDC = 6.8 * 1e-5 <br /> If their currency devalues by another 100 times, the oracle would return zero. <br /> It currently returns a price=68, although the exchange rate is 1.00 Indonesian Rupiah = 0.000067823873 US Dollars. <br /> We are already losing 1% in precision, which, in my opinion, is CRITICAL. <br /> The proposal is actually quite straightforward: set the price in the oracle not for 1 IDR but for 10e18 IDR, and then getAmountInUSDC = amount * getPrice / 10e18.

##### Status.
NEW


______

#### CRITICAL-6.

At
- NarfexExchangerRouter3.sol:472-514

```solidity=472
    function _swap(
        SwapData memory data
        ) internal nonReentrant
    {
        require(data.path.length > 1, "Path length must be at least 2 addresses");
        uint lastIndex = data.path.length - 1;

        Token memory A; /// First token
        Token memory B; /// Last token
        {
            /// Get the oracle data for the first and last tokens
            address[] memory sideTokens = new address[](2);
            sideTokens[0] = data.path[0];
            sideTokens[1] = data.path[lastIndex];
            INarfexOracle.TokenData[] memory tokensData = oracle.getTokensData(sideTokens, true);
            A = _assignTokenData(sideTokens[0], tokensData[0]);
            B = _assignTokenData(sideTokens[1], tokensData[1]);
        }
        require(A.addr != B.addr, "Can't swap the same tokens");

        if (A.isFiat && B.isFiat)
        { /// If swap between fiats
            ExchangeData memory exchange = _getExchangeValues(A, B, data.amount, data.isExactOut);
            _swapFiats(data.from, data.to, A, B, exchange);
            return;
        }
        if (!A.isFiat && !B.isFiat)
        { /// Swap on DEX only
            _swapOnlyDEX(data, A, B);
            return;
        }
        if ((A.isFiat && B.addr == address(USDC))
            || (B.isFiat && A.addr == address(USDC)))
        { /// If swap between fiat and USDC in the pool
            ExchangeData memory exchange = _getExchangeValues(A, B, data.amount, data.isExactOut);
            _swapFiatAndUSDC(data, A, B, exchange, false);
            return;
        }

        /// Swap with DEX and Fiats
        data.path = _getDEXSubPath(data.path, A.isFiat);
        _swapFiatWithDEX(data, A.isFiat ? A : B, A.isFiat ? B : A, A.isFiat);
    }
```

I do think it's mandatory to put require for outAmountMin here. Be very strict. Do not believe that you expected pre-calculations will be correct. Do IERC20.balanceOf(to) call before and after the swap and ensure the difference is >= outAmountMin

##### Status.
NEW


______

#### CRITICAL-7.

At
- NarfexExchangerRouter3.sol:516-522

```solidity=516
    /// @notice Set a new pool address
    /// @param _newPoolAddress Another pool address
    /// @param _decimals Pool token decimals
    function setPool(address _newPoolAddress, uint8 _decimals) public onlyOwner {
        pool = INarfexExchangerPool(_newPoolAddress);
        USDC_PRECISION = 10**_decimals;
    }
```

it's not "Pool token decimals" but "USDC decimals". I believe it should be set in the constructor and not changed later. Or if you change USDC decimals then also change USDC address.

##### Status.
NEW


______

#### MAJOR-1.

At
- NarfexLawyers.sol:145-152

```solidity=145
    /// @notice Randomly returns the currently available lawyer
    /// @return Active lawyer address
    function getLawyer() public view returns(address) {
        (address[] memory active, uint length) = getActiveLawyers();
        if (length == 0) return address(0);
        uint index = uint(keccak256(abi.encodePacked(block.timestamp, block.basefee))) % length;
        return active[index];
    }
```

this is not random! It may easily be manipulated via FlashBot attack

##### Status.
NEW


______

#### MAJOR-2.

At
- NarfexP2pRouter.sol:53-55

```solidity=53
        path[0] = token;
        path[1] = address(USDC);
        path[2] = address(WETH);
```

it could be VERY inefficient, some tokens have big pool for token-WETH and small for token-USDC, such swap will lead to huge slippage. Consider removal of this function completely because you already have swap where path is explicitly passed.

##### Status.
NEW


______

#### MAJOR-3.

At
- NarfexP2pRouter.sol:79

```solidity=79
        USDC.transferFrom(msg.sender, address(pool), _usdcAmount);
```

safeTransferFrom

##### Status.
NEW


______

#### MAJOR-4.

At
- NarfexP2pRouter.sol:83,94

```solidity=83
        pool.increaseLimit(msg.sender, _fiatAddress, fiatAmount);
```
```solidity=94
        pool.decreaseLimit(msg.sender, _fiatAddress, _fiatAmount);
```

??? i'm not sure. What if depositer will transfer out his tokens, limits will not be affected right? Unclear place.

##### Status.
NEW


______

#### MAJOR-5.

At
- NarfexP2pRouter.sol:99

```solidity=99
        usdcAmount = usdcAmount * PERCENT_PRECISION / VALIDATOR_POOL_PERCENT;
```

?????? explain this formula in comments because it's not obvious why do you return bigger usdc amount. In fact it leads to mis-pricing of fiats. time=1 1usd=100rub, deposit 1usd get 80rub, time=2 1usd=20rub, withdraw 80rub, where to get 4usd?

##### Status.
NEW


______

#### MAJOR-6.

At
- NarfexP2pRouter.sol:102

```solidity=102
        USDC.transferFrom(address(pool), msg.sender, usdcAmount);
```

safeTransferFrom

##### Status.
NEW


______

#### MAJOR-7.

At
- NarfexKYC.sol:10

```solidity=10
    mapping(address=>string) private _clients;
```

explain why do you use string here? why not IPFS link? Is it "Encrypted JSON encoded account personnel data"? What does it mean encrypted? Who can decrypt it? What if decryption key will leak?

##### Status.
NEW


______

#### MAJOR-8.

At
- NarfexP2pBuyFactory.sol:86

```solidity=86
    function create(address _fiatAddress, uint16 _commission) public {
```

what if validator disagree with Oracle price?

##### Status.
NEW


______

#### MAJOR-9.

At
- NarfexExchangerPool.sol:77

```solidity=77
        token.approve(router, 0); /// Remove allowance
```

safeApprove?? IERC20 interface attack??

##### Status.
NEW


______

#### MAJOR-10.

At
- NarfexExchangerPool.sol:105

```solidity=105
        token.transfer(_msgSender(), _amount);
```

safeTransfer

##### Status.
NEW


______

#### MAJOR-11.

At
- NarfexExchangerPool.sol:114

```solidity=114
        IERC20(_address).transfer(_msgSender(), _amount);
```

safeTransfer

##### Status.
NEW


______

#### MAJOR-12.

At
- NarfexExchangerPool.sol:154

```solidity=154
        emit WithdrawFee(feeAmount);
```

wrong event argument, place this one line above

##### Status.
NEW


______

#### MAJOR-13.

At
- NarfexFiat.sol:19

```solidity=19
contract NarfexFiat is IERC20 {
```

what is the guarantee that 1 NarfexFiat token actually matches 1 real fiat? Consider adding explanations in the contract.

##### Status.
NEW


______

#### MAJOR-14.

At
- NarfexExchangerRouter3.sol:GLOBAL



there is no unit tests!

##### Status.
NEW


______

#### MAJOR-15.

At
- NarfexExchangerRouter3.sol:GLOBAL



Consider refactoring. In general, I advocate for writing as little code as possible; many areas can be replaced with OpenZeppelin implementations of AccessControl and EnumerableSet.

##### Status.
NEW


______

#### WARNING-1.

At
- NarfexLawyers.sol:12

```solidity=12
        bool[7][24] schedule;
```

should it be more flexible (e.g. 10:15-17:45)? what timezone is it?

##### Status.
NEW


______

#### WARNING-2.

At
- NarfexLawyers.sol:55-59

```solidity=55
            for (uint8 w; w < 7; w++) {
                for (uint8 h; h < 24; h++) {
                    schedule[w][h] = true;
                }
            }
```

all hours are true by default, is it correct?

##### Status.
NEW


______

#### WARNING-3.

At
- NarfexLawyers.sol:76

```solidity=76
            for (uint i; i < list.length - 1; i++) {
```

very gas expensive O(n), use EnumerableMap with O(1)

##### Status.
NEW


______

#### WARNING-4.

At
- NarfexFiatFactory.sol:60

```solidity=60
        fiats[tokenSymbol] = fiat;
```

require(fiats[tokenSymbol]==address(0), "already exists") ???

##### Status.
NEW


______

#### WARNING-5.

At
- INarfexOracle.sol:30

```solidity=30
    function getPrice(address _address) external view returns (uint);
```

explain return how many decimals how is it calculated

##### Status.
NEW


______

#### WARNING-6.

At
- PancakeLibrary.sol:52-74

```solidity=52
    function pairFor(address tokenA, address tokenB) internal view returns (address pair) {
        (address token0, address token1) = sortTokens(tokenA, tokenB);
        /// ETH data
        bytes memory factory = hex'5C69bEe701ef814a2B6a3EDD4B1652CB9cc5aA6f';
        bytes memory initCodeHash = hex'96e8ac4277198ff8b6f785478aa9a39f403cb768dd02cbee326c3e7da348845f';
        if (block.chainid == 56) { /// BSC
            factory = hex'cA143Ce32Fe78f1f7019d7d551a6402fC5350c73';
            initCodeHash = hex'00fb7f630766e6a796048ea87d01acd3068e8ff67d078148a3fa3f4a84f69bd5';
        }
        if (block.chainid == 97) { /// BSC testnet
            factory = hex'B7926C0430Afb07AA7DEfDE6DA862aE0Bde767bc';
            initCodeHash = hex'ecba335299a6693cb2ebc4782e74669b84290b6378ea3a3873c7231a8d7d1074';
        }
        if (block.chainid == 137
        || block.chainid == 42161
        || block.chainid == 80001) { /// Polygon or Arbitrum
            factory = hex'c35DADB65012eC5796536bD9864eD8773aBc74C4';
            initCodeHash = hex'e18a34eb0e04b04f7a0ac29a6e80748dca96319b42c54d679cb821dca90c6303';
        }
        if (block.chainid == 324) { /// zkSync Era
            factory = hex'40be1cBa6C5B47cDF9da7f963B6F761F4C60627D';
            initCodeHash = hex'95d5c05820d58f1c8cc736b47fe10a29ddcd2cf73a0d842e8537b9fe510fc618';
        }
```

check

##### Status.
NEW


______

#### WARNING-7.

At
- NarfexP2pRouter.sol:33

```solidity=33
        uint amountLimit,
```

unclear complex semantic, use separate functions for exactIn and exactOut

##### Status.
NEW


______

#### WARNING-8.

At
- NarfexP2pRouter.sol:34,50

```solidity=34
        uint deadline) external payable ensure(deadline)
```
```solidity=50
    function swapToETH(address to, address token, uint amount) external {
```

consider to put nonReentrant to external functions

##### Status.
NEW


______

#### WARNING-9.

At
- NarfexFiat.sol:36-59

```solidity=36
    /**
     * @dev Sets the values for {name} and {symbol}, initializes {decimals} with
     * a default value of 18.
     *
     * To select a different value for {decimals}, use {_setupDecimals}.
     *
     * All three of these values are immutable: they can only be set once during
     * construction.
     */
    constructor(
        string memory tokenName,
        string memory tokenSymbol,
        address factoryAddress
        ) {
        _name = tokenName;
        _symbol = tokenSymbol;
        _factory = INarfexFiatFactory(factoryAddress);

        uint8 currentDecimals = 6;
        if (block.chainid == 56 || block.chainid == 97) {
            currentDecimals = 18;
        }
        _decimals = currentDecimals;
    }
```

wrong comment about decimals

##### Status.
NEW


______

#### WARNING-10.

At
- NarfexFiat.sol:54

```solidity=54
        uint8 currentDecimals = 6;
```

it looks unsafe, some unclear magic numbers. Consider the usage of _factory.defaultDecimals or USDC.decimals() or somethings like this.

##### Status.
NEW


______

#### WARNING-11.

At
- NarfexFiat.sol:61

```solidity=61
    modifier onlyOwner {
```

rename to onlyFullAccess

##### Status.
NEW


______

#### WARNING-12.

At
- NarfexFiat.sol:GLOBAL



ask some native speaker to check all comments :-)

##### Status.
NEW


______

#### WARNING-13.

At
- NarfexFiat.sol:260

```solidity=260
        _balances[sender] = _balances[sender] - amount;
```

silent failure if sender does not have enough balance, consider adding a require with error message before

##### Status.
NEW


______

#### WARNING-14.

At
- NarfexFiat.sol:296

```solidity=296
        _balances[account] = _balances[account] - amount;
```

silent failure if account does not have enough balance, add require with error message before

##### Status.
NEW


______

#### WARNING-15.

At
- NarfexExchangerRouter3.sol:37

```solidity=37
contract NarfexExchangerRouter3 is Ownable, ReentrancyGuard {
```

You may consider employing the approach used for via.protocol, which operates with dozens of bridges and DEXs, similarly to 1inch and Eywa. <br /> There is a router that accepts the method execute(Operation[] operations). <br /> struct Operation { Adapter; TokenIn; AmountIn; TokenOut; minAmountOut; }. <br /> Your router remains minimally simple. <br /> Subsequently, the logic for working with each adapter is delegated to separate smart contracts: AdapterNarfexFiatToUsdc, AdapterPancakeswapV2, AdapterPancakeswapV3, and Adapter1Inch. <br /> This results in a minimalistic router that merely executes a for loop through an array of operations, while the logic for interacting with each DeFi product (including Narfex fiat to USDC) is contained within separate adapters, where the code is concise, simply restructuring arguments for a specific DEX to ensure a unified interface for all.

##### Status.
NEW


______

#### WARNING-16.

At
- NarfexExchangerRouter3.sol:95

```solidity=95
        if (block.chainid == 56 || block.chainid == 97) {
```

use USDC.decimals()

##### Status.
NEW


______

#### WARNING-17.

At
- NarfexExchangerRouter3.sol:128

```solidity=128
    /// @return USDC amount
```

it's wrong in fact

##### Status.
NEW


______

#### WARNING-18.

At
- NarfexExchangerRouter3.sol:205

```solidity=205
            INarfexFiat(B.addr).mintTo(data.to, exchange.outAmount);
```

usdcAmount is not assigned

##### Status.
NEW


______

#### WARNING-19.

At
- NarfexExchangerRouter3.sol:206

```solidity=206
        } else {
```

require B.addr == address(USDC)

##### Status.
NEW


______

#### WARNING-20.

At
- NarfexExchangerRouter3.sol:236-244,250-255

```solidity=236
    function _processSwapData(SwapData memory data) internal view {
        if (data.isExactOut) {
            data.amounts = PancakeLibrary.getAmountsIn(data.outAmount, data.path);
            data.inAmount = data.amounts[0];
        } else {
            data.amounts = PancakeLibrary.getAmountsOut(data.inAmount, data.path);
            data.outAmount = data.amounts[data.amounts.length - 1];
        }
    }
```
```solidity=250
    function _swapOnlyDEX(
        SwapData memory data,
        Token memory A,
        Token memory B
        ) private
    {
```

Why are we interacting directly with PancakeSwap instead of using their Router2? It seems that if the swap chain includes multiple tokens with feeOnTransfer, there might be an issue; I would recommend testing this on a testnet; at the very least, it seems that we should use the actual received target token amount taken from the return value of IPancakePair(PancakeLibrary.pairFor(input, output)).swap.

##### Status.
NEW


______

#### WARNING-21.

At
- NarfexExchangerRouter3.sol:236

```solidity=236
    function _processSwapData(SwapData memory data) internal view {
```

We could refactor and logically separate everything to reduce the overall code complexity; essentially, we should only have a module for swapping fiat <-> USDC; for everything else, we can utilize the already-written router from PancakeSwap, which would likely reduce the volume of your router by a factor of 3.

##### Status.
NEW


______

#### WARNING-22.

At
- NarfexExchangerRouter3.sol:236

```solidity=236
    function _processSwapData(SwapData memory data) internal view {
```

Will we support PancakeSwap v3 pairs? Currently, some new tokens are being deployed exclusively on v3!

##### Status.
NEW


______

#### WARNING-23.

At
- NarfexExchangerRouter3.sol:286

```solidity=286
            assert(WETH.transfer(firstPair, transferInAmount));
```

strictly following IERC20, you HAVE TO use safeTransfer

##### Status.
NEW


______

#### WARNING-24.

At
- NarfexExchangerRouter3.sol:289,299

```solidity=289
                data.from.transfer(msg.value - transferInAmount);
```
```solidity=299
            data.to.transfer(data.outAmount);
```

never use transfer - https://consensys-net.webpkgcache.com/doc/-/s/consensys.net/diligence/blog/2019/09/stop-using-soliditys-transfer-now/

##### Status.
NEW


______

#### WARNING-25.

At
- NarfexExchangerRouter3.sol:492-497,503-509

```solidity=492
        if (A.isFiat && B.isFiat)
        { /// If swap between fiats
            ExchangeData memory exchange = _getExchangeValues(A, B, data.amount, data.isExactOut);
            _swapFiats(data.from, data.to, A, B, exchange);
            return;
        }
```
```solidity=503
        if ((A.isFiat && B.addr == address(USDC))
            || (B.isFiat && A.addr == address(USDC)))
        { /// If swap between fiat and USDC in the pool
            ExchangeData memory exchange = _getExchangeValues(A, B, data.amount, data.isExactOut);
            _swapFiatAndUSDC(data, A, B, exchange, false);
            return;
        }
```

put require path.length==2

##### Status.
NEW


______

#### WARNING-26.

At
- NarfexExchangerRouter3.sol:541

```solidity=541
        uint amountLimit,
```

unclear complex semantic, use separate functions for exactIn and exactOut

##### Status.
NEW


______

#### WARNING-27.

At
- NarfexExchangerRouter3.sol:537-543

```solidity=537
    function swap(
        address[] memory path,
        bool isExactOut,
        uint amount,
        uint amountLimit,
        uint deadline) public payable ensure(deadline)
    {
```

add returns actual tokens amounts. This could be useful for the contract usage.

##### Status.
NEW


______

#### LOW-1.

At
- NarfexLawyers.sol:12

```solidity=12
        bool[7][24] schedule;
```

I'm not sure is it as efficient as bit-shifts

##### Status.
NEW


______

#### LOW-2.

At
- NarfexLawyers.sol:15

```solidity=15
    address public writer;
```

you can use OpenZeppelin's AccessControl

##### Status.
NEW


______

#### LOW-3.

At
- NarfexLawyers.sol:16-18

```solidity=16
    mapping (address=>Lawyer) private _lawyers;
    mapping (address=>bool) private _isLawyer;
    address[] public list;
```

use EnumerableMap

##### Status.
NEW


______

#### LOW-4.

At
- NarfexLawyers.sol:19

```solidity=19
    uint constant DAY = 86400;
```

use "1 days" included Solidity constant

##### Status.
NEW


______

#### LOW-5.

At
- NarfexLawyers.sol:102

```solidity=102
        _lawyers[msg.sender].schedule = _schedule;
```

emit event

##### Status.
NEW


______

#### LOW-6.

At
- NarfexLawyers.sol:108

```solidity=108
        _lawyers[msg.sender].isActive = _newState;
```

emit event

##### Status.
NEW


______

#### LOW-7.

At
- NarfexLawyers.sol:125-126

```solidity=125
        uint8 weekDay = uint8((block.timestamp / DAY + 4) % 7);
        uint8 hour = uint8((block.timestamp / 60 / 60) % 24);
```

conversion to uint8 is redundant

##### Status.
NEW


______

#### LOW-8.

At
- NarfexLawyers.sol:131

```solidity=131
    /// @return Array of addresses
```

note that last elements of array can be address(0), explain return

##### Status.
NEW


______

#### LOW-9.

At
- NarfexFiatFactory.sol:17

```solidity=17
    /// Mapping TokenSymbol=>Address
```

you could use Openzeppelin EnumerableMap here

##### Status.
NEW


______

#### LOW-10.

At
- NarfexFiatFactory.sol:20

```solidity=20
    NarfexFiat[] public fiatsList;
```

maybe the owner should be able to remove fiats from this list

##### Status.
NEW


______

#### LOW-11.

At
- NarfexFiatFactory.sol:23

```solidity=23
    event CreateFiat(string tokenName, string tokenSymbol, NarfexFiat tokenAddress);
```

indexed

##### Status.
NEW


______

#### LOW-12.

At
- NarfexFiatFactory.sol:47

```solidity=47
    function isHaveFullAccess(address account) internal view returns (bool) {
```

rename to hasFullAccess

##### Status.
NEW


______

#### LOW-13.

At
- INarfexOracle.sol:9

```solidity=9
        bool isCustomReward; // Use defalt referral percent on false
```

typo in word "default"

##### Status.
NEW


______

#### LOW-14.

At
- INarfexOracle.sol:10

```solidity=10
        uint price; // USD price only for fiats
```

decimals unclear add comment or calculations example

##### Status.
NEW


______

#### LOW-15.

At
- INarfexOracle.sol:11

```solidity=11
        uint reward; // Referral percent only for fiats
```

decimals unclear add comment or calculations example

##### Status.
NEW


______

#### LOW-16.

At
- INarfexOracle.sol:12

```solidity=12
        int commission; // Commission percent. Can be lower than zero
```

decimals unclear add comment or calculations example

##### Status.
NEW


______

#### LOW-17.

At
- INarfexOracle.sol:13

```solidity=13
        uint transferFee; // Token transfer fee with 1000 decimals precision (20 for NRFX is 2%)
```

maybe 10_000?

##### Status.
NEW


______

#### LOW-18.

At
- NarfexP2pRouter.sol:8

```solidity=8
    uint constant VALIDATOR_POOL_PERCENT = 8000;
```

explain how it works

##### Status.
NEW


______

#### LOW-19.

At
- NarfexP2pRouter.sol:27

```solidity=27
    /// @dev Fiat to crypto must be exchanged via USDT
```

USDC

##### Status.
NEW


______

#### LOW-20.

At
- NarfexP2pRouter.sol:69

```solidity=69
    function deposit(uint _usdcAmount, address _fiatAddress) public nonReentrant returns(uint) {
```

what does it do? add natspec comments

##### Status.
NEW


______

#### LOW-21.

At
- NarfexP2pRouter.sol:88

```solidity=88
    function withdraw(uint _fiatAmount, address _fiatAddress) public nonReentrant returns(uint) {
```

what does it do? add natspec comments

##### Status.
NEW


______

#### LOW-22.

At
- NarfexKYC.sol:11

```solidity=11
    mapping(address=>bool) private _verificators;
```

bad english, rename to verifiers

##### Status.
NEW


______

#### LOW-23.

At
- NarfexKYC.sol:13

```solidity=13
    address public writer;
```

you can use OpenZeppelin's AccessControl

##### Status.
NEW


______

#### LOW-24.

At
- NarfexKYC.sol:56

```solidity=56
        emit Verify(_account);
```

add _data to event

##### Status.
NEW


______

#### LOW-25.

At
- NarfexKYC.sol:59

```solidity=59
    /// @notice Clead account personnel data
```

typo "Clean"

##### Status.
NEW


______

#### LOW-26.

At
- NarfexKYC.sol:79

```solidity=79
    /// @notice Mark account as verificator
```

rename to verifier everywhere

##### Status.
NEW


______

#### LOW-27.

At
- NarfexKYC.sol:104

```solidity=104
        emit Unblacklisted(_account);
```

bad english, rename to "RemovedFromBlacklist"

##### Status.
NEW


______

#### LOW-28.

At
- NarfexKYC.sol:111

```solidity=111
        return _blacklisted[_account];
```

should it be EnumerableSet to let fetch all blacklisted accounts?

##### Status.
NEW


______

#### LOW-29.

At
- NarfexKYC.sol:116

```solidity=116
    /// @return Is can trade
```

bad english

##### Status.
NEW


______

#### LOW-30.

At
- NarfexP2pBuyOffer.sol:15

```solidity=15
        uint32 createDate;
```

rename to timestamp

##### Status.
NEW


______

#### LOW-31.

At
- NarfexP2pBuyOffer.sol:304

```solidity=304
        require(_amount <= getAvailableBalance(), "Not enouth free balance");
```

typo enough

##### Status.
NEW


______

#### LOW-32.

At
- NarfexP2pBuyOffer.sol:328

```solidity=328
    /// @notice Creade a new trade
```

typo create

##### Status.
NEW


______

#### LOW-33.

At
- NarfexP2pBuyOffer.sol:330

```solidity=330
    /// @param bankAccountId Choosed bank account index
```

typo chosen

##### Status.
NEW


______

#### LOW-34.

At
- NarfexP2pBuyOffer.sol:369

```solidity=369
    /// @param bankAccountId Choosed bank account index
```

typo chosen

##### Status.
NEW


______

#### LOW-35.

At
- NarfexP2pBuyFactory.sol:29

```solidity=29
contract NarfexP2pBuyFactory is Ownable {
```

Buy and Sell factories are similar, consider refactoring

##### Status.
NEW


______

#### LOW-36.

At
- NarfexP2pBuyFactory.sol:31

```solidity=31
    struct Offer { /// Offer getter structure
```

remove "getter"

##### Status.
NEW


______

#### LOW-37.

At
- NarfexP2pBuyFactory.sol:37-41

```solidity=37
        uint commission;
        uint totalCommission;
        uint minTrade;
        uint maxTrade;
        uint tradesQuote;
```

unclear semantics consider documenting

##### Status.
NEW


______

#### LOW-38.

At
- NarfexP2pBuyFactory.sol:49

```solidity=49
    uint public tradesLimit = 2; /// Trades count per one offer in one time
```

how is it used?

##### Status.
NEW


______

#### LOW-39.

At
- TestnetUSDC.sol:23-24

```solidity=23
        _name = 'Circle USD';
        _symbol = 'USDC';
```

add TEST

##### Status.
NEW


______

#### LOW-40.

At
- NarfexExchangerPool.sol:21

```solidity=21
contract NarfexExchangerPool is Ownable {
```

inherit from INarfexExchangerPool

##### Status.
NEW


______

#### LOW-41.

At
- NarfexExchangerPool.sol:28

```solidity=28
    mapping(address => mapping(address => uint)) public validatorLimit;
```

explain mapping keys

##### Status.
NEW


______

#### LOW-42.

At
- NarfexExchangerPool.sol:29

```solidity=29
    uint private feeAmount;
```

explain what this is in the comment

##### Status.
NEW


______

#### LOW-43.

At
- NarfexExchangerPool.sol:34

```solidity=34
    event Withdraw(address recipient, uint amount);
```

indexed

##### Status.
NEW


______

#### LOW-44.

At
- NarfexExchangerPool.sol:36

```solidity=36
    event IncreaseValidatorLimit(address indexed validator, address fiat, uint amount);
```

indexed

##### Status.
NEW


______

#### LOW-45.

At
- NarfexExchangerPool.sol:37

```solidity=37
    event DecreaseValidatorLimit(address indexed validator, address fiat, uint amount);
```

indexed

##### Status.
NEW


______

#### LOW-46.

At
- NarfexExchangerPool.sol:119

```solidity=119
        emit IncreaseValidatorLimit(_validator, _fiatAddress, _amount);
```

emit final amount

##### Status.
NEW


______

#### LOW-47.

At
- NarfexExchangerPool.sol:125

```solidity=125
        emit DecreaseValidatorLimit(_validator, _fiatAddress, _amount);
```

emit final amount

##### Status.
NEW


______

#### LOW-48.

At
- NarfexExchangerPool.sol:137

```solidity=137
        feeAmount += _amount;
```

emit event

##### Status.
NEW


______

#### LOW-49.

At
- NarfexExchangerPool.sol:143

```solidity=143
        path[0] = address(token);
```

I would recommend also to add separate function to withdraw token without swap

##### Status.
NEW


______

#### LOW-50.

At
- NarfexExchangerPool.sol:151

```solidity=151
            block.timestamp + 20 minutes
```

not need to add anything

##### Status.
NEW


______

#### LOW-51.

At
- NarfexFiat.sol:7

```solidity=7
interface INarfexFiatFactory {
```

split to another file

##### Status.
NEW


______

#### LOW-52.

At
- NarfexFiat.sol:19

```solidity=19
contract NarfexFiat is IERC20 {
```

inherit from INarfexFiat

##### Status.
NEW


______

#### LOW-53.

At
- NarfexFiat.sol:30-32

```solidity=30
    string private _name;
    string private _symbol;
    uint8 private _decimals;
```

Consider the usage of the "immutable" declaration.

##### Status.
NEW


______

#### LOW-54.

At
- NarfexFiat.sol:34

```solidity=34
    uint256 constant MAX_INT = type(uint).max;
```

explicit "private" declaration.

##### Status.
NEW


______

#### LOW-55.

At
- NarfexFiat.sol:67

```solidity=67
     * @dev Returns the bep token owner.
```

remove "bep"

##### Status.
NEW


______

#### LOW-56.

At
- NarfexFiat.sol:80

```solidity=80
    /// @notice Returns true if the specified address have full access to user funds management
```

"has" not "have"

##### Status.
NEW


______

#### LOW-57.

At
- NarfexFiat.sol:83

```solidity=83
    function isHaveFullAccess(address account) internal view returns (bool) {
```

rename to hasFullAccess

##### Status.
NEW


______

#### LOW-58.

At
- NarfexFiat.sol:88,95,97,102,109,116,123

```solidity=88
     * @dev Returns the token name.
```
```solidity=95
     * @dev Returns the token decimals.
```
```solidity=97
    function decimals() public view returns (uint8) {
```
```solidity=102
     * @dev Returns the token symbol.
```
```solidity=109
     * @dev See {BEP20-totalSupply}.
```
```solidity=116
     * @dev See {BEP20-balanceOf}.
```
```solidity=123
     * @dev See {BEP20-transfer}.
```

everywhere use "@inheritdoc IERC20" instead

##### Status.
NEW


______

#### LOW-59.

At
- NarfexFiat.sol:164

```solidity=164
     * required by the EIP. See the note at the beginning of {BEP20};
```

remove all this BEP20 comments, just use @inheritdoc everywhere

##### Status.
NEW


______

#### LOW-60.

At
- NarfexFiat.sol:202

```solidity=202
        return true;
```

remove the return value, there is no standard for this method. It's not required.

##### Status.
NEW


______

#### LOW-61.

At
- NarfexFiat.sol:219,200

```solidity=219
    function decreaseAllowance(address spender, uint256 subtractedValue) public returns (bool) {
```
```solidity=200
    function increaseAllowance(address spender, uint256 addedValue) public returns (bool) {
```

add NatSpec notice and param

##### Status.
NEW


______

#### LOW-62.

At
- NarfexFiat.sol:225

```solidity=225
        return true;
```

remove the return value, there is no standard for this method. It's not required.

##### Status.
NEW


______

#### LOW-63.

At
- NarfexFiat.sol:228

```solidity=228
    /// @notice Mint amount of tokens to specified reciever address
```

typo "receiver"

##### Status.
NEW


______

#### LOW-64.

At
- NarfexFiat.sol:232

```solidity=232
    /// @return Always true
```

remove the return value, there is no standard for this method. It's not required.

##### Status.
NEW


______

#### LOW-65.

At
- NarfexFiat.sol:257-258,275,294,319-320

```solidity=257
        require(sender != address(0), "BEP20: transfer from the zero address");
        require(recipient != address(0), "BEP20: transfer to the zero address");
```
```solidity=275
        require(account != address(0), "BEP20: mint to the zero address");
```
```solidity=294
        require(account != address(0), "BEP20: burn from the zero address");
```
```solidity=319
        require(owner != address(0), "BEP20: approve from the zero address");
        require(spender != address(0), "BEP20: approve to the zero address");
```

to be strict it's not always BEP20, consider renaming with "IERC20:" or "NarfexFiat:"

##### Status.
NEW


______

#### LOW-66.

At
- NarfexExchangerRouter3.sol:10-29

```solidity=10
interface INarfexFiat is IERC20 {
    function burnFrom(address _address, uint _amount) external;
    function mintTo(address _address, uint _amount) external;
}

interface INarfexExchangerPool {
    function getFeeAmount() external view returns (uint);
    function getBalance() external view returns (uint);
    function approveRouter() external;
    function increaseLimit(address _validator, address _fiatAddress, uint _amount) external;
    function decreaseLimit(address _validator, address _fiatAddress, uint _amount) external;
    function increaseFeeAmount(uint _amount) external;
    function clearFeeAmount() external;
}

interface IWETH {
    function deposit() external payable;
    function transfer(address to, uint value) external returns (bool);
    function withdraw(uint) external;
}
```

split to other files and reuse

##### Status.
NEW


______

#### LOW-67.

At
- NarfexExchangerRouter3.sol:37

```solidity=37
contract NarfexExchangerRouter3 is Ownable, ReentrancyGuard {
```

rename to NarfexExchangerRouter

##### Status.
NEW


______

#### LOW-68.

At
- NarfexExchangerRouter3.sol:40

```solidity=40
    /// Structures for solving the problem of limiting the number of variables
```

The presence of triple slashes (///) instead of double slashes (//) outside of NatSpec comments is unconventional; it is recommended to follow generally accepted practice; in essence, the utilization of triple slashes (///) can be considered erroneous, as the Solidity compiler (solc) interprets them as NatSpec comments with the Doxygen @notice tag; it is peculiar that the compiler processes them without any warnings.

##### Status.
NEW


______

#### LOW-69.

At
- NarfexExchangerRouter3.sol:43

```solidity=43
        uint rate;
```

add comment or example of calculations, what is the denominator of the rate? consider documenting USDC_PRECISION

##### Status.
NEW


______

#### LOW-70.

At
- NarfexExchangerRouter3.sol:53-56

```solidity=53
        uint inAmount;
        uint inAmountMax;
        uint outAmount;
        uint outAmountMin;
```

what is the difference with inAmount, consider documenting or rename to actualInAmount, actualOutAmount

##### Status.
NEW


______

#### LOW-71.

At
- NarfexExchangerRouter3.sol:65,67-68

```solidity=65
        int commission;
```
```solidity=67
        uint reward;
        uint transferFee;
```

denominated by PRECISION or PERCENT_PRECISION?

##### Status.
NEW


______

#### LOW-72.

At
- NarfexExchangerRouter3.sol:77

```solidity=77
    uint internal USDC_PRECISION = 10**6;
```

set in constructor

##### Status.
NEW


______

#### LOW-73.

At
- NarfexExchangerRouter3.sol:79

```solidity=79
    uint constant MAX_INT = 2**256 - 1;
```

use type(uint).max

##### Status.
NEW


______

#### LOW-74.

At
- NarfexExchangerRouter3.sol:102

```solidity=102
    modifier ensure(uint deadline) {
```

rename with ensureDeadline

##### Status.
NEW


______

#### LOW-75.

At
- NarfexExchangerRouter3.sol:107

```solidity=107
    event SwapFiat(address indexed _account, address _fromToken, address _toToken, ExchangeData _exchange);
```

indexed _fromToken _toToken

##### Status.
NEW


______

#### LOW-76.

At
- NarfexExchangerRouter3.sol:108

```solidity=108
    event SwapDEX(address indexed _account, address _fromToken, address _toToken, uint inAmount, uint outAmount);
```

indexed _fromToken _toToken

##### Status.
NEW


______

#### LOW-77.

At
- NarfexExchangerRouter3.sol:112

```solidity=112
        assert(msg.sender == address(WETH));
```

use require with error message

##### Status.
NEW


______

#### LOW-78.

At
- NarfexExchangerRouter3.sol:246

```solidity=246
    /// @notice Exchange only between crypto Сoins through liquidity pairs
```

wrong term. replace Coins with Tokens, https://zerion.io/blog/crypto-coins-vs-tokens-the-difference-explained/

##### Status.
NEW


______

#### LOW-79.

At
- NarfexExchangerRouter3.sol:284

```solidity=284
            require(msg.value >= transferInAmount, "BNB is not sended");
```

typo in "sent"

##### Status.
NEW


______

#### LOW-80.

At
- NarfexExchangerRouter3.sol:332

```solidity=332
    function _swapFiatWithDEX(
```

very long function consider refactoring

##### Status.
NEW


______

#### LOW-81.

At
- NarfexExchangerRouter3.sol:474

```solidity=474
        ) internal nonReentrant
```

I would prefer to put nonReentrant on external/public functions instead

##### Status.
NEW


______

#### LOW-82.

At
- NarfexExchangerRouter3.sol:519,526

```solidity=519
    function setPool(address _newPoolAddress, uint8 _decimals) public onlyOwner {
```
```solidity=526
    function setOracle(address _newOracleAddress) public onlyOwner {
```

emit event

##### Status.
NEW


______

#### LOW-83.

At
- NarfexExchangerRouter3.sol:551

```solidity=551
        data.inAmountMax = isExactOut ? amountLimit : MAX_INT;
```

looks scary, consider adding a comment

##### Status.
NEW


