WOOFI Swap Audit

Copyright © 2021 by Verilog. All rights reserved.
October 17, 2021
by Verilog Audit

Image Not Showing Possible Reasons

The image file may be corrupted
The server hosting the image is unavailable
The image path is incorrect
The image format is not supported

This report presents our engineering engagement with Wootrade 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 market making processes in Centralized Exchange (CEX).

Table of Content

WOOFI Swap Audit

Project Summary

WOOFI Swap is built and designed by the CEX Wootrade to improve market making processes. 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, Wootrade team has the insight to propose a solution for DEX by using the Wootrade 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, Wootrade team tunes the parameters in their SMM 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 Wootrade 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 Wootrade team. In general, the idea behind WOOFI swap is to provide users a closer performance to CEX slippage trading on DEX. Indubitably, Wootrade innovation with SMM 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, specifically, commit hash 57d7f2eb2230360a009461051235ad3ecf16f707.

Our auditing service with Wootrade will include the following two stages:

Pre-Audit Consulting Service
Audit Service

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

WOOFI Swap Architecture

WOOFI Swap Architecture

There are three major smart contracts in WOOFI Swap:

Wooracle.sol
WooPP.sol
WooRouter.sol

Wooracle.sol

Wooracle.sol is an oracle contract managed and developed by the Wootrade team. The Wootrade team uploads their CEX data to the oracle contract to get the latest trading price, middle price and market spread information on CEX.
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 Wootrade team and can be more than one.
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 DODO to exchange some tokens that are not available for direct exchange on WOOFI Swap.

WOOFI Swap Logic

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, if users are swapping tokens that are not listed in the base token list, WOOFI Swap will redirect the trade to DODO.

The WooRouter.sol UML (after pre-audit & audit):

WooRouter.sol UML

The WooPP.sol UML (after pre-audit & audit):

WooPP.sol UML

The Wooracle.sol UML (after pre-audit & audit):

Wooracle UML

Privilege Roles

There are two main privilege roles: owner and strategist.

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
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 Wootrade team has control over these privilege roles, we suggested for them to transfer the owner to a multisig wallet controlled by the Wootrade team.

Since users will not be able to join as liquidity providers, the current version allows the Wootrade 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 Wootrade 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	0	0
Minor	12	12	12
Informational	4	4	4

Critical

<address>.transfer is used for transferring eth. (WooRouter: L122, L141, 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 reentrancy guard and check the return state.

Result: Resolved.

Avoid using selfdestruct(WooRouter: L284-L286)Critical
```
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.

Medium

Check actual token amount received after transferFrom (WooPP: L343, L373)Medium
```
IERC20(baseToken).safeTransferFrom(from, address(this), baseAmount);
```
```
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: Not resolved.

Minor

The quoteToken is not immutable in WooPP (WooPP: L72)Minor
```
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.
swap function: Adding the requirement for non-zero address to fromToken, toToken, to, rebateTo (WooRouter: L97-L103) Minor
```
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.
externalSwap function: Adding the requirement for non-zero address to approveTarget, swapTarget, fromToken, toToken, to; (WooRouter: L201-L207) Minor
```
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.
setWhiteListed: Adding the requirement for non-zero address to target; (WooRoute: L274)Minor
```
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.
Add non-zero address check for address arguments in other solidity files including libraries. (WooPP: L143, L248-249, L287, L326, L329-331, L356, L359-361, L385, L398, L411, L421, L437, L443, L498, L523, L529-530, L537)Minor
```
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.
Use of actual uint type instead of uint256 for arguments newThreshold, newLpFeeRate and R in tuneParameters (WooPP: 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: Using actual uint type instead.
Result: Not Resolved.
WooPP_proxy.sol is a flattened file (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 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.
The quoteToken is not immutable in WooPP (WooPP: L72)Minor
```
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.
Add require for msg.sender at the receive function in WooRouter(WooRouter: L79)Minor
```
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.senderto only allow WETH contract and whitlisted contracts to send eth.

Recommendation: Add require for msg.sender at the receive function.
Result: Resolved.
Require postBalance to be bigger than preBalance at externalSwap (WooRouter: L201-L227)Minor

Description: There is a subtraction postBalance.sub(preBalance) at the event emit of externalSwap. The function will be revert if overflow happens. Adding a requirepostBalance > preBalanace is recommended.

Recommendation: Adding a requirepostBalance > preBalanace.
Result: Resolved.
internal functions can be changed to private functions(WooPP: L146, L172, L185, L198, L211, L224, L252, L291, L547)Minor

NOTE: Gas Saving Recommendation
```
 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.
```
 function autoUpdate(    
    address baseToken,
    TokenInfo memory baseInfo,
    TokenInfo memory quoteInfo
 ) private view {...}
```
Result: Resolved.

public functions can be changed to external functions (WooPP: L448,L478,L502)Minor

NOTE: Gas Saving Recommendation

 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.

function addBaseToken(
    address baseToken,
    uint256 threshold,
    uint256 lpFeeRate,
    uint256 R,
    address chainlinkRefOracle
 ) external nonReentrant onlyStrategist { .... }

Result: Resolved.

Informational

No indexed parameters in the events defined in WooPP and WooRouter (WooRouter: L67, L77; WooPP: L61)Informational
```
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.
Require message reformat (WooRouter: L82, L83, L90, L111, L236, L238, L245, L249; WooPP: L66, L110, L111, L112, L126, L149, L150, L258, L297, L334, L342, L364, L372, L387, L395, L400, L408, L416, L423, L431, L449-452, L455, L469, L480, L503-505, L508, L551, L553, L557)Informational
```
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.
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.

Using delete in removeBaseToken (WooPP: L478)Informational

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:

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.