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

0 stars 0 forks source link

&& operator can use more gas #128

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Handle

rfa

Vulnerability details

Impact

more expensive gas usage

Proof of Concept

instead of using operator && on single require check (XDEFIDistribution.sol line 255). using double require check can save more gas:

require(amount != uint256(0) && amount <= MAX_TOTAL_XDEFI_SUPPLY, "INVALID_AMOUNT");

Tools Used

Recommended Mitigation Steps

require(amount_ != uint256(0), "INVALIDAMOUNT" ); require(amount <= MAX_TOTAL_XDEFI_SUPPLY, "INVALID_AMOUNT");

deluca-mike commented 2 years ago

While two seperate require checks result in about 14 gas saved on lock, relock, and relockBatch, it costs 10923 more gas to deploy, which it worth it after 780 calls. I guess this is a fair trade-off. In any case, due to the move to if-reverts and custom error messages, and the removal of the MAX_TOTAL_XDEFI_SUPPLY variable, and it's check, this is being changed to a check on the resulting computed units:

if (units == uint256(0)) revert LockResultsInNoUnits();
deluca-mike commented 2 years ago

In release candidate contracts, amount_ is no longer checked, and thus MAX_TOTAL_XDEFI_SUPPLY is not needed, and the condition is no longer two-fold. Instead, resulting units are checked to be greater or equal to some minimum.

Ivshti commented 2 years ago

resolved & confirmed