search

sujithsomraaj / lifi-swap-facet-v3-audit

1 Day Review - 5/28
0 stars 0 forks source link

Increase branch coverage #2

Open sujithsomraaj opened 4 months ago

sujithsomraaj commented 4 months ago

Context: GenericSwapFacetV3.sol#L456, GenericSwapFacetV3.sol#L107, GenericSwapFacetV3.sol#L500, GenericSwapFacetV3.sol#L502, GenericSwapFacetV3.sol#L539, GenericSwapFacetV3.sol#L542

Description: The codebase under review has solid unit test coverage but has certain areas of code not tested, and it would be nice to have them added to the test suite. The uncovered lines include,

  1. L107 and #L456, where the revert case is not tested.

if (!success) revert NativeAssetTransferFailed();

  1. L500, where the current allowance is greater than the case, is not tested, and in #L502, the current allowance equals 0 case as well.

if (currentAllowance < fromAmount) {
   // check if is non-zero, set to 0 if not
   if (currentAllowance != 0) sendingAsset.safeApprove(approveTo, 0);
...

  1. L539 and #L542, where the negative case is not tested when nativeBalance is zero and the send call is a failing case.

if (nativeBalance > 0) {
  // solhint-disable-next-line avoid-low-level-calls
  (bool success, ) = receiver.call{ value: nativeBalance }("");
  if (!success) revert NativeAssetTransferFailed();
}

Recommendation: Consider adding tests for the above branches to improve the code coverage.

LI.FI:

Researcher:

0xDEnYO commented 4 months ago

Looking into this...my force coverage shows me 100% coverage. Not sure why our results differ

0xDEnYO commented 4 months ago

Fixed the issue why coverage was missing branch data. Added tests to achieve 100% coverage.