BlockApex (Auditor) was contracted by VoirStudio (Client) for the purpose of conducting a Smart Contract Audit/Code Review. This document presents the findings of our analysis which started from 26th Jan 2022.
Document log: Initial Audit: 14th Feb 2022 (complete) Quality Control: 14th - 22nd March 2022 Final Audit: 26th March 2022 (Complete)
Scope
The git-repository shared was checked for common code violations along with vulnerability-specific probing to detectmajor issues/vulnerabilities. Some specific checks are as follows:
Code review
Functional review
Reentrancy
Unchecked external call
Business Logics Review
Ownership Takeover
ERC20 API violation
Functionality Checks
Timestamp Dependence
Unchecked math
Access Control & Authorization
Gas Limit and Loops
Unsafe type inference
Escrow manipulation
DoS with (Unexpected) Throw
Implicit visibility level
Token Supply manipulation
DoS with Block Gas Limit
Deployment Consistency
Asset’s integrity
Transaction-Ordering Dependence
Repository Consistency
User Balances manipulation
Style guide violation
Data Consistency
Kill-Switch Mechanism
Costly Loop
Operation Trails & Event Generation
Project Overview
Unipilot is an automated liquidity manager designed to maximize ”in-range” intervals for capital through an optimized rebalancing mechanism of liquidity pools. Unipilot V2 also detects the volatile behavior of the pools and pulls liquidity until the pool gets stable to save the pool from impairment loss.
System Architecture
The protocol is built to support multiple dexes (decentralized exchanges) for liquidity management. Currently it supports only Uniswap v3’s liquidity. In future, the protocol will support other decentralized exchanges like Sushiswap (Trident). The architecture is designed to keep in mind the future releases.
The protocol has 6 main smart contracts and their dependent libraries.
UnipilotActiveFactory.sol
The smart contract is the entry point in the protocol. It allows users to create a vault if it's not present on protocol. Nevertheless Active vaults can only be created by governance.
UnipilotPassiveFactory.sol
The smart contract is the entry point in the protocol. It allows users to create a vault if it's not present on protocol. However passive vaults can be created by anyone.
UnipilotActiveVault.sol
Vault contract allows users to deposit, withdraw, readjustLiquidity and collect fees on liquidity. It mints an LPs to its users representing their individual shares. It also has a pullLiquidity function if liquidity is needed to be pulled.
UnipilotPassiveVault.sol
PassiveVault contract allows users to deposit, withdraw, readjustLiquidity and collect fees on liquidity. It mints an LPs to its users representing their individual shares.
UnipilotStrategy.sol
The smart contract to fetch and process ticks’ data from Uniswap. It also decides the bandwidth of the ticks to supply liquidity.
UnipilotMigrator.sol
The smart contract aids to migrate users liquidity from other Uniswap V3 Liquidity Optimizer Protocols to Unipilot V2 Protocol.
Methodology & Scope
The codebase was audited in an iterative process. Fixes were applied on the way and updated contracts were examined for more bugs. We used a combination of static analysis tool (slither) and Automated testing tool (Foundry) which indicated some of the critical bugs in the code. We also did manual reviews of the code to find logical bugs, code optimizations, solidity design patterns, code style and the bugs/ issues detected by automated tools.
Privileged Roles
In a production environment, the unipilot protocol sets the address for a governance that exercises a privileged position over the factory and vault contracts in the system. The governor has the power to initiate a transfer of the governor role to a new address.
The governance address is capable of executing a set of actions including:
Controlling the various whitelists for vaults in the factory contract
Toggle a vault as whitelisted
Set unipilot details to update addresses of Strategy and Index Fund contracts along with the Index Fund Fee Percentage
Set an operator address
The operator address has following activities it can be used for:
Pull liquidity for a vault contract from its position on a Uniswap V3 Pool
Call Readjust Liquidity to burn from and mint all liquidity back to a less volatile position on the Uniswap V3 Pool.
AUDIT REPORT
Executive Summary
The analysis indicates that some of the functionalities in the contracts audited are working properly.
Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX, Surya and Slither. All the flags raised were manually reviewed and re-tested.
Our team found:
# of issues
Severity of the risk
1
Critical Risk issue(s)
2
High Risk issue(s)
2
Medium Risk issue(s)
7
Low Risk issue(s)
11
Informatory issue(s)
Findings
#
Findings
Risk
Status
1.
Deposit ether with zero value
Critical
Fixed
2.
init() should have only once check
High
Fixed
3.
Pulled liquidity rationale
High
Acknowledged
4.
Unoptimized user liquidity is held in a vault.
Medium
Acknowledged
5.
Deposit should have non-reentrant checks placed in both vaults
Medium
Fixed
6.
Zero Address checks not placed in contract constructors()
Low
Acknowledged
7.
Safecast in UnipilotPassiveVault.sol line 232 & 233 for `swapPercentage`
Low
Acknowledged
8.
The storage Layout is unoptimized.
Low
Acknowledged
9.
Check max cap for indexFundPercentage and swapPercentage
Low
Fixed
10.
Deposit() function optimization
Low
Fixed
11.
toggleWhitelistAccount() redundant.
Low
Pending
12.
SortWeth() optimization
Low
Acknowledged
13.
No natspec documentation in UnipilotActiveVault.sol
Informatory
Acknowledged
14.
Mark token0 and token1 as immutable in UnipilotActivevault.sol and UnipilotPassiveVault.sol
Informatory
Partially Fixed
15.
onlyGovernance() modifier in passive vault contract
Informatory
Fixed
16.
_WETH address should be hardcoded before production wherever necessary
Informatory
Acknowledged
17.
Factory function createVault() optimization version.
Informatory
Acknowledged
18.
Assignment of Params in order to receive the function signature.
Informatory
Acknowledged
19.
Order of functions in solidity style guides.
Informatory
Acknowledged
20.
uint256 can be cheaper than uint8
Informatory
Fixed
21.
pullLiquidity() is vulnerable in its current execution
Informatory
Fixed
22.
Spelling mistakes in function signatures.
Informatory
Acknowledged
23.
Mark private functions as internal
Informatory
Fixed
Critical risk issues
1. Deposit ether with zero value.
Description:
If a deposit of tokens with ether as one is made, all the while the contract has pulled liquidity into the vault, the user making a deposit with 0 value can use the vault’s ether to execute a successful transaction.
Remedy:
Introduce a RefundETH() function that ensures a proper transfer of value either be in ethers or an ERC compatible version (WETH)
Status:
Fixed
High risk issues
1. init() should have only once check
Description:
Calling init() any time after the first time it has been called, can lead to permanent loss of position at uniswap V3.
There should be an onlyOnce modifier or a variable handling (as locks) that ensures init is never called again.
Status:
Fixed as per BlockApex recommendation.
2. Pulled liquidity rationale
Description:
Considering the scenario where the vault has pulled Liquidity with the intention of depositing it back to Uniswap V3 when the pool is relatively less volatile; the smart contract code assumes a rational behavior to override checkDeviation modifier by manually modifying ticks through strategy using onlyGovernance functions. But the code does not guarantee any logic to push liquidity back to v3 in a safe manner
Remedy:
Debatable.
Status:
Acknowledge
Developer’s response:
Governance will manually update deviation in case of price volatility.
Medium risk issues
Unoptimized user liquidity is held in a vault.
Description:
In Active vaults if a pool is created on 1-X ratio on Uniswap V3 and a user makes a deposit with 1-1 ratio through the vault, the vault is found to hold the remaining amount of the token initially at the price of X, giving users a complete share with proportion of the deposited amount while the remaining amount of user sits inactive within the vault.
Remedy:
The vault must ensure that the amounts provided by a user are equal to the amounts of tokens actually deposited on a Uniswap pool.
Status:
Acknowledged
Developer’s response:
Added rearrange liquidity method.
Deposit should have non reentrant checks placed in both vaults
Description:
The deposit() function in both vaults contract, that is, the UnipilotActiveVault and the UnipilotPassiveVault does not contain a non-reentrant modifier which is a standard practice to prevent any adversarial intent related to the reentrancy exploits.
Remedy:
A good industry practice requires that the deposit() function executes with a non-reentrant modifier; this modifier should be placed to ensure security as the deposit marks external calls through linked libraries to deposit values to the Uniswap V3.
Status:
Fixed as per BlockApex recommendation.
Low-risk issue
Zero Address checks not placed in contract constructors()
Description:
Constructor() does not contain checks for accepting params of address type whether an address is zero or not.
Remedy:
Since the constructor accepts an address from an argument, there should be a zero address check to ensure the functionality. These checks should be placed in almost every contract: Unipilot Factory , Unipilot Strategy , Unipilot Migration etc.
Status:
Acknowledged
Safecast in UnipilotPassiveVault.sol line 241 & 242 for `swapPercentage`
Description:
In UnipilotPassiveVault.sol the readjustLiquidity() reads the swapPercentage variable in Line 238 of the contract to calculate the amountSpecified variable in Lines 241-242, this Math is unsafe as the calculation is executed with different types for each param.
Use safecasting for all type variables on lines 232-233 to ensure a seamless execution of the desired arithmetics.
Status:
Acknowledged
Developer’s response:
Percentage calculation is correct with this method. (Well tested)
3. Storage Layout is unoptimized.
Description:
Variable tight packing is strongly recommended for both vaults and factories in state variable declaration as the contracts are composed in order that is gas-consuming.
Remedy:
A solidity design pattern ‘Tight variable Packing’ ensures that the smart contract is optimized to execute efficiently within the EVM environment.
Status:
Fixed as per BlockApex recommendation.
Status:
Slots are arranged bitwise now.
4. Check max cap for indexFundPercentage and swapPercentage.
Description:
In setUnipilotDetails() the param indexFundPercentage is checked to receive a lowest value greater than zero.
Ensure a check placed to bound the maximum value for the indexFundPercentage
Status:
Fixed
5. Deposit() function optimization.
Description:
In UnipilotActiveVault.sol and UnipilotPassiveVault.sol, Users can call deposit() with zero amounts of both tokens and the function executes until the end.
This function’s logic can be concise. The remedy, tested against the required logic, is mentioned as a code snippet in the screenshot above.
Status:
Acknowledged
Informatory issues
1. No NatSpec documentation
Description:
NatSpec documentation is an essential part of smart contract readability; it is therefore advised that all contracts and following files contain proper explanatory commenting;
UnipilotActiveVault.sol
UnipilotPassiveVault.sol
UnipilotMigrator.sol
Status:
Acknowledged
Developer’s response:
Completed netspec for all contracts.
2. Mark token0 and token1 as immutable in UnipilotActivevault.sol and UnipilotPassiveVault.sol
Description:
State variables containing the address of tokens should be marked as immutable as the constructor locks the values for each after deployment.
Status:
Partial Fixed.
Developer’s response:
PassiveVault used immutables however active vaults don’t due to size issues.
3. onlyGovernance() modifier in passive vault contract
Description:
The onlyGovernance modifier in the Passive Vault contract remains unused within the contract.
Status:
Fixed as per BlockApex recommendation.
4. _WETH address should be hardcoded before production wherever necessary
Description:
Address of the WETH token contract is passed as a constructor param in both Factories which can be optimized by hardcoding the actual address of _WETH in the final deployment of the production environment.
6. Assignment of Params in order to receive the function signature.
Description:
In all four contracts of vault and factory the constructor receives arguments in order which is out-of-sync to the one being assigned, reducing the code readability. Ensure param values and actual assignments are in sync for better code readability.
Receive() and Fallback() should be moved on top, below constructor; following the solidity design patterns
Status:
Fixed
8. uint256 can be cheaper than uint8
Description:
Uint8 is proved to be more costly than uint256 variables in a number of scenarios, where a better and optimized variable packing for uint8 variables is recommended or replaced with uint256/ uint64/ uint24 type vars.
Status:
Fixed
9. pullLiquidity() is vulnerable in its current execution
Description:
The pullLiquidity(address _recipient) method is vulnerable to some extent, holding potential for mal-intent or permanent loss of value. Checking for the address argument as not another whitelisted vault can ensure no accidental and permanent loss of tokens happen.
Status:
Pending
Developer’s response:
Vaults will be whitelisted only for the execution of pull liquidity (when needed) soon after execution that vault will be blacklisted in order to avoid accidentally sending tokens to other active vaults.
10. Spelling mistakes in function signatures
Description:
In the UnipilotMigrator.sol file, migrateUnipilotLiquididty() and _refundRemainingLiquidiy() are spelled wrong, causing readability issues as well as creating the wrong function signature.
Status:
Fixed
11. Mark private functions as internal
Description:
In the UnipilotMigrator.sol file, _sortWethAmount() and _addLiquidityUnipilot() are private, which are gas costly.
Status:
Fixed as per BlockApex recommendation.
DISCLAIMER
The smart contracts provided by the client for audit purposes have been thoroughly analyzed in compliance with the global best practices till date w.r.t cybersecurity vulnerabilities and issues in smart contract code, the details of which are enclosed in this report.
This report is not an endorsement or indictment of the project or team, and they do not in any way guarantee the security of the particular object in context. This report is not considered, and should not be interpreted as an influence, on the potential economics of the token, its sale or any other aspect of the project.
Crypto assets/tokens are results of the emerging blockchain technology in the domain of decentralized finance and they carry with them high levels of technical risk and uncertainty. No report provides any warranty or representation to any third-Party in any respect, including regarding the bug-free nature of code, the business model or proprietors of any such business model, and the legal compliance of any such business. No third-party should rely on the reports in any way, including for the purpose of making any decisions to buy or sell any token, product, service or other asset. Specifically, for the avoidance of doubt, this report does not constitute investment advice, is not intended to be relied upon as investment advice, is not an endorsement of this project or team, and it is not a guarantee as to the absolute security of the project.
Smart contracts are deployed and executed on a blockchain. The platform, its programming language, and other software related to the smart contract can have its vulnerabilities that can lead to hacks. The scope of our review is limited to a review of the Solidity code and only the Solidity code we note as being within the scope of our review within this report. The Solidity language itself remains under development and is subject to unknown risks and flaws. The review does not extend to the compiler layer, or any other areas beyond Solidity that could present security risks.
This audit cannot be considered as a sufficient assessment regarding the utility and safety of the code, bug-free status or any other statements of the contract. While we have done our best in conducting the analysis and producing this report, it is important to note that you should not rely on this report only - we recommend proceeding with several independent audits and a public bug bounty program to ensure security of smart contracts.
Our team performed a technique called “Filtered Audit”, where the contract was separately audited by two individuals. After their thorough and rigorous process of manual testing, an automated review was carried out using Mythril, MythX, and Slither. All the flags raised were manually reviewed and re-tested.
Spin is a DeFi derivative infrastructure built on NEAR Protocol, a reliable and scalable L1 solution. The on-chain order book solution offered by Spin provides a CEX-competitive experience to DeFi users.
Rain Protocol lets you build web3 economies at any scale.Rain scripts are a combination of low level functions (opcodes) like addition and subtraction and very high level functions like fetching an ERC20 balance at a given snapshot ID (Open Zeppelin), or fetching a chainlink oracle price.