code-423n4 / 2021-05-visorfinance-findings

0 stars 0 forks source link

Gas optimizations by using external over public #51

Open code423n4 opened 3 years ago

code423n4 commented 3 years ago

Handle

a_delamo

Vulnerability details

Impact

Using public over external has an impact on execution cost.

If we run the following methods on Remix, we can see the difference

    //  transaction cost    21448 gas 
    //  execution cost  176 gas 
    function tt() external returns(uint256) {
        return 0;
    }

    //  transaction cost    21558 gas 
    //  execution cost  286 gas
    function tt_public() public returns(uint256) {
        return 0;
    }

The following are methods that currently use public and should be declared external

stake(address,address,uint256,bytes) should be declared external:
        - Mainframe.stake(address,address,uint256,bytes) (contracts/Mainframe.sol#158-171)
getCurrentVaultStakeUnits(address) should be declared external:
        - Hypervisor.getCurrentVaultStakeUnits(address) (contracts/hypervisor/Hypervisor.sol#576-586)
getPowerSwitch() should be declared external:
        - Powered.getPowerSwitch() (contracts/hypervisor/Powered.sol#67-74)
nft() should be declared external:
        - OwnableERC721.nft() (contracts/visor/OwnableERC721.sol#23-25)
getTimeLockCount(address) should be declared external:
        - Visor.getTimeLockCount(address) (contracts/visor/Visor.sol#305-310)
getTimeLockERC721Count(address) should be declared external:
        - Visor.getTimeLockERC721Count(address) (contracts/visor/Visor.sol#312-318)
setURI(string) should be declared external:
        - Visor.setURI(string) (contracts/visor/Visor.sol#469-471)
timeLockERC721(address,address,uint256,uint256) should be declared external:
        - Visor.timeLockERC721(address,address,uint256,uint256) (contracts/visor/Visor.sol#647-673)
timeUnlockERC721(address,address,uint256,uint256) should be declared external:
        - Visor.timeUnlockERC721(address,address,uint256,uint256) (contracts/visor/Visor.sol#685-708)
timeLockERC20(address,address,uint256,uint256) should be declared external:
        - Visor.timeLockERC20(address,address,uint256,uint256) (contracts/visor/Visor.sol#718-749)
timeUnlockERC20(address,address,uint256,uint256) should be declared external:
        - Visor.timeUnlockERC20(address,address,uint256,uint256) (contracts/visor/Visor.sol#761-782)
addTemplate(bytes32,address) should be declared external:
        - VisorFactory.addTemplate(bytes32,address) (contracts/visor/VisorFactory.sol#26-35)
setActive(bytes32) should be declared external:
        - VisorFactory.setActive(bytes32) (contracts/visor/VisorFactory.sol#37-41)
createSelected(bytes32) should be declared external:
        - VisorFactory.createSelected(bytes32) (contracts/visor/VisorFactory.sol#69-85)
createSelected2(bytes32,bytes32) should be declared external:
        - VisorFactory.createSelected2(bytes32,bytes32) (contracts/visor/VisorFactory.sol#87-107)
nameCount() should be declared external:
        - VisorFactory.nameCount() (contracts/visor/VisorFactory.sol#160-162)
vaultCount(address) should be declared external:
        - VisorFactory.vaultCount(address) (contracts/visor/VisorFactory.sol#164-166)
getUserVault(address,uint256) should be declared external:
        - VisorFactory.getUserVault(address,uint256) (contracts/visor/VisorFactory.sol#168-174)

Tools Used

Slither

Recommended Mitigation Steps

Just change from public to external if possible

ghost commented 3 years ago

sponsor confirmed We will be applying this optimization in our next version

ztcrypto commented 3 years ago

duplicate of #27 and fixed