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
Rank: 49/113
Findings: 1
Award: $38.84
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: c3phas
Also found by: 0x11singh99, Raihan, dharma09, hunter_w3b, slvDev
38.8402 USDC - $38.84
execute()
function for better gas savingexecute()
function for better gas savingImpact :
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); } }
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; }
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 ); } }
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 ); }
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 ") ); }
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 ); }
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; }
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.