Decent - dharma09's results

Decent enables one-click transactions using any token across chains.

General Information

Platform: Code4rena

Start Date: 19/01/2024

Pot Size: $36,500 USDC

Total HM: 9

Participants: 113

Period: 3 days

Judge: 0xsomeone

Id: 322

League: ETH

Decent

Findings Distribution

Researcher Performance

Rank: 49/113

Findings: 1

Award: $38.84

🌟 Selected for report: 0

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: c3phas

Also found by: 0x11singh99, Raihan, dharma09, hunter_w3b, slvDev

Labels

bug
G (Gas Optimization)
grade-b
sufficient quality report
G-05

Awards

38.8402 USDC - $38.84

External Links

Gas report

Table Of Contents

[G-01] Refactor execute() function for better gas saving

Impact :

In this function first if statement check !gasCurrencyIsEth which is state variable than we check !deliverEth.

Proof of Code :

So the trick is to check the first local variable and if it does not satisfy the if condition, then the else part is executed, allowing us to avoid state reading.

File: lib/decent-bridge/src/DecentBridgeExecutor.sol
68: function execute(
        address from,
        address target,
        bool deliverEth,
        uint256 amount,
        bytes memory callPayload
    ) public onlyOwner {
        weth.transferFrom(msg.sender, address(this), amount);

@>        if (!gasCurrencyIsEth || !deliverEth) { //@audit change order of if condition bcz first check state variable
            _executeWeth(from, target, amount, callPayload);
        } else {
            _executeEth(from, target, amount, callPayload);
        }
    }

Optimized code :

