Sakaivault - Audit

`SAKAI` token charges 3% buy/sell fee, and the 5/6 of the fee is collected by `treasuryWallet` and `gasWallet`. If these fees don't flow back to the users in some way, eventually all the SAKAI token would end up in the contract owner's wallet. We'd like to confirm if there's a way for the users/token holders to benefit in the tokenomics by design.

The `stakingToken` should not be deflationary token. When transferring deflationary ERC20 tokens, the input amount may not be equal to the received amount due to the charged transaction fee. For example, if a user sends 100 deflationary tokens (with a 10% transaction fee), only 90 tokens actually arrived to the contract. However, a failure to discount such fees may allow the same user to withdraw 100 tokens from the contract, which causes the contract to lose 10 tokens in such a transaction. Reference: https://thoreum-finance.medium.com/what-exploit-happened-today-for-gocerberus-and-garuda-also-for-lokum-ybear-piggy-caramelswap-3943ee23a39f Specifically in `Sakaivault`, in function `deposit()`, the `safeTransferFrom()` amount is the same as the recorded stake amount. But for deflationary tokens, the actual received amount might be less. ```solidity=1617 ERC20token.safeTransferFrom(msg.sender, address(this), amount); ``` ```solidity=1643 _stakers[msg.sender].balance += amount; ``` On the other hand, function `withdraw()` still transfers out the recorded staked balance. ```solidity=1657 IERC20 ERC20token = IERC20(stakingToken); ERC20token.safeTransfer(msg.sender, balance); totalAddressStaked -= 1; totalStakedToken -= balance; ```

A sandwich attack may happen when an attacker observes a transaction swapping tokens or adding liquidity without setting restrictions on slippage or minimum output amount. The attacker can manipulate the exchange rate by frontrunning (executing before the target) a transaction to purchase one of the assets and make profits by backrunning (executing after the target) a transaction to sell the asset. The following functions are called without setting restrictions on slippage or minimum output amount, so transactions triggering these functions are vulnerable to sandwich attacks, especially when the input amount is large: - `swapExactTokensForTokensSupportingFeeOnTransferTokens()` - `swapExactTokensForETHSupportingFeeOnTransferTokens()` This poses as an issue since the `manualSwapBack()` function is callable by anyone. It might be possible for an attacker to manipulate the price of the SAKAI-USDT and/or SAKAI-WETH pool using flashloan, and then call `manualSwapBack()`. This would result in less USDT for `treasuryWallet` and less WETH for `gasWallet`.

1. Expected total reward for a user is not recorded in the contract. As indicated by the code, the stakers should receive a fixed-rate daily interest of 0.8%. If a user makes two 100 token deposits on different days, the reward amount should be different for the 2 deposits, because of the different stake time. But only the deposited balance and the time of the first deposit are recorded in the `Staker` struct. This means that there would be no way to calculate the accurate amount of expected total rewards for a user, solely from the data in this contract. We'd like to confirm if this is working as designed. 2. The daily interest rate in the contract is 0.8%, and the allocated reward is calculated based on it. But the actual reward amounts handed out to stakers are decided by `backendWallet` when it calls `claimReward()` function without limitation on the amounts. We'd like to confirm if this is working per design. 3. The stakers cannot withdraw token before `endTime`, while the rewards can be handed out before `endTime`. We'd like to confirm if this is intentional. 4. When `claimRewards()` function is called, the reward recipient is not validated against `_stakers`. This means that the `backendWallet` could allocate rewards to any address, regardless of whether it has a valid staking record. This could be volatile. For example, if the `backendWallet` accidentally calls `claimRewards()` twice with the same input, the code may not revert, and the reward could be handed out twice. On the other hand, the current code allows users to withdraw the staked tokens before reward is handed out, and the user's staking record will be deleted after withdrawal. This makes it necessary to allow for reward distribution to addresses without staking records. We'd like to point out the potential risk in the current design and confirm if this is intended. Resolution of this issue may need a change in design.

In contract `SAKAI` and `SakaiStaking`, the following addresses are hardcoded, and some of them cannot be updated after deployment. These addresses appear to be in BSC testnet instead of mainnet. - SAKAI contract ``` solidity=1379 address public treasuryWallet = 0x6b269c07e8f94f0fa1769CBD362879AFeA0206dB; address public stakingContract = 0x7BC537e26478789d3709cD68972C6A6DC6611Cde; address public gasWallet = 0x9Ffd803BD8048Ed59Fccb73d688326d7085A4e54; ``` - SakaiStaking contract ```solidity=1363 address public backendWallet = 0x9Ffd803BD8048Ed59Fccb73d688326d7085A4e54; address public treasuryWallet = 0x6b269c07e8f94f0fa1769CBD362879AFeA0206dB; ```

Following third-party dependency exists with scope of audit. - Contract `SAKAI` and `SakaiStaking` are serving as the underlying entity to interact with third-party DEX. - The reward distribution in `SakaiStaking` contract is dependent on the external backend wallet to correctly calculate and distribute. The scope of the audit treats third-party entities as black boxes and assumes their functional correctness. However, in the real world, third parties can be compromised and this may lead to lost or stolen assets.