Skip to content

Add preliminary support for updating emode category parameters with the config engine#88

Open
reem wants to merge 1 commit intoaave-dao:mainfrom
Gauntlet-xyz:emode-category-updates
Open

Add preliminary support for updating emode category parameters with the config engine#88
reem wants to merge 1 commit intoaave-dao:mainfrom
Gauntlet-xyz:emode-category-updates

Conversation

@reem
Copy link
Copy Markdown
Contributor

@reem reem commented Apr 26, 2023

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.

);
}

if (eModeCategories.length != 0) {
Copy link
Copy Markdown
Contributor

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


/// @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';
Copy link
Copy Markdown
Contributor

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);
Copy link
Copy Markdown
Contributor

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


function _configEModeCategories(EModeCategories[] memory updates) internal {
for (uint256 i = 0; i < updates.length; i++) {
if (
Copy link
Copy Markdown
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants