Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gas optimizations by using external over public #18

Open
code423n4 opened this issue May 19, 2021 · 3 comments
Open

Gas optimizations by using external over public #18

code423n4 opened this issue May 19, 2021 · 3 comments

Comments

@code423n4
Copy link
Contributor

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

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels May 19, 2021
code423n4 added a commit that referenced this issue May 19, 2021
@ZeframLou
Copy link
Collaborator

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

@ghoul-sol
Copy link
Collaborator

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

@ZeframLou
Copy link
Collaborator

Addressed in 88mphapp/88mph-contracts@e1df42d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants