-
Notifications
You must be signed in to change notification settings - Fork 98
[Vault] Simplify mint()
#2735
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
[Vault] Simplify mint()
#2735
Conversation
mint()
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## clement/simplify-ousd #2735 +/- ##
=========================================================
+ Coverage 33.51% 34.20% +0.68%
=========================================================
Files 128 128
Lines 5517 5514 -3
Branches 1463 1460 -3
=========================================================
+ Hits 1849 1886 +37
+ Misses 3667 3627 -40
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
naddison36
left a comment
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.
DVF would be proud of all the red
|
|
||
| /// @dev mapping of supported vault assets to their configuration | ||
| mapping(address => Asset) internal _deprecated_assets; | ||
| mapping(address => uint256) internal _deprecated_assets; |
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 this can be simplified further to just
uint256 private _deprecated_assets;
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.
Indeed! Changed in this commit: 955b851.
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 was suggesting to go one step further by getting rid of the mapping. eg
mapping(address => uint256) private _deprecated_assets;
becomes
uint256 private _deprecated_assets;
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.
Description
Simplify
mint()on the VaultCore, by removing old params that are no longer relevant:asset, because Vault are now single asset.minimumOusdAmount, because always minted 1:1Simplify VaultStorage, by removing old Struct and constants.
Note:
mint()signature (with 3 args) for backward compatibility, but mark it as deprecated.OETHPlumeVaultCore, but as the project itself is deprecated, it shouldn't be an issue.