WOOFi Swap Audit === > Copyright © 2021 by Verilog. All rights reserved. > October 17, 2021 > by **Verilog Audit** <!-- <span style="position:fixed; top:200px; right:400px; opacity:0.5; font-size: 20px; z-index:99;">watermark</span> --> ![WOOFI-SWAP-COVER](https://i.imgur.com/Jjgfyav.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 orderbooks processes 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 which 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 an 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 Audit believes WOOFi Swap has the potential to attract major market share through sheer product market fit of their low slippage offerings. --- ## Service Scope The review was conducted over two weeks, from Oct 4, 2021 to Oct 18, 2021 by Verilog team. 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 conducted due diligence and inspections of the product current state with our full team of smart contract auditors. Verilog then developed a high level understanding of the project and its required testing methods. After, we proposed to change from truffle framework to 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 auditing has proposed new insights on how to refactor the 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 Verilog engineering team lasted 2 weeks. The Verilog engineering team studied the entire project using a detailed oriented approach to capture fundamentals and proposed meaningful changes on existing code. Details can be found under [Findings & Improvement Suggestions](#Findings-amp-Improvement-Suggestions). --- ## WOOFi Swap Architecture <br /> <figure> <center> <img src="https://i.imgur.com/DnhsN8Y.png" width="600" alt="WOOF Swap Architecture"> <center><em><br />WOOFi Swap Architecture</em></center></center> </figure> <br />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. Router contract will interact with the lower layer `WooPP.sol` to execute the sell logic and send back users desired amount of tokens back. 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. <br /> <figure> <center> <img src="https://user-images.githubusercontent.com/26049843/137649878-45348e72-fb37-465e-acf7-147408514731.png" width="350" alt="Swap Logic"> <center><em><br />WOOFi Swap Logic</em></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"> <em>WooRouter.sol UML</em> </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><em><br />WooPP.sol UML</em></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><em>Wooracle UML</em></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><em>WooGuardian UML</em></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 potentials risks of being arbitraged. In the smart contract, there is a price feed comparison between Chainlink Oracle and Wooracle. Verilog 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 <span style=" 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">Informational</span><span style=" 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">Minor</span><span style=" 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">Medium</span><span style=" 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">Major</span><span style=" 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">Critical</span> | | Total | Acknowledged | Resolved | | ------------- | ----- | ------------ | -------- | | Critical | 2 | 2 | 2 | | Major | 0 | 0 | 0 | | Medium | 1 | 1 | 0 | | Minor | 12 | 12 | 12 | | Inforamtional | 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)) <span style=" 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">Critical</span> ```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 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))<span style=" 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">Critical</span> ```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 contract instead of using `selfdestruct`. When user 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). <!-- ### Major <span style=" 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">Major</span> --> ### 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))<span style=" 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">Medium</span> ```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 main stream 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))<span style=" 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">Minor</span> ```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)) <span style=" 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">Minor</span> ```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)) <span style=" 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">Minor</span> ```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))<span style=" 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">Minor</span> ```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))<span style=" 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">Minor</span> ```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))<span style=" 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">Minor</span> **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**: Using 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))<span style=" 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">Minor</span> **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 using proxy contracts as an upgradable method. So this issue has been resolved. Besides, all liquidity provided by Wootrade team themselves, so `WooPP.sol` can be replaced and changed to the new one without a need of 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))<span style=" 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">Minor</span> ```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))<span style=" 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">Minor</span> ```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))<span style=" 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">Minor</span> **Description**: There is a subtraction `postBalance.sub(preBalance)` at the event emit of `externalSwap`. The function will be revert if 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))<span style=" 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">Minor</span> **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 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))<span style=" 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">Minor</span> **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` differs 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))<span style=" 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">Informational</span> ```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))<span style=" 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">Informational</span> ```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))<span style=" 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">Informational</span> ```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 to the below format: ```solidity delete tokenInfo[baseToken]; ``` **Result**: Resolved. --- ## 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 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 innovations.