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

Short the following require messages #3

Open
code423n4 opened this issue Dec 2, 2021 · 2 comments
Open

Short the following require messages #3

code423n4 opened this issue Dec 2, 2021 · 2 comments
Labels
bug Something isn't working G (Gas Optimization) 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

robee

Vulnerability details

The following require messages are of length more than 32 and we think are short enough to short
them into exactly 32 characters such that it will be placed in one slot of memory and the require
function will cost less gas.
The list:

    Solidity file: SushiswapStrategy.sol, In line 53, Require message length to shorten: 38
    Solidity file: SushiswapStrategy.sol, In line 74, Require message length to shorten: 33
    Solidity file: UniswapV2Strategy.sol, In line 53, Require message length to shorten: 38
    Solidity file: UniswapV2Strategy.sol, In line 74, Require message length to shorten: 33
    Solidity file: MapleProxyFactory.sol, In line 33, Require message length to shorten: 36
    Solidity file: MapleProxyFactory.sol, In line 42, Require message length to shorten: 36
@code423n4 code423n4 added bug Something isn't working G (Gas Optimization) labels Dec 2, 2021
code423n4 added a commit that referenced this issue Dec 2, 2021
@deluca-mike
Copy link
Collaborator

deluca-mike commented Dec 3, 2021

Agreed with 4 out ot 6, but these aren't bugs.

SushiswapStrategy.sol, line 53: agreed, it would be better as "SS:WRONG_COLLATERAL_AMT" or "SS:WRONG_COLLATERAL"
SushiswapStrategy.sol, line 74: agreed, it would be better as "SS:TRANSFER_FAILED"
UniswapV2Strategy.sol, line 53: agreed, it would be better as "U2S:WRONG_COLLATERAL_AMT" or "U2S:WRONG_COLLATERAL"
UniswapV2Strategy.sol, line 74: agreed, it would be better as "U2S:TRANSFER_FAILED"
MapleProxyFactory.sol, line 33: possibly it can be "MPF:DUP:NO_OVERWRITE_INITIALIZER" but it would be grammatically awkward
MapleProxyFactory.sol, line 42: possibly it can be "MPF:EUP:NO_OVERWRITE_INITIALIZER" but it would be grammatically awkward

@deluca-mike deluca-mike added disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") and removed disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) labels Dec 3, 2021
@pauliax
Copy link
Collaborator

pauliax commented Dec 15, 2021

Valid optimization.

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

3 participants