WOOFi Swap Audit

###### tags: `Final Report` WOOFi Swap Audit === > Copyright © 2021 by Verilog Solutions. All rights reserved. > October 17, 2021 > by **Verilog Solutions**  ![WOOFI-SWAP-COVER](https://hackmd.io/_uploads/ByJvtnOX9.png) This report presents our engineering engagement with WOO Network on their first DeFi protocol WOOFi Swap. Unlike popular Automated Market Making (AMM) or Proactive Market Making (PMM), WOOFi Swap’s **Sythethetic Proactive Market Making (sPMM)** is a brand new market-making algorithm that can successfully solve the slippage issue in Decentralized Exchange (DEX) by simulating order book structure in Centralized Exchange (CEX). --- ## Table of Content [TOC] --- ## Project Summary WOOFi Swap is built and designed by the WOO Network to improve on-chain liquidity and capital efficiency. Under the current EVM development environment, market making is a very challenging problem that requires the design of the system to make trade-offs on various aspects (e.g slippage, market depth) for operation. Due to their abundant experience in centralized exchange market making, WOO Network team has the insight to propose a solution for DEX by using the WOO Network oracle (a.k.a. Wooracle) to simulate the market depth of the CEX on a DEX. Similar to PMM, the Wooracle will feed the DEX with the latest price traded on WOO CEX, the DEX will also load the Chainlink oracle price feed as a cross-reference to make sure the price feed is correct and decentralized. Besides the price feed, WOO Network team tunes the parameters in their sPMM algorithm to enable their DEX to successfully simulate the market depth and slippage for Defi users. Those parameters are calculated off-chain by the quantitative analysis team and tested by the WOO Network team off-chain. In the current smart contract, WOOFi Swap does not allow users to create trading pairs and provide liquidity. Instead, the tokens in DEX are initially deposited by the dedicated market maker (i.e. Kronos Research). In general, the idea behind WOOFi swap is to provide users a closer performance to CEX slippage trading on DEX. Indubitably, WOO Network innovation with sPMM is a valuable differentiator since no other exchanges offer this unique feature in the current market. If executed thoughtfully, Verilog Solutions believes WOOFi Swap has the potential to attract major market share through the sheer product-market fit of their low slippage offerings. --- ## Service Scope Our review focused on the [**main** branch](https://github.com/woonetwork/woofi_swap_smart_contracts), specifically, commit hash [**57d7f2eb2230360a009461051235ad3ecf16f707**](https://github.com/woonetwork/woofi_swap_smart_contracts/tree/57d7f2eb2230360a009461051235ad3ecf16f707). Our auditing service with WOO Network will include the following two stages: - Pre-Audit Consulting Service - Audit Service 1. **Pre-Audit Consultancy Service** Initially, Verilog Solutions conducted due diligence and inspections of the product's current state with our full team of smart contract auditors. Verilog Solutions then developed a high-level understanding of the project and its required testing methods. After, we proposed to change from the Truffle framework to the Hardhat framework which has better testing support and community recognition. Indeed, some testing methods can only be done by using hardhat which is another strong reason why the framework shift is necessary. Besides the framework, our team provides many smart contract development conventions. For example, Verilog Solutions has proposed new insights on how to refactor smart contracts as well as their structures. With long-term growth in mind, our proposal helps future researchers and developers to continue studying and proposing new models for WOOFi Swap. 2. **Smart Contract Auditing Service** In total, the audit service between WOOFi Swap and the Verilog Solutions engineering team lasted 2 weeks. The Verilog Solutions engineering team studied the entire project using a detailed-oriented approach to capture fundamentals and proposed meaningful changes to existing code. Details can be found under [Findings & Improvement Suggestions](#Findings-amp-Improvement-Suggestions). --- ## WOOFi Swap Architecture <figure> <center> <img src="https://i.imgur.com/DnhsN8Y.png" width="600" alt="WOOF Swap Architecture"> <center> WOOFi Swap Architecture</center></center> </figure> There are four major smart contracts in WOOFi Swap: - `Wooracle.sol` - `WooPP.sol` - `WooRouter.sol` - `WooGuardian.sol` 1. `Wooracle.sol` `Wooracle.sol` is an oracle contract managed and developed by the WOO Network team. The WOO Network team uploads their CEX data to the oracle contract to get the latest trading price, middle price, and market spread information on CEX. 2. `WooPP.sol` `WooPP.sol` is the basic contract that handles the logic operation of DEX, including setting the token info, calculating slippage, calculating the exchange amount, and so on. This contract will store all tokens that are supported by trading on the WOOFi Swap. In this contract it defines quote token and base token: - Quote Token: the reference token in the contract, only have one. - Base Token: can be added by the strategist from the WOO Network team and can be more than one. 3. `WooRouter.sol` `WooRouter.sol` is the router contract. The general public will only need to approve this contract to spend their ERC20. Similar to Uniswap, approval is required to interact with any trading pair on this contract. The router contract will interact with the lower layer `WooPP.sol` to execute the sell logic and send back the users desired amount of tokens. Router contracts can also interact with 3rd party DEX when the market-making strategy is on pause or the asset is not available for direct exchange on WOOFi Swap. 4. `WooGuardian.sol` `WooGuardian.sol` is the smart contract that provides the tolerance threshold of token prices given by the oracle. For the swap amounts and prices given by the oracle, the functions (_i.e._, `checkSwapAmount()` and `checkSwapPrice()`) in this smart contract are used to check if the swapping amounts and swapping prices are within the thresholds or not. <figure> <center> <img src="https://user-images.githubusercontent.com/26049843/137649878-45348e72-fb37-465e-acf7-147408514731.png" width="350" alt="Swap Logic"> <center> WOOFi Swap Logic</center></center> </figure> In `WooPP.sol`, the project team will set their desired Quote Token (e.g USDC), and multiple Base Tokens (e.g wBTC, wETH ...). There are two routines as indicated in the above pictures. 1. From base token to quote token 2. From one base token to another base token. In short, if a user wants to swap from wBTC (Base Token) to USDC (Quote Token), the WOOFi Swap will swap these two directly. If a user wants to swap from wBTC (Base Token) to wETH (Base Token), the WOOFi Swap will swap wBTC to quote token and then from quote token to wETH. However, when the market-making strategy is put on pause due to various reasons (e.g. maintenance) or the asset is not available, WOOFi Swap will redirect the trade to 3rd party DEX. **The `WooRouter.sol` UML (after pre-audit & audit)**: <figure> <center> <img src="https://i.imgur.com/OoPUmMY.png" width="600" alt="WooRouter.sol UML"> WooRouter.sol UML </center> </figure> **The `WooPP.sol` UML (after pre-audit & audit)**: <figure> <center> <img src="https://i.imgur.com/OVwO5CD.png" width="700" alt="WooPP.sol UML"> <center> WooPP.sol UML</center></center> </figure> **The `Wooracle.sol` UML (after pre-audit & audit)**: <figure> <center> <img src="https://i.imgur.com/82k1gm0.png" height="500" alt="Wooracle UML"> </center> <center>Wooracle UML</center> </figure> **The `WooGuardian.sol` UML (after pre-audit & audit)**: <figure> <center> <img src="https://i.imgur.com/3AjxJpi.png" height="350" alt="WooGuardian UML"> </center> <center>WooGuardian UML</center> </figure> --- ## Privilege Roles There are two main privilege roles: `owner` and `strategist`. 1. **`owner`** - manage the ownership of WooRouter, WooPP, Wooracle - set new pool for WooRouter - add and remove whitelist address - pause and unpause the following functions in WooPP - `querySellBase` - `querySellQuote` - `sellBase` - `sellQuote` - withdraw ERC20 tokens from WooRouter and WooPP - set all important variables in Wooracle 2. **`strategist`(including `owner`)** - add and remove strategist - set price oracles for WooPP - set rewardManager - add and remove baseToken at WooPP - tune parameters of baseToken and quoteToken NOTE: Also there is a `rewardManager` contract, which will receive the earned lp fee when users interact with the protocol. Given that the WOO Network team has control over these privilege roles, we suggested for them to transfer the `owner` to a multisig wallet controlled by the WOO Network team. Since users will not be able to join as liquidity providers, the current version allows the WOO Network team to deposit assets, set base tokens, and shoulder potential risks of being arbitraged. In the smart contract, there is a price feed comparison between Chainlink Oracle and Wooracle. Verilog Solutions auditors believe that if WOO Network team can properly manage the price feed oracle and gnosis wallet to tune the parameters of the DEX, users will have a smooth experience reaping benefits similar to what is offered through CEX trading with a better slippage. --- ## Findings & Improvement Suggestions InformationalMinorMediumMajorCritical | | Total | Acknowledged | Resolved | | ------------- | ----- | ------------ | -------- | | Critical | 2 | 2 | 2 | | Major | 0 | 0 | 0 | | Medium | 1 | 1 | 0 | | Minor | 12 | 12 | 12 | | Informational | 4 | 4 | 4 | ### Critical 1. `<address>.transfer` is used for transferring eth. (`WooRouter`: [L122](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L122), [L141](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L141), [L259](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L259)) Critical ```solidity if (isToETH) { realToAmount = pool.sellQuote(toToken, fromAmount, minToAmount, address(this), address(this), rebateTo); IWETH(_WETH_ADDRESS_).withdraw(realToAmount); to.transfer(realToAmount); } else { realToAmount = pool.sellQuote(toToken, fromAmount, minToAmount, address(this), to, rebateTo); } ``` **Description**: Transfer forwards a fixed amount of gas(e.g. 2300). Gas cost is subject to change and the smart contract should not depend on any particular gas cost. It is important to note that sending Ether can also fail because the execution of the recipient contract requires more than the allotted amount of gas. **Recommendation**: Use `call()` with the reentrancy guard and check the return state. **Result**: Resolved. 1. Avoid using `selfdestruct`(`WooRouter`: [L284-L286](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L284-L286))Critical ```solidity function destroy() external onlyOwner { selfdestruct(msg.sender); } ``` **Description**: WOOFi Swap was originally designed to use `selfdestruct` function to disable the `WooRouter`. However, even if a contract is removed by `selfdestruct`, it is still part of the history of the blockchain and probably retained by most Ethereum nodes. So using selfdestruct is not the same as deleting data from a hard disk. Besides, even if a contract’s code does not contain a call to selfdestruct, it can still perform that operation using delegatecall or callcode. **Recommendation**: Pause or disable the functions in the contract instead of using `selfdestruct`. When users are sending `eth` or tokens to the contract, their transactions will be reverted instead of losing their money. **Result**: Resolved in commit [b042a36301acff42e76aaa1ff12c8f5e4e8a353b](https://github.com/woonetwork/woofi_swap_smart_contracts/commit/b042a36301acff42e76aaa1ff12c8f5e4e8a353b).  ### Medium 1. Check actual token amount received after `transferFrom` (WooPP: [L343](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L343), [L373](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L373))Medium ```solidity IERC20(baseToken).safeTransferFrom(from, address(this), baseAmount); ``` ```solidity IERC20(quoteToken).safeTransferFrom(from, address(this), quoteAmount.add(lpFee)); ``` **Description**: All function logic related to deposit, should have a secondary check after `transferFrom`. Some tokens will deduct some fees when transferring. This case might not happen for most tokens, but it is a safer practice to add another layer of protection to the design. **Recommendation**: Have a secondary check after `transferFrom`. **Result**: Talked with WOOFi Swap team, and confirmed only the mainstream tokens (e.g. btc, eth, bnb, etc) will be added. The WOOFi team will further make sure all the base/quote tokens are non-burnable tokens in transfer functions when added into WooPP. ### Minor 1. The `quoteToken` is not immutable in `WooPP` (`WooPP`: [L72](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L72))Minor ```solidity address public quoteToken; ``` **Description**: As a variable that does not change after initialization in the constructor, the `immutable` keyword should be used to protect the variable from being modified after initialization. **Recommendation**: Use the `immutable` keyword. **Result**: Resolved. 7. `swap` function: Adding the requirement for non-zero address to `fromToken`, `toToken`, `to`, `rebateTo` (WooRouter: [L97-L103](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L97-L103)) Minor ```solidity function swap( address fromToken, address toToken, uint256 fromAmount, uint256 minToAmount, address payable to, address rebateTo ) external payable returns (uint256 realToAmount) {...} ``` **Description**: In `WooRouter.sol`, adding require statement for the input arguments `fromToken`, `toToken`, `to`, `rebateTo`. **Recommendation**: Adding the requirement for non-zero address to `fromToken`, `toToken`, `to`, `rebateTo`. **Result**: Resolved in commit. [82d25b10cd7c6358ab7cf22296c01b58840ecced](https://github.com/woonetwork/woofi_swap_smart_contracts/commit/82d25b10cd7c6358ab7cf22296c01b58840ecced). 8. `externalSwap` function: Adding the requirement for non-zero address to `approveTarget`, `swapTarget`, `fromToken`, `toToken`, `to`; (`WooRouter`: [L201-L207](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L201-L207)) Minor ```solidity function externalSwap( address approveTarget, address swapTarget, address fromToken, address toToken, uint256 fromAmount, address payable to, bytes calldata data ) external payable nonReentrant {...} ``` **Description**: In `WooRouter.sol`, no non-zero address check for arguments `approveTarget`, `swapTarget`, `fromToken`, `toToken`, `to` of function `externalSwap`. **Recommendation**: Including the requirement for non-zero address to the arguments `approveTarget`, `swapTarget`, `fromToken`, `toToken`, `to` of function `externalSwap`. **Result**: Resolved in commit. [82d25b10cd7c6358ab7cf22296c01b58840ecced](https://github.com/woonetwork/woofi_swap_smart_contracts/commit/82d25b10cd7c6358ab7cf22296c01b58840ecced). 9. `setWhiteListed`: Adding the requirement for non-zero address to target; (`WooRoute`: [L274](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L274))Minor ```solidity function setWhitelisted(address target, bool whitelisted) external onlyOwner { isWhitelisted[target] = whitelisted; } ``` **Description**: In `WooRouter.sol`, no non-zero address check for argument `target` of function `setWhiteListed`. **Recommendation**: Adding the requirement for non-zero address to argument `target` of function `setWhiteListed`. **Result**: Resolved in commit [82d25b10cd7c6358ab7cf22296c01b58840ecced](https://github.com/woonetwork/woofi_swap_smart_contracts/commit/82d25b10cd7c6358ab7cf22296c01b58840ecced). 10. Add non-zero address check for address arguments in other solidity files including libraries. (`WooPP`: [L143](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L143), [L248-249](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L248), [L287](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L287), [L326](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L326), [L329-331](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L329), [L356](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L356), [L359-361](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L359), [L385](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L385), [L398](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L398), [L411](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L411), [L421](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L421), [L437](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L437), [L443](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L443), [L498](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L498), [L523](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L523), [L529-530](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L529), [L537](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L537))Minor ```solidity function autoUpdate( address baseToken, TokenInfo memory baseInfo, TokenInfo memory quoteInfo ) internal view {...) ``` **Description**: No non-zero address check for `address` argument. **Recommendation**: Add zero address check for `address` argument in other solidity files including libraries. **Result**: Resolved in commit. [82d25b10cd7c6358ab7cf22296c01b58840ecced](https://github.com/woonetwork/woofi_swap_smart_contracts/commit/82d25b10cd7c6358ab7cf22296c01b58840ecced). 19. Use of actual uint type instead of `uint256` for arguments `newThreshold`, `newLpFeeRate` and `R` in `tuneParameters` (`WooPP`: [L499-L501](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L499-L501))Minor **Description**: `uint256` is used for the arguments type to input parameters `newThreshold`, `newLpFeeRate` and `R` in `tuneParameters` and convert to their actual uint type inside the function. **Recommendation**: Use the actual uint type instead. **Result**: Not Resolved. 5. `WooPP_proxy.sol` is a flattened file ([`WooPP_proxy.sol`](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP_proxy.sol))Minor **Description**: Since the proxy contract of `WooPP.sol`, the file was flattened, we suggested the engineering team of Wootrade to rearrange it. **Recommendation**: Refactor the `WooPP_proxy.sol`. **Result**: WOOFi team decided no longer to use proxy contracts as an upgradeable method. So this issue has been resolved. Besides, all liquidity is provided by WOOFi team themselves, so `WooPP.sol` can be replaced and changed to the new one without a need for an upgradable contract. 4. The `quoteToken` is not immutable in `WooPP` (`WooPP`: [L72](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L72))Minor ```solidity address public quoteToken; ``` **Description**: As a variable that does not change after initialization in the constructor, the `immutable` keyword should be used to protect the variable from being modified after initialization. **Recommendation**: Use the `immutable` keyword. **Result**: Resolved. 17. Add `require` for `msg.sender` at the `receive` function in `WooRouter`(`WooRouter`: [L79](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L79))Minor ```solidity receive() external payable {} ``` **Description**: No `require` at `receive` function which means anyone can send `eth` to the contract address and the `eth` cannot be rescued. Adding `require` for the `msg.sender`to only allow `WETH` contract and `whitlisted` contracts to send `eth`. **Recommendation**: Add require for `msg.sender` at the `receive` function. **Result**: Resolved. 18. Require `postBalance` to be bigger than `preBalance` at `externalSwap` (`WooRouter`: [L201-L227](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L201-L227))Minor **Description**: There is a subtraction `postBalance.sub(preBalance)` at the event emit of `externalSwap`. The function will be reverted if an overflow happens. Adding a require`postBalance > preBalanace` is recommended. **Recommendation**: Adding a require`postBalance > preBalanace`. **Result**: Resolved. 12. `internal` functions can be changed to `private` functions(`WooPP`: [L146](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L146), [L172](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L172), [L185](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L185), [L198](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L198), [L211](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L211), [L224](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L224), [L252](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L252), [L291](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L291), [L547](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L547))Minor **NOTE**: Gas Saving Recommendation ```solidity function autoUpdate( address baseToken, TokenInfo memory baseInfo, TokenInfo memory quoteInfo ) internal view {...} ``` **Description**: `internal` functions can be accessed within the contract and the contracts deriving from. `private` functions instead can only be accessed within the contract. In `WooPP.sol`, some functions are designed to serve the computation purposes within the `WooPP.sol`. Thus, `private` function is recommended. **Recommendation**: In `WooPP.sol`, change `internal` functions to `private` functions. ```solidity function autoUpdate( address baseToken, TokenInfo memory baseInfo, TokenInfo memory quoteInfo ) private view {...} ``` **Result**: Resolved. 13. `public` functions can be changed to `external` functions (`WooPP`: [L448](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L448),[L478](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L478),[L502](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L502))Minor **NOTE**: Gas Saving Recommendation ```solidity function addBaseToken( address baseToken, uint256 threshold, uint256 lpFeeRate, uint256 R, address chainlinkRefOracle ) public nonReentrant onlyStrategist { .... } ``` **Description**: `Public` functions are part of the contract interface and can be either called internally or via messages. `external` functions are part of the contract interface, which means they can be called from other contracts and via transactions. Besides, `public` and `external` differ in terms of gas usage. The former uses more than the latter when applied to large arrays of data. **Recommendation**: Change the function above from `public` to `external` to save gas. ```solidity function addBaseToken( address baseToken, uint256 threshold, uint256 lpFeeRate, uint256 R, address chainlinkRefOracle ) external nonReentrant onlyStrategist { .... } ``` **Result**: Resolved. ### Informational 1. No `indexed` parameters in the events defined in `WooPP` and `WooRouter` (`WooRouter`: [L67](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L67), [L77]((https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L77)); `WooPP`: [L61](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L61))Informational ```solidity event WooRouterSwap( SwapType swapType, address fromToken, address toToken, uint256 fromAmount, uint256 toAmount, address from, address to ); ``` **Description**: The indexed parameters can be used as filters or queried for the logged events. There are no indexed parameters in events in WooPP. **Recommendation**: Add `indexed` keyword to parameters in the events. **Result**: Resolved. 6. Require message reformat (`WooRouter`: [L82](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L82), [L83](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L83), [L90](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L90), [L111](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L111), [L236](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L236), [L238](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L238), [L245](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L245), [L249](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooRouter.sol#L249); `WooPP`: [L66](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L66), [L110](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L110), [L111](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L111), [L112](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L112), [L126](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L126), [L149](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L149), [L150](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L150), [L258](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L258), [L297](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L297), [L334](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L334), [L342](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L342), [L364](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L364), [L372](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L372), [L387](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L387), [L395](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L395), [L400](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L400), [L408](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L408), [L416](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L416), [L423](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L423), [L431](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L431), [L449-452](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L449), [L455](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L455), [L469](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L469), [L480](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L480), [L503-505](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L503), [L508](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L508), [L551](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L551), [L553](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L553), [L557](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L557))Informational ```solidity constructor( address _quoteToken, address _pool ) public { require(_quoteToken != address(0), 'INVALID_QUOTE'); require(address(_pool) != address(0), 'Pool address cannot be 0'); quoteToken = _quoteToken; pool = IWooPP(_pool); emit PoolChanged(_pool); } ``` **Description**: In `WooRouter.sol` / `WooPP.sol`, the required message should be clearly distinguished and have a uniform format. The format below is what we suggest instead: ``` WooRouter: ZERO ADDRESS ｜ | Filename : capitalized warning message ``` This method allows engineers to clearly distinguish which module in the architecture is causing the problem. **Recommendation**: Change `WooRouter.sol` and `WooPP.sol` require message to the above format. **Result**: Resolved in commit [82d25b10cd7c6358ab7cf22296c01b58840ecced](https://github.com/woonetwork/woofi_swap_smart_contracts/commit/82d25b10cd7c6358ab7cf22296c01b58840ecced). 16. Create interface for `WooPP` and `WooRouter`. **Description**:`interface` provides a clear structure of the code and makes it easy for others to interact with the contract **Recommendation**: Creating interface for `WooPP` and `WooRouter`. **Result**: Resolved. 14. Using `delete` in `removeBaseToken` (`WooPP`: [L478](https://github.com/woonetwork/woofi_swap_smart_contracts/blob/57d7f2eb2230360a009461051235ad3ecf16f707/contracts/WooPP.sol#L478))Informational ```solidity function removeBaseToken( address baseToken ) public nonReentrant onlyStrategist { TokenInfo memory info = tokenInfo[baseToken]; require(info.isValid, 'TOKEN_DOES_NOT_EXIST'); info.reserve = 0; info.threshold = 0; info.lastResetTimestamp = 0; info.lpFeeRate = 0; info.R = 0; info.target = 0; info.isValid = false; info.chainlinkRefOracle = address(0); info.refPriceFixCoeff = 0; tokenInfo[baseToken] = info; emit ParametersUpdated(baseToken, 0, 0, 0); emit ChainlinkRefOracleUpdated(baseToken, address(0)); } ``` **Description**: `WooPP.removeBaseToken` is designed to remove stored `baseToken`. The struct value of `baseToken` is reset to `zero` here. **Recommendation**: There are too many members inside the struct and some might be left out when resetting those values. `delete` is recommended to reset the struct values here. Replace the above code with the below format: ```solidity delete tokenInfo[baseToken]; ``` **Result**: Resolved. --- ## Disclaimer Verilog Solutions 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 Solutions in no way claims any guarantee of security or functionality of the technology we agree to analyze. In addition, Verilog Solutions 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 Solutions has the right to distribute the Report through other means, including via Verilog Solutions publications and other distributions. Verilog Solutions 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.