feat: risk stewards

[brotherlymite]

No description provided.

[eboadom]

1. Why using data provider instead of the pool directly?
2. Probably makes sense, but what is the rationale of _restrictedAssets?
3. Why updating the …lastUpdated on the validation functions?
4. Check that comments are not referring by mistake to “config engine” instead of risk steward
5. RiskConfigSet event should emit the data
6. Why other events only emit asset and not data?
7. Could it be cleaner design-wise to have a system of enums for updates and 2 function: validate() and update(), and within it, just if..else doing each validation and type of update. Leaving the external functions only for access control and explicit interface. Specially on the validation stage, you have common checks for all updates, like restricted assets
   Or is there any rationale of compatibility with config engine I am missing?
8. I would set the isChangeRelative as part of the data maybe
9. On _updateWithinAllowedRange, maxDiff seems to be 0 quite frequently: if maxPercentChange is for example 50 and from is 1_00 = 50_00 / 100_00 = 0
10. I don’t understand how it works when the config of change is absolute

[brotherlymite]

1. Why using data provider instead of the pool directly?
2. Probably makes sense, but what is the rationale of _restrictedAssets?
3. Why updating the …lastUpdated on the validation functions?
4. Check that comments are not referring by mistake to “config engine” instead of risk steward
5. RiskConfigSet event should emit the data
6. Why other events only emit asset and not data?
7. Could it be cleaner design-wise to have a system of enums for updates and 2 function: validate() and update(), and within it, just if..else doing each validation and type of update. Leaving the external functions only for access control and explicit interface. Specially on the validation stage, you have common checks for all updates, like restricted assets
   Or is there any rationale of compatibility with config engine I am missing?
8. I would set the isChangeRelative as part of the data maybe
9. On _updateWithinAllowedRange, maxDiff seems to be 0 quite frequently: if maxPercentChange is for example 50 and from is 1_00 = 50_00 / 100_00 = 0
10. I don’t understand how it works when the config of change is absolute

1. No strong opinion, thought it would be cleaner to use the data provider
2. The rationale was to give the governance some flexibility to disallow certain assets to be used by stewards - for example GHO, and perhaps LST / LRT’s or some other token for which the community has opinionated views and wishes to go with standard governance procedure.
3. Agree, have refactored it
4. Was not able to find any, could you point out if you noticed any comment
5. Fixed
6. Fixed
7. Refactored a bit, but not sure if I should abstract it more as the function params are different.
   Could be done via: validate(Type.CAPS_UPDATE, abi.encode(capsUpdate)) instead of _validateCapsUpdate(capsUpdate) but not sure if it would do much
8. I didn’t quite get it, did you mean abstracting this logic to a separate function
9. maxPercentChange is actually in BPS, so 50 is actually 0.5%, so if from is 1_00 the max diff is 0.5 and the max value on the upper side is 100.5, which cannot be represented and so it is taking the lower bound to find the max allowed diff.
10. So lets say we use the absolute config for interest rate slope 1 and configure it to be 5_00 (5%), so over here the max diff allowed is the maxPercentChange itself which is 5_00.
    Example: If the current interest rate slope 1 is 5_00 (5%) so if the value being set is 9_00 (9%) it is allowed while if it is 12_00 (12%) it is disallowed

[kyzia551]

If the system will be changed to params per asset, then I would put debounce to this struct as well. It could simplify the code a bit

[kyzia551]

Suggested change

	// diff denotes the difference between the from and to values, ensuring it is a positive value always
	int256 diff = int256(from) - int256(to);
	if (diff < 0) diff = -diff;
	uint256 diff = from > to ? from - to : to - from;

that way we will sure that it's positive, because it's uint

[kyzia551]

Suggested change

	if (uint256(diff) > maxDiff) return false;
	if (diff > maxDiff) return false;

following previous change