Update UniversalRewardsDistributor.sol: Modifier Optimization

[0xScratch]

This PR aims to optimize the gas by using these two modifiers -> 'onlyOwner' and 'notFrozen'. Actually, they are the ones which are used the most in this particular code and consumes a lot of gas. Why they do?

Usually, modifiers copies the code in each instances whenever they are used in a smart contract and thus increases the bytecode size which costs gas. So to tackle this, a good idea is to refactor them with the help of internal function like I did in this code. Moreover, there's a point to keep in mind -> This method do prevent the increase of bytecode size but at the cost of JUMP opcode, which means usually the developer needs to have a trade-off in what's best thing to use at a particular point.
That's why I used this 'refactoring' method with just these two modifiers because they were used the most in this contract (Try to prefer this method only when a modifier is used more than 2 times).

Anyways, enough talking...Let's get down to results, Below is a simple demo code which include two modifiers (simple ones) and some functions (without any code) that are using these modifiers. Then we gonna run them on remix and see the gas usage. Here's the code:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract Demo{
    address public seller;
    address public buyer;

    error notBuyer();
    error notSeller();

    modifier onlyBuyer(address _buyer){
        if(_buyer != buyer) revert notBuyer();
        _;
    }
    modifier onlySeller(address _seller){
        if(_seller != seller) revert notSeller();
        _;
    }

    function Buyer1(address buyer_) external onlyBuyer(buyer_){
        // some code
    }

    function Buyer2(address buyer_) external onlyBuyer(buyer_){
        // some code
    }

    function Buyer3(address buyer_) external onlyBuyer(buyer_){
        // some code
    }

    function Seller1(address seller_) external onlySeller(seller_){
        // some code
    }

    function Seller2(address seller_) external onlySeller(seller_){
        // some code
    }

    function Seller3(address seller_) external onlySeller(seller_){
        // some code
    }

    function Seller4(address seller_) external onlySeller(seller_){
        // some code
    }

    function Seller5(address seller_) external onlySeller(seller_){
        // some code
    }
}

After deploying this contract on remix, I got this

then I made some desired changes which are:

    modifier onlyBuyer(address _buyer){
        checkBuyer(_buyer);
        _;
    }
    modifier onlySeller(address _seller){
        checkSeller(_seller);
        _;
    }

    function checkBuyer(address _buyer) internal view{
        if (_buyer != buyer) revert notBuyer();
    }

    function checkSeller(address _seller) internal view{
        if (_seller != seller) revert notSeller();
    }

and these changes do brought me some fruitful results

That's it. I know it was kind of a long read but explanation matters!!
Moreover, if anyone kind of feels somewhat wrong in this change then do let me know. Will be a great learning experience then😄

[julien-devatom]

Hey @Aryan9592, thank you for your contribution, it's really appreciated.
You're right that this is reducing the bytecode size, obviously. However, finding a tradeoff between bytecode size and execution cost is the role of the solidity optimizer. This kind of refactoring is done automatically by the optimizer if it makes sense (during the compilation). So, it's useless to refactor the modifiers if we use compiler opti.

See the Solidity documentation for more information.

[0xScratch]

Hey @julien-devatom, Thanks for your comment and that great info. If you don't mind, Can your provide me the exact part which explain your point such that I could get more insights about it. Although, I got your point but still want to a have a good catch about it

[0xScratch]

Btw, Look at this one
OpenZeppelin/openzeppelin-contracts#3347
and this
OpenZeppelin/openzeppelin-contracts#3223

[julien-devatom]

Mmmh, yes, it can make sense. Have you tried to run it with a 200 optimizer run?
Unfortunately, I don't have the time to test it on my side.

[0xScratch]

Yeah, I tried to do this but got problems with installing Foundry in my cloned repository. I don't know why.
Kinda new to foundry and it's presenting me problem related to curl if I am not wrong. Here's the screenshot of the error

Still trying to resolve it, don't know how much time it will take. Although 'curl' is installed and added to my system environment variables, So there's not a particular problem with running curl as far I am concerned..

[julien-devatom]

Yeah, I tried to do this but got problems with installing Foundry in my cloned repository. I don't know why. Kinda new to foundry and it's presenting me problem related to curl if I am not wrong. Here's the screenshot of the error

Still trying to resolve it, don't know how much time it will take. Although 'curl' is installed and added to my system environment variables, So there's not a particular problem with running curl as far I am concerned..

I cannot help you, it's an issue with your computer setup (you're on Windows)