hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

The smart contracts of the Intuition protocol v1.
https://intuition.systems
Other
0 stars 1 forks source link

`minDelay` once set can not be changed later #28

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xb779063340445861794012928095f3ba48e1d34bd93eb9143c2bdbd1d25c7675 Severity: medium

Description: Description\

EthMultiVault.sol has used minDelay to execute certain transactions after passing some delay i.e a timelock mechanism is implemented for Admin and Exit fee setter functions. When this minimum delay is passed then only these setters can be executed.

The issue is that, once minDelay is set then it can not be set again. It would become immutable.

GeneralConfig is a General configuration struct used in EthMultiVault.sol contract and below snip shows which of its variables have setter function to set their values again.

    struct GeneralConfig {
        address admin;                         -------Can be set again via `setAdmin()`
        address protocolVault;             -------Can be set again via `setProtocolVault()`
        uint256 feeDenominator;         -------Can NOT set again
        uint256 minDeposit;                 -------Can be set again via `setMinDeposit()`
        uint256 minShare;                    -------Can be set again via `setMinShare()`
        uint256 atomUriMaxLength;     -------Can be set again via `setAtomUriMaxLength()`
        uint256 decimalPrecision;       -------Can NOT set again
        uint256 minDelay;                    -------Can NOT set again
    }

As can be seen, the most important minDelay can not be set as it does not have any setter functions.

It should be noted that, Intuition also used Openzeppelin's TimelockController.sol` to manage contract upgrades has set a 2 days minimum delay and openzeppelin allows to change the minDelay which is implemented as:

    function updateDelay(uint256 newDelay) external virtual {
        address sender = _msgSender();
        if (sender != address(this)) {
            revert TimelockUnauthorizedCaller(sender);
        }
        emit MinDelayChange(_minDelay, newDelay);
        _minDelay = newDelay;
    }

Therefore, there would be inconsistent design where timelockController for contracts upgrades allows to set the minDelay again and critical setters i.e Admin and Exit fee in EthMultiVault, minDelay can not be changed again.

Recommendations\ Allow to set the minDelay by adding admin restricted setMinDelay() function. As explained above, this would be inline with openzeppelin's timelockController.

mihailo-maksa commented 1 week ago

The minDelay parameter's immutability is a deliberate design choice for the following reasons:

  1. Security and Stability: Keeping minDelay constant ensures the stability and predictability of the protocol's time-based operations. It prevents potential abuse or arbitrary changes that could affect users' expectations and trust.
  2. Upgradeable Contracts: In the unlikely event that a change to minDelay is necessary, the protocol can be upgraded. This approach maintains security while providing a mechanism for necessary changes.

In conclusion, the immutability of minDelay is intentional and contributes to the protocol's stability and security. Therefore, this issue is invalid.