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

Make sure that liquidity is isolated from a market to another #92

Closed
MerlinEgalite opened this issue Jul 10, 2023 · 13 comments
Closed

Make sure that liquidity is isolated from a market to another #92

MerlinEgalite opened this issue Jul 10, 2023 · 13 comments

Comments

@MerlinEgalite
Copy link
Contributor

We want to avoid that because of a bad IRM or oracle, someone is able to steal money from different markets using the same asset

@MathisGD
Copy link
Contributor

It's already the case now, thanks to the liquidity check. What do you want to add exactly ? Tests ? Proofs ?

@MerlinEgalite
Copy link
Contributor Author

Proof would be great. I fear that integrators could fear that

@MathisGD
Copy link
Contributor

I suggest that we open an issue with invariants idea for when we will formal prove the contract, to gather everything together (and avoid having too much issues). wdyt ? @MerlinEgalite @QGarchery

@MerlinEgalite
Copy link
Contributor Author

Done here: #96

Closing this one

@MerlinEgalite MerlinEgalite closed this as not planned Won't fix, can't repro, duplicate, stale Jul 10, 2023
@QGarchery
Copy link
Contributor

Lol I thought of the same thing, and opened #96

Thinking a little bit about it, this invariant does not seem to be verified because of the liquidate function which can reduce the total supply. This seems to be a big issue

@QGarchery
Copy link
Contributor

Reopening because this particular issue is not solved I think (see comment above)

@QGarchery QGarchery reopened this Jul 10, 2023
@MerlinEgalite
Copy link
Contributor Author

I was closing not because it's solved but because you listed it on the things to verify. But I'm fine keeping it since we should fix it (it seems).

@MathisGD
Copy link
Contributor

Thinking a little bit about it, this invariant does not seem to be verified because of the liquidate function which can reduce the total supply. This seems to be a big issue

The liquidation doesn't touch the totalSupply right ?

@QGarchery
Copy link
Contributor

The liquidation doesn't touch the totalSupply right ?

It does when bad debt is realized

@MathisGD
Copy link
Contributor

I don't think that there is an issue. Worst case scenario the withdraw reverts because there are not any assets (div by zero) but it seems ok.

@QGarchery
Copy link
Contributor

I don't think that there is an issue. Worst case scenario the withdraw reverts because there are not any assets (div by zero) but it seems ok.

Yes but the issue is that we can have totalSupply[id] < totalBorrow[id], which essentially means that we have taken tokens from a market other than id

@MathisGD
Copy link
Contributor

How ? the totalBorrow is also reduced when realizing bad debt

@QGarchery
Copy link
Contributor

Oh yes you are right, this is beautiful !

Closing this because it will be verified in #96

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants