Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reem
Copy link
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.

@@ -92,6 +94,12 @@ abstract contract AaveV3PayloadBase {
);
}

if (eModeCategories.length != 0) {
Copy link
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

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

@@ -453,6 +472,59 @@ contract AaveV3ConfigEngine is IAaveV3ConfigEngine {
}
}

function _configEModeCategories(EModeCategories[] memory updates) internal {
for (uint256 i = 0; i < updates.length; i++) {
if (
Copy link
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