---
# System prepended metadata

title: Security Audit of `SpotV1` contracts.

---

# Security Audit of `SpotV1` contracts. 

## Conclusion

This audit was made by 
Auditor: TODO.
Date: 2023-03-24

## Scope


https://testnet.bscscan.com/address/0x36AD17070546CAB7BcCFecbe1e8DBC45DE933191
- IRDNRegistry.sol
- RDNRegistry.sol
- WithdrawAnyERC20Token.sol

https://testnet.bscscan.com/address/0xBC526044b63b0aEe97F66195B956aA932C6776e9
- ERC20Spot.sol
- SafeUniswapV2Router.sol
- SpotV1.sol
- IRDNRegistry.sol
- RDNOwnable.sol

https://testnet.bscscan.com/address/0x81953203B4DDa69eF89d4a2370B5DA7E782a6Fc6
- ISpotV1.sol
- SpotFactoryV1.sol
- IRDNRegistry.sol

md5 hashes:
- 557e42e5e9eee64eab9ab17c5ccad488  contracts/ERC20Spot.sol
- d9f4aa3a24ec1985b279e625e7f2525e  contracts/IRDNRegistry.sol
- b291a12383ca9ed97198831ee0b33803  contracts/ISpotV1.sol
- 8fb5c78bf695e5f0b83aa771d4a5f9ea  contracts/RDNOwnable.sol
- baff4823cf421cdb2a4bf02291a72f62  contracts/RDNRegistry.sol
- fda3b5335bab4284a0378c54118ac48d  contracts/SafeUniswapV2Router.sol
- c6f88a1e59d65fe467cd08244101049c  contracts/SpotFactoryV1.sol
- 098ea1a4a7ecf95f32875716d46289ce  contracts/SpotV1.sol
- 22bad6e816918f504a649cd45228888c  contracts/WithdrawAnyERC20Token.sol


## 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

#### MAJOR-1.

At

- SafeUniswapV2Router.sol:50
```solidity=50
return router.addLiquidity(tokenA, tokenB, amountA, amountB, 0, 0, to, deadline);
```


- SafeUniswapV2Router.sol:76
```solidity=76
(amountA, amountB) = router.removeLiquidity(tokenA, tokenB, balance, 0, 0, to, deadline);
```


minAmountOut=0 sandwich transaction / MEV Flashbots attacks are possible

##### Status.
NEW


______

#### MAJOR-2.

At

- SpotFactoryV1.sol:63
```solidity=63
IERC20(swap0.path[0]).safeTransferFrom(msg.sender, address(this), amount);
```


some deflationary or fee-on-transfer tokens may deliver you different amount of tokens, consider using balanceOf check to ensure how much tokens did you receive after transferFrom

##### Status.
NEW


______

#### MAJOR-3.

At

- SpotFactoryV1.sol:64
```solidity=64
IERC20(swap0.path[0]).approve(spot, amount);
```


use safeApprove

##### Status.
NEW


______

#### MAJOR-4.

At

- SpotV1.sol:82-89
```solidity=82
function deposit(
    uint amount,
    Swap memory swap0,
    Swap memory swap1,
    Swap memory swapReward0,
    Swap memory swapReward1,
    uint deadline
) public payable {
```


- SpotV1.sol:129-136
```solidity=129
function withdraw(
    uint amountToBurn,
    Swap memory swap0,
    Swap memory swap1,
    Swap memory swapReward0,
    Swap memory swapReward1,
    uint deadline
) public onlyRDNOwner(msg.sender) {
```


- SpotV1.sol:172-174
```solidity=172
function callAny(address payable _addr, bytes memory _data) public payable onlyRDNOwner(msg.sender) returns(bool success, bytes memory data){
    (success, data) = _addr.call{value: msg.value}(_data);
}
```


- SpotV1.sol:176-180
```solidity=176
function restake(
    Swap memory swapReward0,
    Swap memory swapReward1,
    uint deadline
) public {
```


use nonReentrant modifier to protect fron reentrancy attacks

##### Status.
NEW


______

#### MAJOR-5.

At

- SpotV1.sol:108
```solidity=108
IERC20(swap0.path[0]).safeTransferFrom(msg.sender, address(this), amount);
```


some deflationary or fee-on-transfer tokens may deliver you different amount of tokens, consider using balanceOf check to ensure how much tokens did you receive after transferFrom, otherwise your code will not work with such tokens because receiving amount < transferred

##### Status.
NEW


______

#### MAJOR-6.

At

- SpotV1.sol:287-288
```solidity=287
0,
0,
```


- SpotV1.sol:310-311
```solidity=310
0,
0,
```


no minAmountOut sandwich attacks / MVE flashbots possible

##### Status.
NEW


______

#### MAJOR-7.

At

