-
Notifications
You must be signed in to change notification settings - Fork 26
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
Update last total assets in _setCap #338
Changes from 11 commits
7ddf9f3
04b6518
eca6d39
f23a33f
f0b28c4
e3ecd9b
e06f7f0
c796fde
7f606fd
e5ec31d
89f0057
f462f5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -286,7 +286,7 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph | |
if (newSupplyCap == supplyCap) revert ErrorsLib.AlreadySet(); | ||
|
||
if (newSupplyCap < supplyCap) { | ||
_setCap(id, newSupplyCap.toUint184()); | ||
_setCap(marketParams, id, newSupplyCap.toUint184()); | ||
} else { | ||
// newSupplyCap > supplyCap >= 0 so there's no need to check `pendingCap[id].validAt != 0`. | ||
if (newSupplyCap == pendingCap[id].value) revert ErrorsLib.AlreadyPending(); | ||
|
@@ -298,11 +298,12 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph | |
} | ||
|
||
/// @inheritdoc IMetaMorphoBase | ||
function submitMarketRemoval(Id id) external onlyCuratorRole { | ||
function submitMarketRemoval(MarketParams memory marketParams) external onlyCuratorRole { | ||
Id id = marketParams.id(); | ||
if (config[id].removableAt != 0) revert ErrorsLib.AlreadySet(); | ||
if (!config[id].enabled) revert ErrorsLib.MarketNotEnabled(id); | ||
|
||
_setCap(id, 0); | ||
_setCap(marketParams, id, 0); | ||
|
||
// Safe "unchecked" cast because timelock <= MAX_TIMELOCK. | ||
config[id].removableAt = uint64(block.timestamp + timelock); | ||
|
@@ -480,9 +481,14 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph | |
} | ||
|
||
/// @inheritdoc IMetaMorphoBase | ||
function acceptCap(Id id) external afterTimelock(pendingCap[id].validAt) { | ||
function acceptCap(MarketParams memory marketParams) | ||
external | ||
afterTimelock(pendingCap[marketParams.id()].validAt) | ||
{ | ||
Id id = marketParams.id(); | ||
|
||
// Safe "unchecked" cast because pendingCap <= type(uint184).max. | ||
_setCap(id, uint184(pendingCap[id].value)); | ||
_setCap(marketParams, id, uint184(pendingCap[id].value)); | ||
} | ||
|
||
/// @inheritdoc IMetaMorphoBase | ||
|
@@ -739,7 +745,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph | |
} | ||
|
||
/// @dev Sets the cap of the market defined by `id` to `supplyCap`. | ||
function _setCap(Id id, uint184 supplyCap) internal { | ||
/// @dev Assumes that the inputs `marketParams` and `id` match. | ||
function _setCap(MarketParams memory marketParams, Id id, uint184 supplyCap) internal { | ||
MarketConfig storage marketConfig = config[id]; | ||
|
||
if (supplyCap > 0) { | ||
|
@@ -750,6 +757,8 @@ contract MetaMorpho is ERC4626, ERC20Permit, Ownable2Step, Multicall, IMetaMorph | |
|
||
marketConfig.enabled = true; | ||
|
||
_updateLastTotalAssets(lastTotalAssets + MORPHO.expectedSupplyAssets(marketParams, address(this))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If not documented yet, consider documenting the side effects of this operation. The share value (of MM) is increased. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should there be a check that reverts if the market is broken?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's ok not to revert in this case. Vaults curators should not list broken markets in the first place. |
||
|
||
emit EventsLib.SetWithdrawQueue(msg.sender, withdrawQueue); | ||
} | ||
|
||
|
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.
Consider avoiding executing this function if
MORPHO.expectedSupplyAssets(marketParams, address(this))
is equal to zero.I would say that the normal scenario should be that you are enabling a market where no one has supplied on behalf of MM.
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.
True, but it would save only ~2k gas since we still need to call
expectedSupplyAssets
. In the current configuration we're adding one coldSLOAD
and one warmSSTORE
that is not changing the value. So no sure it's worth considering this case.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'm not sure it's worth it considering this case. The current implementation only adds one cold
SLOAD
and one warmSSTORE
so around 2k gas.