Sherlock contest - RamiRond's results

Handle

RamiRond

Vulnerability details

Impact

Gas saving.

Proof of Concept

Reading from storage (SLOAD) is quite expensive, so it makes sense to cache these values in memory if they are needed multiple times in the same scope. I verified most of these findings with hardhat-gas-reporter; with the projects optimizer settings this saves at least 140 gas for each call and occurence. It can be applied to the mentioned state variables in the following functions:

SherlockClaimManager.sol

• _setState: Use the exisiting storage reference variable claim in lines 197 and 198
• addCallback: claimCallbacks.length
• escalate: claim.timestamp

Sherlock.sol

• updateYieldStrategy, yieldStrategyDeposit: yieldStrategy
• pause, unpause: yieldStrategy, sherDistributionManager, sherlockProtocolManager, sherlockClaimManager
• _stake: sherDistributionManager

Tools Used

vscode, solidity visual auditor, hardhat-gas-reporter

Recommended Mitigation Steps

Use a local memory variable as cache to avoid SLOADs.

Handle

RamiRond

Vulnerability details

Impact

Gas saving.

Proof of Concept

An assignment also returns the new value, so it can be processed further. This can be used with storage variables to save gas. According to hardhat-gas-reporter it's about 110 gas per call and occurence with the projects optimizer settings. This can impair readability in many cases, for example in Sherlock.sol::enableStakingPeriod:

function enableStakingPeriod(uint256 _period) public override onlyOwner {
  if (_period == 0) revert ZeroArgument();
  if (!(stakingPeriods[_period] = !stakingPeriods[_period])) revert InvalidArgument();
  emit StakingPeriodEnabled(_period);
}

In other cases, however, the impact is less severe. For example, Sherlock.sol::updateSherDistributionManager could be changed to this:

function updateSherDistributionManager(ISherDistributionManager _sherDistributionManager) external override onlyOwner {
    if (address(_sherDistributionManager) == address(0)) revert ZeroArgument();
    if (sherDistributionManager == _sherDistributionManager) revert InvalidArgument();

    emit SherDistributionManagerUpdated(sherDistributionManager,
    sherDistributionManager = _sherDistributionManager);
  }

Here's a list of cases where impact on readability would be acceptable:

Sherlock.sol

• removeSherDistributionManager
• updateNonStakersAddress
• updateSherlockProtocolManager
• updateSherlockClaimManager

Tools Used

vscode, solidity visual auditor, hardhat-gas-reporter

Recommended Mitigation Steps

Inline assignments to save gas.