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

feat: Add zero IR strategy #818

Merged
merged 7 commits into from
Mar 22, 2023
Merged

feat: Add zero IR strategy #818

merged 7 commits into from
Mar 22, 2023

Conversation

miguelmtzinf
Copy link
Contributor

Closes #817

@height
Copy link

height bot commented Mar 3, 2023

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (94e571f) 99.65% compared to head (87d3cf7) 99.65%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #818   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files          45       46    +1     
  Lines        2300     2320   +20     
  Branches      421      421           
=======================================
+ Hits         2292     2312   +20     
  Misses          8        8           
Impacted Files Coverage Δ
contracts/misc/ZeroReserveInterestRateStrategy.sol 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@miguelmtzinf miguelmtzinf changed the base branch from master to feat/3.0.2 March 15, 2023 08:15
*/
contract ZeroReserveInterestRateStrategy is IDefaultInterestRateStrategy {
/// @inheritdoc IDefaultInterestRateStrategy
uint256 public immutable OPTIMAL_USAGE_RATIO = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why immutable and just not constant?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, fixed now

@eboadom
Copy link
Collaborator

eboadom commented Mar 15, 2023

Generally, I don't see any major problem with the implementation from the perspective of the protocol having a ZERO rate strategy configured, just an small consideration.
More specifically:

  • There is no problem with returning 0 on all functions non-calculateInterestRates(), as there is no dependency on them in the protocol.
  • It is correct to keep the addresses provider initialization, as there could be some external dependency checking it for some reason.
  • Regarding where calculateInterestRates() is used:
    • On updateInterestRates() there should not cause any problem: rates on the storage of the reserve data will be update to 0, and this will just cause that on the next pool action, indexes will not increase, and there is no "inflow" coming from rates. Additionally, this is the same case as having RF configured to 100%, which we know is perfectly fine since 3.0.1
    • On validateRebalanceStableBorrowRate() is it slightly more interesting, but I would say no big problem and definitely not related to this implementation. If there is no stable debt supply of an asset, no matter if the stable rate mode is active or not, the rebalancing will just revert().
      If there would be stable rate active and the rate strategy ZERO would be set, it would actually allow to continuously rebalance of any borrower with a positive balance of stable debt. This should not cause any problem but seems antinatural to allow continuous minting/burn of stable debt for the user, so maybe it could be an option to circuit-break somehow the rebalance.

@miguelmtzinf
Copy link
Contributor Author

miguelmtzinf commented Mar 15, 2023

Thanks for the analysis and review @eboadom .

Regarding the side-effect on validateRebalanceStableBorrowRate(), these hypothetical rebalance actions would not make sense at all, because the rate is 0 no matter what.

@eboadom
Copy link
Collaborator

eboadom commented Mar 15, 2023

Thanks for the analysis and review @eboadom .

Regarding the side-effect on validateRebalanceStableBorrowRate(), these hypothetical rebalance actions would not make sense at all, because the rate is 0 no matter what.

Yes, agree. Just something to take into account, because it feels weird that we allow repeated actions that make no change

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

Successfully merging this pull request may close these issues.

Add a ReserveInterestRateStrategy with all parameters zeroed
5 participants