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

Unused Named Returns #115

Open
code423n4 opened this issue Nov 18, 2021 · 2 comments
Open

Unused Named Returns #115

code423n4 opened this issue Nov 18, 2021 · 2 comments
Labels
bug Something isn't working G (Gas Optimization) sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue wont fix

Comments

@code423n4
Copy link
Contributor

Handle

ye0lde

Vulnerability details

Impact

Removing unused named return variables can reduce gas usage and improve code clarity.

Proof of Concept

Unused named returns
https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L394-L397
https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L424

Tools Used

Visual Studio Code, Remix

Recommended Mitigation Steps

Either remove the named variables or
Delete the similar local variables that were created and used instead of the named variables.
The return statement can also be deleted.

For example, refactor the userInfo function
https://github.com/code-423n4/2021-11-yaxis/blob/146febcb61ae7fe20b0920849c4f4bbe111c6ba7/contracts/v3/alchemix/Transmuter.sol#L394-L397
like this:

    function userInfo(address user)  
        public
        view
        returns (
            uint256 depositedAl,
            uint256 pendingdivs,
            uint256 inbucket,
            uint256 realised
        )
    {
        depositedAl = depositedAlTokens[user];
        uint256 _toDistribute = buffer.mul(block.number.sub(lastDepositBlock)).div(TRANSMUTATION_PERIOD);
        if(block.number.sub(lastDepositBlock) > TRANSMUTATION_PERIOD){
            _toDistribute = buffer;
        }
        pendingdivs = _toDistribute.mul(depositedAlTokens[user]).div(totalSupplyAltokens);
        inbucket = tokensInBucket[user].add(dividendsOwing(user));
        realised = realisedTokens[user];
    }

getMultipleUserInfo can be refactored in the same way.

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Nov 18, 2021
code423n4 added a commit that referenced this issue Nov 18, 2021
@Xuefeng-Zhu Xuefeng-Zhu added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Nov 23, 2021
@Xuefeng-Zhu
Copy link
Collaborator

I think this is for readability

@0xleastwood
Copy link
Collaborator

0xleastwood commented Dec 21, 2021

This is a valid finding. As far as I'm aware, declaring named returns uses memory to store the created state variables. As this is unnecessary, there are a number of memory operations which can be removed, slightly improving gas costs for affected function calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working G (Gas Optimization) sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue wont fix
Projects
None yet
Development

No branches or pull requests

3 participants