Omniscia Native Audit
Registry Code Style Findings
Registry Code Style Findings
RYR-01C: Loop Iterator Optimization
Type | Severity | Location |
---|---|---|
Gas Optimization | Registry.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:
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
Type | Severity | Location |
---|---|---|
Code Style | Registry.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:
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
Type | Severity | Location |
---|---|---|
Gas Optimization | Registry.sol:L32 |
Description:
The registerPricer
function will always yield true
when executed rendering its return variable ineffectual.
Example:
28// public methods29function 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
Type | Severity | Location |
---|---|---|
Code Style | Registry.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:
64function _getAmountOut(65 uint256 amountIn,66 uint256 reserveIn,67 uint256 reserveOut,68 uint256 fee,69 uint256 id70) 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 fee79 );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 id91) 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 fee100 );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.