-
Notifications
You must be signed in to change notification settings - Fork 24
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
remove idle supply #260
remove idle supply #260
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Merlin Egalite <44097430+MerlinEgalite@users.noreply.github.com>
f21498c
to
586ed73
Compare
The CI is failing though |
a997930
to
210f943
Compare
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.
lgtm
README.md
Outdated
- The vault's asset as loan token. | ||
- No collateral token (`address(0)`). | ||
- An arbitrary IRM. | ||
- No oracle (`address(0)`). |
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.
- No oracle (`address(0)`). | |
- An arbitrary oracle. |
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.
Can I still suggest address(0)? I think it's interesting to have at least the maximum nb of parameters standard for idle markets
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.
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.
Was a tough one to review, not 100% confident about it.
Still I think it's good to go, except for the ERC4626 compliance in maxDeposit
and maxMint
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.
LGTM
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.
LGTM
fixes #254
Fixes #301