- RDNOwnable.sol:22
```solidity=22
require(IRDNRegistry(registry).getUserIdByAddress(_userAddress) == ownerId, "RDNOwnable: access denied");
```


getUserIdByAddress returns 0 for non existent _userAddress, in case if ownerId=0, any non-existent user may pass this check. I'm not sure how serious is it. Probably you will never set ownerId=0. Maybe it makes sense to add explicit require that ownerId!=0. And add explaining comment to the code. Maybe use isRegisteredByAddress check.

##### Status.
NEW


______

#### WARNING-1.

At

- SpotFactoryV1.sol:13-17
```solidity=13
// todo
// ownerid shoud be active / RDNConnected
// tariffs / subscription rules
// onlyRDN
// BNB create and deposit
```


- SpotV1.sol:239
```solidity=239
// todo: return remained
```


- SpotV1.sol:268
```solidity=268
// todo: return remained
```


resolve todo

##### Status.
NEW


______

#### WARNING-2.

At

- WithdrawAnyERC20Token.sol:18
```solidity=18
if (_isDefaultAdminRole) {
```


i believe it should always happen otherwise there will be no admin

##### Status.
NEW


______

#### WARNING-3.

At

- WithdrawAnyERC20Token.sol:25
```solidity=25
IERC20(_token).transfer(_target, _amount);
```


use safeTransfer

##### Status.
NEW


______

#### WARNING-4.

At

- SafeUniswapV2Router.sol:12-22
```solidity=12
function safeSwapExactTokensForTokens(
  IUniswapV2Router router,
  uint256 amountIn,
  uint256 amountOutMin,
  address[] memory path,
  address to,
  uint256 deadline
) internal returns (uint256[] memory amounts) {
  if (path[0] != path[path.length - 1])
    amounts = router.swapExactTokensForTokens(amountIn, amountOutMin, path, to, deadline);
}
```


you may want to use swapExactTokensForTokensSupportingFeeOnTransferTokens https://github.com/Uniswap/v2-periphery/blob/master/contracts/UniswapV2Router02.sol#L339

##### Status.
NEW


______

#### WARNING-5.

At

- SafeUniswapV2Router.sol:24
```solidity=24
function addAllLiquidity(
```


please note in the function comment that it may not spend all of your tokens balance just to not misuse it

##### Status.
NEW


______

#### WARNING-6.

At

- SpotV1.sol:17-30
```solidity=17
// todo
// return remained !!!
// swap fees support !!!
// safeERC20 !!!
// BNB support !!!
// restake rules
// transfers logs to userId
// informative name and symbol
// owner should be active
// tariffs / subscription rules
// deposit / withdraw without restake ??
// APR / APY
// StableSwap support
// !!!!!! init functions permissions, close reInit
```


resolve todos

##### Status.
NEW


______

#### WARNING-7.

At

- SpotV1.sol:42
```solidity=42
address public pool;
```


there is a lot of logic about interacting with IMasterChef2, but in the example contract there is no pool set so I cannot check interactions - https://testnet.bscscan.com/address/0xBC526044b63b0aEe97F66195B956aA932C6776e9#readContract pool==address(0) probably this one is meant to be used https://bscscan.com/address/0xa5f8C5Dbd5F286960b9d90548680aE5ebFf07652#code

##### Status.
NEW


______

#### WARNING-8.

At

- SpotV1.sol:104
```solidity=104
if (msg.value > 0) {
```


require path[0] == wETH?

##### Status.
NEW


______

#### WARNING-9.

At

- SpotV1.sol:105
```solidity=105
amount = msg.value;
```


require amount == msg.value?

##### Status.
NEW


______

#### WARNING-10.

At

- SpotV1.sol:121
```solidity=121
amountToMint = ((afterDepositBalance - beforeDepositBalance) * beforeDepositSupply) / beforeDepositBalance;
```


- SpotV1.sol:152
```solidity=152
uint amountToWithdraw = ((amountToBurn * beforeWithdrawBalance)) / beforeWithdrawSupply;
```


tiny rounding error may possible what may lead to tokens dust aggregation

##### Status.
NEW


______

#### WARNING-11.

At

- SpotV1.sol:282-291
```solidity=282
IUniswapV2Router(_router).addLiquidity(
    token0,
    token1,
    amountIn0,
    amountIn1,
    0,
    0,
    address(this),
    deadline
);
```


unused tokens remains on the contract balance, check https://github.com/Uniswap/v2-periphery/blob/master/contracts/UniswapV2Router01.sol#L58 it transfers only amount needed for even addLiquidity ensure _returnRemainder is always called

##### Status.
NEW


______

#### WARNING-12.

At

- SpotV1.sol:336
```solidity=336
IUniswapV2Router(router).swapExactTokensForTokensSupportingFeeOnTransferTokens(
```


do you want to support Native currency?

##### Status.
NEW