File: lib/decent-bridge/src/DecentBridgeExecutor.sol
68: function execute(
        address from,
        address target,
        bool deliverEth,
        uint256 amount,
        bytes memory callPayload
    ) public onlyOwner {
        weth.transferFrom(msg.sender, address(this), amount);

-        if (!gasCurrencyIsEth || !deliverEth) { 
+        if (!deliverEth || !gasCurrencyIsEth) {
            _executeWeth(from, target, amount, callPayload);
        } else {
            _executeEth(from, target, amount, callPayload);
        }
    }

[G-02] Avoid the state variable read in modifier, instead we can use the memory variable

Impact: In the modifier avoid the state variable read, instead use the memory variable

To prevent integer overflow/underflow issues, consider using the SafeMath library. This library provides safe arithmetic operations. If you're not already using it, you can add it to your contract.

Proof of Concept :

userIsWithdrawing modifier update the balance of the user after withdraw, so based on below logic we can avoid state read-through using the memory value of balanceog[msg.sender], and as a result we can save gas.

File: lib/decent-bridge/src/DecentEthRouter.sol
60: modifier userIsWithdrawing(uint256 amount) {
        uint256 balance = balanceOf[msg.sender];
        require(balance >= amount, "not enough balance");
        _;
@>        balanceOf[msg.sender] -= amount; //@audit use memory value
    }

Optimized code :

File: lib/decent-bridge/src/DecentEthRouter.sol
60: modifier userIsWithdrawing(uint256 amount) {
        uint256 balance = balanceOf[msg.sender];
        require(balance >= amount, "not enough balance");
        _;
-        balanceOf[msg.sender] -= amount; 
+        balanceOf[msg.sender] = balance - amount;
    }

[G-03] Mark this value as constant

Impact : Consider using the constant keyword to declare the GAS_FOR_RELAY variable as a constant. This helps the compiler optimize the code and makes it clear that the value is intended to be constant.

Proof of Concept: We can save large amounts of gas by marking the variable as a constant.

File: lib/decent-bridge/src/DecentEthRouter.sol
80: function _getCallParams(
        uint8 msgType,
        address _toAddress,
        uint16 _dstChainId,
        uint64 _dstGasForCall,
        bool deliverEth,
        bytes memory additionalPayload
    )
        private
        view
        returns (
            bytes32 destBridge,
            bytes memory adapterParams,
            bytes memory payload
        )
    {
        uint256 GAS_FOR_RELAY = 100000; //@audit make constant variable
        uint256 gasAmount = GAS_FOR_RELAY + _dstGasForCall;
        adapterParams = abi.encodePacked(PT_SEND_AND_CALL, gasAmount);
        destBridge = bytes32(abi.encode(destinationBridges[_dstChainId]));
        if (msgType == MT_ETH_TRANSFER) {
            payload = abi.encode(msgType, msg.sender, _toAddress, deliverEth);
        } else {
            payload = abi.encode(
                msgType,
                msg.sender,
                _toAddress,
                deliverEth,
                additionalPayload
            );
        }
    }

Optimized code:

File: lib/decent-bridge/src/DecentEthRouter.sol

+       uint256 public constant GAS_FOR_RELAY = 100000; 
80: function _getCallParams(
        uint8 msgType,
        address _toAddress,
        uint16 _dstChainId,
        uint64 _dstGasForCall,
        bool deliverEth,
        bytes memory additionalPayload
    )
        private
        view
        returns (
            bytes32 destBridge,
            bytes memory adapterParams,
            bytes memory payload
        )
    {
-       uint256 GAS_FOR_RELAY = 100000;
        uint256 gasAmount = GAS_FOR_RELAY + _dstGasForCall;
        adapterParams = abi.encodePacked(PT_SEND_AND_CALL, gasAmount);
        destBridge = bytes32(abi.encode(destinationBridges[_dstChainId]));
        if (msgType == MT_ETH_TRANSFER) {
            payload = abi.encode(msgType, msg.sender, _toAddress, deliverEth);
        } else {
            payload = abi.encode(
                msgType,
                msg.sender,
                _toAddress,
                deliverEth,
                additionalPayload
            );
        }
    }

[G-04] storage variable should be cached with stack variable (This instance are not in Bot report )

Impact : Caching the tokenomics in a state variable is more gas-efficient compared to making recursive calls. This approach saves 100 gas and 1 SLOAD

Proof of Concept :

feeCollector storage variable should be cached with stack variable.

Optimized code:

File: src/UTB.sol
228: modifier retrieveAndCollectFees(
        FeeStructure calldata fees,
        bytes memory packedInfo,
        bytes calldata signature
    ) {
+      address _feeCollector = feeCollector;
-        if (address(feeCollector) != address(0)) { //@audit cache state variable
+        if (address(_feeCollector) != address(0))
            uint value = 0;
            if (fees.feeToken != address(0)) {
                IERC20(fees.feeToken).transferFrom(
                    msg.sender,
                    address(this),
                    fees.feeAmount
                );
                IERC20(fees.feeToken).approve(
-                    address(feeCollector),
+                    address(_feeCollector),
                    fees.feeAmount
                );
            } else {
                value = fees.feeAmount;
            }
-            feeCollector.collectFees{value: value}(fees, packedInfo, signature);
+            _feeCollector.collectFees{value: value}(fees, packedInfo, signature);
        }
        _;
    }

Proof of Concept :

bridgeToken storage variable should be cached with stack variable.

Optimized code:

File: src/bridge_adapters/DecentBridgeAdapter.sol
81: function bridge(
        uint256 amt2Bridge,
        SwapInstructions memory postBridge,
        uint256 dstChainId,
        address target,
        address paymentOperator,
        bytes memory payload,
        bytes calldata additionalArgs,
        address payable refund
    ) public payable onlyUtb returns (bytes memory bridgePayload) {
        require(
            destinationBridgeAdapter[dstChainId] != address(0),
            string.concat("dst chain address not set ")
        );

        uint64 dstGas = abi.decode(additionalArgs, (uint64));

        bridgePayload = abi.encodeCall(
            this.receiveFromBridge,
            (postBridge, target, paymentOperator, payload, refund)
        );

        SwapParams memory swapParams = abi.decode(
            postBridge.swapPayload,
            (SwapParams)
        );
+       address _bridgeToken = bridgeToken;
        if (!gasIsEth) {
-            IERC20(bridgeToken).transferFrom( //@audit bridgeToken
+            IERC20(_bridgeToken).transferFrom(
                msg.sender,
                address(this),
                amt2Bridge
            );
-            IERC20(bridgeToken).approve(address(router), amt2Bridge);
+            IERC20(_bridgeToken).approve(address(router), amt2Bridge)
        }

        router.bridgeWithPayload{value: msg.value}( 
            lzIdLookup[dstChainId],
            destinationBridgeAdapter[dstChainId],
            swapParams.amountIn,
            false,
            dstGas,
            bridgePayload
        );
    }

[G-05] No need to cache state variables only used once

Impact : If a state variable is only read but not modified within a function, the compiler can sometimes optimize the read operation. It may choose to read the value only once and use that value throughout the function, so there is no need to cache state variable

Proof of Concept : In getDestAdapter function check zero address against destinationBridgeAdapter[chainID].

File: src/bridge_adapters/StargateBridgeAdapter.sol
154: function getDestAdapter(uint chainId) private view returns (address dstAddr) { 
        dstAddr = destinationBridgeAdapter[chainId]; //@audit no need to cache

        require(
            dstAddr != address(0),
            string.concat("dst chain address not set ")
        );
    }

Optimized Code:

File: src/bridge_adapters/StargateBridgeAdapter.sol
154: function getDestAdapter(uint chainId) private view returns (address dstAddr) { 
-        dstAddr = destinationBridgeAdapter[chainId]; //@audit no need to cache

        require(
-            dstAddr != address(0),
+            destinationBridgeAdapter[chainId] != address(0),
            string.concat("dst chain address not set ")
        );
    }

[G-06] remove function and replace with inline condition or modifier

Impact : Modifiers are generally more efficient in terms of gas consumption because they are designed to modify the behavior of functions without duplicating code. This can lead to more concise and gas-efficient contracts.

Proof of Concept :

Refactor getDestAdapter function and implement inline require a check or make a separate modifier will check for zero address.

File: 
154: function getDestAdapter(uint chainId) private view returns (address dstAddr) { //@audit remove this function instead we can make modifier
        dstAddr = destinationBridgeAdapter[chainId];

        require(
            dstAddr != address(0),
            string.concat("dst chain address not set ")
        );
    }

In this below function getDestAdapter used and check for zero address we can modify below function for better gas efficiency

File: src/bridge_adapters/StargateBridgeAdapter.sol
163: function callBridge(
        uint256 amt2Bridge,
        uint256 dstChainId,
        bytes memory bridgePayload,
        bytes calldata additionalArgs,
        address payable refund
    ) private {
        router.swap{value: msg.value}(
            getDstChainId(additionalArgs), //lzBridgeData._dstChainId, // send to LayerZero chainId
            getSrcPoolId(additionalArgs), //lzBridgeData._srcPoolId, // source pool id
            getDstPoolId(additionalArgs), //lzBridgeData._dstPoolId, // dst pool id
            refund, // refund adddress. extra gas (if any) is returned to this address
            amt2Bridge, // quantity to swap
            (amt2Bridge * (10000 - SG_FEE_BPS)) / 10000, // the min qty you would accept on the destination, fee is 6 bips
            getLzTxObj(additionalArgs), // additional gasLimit increase, airdrop, at address
            abi.encodePacked(getDestAdapter(dstChainId)),
            bridgePayload // bytes param, if you wish to send additional payload you can abi.encode() them here
        );
    }
File: src/bridge_adapters/StargateBridgeAdapter.sol
163: function callBridge( 
        uint256 amt2Bridge,
        uint256 dstChainId,
        bytes memory bridgePayload,
        bytes calldata additionalArgs,
        address payable refund
    ) private {
+          require(
+            destinationBridgeAdapter[dstChainId] != address(0),
+            string.concat("dst chain address not set ")
+        );
        router.swap{value: msg.value}(
            getDstChainId(additionalArgs), //lzBridgeData._dstChainId, // send to LayerZero chainId
            getSrcPoolId(additionalArgs), //lzBridgeData._srcPoolId, // source pool id
            getDstPoolId(additionalArgs), //lzBridgeData._dstPoolId, // dst pool id
            refund, // refund adddress. extra gas (if any) is returned to this address
            amt2Bridge, // quantity to swap
            (amt2Bridge * (10000 - SG_FEE_BPS)) / 10000, // the min qty you would accept on the destination, fee is 6 bips
            getLzTxObj(additionalArgs), // additional gasLimit increase, airdrop, at address
-            abi.encodePacked(getDestAdapter(dstChainId)),
+            abi.encodePacked(dstChainId),
            bridgePayload // bytes param, if you wish to send additional payload you can abi.encode() them here
        );
    }

[G-07] State variables only set in the constructor should be declared immutable (this instance is not cover in bot report)

Impact : Avoids a Gsset(** 20000 gas**) in the constructor, and replaces the first access in each transaction(Gcoldsload - ** 2100 gas **) and each access thereafter(Gwarmacces - ** 100 gas ) with aPUSH32( 3 gas **).

Proof of Code Whilestrings are not value types, and therefore cannot beimmutable / constant if not hard - coded outside of the constructor, the same behavior can be achieved by making the current contract abstract with virtual functions for thestring accessors, and having a child contract override the functions with the hard - coded implementation - specific values.

File: DecentBridgeExecutor.sol#L13
-9:    IWETH weth; //@audit
+9:    IWETH public immutable weth;
    bool public gasCurrencyIsEth; // for chains that use ETH as gas currency

    constructor(address _weth, bool gasIsEth) Owned(msg.sender) {
        weth = IWETH(payable(_weth));
        gasCurrencyIsEth = gasIsEth;
    }

[G-08] Leverage mapping and dot notation for struct assignment

Impact: In dot notation, values are directly written to storage variable, When we use the current method in the code the compiler will allocate some memory to store the struct instance first before writing it to storage Proof of Code:

File: src/swappers/UniSwapper.sol
128: function swapExactIn(
        SwapParams memory swapParams, // SwapParams is a struct
        address receiver
    ) public payable routerIsSet returns (uint256 amountOut) {
        swapParams = _receiveAndWrapIfNeeded(swapParams);

        IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter //@audit use struct notation
            .ExactInputParams({
                path: swapParams.path,
                recipient: address(this),
                amountIn: swapParams.amountIn,
                amountOutMinimum: swapParams.amountOut
            });

        IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);//ok bot
        amountOut = IV3SwapRouter(uniswap_router).exactInput(params); 

        _sendToRecipient(receiver, swapParams.tokenOut, amountOut); 
    }

Optimized code:

128: function swapExactIn(
        SwapParams memory swapParams, // SwapParams is a struct
        address receiver
    ) public payable routerIsSet returns (uint256 amountOut) {
        swapParams = _receiveAndWrapIfNeeded(swapParams);
+     IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter.ExactInputParams
-       IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter //@audit use struct notation
-            .ExactInputParams({
-                path: swapParams.path,
-                recipient: address(this),
-                amountIn: swapParams.amountIn,
-                amountOutMinimum: swapParams.amountOut
-            });
+          params.path: swapParams.path,
+          params.recipient: address(this),
+          params.amountIn: swapParams.amountIn,
+          params.amountOutMinimum: swapParams.amountOut

        IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn);//ok bot
        amountOut = IV3SwapRouter(uniswap_router).exactInput(params); 

        _sendToRecipient(receiver, swapParams.tokenOut, amountOut); 
    }
