LSD Network - Stakehouse contest - 0xSmartContract's results

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LPTokenFactory.sol#L27-L48

Vulnerability details

Impact

Mistakes made on one chain can be re-applied to a new chain

There is no chain.id in the data

If a user does deployLPToken using the wrong network, an attacker can replay the action on the correct chain, and steal the funds a-la the wintermute gnosis safe attack, where the attacker can create the same address that the user tried to, and steal the funds from there

https://mirror.xyz/0xbuidlerdao.eth/lOE5VN-BHI0olGOXe27F0auviIuoSlnou_9t3XRJseY

Proof of Concept

contracts/liquid-staking/LPTokenFactory.sol:
  26      /// @param _tokenName Name of the LP token to be deployed
  27:     function deployLPToken(
  28:         address _deployer,
  29:         address _transferHookProcessor,
  30:         string calldata _tokenSymbol,
  31:         string calldata _tokenName
  32:     ) external returns (address) {
  33:         require(address(_deployer) != address(0), "Zero address");
  34:         require(bytes(_tokenSymbol).length != 0, "Symbol cannot be zero");
  35:         require(bytes(_tokenName).length != 0, "Name cannot be zero");
  36: 
  37:         address newInstance = Clones.clone(lpTokenImplementation);
  38:         ILPTokenInit(newInstance).init(
  39:             _deployer,
  40:             _transferHookProcessor,
  41:             _tokenSymbol,
  42:             _tokenName
  43:         );
  44: 
  45:         emit LPTokenDeployed(newInstance);
  46: 
  47:         return newInstance;
  48:     }

Tools Used

Manual Code Review

Recommended Mitigation Steps

Include the chain.id 

Summary

Low Risk Issues List

Total 12 issues

Non-Critical Issues List

Total 13 issues

Suggestions

Total 1 suggestion

[L-01] Draft Openzeppelin Dependencies

The LPToken.sol contract utilised draft-ERC20PermitUpgradeable.sol , an OpenZeppelin contract. This contract is still a draft and is not considered ready for mainnet use. OpenZeppelin contracts may be considered draft contracts if they have not received adequate security auditing or are liable to change with future development.

LPToken.sol#L6

contracts/liquid-staking/LPToken.sol:

  6: import { ERC20PermitUpgradeable } from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/draft-ERC20PermitUpgradeable.sol";

[L-02] Stack too deep when compiling

The project cannot be compiled due to the "stack too deep" error.

The “stack too deep” error is a limitation of the current code generator. The EVM stack only has 16 slots and that’s sometimes not enough to fit all the local variables, parameters and/or return variables. The solution is to move some of them to memory, which is more expensive but at least makes your code compile.

[⠒] Compiling...
[⠰] Compiling 100 files with 0.8.13
[⠔] Solc 0.8.13 finished in 3.35s
Error: 
Compiler run failed
CompilerError: Stack too deep when compiling inline assembly: Variable headStart is 1 slot(s) too deep inside the stack.

ref:https://forum.openzeppelin.com/t/stack-too-deep-when-compiling-inline-assembly/11391/6

[L-03] Remove unused code

This code is not used in the project, remove it or add event-emit;

contracts/liquid-staking/GiantPoolBase.sol:

  100:     function _onDepositETH() internal virtual {}
  103:     function _onWithdraw(LPToken[] calldata _lpTokens) internal virtual {}
  104  }

[L-04] Insufficient coverage

Description: Testing all functions is best practice in terms of security criteria.

This function test coverage is not found in test files

 function rawExecute(
        address target,
        bytes memory callData,
        uint256 value
    )
    external
    override
    payable
    onlyOwner
    returns (bytes memory)
    {
        (bool result, bytes memory message) = target.call{value: value}(callData);
        require(result, "Failed to execute");
        return message;
    }

Due to its capacity, test coverage is expected to be 100%

[L-05] Critical Address Changes Should Use Two-step Procedure

The critical procedures should be two step process.