______

#### WARNING-13.

At

- SpotV1.sol:360
```solidity=360
if (tokens[i] == address(0)) continue;
```


should we process native eth in that case?

##### Status.
NEW


______

#### WARNING-14.

At

- RDNOwnable.sol:14
```solidity=14
ownerId = _ownerId;
```


ownerId is unchangeable, maybe you will want to add a transferOwnership method (then not forget to change Factory mapping)

##### Status.
NEW


______

#### WARNING-15.

At

- ERC20Spot.sol:10
```solidity=10
mapping(uint => uint) private _balances;
```


it's not clear why mapping is from userId=>amount not address=>amount as usual. What is the overall business rationale behind this userId uint based mechanism? Can you write explaining comment?

##### Status.
NEW


______

#### WARNING-16.

At

- ERC20Spot.sol:11
```solidity=11
mapping(address => mapping(address => uint)) private _allowances;
```


allowance is given to address not userId, ensure there is no misusing possible

##### Status.
NEW


______

#### WARNING-17.

At

- ERC20Spot.sol:26
```solidity=26
uint userId = IRDNRegistry(registry).getUserIdByAddress(_account);
```


be careful with non existent user

##### Status.
NEW


______

#### WARNING-18.

At

- ERC20Spot.sol:50-64
```solidity=50
    function transfer(address to, uint amount) external returns (bool) {
        return false;
    }

    function approve(address spender, uint amount) external returns (bool) {
        return false;
    }

    function transferFrom(
        address from,
        address to,
        uint amount
    ) external returns (bool) {
        return false;
    }
```


not all DeFi projects correctly implement erc20 interface call and check return value after the call, so the may think that transfer did success because it did not fail, consider add a revert inside

##### Status.
NEW


______

#### WARNING-19.

At

- ERC20Spot.sol:76
```solidity=76
}
```


- ERC20Spot.sol:91
```solidity=91
}
```


external DeFi tool may wrongly estimate account balance. Transfer is emitted only on mint/burn however in RDNRegistry user may transfer the userId ownership to another wallet, no Transfer event will be emited in this case and some DeFi tools will still think that the previous wallet has this balance, but its not.

##### Status.
NEW


______

#### WARNING-20.

At

- RDNRegistry.sol:120
```solidity=120
users[_userId].userAddress = _newAddress;
```


emit event, it's important state change you may want to catch it on backend/frontend

##### Status.
NEW


______

#### WARNING-21.

At

- RDNRegistry.sol:138
```solidity=138
changeAddressAccess[_userId][_grantedAddress] = false;
```


changeAddressAddresses is not cleared, i'm not sure how serious is it, maybe use EnumerableSet for changeAddressAddresses for cheap deletion

##### Status.
NEW


______

#### WARNING-22.

At

- RDNRegistry.sol:146
```solidity=146
function levelUp(uint _userId, uint _level) public onlyRole(LEVELUP_ROLE) onlyValidUser(_userId) {
```


role system is account based, it may happen that account userId will change but the permission remain the same, is it OK?

##### Status.
NEW


______

#### WARNING-23.

At

- RDNRegistry.sol:180
```solidity=180
function grantCompleteAdmin(address _admin) public {
```


- RDNRegistry.sol:192
```solidity=192
function revokeCompleteAdmin(address _admin) public {
```


place onlyRole(DEFAULT_ADMIN_ROLE) modifier and use internal _grantRole to avoid multiple role checks and to state in the declaration that the method is for admin only

##### Status.
NEW


______

#### WARNING-24.

At

- RDNRegistry.sol:218
```solidity=218
function _addUser(address _userAddress, uint _parentId) private returns(uint) {
```


any parent may be set by the user, I don't know which mechanism will you use but you could add signature backend verification to prevent setting wrong parent

##### Status.
NEW


______

#### WARNING-25.

At

- RDNRegistry.sol:290-312
```solidity=290
    function isActive(uint _userId) public view returns(bool) {
        return (users[_userId].activeUntill > block.timestamp);
    }

    function getParentId(uint _userId) public view returns(uint) {
        return users[_userId].parentId;
    }

    function getLevel(uint _userId) public view returns(uint) {
        return users[_userId].level;
    }

    function getTariff(uint _userId) public view returns(uint) {
        return users[_userId].tariff;
    }

    function getActiveUntill(uint _userId) public view returns(uint) {
        return users[_userId].activeUntill;
    }

    function getUserAddress(uint _userId) public view returns(address) {
        return users[_userId].userAddress;
    }
```


- RDNRegistry.sol:318-320
```solidity=318
function getUser(uint _userId) public view returns(User memory) {
    return users[_userId];
}
```


maybe use isValidUser or isRegisteredByAddress

##### Status.
NEW


______

#### WARNING-26.

At

- RDNRegistry.sol:322-324
```solidity=322
function getUserIdByAddress(address _userAddress) public view returns(uint) {
    return userId[_userAddress];
}
```


it returns 0 if user not exist. It may lead to misuse the info for userId=0. Place isRegistered or is isValidUser modifier

##### Status.
NEW


______

#### WARNING-27.

At

- RDNRegistry.sol:343
```solidity=343
function isHasChangeAddressAccess(uint _userId, address _grantedAddress) public view returns(bool) {
```


check if _userId is valid, otherwise isHasChangeAddressAccess(666, address(0))==true

##### Status.
NEW


______

#### LOW-1.

At

- ERC20Spot.sol:6
```solidity=6

```


- ERC20Spot.sol:21
```solidity=21

```


- ERC20Spot.sol:24
```solidity=24

```


- RDNRegistry.sol:350-351
```solidity=350


```


remove blank line, follow style guide - https://docs.soliditylang.org/en/v0.8.17/style-guide.html

##### Status.
NEW


______

#### LOW-2.

At

- RDNOwnable.sol:11
```solidity=11
function initRDNOwnable(address _registry, uint _ownerId) internal {
```


- ERC20Spot.sol:16
```solidity=16
function initERC20Spot(address _registry, uint _ownerId) internal {
```


prefix internal method with _

##### Status.
NEW


______

#### LOW-3.

At

- RDNOwnable.sol:7
```solidity=7
contract RDNOwnable {
```


- ERC20Spot.sol:8
```solidity=8
contract ERC20Spot is IERC20, RDNOwnable {
```


put the "abstract" declaration for the contract which will not be deployed but just used as a parent contract

##### Status.
NEW


______

#### LOW-4.

At

- SpotV1.sol:94
```solidity=94
if (IMasterChef2(pool).pendingCake(poolIndex, address(this)) > 0) {
```


- SpotV1.sol:141
```solidity=141
if (IMasterChef2(pool).pendingCake(poolIndex, address(this)) > 0) {
```


use _pool, _poolIndex

##### Status.
NEW


______

#### LOW-5.

At

- WithdrawAnyERC20Token.sol:24-26
```solidity=24
function withdrawAnyERC20Token(address _token, address _target, uint _amount) public onlyRole(WITHDRAWANY_ROLE) {
    IERC20(_token).transfer(_target, _amount);
}
```


- SafeUniswapV2Router.sol:12
```solidity=12
function safeSwapExactTokensForTokens(
```


- SafeUniswapV2Router.sol:12
```solidity=12
function safeSwapExactTokensForTokens(
```


- SafeUniswapV2Router.sol:24
```solidity=24
function addAllLiquidity(
```


- SafeUniswapV2Router.sol:53
```solidity=53
function removeAllLiquidity(
```


this function is never used in the scope of this audit

##### Status.
NEW


______

#### LOW-6.

At

- WithdrawAnyERC20Token.sol:3
```solidity=3
// SPDX-License-Identifier: MIT
```


- SpotFactoryV1.sol:2
```solidity=2
// SPDX-License-Identifier: MIT
```


- SpotV1.sol:2
```solidity=2
// SPDX-License-Identifier: MIT
```


- ISpotV1.sol:2
```solidity=2
// SPDX-License-Identifier: MIT
```


- ERC20Spot.sol:2
```solidity=2
// SPDX-License-Identifier: MIT
```


place on top https://docs.soliditylang.org/en/v0.8.19/layout-of-source-files.html#spdx-license-identifier

##### Status.
NEW


______

#### LOW-7.

At

- WithdrawAnyERC20Token.sol:8-12
```solidity=8
// interface IWithdrawAnyERC20Token {

//     function withdrawAnyERC20Token(address _token, address _target, uint _amount) external;

// }
```


remove

##### Status.
NEW


______

#### LOW-8.

At

- WithdrawAnyERC20Token.sol:15
```solidity=15
bytes32 public constant WITHDRAWANY_ROLE = keccak256("WITHDRAWANY_ROLE");
```


place underscore

##### Status.
NEW


______

#### LOW-9.

At

- WithdrawAnyERC20Token.sol:19
```solidity=19
_setupRole(DEFAULT_ADMIN_ROLE, _admin);
```


- WithdrawAnyERC20Token.sol:21
```solidity=21
_setupRole(WITHDRAWANY_ROLE, _admin);
```


deprecated use _grantRole

##### Status.
NEW


______

#### LOW-10.

At

- WithdrawAnyERC20Token.sol:24-26
```solidity=24
function withdrawAnyERC20Token(address _token, address _target, uint _amount) public onlyRole(WITHDRAWANY_ROLE) {
    IERC20(_token).transfer(_target, _amount);
}
```


emit event to make transfers made by calling withdrawAnyERC20Token

##### Status.
NEW


______

#### LOW-11.

