code-423n4 / 2021-12-yetifinance-findings

0 stars 0 forks source link

`HintHelpers.sol#setAddresses()` can be replaced with `constructor` and save gas #277

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

WatchPug

Vulnerability details

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/HintHelpers.sol#L31-L52

function setAddresses(
        address _sortedTrovesAddress,
        address _troveManagerAddress,
        address _whitelistAddress
    )
        external
        onlyOwner
    {
        checkContract(_sortedTrovesAddress);
        checkContract(_troveManagerAddress);
        checkContract(_whitelistAddress);

        sortedTroves = ISortedTroves(_sortedTrovesAddress);
        troveManager = ITroveManager(_troveManagerAddress);
        whitelist = IWhitelist(_whitelistAddress);

        emit SortedTrovesAddressChanged(_sortedTrovesAddress);
        emit TroveManagerAddressChanged(_troveManagerAddress);
        emit WhitelistAddressChanged(_troveManagerAddress);

        _renounceOwnership();
    }

Across the codebase, including in HintHelpers.sol, setAddresses() is being used as a initializer function, it's a onlyOwner function and it will _renounceOwnership() at the end of the function.

There are no other onlyOwner functions, the addresses set in the function are immutable after being set.

Therefore, setAddresses() can be replaced with constructor and all the addresses can be declared as immutable variables instead of storage variables.

And Ownable can be removed to further reduce gas costs.

This issue also applies to:

kingyetifinance commented 2 years ago

@LilYeti : Confirm for HintHelpers. This issue does not apply to other contracts other than hinthelpers, since we need the other addresses to set them. No other contract takes hinthelpers though, so this could be used in the constructor technically.