Gas optimizations by using external over public

[code423n4]

Handle

a_delamo

Vulnerability details

Impact

Some functions could use external instead of public in order to save gas.

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;
  }

surplusOfDeposit(uint256) should be declared external:
        - DInterest.surplusOfDeposit(uint256) (contracts/DInterest.sol#623-648)
dividendOf(uint256,address,address) should be declared external:
        - ERC1155DividendToken.dividendOf(uint256,address,address) (contracts/libs/ERC1155DividendToken.sol#101-107)
withdrawnDividendOf(uint256,address,address) should be declared external:
        - ERC1155DividendToken.withdrawnDividendOf(uint256,address,address) (contracts/libs/ERC1155DividendToken.sol#114-126)
uri(uint256) should be declared external:
        - ERC1155Upgradeable.uri(uint256) (contracts/libs/ERC1155Upgradeable.sol#92-94)
balanceOfBatch(address[],uint256[]) should be declared external:
        - ERC1155Upgradeable.balanceOfBatch(address[],uint256[]) (contracts/libs/ERC1155Upgradeable.sol#124-143)
setApprovalForAll(address,bool) should be declared external:
        - ERC1155Upgradeable.setApprovalForAll(address,bool) (contracts/libs/ERC1155Upgradeable.sol#148-160)
safeTransferFrom(address,address,uint256,uint256,bytes) should be declared external:
        - ERC1155Upgradeable.safeTransferFrom(address,address,uint256,uint256,bytes) (contracts/libs/ERC1155Upgradeable.sol#178-190)
safeBatchTransferFrom(address,address,uint256[],uint256[],bytes) should be declared external:
        - ERC1155Upgradeable.safeBatchTransferFrom(address,address,uint256[],uint256[],bytes) (contracts/libs/ERC1155Upgradeable.sol#195-207)
increaseAllowance(address,uint256) should be declared external:
        - ERC20Wrapper.increaseAllowance(address,uint256) (contracts/libs/ERC20Wrapper.sol#146-157)
decreaseAllowance(address,uint256) should be declared external:
        - ERC20Wrapper.decreaseAllowance(address,uint256) (contracts/libs/ERC20Wrapper.sol#173-186)
mint(address,uint256) should be declared external:
        - ERC20Mock.mint(address,uint256) (contracts/mocks/ERC20Mock.sol#7-9)
deposit(uint256) should be declared external:
        - VaultMock.deposit(uint256) (contracts/mocks/VaultMock.sol#16-21)
withdraw(uint256) should be declared external:
        - VaultMock.withdraw(uint256) (contracts/mocks/VaultMock.sol#23-29)
deposit(uint256) should be declared external:
        - VaultWithDepositFeeMock.deposit(uint256) (contracts/mocks/VaultWithDepositFeeMock.sol#23-33)
withdraw(uint256) should be declared external:
        - VaultWithDepositFeeMock.withdraw(uint256) (contracts/mocks/VaultWithDepositFeeMock.sol#35-41)
updateAndQuery() should be declared external:
        - EMAOracle.updateAndQuery() (contracts/models/interest-oracle/EMAOracle.sol#55-84)
query() should be declared external:
        - EMAOracle.query() (contracts/models/interest-oracle/EMAOracle.sol#86-88)
initialize() should be declared external:
        - MPHToken.initialize() (contracts/rewards/MPHToken.sol#19-23)
ownerMint(address,uint256) should be declared external:
        - MPHToken.ownerMint(address,uint256) (contracts/rewards/MPHToken.sol#26-33)

Tools Used

Slither

[ZeframLou]

Many of the functions listed are also called within the contract, so changing their visibility to public will break things.

[ghoul-sol]

Even though out of the whole list, only few functions are good candidates to be changed, it's technically a valid suggestion.

[ZeframLou]

Addressed in https://github.com/88mphapp/88mph-contracts/commit/e1df42dc46960ecd0c67a8d896f933149ea129e4