Ondo Finance - SY_S's results

Summary

Gas Optimization

| [G-1] | A modifier used only once and not being inherited should be inlined to save gas | 1 | | [G-2] | abi.encode() is less efficient than abi.encodepacked() | 3 | | [G-3] | Do not calculate constant | 1 | | [G-4] | require() Should Be Used Instead Of assert() | 1 | | [G-5] | Use hardcode address instead address(this) | 3 | | [G-6] | Use assembly for math (add, sub, mul, div) | 3 | | [G-7] | Multiplication/division by two should use bit shifting | 1 | | [G-8] | Refactor event to avoid emitting empty data | 1 | | [G-9] | 2**<N> should be re-written as type(uint<N>).max | 1 | | [G-10] | Shorten the array rather than copying to a new one | 1 | | [G-11] | Avoid contract existence checks by using low level calls | 2 | | [G-12] | Pre-increments and pre-decrements are cheaper than post-increments and post-decrements | 1 | | [G-13] | Using fixed bytes is cheaper than using string | 2 | | [G-14] | Expressions for constant values such as a call to keccak256(), should use immutable rather than constant | 7 | | [G-15] | Not using the named return variable when a function returns, wastes deployment gas | 2 | | [G-16] | Should use arguments instead of state variable | 4 | | [G-17] | Before transfer of some functions, we should check some variables for possible gas save | 2 | | [G-18] | With assembly, .call (bool success) transfer can be done gas-optimized | 2 | | [G-19] | Duplicated require()/if() checks should be refactored to a modifier or function | 1 |

Gas Optimizations

[G-1] A modifier used only once and not being inherited should be inlined to save gas

file: /contracts/usdy/rUSDYFactory.sol

154  modifier onlyGuardian() {
     require(msg.sender == guardian, "rUSDYFactory: You are not the Guardian");
     _;
157  }

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDYFactory.sol#L154-L157

[G-2] abi.encode() is less efficient than abi.encodepacked()

In terms of efficiency, abi.encodePacked() is generally considered to be more gas-efficient than abi.encode(), because it skips the step of adding function signatures and other metadata to the encoded data. However, this comes at the cost of reduced safety, as abi.encodePacked() does not perform any type checking or padding of data.

file: /contracts/bridge/DestinationBridge.sol

