-
Notifications
You must be signed in to change notification settings - Fork 46
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
Apply and mint interest #85
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
3602904
Include agg. interest state updates in to openTrove
RickGriff 7b4fc23
Include agg. interest state updates in SP deposit and withdrawal
RickGriff 48085d0
Add comment and fix hardhat tests
RickGriff 8843357
Include ag. interest state updates in closeTrove
RickGriff 7a8b8fc
Add tests for calcPendingTroveInterest
RickGriff b894aca
Apply aggregate interest when adjusting Trove interest rate
RickGriff af19db2
Apply aggregate interest when adjusting debt and/or coll
RickGriff a0d2bdd
Add aggregate interest tests for addColl and withdrawColl
RickGriff aaabd9c
Add permisionless Trove interest update func
RickGriff 7ed854d
Fix individual ICR calc and add interest tests for withdrawETHGainToT…
RickGriff 8beb3eb
Apply agg interest when liquidating (offset) in NM
RickGriff a20b8b1
Apply agg interest when liquidating (redist) in NM
RickGriff 10c0dea
Apply interest in RM and fix hardhat tests
RickGriff e67afb9
contracts: Fixes after rebase
bingen 4e18bed
Add tests for TCR and ICR getters
RickGriff 2e111ce
Add test for applyTroveInterestPermissionless
RickGriff 8f52365
contracts: Comment out console.log imports
bingen 253786e
Merge branch 'main' into apply_and_mint_interest_rebase
bingen 3942a9d
Add fixes based on review comments
RickGriff File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
I think it would be a good idea to merge this function with the ones for G (increase/decrease recorded debt).
At least the
external
end point (we can keep internally the functions as they are). For 2 reasons:I wonder if we could even have
S
there too.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.
As I see there are calls with
(0,0)
as argument, maybe we could have apublic
functionmintAggInterestNoDebtChange
that would be called by the more genericmintAggInterest
which also calls theincreaseBoldDebt
anddecreaseBoldDebt
.This generic one could also accept the accrued interest as a parameter, to update
S
if non-zero.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.
I've refactored the D, G and S updates into a single call in this PR. Let me know what you think. It's still a bit unintuitive. This should get simpler if/when we optimize redistribution gains - then we can just update each tracker once with all the changes (debt change, accrued interest, pending gains).