search

code-423n4 / 2022-03-rolla-findings

1 stars 1 forks source link

Gas Optimizations #19

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

No need to define data variable at various functions in ConfigTimelockController.sol

Target codebase

_encodeSetProtocolAddress function is called at the top of the scheduleSetProtocolAddress function. 

bytes memory data = _encodeSetProtocolAddress(
    protocolAddress,
    newAddress,
    quantConfig
);

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/timelock/ConfigTimelockController.sol#L71-L75

data variable does not need to be defined.

Potential workarounds

Move the call of _encodeSetProtocolAddress into super.schedule as follows:

function scheduleSetProtocolAddress(
    bytes32 protocolAddress,
    address newAddress,
    address quantConfig,
    uint256 eta
) public onlyRole(PROPOSER_ROLE) {
    uint256 delay = _getProtocolValueDelay(
        quantConfig,
        protocolAddress,
        ProtocolValue.Type.Address
    );

    require(
        eta >= delay + block.timestamp,
        "ConfigTimelockController: Estimated execution block must satisfy delay"
    );

    super.schedule(
        quantConfig,
        0,
        _encodeSetProtocolAddress(
            protocolAddress,
            newAddress,
            quantConfig
        ),
        bytes32(0),
        bytes32(eta),
        delay,
        true
    );
}

There are other codebase which defines bytes memory data at the top of the functions which can do the same refactors. https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/timelock/ConfigTimelockController.sol#L110-L114 https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/timelock/ConfigTimelockController.sol#L149-L153 https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/timelock/ConfigTimelockController.sol#L187-L191

Gas improvements

Confirmed that the gas fees when calling these methods decreased. However, deployments gas fee of ConfigTimelockController.sol slightly increased.

Gas reduction is possible by refactoring scheduleBatch function

Target codebase

Following codebase has two potential improvements:

uint256 length = targets.length;
for (uint256 i = 0; i < length; ) {
    require(
        !_isProtocoValueSetter(datas[i]),
        "ConfigTimelockController: Can not schedule changes to a protocol value with an arbitrary delay"
    );
    unchecked {
        ++i;
    }
}

Potential workarounds

for (uint256 i; i < targets.length; ) {
    require(
        !_isProtocoValueSetter(datas[i]),
        "ConfigTimelockController: Can not schedule changes to a protocol value with an arbitrary delay"
    );
    unchecked {
        ++i;
    }
}

Gas improvements

Confirmed that gas fee of deployment of ConfigTimelockController.sol decreased slightly.

ProviderOracleManager.sol calls assetOracles[_asset] multiple times

Target codebase

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/pricing/oracle/ProviderOracleManager.sol#L76-L78

address assetOracle = assetOracles[_asset];
require(
    assetOracles[_asset] != address(0),
    "ProviderOracleManager: Oracle doesn't exist for that asset"
);
return assetOracle;

assetOracles[_asset] is called in require function although assetOracle variable is defined.

Potential workarounds

Here is a potential workaround:

address assetOracle = assetOracles[_asset];
require(
    assetOracle != address(0),
    "ProviderOracleManager: Oracle doesn't exist for that asset"
);
return assetOracle;

No need to define roundAfterExpiry variable at setExpiryPriceInRegistry function in ChainlinkOracleManager.sol

Target codebase

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/pricing/oracle/ChainlinkOracleManager.sol#L63-L70

uint80 roundAfterExpiry = searchRoundToSubmit(_asset, _expiryTimestamp);

//submit price to registry
_setExpiryPriceInRegistryByRound(
    _asset,
    _expiryTimestamp,
    roundAfterExpiry
);

roundAfterExpiry variable is defined, which is not necessary.

Potential workarounds

Simply calling searchRoundToSubmit(_asset, _expiryTimestamp) in _setExpiryPriceInRegistryByRound function.

_setExpiryPriceInRegistryByRound(
    _asset,
    _expiryTimestamp,
    searchRoundToSubmit(_asset, _expiryTimestamp)
);

Gas improvements

Confirmed that both methods and deployments gas fee decreased for following functions and contracts.

ChainlinkFixedTimeOracleManager, setExpiryPriceInRegistry function
ChainlinkOracleManager, setExpiryPriceInRegistry function
ChainlinkFixedTimeOracleManager.sol
ChainlinkOracleManager.sol

isValidOption function in ChainlinkFixedTimeOracleManager.sol can use unchecked to decrease the gas fee

Target codebase

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/pricing/oracle/ChainlinkFixedTimeOracleManager.sol#L63-L64

uint256 timeInSeconds = _expiryTime % 86400;
return chainlinkFixedTimeUpdates[timeInSeconds];

_expiryTime % 86400 can be wrapped by unchecked function since % operations on uint256 will not underflow.

Potential workarounds

Simply wrap by unchecked directory like this:

function isValidOption(
    address,
    uint256 _expiryTime,
    uint256
)
    public
    view
    override(ChainlinkOracleManager, IProviderOracleManager)
    returns (bool)
{
    unchecked {
        return chainlinkFixedTimeUpdates[_expiryTime % 86400];
    }
}

Gas improvements

Following gas reductions can be achieved:

CollateralToken.sol can reduce gas usage by simple refactoring

Target codebase

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/options/CollateralToken.sol#L151-L157

uint256 length = ids.length;
for (uint256 i = 0; i < length; ) {
    emit CollateralTokenMinted(recipient, ids[i], amounts[i]);
    unchecked {
        ++i;
    }
}

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/options/CollateralToken.sol#L177-L183

uint256 length = ids.length;
for (uint256 i = 0; i < length; ) {
    emit CollateralTokenBurned(owner, ids[i], amounts[i]);
    unchecked {
        ++i;
    }
}

Potential workarounds

for (uint256 i; i < ids.length; ) {
    emit CollateralTokenMinted(recipient, ids[i], amounts[i]);
    unchecked {
        ++i;
    }
}
for (uint256 i; i < ids.length; ) {
    emit CollateralTokenBurned(owner, ids[i], amounts[i]);
    unchecked {
        ++i;
    }
}

Gas improvements

Gas fee for CollateralToken deployment can be reduced. Gas fee for burnCollateralTokenBatch and mintCollateralTokenBatch functions in CollateralToken.sol are slighly decreased.

Can use != rather than using > at Controller.sol

Target codebase

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/Controller.sol#L268

if (collateralAmount > 0) {

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/Controller.sol#L393

if (returnableCollateral > 0) {

Potential workarounds

Can use != rather than using >.

if (collateralAmount != 0) {
if (returnableCollateral != 0) {

Gas reduction is possible by refactoring operate function in Controller.sol

Target codebase

https://github.com/code-423n4/2022-03-rolla/blob/main/quant-protocol/contracts/Controller.sol#L55-L56

uint256 length = _actions.length;
for (uint256 i = 0; i < length; ) {

Potential workarounds

Can use following codebase which results in gas reduction.

for (uint256 i; i < _actions.length; ) {
alcueca commented 2 years ago

Score: 12