search

code-423n4 / 2022-01-notional-findings

1 stars 3 forks source link

using multiple require rather than && #208

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

rfa

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

https://github.com/code-423n4/2022-01-notional/blob/main/contracts/TreasuryAction.sol#L46

Recommended Mitigation Steps

require(0 < currencyId , "Invalid currency id");
require(currencyId <= maxCurrencyId, "Invalid currency id");
jeffywu commented 2 years ago

Unclear that this actually reduces gas, will definitely increase bytecode size with the duplicated string which is not good.

pauliax commented 2 years ago

While I think it will cost less gas to execute separate statements, the improvement is not that significant, and as per the sponsor's comment, the revert message has to be duplicated also.