Vesta Finance Audit

###### tags: `Final Report` Vesta Finance Audit === > Copyright © 2022 by Verilog Solutions. All rights reserved. > January 30, 2022 > by **Verilog Solutions**  ![Vesta-COVER](https://hackmd.io/_uploads/rJs7w2uX9.png) ## Project Summary Vesta Finance is an Arbitrum-based lending protocol. Users can collateralize ETH and other supported assets to borrow $VST, which is Vesta Finance's stablecoin. Vesta will support other collateral after the launch. A portion of Vesta Finance's code is based on [Liquity](https://github.com/liquity). --- ## Service Scope Our review focused on the [**master branch**](https://github.com/vesta-finance/vesta-protocol-v1/tree/master), specifically, commit hash [abf3f51bbf74e98cf35e81b37cc671aea60c04e3](https://github.com/vesta-finance/vesta-protocol-v1/tree/abf3f51bbf74e98cf35e81b37cc671aea60c04e3). Our auditing service for Vesta Finance includes the following two stages: 1. **Pre-Audit Consulting Service** As a part of the pre-audit service, the Verilog team worked closely with the Vesta Finance development to uncover potential vulnerability and the best practices of smart contract development. Verilog team appreciates the efficient and effective communication channel with the Vesta Finance team, as new findings were often exchanged promptly and fixes were applied quickly, during the preliminary report stage. 2. **Audit Service** The Verilog team conducted a thorough study of Vesta Finance's smart contract and produced the architecture graph and UML graph presented below in the Vesta Finance Architecture section. The list of findings, along with the severity and solution, is available under section [**Findings & Improvement Suggestions**](https://hackmd.io/GAg8_BWSQ8C9a4asAccwLA#Findings-amp-Improvement-Suggestions). --- ## Vesta Finance Architecture ![Liquity Archtecture](https://hackmd.io/_uploads/H17GsFVAF.png) **There are 13 major smart contracts in Vesta Finance:** - `ActivePool.sol` This contract holds the total \$ETH as well as other assets balances and records the total stablecoin debt of the active Troves. - `AdminContract.sol` This contract is the portal for the admin or DAO to trigger changes to the smart contract. - `BorrowerOperations.sol` This contract contains the basic operations by which borrowers interact with their Trove: Trove creation, $ETH top-up/withdrawal, stablecoin issuance, and repayment. It also sends issuance fees to the VSTAStaking contract. BorrowerOperations functions call TroveManager and pass the Trove state, where necessary. BorrowerOperations functions also call various Pools to move Ether/Tokens between Pools or between Pool <> user, where necessary. - `CollSurplusPool.sol` This contract holds the $ETH and other assets surpluses from Troves that have been fully redeemed as well as from Troves with an ICR > MCR that were liquidated in Recovery Mode. This contract sends the surplus back to the owning borrower when triggered by ``BorrowerOperations.sol`` contract. - `DefaultPool.sol` This contract holds the total Ether and other assets balances and records the total stablecoin debt of the liquidated Troves that are pending redistribution to active Troves. If a Trove has pending Ether/debt "rewards" in this DefaultPool, they will be applied to the Trove when it next undergoes a borrower operation, a redemption, or a liquidation. - `HintHelper.sol` A helper contract, containing the read-only functions to calculate the accurate hints to be supplied to borrower operations and redemptions. - `LiquidityFarming.sol` The smart contract contains the business logic for liquidity mining. - `MultiTroveGetter.sol` Ths smart contract provides the interface for the user and frontend API to access the Trove of different assets. - `PriceFeed.sol` Contains functionality for obtaining the current \$ETH:$USD price, which the system uses for calculating collateralization ratios. - `SortedTroves.sol` This contract implements a doubly-linked list that stores the addresses of Trove owners, sorted by their collateralization ratio (ICR). It inserts and re-inserts Troves at the correct position, based on their ICR. - `StabilityPool.sol` This contract contains functionality for StabilityPool operations including making deposits and withdrawing the compounded deposits and/or accumulated $ETH and $VST gains. The contract holds the $VST StabilityPool deposits and the $ETH gains for depositors, from liquidations. - `StabilityPoolManagement.sol` This contract is the portal for admin or DAO to manage the ``StabilityPool.sol`` contracts. - `TroveManager.sol` This contract contains functionality for liquidations and redemptions. It sends redemption fees to the VSTAStaking contract. Also contains the state of each Trove -- _i.e._, a record of the Trove’s collateral and debt. TroveManager does not hold value (_i.e._, Ether / other tokens). TroveManager functions call the various Pools to transfer Ether/tokens between Pools, where necessary. - `VSTAToken.sol` The ERC20 smart contract for $VSTA. - `VestaParameters.sol` The smart contract provides the parameters for the lending protocol. --- ## Privileged Roles There are two privileged roles `owner` and `controller` 1. `StabilityPoolManager.sol` `owner` and `controller` can add more `StabilityPool`. 2. `VestaParameters.sol` `owner` and `controller` can set and tune key parameters inside the `VestaParameters.sol`: `MCR`, `CCR`, `VSTGasCompensation`, `MinNetDebt`, `PercentDivisor`, `BorrowngFeeFloor`, `MaxBorrowingFee`, and `RedemptionFloor`. Those parameters initially will be set by the core team of Vesta Finance and will gradually shift to the governance vote model. 3. `PriceFeed.sol` `owner` and `controller` can add oracles for tokens. 4. `AdminContract.sol` `owner` can add new collateral. --- ## Findings & Improvement Suggestions InformationalMinorMediumMajorCritical | | Total | Acknowledged | Resolved | | ------------- | ----- | ------------ | -------- | | Critical | 0 | 0 | 0 | | Major | 0 | 0 | 0 | | Medium | 0 | 0 | 0 | | Minor | 10 | 10 | 5 | | Informational | 3 | 3 | 3 | ### Critical No Critical issue was found ### Major No Major issue was found ### Medium No Medium issue was found. ### Minor 1. Event emit of `receivedERC20()` (`ActivePool.sol`: L185, `DefaultPool.sol`: L132) used the wrong indexed value. Minor **Description**: **Recommendation**: Replace `assetsBalance[ETH_REF_ADDRESS]` with `assetsBalance[_asset]`. ``` Solidity function receivedERC20(address _asset, uint256 _amount) external override callerIsActivePool { assetsBalance[_asset] = assetsBalance[_asset].add(_amount); emit DefaultPoolAssetBalanceUpdated( _asset, assetsBalance[_asset] ); } ``` **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 2. Lack of asset address check in `receivedERC20()` (`DefaultPool.sol` and `ActivePool.sol`). Minor **Description**: Lack of asset address check in `receivedERC20()`. **Recommendation**: Add `require asset != ETH_REF_ADDRESS` check for `receivedERC20()`. **Results**: Not Resolved. 3. Redundant function (`BorrowerOperations.setAddresses`, `TroveManager.setAddresses`, and `CommunityIssuance.sol`). Minor **Description**: The `initializer` modifier is enough for the check, no need for `isInitialized`. **Recommendation**: Keep one of them. **Results**: This suggestion is not adopted. 1. Add `checkContract` for `_adminContract` at `setAddresses()` (`StabilityPoolManager.sol`). Minor **Description**: Check contract before setting the address. **Recommendation**: : Check contract before setting the address. **Results**: Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 1. Add extra check to prevent stability pool address being overwritten in `addStabilityPool()` (`StabilityPoolManager.sol`). Minor **Description**: Did not check if stability pool has been added or not. **Recommendation**: Check if asset has been mapping to a stability pool or not before adding. **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 2. Suggest to add cross check to make sure the asset is the same as the `assetAddress` in `stabilityPool`(`StabilityPoolManager.sol`). Minor **Description**: Lack of check for argument `asset` and asset address of `StabilitityPool`. **Recommendation**: Add cross check to make sure the `asset` is the same as the asset address in `stabilityPool`. **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 3. Unused import `@openzeppelin/contracts/proxy/Clones.sol` (`StabilityPoolManager.sol`).Minor **Description**: Unused import. **Recommendation**: Delete unused imports. **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 4. Add check for `_asset` in `TroveManager.addNewAsset()` such as `vestaParams.sanitizeParameters(_asset)`. Minor **Description**: Additional layer of check is necessary. **Recommendation**: ```solidity function sanitizeParameters(address _asset) public { if (!hasCollateralConfigured[_asset]) { _setAsDefault(_asset); } } ``` **Results:** Function was removed. 5. Suggest to add magic number `1e12` as a constant in (``LiquidityFarming.sol``). Minor **Description**: Add `1e12` as a constant. **Recommendation**: ```solidity // pre-define PRECISION at the begining of the code uint256 constant PRECISION = 1e12; ``` **Results:** Contract removed. 6. Suggest to add a check to sanitize the input argument `addSecond` in function `changeEndTime(uint32 addSeconds)` under (`LiquidityFarming.sol`). Minor **Description**: Since owner can change the end time, it is recommended to add a `require` to limit the maximum value that the owner can call the function. **Recommendation**: Add limitation requirement for `addSeconds`. **Results:** Contract removed. ### Informational 1. The event name does not match the event action (`CollSurplusPool.sol`). Informational **Description**: Event `EtherSent` can be emitted when assets other than Eth is sent. **Recommendation**: Change the event name, and include asset address in the event. **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 2. Typo in function name (`CommunityIssuance.sol`). Informational **Description**: `transferFunToAnotherStabilityPool` should be `transferFundToAnotherStabilityPool`. **Recommendation**: Change the event name, and include asset address in the event as well. **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). 3. Suggest to add address check for `_treasurySig` at constructor (`VSTAToken.sol`). Informational **Description**: No check for input`_treasurySig`. **Recommendation**: Add address check for `_treasurySig`. **Results:** Resolved in commit [cc83e24d6342d7a921b82b54b492c9bb91ea5dd2](https://github.com/vesta-finance/vesta-protocol-v1/tree/cc83e24d6342d7a921b82b54b492c9bb91ea5dd2). --- ## Disclaimer Verilog receives compensation from one or more clients for performing the smart contract and auditing analysis contained in these reports. The report created is solely for Clients and published with their consent. As such, the scope of our audit is limited to a review of code, and only the code we note as being within the scope of our audit detailed in this report. It is important to note that the Solidity code itself presents unique and unquantifiable risks since the Solidity language itself remains under current development and is subject to unknown risks and flaws. Our sole goal is to help reduce the attack vectors and the high level of variance associated with utilizing new and consistently changing technologies. Thus, Verilog in no way claims any guarantee of security or functionality of the technology we agree to analyze. In addition, Verilog reports do not provide any indication of the technologies proprietors, business, business model, or legal compliance. As such, reports do not provide investment advice and should not be used to make decisions about investment or involvement with any particular project. Verilog has the right to distribute the Report through other means, including via Verilog publications and other distributions. Verilog makes the reports available to parties other than the Clients (i.e., “third parties”) – on its website in hopes that it can help the blockchain ecosystem develop technical best practices in this rapidly evolving area of innovation.