Connext Amarok contest - 0x29A's results

Low

Solidity version

You are currently using solidity 0.8.14, best recommendation is to switch to 0.8.15 two major bugs introduced on previus versions were fixes, plus its more gas efficient.

• SOURCE
• tldr;

Gas Report

Get rid of SafeMath lib

Since solidity 0.8.0 all arithmetic is checked by default. So there is no need to use it on most of your contracts, you are currently using v0.8.14.

Check this comment on OZ SafeMath and this stackoverflow thread

Files to refactor; ConnextPriceOracle.sol AmplificationUtils.sol SwapUtils.sol

I have make changes only on AmplificationUtils.sol and ConnextPriceOracle.sol

Recommendation (still missing the SwapUtils.sol refactor)

ConnextPriceOracle.sol

diff --git a/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol b/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol
index 1956333..f4b1379 100644
--- a/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol
+++ b/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.14;
 
-import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
+// import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import {IERC20Extended} from "../interfaces/IERC20Extended.sol";
 
@@ -42,7 +42,7 @@ interface AggregatorV3Interface {
 }
 
 contract ConnextPriceOracle is PriceOracle {
-  using SafeMath for uint256;
+  // using SafeMath for uint256;
   using SafeERC20 for IERC20Extended;
 
   address public admin;
@@ -101,12 +101,12 @@ contract ConnextPriceOracle is PriceOracle {
     if (priceInfo.active) {
       uint256 rawTokenAmount = IERC20Extended(priceInfo.token).balanceOf(priceInfo.lpToken);
       uint256 tokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.token).decimals());
-      uint256 tokenAmount = rawTokenAmount.mul(10**tokenDecimalDelta);
+      uint256 tokenAmount = rawTokenAmount * (10**tokenDecimalDelta);
       uint256 rawBaseTokenAmount = IERC20Extended(priceInfo.baseToken).balanceOf(priceInfo.lpToken);
       uint256 baseTokenDecimalDelta = 18 - uint256(IERC20Extended(priceInfo.baseToken).decimals());
-      uint256 baseTokenAmount = rawBaseTokenAmount.mul(10**baseTokenDecimalDelta);
+      uint256 baseTokenAmount = rawBaseTokenAmount * (10**baseTokenDecimalDelta);
       uint256 baseTokenPrice = getTokenPrice(priceInfo.baseToken);
-      uint256 tokenPrice = baseTokenPrice.mul(baseTokenAmount).div(tokenAmount);
+      uint256 tokenPrice = baseTokenPrice * (baseTokenAmount) / (tokenAmount);
 
       return tokenPrice;
     } else {
@@ -131,7 +131,7 @@ contract ConnextPriceOracle is PriceOracle {
 
       // Extend the decimals to 1e18.
       uint256 retVal = uint256(answer);
-      uint256 price = retVal.mul(10**(18 - uint256(aggregator.decimals())));
+      uint256 price = retVal * (10**(18 - uint256(aggregator.decimals())));
 
       return price;
     }

AmplificationUtils.sol

diff --git a/contracts/contracts/core/connext/libraries/AmplificationUtils.sol b/contracts/contracts/core/connext/libraries/AmplificationUtils.sol
index 6cd8a8d..d7006da 100644
--- a/contracts/contracts/core/connext/libraries/AmplificationUtils.sol
+++ b/contracts/contracts/core/connext/libraries/AmplificationUtils.sol
@@ -2,7 +2,7 @@
 pragma solidity 0.8.14;
 
 import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
-import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
+// import {SafeMath} from "@openzeppelin/contracts/utils/math/SafeMath.sol";
 
 import {SwapUtils} from "./SwapUtils.sol";
 
@@ -12,7 +12,7 @@ import {SwapUtils} from "./SwapUtils.sol";
  * This library assumes the struct is fully validated.
  */
 library AmplificationUtils {
-  using SafeMath for uint256;
+  // using SafeMath for uint256;
 
   event RampA(uint256 oldA, uint256 newA, uint256 initialTime, uint256 futureTime);
   event StopRampA(uint256 currentA, uint256 time);
@@ -30,7 +30,7 @@ library AmplificationUtils {
    * @return A parameter
    */
   function getA(SwapUtils.Swap storage self) internal view returns (uint256) {
-    return _getAPrecise(self).div(A_PRECISION);
+    return _getAPrecise(self) / A_PRECISION;
   }
 
   /**
@@ -58,10 +58,10 @@ library AmplificationUtils {
       uint256 a0 = self.initialA; // initial A value when ramp is started
       if (a1 > a0) {
         // a0 + (a1 - a0) * (block.timestamp - t0) / (t1 - t0)
-        return a0.add(a1.sub(a0).mul(block.timestamp.sub(t0)).div(t1.sub(t0)));
+        return a0 + (a1 - a0) * (block.timestamp - t0) / (t1 - t0);
       } else {
         // a0 - (a0 - a1) * (block.timestamp - t0) / (t1 - t0)
-        return a0.sub(a0.sub(a1).mul(block.timestamp.sub(t0)).div(t1.sub(t0)));
+        return a0 - (a0 - a1) * (block.timestamp - t0) / (t1 - t0);
       }
     } else {
       return a1;
@@ -81,17 +81,17 @@ library AmplificationUtils {
     uint256 futureA_,
     uint256 futureTime_
   ) internal {
-    require(block.timestamp >= self.initialATime.add(1 days), "Wait 1 day before starting ramp");
-    require(futureTime_ >= block.timestamp.add(MIN_RAMP_TIME), "Insufficient ramp time");
+    require(block.timestamp >= self.initialATime + 1 days, "Wait 1 day before starting ramp");
+    require(futureTime_ >= block.timestamp + MIN_RAMP_TIME, "Insufficient ramp time");
     require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
 
     uint256 initialAPrecise = _getAPrecise(self);
-    uint256 futureAPrecise = futureA_.mul(A_PRECISION);
+    uint256 futureAPrecise = futureA_ * A_PRECISION;
 
     if (futureAPrecise < initialAPrecise) {
-      require(futureAPrecise.mul(MAX_A_CHANGE) >= initialAPrecise, "futureA_ is too small");
+      require(futureAPrecise * MAX_A_CHANGE >= initialAPrecise, "futureA_ is too small");
     } else {
-      require(futureAPrecise <= initialAPrecise.mul(MAX_A_CHANGE), "futureA_ is too large");
+      require(futureAPrecise <= initialAPrecise * MAX_A_CHANGE, "futureA_ is too large");
     }
 
     self.initialA = initialAPrecise;

Gas Savings

Using forge snapshot from forge test cases

test_ConnextPriceOracle_setAggregators_worksIfOnlyAdmin() (gas: -32 (-0.000%)) 
test_ConnextPriceOracle_setDirectPrice_worksIfOnlyAdmin() (gas: -4 (-0.000%)) 
test_ConnextPriceOracle_setDexPriceInfo_failsIfInvalidBaseToken() (gas: -4 (-0.000%)) 
test_ConnextPriceOracle_getTokenPrice_worksIfExistsInAssetPrices() (gas: -4 (-0.000%)) 
test_ConnextPriceOracle_setDirectPrice_failsIfNotAdmin() (gas: -4 (-0.000%)) 
test_ConnextPriceOracle_setAdmin_worksIfOnlyAdmin() (gas: -4 (-0.000%)) 
test_ConnextPriceOracle_getTokenPrice_fails() (gas: -8 (-0.000%)) 
test_ConnextPriceOracle_setAdmin_failsIfNotAdmin() (gas: -4 (-0.000%)) 
test_ConnextPriceOracle_getTokenPrice_worksIfv1Exists() (gas: -8 (-0.000%)) 
test_ConnextPriceOracle_getPriceFromChainlink_fails() (gas: -4 (-0.001%)) 
test_ConnextPriceOracle_getPriceFromDex_fails() (gas: -4 (-0.001%)) 
test_ConnextPriceOracle_setDexPriceInfo_worksIfOnlyAdmin() (gas: -205 (-0.001%)) 
test_ConnextPriceOracle_getTokenPrice_worksIfDexRecordExists() (gas: -205 (-0.001%)) 
test_ConnextPriceOracle_getPriceFromDex_works() (gas: -205 (-0.001%)) 
test_ConnextPriceOracle_getPriceFromOracle_works() (gas: -57 (-0.003%)) 
test_ConnextPriceOracle_getPriceFromChainlink_works() (gas: -57 (-0.003%)) 
test_ConnextPriceOracle_getTokenPrice_worksIfAggregatorExists() (gas: -114 (-0.004%)) 
test_ConnextPriceOracle_setV1PriceOracle_worksIfOnlyAdmin() (gas: -4618 (-0.005%)) 
test_ConnextPriceOracle_setV1PriceOracle_failsIfNotAdmin() (gas: -4618 (-0.005%)) 
test_AmplificationUtils_rampA_works() (gas: -180 (-0.007%)) 
test_AmplificationUtils_rampA_revertIfWaitTimeNotEnough() (gas: -45 (-0.007%)) 
test_AmplificationUtils_stopRampA_works() (gas: -264 (-0.010%)) 
test_AmplificationUtils_rampA_revertIfInsufficientRampTime() (gas: -90 (-0.014%)) 
test_AmplificationUtils_rampA_revertIfInvalidFutureTime() (gas: -90 (-0.014%)) 
test_AmplificationUtils_rampA_revertIfFuturePriceTooLarge() (gas: -180 (-0.016%)) 
test_AmplificationUtils_rampA_revertIfFuturePriceTooSmall() (gas: -180 (-0.016%)) 
test_AmplificationUtils_getAPrecise_works() (gas: -264 (-0.021%)) 
test_AmplificationUtils_getA_works() (gas: -306 (-0.024%)) 
test_AmplificationUtils__getAPrecise_works() (gas: -528 (-0.034%)) 
Overall gas change: -12286 (-0.191%)

Add unchecked to MathUtils.sol

You can safely add unchecked to the difference function because you check the overflow. You could also write it more simple; unchecked { return a > b ? a - b : b - 1; }

diff --git a/contracts/contracts/core/connext/libraries/MathUtils.sol b/contracts/contracts/core/connext/libraries/MathUtils.sol
index 463d01e..06ed7e5 100644
--- a/contracts/contracts/core/connext/libraries/MathUtils.sol
+++ b/contracts/contracts/core/connext/libraries/MathUtils.sol
@@ -26,9 +26,11 @@ library MathUtils {
    * @return Difference between a and b
    */
   function difference(uint256 a, uint256 b) internal pure returns (uint256) {
-    if (a > b) {
-      return a - b;
+    unchecked {
+      if (a > b) {
+        return a - b;
+      }
+      return b - a;
     }
-    return b - a;
   }
 }

Gas Savings

Using forge snapshot from forge test cases

test_MathUtils_within1_works() (gas: -183 (-0.182%)) 
test_MathUtils_difference_works() (gas: -122 (-0.214%)) 
Overall gas change: -305 (-0.396%)

Comparasions

> 0 is less efficient than != 0 for unsigned integers (with proof) != 0 costs less gas compared to > 0 for unsigned integers in require statements with the optimizer enabled (6 gas)

diff --git a/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol b/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol
index 1956333..e3a7351 100644
--- a/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol
+++ b/contracts/contracts/core/connext/helpers/ConnextPriceOracle.sol
@@ -147,7 +147,7 @@ contract ConnextPriceOracle is PriceOracle {
   ) external onlyAdmin {
     PriceInfo storage priceInfo = priceRecords[_token];
     uint256 baseTokenPrice = getTokenPrice(_baseToken);
-    require(baseTokenPrice > 0, "invalid base token");
+    require(baseTokenPrice != 0, "invalid base token");
     priceInfo.token = _token;
     priceInfo.baseToken = _baseToken;
     priceInfo.lpToken = _lpToken;
diff --git a/contracts/contracts/core/connext/libraries/AmplificationUtils.sol b/contracts/contracts/core/connext/libraries/AmplificationUtils.sol
index 6cd8a8d..0f65c7d 100644
--- a/contracts/contracts/core/connext/libraries/AmplificationUtils.sol
+++ b/contracts/contracts/core/connext/libraries/AmplificationUtils.sol
@@ -83,7 +83,7 @@ library AmplificationUtils {
   ) internal {
     require(block.timestamp >= self.initialATime.add(1 days), "Wait 1 day before starting ramp");
     require(futureTime_ >= block.timestamp.add(MIN_RAMP_TIME), "Insufficient ramp time");
-    require(futureA_ > 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
+    require(futureA_ != 0 && futureA_ < MAX_A, "futureA_ must be > 0 and < MAX_A");
 
     uint256 initialAPrecise = _getAPrecise(self);
     uint256 futureAPrecise = futureA_.mul(A_PRECISION);
diff --git a/contracts/contracts/core/connext/libraries/LibDiamond.sol b/contracts/contracts/core/connext/libraries/LibDiamond.sol
index a586e6c..f6242c6 100644
--- a/contracts/contracts/core/connext/libraries/LibDiamond.sol
+++ b/contracts/contracts/core/connext/libraries/LibDiamond.sol
@@ -223,7 +223,7 @@ library LibDiamond {
     if (_init == address(0)) {
       require(_calldata.length == 0, "LibDiamondCut: _init is address(0) but_calldata is not empty");
     } else {
-      require(_calldata.length > 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
+      require(_calldata.length != 0, "LibDiamondCut: _calldata is empty but _init is not address(0)");
       if (_init != address(this)) {
         enforceHasContractCode(_init, "LibDiamondCut: _init address has no code");
       }
@@ -244,6 +244,6 @@ library LibDiamond {
     assembly {
       contractSize := extcodesize(_contract)
     }
-    require(contractSize > 0, _errorMessage);
+    require(contractSize != 0, _errorMessage);
   }
 }