File: src/swappers/UniSwapper.sol
143:  function swapExactOut( 
       SwapParams memory swapParams,
       address receiver,
       address refundAddress
   ) public payable routerIsSet returns (uint256 amountIn) {
       swapParams = _receiveAndWrapIfNeeded(swapParams);
       IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter //@audit use struct notation
           .ExactOutputParams({
               path: swapParams.path,
               recipient: address(this),
               //deadline: block.timestamp, 
               amountOut: swapParams.amountOut,
               amountInMaximum: swapParams.amountIn
           });

       IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn); 
       amountIn = IV3SwapRouter(uniswap_router).exactOutput(params); 

       // refund sender
       _refundUser(
           refundAddress,
           swapParams.tokenIn,
           params.amountInMaximum - amountIn
       );

       _sendToRecipient(receiver, swapParams.tokenOut, swapParams.amountOut); 
   }

Optimizes Code

File: src/swappers/UniSwapper.sol
143:  function swapExactOut( 
        SwapParams memory swapParams,
        address receiver,
        address refundAddress
    ) public payable routerIsSet returns (uint256 amountIn) {
        swapParams = _receiveAndWrapIfNeeded(swapParams);
+     IV3SwapRouter.ExactInputParams memory params = IV3SwapRouter.ExactInputParams        
-        IV3SwapRouter.ExactOutputParams memory params = IV3SwapRouter //@audit use struct notation
-            .ExactOutputParams({
-                path: swapParams.path,
-                recipient: address(this),
-                //deadline: block.timestamp, 
-                amountOut: swapParams.amountOut,
-                amountInMaximum: swapParams.amountIn
-            });
+                params.path: swapParams.path,
+                params.recipient: address(this),
+                //deadline: block.timestamp, 
+                params.amountOut: swapParams.amountOut,
+                params.amountInMaximum: swapParams.amountIn
        IERC20(swapParams.tokenIn).approve(uniswap_router, swapParams.amountIn); 
        amountIn = IV3SwapRouter(uniswap_router).exactOutput(params); 

        // refund sender
        _refundUser(
            refundAddress,
            swapParams.tokenIn,
            params.amountInMaximum - amountIn
        );

        _sendToRecipient(receiver, swapParams.tokenOut, swapParams.amountOut); 
    }

#0 - raymondfam

2024-01-26T16:21:53Z

G-06: Modifier costs more gas

The rest of the findings are generically known to complement the bot report.

#1 - c4-pre-sort

2024-01-26T16:21:57Z

raymondfam marked the issue as sufficient quality report

#2 - c4-judge

2024-02-04T18:01:09Z

alex-ppg marked the issue as grade-b

#3 - alex-ppg

2024-02-04T18:01:18Z

No penalizations were performed.

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