-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add preliminary support for updating emode category parameters with the config engine #88
base: master
Are you sure you want to change the base?
Conversation
…he config engine.
@@ -92,6 +94,12 @@ abstract contract AaveV3PayloadBase { | |||
); | |||
} | |||
|
|||
if (eModeCategories.length != 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.
As setEModeCategory()
on PoolConfigurator is used both to create a new e-mode category and update an existing one, better to have this step before listings.
That way it is possible to, in the same payload and without boiling down to preExecute()
, create an e-mode category and assign a new listing to it.
Not something too frequent I would say, but possible and doesn't hurt
@@ -6,6 +6,14 @@ library EngineFlags { | |||
/// Strongly assumes that the value `type(uint256).max - 42` will never be used, which seems reasonable | |||
uint256 internal constant KEEP_CURRENT = type(uint256).max - 42; | |||
|
|||
/// @dev magic value to be used as flag to keep unchanged any current configuration | |||
/// Strongly assumes that the value `AaveV3ConfigEngine.EngineFlags.KEEP_CURRENT_STRING` will never be used, which seems reasonable | |||
string internal constant KEEP_CURRENT_STRING = 'AaveV3ConfigEngine.EngineFlags.KEEP_CURRENT_STRING'; |
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.
Maybe not necessary to have so long string on this, as I don't see any chance for KEEP_CURRENT_STRING
to be used anyhow in the context of any string config on Aave
|
||
/// @dev magic value to be used as flag to keep unchanged any current configuration | ||
/// Strongly assumes that the value `0x00000000000000000000000000000000000042` will never be used, which seems reasonable | ||
address internal constant KEEP_CURRENT_ADDRESS = address(0x00000000000000000000000000000000000042); |
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'd say better something different. Projects tend to use the 42
, so even if maybe not clashing now, you never now.
E.g. https://optimistic.etherscan.io/address/0x4200000000000000000000000000000000000000
@@ -453,6 +472,59 @@ contract AaveV3ConfigEngine is IAaveV3ConfigEngine { | |||
} | |||
} | |||
|
|||
function _configEModeCategories(EModeCategories[] memory updates) internal { | |||
for (uint256 i = 0; i < updates.length; i++) { | |||
if ( |
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.
A case that is missing here (and actually in some other functions that we are fixing) is all params == KEEP_CURRENT.
Right now it will still query the pool and call setEModeCategory
, which even if not damaging, it is unnecessary
Still needs some finishing touches like a _validate function for emode category changes and more checks on the params before calling into the pool configurator, and the new engine would need to be deployed before this was usable downstream.