Platform: Code4rena
Start Date: 01/09/2023
Pot Size: $36,500 USDC
Total HM: 4
Participants: 70
Period: 6 days
Judge: kirk-baird
Id: 281
League: ETH
Rank: 55/70
Findings: 1
Award: $9.75
๐ Selected for report: 0
๐ Solo Findings: 0
๐ Selected for report: c3phas
Also found by: 0x11singh99, 0xhex, 0xta, Eurovickk, K42, MohammedRizwan, SAAJ, SAQ, SY_S, adriro, albahaca, castle_chain, jeffy, kaveyjoe, matrix_0wl, naman1778, petrichor, wahedtalash77, ybansal2403, zabihullahazadzoi
9.7506 USDC - $9.75
Number | Issue | Instances |
---|---|---|
[G-01] | Structs can be packed into fewer storage slots. | 1 |
[G-02] | abi.encode() is less efficient than abi.encodePacked() to save gas. | 3 |
[G-03] | Use calldata instead of memory for function arguments that do not get mutated. | 3 |
[G-04] | No need to explicitly initialize variables with default values. | 8 |
[G-05] | Using ternary operator instead of if-else saves gas | 1 |
[G-06] | Use hardcode address instead address(this) | 2 |
[G-07] | Change Constant to Immutable for keccak Variables (20 gas per keccak) | 7 |
Total 7 issues.
The EVM works with 32 byte words. Variables less than 32 bytes can be declared next to each other in storage and this will pack the values together into a single 32 byte storage slot (if values combined are <= 32 bytes). If the variables packed together are retrieved together in functions (more likely with structs), we will effectively save ~2000 gas with every subsequent SLOAD for that storage slot. This is due to us incurring a Gwarmaccess (100 gas) versus a Gcoldsload (2100 gas).
1 Instance - 1 File
start
,end
and dailyInterestRate
and pack them into 1 storage slot to save 2 SLOT (~4000 Gas)These start
,end
can be reduced to uint64
because they are holding timestamps and uint64
is more than enough to hold timestamps.
dailyInterestRate
are holding % value with 1e27 precision. And max value a uint128
can hold is ~ 3.40 ร 10^38 . So uint128
is big enough to hold dailyInterestRate
.
File : contracts/rwaOracles/RWADynamicOracle.sol 295: struct Range { 296: uint256 start; 297: uint256 end; 298: uint256 dailyInterestRate; 299: uint256 prevRangeClosePrice; 300: }
File : contracts/rwaOracles/RWADynamicOracle.sol struct Range { - uint256 start; - uint256 end; - uint256 dailyInterestRate; + uint64 start; + uint64 end; + uint128 dailyInterestRate; uint256 prevRangeClosePrice; }
In Solidity, abi.encode() and abi.encodePacked() are used to encode function arguments and data into a byte array for storage or transmission. However, abi.encodePacked() is generally more gas-efficient than abi.encode() because it does not add padding to the encoded data. When using abi.encode(), the function arguments are encoded with padding to ensure that the encoded data is a multiple of 32 bytes. This padding can add unnecessary zeros to the encoded data, which can increase the size and gas cost of the transaction. On the other hand, abi.encodePacked() does not add padding to the encoded data and returns a tightly packed representation of the input data. This can reduce the size and gas cost of the transaction, as it avoids unnecessary padding.
3 Instances - 2 Files
File : contracts/bridge/SourceBridge.sol function burnAndCallAxelar ... - 79: bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++); + 79: bytes memory payload = abi.encodePacked(VERSION, msg.sender, amount, nonce++);
File : contracts/bridge/DestinationBridge.sol function _execute ... - 99: if (chainToApprovedSender[srcChain] != keccak256(abi.encode(srcAddr))) { + 99: if (chainToApprovedSender[srcChain] != keccak256(abi.encodePacked(srcAddr))) { function addChainSupport ... - 238: chainToApprovedSender[srcChain] = keccak256(abi.encode(srcContractAddress)); + 238: chainToApprovedSender[srcChain] = keccak256(abi.encodePacked(srcContractAddress));
When you specify a data location as memory, that value will be copied into memory. When you specify the location as calldata, the value will stay static within calldata. If the value is a large, complex type, using memory may result in extra memory expansion costs.
Note: These instances missed by bot report
3 Instances - 2 Files
File : contracts/bridge/SourceBridge.sol 91: function _payGasAndCallContract( 92: string calldata destinationChain, 93: string memory destContract, 94: bytes memory payload 95: ) private {
File : contracts/bridge/SourceBridge.sol function _payGasAndCallContract( string calldata destinationChain, - string memory destContract, - bytes memory payload + string calldata destContract, + bytes calldata payload ) private {
File : contracts/bridge/DestinationBridge.sol 128: function _attachThreshold( 129: uint256 amount, 130: bytes32 txnHash, 131: string memory srcChain 132: ) internal {
File : contracts/bridge/DestinationBridge.sol function _attachThreshold( uint256 amount, bytes32 txnHash, - string memory srcChain + string calldata srcChain ) internal {
If a variable is not set/initialized, it is assumed to have the default value (0 for uint, false for bool, address(0) for address, etc.). Explicitly initializing it with its default value is an anti-pattern and wastes gas.
8 Instances - 4 Files
for (uint256 i = 0; i < numIterations; ++i)
should be replaced with: for (uint256 i; i < numIterations; ++i)
File : contracts/bridge/SourceBridge.sol 164: for (uint256 i = 0; i < exCallData.length; ++i) {
Fle : contracts/bridge/DestinationBridge.sol 134: for (uint256 i = 0; i < thresholds.length; ++i) { 160: for (uint256 i = 0; i < t.approvers.length; ++i) { 264: for (uint256 i = 0; i < amounts.length; ++i) {
File : contracts/rwaOracles/RWADynamicOracle.sol 77: for (uint256 i = 0; i < length; ++i) { 113: for (uint256 i = 0; i < length; ++i) { 129: for (uint256 i = 0; i < length + 1; ++i) {
File : contracts/usdy/rUSDYFactory.sol 130: for (uint256 i = 0; i < exCallData.length; ++i) {
When using the if-else construct, Solidity generates bytecode that includes a JUMP operation. The JUMP operation requires the EVM to change the program counter to a different location in the bytecode, resulting in additional gas costs. On the other hand, the ternary operator doesn't involve a JUMP operation, as it generates bytecode that utilizes conditional stack manipulation. The exact amount of gas saved by using the ternary operator instead of the if-else construct in Solidity can vary depending on the specific scenario, the complexity of the surrounding code, and the conditions involved
1 Instance - 1 File
File : contracts/bridge/DestinationBridge.sol 177: function _checkThresholdMet(bytes32 txnHash) internal view returns (bool) { 178: TxnThreshold memory t = txnToThresholdSet[txnHash]; 179: if (t.numberOfApprovalsNeeded <= t.approvers.length) { 180: return true; 181: } else { 182: return false; 183: } 184: }
File : contracts/bridge/DestinationBridge.sol function _checkThresholdMet(bytes32 txnHash) internal view returns (bool) { TxnThreshold memory t = txnToThresholdSet[txnHash]; - if (t.numberOfApprovalsNeeded <= t.approvers.length) { - return true; - } else { - return false; - } + return t.numberOfApprovalsNeeded <= t.approvers.length ? true : false; }
Instead of using address(this), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundryโs script.sol and solmateโs LibRlp.sol contracts can help achieve this. In Solidity, the address(this) expression returns the address of the current contract instance. This expression is commonly used to send or receive Ether to or from the contract. Using a hardcoded address instead of the address(this) expression can be more gas-efficient when sending or receiving Ether to or from the contract. This is because using the address(this) expression requires additional gas to be consumed to retrieve the address of the current contract, while using a hardcoded address does not.
2 Instances - 2 Files
File : contracts/bridge/DestinationBridge.sol 322: function rescueTokens(address _token) external onlyOwner { 323: uint256 balance = IRWALike(_token).balanceOf(address(this)); 324: IRWALike(_token).transfer(owner(), balance); 325: }
File : contracts/usdy/rUSDY.sol 434: function wrap(uint256 _USDYAmount) external whenNotPaused { 435: require(_USDYAmount > 0, "rUSDY: can't wrap zero USDY tokens"); 436: _mintShares(msg.sender, _USDYAmount * BPS_DENOMINATOR); 437: usdy.transferFrom(msg.sender, address(this), _USDYAmount);
The use of constant keccak variables results in extra hashing (and so gas). This results in the keccak operation being performed whenever the variable is used, increasing gas costs relative to just storing the output hash. Changing to immutable will only perform hashing on contract deployment which will save gas. You should use immutables until the referenced issues are implemented, then you only pay the gas costs for the computation at deploy time and you can save about 20 gas per one keccak
.
7 Instances - 2 Files
File : contracts/rwaOracles/RWADynamicOracle.sol - 27: bytes32 public constant SETTER_ROLE = keccak256("SETTER_ROLE"); - 28: bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); + 27: bytes32 public immutable SETTER_ROLE = keccak256("SETTER_ROLE"); + 28: bytes32 public immutable PAUSER_ROLE = keccak256("PAUSER_ROLE");
File : contracts/usdy/rUSDY.sol - 97: bytes32 public constant USDY_MANAGER_ROLE = keccak256("ADMIN_ROLE"); - 98: bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); - 99: bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); - 100: bytes32 public constant BURNER_ROLE = keccak256("BURN_ROLE"); - 101: bytes32 public constant LIST_CONFIGURER_ROLE = 102: keccak256("LIST_CONFIGURER_ROLE"); + 97: bytes32 public immutable USDY_MANAGER_ROLE = keccak256("ADMIN_ROLE"); + 98: bytes32 public immutable MINTER_ROLE = keccak256("MINTER_ROLE"); + 99: bytes32 public immutable PAUSER_ROLE = keccak256("PAUSER_ROLE"); + 100: bytes32 public immutable BURNER_ROLE = keccak256("BURN_ROLE"); + 101: bytes32 public immutable LIST_CONFIGURER_ROLE = 102: keccak256("LIST_CONFIGURER_ROLE");
#0 - c4-pre-sort
2023-09-08T14:31:54Z
raymondfam marked the issue as sufficient quality report
#1 - c4-judge
2023-09-24T06:55:23Z
kirk-baird marked the issue as grade-b