-
Notifications
You must be signed in to change notification settings - Fork 5
Proof type invariants #55
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
Conversation
Staging contract is deployed to Gas Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way, although a bit more involved, would be to use the type state pattern. In this case we could have a generic type which enables us to hide certain functions/methods under the InterestAccumulated
type constraint.
If it is something you'd want to try out, I can do a similar implementation to this PR sometime this week
@petarvujovic98 Let's give it a shot. |
Just clarifying that interest rates, and thus interest/yield amounts, will only change based on stablecoin utilization (aka demand for specific stablecoin lending market). Addition or removal of collateral only changes the collateral ratio, not yield for borrower or lender. Changes to the stablecoin borrowed/repaid will (marginally) affect the interest rate of the market and thus the total interest on the loan. I don't have a strong opinion on the details of how this calculation is implemented. |
@royalf00l Good clarification. The way the contract currently calculates yield/interest is as follows:
Maybe it would be better to rethink that last point and track position history as well? |
That's fine for now, even if it's not optimally efficient. Ideally, the yield recalculations are only done when changes are made to the stablecoin position, not the collateral. I'm fine noting this for now, saving as a low priority issue (optimization) to come back to later. |
Currently no calculations are triggered for collateral balance changes. Only for supply deposit and borrow principal. This issue is more of a matter of safety than optimization, to be honest. It has the upside of a small amount of optimization, but that is not the reason I proposed this change, it's just a happy side-effect. |
My initial idea for the typestate pattern would overcomplicate things substantially, mainly in ways where we would have the generic types spread into too much of the codebase to only save us from using a simple marker (proof) type like you implemented here. |
Adds some compile-time guarantees to interest and yield accumulations. It's not incredibly obvious how the code works, but I think it's actually quite simple once the intent is understood.
Basic idea:
Before certain operations (adding/removing principal to borrow positions, adding/removing supply to supply positions), we need to recalculate the interest/yield (since the rate of accumulation will change after the addition/removal).
One way to ensure this happens is to just call the accumulation function at the top of the addition/removal functions. The problem with this approach is twofold:
The way presented in this PR is to require any change of those balances (borrow principal or supply deposit) to also take ownership of an argument of a type that can only be created by calling the corresponding accumulation function. We call this special ZST a "Proof" type. (There might be a more formal name for this pattern that is escaping me at present.)