YuzuSwap Staking Contracts Audit

###### tags: `Final Report` YuzuSwap Staking Contracts Audit === > Copyright © 2022 by Verilog Solutions. All rights reserved. > March 30, 2022 > by **Verilog Solutions**  ![Yuzu-SWAP-COVER](https://hackmd.io/_uploads/HkwGO3O79.png) This report presents our second engineering engagement with YuzuSwap, one of the first DEX projects for the Emerald paratime on the Oasis Network. YuzuSwap is an AMM DEX with innovative trading incentive designs, such as the **trading pool share token (TPST)**. After a successful launch of YuzuSwap on the Oasis Emerald chain, the YuzuSwap team asked Verilog to audit their newest feature, single token staking contracts. --- ## Table of Content [TOC] --- ## Project Summary YuzuSwap is a decentralized exchange on the Oasis Emerald paratime that includes incentive programs such as liquidity mining and trade mining. YuzuSwap follows a non-custodial, peer-to-peer, automated-market-maker model for swapping tokens within the Oasis ecosystem. The YuzuSwap platform itself is fully open to developers and members of the Yuzu DAO. ___ ## Service Scope Our review focused on the [**main** branch](https://github.com/Yuzu-swap/yuzuswap-contract), specifically, commit hash [**48aaf981151f52b639a88b5b6f2ef788c84c20ca**](https://github.com/Yuzu-swap/yuzuswap-contract/tree/48aaf981151f52b639a88b5b6f2ef788c84c20ca). Our second auditing service for YuzuSwap includes the following two stages: - Audit Service - Testing Service 1. **Audit Service** The Verilog team conducted a thorough study of the YuzuSwap staking contract code. The list of findings, along with the severity and solution, is available under the section [**Findings & Improvement Suggestions**](#Findings-amp-Improvement-Suggestions). 2. **Testing Service** The Verilog team conducted thorough testing of the YuzuSwap staking contract(YuzuStake.sol). Introduced new testing frameworks and new testing methods to the contract repo, as well as very detailed testing. Details can be found in the following PR: https://github.com/Yuzu-swap/yuzuswap-contract/pull/1 --- ## Privileged Roles 1. owner a. `setConfig()` b. `addConfig()` The privileged roles in this smart contract can set and add config rules for staking. `setConfig()` can update staking config such as staked last block count and xYUZU mint ratio. `addConfig()` can add staking config such as staked last block count and xYUZU mint ratio. --- ## Findings & Improvement Suggestions #### <html></html> <style> .info { background-color:mediumseagreen; font-size: 12px; color: white; border-radius:4px; padding: 1px 4px; font-weight: 500; display: inline-block; margin: 2px; letter-spacing: 0.3px} </style><style> .minor { background-color: #698999; font-size: 12px; color: white; border-radius:4px; padding: 1px 4px; font-weight: 500; display: inline-block; margin: 2px; letter-spacing: 0.3px} </style><style> .medium { background-color: #FFCA0F; color: #121212; font-size: 12px; border-radius:4px; padding: 1px 4px; font-weight: 500; display: inline-block; margin: 2px; letter-spacing: 0.3px} </style><style> .major{ background-color: #FF6B4A; color: white; font-size: 12px; border-radius:4px; padding: 1px 4px; font-weight: 500; display: inline-block; margin: 2px; letter-spacing: 0.3px} </style><style> .critical{ background-color: #FF0000; color: white; font-size: 12px; border-radius:4px; padding: 1px 4px; font-weight: 500; display: inline-block; margin: 2px; letter-spacing: 0.3px} </style> <span class='info'>Informational</span><span class='minor'>Minor</span><span class='medium'>Medium</span><span class='major'>Major</span><span class='critical'>Critical</span> | | Total | Acknowledged | Resolved | | ------------- | ----- | ------------ | -------- | | Critical | 0 | 0 | 0 | | Major | 0 | 0 | 0 | | Medium | 0 | 0 | 0 | | Minor | 0 | 0 | 0 | | Informational | 8 | 8 | 5 | ### Critical none ; ) ### Major none ; ) ### Medium none ; ) ### Minor none ; ) ### Informational 1. `nonReentrant` function `stake()`, `withdraw()` call external function `yuzuTokenIns.safeTransferFrom()` (`YuzuStake.sol`: [L151]((https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L151)), [L237]((https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L237))) <span class='info'>Informational</span> **Description**: Functions marked as `nonReentrant` may not call one another. Given `yuzuTokenIns.safeTransferFrom()` is arbitrary, it is possible to have another `nonReentrant` modifier. **Recommendation**: Make `stake()`, `withdraw()` private (`_stake()`, `_withdraw()`), and then add external `nonReentrant` entry points. Or alert deployer not to connect custom ERC20 contract with `nonReentrant` functions. **Result**: Acknolwdged 2. `xYUZU` transfer issue <span class='info'>Informational</span> **Description**: transfer of `xYUZU` token to another address will cause `xYUZU` unable to be redeemed from the staking contract **Recommendation**: Verilog team will discuss this with YUZU team regarding whether this design has been done intentionally or not. If this feature is not the intended design, then the suggested change is to turn `xYUZU` token into a non-transferable token. **Result**: Discussed with YuzuSwap team, that this design is on purpose. Thus no actions are required. **For users, please be aware that you can only redeem your xYUZU to YUZU token with the initial staking address.** 3. `YuzuStake.sol` function `setConfig()` comment error(`YuzuStake.sol`: [115]((https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L115))) <span class='info'>Informational</span> **Description**: The line 115 of `YuzuStake.sol` function `setConfig()` comment is wrong **Recommendation**: change the comment to `precision ratio base` **Result**: Resolved in [PR](https://github.com/Yuzu-swap/yuzuswap-contract/pull/1) 4. `YuzuStake.sol` struct `StakeConfig` (`YuzuStake.sol`: [158]((https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L158))) <span class='info'>Informational</span> **Description**: The variable name is `ratioBase10000` while the comment specifies the value is based of 100,000. **Recommendation**: Please double-check the precision of the math calculation and make the variable name consistent with the comments. **Result**: Resolved in [PR](https://github.com/Yuzu-swap/yuzuswap-contract/pull/1) 5. `YuzuStake.sol` NatSpec comments wrong in function `setConfig()`, `addConfig()` <span class='info'>Informational</span> **Description**: The comments for `param` are wrong. **Recommendation**: Please update the comments **Result**: Resolved in [PR](https://github.com/Yuzu-swap/yuzuswap-contract/pull/1) 6. Lack of input check for `YuzuStake.addConfig`. (`YuzuStake.sol`: [131]((https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L131))) <span class='info'>Informational</span> **Description**: Lack of input check for `_blockCount` and `_ratioBase10000` in function `addConfig`. **Recommendation**: require `_blockCount` and `_ratioBase10000` non zero **Result**: Resolved in [PR](https://github.com/Yuzu-swap/yuzuswap-contract/pull/1) 7. Be careful about overflow when `stake` if the `stakeConfig.blockCount` is accidentally set to a very big number. (`YuzuStake.sol`: [L179](https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L179)) <span class='info'>Informational</span> **Description**: `currentBlock + stakeConfig.blockCount` might overflow if the `stakeConfig.blockCount` is accidentally set to a very big number. **Recommendation**: Be careful when setting or adding config. **Result**: Acknolwdged 8. Lack of indexed variables in events (`YuzuStake.sol`: [L70]((https://github.com/Yuzu-swap/yuzuswap-contract/blob/8b7a08837181a748d35a81f31f5d0b4b5ceb95d8/contracts_for_review/YuzuStake.sol#L70))) <span class='info'>Informational</span> **Description**: Lack of indexed variables (`oid`, `from`, `cid`) in events (`OrderCreated`, `OrderUnstaked`, `OrderWithdrawed`, `ConfigChanged`). **Recommendation**: Add `indexed` modifier to event parameters accordingly. **Result**: Acknolwdged ## Disclaimer Verilog receives compensation from one or more clients for performing the smart contract and auditing analysis contained in these reports. The report created is solely for Clients and published with their consent. As such, the scope of our audit is limited to a review of code, and only the code we note as being within the scope of our audit detailed in this report. It is important to note that the Solidity code itself presents unique and unquantifiable risks since the Solidity language itself remains under current development and is subject to unknown risks and flaws. Our sole goal is to help reduce the attack vectors and the high level of variance associated with utilizing new and consistently changing technologies. Thus, Verilog in no way claims any guarantee of security or functionality of the technology we agree to analyze. In addition, Verilog reports do not provide any indication of the technologies proprietors, business, business model, or legal compliance. As such, reports do not provide investment advice and should not be used to make decisions about investment or involvement with any particular project. Verilog has the right to distribute the Report through other means, including via Verilog publications and other distributions. Verilog makes the reports available to parties other than the Clients (i.e., “third parties”) – on its website in hopes that it can help the blockchain ecosystem develop technical best practices in this rapidly evolving area of innovation.