Add a timelock to `TokenManager.sol:function changeFee()` + add upper-bounds to fees

[code423n4]

Lines of code

https://github.com/code-423n4/2022-03-biconomy/blob/db8a1fdddd02e8cc209a4c73ffbb3de210e4a81a/contracts/hyphen/token/TokenManager.sol#L44-L54

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 seem to be used:

File: TokenManager.sol
44:     function changeFee(
45:         address tokenAddress,
46:         uint256 _equilibriumFee,
47:         uint256 _maxFee
48:     ) external override onlyOwner whenNotPaused {
49:         require(_equilibriumFee != 0, "Equilibrium Fee cannot be 0");
50:         require(_maxFee != 0, "Max Fee cannot be 0");
51:         tokensInfo[tokenAddress].equilibriumFee = _equilibriumFee;
52:         tokensInfo[tokenAddress].maxFee = _maxFee;
53:         emit FeeChanged(tokenAddress, tokensInfo[tokenAddress].equilibriumFee, tokensInfo[tokenAddress].maxFee);
54:     }

I believe this impacts multiple users enough to make them want to react / be notified ahead of time.

Recommended Mitigation Steps

Consider adding a timelock to TokenManager.sol:function changeFee() Also, a definitive upper-bound should be added to prevent the fees from being too high.

[pauliax]

An owner can be a timelock contract. Nevertheless, I consider this as of low severity and thus putting to the QA report of the warden: #193