99    if (chainToApprovedSender[srcChain] != keccak256(abi.encode(srcAddr))) {

238    chainToApprovedSender[srcChain] = keccak256(abi.encode(srcContractAddress));    

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L99

file: /contracts/bridge/SourceBridge.sol

79    bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L79

[G-3] Do not calculate constant

When you define a constant in Solidity, the compiler can calculate its value at compile-time and replace all references to the constant with its computed value. This can be helpful for readability and reducing the size of the compiled code, but it can also increase gas usage at runtime.

file: /contracts/rwaOracles/RWADynamicOracle.sol

343  uint256 private constant ONE = 10 ** 27;

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L343

[G-4] require() Should Be Used Instead Of assert()

file: /contracts/usdy/rUSDYFactory.sol

100    assert(rUSDYProxyAdmin.owner() == guardian);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDYFactory.sol#L100

[G-5] Use hardcode address instead address(this)

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. References: https://book.getfoundry.sh/reference/forge-std/compute-create-address

file: /contracts/bridge/SourceBridge.sol

97      address(this),

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L97

file: /contracts/bridge/DestinationBridge.sol

323    uint256 balance = IRWALike(_token).balanceOf(address(this));

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L323

file: /contracts/usdy/rUSDY.sol

437    usdy.transferFrom(msg.sender, address(this), _USDYAmount);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDY.sol#L437

[G-6] Use assembly for math (add, sub, mul, div)

Use assembly for math instead of Solidity. You can check for overflow/underflow in assembly to ensure safety. If using Solidity versions < 0.8.0 and you are using Safemath, you can gain significant gas savings by using assembly to calculate values and checking for overflow/underflow.

file: /contracts/rwaOracles/RWADynamicOracle.sol

266    uint256 elapsedDays = (currentTime - currentRange.start) / DAY;

283    uint256 remainder = value % 1e10;

401    z = _mul(x, y) / ONE;

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L266

[G-7] Multiplication/division by two should use bit shifting

<x> * 2 is the same as <x> << 1. While the compiler uses the SHL opcode to accomplish both, the version that uses multiplication incurs an overhead of 20 gas due to JUMPs to and from a compiler utility function that introduces checks which can be avoided by using unchecked {} around the division by two.

file: /contracts/rwaOracles/RWADynamicOracle.sol

405    require(y == 0 || (z = x * y) / y == x);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L405

[G-8] Refactor event to avoid emitting empty data

file: /contracts/usdy/rUSDYFactory.sol

87      ""

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDYFactory.sol#L87

[G-9] 2**<N> should be re-written as type(uint<N>).max

Earlier versions of solidity can use uint<n>(-1) instead. Expressions not including the - 1 can often be re-written to accomodate the change (e.g. by using a > rather than a >=, which will also save some gas)

file: /contracts/rwaOracles/RWADynamicOracle.sol
 
343  uint256 private constant ONE = 10 ** 27;

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L343

##[G-10] Shorten the array rather than copying to a new one

Inline-assembly can be used to shorten the array by changing the length slot, so that the entries don't have to be copied to a new, shorter array

file: /contracts/rwaOracles/RWADynamicOracle.sol

112    Range[] memory rangeList = new Range[](length + 1);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L112

[G-11] Avoid contract existence checks by using low level calls

Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low level calls never check for contract existence.

file: /contracts/bridge/SourceBridge.sol

125    destChainToContractAddr[destinationChain] = AddressToString.toString(
126      contractAddress
127    );

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L125-L127

file: /contracts/bridge/DestinationBridge.sol

323    uint256 balance = IRWALike(_token).balanceOf(address(this));

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L323

[G-12] Pre-increments and pre-decrements are cheaper than post-increments and post-decrements

Saves 5 gas per iteration

file: /contracts/bridge/SourceBridge.sol

79    bytes memory payload = abi.encode(VERSION, msg.sender, amount, nonce++);     //FOUND  nonce++

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L79

[G-13] Using fixed bytes is cheaper than using string

As a rule of thumb, use bytes for arbitrary-length raw byte data and string for arbitrary-length string (UTF-8) data. If you can limit the length to a certain number of bytes, always use one of bytes1 to bytes32 because they are much cheaper.

file: /contracts/usdy/rUSDY.sol

202  function symbol() public pure returns (string memory) {
203    return "rUSDY";

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDY.sol#L202-L203

file: /contracts/bridge/DestinationBridge.sol

236    string calldata srcContractAddress

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L236

[G-14] Expressions for constant values such as a call to keccak256(), should use immutable rather than constant

While it doesn't save any gas because the compiler knows that developers often make this mistake, it's still best to use theright tool for the task at hand. There is a difference between constant variables and immutable variables, and they shouldeach be used in their appropriate contexts. constants should be used for literal values written into the code, and immutablevariables should be used for expressions, or values calculated in, or passed into the constructor.

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");

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDY.sol#L97-L102

file: /contracts/rwaOracles/RWADynamicOracle.sol

27  bytes32 public constant SETTER_ROLE = keccak256("SETTER_ROLE");
28  bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE");

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L27-L28

[G-15] Not using the named return variable when a function returns, wastes deployment gas

When you execute a function that returns values in Solidity, the EVM still performs the necessary operations to execute and return those values. This includes the cost of allocating memory and packing the return values. If the returned values are not utilized, it can be seen as wasteful since you are incurring gas costs for operations that have no effect.

file: /contracts/usdy/rUSDYFactory.sol

/// @audit the ' address ' data type  on line 81

108    return (
      address(rUSDYProxy),
      address(rUSDYProxyAdmin),
      address(rUSDYImplementation)
112    );

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDYFactory.sol#L108-L112

file: /contracts/rwaOracles/RWADynamicOracle.sol

61    returns (uint256 price, uint256 timestamp)
  {
    price = getPrice();
    timestamp = block.timestamp;
  }
 

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L61-L65

[G-16] Should use arguments instead of state variable

state variables should not used in emit , This will save near 97 gas

file: /contracts/usdy/rUSDYFactory.sol

///@audit the ' (rUSDYProxy),(rUSDYProxyAdmin),rUSDYImplementation), ' are state variables

101    emit rUSDYDeployed(
      address(rUSDYProxy),
      address(rUSDYProxyAdmin),
      address(rUSDYImplementation),
      "Ondo Rebasing U.S. Dollar Yield",
      "rUSDY"
107    );

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDYFactory.sol#L101-L107

[G-17] Before transfer of some functions, we should check some variables for possible gas save

Before transfer, we should check for amount being 0 so the function doesn't run when its not gonna do anything

file: /contracts/bridge/DestinationBridge.sol

324    IRWALike(_token).transfer(owner(), balance);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/DestinationBridge.sol#L324

file: /contracts/usdy/rUSDY.sol

680    usdy.transfer(msg.sender, sharesAmount / BPS_DENOMINATOR);

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDY.sol#L680

[G-18] With assembly, .call (bool success) transfer can be done gas-optimized

return data (bool success,) has to be stored due to EVM architecture, but in a usage like below, ‘out’ and ‘outsize’ values are given (0,0), this storage disappears and gas optimization is provided.

file: /contracts/bridge/SourceBridge.sol

165      (bool success, bytes memory ret) = address(exCallData[i].target).call{

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/bridge/SourceBridge.sol#L165

file: /contracts/usdy/rUSDYFactory.sol

131      (bool success, bytes memory ret) = address(exCallData[i].target).call{

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDYFactory.sol#L131

[G-19] Duplicated require()/if() checks should be refactored to a modifier or function

to reduce code duplication and improve readability. •  Modifiers can be used to perform additional checks on the function inputs or state before it is executed. By defining a modifier to perform a specific check, we can reuse it across multiple functions that require the same check. • A function can also be used to perform a specific check and return a boolean value indicating whether the check has passed or failed. This can be useful when the check is more complex and cannot be performed easily in a modifier.

file: /contracts/rwaOracles/RWADynamicOracle.sol

/// @audit duplicated if is on line 218

198    if (indexToModify == 0) {

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/rwaOracles/RWADynamicOracle.sol#L198