Back to Pechenite's contests

Salty.IO - Pechenite's results

An Ethereum-based DEX with zero swap fees, yield-generating Automatic Arbitrage, and a native WBTC/WETH backed stablecoin.

General Information

Platform: Code4rena

Start Date: 16/01/2024

Pot Size: $80,000 USDC

Total HM: 37

Participants: 178

Period: 14 days

Judge: Picodes

Total Solo HM: 4

Id: 320

League: ETH

Salty.IO

Findings Distribution

Researcher Performance

Rank: 143/178

Findings: 1

Award: $20.79

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest PageGithub Contest RepositoryGithub Findings RepositorySalty.IO

Findings Information

🌟 Selected for report: 0xVolcano

Also found by: 0x11singh99, 0xAnah, Beepidibop, JCK, JcFichtner, K42, Kaysoft, Pechenite, Raihan, Rolezn, dharma09, hunter_w3b, lsaudit, n0kto, naman1778, niroh, sivanesh_808, slvDev, unique

Awards

20.7932 USDC - $20.79

Labels

bug
G (Gas Optimization)
grade-b
G-19

External Links

Link to GitHub issue

Summary

[G-01] Replace type(uint<n>).max with constants

Saved gas per instance: 4

File: src/ManagedWallet.sol

37: 		activeTimelock = type(uint256).max;

68: 			activeTimelock = type(uint256).max; // effectively never

86: 		activeTimelock = type(uint256).max;
Optimized Code:
                // Write a value so subsequent writes take less gas
-               activeTimelock = type(uint256).max;
+               activeTimelock = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
         }


        else
-                       activeTimelock = type(uint256).max; // effectively never
+                       activeTimelock = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF; // effectively never
         }


                // Reset
-               activeTimelock = type(uint256).max;
+               activeTimelock = 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF;
                }
<details> <summary><b>Other instances:</b></summary> </details>

[G-02] State variables can be packed into fewer storage slots by truncating timestamp bytes

By using a uint32 rather than a larger type for variables that track timestamps, one can save gas by using fewer storage slots per struct, at the expense of the protocol breaking after the year 2106 (when uint32 wraps). If this is an acceptable tradeoff, if variables occupying the same slot are both written the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper

Saved gas: 20,000

File: src/price_feed/PriceAggregator.sol

