Add a timelock to `ModuleGlobals.setTreasuryFee()`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/modules/ModuleGlobals.sol#L60-L62 https://github.com/code-423n4/2022-02-aave-lens/blob/main/contracts/core/modules/ModuleGlobals.sol#L108-L113

Vulnerability details

Impact

It is a good practice to give time for users to react and adjust to critical changes. A timelock provides more guarantees and reduces the level of trust required, thus decreasing risk for users. It also indicates that the project is legitimate.

Here, no timelock capabilities are implemented on ModuleGlobals.setTreasuryFee(), which is a function that can make some users want to react.

Proof of Concept

File: ModuleGlobals.sol
60:     function setTreasuryFee(uint16 newTreasuryFee) external override onlyGov {
61:         _setTreasuryFee(newTreasuryFee);
62:     }
...
108:     function _setTreasuryFee(uint16 newTreasuryFee) internal {
109:         if (newTreasuryFee >= BPS_MAX / 2) revert Errors.InitParamsInvalid(); //@audit-ok good
110:         uint16 prevTreasuryFee = _treasuryFee;
111:         _treasuryFee = newTreasuryFee;
112:         emit Events.ModuleGlobalsTreasuryFeeSet(prevTreasuryFee, newTreasuryFee, block.timestamp); //@audit-ok good
113:     }

Recommended Mitigation Steps

Consider using a timelock for ModuleGlobals.setTreasuryFee(). It's a good thing that an event is already being emitted and that _treasuryFee is upper-bounded (BPS_MAX / 2 where BPS_MAX = 10000).

[oneski]

Structure will be a timelock... per comment on #3... this is a recommendation not a bug.

[0xleastwood]

The governance role is held behind a timelock, hence this is already in effect.