-
Notifications
You must be signed in to change notification settings - Fork 1
[VPD-154]: Undertaker Contract #11
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
base: develop
Are you sure you want to change the base?
Conversation
| expiries[market] = Expiry({ | ||
| toBePausedAfterTimestamp: toBePausedAfterTimestamp, | ||
| canUnlist: canUnlist, | ||
| toBeUnlistedMinTotalSupplyUSD: toBeUnlistedMinTotalSupplyUSD, | ||
| pauseTimestamp: 0, | ||
| unlistTimestamp: 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.
Is overwriting an existing entry allowed?
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.
Yes.
| function exchangeRateStored() external view returns (uint256); | ||
| } | ||
|
|
||
| interface IComptroller { |
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 are many functions not being used.
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's fine to keep them as other PRs of this repo might be using them. At the end all PRs will be merged so we will end up having these functions.
|
|
||
| if (expiry.toBePausedAfterTimestamp != 0 && block.timestamp < expiry.toBePausedAfterTimestamp) { | ||
| return false; | ||
| } |
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.
This means for a market that already has a scheduled paused time we won't pause it even if it meets the pausing criteria ?
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.
Yes right.
| comptroller.actionPaused(market, IComptroller.Action.ENTER_MARKET) | ||
| ) { | ||
| return false; | ||
| } |
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.
may i ask what's the rationale behind, if let's say only MINT action is paused, shouldn't we continue and pause the rest actions as well ?
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.
We are assuming there won't be cases where just the MINT is paused. If that's the case then this market will be handled manually for pausing other actions.
|
|
||
| (, uint256 collateralFactorMantissa, ) = comptroller.markets(market); | ||
| if (collateralFactorMantissa == 0) { | ||
| return false; |
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.
also if CF = 0, will it have any impact on the pause ?
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.
Here also we are assuming if a market is not paused then it's unlikely to make CF as 0.
| IComptroller(comptroller).setMarketSupplyCaps(markets, caps); | ||
|
|
||
| IComptroller(comptroller).unlistMarket(market); | ||
|
|
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.
may i ask what will happen for those remaining funding inside this market ? They got stuck forever ?
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.
We check if the total deposit is below toBeUnlistedMinTotalSupplyUSD so a very small amount is stuck. Which I think is acceptable.
Description
Resolves #
Checklist