13: contract PriceAggregator is IPriceAggregator, Ownable
14:     {
15:     event PriceFeedSet(uint256 indexed priceFeedNum, IPriceFeed indexed newPriceFeed);
16:     event MaximumPriceFeedPercentDifferenceChanged(uint256 newMaxDifference);
17:     event SetPriceFeedCooldownChanged(uint256 newCooldown);
18:
19: 	IPriceFeed public priceFeed1; // CoreUniswapFeed by default
20: 	IPriceFeed public priceFeed2; // CoreChainlinkFeed by default
21: 	IPriceFeed public priceFeed3; // CoreSaltyFeed by default
22:
23: 	// The next time at which setPriceFeed can be called
24: 	uint256 public priceFeedModificationCooldownExpiration;
25:
26: 	// The maximum percent difference between two non-zero PriceFeed prices when aggregating price.
27: 	// When the two closest PriceFeeds (out of the three) have prices further apart than this the aggregated price is considered invalid.
28: 	// Range: 1% to 7% with an adjustment of .50%
29: 	uint256 public maximumPriceFeedPercentDifferenceTimes1000 = 3000; // Defaults to 3.0% with a 1000x multiplier
30:
31: 	// The required cooldown between calls to setPriceFeed.
32: 	// Allows time to evaluate the performance of the recently updatef PriceFeed before further updates are made.
33: 	// Range: 30 to 45 days with an adjustment of 5 days
34: 	uint256 public priceFeedModificationCooldown = 35 days;
Optimized Code:

        // The next time at which setPriceFeed can be called
-       uint256 public priceFeedModificationCooldownExpiration;
+       uint32 public priceFeedModificationCooldownExpiration;
+
+       // The required cooldown between calls to setPriceFeed.
+       // Allows time to evaluate the performance of the recently updatef PriceFeed before further updates are made.
+       // Range: 30 to 45 days with an adjustment of 5 days
+       uint32 public priceFeedModificationCooldown = 35 days;

        // The maximum percent difference between two non-zero PriceFeed prices when aggregating price.
        // When the two closest PriceFeeds (out of the three) have prices further apart than this the aggregated price is considered invalid.
        // Range: 1% to 7% with an adjustment of .50%
        uint256 public maximumPriceFeedPercentDifferenceTimes1000 = 3000; // Defaults to 3.0% with a 1000x multiplier

-       // The required cooldown between calls to setPriceFeed.
-       // Allows time to evaluate the performance of the recently updatef PriceFeed before further updates are made.
-       // Range: 30 to 45 days with an adjustment of 5 days
-       uint256 public priceFeedModificationCooldown = 35 days;

[G-03] Order of initial checks in a function can be optimized

Some of the checks below involve expensive operations, such as SLOAD. We can reorder them so cheaper checks are done first, and expensive checks are done last. This way, if a cheaper check fails, we can avoid expensive operations.

ManagedWallet.sol:

42: 	function proposeWallets( address _proposedMainWallet, address _proposedConfirmationWallet ) external
43: 		{
44: 		require( msg.sender == mainWallet, "Only the current mainWallet can propose changes" ); // @audit expensive operation
45: 		require( _proposedMainWallet != address(0), "_proposedMainWallet cannot be the zero address" );
46: 		require( _proposedConfirmationWallet != address(0), "_proposedConfirmationWallet cannot be the zero address" );
47:
48: 		// Make sure we're not overwriting a previous proposal (as only the confirmationWallet can reject proposals)
49: 		require( proposedMainWallet == address(0), "Cannot overwrite non-zero proposed mainWallet." ); // @audit expensive operation
Optimized Code:
        // Make a request to change the main and confirmation wallets.
        function proposeWallets( address _proposedMainWallet, address _proposedConfirmationWallet ) external
                {
-               require( msg.sender == mainWallet, "Only the current mainWallet can propose changes" );
                require( _proposedMainWallet != address(0), "_proposedMainWallet cannot be the zero address" );
                require( _proposedConfirmationWallet != address(0), "_proposedConfirmationWallet cannot be the zero address" );
+               require( msg.sender == mainWallet, "Only the current mainWallet can propose changes" );

                // Make sure we're not overwriting a previous proposal (as only the confirmationWallet can reject proposals)
                require( proposedMainWallet == address(0), "Cannot overwrite non-zero proposed mainWallet." );
BootstrapBallot.sol
69: 	function finalizeBallot() external nonReentrant
70: 		{
71: 		require( ! ballotFinalized, "Ballot has already been finalized" ); // @audit expensive operation
72: 		require( block.timestamp >= completionTimestamp, "Ballot is not yet complete" );
Optimized Code:
        // Ensures that the completionTimestamp has been reached and then calls InitialDistribution.distributionApproved and DAO.initialGeoExclusion if the voters have approved the ballot.
        function finalizeBallot() external nonReentrant
                {
-               require( ! ballotFinalized, "Ballot has already been finalized" );
                require( block.timestamp >= completionTimestamp, "Ballot is not yet complete" );
+               require( ! ballotFinalized, "Ballot has already been finalized" );
Pools.sol
140: 	function addLiquidity( IERC20 tokenA, IERC20 tokenB, uint256 maxAmountA, uint256 maxAmountB, uint256 minLiquidityReceived, uint256 totalLiquidity ) external nonReentrant returns (uint256 addedAmountA, uint256 addedAmountB, uint256 addedLiquidity)
141: 		{
142: 		require( msg.sender == address(collateralAndLiquidity), "Pools.addLiquidity is only callable from the CollateralAndLiquidity contract" ); // @audit expensive operation
143: 		require( exchangeIsLive, "The exchange is not yet live" ); // @audit expensive operation
144: 		require( address(tokenA) != address(tokenB), "Cannot add liquidity for duplicate tokens" );
145:
146: 		require( maxAmountA > PoolUtils.DUST, "The amount of tokenA to add is too small" );
147: 		require( maxAmountB > PoolUtils.DUST, "The amount of tokenB to add is too small" );

170: 	function removeLiquidity( IERC20 tokenA, IERC20 tokenB, uint256 liquidityToRemove, uint256 minReclaimedA, uint256 minReclaimedB, uint256 totalLiquidity ) external nonReentrant returns (uint256 reclaimedA, uint256 reclaimedB)
171: 		{
172: 		require( msg.sender == address(collateralAndLiquidity), "Pools.removeLiquidity is only callable from the CollateralAndLiquidity contract" ); // @audit expensive operation
173: 		require( liquidityToRemove > 0, "The amount of liquidityToRemove cannot be zero" );

219:     function withdraw( IERC20 token, uint256 amount ) external nonReentrant
220:     	{
221:     	require( _userDeposits[msg.sender][token] >= amount, "Insufficient balance to withdraw specified amount" ); // @audit expensive operation
222:         require( amount > PoolUtils.DUST, "Withdraw amount too small");
Optimized Code:
        function addLiquidity( IERC20 tokenA, IERC20 tokenB, uint256 maxAmountA, uint256 maxAmountB, uint256 minLiquidityReceived, uint256 totalLiquidity ) external nonReentrant returns (uint256 addedAmountA, uint256 addedAmountB, uint256 addedLiquidity)
                {
-               require( msg.sender == address(collateralAndLiquidity), "Pools.addLiquidity is only callable from the CollateralAndLiquidity contract" );
-               require( exchangeIsLive, "The exchange is not yet live" );
                require( address(tokenA) != address(tokenB), "Cannot add liquidity for duplicate tokens" );

                require( maxAmountA > PoolUtils.DUST, "The amount of tokenA to add is too small" );
                require( maxAmountB > PoolUtils.DUST, "The amount of tokenB to add is too small" );

+               require( msg.sender == address(collateralAndLiquidity), "Pools.addLiquidity is only callable from the CollateralAndLiquidity contract" );
+               require( exchangeIsLive, "The exchange is not yet live" );
+

        function removeLiquidity( IERC20 tokenA, IERC20 tokenB, uint256 liquidityToRemove, uint256 minReclaimedA, uint256 minReclaimedB, uint256 totalLiquidity ) external nonReentrant returns (uint256 reclaimedA, uint256 reclaimedB)
                {
-               require( msg.sender == address(collateralAndLiquidity), "Pools.removeLiquidity is only callable from the CollateralAndLiquidity contract" );
                require( liquidityToRemove > 0, "The amount of liquidityToRemove cannot be zero" );
+               require( msg.sender == address(collateralAndLiquidity), "Pools.removeLiquidity is only callable from the CollateralAndLiquidity contract" );


     function withdraw( IERC20 token, uint256 amount ) external nonReentrant
        {
-       require( _userDeposits[msg.sender][token] >= amount, "Insufficient balance to withdraw specified amount" );
         require( amount > PoolUtils.DUST, "Withdraw amount too small");
+       require( _userDeposits[msg.sender][token] >= amount, "Insufficient balance to withdraw specified amount" );
SaltRewards.sol
117: 	function performUpkeep( bytes32[] calldata poolIDs, uint256[] calldata profitsForPools ) external
118: 		{
119: 		require( msg.sender == address(exchangeConfig.upkeep()), "SaltRewards.performUpkeep is only callable from the Upkeep contract" ); // @audit expensive operation
120: 		require( poolIDs.length == profitsForPools.length, "Incompatible array lengths" );
Optimized Code:
 function performUpkeep( bytes32[] calldata poolIDs, uint256[] calldata profitsForPools ) external
                {
-               require( msg.sender == address(exchangeConfig.upkeep()), "SaltRewards.performUpkeep is only callable from the Upkeep contract" );
                require( poolIDs.length == profitsForPools.length, "Incompatible array lengths" );
+               require( msg.sender == address(exchangeConfig.upkeep()), "SaltRewards.performUpkeep is only callable from the Upkeep contract" );
CollateralAndLiquidity.sol
115:      function repayUSDS( uint256 amountRepaid ) external nonReentrant
116: 		{
117: 		require( userShareForPool( msg.sender, collateralPoolID ) > 0, "User does not have any collateral" ); // @audit expensive operation
118: 		require( amountRepaid <= usdsBorrowedByUsers[msg.sender], "Cannot repay more than the borrowed amount" ); // @audit expensive operation
119: 		require( amountRepaid > 0, "Cannot repay zero amount" );
Optimized Code:
      function repayUSDS( uint256 amountRepaid ) external nonReentrant
                {
+               require( amountRepaid > 0, "Cannot repay zero amount" );
                require( userShareForPool( msg.sender, collateralPoolID ) > 0, "User does not have any collateral" );
                require( amountRepaid <= usdsBorrowedByUsers[msg.sender], "Cannot repay more than the borrowed amount" );
-               require( amountRepaid > 0, "Cannot repay zero amount" );
USDS.sol
40: 	function mintTo( address wallet, uint256 amount ) external
41: 		{
42: 		require( msg.sender == address(collateralAndLiquidity), "USDS.mintTo is only callable from the Collateral contract" ); // @audit expensive operation
43: 		require( amount > 0, "Cannot mint zero USDS" );
Optimized Code:
 function mintTo( address wallet, uint256 amount ) external
                {
-               require( msg.sender == address(collateralAndLiquidity), "USDS.mintTo is only callable from the Collateral contract" );
                require( amount > 0, "Cannot mint zero USDS" );
+               require( msg.sender == address(collateralAndLiquidity), "USDS.mintTo is only callable from the Collateral contract" );
StakingRewards.osl
57: 	function _increaseUserShare( address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown ) internal
58: 		{
59: 		require( poolsConfig.isWhitelisted( poolID ), "Invalid pool" ); // @audit expensive operation
60: 		require( increaseShareAmount != 0, "Cannot increase zero share" );
Optimized Code:
function _increaseUserShare( address wallet, bytes32 poolID, uint256 increaseShareAmount, bool useCooldown ) internal
                {
-               require( poolsConfig.isWhitelisted( poolID ), "Invalid pool" );
                require( increaseShareAmount != 0, "Cannot increase zero share" );
+               require( poolsConfig.isWhitelisted( poolID ), "Invalid pool" );

[G-04] Stack variable is only used once

If the variable is only accessed once, it's cheaper to use the assigned value directly that one time, and save the 3 gas the extra stack assignment would spend.

Saved gas per instances: 3

File: src/AccessManager.sol

51:    function _verifyAccess(address wallet, bytes memory signature ) internal view returns (bool)
52:    	{
53:		bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, geoVersion, wallet));
54:
55:		return SigningTools._verifySignature(messageHash, signature);
56:    	}
Optimized Code:
     function _verifyAccess(address wallet, bytes memory signature ) internal view returns (bool)
        {
-               bytes32 messageHash = keccak256(abi.encodePacked(block.chainid, geoVersion, wallet));
-
-               return SigningTools._verifySignature(messageHash, signature);
+               return SigningTools._verifySignature(keccak256(abi.encodePacked(block.chainid, geoVersion, wallet)), signature);
        }
<details> <summary><b>Other instances:</b></summary> </details>

[G-05] Consider caching FixedPoint96.Q96 * (10 ** 18)

The following computation is repeated in multiple places in one function and can be cached to save gas:

📁 File: src/price_feed/CoreUniswapFeed.sol

50:     function _getUniswapTwapWei( IUniswapV3Pool pool, uint256 twapInterval ) public view returns (uint256)
51:     	{
52: 		uint32[] memory secondsAgo = new uint32[](2);
53: 		secondsAgo[0] = uint32(twapInterval); // from (before)
54: 		secondsAgo[1] = 0; // to (now)
55:
56:         // Get the historical tick data using the observe() function
57:          (int56[] memory tickCumulatives, ) = pool.observe(secondsAgo);
58:
59: 		int24 tick = int24((tickCumulatives[1] - tickCumulatives[0]) / int56(uint56(twapInterval)));
60: 		uint160 sqrtPriceX96 = TickMath.getSqrtRatioAtTick( tick );
61: 		uint256 p = FullMath.mulDiv(sqrtPriceX96, sqrtPriceX96, FixedPoint96.Q96 );
62:
63: 		uint8 decimals0 = ( ERC20( pool.token0() ) ).decimals();
64: 		uint8 decimals1 = ( ERC20( pool.token1() ) ).decimals();
65:
66: 		if ( decimals1 > decimals0 )
67: 			return FullMath.mulDiv( 10 ** ( 18 + decimals1 - decimals0 ), FixedPoint96.Q96, p );
68:
69: 		if ( decimals0 > decimals1 )
70: 			return ( FixedPoint96.Q96 * ( 10 ** 18 ) ) / ( p * ( 10 ** ( decimals0 - decimals1 ) ) );
71:
72: 		return ( FixedPoint96.Q96 * ( 10 ** 18 ) ) / p;
73:     	}
Optimized Code:
if ( decimals1 > decimals0 )
                        return FullMath.mulDiv( 10 ** ( 18 + decimals1 - decimals0 ), FixedPoint96.Q96, p );

+               uint _temp = FixedPoint96.Q96 * ( 10 ** 18 );
                if ( decimals0 > decimals1 )
-                       return ( FixedPoint96.Q96 * ( 10 ** 18 ) ) / ( p * ( 10 ** ( decimals0 - decimals1 ) ) );
+                       return ( _temp ) / ( p * ( 10 ** ( decimals0 - decimals1 ) ) );

-               return ( FixedPoint96.Q96 * ( 10 ** 18 ) ) / p;
+               return ( _temp ) / p;
        }

[G-06] Use hardcoded address instead of address(this)

It can be more gas-efficient to use a hardcoded address instead of the address(this) expression, especially if you need to use the same address multiple times in your contract.

The reason for this, is that using address(this) requires an additional EXTCODESIZE operation to retrieve the contract’s address from its bytecode, which can increase the gas cost of your contract. By pre-calculating and using a hardcoded address, you can avoid this additional operation and reduce the overall gas cost of your contract.

<details> <summary><b>Instances:</b></summary> </details>

[G-07] Use assembly to perform efficient back-to-back calls

If a similar external call is performed back-to-back, we can use assembly to reuse any function signatures and function parameters that stay the same. In addition, we can also reuse the same memory space for each function call (scratch space + free memory pointer + zero slot), which can potentially allow us to avoid memory expansion costs.

Note: In order to do this optimization safely we will cache the free memory pointer value and restore it once we are done with our function calls. We will also set the zero slot back to 0 if neccessary.

File: src/rewards/SaltRewards.sol

41: 		salt.approve( address(stakingRewardsEmitter), type(uint256).max );
42: 		salt.approve( address(liquidityRewardsEmitter), type(uint256).max );
Optimized Code:
-               salt.approve( address(stakingRewardsEmitter), type(uint256).max );
-               salt.approve( address(liquidityRewardsEmitter), type(uint256).max );
+               ISalt _salt = salt;
+               assembly {
+                       // function selectors for `approve()`
+                       mstore(0x00, 0x095ea7b3)
+
+                       // store memory pointer
+                       let memptr := mload(0x40)
+                       mstore(0x20, _stakingRewardsEmitter)
+                       mstore(0x40, 0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF)
+                       if iszero(call(gas(), _salt, 0x00, 0x1c, 0x44, 0x00, 0x00)) { revert(0,0) }
+
+                       mstore(0x20, _liquidityRewardsEmitter)
+                       if iszero(call(gas(), _salt, 0x00, 0x1c, 0x44, 0x00, 0x00)) { revert(0,0) }
+
+                       // restore memory pointer
+                       mstore(0x40, memptr)
+               }
        }
<details> <summary><b>Other instances:</b></summary> </details>

[G-08] Inverting the condition of an if-else-statement wastes gas

Flipping the true and false blocks instead saves 3 gas.

File: src/price_feed/CoreUniswapFeed.sol

125:         if ( ! weth_usdcFlipped )
126:         	return 10**36 / uniswapWETH_USDC;
127:         else
128:         	return uniswapWETH_USDC;
Optimized Code:
-        if ( ! weth_usdcFlipped )
-               return 10**36 / uniswapWETH_USDC;
-        else
+        if ( weth_usdcFlipped )
                return uniswapWETH_USDC;
+        else
+               return 10**36 / uniswapWETH_USDC;
                }

[G-09] Declare variables outside of loops

Variables should be declared outside of loops, and get overriden with each iteration of loop, By doing so we save gas cost for memory variable declaration in each iteration.

Saved gas per instance: 15

File: src/arbitrage/ArbitrageSearch.sol

117: 				uint256 midpoint = (leftPoint + rightPoint) >> 1;
Optimized Code:
                        uint256 leftPoint = swapAmountInValueInETH >> 7;
                        uint256 rightPoint = swapAmountInValueInETH + (swapAmountInValueInETH >> 2); // 100% + 25% of swapAmountInValueInETH

+                       uint256 midpoint;
                        // Cost is about 492 gas per loop iteration
                        for( uint256 i = 0; i < 8; i++ )
                                {
-                               uint256 midpoint = (leftPoint + rightPoint) >> 1;
+                               midpoint = (leftPoint + rightPoint) >> 1;

                                // Right of midpoint is more profitable?
<details> <summary><b>Other instances:</b></summary> </details>

[G-10] Do not calculate constants

Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas.

File: src/Salt.sol

13: 	uint256 public constant INITIAL_SUPPLY = 100 * MILLION_ETHER ;
Optimized Code:
-       uint256 public constant INITIAL_SUPPLY = 100 * MILLION_ETHER ;
+       uint256 public constant INITIAL_SUPPLY = 1e27; // 100 milion ether

[G-11] Counting down in for statements is more gas efficient

Counting down is more gas efficient than counting up because neither we are making zero variable to non-zero variable and also we will get gas refund in the last transaction when making non-zero to zero variable. More info

File: src/pools/PoolStats.sol

53: 		for( uint256 i = 0; i < poolIDs.length; i++ )
Optimized Code:
-               for( uint256 i = 0; i < poolIDs.length; i++ )
+               uint256 len = poolIDs.length - 1;
+               for( uint256 i = len; i >= 0; i--)
                        _arbitrageProfits[ poolIDs[i] ] = 0;
                }
<details> <summary><b>Other instances:</b></summary> </details>

[G-12] Use assembly scratch space to build calldata for event emits

Utilizing Solidity's assembly scratch space to build calldata for emitting events with just one or two arguments can optimize gas usage. The scratch space, a designated area in the first 64 bytes of memory, is ideal for temporary storage during assembly-level operations. By directly writing the event arguments into this area, developers can bypass the standard memory allocation process required for event emission. This approach results in gas savings, particularly for contracts where events are frequently emitted. However, such low-level optimization requires a deep understanding of Ethereum Virtual Machine (EVM) mechanics and careful coding to prevent data mishandling. Rigorous testing is essential to ensure the integrity and correct functionality of the contract.

Saved gas per instance: 220

📁 File: src/AccessManager.sol

67:         emit AccessGranted( msg.sender, geoVersion );
Optimized Code:
-        emit AccessGranted( msg.sender, geoVersion );
+        // emit AccessGranted( msg.sender, geoVersion );
+                               uint256 _geoVersion = geoVersion;
+                               assembly {
+            mstore(0x00, caller())
+            mstore(0x20, _geoVersion)
+            log1(
+                0x00,
+                0x40,
+                // keccak256("AccessGranted(address,uint256)")
+                0xb4c6779ceb4a20f448e76a0e11f39bd183cff9c9dbac53df6bfcc202e2eb32f1
+            )
+        }
<details> <summary><b>Other instances:</b></summary> </details>

#0 - c4-judge

2024-02-03T14:34:37Z

Picodes marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter