Best Practices for smart contracts

My first review on Smart contracts

Ravi Kiran Betha's photo
Ravi Kiran Betha
·

4 min read

This is my first review of smart contracts of a project that is already live on mainnet. This is a learning exercise and hence I was keen to find a project that was simple to start with. It was also important for me to go back and check if this attempt was in the right direction.

So, I looked for projects that were already audited in a contest and had reports generated. Reviewing it independently and then comparing it with the audit report helps me evaluate myself and areas to improve.

Project:

https://github.com/code-423n4/2022-10-inverse/

Below is my approach in steps in the listed order

a) Understand the Protocol and its purpose

b) Do a manual code review with the above understanding

c) using known Vulnerabilities, review and list the findings

d) Use Visualizing tools to understand the function calls between contracts

d) Review the code again with vulnerabilities and potential exploits

e) Use Slither to run and see if there are any findings/misses between my code review and slither report. Note the gaps, if any.

f) Document the issues and suggestions to fix them

g) Generate a summary report using a reference template

h) Compare my report against the report captured in the audit

i) Identify the gaps and list them as things to do in future reviews.

j) repeat this exercise across other projects

Understanding Inverse Finance

Read through the below documentation to get a grasp on Inverse Finance and the protocol. With the overall understanding, the code review will be only to the extent of the scope defined in the above repository and nominated contracts.

https://docs.inverse.finance/inverse-finance/inverse-finance/introduction

Overview of the project:

Protocol - Logical Hierarchy

Currently, in Defi, Variable interest rates loans are more popular and are risky as they accept tokens as collateral. If the token price jump, the collateral requirements or liquidation risk also shoots up. As the liquidity of tokens drops, the rate of interest shoots up.

Inverse Finance is trying to address this limitation in Defi using Fixed interest rate loans.

DBR(Dola Borrowing Rights) is a replacement for interest. As you borrow a loan, you buy DBR and they are consumed by the protocol over time.

Working:

Let's say Alex wants to borrow 100 Dola. So, Alex deposits 125 USD worth of Eth.

Each DBR allows Alex to borrow 1 Dola for 1 year. So, in order to get 100 Dola, Alex needs to buy 100 DBR. Let's say, each DBR is 5 cents. So, using 5 USD, Alex will borrow 100 DBR, that will entitle him to borrow 100 Dola for 1 year.

Now, that Alex has loan, the DBR will be consumed against the dollar borrowed.

In the DBR reaches 0 over time, after one year, Alex has the ability to recharge DBR and add to the balance. Incase the balance reaches negative balance, third parties can add DBR(premium) to keep the loan healthy. Such addition is incentivized for the third party.

Such added DBR adds to Alex's borrowed amount. This can continue until the loan amount is less than total ether deposited. After this, the loan will be liquidated.

Contracts in Scope:

Market.sol

In the computation of liquidation fee, using braces, priority should be explicitly given to multiplication operation over division.

if(liquidationFeeBps > 0) {
            uint liquidationFee = repaidDebt * 1 ether / price * liquidationFeeBps / 10000;
            if(escrow.balance() >= liquidationFee) {
                escrow.pay(gov, liquidationFee);
            }
        }

DBR.sol

BorrowController.sol

function borrowAllowed(address msgSender, address, uint) public view returns (bool) {
        if(msgSender == tx.origin) return true;
        return contractAllowlist[msgSender];
    }

Oracle.sol

there is lot of common code between getPrice() and viewPrice(). Should centralized if possible.

Fed.sol

Low:

a) Times when a chair has resigned, the expansion and contraction functions will stop working. The absence of a chair should be limited as it impacts functionality. But, the situation is in the control of

b) Gov can also become the chair.

c) take profit function need not be a public function. it could be scoped under a chair or gov.

d) checks could have written a modifier for better identification of access.

High:

e) It is possible to manipulate the take-profit function as the logic operates only on the parameters passed by the caller.

Example:

Hacker implements a smart contract that implements IMarket. Using the contract, he can manipulate the net profit function to return a positive number.

And then in the take-profit function, the hacker can manipulate the supply of Dola.

Escrow Contracts

SimpleERC20Escrow.sol

INVEscrow.sol

GovTokenEscrow.sol

Compare Audit Report:

There are many issues reported around the calculation details with gaps that I did not capture in my review. There seems little overlap between my review and findings done by audit.

Takeaway:

The learning is to spend more time reviewing the computation logic and see if anything can go wrong.

EVMSmart ContractsauditingBlockchain