At

- WithdrawAnyERC20Token.sol:24-26
```solidity=24
function withdrawAnyERC20Token(address _token, address _target, uint _amount) public onlyRole(WITHDRAWANY_ROLE) {
    IERC20(_token).transfer(_target, _amount);
}
```


you may also add withdrawAnyNative method

##### Status.
NEW


______

#### LOW-12.

At

- SafeUniswapV2Router.sol:1
```solidity=1
// SPDX-License-Identifier: BSD-3-Clause
```


SPDX license is different from other files, are you sure?

##### Status.
NEW


______

#### LOW-13.

At

- SafeUniswapV2Router.sol:2
```solidity=2
pragma solidity ^0.8.6;
```


maybe use "0.8.17" for consistency

##### Status.
NEW


______

#### LOW-14.

At

- SafeUniswapV2Router.sol:12-22
```solidity=12
function safeSwapExactTokensForTokens(
  IUniswapV2Router router,
  uint256 amountIn,
  uint256 amountOutMin,
  address[] memory path,
  address to,
  uint256 deadline
) internal returns (uint256[] memory amounts) {
  if (path[0] != path[path.length - 1])
    amounts = router.swapExactTokensForTokens(amountIn, amountOutMin, path, to, deadline);
}
```


what safe check this function makes? it's not clear

##### Status.
NEW


______

#### LOW-15.

At

- SafeUniswapV2Router.sol:20
```solidity=20
if (path[0] != path[path.length - 1])
```


check that path.length > 0

##### Status.
NEW


______

#### LOW-16.

At

- SafeUniswapV2Router.sol:21
```solidity=21
amounts = router.swapExactTokensForTokens(amountIn, amountOutMin, path, to, deadline);
```


use named parameters to avoid misplacing args, see https://docs.soliditylang.org/en/v0.8.19/control-structures.html#function-calls-with-named-parameters

##### Status.
NEW


______

#### LOW-17.

At

- SafeUniswapV2Router.sol:33-35
```solidity=33
uint256,
uint256,
uint256
```


place NatSpec description of return values

##### Status.
NEW


______

#### LOW-18.

At

- SafeUniswapV2Router.sol:39-42
```solidity=39
if (IERC20(tokenA).allowance(address(this), address(router)) > 0) {
  IERC20(tokenA).safeApprove(address(router), 0);
}
IERC20(tokenA).safeApprove(address(router), amountA);
```


- SafeUniswapV2Router.sol:45-48
```solidity=45
if (IERC20(tokenB).allowance(address(this), address(router)) > 0) {
  IERC20(tokenB).safeApprove(address(router), 0);
}
IERC20(tokenB).safeApprove(address(router), amountB);
```


- SafeUniswapV2Router.sol:71-74
```solidity=71
if (IERC20(pair).allowance(address(this), address(router)) > 0) {
  IERC20(pair).safeApprove(address(router), 0);
}
IERC20(pair).safeApprove(address(router), balance);
```


what is the purpose of this? why do we need to set allowance to 0

##### Status.
NEW


______

#### LOW-19.

At

- SpotFactoryV1.sol:11
```solidity=11
contract SpotFactoryV1 {
```


what is the purpose of this contract? write detailed NatSpec docs

##### Status.
NEW


______

#### LOW-20.

At

- SpotFactoryV1.sol:21
```solidity=21
using SafeERC20 for IERC20;
```


place on the top of the contract code

##### Status.
NEW


______

#### LOW-21.

At

- SpotFactoryV1.sol:30
```solidity=30
mapping(uint => mapping(uint => address)) public spots;
```


what is the key in this mapping, add comment

##### Status.
NEW


______

#### LOW-22.

At

- SpotFactoryV1.sol:51-54
```solidity=51
ISpotV1.Swap memory swap0,
ISpotV1.Swap memory swap1,
ISpotV1.Swap memory swapReward0,
ISpotV1.Swap memory swapReward1,
```


unclear semantics, consider documenting in NatSpec what is swap0 and swap1

##### Status.
NEW


______

#### LOW-23.

At

- SpotFactoryV1.sol:61
```solidity=61
ISpotV1(spot).deposit{value: msg.value}(amount, swap0, swap1, swapReward0, swapReward1, deadline);
```


- SpotFactoryV1.sol:65
```solidity=65
ISpotV1(spot).deposit(amount, swap0, swap1, swapReward0, swapReward1, deadline);
```


- SpotFactoryV1.sol:78
```solidity=78
ISpotV1(spot).init(wrapper, pool, router, stakingToken, rewardToken, poolIndex, ownerId, registry, address(this));
```


use named parameters for readability and to avoid misplacing args, see https://docs.soliditylang.org/en/v0.8.19/control-structures.html#function-calls-with-named-parameters

##### Status.
NEW


______

#### LOW-24.

