Omniscia Native Audit

Registry Code Style Findings

Registry Code Style Findings

RYR-01C: Loop Iterator Optimization

TypeSeverityLocation
Gas OptimizationRegistry.sol:L23

Description:

The linked for loop increments / decrements the iterator "safely" due to Solidity's built-in safe arithmetics(post - 0.8.X).

Example:

contracts/Registry.sol
23for (uint256 i = 0; i < pricers.length; i++) {

Recommendation:

We advise the increment / decrement operation to be performed in an unchecked code block as the last statement within the for loop to optimize its execution cost.

Alleviation:

The loop iterator is now optimally incremented in an unchecked code block as advised.

RYR-02C: Non-Standard Definition of Function Signatures

TypeSeverityLocation
Code StyleRegistry.sol:L13-L16

Description:

The referenced GETAMOUNTOUT_SELECTOR and GETAMOUNTIN_SELECTOR variables are computing the relevant function signatures using a string literal processed via a keccak256 operation and cast to a bytes4 variable.

Example:

contracts/Registry.sol
13bytes4 private constant GETAMOUNTOUT_SELECTOR =
14 bytes4(keccak256("getAmountOut(uint256,uint256,uint256,uint256)"));
15bytes4 private constant GETAMOUNTIN_SELECTOR =
16 bytes4(keccak256("getAmountIn(uint256,uint256,uint256,uint256)"));

Recommendation:

We advise the function selector to be utilized directly by extracting it from the IPricer interface. This can be achieved by importing the relevant file to the codebase and f.e. specifying IPricer.getAmountOut.selector for the GETAMOUNTOUT_SELECTOR assignment. In turn, this can also render the constants redundant as their utilization can be replaced by the selector methodology outlined in this exhibit.

Alleviation:

The IPricer selectors are now properly utilized in place of the constant variables that were previously present in the codebase, optimizing the code's legibility as well as gas cost significantly.

RYR-03C: Redundant Return Statement

TypeSeverityLocation
Gas OptimizationRegistry.sol:L32

Description:

The registerPricer function will always yield true when executed rendering its return variable ineffectual.

Example:

contracts/Registry.sol
28// public methods
29function registerPricer(uint256 id, address addr) public onlyOwner returns (bool) {
30 require(pricer[id] == address(0), "pricer already set for this id");
31 pricer[id] = addr;
32 return true;
33}

Recommendation:

We advise it to be omitted optimizing the gas cost of invoking the function.

Alleviation:

The redundant return statement has been safely omitted from the codebase.

RYR-04C: Redundantly Convoluted Calls

TypeSeverityLocation
Code StyleRegistry.sol:L73-L82, L94-L103

Description:

The referenced statements perform a low-level staticcall instruction on a known implementation instead of casting it to an interface and invoking the relevant function.

Example:

contracts/Registry.sol
64function _getAmountOut(
65 uint256 amountIn,
66 uint256 reserveIn,
67 uint256 reserveOut,
68 uint256 fee,
69 uint256 id
70) internal view returns (uint amountOut) {
71 require(reserveIn > 0 && reserveOut > 0, "Registry: INSUFFICIENT_LIQUIDITY");
72
73 bytes memory data = abi.encodeWithSelector(
74 GETAMOUNTOUT_SELECTOR,
75 amountIn,
76 reserveIn,
77 reserveOut,
78 fee
79 );
80 (bool success, bytes memory returnData) = address(pricer[id]).staticcall(data);
81 require(success, "Failed to get amount out");
82 amountOut = abi.decode(returnData, (uint256));
83}
84
85function _getAmountIn(
86 uint256 amountOut,
87 uint256 reserveIn,
88 uint256 reserveOut,
89 uint256 fee,
90 uint256 id
91) internal view returns (uint amountIn) {
92 require(reserveIn > 0 && reserveOut > 0, "Insufficient liquidity");
93
94 bytes memory data = abi.encodeWithSelector(
95 GETAMOUNTIN_SELECTOR,
96 amountOut,
97 reserveIn,
98 reserveOut,
99 fee
100 );
101 (bool success, bytes memory returnData) = address(pricer[id]).staticcall(data);
102 require(success, "Failed to get amount in");
103 amountIn = abi.decode(returnData, (uint256));
104}

Recommendation:

We advise the pricer[id] values to be cast to an IPricer interface, permitting the getAmountOut and getAmountIn functions to be invoked directly and thus optimizing the legibility of the codebase.

Alleviation:

The Native team stated that they wish to use staticcall to prevent changes to the blockchain state. We would like to note that using an interface which declares a view or pure function will employ a staticcall under the hood, achieving the same result with significantly increased legibility. As such, the finding remains unaddressed.