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

Public view vs external&internal view #186

Open
Quazia opened this issue Aug 18, 2023 · 2 comments
Open

Public view vs external&internal view #186

Quazia opened this issue Aug 18, 2023 · 2 comments
Labels
improvement changes that improve the security, or performance of the code without modifying the functionality

Comments

@Quazia
Copy link
Member

Quazia commented Aug 18, 2023

There's a few places we use a public view where we could have an internal view that's accessible by way of another external view.

For example:

    function getNftQuestFee(address address_) public view returns (uint256) {
        return nftQuestFeeList[address_].exists ? nftQuestFeeList[address_].fee : nftQuestFee;
    }

Would become:

    function getNftQuestFee(address address_) external view returns (uint256) {
        _getNftQuestFee(address_);
    }
     
     function _getNftQuestFee(address address_) internal view returns (uint256) {
        return nftQuestFeeList[address_].exists ? nftQuestFeeList[address_].fee : nftQuestFee;
    }
@Quazia Quazia added the improvement changes that improve the security, or performance of the code without modifying the functionality label Aug 18, 2023
@waynehoover
Copy link
Contributor

Could you tell me what the reason is for this change?

@Quazia
Copy link
Member Author

Quazia commented Aug 22, 2023

It's just a gas improvement, not super pressing. As far as I know internal and external are both more efficient than public. It's possible recent compiler optimizations have made this an outdated optimization and I think if you crank the solc optimization up enough it actually handles stuff like this. I can wait and do the gas testing before making this change so we can see sure. It takes up more storage in exchange for a little cheaper execution. You see a lot more benefit in situations where you're calling the same public function multiple times internally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement changes that improve the security, or performance of the code without modifying the functionality
Projects
None yet
Development

No branches or pull requests

2 participants