Treasury Code Style Findings

TRE-01C: Improper Failure Enforcement

Description:

The require check linked performs a guaranteed-to-fail check (1 == 0) to illustrate the error message that accompanies it.

Example:

Copy
106require(1 == 0, "neither reserve nor liquidity token"); // guarantee revert

Recommendation:

We advise a revert to be used directly instead that accepts the error message directly.

Alleviation:

The require check was substituted for a revert check as recommended.

TRE-02C: Improper Permitted Execution Flow

Description:

The enableOnChainGovernance function should not be invoke-able if the onChainGoverned status has already been set.

Example:

Copy
353/**
354 * @notice disables timelocked functions
355 */
356function enableOnChainGovernance() external onlyOwner {
357    if (onChainGovernanceTimelock != 0 && onChainGovernanceTimelock <= block.number) {
358        onChainGoverned = true;
359    } else {
360        onChainGovernanceTimelock = block.number.add(blocksNeededForQueue.mul(7)); // 7-day timelock
361    }
362}

Recommendation:

We advise this to be enforced by introducing a require check that prevents this scenario at the top of the function.

Alleviation:

The function now properly validates that onChainGoverned has not been set already.

TRE-03C: Inexistent Error Messages

Description:

The linked require checks contain no descriptive error messages.

Example:

Copy
80constructor(address _OHM, uint256 _timelock) {
81    require(_OHM != address(0));
82    OHM = IOHMERC20(_OHM);
83
84    blocksNeededForQueue = _timelock;
85}

Recommendation:

We advise them to be set so to aid in the debugging of the application and to also enable more accurate validation of the require condition purposes.

Alleviation:

Error messages were introduced in all linked require checks.

TRE-04C: Inexistent Variable Visibility Specifier

Description:

The linked variable has no visibility specifier explicitly set.

Example:

Copy
60IOHMERC20 immutable OHM;

Recommendation:

We advise one to be set so to avoid potential compilation discrepancies in the future as the current compiler behaviour is to assign a specifier automatically.

Alleviation:

The public visibility specifier was explicitly set to the linked variable.