contracts/smart-wallet/OwnableSmartWallet.sol:
   94:     function transferOwnership(address newOwner)
   95:         public
   96:         override(IOwnableSmartWallet, Ownable)
   97:     {

Recommended Mitigation Steps Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.

[L-06] Avoid variable names that can shade

With global variable names in the form of call{value: value } , argument name similarities can shade and negatively affect code readability.

contracts/smart-wallet/OwnableSmartWallet.sol:
  77:     {
  78:         (bool result, bytes memory message) = target.call{value: value}(callData);
  79:         require(result, "Failed to execute");
  80:         return message;
  81:     }

[L-07] Use a more recent version of Solidity

Context: All contracts

Description: For security, it is best practice to use the latest Solidity version. For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md

Recommendation: Old version of Solidity is used (0.8.13), newer version can be used (0.8.17)

[L-08] Owner can renounce Ownership

Context:

LiquidStakingManager.sol#L6 Syndicate.sol#L8

Description: Typically, the contract’s owner is the account that deploys the contract. As a result, the owner is able to perform certain privileged activities.

The StakeHouse Ownable used in this project contract implements renounceOwnership. This can represent a certain risk if the ownership is renounced for any other reason than by design. Renouncing ownership will leave the contract without an owner, thereby removing any functionality that is only available to the owner.

onlyOwner functions;

8 results - 2 files

contracts/smart-wallet/OwnableSmartWallet.sol:
   44          payable
   45:         onlyOwner // F: [OSW-6A]
   46          returns (bytes memory)

   59          payable
   60:         onlyOwner // F: [OSW-6A]
   61          returns (bytes memory)

   74      payable
   75:     onlyOwner
   76      returns (bytes memory)

  114:     function setApproval(address to, bool status) external onlyOwner override {

contracts/syndicate/Syndicate.sol:
  147:     ) external onlyOwner {

  154:     function deRegisterKnots(bytes[] calldata _blsPublicKeys) external onlyOwner {

  161:     function addPriorityStakers(address[] calldata _priorityStakers) external onlyOwner {

  168:     function updatePriorityStakingBlock(uint256 _endBlock) external onlyOwner {

Recommendation: We recommend to either reimplement the function to disable it or to clearly specify if it is part of the contract design.

[L-09] Lock pragmas to specific compiler version

Description: Pragma statements can be allowed to float when a contract is intended for consumption by other developers, as in the case with contracts in a library or EthPM package. Otherwise, the developer would need to manually update the pragma in order to compile locally. https://swcregistry.io/docs/SWC-103

Recommendation: Ethereum Smart Contract Best Practices - Lock pragmas to specific compiler version. solidity-specific/locking-pragmas

24 files

pragma solidity ^0.8.13;
contracts/interfaces/IBrandNFT.sol:
contracts/interfaces/ILiquidStakingManagerChildContract.sol:
contracts/interfaces/ILPTokenInit.sol:
contracts/interfaces/ISyndicateFactory.sol:
contracts/interfaces/ISyndicateInit.sol:
contracts/interfaces/ITransferHookProcessor.sol:
contracts/liquid-staking/ETHPoolLPFactory.sol:
contracts/liquid-staking/GiantLP.sol:
contracts/liquid-staking/GiantMevAndFeesPool.sol:
contracts/liquid-staking/GiantPoolBase.sol:
contracts/liquid-staking/GiantSavETHVaultPool.sol:
contracts/liquid-staking/LiquidStakingManager.sol:
contracts/liquid-staking/LPToken.sol:
contracts/liquid-staking/LPTokenFactory.sol:
contracts/liquid-staking/LSDNFactory.sol:
contracts/liquid-staking/OptionalGatekeeperFactory.sol:
contracts/liquid-staking/OptionalHouseGatekeeper.sol:
contracts/liquid-staking/SavETHVault.sol:
contracts/liquid-staking/SavETHVaultDeployer.sol:
contracts/liquid-staking/StakingFundsVault.sol:
contracts/liquid-staking/StakingFundsVaultDeployer.sol:
contracts/liquid-staking/SyndicateRewardsProcessor.sol:
contracts/smart-wallet/OwnableSmartWallet.sol:
contracts/smart-wallet/OwnableSmartWalletFactory.sol:

[L-10] Loss of precision due to rounding

Due to / PRECISION, users can avoid paying fee if claimed [][] result is below PRECISION

contracts/liquid-staking/GiantMevAndFeesPool.sol:
  199  
  200:     /// @dev Internal re-usable method for setting claimed to max for msg.sender
  201:     function _setClaimedToMax(address _user) internal {
  202:         // New ETH stakers are not entitled to ETH earned by
  203:         claimed[_user][address(lpTokenETH)] = (accumulatedETHPerLPShare * lpTokenETH.balanceOf(_user)) / PRECISION;
  204:     }

Recommendation: A lower limit can be added to the claimed values

[L-11] Using vulnerable dependency of OpenZeppelin

The package.json configuration file says that the project is using 4.5.0 of OZ which has a not last update version

1 result - 1 file

package.json:
  10:   "dependencies": {
  14:     "@openzeppelin/contracts": "^4.5.0",
  15:     "@openzeppelin/contracts-upgradeable": "4.5.0",

VULNERABILITY	VULNERABLE VERSION
H    	Improper Verification of Cryptographic Signature	<4.7.3
M   	Denial of Service (DoS)	>=2.3.0 <4.7.2
L     	Incorrect Resource Transfer Between Spheres	 >=4.6.0 <4.7.2
H	Incorrect Calculation	>=4.3.0 <4.7.2
H	Information Exposure	>=4.1.0 <4.7.1
H	Information Exposure	>=4.0.0 <4.7.1

Recommendation: Use patched versions Latest non vulnerable version 4.8.0

[L-12] Use safeTransferOwnership instead of transferOwnership function

Context: LiquidStakingManager.sol#L6 Syndicate.sol#L8

contracts/smart-wallet/OwnableSmartWallet.sol:
   93      /// @inheritdoc IOwnableSmartWallet
   94:     function transferOwnership(address newOwner)
   95:         public
   96:         override(IOwnableSmartWallet, Ownable)
   97:     {
   98:         // Only the owner themselves or an address that is approved for transfers
   99:         // is authorized to do this
  100:         require(
  101:             isTransferApproved(owner(), msg.sender),
  102:             "OwnableSmartWallet: Transfer is not allowed"
  103:         ); // F: [OSW-4]
  104: 
  105:         // Approval is revoked, in order to avoid unintended transfer allowance
  106:         // if this wallet ever returns to the previous owner
  107:         if (msg.sender != owner()) {
  108:             _setApproval(owner(), msg.sender, false); // F: [OSW-5]
  109:         }
  110:         _transferOwnership(newOwner); // F: [OSW-5]
  111:     }

Description: transferOwnership function is used to change Ownership

Use a 2 structure transferOwnership which is safer. safeTransferOwnership, use it is more secure due to 2-stage ownership transfer.

Recommendation: Use Ownable2Step.sol Ownable2Step.sol

 /**
     * @dev The new owner accepts the ownership transfer.
     */
    function acceptOwnership() external {
        address sender = _msgSender();
        require(pendingOwner() == sender, "Ownable2Step: caller is not the new owner");
        _transferOwnership(sender);
    }
}

[NC-01] 0 address check

0 address control should be done in these parts;

Context: GiantLP.sol#L20-L21 LiquidStakingManager.sol#L170-L177 LPToken.sol#L33-L34 OptionalHouseGatekeeper.sol#L15 SavETHVault.sol#L45 Syndicate.sol#L130 SyndicateFactory.sol#L17

Recommendation: Add code like this; if (oracle == address(0)) revert ADDRESS_ZERO();

[NC-02] Add parameter to Event-Emit

Some event-emit description hasn’t parameter. Add to parameter for front-end website or client app , they can has that something has happened on the blockchain.

contracts/syndicate/Syndicate.sol:
  468      /// @dev Internal logic for initializing the syndicate contract
  469:     function _initialize(
  470:         address _contractOwner,
  471:         uint256 _priorityStakingEndBlock,
  472:         address[] memory _priorityStakers,
  473:         bytes[] memory _blsPubKeysForSyndicateKnots
  474:     ) internal {
  475:         // Transfer ownership from the deployer to the address specified as the owner
  476:         _transferOwnership(_contractOwner);
  477: 
  478:         // Add the initial set of knots to the syndicate
  479:         _registerKnotsToSyndicate(_blsPubKeysForSyndicateKnots);
  480: 
  481:         // Optionally process priority staking if the required params and array is configured
  482:         if (_priorityStakingEndBlock > block.number) {
  483:             priorityStakingEndBlock = _priorityStakingEndBlock;
  484:             _addPriorityStakers(_priorityStakers);
  485:         }
  486: 
  487:         emit ContractDeployed();
  488:     }

[NC-03] Omissions in Events

Throughout the codebase, events are generally emitted when sensitive changes are made to the contracts. However, some events are missing important parameters

The events should include the new value and old value where possible:

Events with no old value;

contracts/liquid-staking/LiquidStakingManager.sol:
  254      /// @notice Allow the DAO to rotate the network ticker before the network house is created
  255:     function updateTicker(string calldata _newTicker) external onlyDAO {
  256:         require(bytes(_newTicker).length >= 3, "String must be 3-5 characters long");
  257:         require(bytes(_newTicker).length <= 5, "String must be 3-5 characters long");
  258:         require(numberOfKnots == 0, "Cannot change ticker once house is created");
  259: 
  260:         stakehouseTicker = _newTicker;
  261: 
  262:         emit NetworkTickerUpdated(_newTicker);
  263      }

[NC-04] Include return parameters in NatSpec comments

Context: All Contracts

Description:

https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

If Return parameters are declared, you must prefix them with "/// @return".

Some code analysis programs do analysis by reading NatSpec details, if they can't see the "@return" tag, they do incomplete analysis.

Recommendation: Include return parameters in NatSpec comments

Recommendation Code Style:

   /// @notice information about what a function does
   /// @param pageId The id of the page to get the URI for.
   /// @return Returns a page's URI if it has been minted 
   function tokenURI(uint256 pageId) public view virtual override returns (string memory) {
       if (pageId == 0 || pageId > currentId) revert("NOT_MINTED");

       return string.concat(BASE_URI, pageId.toString());
   }

[NC-05] Use a more recent version of Solidity

Context: All contracts

Description: For security, it is best practice to use the latest Solidity version. For the security fix list in the versions; https://github.com/ethereum/solidity/blob/develop/Changelog.md

Recommendation: Old version of Solidity is used (^0.8.13), newer version can be used (0.8.17)

[NC-06] Solidity compiler optimizations can be problematic

hardhat.config.js:
   3  
   4: module.exports = {
   5:   solidity: {
   6:     version: "0.8.13",
   7:     settings: {
   8:       optimizer: {
   9:         enabled: true,
  10:         runs: 200
  11:       }

Description: Protocol has enabled optional compiler optimizations in Solidity. There have been several optimization bugs with security implications. Moreover, optimizations are actively being developed. Solidity compiler optimizations are disabled by default, and it is unclear how many contracts in the wild actually use them.

Therefore, it is unclear how well they are being tested and exercised. High-severity security issues due to optimization bugs have occurred in the past. A high-severity bug in the emscripten-generated solc-js compiler used by Truffle and Remix persisted until late 2018. The fix for this bug was not reported in the Solidity CHANGELOG.

Another high-severity optimization bug resulting in incorrect bit shift results was patched in Solidity 0.5.6. More recently, another bug due to the incorrect caching of keccak256 was reported. A compiler audit of Solidity from November 2018 concluded that the optional optimizations may not be safe. It is likely that there are latent bugs related to optimization and that new bugs will be introduced due to future optimizations.

Exploit Scenario A latent or future bug in Solidity compiler optimizations—or in the Emscripten transpilation to solc-js—causes a security vulnerability in the contracts.

Recommendation: Short term, measure the gas savings from optimizations and carefully weigh them against the possibility of an optimization-related bug. Long term, monitor the development and adoption of Solidity compiler optimizations to assess their maturity.

[NC-07] NatSpec is missing

Description: NatSpec is missing for the following functions , constructor and modifier:

27 results

contracts/interfaces/IBrandNFT.sol:
  6:     function toLowerCase(string memory _base) external pure returns (string memory);
  7:     function lowercaseBrandTickerToTokenId(string memory _ticker) external returns (uint256);

contracts/interfaces/ILiquidStakingManager.sol:
  7:     function stakehouse() external view returns (address);

contracts/interfaces/ILiquidStakingManagerChildContract.sol:
  6:     function liquidStakingManager() external view returns (address);

contracts/interfaces/ILPTokenInit.sol:
  7:     function init(

contracts/interfaces/ISyndicateInit.sol:
  7:     function initialize(

contracts/interfaces/ITransferHookProcessor.sol:
  6:     function beforeTokenTransfer(address _from, address _to, uint256 _amount) external;
  7:     function afterTokenTransfer(address _from, address _to, uint256 _amount) external;

contracts/liquid-staking/GiantLP.sol:
  29:     function mint(address _recipient, uint256 _amount) external {
  34:     function burn(address _recipient, uint256 _amount) external {
  39:     function _beforeTokenTransfer(address _from, address _to, uint256 _amount) internal override {
  43:     function _afterTokenTransfer(address _from, address _to, uint256 _amount) internal override {

contracts/liquid-staking/OptionalGatekeeperFactory.sol:
  11:     function deploy(address _liquidStakingManager) external returns (OptionalHouseGatekeeper) {

contracts/liquid-staking/SavETHVault.sol:
   45:     function init(address _liquidStakingManagerAddress, LPTokenFactory _lpTokenFactory) external virtual initializer {

contracts/liquid-staking/SavETHVaultDeployer.sol:
  18:     function deploySavETHVault(address _liquidStakingManger, address _lpTokenFactory) external returns (address) {

contracts/smart-wallet/OwnableSmartWalletFactory.sol:
  28:     function createWallet() external returns (address wallet) {
  32:     function createWallet(address owner) external returns (address wallet) {
  36:     function _createWallet(address owner) internal returns (address wallet) {

contracts/smart-wallet/interfaces/IOwnableSmartWalletFactory.sol:
   9:     function createWallet() external returns (address wallet);
  11:     function createWallet(address owner) external returns (address wallet);
  13:     function walletExists(address wallet) external view returns (bool);

contracts/testing/interfaces/IFactoryDependencyInjector.sol:
   6:     function accountMan() external view returns (address);
   8:     function txRouter() external view returns (address);
  10:     function uni() external view returns (address);
  12:     function slot() external view returns (address);
  14:     function saveETHRegistry() external view returns (address);
  16:     function dETH() external view returns (address);

[NC-08] Lines are too long

Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length Reference: https://docs.soliditylang.org/en/v0.8.10/style-guide.html#maximum-line-length

9 results

contracts/syndicate/Syndicate.sol:
  216:             if (!isKnotRegistered[_blsPubKey] || isNoLongerPartOfSyndicate[_blsPubKey]) revert KnotIsNotRegisteredWithSyndicate();

  447:         return ((calculateETHForFreeFloatingOrCollateralizedHolders() - lastSeenETHPerCollateralizedSlotPerKnot) / numberOfRegisteredKnots);

 511:                     accruedEarningPerCollateralizedSlotOwnerOfKnot[_blsPubKey][collateralizedOwnerAtIndex] += unprocessedETHForCurrentKnot;

contracts/liquid-staking/ETHPoolLPFactory.sol:
  92:             getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfPreviousKnot) == IDataStructures.LifecycleStatus.INITIALS_REGISTERED,

  97:             getAccountManager().blsPublicKeyToLifecycleStatus(blsPublicKeyOfNewKnot) ==IDataStructures.LifecycleStatus.INITIALS_REGISTERED,

contracts/liquid-staking/GiantLP.sol:
  40:         if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).beforeTokenTransfer(_from, _to, _amount);

  46:         if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).afterTokenTransfer(_from, _to, _amount);

contracts/liquid-staking/GiantMevAndFeesPool.sol:
   97:         return _previewAccumulatedETH(_user, address(lpTokenETH), lpTokenETH.balanceOf(_user), lpTokenETH.totalSupply(), accumulated);

  118:             StakingFundsVault(payable(_stakingFundsVaults[i])).batchRotateLPTokens(_oldLPTokens[i], _newLPTokens[i], _amounts[i]);

[NC-09] Missing Event for critical parameters change

contracts/smart-wallet/OwnableSmartWallet.sol:
  66      /// @inheritdoc IOwnableSmartWallet
  67:     function rawExecute(
  68:         address target,
  69:         bytes memory callData,
  70:         uint256 value
  71:     )
  72:     external
  73:     override
  74:     payable
  75:     onlyOwner
  76:     returns (bytes memory)
  77:     {
  78:         (bool result, bytes memory message) = target.call{value: value}(callData);
  79:         require(result, "Failed to execute");
  80:         return message;
  81:     }

Description: Events help non-contract tools to track changes, and events prevent users from being surprised by changes

Recommendation: Add Event-Emit

[NC-10] Add to indexed parameter for countable Events

Context:

contracts/liquid-staking/ETHPoolLPFactory.sol:
  15      /// @notice signalize withdrawing of ETH by depositor
  16:     event ETHWithdrawnByDepositor(address depositor, uint256 amount);
  17: 
  18:     /// @notice signalize burning of LP token
  19:     event LPTokenBurnt(bytes blsPublicKeyOfKnot, address token, address depositor, uint256 amount);
  20: 
  21:     /// @notice signalize issuance of new LP token
  22:     event NewLPTokenIssued(bytes blsPublicKeyOfKnot, address token, address firstDepositor, uint256 amount);
  23: 
  24:     /// @notice signalize issuance of existing LP token
  25:     event LPTokenMinted(bytes blsPublicKeyOfKnot, address token, address depositor, uint256 amount);

Description: Add to indexed parameter for countable Events

Recommendation: Add Event-Emit

[NC-11] NatSpec comments should be increased in contracts

Context: All Contracts

Description: It is recommended that Solidity contracts are fully annotated using NatSpec for all public interfaces (everything in the ABI). It is clearly stated in the Solidity official documentation. In complex projects such as Defi, the interpretation of all functions and their arguments and returns is important for code readability and auditability. https://docs.soliditylang.org/en/v0.8.15/natspec-format.html

Recommendation: NatSpec comments should be increased in contracts

[NC-12] Open TODOs

Context:

1 result

contracts/syndicate/Syndicate.sol:
  194          } else {
  195:             // todo - check else case for any ETH lost
  196          }

Recommendation: Use temporary TODOs as you work on a feature, but make sure to treat them before merging. Either add a link to a proper issue in your TODO, or remove it from the code.

[NC-13] Empty blocks should be removed or Emit something

Description: Code contains empty block

10 results - 8 files

contracts/liquid-staking/GiantPoolBase.sol:
  101:     function _onDepositETH() internal virtual {}
  104:     function _onWithdraw(LPToken[] calldata _lpTokens) internal virtual {}
  
contracts/liquid-staking/LiquidStakingManager.sol:
  166:     constructor() initializer {}
  629:     receive() external payable {}

contracts/liquid-staking/LPToken.sol:
  28:     constructor() initializer {}
 
contracts/liquid-staking/SavETHVault.sol:
  43:     constructor() initializer {}
  
contracts/liquid-staking/StakingFundsVault.sol:
  43:     constructor() initializer {}
  
contracts/liquid-staking/SyndicateRewardsProcessor.sol:
  98:     receive() external payable {}

contracts/smart-wallet/OwnableSmartWallet.sol:
  25:     constructor() initializer {}
  
contracts/syndicate/Syndicate.sol:
  123:     constructor() initializer {}

Recommendation: The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting.

[S-01] Generate perfect code headers every time

Description: I recommend using header for Solidity code layout and readability

https://github.com/transmissions11/headers

/*//////////////////////////////////////////////////////////////
                           TESTING 123
//////////////////////////////////////////////////////////////*/

Gas Optimizations List

Total 11 issues

Suggestions

Total 1 suggestion

[G-01] Use unchecked in withdrawETH function

Context:

contracts/liquid-staking/GiantPoolBase.sol:
  51      /// @param _amount of LP tokens user is burning in exchange for same amount of ETH
  52:     function withdrawETH(uint256 _amount) external nonReentrant {
  53:         require(_amount >= MIN_STAKING_AMOUNT, "Invalid amount");
  54:         require(lpTokenETH.balanceOf(msg.sender) >= _amount, "Invalid balance");
  55:         require(idleETH >= _amount, "Come back later or withdraw less ETH");
  56: 
  57:         idleETH -= _amount;
  58: 
  59:         lpTokenETH.burn(msg.sender, _amount);
  60:         (bool success,) = msg.sender.call{value: _amount}("");
  61:         require(success, "Failed to transfer ETH");
  62: 
  63:         emit LPBurnedForETH(msg.sender, _amount);
  64:     }

Description: In the withdrawETH function in the GiantPoolBase.sol contract, the idleETH status variable will never be less than the _amount function parameter |(require(idleETH >= _amount, "Come back later or withdraw less ETH");)| underflow does not occur. So here unchecked can be used.

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTest is DSTest {

    Contract0 c0;
    Contract1 c1;


    function setUp() public {

        c0 = new Contract0();
        c1 = new Contract1();
    }

        function testGas() public {
            c0.withdrawETH(5);
            c1.withdrawETH(5);            
        }
}

contract Contract0 {
    uint256 idleETH = 10;

    function withdrawETH(uint256 _amount) external {
        require(idleETH >= _amount, "Come back later or withdraw less ETH");
        idleETH -= _amount;    
    }
}


contract Contract1 {
    uint256 idleETH = 10;

    function withdrawETH(uint256 _amount) external {
        require(idleETH >= _amount, "Come back later or withdraw less ETH");
        unchecked {
         idleETH -= _amount;
        }
    }
}

Gas Report:

╭──────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/test.sol:Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞══════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                      ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 99029                                ┆ 421             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                        ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ withdrawETH                          ┆ 5419            ┆ 5419 ┆ 5419   ┆ 5419 ┆ 1       │
╰──────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭──────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/test.sol:Contract1 contract ┆                 ┆      ┆        ┆      ┆         │
╞══════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                      ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 82217                                ┆ 337             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                        ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ withdrawETH                          ┆ 5346            ┆ 5346 ┆ 5346   ┆ 5346 ┆ 1       │
╰──────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯

[G-02] Functions guaranteed to revert_ when callled by normal users can be marked payable

Context: 

17 results - 5 files

contracts/smart-wallet/OwnableSmartWallet.sol:
  114:     function setApproval(address to, bool status) external onlyOwner override {

contracts/syndicate/Syndicate.sol:
  145      function registerKnotsToSyndicate(
  146          bytes[] calldata _newBLSPublicKeyBeingRegistered
  147:     ) external onlyOwner {
  154:     function deRegisterKnots(bytes[] calldata _blsPublicKeys) external onlyOwner {
  161:     function addPriorityStakers(address[] calldata _priorityStakers) external onlyOwner {
  168:     function updatePriorityStakingBlock(uint256 _endBlock) external onlyOwner {

contracts/liquid-staking/LiquidStakingManager.sol:
  218:     function deRegisterKnotFromSyndicate(bytes[] calldata _blsPublicKeys) external onlyDAO {
  226      function restoreFreeFloatingSharesToSmartWalletForRageQuit(
  227         address _smartWallet,
  228         bytes[] calldata _blsPublicKeys,
  229         uint256[] calldata _amounts
  230:     ) external onlyDAO {
  239:     function updateDAOAddress(address _newAddress) external onlyDAO {
  249:     function updateDAORevenueCommission(uint256 _commissionPercentage) external onlyDAO {
  255:     function updateTicker(string calldata _newTicker) external onlyDAO {
  267:     function updateWhitelisting(bool _changeWhitelist) external onlyDAO returns (bool) {
  278:     function updateNodeRunnerWhitelistStatus(address _nodeRunner, bool isWhitelisted) external onlyDAO {
  308:     function rotateEOARepresentativeOfNodeRunner(address _nodeRunner, address _newRepresentative) external onlyDAO {
 
contracts/liquid-staking/SavETHVault.sol:
  200     function withdrawETHForStaking(
  201          address _smartWallet,
  202          uint256 _amount
  203:     ) public onlyManager nonReentrant returns (uint256) {

contracts/liquid-staking/StakingFundsVault.sol:
   56:     function updateDerivativesMinted() external onlyManager {
  239:     function withdrawETH(address _wallet, uint256 _amount) public onlyManager nonReentrant returns (uint256) {
  255      function unstakeSyndicateSharesForRageQuit(
  256          address _sETHRecipient,
  257          bytes[] calldata _blsPublicKeys,
  258          uint256[] calldata _amounts
  259:     ) external onlyManager nonReentrant {

Description: If a function modifier or require such as onlyOwner-admin is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are CALLVALUE(2), DUP1(3), ISZERO(3), PUSH2(3), JUMPI(10), PUSH1(3), DUP1(3), REVERT(0), JUMPDEST(1), POP(2) which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost.

Recommendation: Functions guaranteed to revert when called by normal users can be marked payable  (for only onlyOwner or onlyDAO or onlyManager functions)

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

- function registerKnotsToSyndicate(
        bytes[] calldata _newBLSPublicKeyBeingRegistered
    ) external onlyOwner {
        // update accrued ETH per SLOT type
        updateAccruedETHPerShares();
        _registerKnotsToSyndicate(_newBLSPublicKeyBeingRegistered);
    }

+ function registerKnotsToSyndicate(
        bytes[] calldata _newBLSPublicKeyBeingRegistered
    ) external payable onlyOwner {
        // update accrued ETH per SLOT type
        updateAccruedETHPerShares();
        _registerKnotsToSyndicate(_newBLSPublicKeyBeingRegistered);
    }

Gas Report:

╭──────────────────────────────────────────────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮
│ contracts/testing/syndicate/SyndicateMock.sol:SyndicateMock contract ┆                 ┆        ┆        ┆        ┆         │
╞══════════════════════════════════════════════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡
│ Deployment Cost                                                      ┆ Deployment Size ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 2990771                                                              ┆ 15093           ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                                                        ┆ min             ┆ avg    ┆ median ┆ max    ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ registerKnotsToSyndicate                                             ┆ 6503            ┆ 57756  ┆ 64257  ┆ 108857 ┆ 5       │
╰──────────────────────────────────────────────────────────────────────┴─────────────────┴────────┴────────┴────────┴─────────╯
╭──────────────────────────────────────────────────────────────────────┬─────────────────┬────────┬────────┬────────┬─────────╮
│ contracts/testing/syndicate/SyndicateMock.sol:SyndicateMock contract ┆                 ┆        ┆        ┆        ┆         │
╞══════════════════════════════════════════════════════════════════════╪═════════════════╪════════╪════════╪════════╪═════════╡
│ Deployment Cost                                                      ┆ Deployment Size ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 2980355                                                              ┆ 15041           ┆        ┆        ┆        ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                                                        ┆ min             ┆ avg    ┆ median ┆ max    ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ registerKnotsToSyndicate                                             ┆ 6479            ┆ 57732  ┆ 64233  ┆ 108833 ┆ 5       │
╰──────────────────────────────────────────────────────────────────────┴─────────────────┴────────┴────────┴────────┴─────────╯

[G-03] x -= y (x += y) costs more gas than x = x – y (x = x + y) for state variables

Description: x -= y costs more gas than x = x – y for state variables.

Context:

9 results - 5 files

contracts/liquid-staking/GiantMevAndFeesPool.sol:
  40:             idleETH -= _ETHTransactionAmounts[i];

contracts/liquid-staking/GiantPoolBase.sol:
  57:             idleETH -= _amount;    
         
contracts/liquid-staking/GiantSavETHVaultPool.sol:
  46:             idleETH -= transactionAmount;

contracts/liquid-staking/LiquidStakingManager.sol:
  615:             stakedKnotsOfSmartWallet[smartWallet] -= 1;

contracts/syndicate/Syndicate.sol:
  269:             totalFreeFloatingShares -= _sETHAmount;
  272:             sETHTotalStakeForKnot[_blsPubKey] -= _sETHAmount;
  273:             sETHStakedBalanceForKnot[_blsPubKey][msg.sender] -= _sETHAmount;
  621:             totalFreeFloatingShares -= sETHTotalStakeForKnot[_blsPublicKey];
  624:             numberOfRegisteredKnots -= 1;

contracts/liquid-staking/StakingFundsVault.sol:
   58:         totalShares += 4 ether;
   96          uint256 amount = _amounts[i];
   97:         totalAmount += amount;
  277          LPToken token = lpTokenForKnot[_blsPubKeys[i]];
  278:         totalAccumulated += previewAccumulatedETH(_user, token);
  287:         totalAccumulated += previewAccumulatedETH(_user, _token[i]);
 
contracts/liquid-staking/SyndicateRewardsProcessor.sol:
  65:           totalClaimed += due;
  85:           accumulatedETHPerLPShare += (unprocessed * PRECISION) / _numOfShares;
 
contracts/syndicate/Syndicate.sol:
  185:          accumulatedETHPerFreeFloatingShare += _calculateNewAccumulatedETHPerFreeFloatingShare(freeFloatingUnprocessed);

  190:          accumulatedETHPerCollateralizedSlotPerKnot += collateralizedUnprocessed;
  225:          totalFreeFloatingShares += _sETHAmount;
  226:          sETHTotalStakeForKnot[_blsPubKey] += _sETHAmount;
  227:          sETHStakedBalanceForKnot[_blsPubKey][_onBehalfOf] += _sETHAmount;
  317:          totalClaimed += unclaimedUserShare;
  511:          accruedEarningPerCollateralizedSlotOwnerOfKnot[_blsPubKey][collateralizedOwnerAtIndex] += unprocessedETHForCurrentKnot;
  558:          numberOfRegisteredKnots += knotsToRegister;
  658:          totalClaimed += unclaimedUserShare;

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTest is DSTest {
  
    Contract0 c0;
    Contract1 c1;
    
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    
    function testGas() public {
        c0.swap(2,3);
        c1.swap1(2,3);
    }
}
contract Contract0 {
    uint256 public amountIn = 10;
    
    function swap(uint256 amountInToBin, uint256 fee)  external returns (uint256 ) {
       return amountIn -= amountInToBin + fee;
    }
}

contract Contract1 {
    uint256 public amountIn = 10;
    
    function swap1(uint256 amountInToBin, uint256 fee)  external returns (uint256 result1) {
       return (amountIn = amountIn - (amountInToBin + fee));
    }
}

Gas Report:

╭──────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/test.sol:Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞══════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                      ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 83017                                ┆ 341             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                        ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ swap                                 ┆ 5454            ┆ 5454 ┆ 5454   ┆ 5454 ┆ 1       │
╰──────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭──────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/test.sol:Contract1 contract ┆                 ┆      ┆        ┆      ┆         │
╞══════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                      ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 83017                                ┆ 341             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                        ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ swap1                                ┆ 5431            ┆ 5431 ┆ 5431   ┆ 5431 ┆ 1       │
╰──────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯

[G-04] Optimize names to save gas

Context:  All Contracts

Description:  Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions. 

Recommendation:  Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Syndicate.sol contract will be the most used; A lower method ID may be given.

Proof of Consept: https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92

Syndicate.sol function names can be named and sorted according to METHOD ID

Sighash   |   Function Signature
========================
793193ae  =>  initialize(address,uint256,address[],bytes[])
d383eb94  =>  registerKnotsToSyndicate(bytes[])
42e5ddee  =>  deRegisterKnots(bytes[])
964bb65e  =>  addPriorityStakers(address[])
e09e6d7f  =>  updatePriorityStakingBlock(uint256)
6d4ef6b9  =>  updateAccruedETHPerShares()
8462c5c8  =>  stake(bytes[],uint256[],address)
cda82f20  =>  unstake(address,address,bytes[],uint256[])
7f3a7f7f  =>  claimAsStaker(address,bytes[])
30237f5e  =>  claimAsCollateralizedSLOTOwner(address,bytes[])
12499ee9  =>  updateCollateralizedSlotOwnersAccruedETH(bytes)
dbd13782  =>  batchUpdateCollateralizedSlotOwnersAccruedETH(bytes[])
758f2941  =>  calculateUnclaimedFreeFloatingETHShare(bytes,address)
4a65fbb5  =>  calculateETHForFreeFloatingOrCollateralizedHolders()
4c480979  =>  previewUnclaimedETHAsFreeFloatingStaker(address,bytes)
5271337a  =>  previewUnclaimedETHAsCollateralizedSlotOwner(address,bytes)
1f377fed  =>  getUnprocessedETHForAllFreeFloatingSlot()
b909757f  =>  getUnprocessedETHForAllCollateralizedSlot()
0bba3f7b  =>  calculateNewAccumulatedETHPerFreeFloatingShare()
94058b1c  =>  calculateNewAccumulatedETHPerCollateralizedSharePerKnot()
6e1682a0  =>  totalETHReceived()
ed3e91d6  =>  _initialize(address,uint256,address[],bytes[])
17430f08  =>  _updateCollateralizedSlotOwnersLiabilitySnapshot(bytes)
ba345cbb  =>  _calculateCollateralizedETHOwedPerKnot()
5dd47bdd  =>  _calculateNewAccumulatedETHPerCollateralizedShare(uint256)
2896c45f  =>  _calculateNewAccumulatedETHPerFreeFloatingShare(uint256)
f7919ba4  =>  _registerKnotsToSyndicate(bytes[])
701d5499  =>  _addPriorityStakers(address[])
5f18ea80  =>  _deRegisterKnots(bytes[])
ab11a8e3  =>  _deRegisterKnot(bytes)
35acddeb  =>  _getCorrectAccumulatedETHPerFreeFloatingShareForBLSPublicKey(bytes)
bb7c015b  =>  _claimAsStaker(address,bytes[])

[G-05] Use assembly to check for address(0)

Context: 

72 results - 13 files

contracts/liquid-staking/ETHPoolLPFactory.sol:
   77:         require(address(_oldLPToken) != address(0), "Zero address");
   78:         require(address(_newLPToken) != address(0), "Zero address");
  117:         if(address(lpToken) != address(0)) {

contracts/liquid-staking/GiantLP.sol:
  40:         if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).beforeTokenTransfer(_from, _to, _amount);
  46:         if (address(transferHookProcessor) != address(0)) ITransferHookProcessor(transferHookProcessor).afterTokenTransfer(_from, _to, _amount);
 
contracts/liquid-staking/GiantMevAndFeesPool.sol:
  151:         if (_from != address(0)) {
  
contracts/liquid-staking/LiquidStakingManager.sol:
  209:         require(smartWallet != address(0), "No wallet found");
  240:         require(_newAddress != address(0), "Zero address");
  279:         require(_nodeRunner != address(0), "Zero address");
  290:         require(_newRepresentative != address(0), "Zero address");
  294:         require(smartWallet != address(0), "No smart wallet");
  309:         require(_newRepresentative != address(0), "Zero address");
  312:         require(smartWallet != address(0), "No smart wallet");
  327:         require(_recipient != address(0), "Zero address");
  357:         require(_new != address(0) && _current != _new, "New is zero or current");
  360:         require(wallet != address(0), "Wallet does not exist");
  364:         require(newRunnerCurrentWallet == address(0), "New runner has a wallet");
  387:         require(_recipient != address(0), "Zero address");
  390:         require(smartWallet != address(0), "Unknown node runner");
  441:         if(smartWallet == address(0)) {
  454:         if(smartWalletRepresentative[smartWallet] != address(0)) {
  496:         return smartWalletOfKnot[_blsPublicKeyOfKnot] != address(0);
  501:         return !isBLSPublicKeyPartOfLSDNetwork(_blsPublicKeyOfKnot) || bannedBLSPublicKeys[_blsPublicKeyOfKnot] != address(0);
  544:         require(associatedSmartWallet != address(0), "Unknown BLS public key");
  563:         if(representative != address(0)) {
  658:         require(_dao != address(0), "Zero address");
  659:         require(_syndicateFactory != address(0), "Zero address");
  660:         require(_smartWalletFactory != address(0), "Zero address");
  661:         require(_brand != address(0), "Zero address");
  685:         require(_nodeRunner != address(0), "Zero address");
  700:         if(!_isEnabled && smartWalletRepresentative[_smartWallet] != address(0)) {
  717:         else if(_isEnabled && smartWalletRepresentative[_smartWallet] == address(0)) {
  854:         if (address(gatekeeper) != address(0)) {
  939:         require(address(stakingFundsLP) != address(0), "No funds staked in staking funds vault");
  943:         require(address(savETHVaultLP) != address(0), "No funds staked in savETH vault");
 

contracts/liquid-staking/LPToken.sol:
  62:         if (address(transferHookProcessor) != address(0)) transferHookProcessor.beforeTokenTransfer(_from, _to, _amount);
  69:         if (address(transferHookProcessor) != address(0)) transferHookProcessor.afterTokenTransfer(_from, _to, _amount);
 
contracts/liquid-staking/LPTokenFactory.sol:
  19:         require(_lpTokenImplementation != address(0), "Address cannot be zero");
  33:         require(address(_deployer) != address(0), "Zero address");

contracts/liquid-staking/LSDNFactory.sol:
  51:         require(_liquidStakingManagerImplementation != address(0), "Zero Address");
  52:         require(_syndicateFactory != address(0), "Zero Address");
  53:         require(_lpTokenFactory != address(0), "Zero Address");
  54:         require(_smartWalletFactory != address(0), "Zero Address");
  55:         require(_brand != address(0), "Zero Address");
  56:         require(_savETHVaultDeployer != address(0), "Zero Address");
  57:         require(_stakingFundsVaultDeployer != address(0), "Zero Address");
  58:         require(_optionalGatekeeperDeployer != address(0), "Zero Address");

contracts/liquid-staking/SavETHVault.sol:
  206:         require(_smartWallet != address(0), "Zero address");
  236:         require(_liquidStakingManagerAddress != address(0), "Zero address");
  237:         require(address(_lpTokenFactory) != address(0), "Zero address");

contracts/liquid-staking/StakingFundsVault.sol:
   86:         if (address(tokenForKnot) != address(0)) {
  127:         if (address(tokenForKnot) != address(0)) {
  154:         require(address(token) != address(0), "No ETH staked for specified BLS key");
  177:         require(address(_lpToken) != address(0), "Zero address specified");
  229:         require(address(token) != address(0), "Invalid BLS key");
  242:         require(_wallet != address(0), "Zero address");
  317:         if (syndicate != address(0)) {
  332:         if (_from != address(0)) {
  344:         if (LiquidStakingManager(payable(liquidStakingNetworkManager)).syndicate() != address(0)) {
  361:         require(_syndicate != address(0), "Invalid configuration");
  372:         require(address(_liquidStakingNetworkManager) != address(0), "Zero Address");
  373:         require(address(_lpTokenFactory) != address(0), "Zero Address");
  
contracts/liquid-staking/SyndicateRewardsProcessor.sol:
  57:         require(_recipient != address(0), "Zero address");

contracts/smart-wallet/OwnableSmartWallet.sol:
   34:             initialOwner != address(0),
  116:             to != address(0),

contracts/smart-wallet/OwnableSmartWalletFactory.sol:
  37:         require(owner != address(0), 'Wallet cannot be address 0');

contracts/syndicate/Syndicate.sol:
  206:         if (_onBehalfOf == address(0)) revert ZeroAddress();
  231:         if (stakeHouse == address(0)) revert KnotIsNotAssociatedWithAStakeHouse();
  253:         if (_unclaimedETHRecipient == address(0)) revert ZeroAddress();
  254:         if (_sETHRecipient == address(0)) revert ZeroAddress();
  295:         if (_recipient == address(0)) revert ZeroAddress();
  642:         if (_recipient == address(0)) revert ZeroAddress();

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTest is DSTest {
    Contract0 c0;
    Contract1 c1;
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    function testGas() public view {
        c0.setOperator(address(this));
        c1.setOperator(address(this));
    }
}
contract Contract0 {
    function setOperator(address operator_) public pure {
        require(operator_) != address(0), "INVALID_RECIPIENT");    }
}
contract Contract1 {
    function setOperator(address operator_) public pure {
        assembly {
            if iszero(operator_) {
                mstore(0x00, "Callback_InvalidParams")
                revert(0x00, 0x20)
            }
        }
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract0 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 50899                                     ┆ 285             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ setOperator                               ┆ 258             ┆ 258 ┆ 258    ┆ 258 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯
╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 44893                                     ┆ 255             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ setOperator                               ┆ 252             ┆ 252 ┆ 252    ┆ 252 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

[G-06] Use assembly to write address storage values

Context: GiantLP.sol#L25, GiantMevAndFeesPool.sol#L19, GiantSavETHVaultPool.sol#L20, LiquidStakingManager.sol#L245, LPToken.sol#L38, LPTokenFactory.sol#L21, LSDNFactory.sol#L60-L67, SavETHVault.sol#L239, StakingFundsVault.sol#L375-L376, SyndicateFactory.sol#L17

Proof Of Concept: The optimizer was turned on and set to 10000 runs.

contract GasTestFoundry is DSTest {
    Contract1 c1;
    Contract2 c2;
    
    function setUp() public {
        c1 = new Contract1();
        c2 = new Contract2();
    }
    
    function testGas() public {
        c1.setRewardTokenAndAmount(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,356);
        c2.setRewardTokenAndAmount(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,356);
    }
}

contract Contract1  {
    address rewardToken ;
    uint256 reward;

    function setRewardTokenAndAmount(address token_, uint256 reward_) external {
        rewardToken = token_;
        reward = reward_;
    }
}

contract Contract2  {
    address rewardToken ;
    uint256 reward;

    function setRewardTokenAndAmount(address token_, uint256 reward_) external {
        assembly {
            sstore(rewardToken.slot, token_)
            sstore(reward.slot, reward_)           

        }
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬───────┬────────┬───────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆       ┆        ┆       ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═══════╪════════╪═══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 50899                                     ┆ 285             ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg   ┆ median ┆ max   ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ setRewardTokenAndAmount                   ┆ 44490           ┆ 44490 ┆ 44490  ┆ 44490 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴───────┴────────┴───────┴─────────╯
╭───────────────────────────────────────────┬─────────────────┬───────┬────────┬───────┬─────────╮
│ src/test/GasTest.t.sol:Contract2 contract ┆                 ┆       ┆        ┆       ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═══════╪════════╪═══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 38087                                     ┆ 221             ┆       ┆        ┆       ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg   ┆ median ┆ max   ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ setRewardTokenAndAmount                   ┆ 44457           ┆ 44457 ┆ 44457  ┆ 44457 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴───────┴────────┴───────┴─────────╯

[G-07] The solady Library's Ownable contract is significantly gas-optimized, which can be used

Description: The project uses the onlyOwner, onlyDAO, onlyManager authorization model I recommend using Solady's highly gas optimized contract.

https://github.com/Vectorized/solady/blob/main/src/auth/OwnableRoles.sol

17 results - 5 files

contracts/smart-wallet/OwnableSmartWallet.sol:
  114:     function setApproval(address to, bool status) external onlyOwner override {

contracts/syndicate/Syndicate.sol:
  145      function registerKnotsToSyndicate(
  146          bytes[] calldata _newBLSPublicKeyBeingRegistered
  147:     ) external onlyOwner {
  154:     function deRegisterKnots(bytes[] calldata _blsPublicKeys) external onlyOwner {
  161:     function addPriorityStakers(address[] calldata _priorityStakers) external onlyOwner {
  168:     function updatePriorityStakingBlock(uint256 _endBlock) external onlyOwner {

contracts/liquid-staking/LiquidStakingManager.sol:
  218:     function deRegisterKnotFromSyndicate(bytes[] calldata _blsPublicKeys) external onlyDAO {
  226      function restoreFreeFloatingSharesToSmartWalletForRageQuit(
  227         address _smartWallet,
  228         bytes[] calldata _blsPublicKeys,
  229          uint256[] calldata _amounts
  230:     ) external onlyDAO {
  239:     function updateDAOAddress(address _newAddress) external onlyDAO {
  249:     function updateDAORevenueCommission(uint256 _commissionPercentage) external onlyDAO {
  255:     function updateTicker(string calldata _newTicker) external onlyDAO {
  267:     function updateWhitelisting(bool _changeWhitelist) external onlyDAO returns (bool) {
  278:     function updateNodeRunnerWhitelistStatus(address _nodeRunner, bool isWhitelisted) external onlyDAO {
  308:     function rotateEOARepresentativeOfNodeRunner(address _nodeRunner, address _newRepresentative) external onlyDAO {
 
contracts/liquid-staking/SavETHVault.sol:
  200     function withdrawETHForStaking(
  201          address _smartWallet,
  202          uint256 _amount
  203:     ) public onlyManager nonReentrant returns (uint256) {

contracts/liquid-staking/StakingFundsVault.sol:
   56:     function updateDerivativesMinted() external onlyManager {
  239:     function withdrawETH(address _wallet, uint256 _amount) public onlyManager nonReentrant returns (uint256) {
  255      function unstakeSyndicateSharesForRageQuit(
  256          address _sETHRecipient,
  257          bytes[] calldata _blsPublicKeys,
  258          uint256[] calldata _amounts
  259:     ) external onlyManager nonReentrant {

[G-08] Setting The Constructor To Payable

Description: You can cut out 10 opcodes in the creation-time EVM bytecode if you declare a constructor payable. Making the constructor payable eliminates the need for an initial check of msg.value == 0 and saves 13 gas on deployment with no security risks.

Context:

16 results - 16 files

contracts/liquid-staking/GiantLP.sol:
  19:     constructor(
  20      address _pool,
  21      address _transferHookProcessor,
  22      string memory _name,
  23      string memory _symbol
  24      ERC20(_name, _symbol) {

contracts/liquid-staking/GiantMevAndFeesPool.sol:
  17:     constructor(LSDNFactory _factory) {
 
contracts/liquid-staking/GiantSavETHVaultPool.sol:
  18:     constructor(LSDNFactory _factory) {
  
contracts/liquid-staking/LiquidStakingManager.sol:
  166:     constructor() initializer {}

contracts/liquid-staking/LPToken.sol:
  28:     constructor() initializer {} 

contracts/liquid-staking/LPTokenFactory.sol:
  18:     constructor(address _lpTokenImplementation) {

contracts/liquid-staking/LSDNFactory.sol:
  41:     constructor(
  42          address _liquidStakingManagerImplementation,
  43          address _syndicateFactory,
  44          address _lpTokenFactory,
  45          address _smartWalletFactory,
  46          address _brand,
  47          address _savETHVaultDeployer,
  48          address _stakingFundsVaultDeployer,
  49          address _optionalGatekeeperDeployer
  50       ) {

contracts/liquid-staking/OptionalHouseGatekeeper.sol:
  14:     constructor(address _manager) {
  15          liquidStakingManager = ILiquidStakingManager(_manager);

contracts/liquid-staking/SavETHVault.sol:
  43:     constructor() initializer {}
  
contracts/liquid-staking/SavETHVaultDeployer.sol:
  14:     constructor() {

contracts/liquid-staking/StakingFundsVault.sol:
  43:     constructor() initializer {}
 
contracts/liquid-staking/StakingFundsVaultDeployer.sol:
  14:     constructor() {

contracts/smart-wallet/OwnableSmartWallet.sol:
  25:     constructor() initializer {}

contracts/smart-wallet/OwnableSmartWalletFactory.sol:
  22:     constructor() {

contracts/syndicate/Syndicate.sol:
  123:     constructor() initializer {}

contracts/syndicate/SyndicateFactory.sol:
  16:     constructor(address _syndicateImpl) {

Recommendation: Set the constructor to payable

Proof Of Concept: https://forum.openzeppelin.com/t/a-collection-of-gas-optimisation-tricks/19966/5?u=pcaversaccio

The optimizer was turned on and set to 10000 runs

contract GasTestFoundry is DSTest {
    
    Contract1 c1;
    Contract2 c2;
    
    function setUp() public {
        c1 = new Contract1();
        c2 = new Contract2();
    }
    
    function testGas() public {
        c1.x();
        c2.x();
    }
}

contract Contract1 {
    
    uint256 public dummy;
    constructor() payable {
        dummy = 1;
    }
    
    function x() public {
    }
}

contract Contract2 {
    
    uint256 public dummy;
    constructor() {
        dummy = 1;
    }
    
    function x() public {
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 49563                                     ┆ 159             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract2 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 49587                                     ┆ 172             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤

[G-09] Use a different pattern when using for loops

Context:

2 results - 1 file

contracts/liquid-staking/LiquidStakingManager.sol:
  391  
  392:         for(uint256 i; i < _blsPubKeys.length; ++i) {
  393              require(isBLSPublicKeyBanned(_blsPubKeys[i]) == false, "BLS public key is banned or not a part of LSD network");

  464  
  465:         for(uint256 i; i < len; ++i) {
  466              bytes calldata _blsPublicKey = _blsPublicKeys[i];

Description: In the use of for loops, this structure, which will reduce gas costs, can be preferred. It saves approximately 400 gas in an array with 6 members.

Proof Of Concept: The optimizer was turned on and set to 10000 runs

contract GasTest is DSTest {
  
    Contract0 c0;
    Contract1 c1;
    
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    
    function testGas() public {
        address[] memory Inputs = new address[](6);
        Inputs[0] = 0x5B38Da6a701c568545dCfcB03FcB875f56beddC4;
        Inputs[1] = 0xAb8483F64d9C6d1EcF9b849Ae677dD3315835cb2;
        Inputs[2] = 0x4B20993Bc481177ec7E8f571ceCaE8A9e22C02db;
        Inputs[3] = 0x78731D3Ca6b7E34aC0F824c42a7cC18A495cabaB;
        Inputs[4] = 0x617F2E2fD72FD9D5503197092aC168c91465E7f2;
        Inputs[5] = 0x17F6AD8Ef982297579C203069C1DbfFE4348c372;
        
        c0.dummy(Inputs);
        c1.dummy(Inputs);
    }
}

contract Contract0 {
    function dummy(address[] memory test) public {
        for (uint256 i = 0; i < test.length; ++i) {
        }
    }
}

contract Contract1 { 
    function dummy(address[] memory test) public {
        for (uint256 i = 0; i < test.length; i = unchecked_inc(i)) {
            // do something that doesn't change the value of i
        }
    }
    
    function unchecked_inc(uint i) internal pure returns (uint) {
        unchecked {
            return i + 1;
        }
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/GasTest.t.sol:Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞═══════════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 113159                                    ┆ 597             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ dummy                                     ┆ 2044            ┆ 2044 ┆ 2044   ┆ 2044 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭───────────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆      ┆        ┆      ┆         │
╞═══════════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 92541                                     ┆ 494             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ dummy                                     ┆ 1666            ┆ 1666 ┆ 1666   ┆ 1666 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯

[G-10] Use inline assembly for contract balance

Context:

8 results - 7 files

contracts/liquid-staking/GiantMevAndFeesPool.sol:
  177:         return address(this).balance + totalClaimed - idleETH;

contracts/liquid-staking/LiquidStakingManager.sol:
  400:         uint256 balBefore = address(this).balance;
  410:         (uint256 nodeRunnerAmount, uint256 daoAmount) = _calculateCommission(address(this).balance - balBefore);

contracts/liquid-staking/SavETHVault.sol:
  205:         require(address(this).balance >= _amount, "Insufficient withdrawal amount");
 
contracts/liquid-staking/StakingFundsVault.sol:
  241:         require(_amount <= address(this).balance, "Not enough ETH to withdraw");

contracts/liquid-staking/SyndicateRewardsProcessor.sol:
  94:         return address(this).balance + totalClaimed;

contracts/syndicate/Syndicate.sol:
  465:         return address(this).balance + totalClaimed;

contracts/testing/liquid-staking/MockSavETHVault.sol:
  44:         return address(this).balance;

Description: You can use balance(address) instead of address.balance() when getting an contract’s balance of ETH.

Proof Of Concept The optimizer was turned on and set to 10000 runs

contract GasTest is DSTest {
    
    Contract0 c0;
    Contract1 c1;
    
    function setUp() public {
        c0 = new Contract0();
        c1 = new Contract1();
    }
    
    function testGas() public {
        
        c0._withdraw(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4);
        c1._withdraw(0x5B38Da6a701c568545dCfcB03FcB875f56beddC4);
    }
}

contract Contract0 {
    
    function _withdraw(address addr) external {
        uint256 amount = address(this).balance;
        (bool sent, ) = addr.call{ value: amount }('');
    }
}

contract Contract1 {
    
    function _withdraw(address addr) external returns (uint256) {
        uint256 amount ;
        assembly {
            amount := selfbalance()
        }
        (bool sent, ) = addr.call{ value: amount }('');
    }
}

Gas Report:

╭───────────────────────────────────────────┬─────────────────┬──────┬────────┬──────┬─────────╮
│ src/test/GasTest.t.sol:Contract0 contract ┆                 ┆      ┆        ┆      ┆         │
╞═══════════════════════════════════════════╪═════════════════╪══════╪════════╪══════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 55305                                     ┆ 308             ┆      ┆        ┆      ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg  ┆ median ┆ max  ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ _withdraw                                 ┆ 2949            ┆ 2949 ┆ 2949   ┆ 2949 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴──────┴────────┴──────┴─────────╯
╭───────────────────────────────────────────┬─────────────────┬─────┬────────┬─────┬─────────╮
│ src/test/GasTest.t.sol:Contract1 contract ┆                 ┆     ┆        ┆     ┆         │
╞═══════════════════════════════════════════╪═════════════════╪═════╪════════╪═════╪═════════╡
│ Deployment Cost                           ┆ Deployment Size ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ 59511                                     ┆ 329             ┆     ┆        ┆     ┆         │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ Function Name                             ┆ min             ┆ avg ┆ median ┆ max ┆ # calls │
├╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌┼╌╌╌╌╌┼╌╌╌╌╌╌╌╌╌┤
│ _withdraw                                 ┆ 509             ┆ 509 ┆ 509    ┆ 509 ┆ 1       │
╰───────────────────────────────────────────┴─────────────────┴─────┴────────┴─────┴─────────╯

[G-11] Use double require instead of using &&

Context: 

9 results - 3 files

contracts/liquid-staking/LiquidStakingManager.sol:
  357:         require(_new != address(0) && _current != _new, "New is zero or current");
  371:         if (msg.sender == dao && _wasPreviousNodeRunnerMalicious) {
  700:         if(!_isEnabled && smartWalletRepresentative[_smartWallet] != address(0)) {
  717:         else if(_isEnabled && smartWalletRepresentative[_smartWallet] == address(0)) {

contracts/liquid-staking/StakingFundsVault.sol:
  215:         if (i == 0 && !Syndicate(payable(liquidStakingNetworkManager.syndicate())).isNoLongerPartOfSyndicate(_blsPubKeys[i])) {
 
contracts/syndicate/Syndicate.sol:
  218:         if (block.number < priorityStakingEndBlock && !isPriorityStaker[_onBehalfOf]) revert NotPriorityStaker();
  500:         if (unprocessedETHForCurrentKnot > 0 && !isNoLongerPartOfSyndicate[_blsPubKey]) { 
  533:         if (!isActive && !isNoLongerPartOfSyndicate[_blsPubKey]) {
  588:         if (i > 0 && staker < _priorityStakers[i-1]) revert DuplicateArrayElements();

Description: Using double require instead of operator && can save more gas When having a require statement with 2 or more expressions needed, place the expression that cost less gas first.

Recommendation: LiquidStakingManager.sol:#L357;

require(_new != address(0) && _current != _new, "New is zero or current");

Recommendation Code:

require(_new != address(0), "New is zero"); 
require(_current != _new, "New is current"); 

[S-01] Use v4.8.0 OpenZeppelin contracts

Description: The upcoming v4.8.0 version of OpenZeppelin provides many small gas optimizations.

https://github.com/OpenZeppelin/openzeppelin-contracts/releases/tag/v4.8.0-rc.0

v4.8.0-rc.0
⛽ Many small optimizations