At

- SpotFactoryV1.sol:80
```solidity=80
// pools[ownerId].push(poolIndex);
```


remove this comment

##### Status.
NEW


______

#### LOW-25.

At

- SpotFactoryV1.sol:89
```solidity=89
uint poolsCount = IMasterChef2(pool).poolLength();
```


IMasterChef2 is not part of this audit scope, I cannot be sure this is a valid pools length

##### Status.
NEW


______

#### LOW-26.

At

- SpotFactoryV1.sol:98
```solidity=98
return spots[ownerId][poolIndex];
```


add a check that this spot exists

##### Status.
NEW


______

#### LOW-27.

At

- SpotV1.sol:13
```solidity=13
contract SpotV1 is ERC20Spot {
```


inherit ISpotV1

##### Status.
NEW


______

#### LOW-28.

At

- SpotV1.sol:33-35
```solidity=33
// interaction changes:
// wrapper in init
// deposit BNB, withdraw BNB
```


remove these comments

##### Status.
NEW


______

#### LOW-29.

At

- SpotV1.sol:37-40
```solidity=37
struct Swap {
    address[] path;
    uint outMin;
}
```


remove it because it's already a part of ISpotV1

##### Status.
NEW


______

#### LOW-30.

At

- SpotV1.sol:43
```solidity=43
address public router;
```


maybe rename as IMasterChef2 public masterChef2Pool; for clarity

##### Status.
NEW


______

#### LOW-31.

At

- SpotV1.sol:51
```solidity=51
address public wrapper;
```


maybe delcare it as "IWrapper public wBNB;"

##### Status.
NEW


______

#### LOW-32.

At

- SpotV1.sol:53
```solidity=53
constructor() {
```


why is it empty? note that this contract is intended to be used with proxy

##### Status.
NEW


______

#### LOW-33.

At

- SpotV1.sol:82-89
```solidity=82
function deposit(
    uint amount,
    Swap memory swap0,
    Swap memory swap1,
    Swap memory swapReward0,
    Swap memory swapReward1,
    uint deadline
) public payable {
```


- SpotV1.sol:129-136
```solidity=129
function withdraw(
    uint amountToBurn,
    Swap memory swap0,
    Swap memory swap1,
    Swap memory swapReward0,
    Swap memory swapReward1,
    uint deadline
) public onlyRDNOwner(msg.sender) {
```


unclear functions semantic consider documenting

##### Status.
NEW


______

#### LOW-34.

At

- SpotV1.sol:161
```solidity=161
require(sentRecipient, "transfer ETH to recipeint failed");
```


typo in word recipient (also not ETH but BNB)

##### Status.
NEW


______

#### LOW-35.

At

- SpotV1.sol:166
```solidity=166
//burn
```


comment not need

##### Status.
NEW


______

#### LOW-36.

At

- SpotV1.sol:214
```solidity=214
function _buyLiquidity(
```


what does this function supposed to do? why does it have 2 swap arguments?

##### Status.
NEW


______

#### LOW-37.

At

- SpotV1.sol:220
```solidity=220
require(swap0.path[0] == swap1.path[0], "start tokens should be equal");
```


explain. write comments

##### Status.
NEW


______

#### LOW-38.

At

- SpotV1.sol:323
```solidity=323
token.safeApprove(spender, 0);
```


why do we need this?

##### Status.
NEW


______

#### LOW-39.

At

- SpotV1.sol:334
```solidity=334
if (path[0] == path[path.length - 1]) return;
```


maybe fail? it's something really unexpected

##### Status.
NEW


______

#### LOW-40.

At

- SpotV1.sol:337
```solidity=337
// IUniswapV2Router(router).swapExactTokensForTokens(
```


remove comment

##### Status.
NEW


______

#### LOW-41.

At

- SpotV1.sol:366
```solidity=366
require(sentRecipient, "transfer ETH to recipeint failed");
```


typo in word recipient (also in fact it's not ETH it's BNB)

##### Status.
NEW


______

#### LOW-42.

At

- IRDNRegistry.sol:12
```solidity=12
uint activeUntill;
```


- IRDNRegistry.sol:40
```solidity=40
function getActiveUntill(uint _userId) external view returns(uint);
```


- IRDNRegistry.sol:48
```solidity=48
function setActiveUntill(uint _userId, uint _activeUntill) external;
```


typo in word "until"

##### Status.
NEW


______

#### LOW-43.

At

- RDNOwnable.sol:8-9
```solidity=8
address public registry;
uint public ownerId;
```


consider the usage of immutable declaration or add setter

##### Status.
NEW


______

#### LOW-44.

At

- RDNOwnable.sol:13
```solidity=13
require(IRDNRegistry(registry).isValidUser(_ownerId));
```


add revert message for clarity of failed transactions

##### Status.
NEW


