Add borrower-chosen interest rate ordering #63
Conversation
|
Closes #54 |
RickGriff
force-pushed
the
interest_rate_basics
branch
from
0b67d4b
to
cbe9d80
Compare
| _adjustTrove(msg.sender, _collWithdrawal, _boldChange, _isDebtIncrease, _maxFeePercentage); | ||
| } | ||
|
|
||
| function adjustTroveInterestRate(uint _newAnnualInterestRate, address _upperHint, address _lowerHint) external { |
There was a problem hiding this comment.
Shouldn’t this function be permissioned? Either trove owner or the delegate. We may add a TODO and add it when we do delegation?
There was a problem hiding this comment.
Yes, this one is implicitly permissioned by _requireTroveIsActive, and the function only acts on the Trove owned by msg.sender.
The delegate functionality is TODO.
| @@ -1282,470 +1282,6 @@ contract("Gas compensation tests", async (accounts) => { | |||
| assert.isAtMost(th.getDifference(expectedGasComp_B, loggedGasComp_B), 1000); | |||
| }); | |||
|
|
|||
| // liquidateTroves - full offset | |||
| it("liquidateTroves(): full offset. Compensates the correct amount, and liquidates the remainder", async () => { | |||
There was a problem hiding this comment.
Are we removing these ones because they use liquidate(n) function? Maybe we can translate them to batchLiquidateTroves. Happy to add a commit for that myself if you want me to.
There was a problem hiding this comment.
Yes, right. Some could be translated, most don't seem to have an equivalent batchLiquidateTroves test.
| @@ -56,7 +56,46 @@ contract BaseTest is Test { | |||
| accountsList = tempAccounts; | |||
| } | |||
|
|
|||
| function logContractAddresses() view public { | |||
| function openTroveNoHints100pctMaxFee( | |||
There was a problem hiding this comment.
What does the 100pctMaxFee mean here?
There was a problem hiding this comment.
It's just a helper function that puts the max fee at 100%. Though we're not using the maxFeePercentage for anything now, so it could even be removed as an input param for openTrove.
I guess under the current scope there's no need for it as the borrow fee would purely depend on the user chosen interest rate, rather than other user actions or time decay.
| // --- batchLiquidateTroves() --- | ||
| // TODO: revisit the relevance of this test since it relies on liquidateTroves(), which now no longer works due to ordering by interest rate. | ||
| // Can we achieve / test the same thing using another liquidation function? |
There was a problem hiding this comment.
Yes, it seems the test is wrong, right? It should do batchLiquidateTroves([B,C]) instead of liquidateTroves(2).
There was a problem hiding this comment.
Right, yeah! I had test fatigue by this point
| @@ -265,77 +265,5 @@ contract( | |||
| assert.equal((await troveManager.Troves(carol))[3], "1"); | |||
| }); | |||
| }); | |||
|
|
|||
| context("Sequential liquidations", () => { | |||
There was a problem hiding this comment.
I guess we can translate these ones too. Or comment them out and put a TODO, so that we do if we finally leave Recovery Mode.
| @@ -2426,1949 +2425,6 @@ contract("TroveManager - in Recovery Mode", async (accounts) => { | |||
| ); | |||
| }); | |||
|
|
|||
| // --- liquidateTroves --- | |||
There was a problem hiding this comment.
I guess we can translate these ones too. Or comment them out and put a TODO, so that we do if we finally leave Recovery Mode.
annualInterestRateproperty to Troves and orders theSortedListby it.openTrove, and changes it later with the dedicatedadjustTroveInterestRatefunction. Interest rate is not changed inadjustTrove- which only alters collateral and/or debtPotential gas optimizations:
SortedListto reduce costs of any on-chain traversals needed (though would make someTroveManagerops more expensive - e.g. liquidations)