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: Optimize Conditional Statements in CDSTemplate.sol:deposit() #285

Open
code423n4 opened this issue Jan 13, 2022 · 2 comments
Open

Gas: Optimize Conditional Statements in CDSTemplate.sol:deposit() #285

code423n4 opened this issue Jan 13, 2022 · 2 comments
Labels
bug Something isn't working G (Gas Optimization) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Handle

Dravee

Vulnerability details

Impact

It's possible to save gas by optimizing the checks in conditional statements (if, else if and else). This would save a few opcodes and avoid redundant checks.

Proof of Concept

In CDSTemplate.sol:deposit(), the code is as follows:

140:         if (_supply > 0 && _liquidity > 0) { 
141:             _mintAmount = (_amount * _supply) / _liquidity;
142:         } else if (_supply > 0 && _liquidity == 0) {
143:             //when vault lose all underwritten asset = 
144:             _mintAmount = _amount * _supply; //dilute LP token value af. Start CDS again.
145:         } else {
146:             //when _supply == 0,
147:             _mintAmount = _amount;
148:         }

The conditions checks can be optimized with the following (read the @audit-info comments for futher information):

        if (_supply == 0) { 
            _mintAmount = _amount;
        } else if (_liquidity == 0) { // @audit-info : implicit _supply > 0 as above condition is false
            //when vault lose all underwritten asset = 
            _mintAmount = _amount * _supply; //dilute LP token value af. Start CDS again.
        } else { // @audit-info : implicit _supply > 0 and _liquidity > 0 as both the previous conditions are false
            _mintAmount = (_amount * _supply) / _liquidity;
        }

Tools Used

VS Code

Recommended Mitigation Steps

Compact conditions in mentioned logic statements

@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Jan 13, 2022
code423n4 added a commit that referenced this issue Jan 13, 2022
@oishun1112 oishun1112 added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jan 17, 2022
@oishun1112
Copy link
Collaborator

most of the case will be _supply > 0 && _liquidity > 0
so, it is cheaper to keep this order of validations

@oishun1112
Copy link
Collaborator

I performed experiment and confirmed that even above premise, this impl reduce gas

#355

@oishun1112 oishun1112 added sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") and removed sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons labels Jan 17, 2022
@oishun1112 oishun1112 added the resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) label Jan 25, 2022
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) resolved Finding has been patched by sponsor (sponsor pls link to PR containing fix) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

2 participants