______

#### LOW-45.

At

- RDNOwnable.sol:17
```solidity=17
// modifier RDNOnly(address _sender) {
```


remove unused code

##### Status.
NEW


______

#### LOW-46.

At

- ERC20Spot.sol:13-14
```solidity=13
string private _name;
string private _symbol;
```


use immutable declaration

##### Status.
NEW


______

#### LOW-47.

At

- ERC20Spot.sol:18
```solidity=18
_name = "SPOTv0 Share Token";
```


what does v0 mean? Consider renaming to v1 or explain what does v0 mean

##### Status.
NEW


______

#### LOW-48.

At

- ERC20Spot.sol:28
```solidity=28
}
```


maybe it's useful to have function balanceOf(uint _userId) as well

##### Status.
NEW


______

#### LOW-49.

At

- ERC20Spot.sol:31
```solidity=31
return _allowances[owner][spender];
```


this will always return 0, since approve is not implemented

##### Status.
NEW


______

#### LOW-50.

At

- ERC20Spot.sol:68
```solidity=68
require(account != address(0), "ERC20: mint to the zero address");
```


everywhere, in general string error messages are suboptimal, consider the usage of custom solidity errors, see - https://blog.soliditylang.org/2021/04/21/custom-errors/ https://github.com/OpenZeppelin/openzeppelin-contracts/issues/2839

##### Status.
NEW


______

#### LOW-51.

At

- RDNRegistry.sol:9
```solidity=9
// Parent 0 address restricted
```


make the comment more clear

##### Status.
NEW


______

#### LOW-52.

At

- RDNRegistry.sol:15-29
```solidity=15
bytes32 public constant SETLEVEL_ROLE = keccak256("SETLEVEL_ROLE");
// admin role for userlevel change (levelUp function)
bytes32 public constant LEVELUP_ROLE = keccak256("LEVELUP_ROLE");
// admin role for changing factors contract address
bytes32 public constant FACTORSADDRESS_ROLE = keccak256("FACTORSADDRESS_ROLE");
// admin role for RDNPOS contract
bytes32 public constant TARIFFUPDATE_ROLE = keccak256("TARIFFUPDATE_ROLE");
// admin role for RDNPOS contract
bytes32 public constant ACTIVEUNTILLUPDATE_ROLE = keccak256("ACTIVEUNTILLUPDATE_ROLE");
// admin role fore points rate updating
bytes32 public constant POINTSRATEUPDATE_ROLE = keccak256("POINTSRATEUPDATE_ROLE");
// admin role for RDNDistributors configuration
bytes32 public constant SETDISTRIBUTOR_ROLE = keccak256("SETDISTRIBUTOR_ROLE");
// admin role for adding custom users
bytes32 public constant ADDUSERBYADMIN_ROLE = keccak256("ADDUSERBYADMIN_ROLE");
```


Rename SETLEVEL_ROLE to SET_LEVEL_ROLE and update references accordingly for better readability.

##### Status.
NEW


______

#### LOW-53.

At

- RDNRegistry.sol:20
```solidity=20
// admin role for RDNPOS contract
```


- RDNRegistry.sol:22
```solidity=22
// admin role for RDNPOS contract
```


unclear comment

##### Status.
NEW


______

#### LOW-54.

At

- RDNRegistry.sol:32
```solidity=32
mapping (address => uint) public userId;
```


- RDNRegistry.sol:34
```solidity=34
User[] public users;
```


- RDNRegistry.sol:36-37
```solidity=36
uint public usersCount;
mapping(uint => uint[]) public children;
```


- RDNRegistry.sol:32
```solidity=32
mapping (address => uint) public userId;
```


- RDNRegistry.sol:40-41
```solidity=40
mapping(uint => mapping(address => bool)) public changeAddressAccess;
mapping(uint => address[]) public changeAddressAddresses;
```


Consider declaring variables like userId, users, and children as internal to avoid having duplicate getters.

##### Status.
NEW


______

#### LOW-55.

At

- RDNRegistry.sol:35-36
```solidity=35
// gas saving counter
uint public usersCount;
```


it's saves nothing, array length is places at the storage slot of the array pointer and cheap to SLOAD (the same cheap as sload usersCount), see e.g. https://mixbytes.io/blog/collisions-solidity-storage-layouts

##### Status.
NEW


______

#### LOW-56.

At

- RDNRegistry.sol:37
```solidity=37
mapping(uint => uint[]) public children;
```


maybe use EnumerableSet for cheap checks for "consist" in future in case if you will need it

##### Status.
NEW


______

#### LOW-57.

At

- RDNRegistry.sol:41
```solidity=41
mapping(uint => address[]) public changeAddressAddresses;
```


never used for any logic, consider removal

##### Status.
NEW


______

#### LOW-58.

At

- RDNRegistry.sol:44
```solidity=44
address public factorsAddress;
```


this variable is never used in the scope of this audit

##### Status.
NEW


______

#### LOW-59.

At

- RDNRegistry.sol:46
```solidity=46
// token => rate (rate is 1/USDprice for token, based in token.decimals)
```


the comment is unclear, what rate is it

##### Status.
NEW


______

#### LOW-60.

At

- RDNRegistry.sol:47
```solidity=47
mapping (address => uint) public pointsRate;
```


this is never used in the scope of this audit for any logic

##### Status.
NEW


______

#### LOW-61.

At

- RDNRegistry.sol:58
```solidity=58
// when users activeUntill updated
```


everywhere in project fix typo in word "until"

##### Status.
NEW


______

#### LOW-62.

At

- RDNRegistry.sol:71-72
```solidity=71
* @param _root: userAdrress for userId = 1
* @param _admin: default admin
```


typo in word address

##### Status.
NEW


______

#### LOW-63.

At

- RDNRegistry.sol:71
```solidity=71
* @param _root: userAdrress for userId = 1
```


NatSpec comments do not need ":"

##### Status.
NEW


______

#### LOW-64.

At

- RDNRegistry.sol:80
```solidity=80
//add root user (userId 1), referencing parantId=0.
```


typo in word parentId

##### Status.
NEW


______

#### LOW-65.

At

- RDNRegistry.sol:81
```solidity=81
User memory _rootUser = User(12, _root, 0, 7, block.timestamp + 36500 days, block.timestamp);
```


what does the magic value "12" and "7" mean?

##### Status.
NEW


______

#### LOW-66.

At

- RDNRegistry.sol:117
```solidity=117
emit UserAddressChanged(_userId, _newAddress, msg.sender, users[_userId].userAddress);
```


tiny gas optimisation is possible

##### Status.
NEW


______

#### LOW-67.

At

- RDNRegistry.sol:126-127
```solidity=126
function grantChangeAddressAccess(address _grantedAddress) public onlyRegisteredByAddress(msg.sender) {
    uint _userId = userId[msg.sender];
```


gas optimization possible, userId sload twice (inside modifier and below)

##### Status.
NEW


______

#### LOW-68.

At

- RDNRegistry.sol:158
```solidity=158
factorsAddress = _factorsAddress;
```


maybe emit event here

##### Status.
NEW


______

#### LOW-69.

At

- RDNRegistry.sol:177
```solidity=177
distributors[_token] = _distributor;
```


emit event

##### Status.
NEW


______

#### LOW-70.

At

- RDNRegistry.sol:205
```solidity=205
* @notice sender must have 3 roles
```


list all required roles

##### Status.
NEW


______

#### LOW-71.

At

- RDNRegistry.sol:208
```solidity=208
(uint _userId) = _addUser(_userAddress, _parentId);
```


brackets are not needed

##### Status.
NEW


______

#### LOW-72.

At

- RDNRegistry.sol:211
```solidity=211
setLevel(_userId, _level);
```


maybe emit specific event to note that the user was added by admin

##### Status.
NEW


______

#### LOW-73.

At

- RDNRegistry.sol:222
```solidity=222
User memory _user = User(1, _userAddress, _parentId, 0, 0, block.timestamp);
```


better to use User({level: 1, userAddress: _userAddress, ...}) for readability and decreasing the chance to misuse an argument

##### Status.
NEW


______

#### LOW-74.

At

- RDNRegistry.sol:224-228
```solidity=224
usersCount += 1;
userId[_userAddress] = usersCount - 1;
children[_parentId].push(usersCount - 1);
emit UserAdded(usersCount - 1, _parentId, _userAddress);
return (usersCount - 1);
```


SLOAD(usersCount) called 5 times, it's inefficient, consider to put it to stack

##### Status.
NEW


______

#### LOW-75.

At

- RDNRegistry.sol:238
```solidity=238
require(isRegisteredByAddress(_userAddress), "user not registered");
```


gas optimisation is possible, instead of using this check as modifier and then get userId you can write getUserIdByAddressSafe reverting if user does not exist, and use it at the beginning of functions

##### Status.
NEW


______

#### LOW-76.

At

- RDNRegistry.sol:242
```solidity=242
modifier onlyRegistered(uint _userId) {
```


this modifier is never used in the scope of this audit

##### Status.
NEW


______

#### LOW-77.

At

- RDNRegistry.sol:247
```solidity=247
modifier onlyValidUser(uint _userId) {
```


it's not clear what does it mean "valid" what is the difference with onlyRegisteredByAddress, add comment to explain

##### Status.
NEW


______

#### LOW-78.

At

- RDNRegistry.sol:330
```solidity=330
function getChildren(uint _userId) public view returns(uint[] memory) {
```


check if _userId is valid

##### Status.
NEW


