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 #204

Open
code423n4 opened this issue Jun 14, 2022 · 0 comments
Open

Gas Optimizations #204

code423n4 opened this issue Jun 14, 2022 · 0 comments
Labels

Comments

@code423n4
Copy link
Contributor

Using Clone instead of CREATE2

CREATE2 is consuming a lot of gas while Clone consume 10 times less gas than CREATE2

The WrappedFCash contract already used initialize pattern, so it is already compatible with Clone.

Clone is smaller than Beacon.

Read more: https://blog.logrocket.com/cloning-solidity-smart-contracts-factory-pattern/

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/proxy/WrappedfCashFactory.sol#L26-L37

    function deployWrapper(uint16 currencyId, uint40 maturity) external returns (address) {
        address _computedWrapper = computeAddress(currencyId, maturity);

        if (Address.isContract(_computedWrapper)) {
            // If wrapper has already been deployed then just return it's address
            return _computedWrapper;
        } else {
            address wrapper = Create2.deploy(0, SALT, _getByteCode(currencyId, maturity));
            emit WrapperDeployed(currencyId, maturity, wrapper);
            return wrapper;
        }
    }

This part use Create2 to deploy Beacon, can be optimized to use Clone.

You may use clone with any kind of upgradable proxy to achieve upgradability.

Use solidity custom error instead of revert message

It not only provide better gas optimization but also reduce contract size

https://blog.soliditylang.org/2021/04/21/custom-errors/

Applied to all area with require(...)

require(cashGroup.maxMarketIndex > 0, "Invalid currency");

        require(
            DateTime.isValidMaturity(cashGroup.maxMarketIndex, maturity, block.timestamp),
            "Invalid maturity"
        );

require(pvExternal >= 0);

require(!hasMatured(), "fCash matured");

... any many more ...

Can be written like this

// Before contract definition
error Matured();

// Inside a function
if (hasMatured()) {
    revert Matured();
}

Duplicated function call on convertToShares

https://github.com/code-423n4/2022-06-notional-coop/blob/6f8c325f604e2576e2fe257b6b57892ca181509a/notional-wrapped-fcash/contracts/wfCashERC4626.sol#L52-L61

    function convertToShares(uint256 assets) public view override returns (uint256 shares) {
        uint256 supply = totalSupply();
        if (supply == 0) {
            // Scales assets by the value of a single unit of fCash
            uint256 unitfCashValue = _getPresentValue(uint256(Constants.INTERNAL_TOKEN_PRECISION));
            return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / unitfCashValue;
        }

        return (assets * totalSupply()) / totalAssets();
    }

Noticed that totalSupply() is called twice. Can be optimized to

    function convertToShares(uint256 assets) public view override returns (uint256 shares) {
        uint256 supply = totalSupply();
        if (supply == 0) {
            // Scales assets by the value of a single unit of fCash
            uint256 unitfCashValue = _getPresentValue(uint256(Constants.INTERNAL_TOKEN_PRECISION));
            return (assets * uint256(Constants.INTERNAL_TOKEN_PRECISION)) / unitfCashValue;
        }

        return (assets * supply) / totalAssets();
    }
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jun 14, 2022
code423n4 added a commit that referenced this issue Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants