Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Verify invariants about the protocol #96

Closed
9 tasks done
QGarchery opened this issue Jul 10, 2023 · 9 comments
Closed
9 tasks done

Verify invariants about the protocol #96

QGarchery opened this issue Jul 10, 2023 · 9 comments
Assignees
Labels
verif Modifies the formal verification

Comments

@QGarchery
Copy link
Contributor

QGarchery commented Jul 10, 2023

The idea is to list the invariants about the protocols. This way we will remember to add them as forge invariants or certora invariants.

Current invariant list :

  • borrow is less than supply on a given market. It has to be checked with the liquidate function too. (done in [Certora] Added borrow less supply invariant #294)
  • for each market, on the singleton there is at least (totalSupply - totalBorrow) borrowable tokens and the whole collateral (done in [Certora] Liquidity #304)
  • only the markets with enabled LLTVs have non-zero data (done in [Certora] Only enabled lltv/irm #290)
  • only the markets with enabled IRMs have non-zero data (done in [Certora] Only enabled lltv/irm #290)
  • the ratio shares / assets on a market is increasing, except in liquidate (in progress in [Certora] Verify share ratio #341)
  • sum_addresses(supplyShares(address)) = totalSupplyShare (done in [Certora] Dev #136)
  • sum_addresses(borrowShares(address)) = totalBorrowShare (done in [Certora] Dev #136)
  • sum_addresses(supplyShares(address).toAssetsDown()) <= totalSupply
    This invariant is difficult to check because every user's owed assets can change at each interaction. Moreover, it can be deduced (modulo virtual assets and shares) given the 2 previous invariants by applying toAssetsDown on both sides. For these reasons, it has been replaced with the following property: a given user cannot withdraw more than supplyShares(address).toAssetsDown() assets. (done in [Certora] Exit liquidity #316)
  • sum_addresses(borrowShares(address).toAssetsUp()) >= totalBorrow
    For similar reasons as the invariant above, this invariant has been replaced with the following property: when repaying the whole position, a user is transfering more tokens than borrowShares(address).toAssetsUp()) (done in [Certora] Exit liquidity #316)

(some of the invariants were taken from comments below, and put there for easier tracking)

@MathisGD
Copy link
Contributor

MathisGD commented Aug 7, 2023

Some more ideas:

  • sum_addresses(supplyShares(address)) = totalSupplyShare
  • sum_addresses(borrowShares(address)) = totalBorrowShare
  • sum_addresses(supplyShares(address).toAssets()) <= totalSupply
  • sum_addresses(borrowShares(address).toAssets()) >= totalBorrow

@Jean-Grimal
Copy link
Contributor

I conducted all of these tests (except for the enabled LLTVs and IRMs) and they all pass, Do you have other invariants in mind that could be tested ?

@Rubilmax
Copy link
Contributor

Rubilmax commented Aug 9, 2023

Some more ideas:

  • sum_markets(totalSupply(id) - totalBorrow(id)) <= ERC20(market.borrowableAsset).balanceOf(blue)

@pakim249CAL
Copy link
Contributor

Some more ideas:

* [ ]  `sum_markets(totalSupply(id) - totalBorrow(id)) <= ERC20(market.borrowableAsset).balanceOf(blue)`

Related to this, I would also consider adding collateral to the left hand side of this assertion

@Rubilmax
Copy link
Contributor

Rubilmax commented Aug 9, 2023

So it would be something like:

For every asset, the sum of totalSupply - totalBorrow of each market having this asset as borrowable + the sum of collateral supplied on each market having this asset as collateral should equal Blue's balance

@pakim249CAL
Copy link
Contributor

Yes, except technically I believe it should be <= since it is possible that blue is sent tokens without an interaction in blue.

@Rubilmax
Copy link
Contributor

Rubilmax commented Aug 9, 2023

So we can have equality of the invariant in tests, but only inequality in production

@Jean-Grimal
Copy link
Contributor

So it would be something like:

For every asset, the sum of totalSupply - totalBorrow of each market having this asset as borrowable + the sum of collateral supplied on each market having this asset as collateral should equal Blue's balance

In #231 the equality stands

@MathisGD MathisGD added the verif Modifies the formal verification label Aug 10, 2023
@QGarchery QGarchery self-assigned this Aug 14, 2023
@MathisGD MathisGD linked a pull request Aug 20, 2023 that will close this issue
@QGarchery
Copy link
Contributor Author

I'm closing this as I opened a general issue about formal verification to keep track of what's left to do. See #390

@QGarchery QGarchery removed a link to a pull request Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
verif Modifies the formal verification
Projects
None yet
Development

No branches or pull requests

5 participants