Omniscia Native Audit
Pool Code Style Findings
Pool Code Style Findings
PLO-01C: Deprecated Conditional Revert Pattern
Type | Severity | Location |
---|---|---|
Code Style | Pool.sol:L127-L129, L135-L137 |
Description:
The referenced statements implement an if-revert
pattern that is ill-advised.
Example:
126function addSigner(address _signer) external override onlyOwner whenNotPaused {127 if (isSigner[_signer]) {128 revert("Signer is already added");129 }130 isSigner[_signer] = true;131 emit AddSigner(_signer);132}
Recommendation:
We advise either specialized error
declarations to be made on the contract that the revert
statements yield or them to be replaced by require
checks either of which we consider an adequate remediation to this exhibit.
Alleviation:
Both if-revert
patterns were replaced by require
checks as advised.
PLO-02C: Ineffectual Conditional Check
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L445 |
Description:
The referenced require
check is ineffectual as the ECDSA::recover
function will never yield an address(0)
signer due to its implementation in OpenZeppelin.
Example:
444address recoveredSigner = ECDSA.recover(digest, signature);445require(recoveredSigner != address(0), "Signature is not valid");446return _order.signer == recoveredSigner;
Recommendation:
We advise it to be safely omitted from the codebase, optimizing its gas cost.
Alleviation:
The referenced require
check has been safely omitted from the codebase as advised.
PLO-03C: Inefficient Hash of Literal
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L419-L421 |
Description:
The referenced keccak256
hash of a string
literal is computed each time the getMessageHash
function is invoked.
Example:
416function getMessageHash(Orders.Order memory _order) internal pure returns (bytes32) {417 bytes32 hash = keccak256(418 abi.encode(419 keccak256(420 "Order(uint256 id,address signer,address buyer,address seller,address buyerToken,address sellerToken,uint256 buyerTokenAmount,uint256 sellerTokenAmount,uint256 deadlineTimestamp,address txOrigin)"421 ),422 _order.id,423 _order.signer,424 _order.buyer,425 _order.seller,426 _order.buyerToken,427 _order.sellerToken,428 _order.buyerTokenAmount,429 _order.sellerTokenAmount,430 _order.deadlineTimestamp,431 _order.txOrigin432 )433 );434 return hash;435}
Recommendation:
We advise it to be relocated to a contract-level constant
that is consequently utilized in the getMessageHash
function, optimizing the code's gas cost and legibility.
Alleviation:
The referenced keccak256
hash has been relocated to a contract-level constant
variable labelled ORDER_SIGNATURE_HASH
, optimizing the code's gas cost.
PLO-04C: Inefficient Loop Limit Evaluation
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L267 |
Description:
The linked for
loop evaluates its limit inefficiently on each iteration.
Example:
267for (uint i = 0; i < tokenAs.length; i++) {
Recommendation:
We advise the statements within the for
loop limit to be relocated outside to a local variable declaration that is consequently utilized for the evaluation to significantly reduce the codebase's gas cost. We should note the same optimization is applicable for storage reads present in such limits as they are newly read on each iteration (i.e. length
members of arrays in storage).
Alleviation:
The limit of the loop is now efficiently cached to a local tokenAsLength
variable, optimizing the loop's gas cost.
PLO-05C: Inefficient Memory Pointer
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L206 |
Description:
The referenced struct
variable is declared as memory
yet only one of its members is utilized in the function.
Example:
206Pair memory pair = pairs[token0][token1];
Recommendation:
We advise it to be set as storage
optimizing its gas cost.
Alleviation:
The inefficient memory
pointer has been removed, directly yielding the value of pairs[token0][token1].isExist
instead in an optimal way.
PLO-06C: Inexistent Error Message
Type | Severity | Location |
---|---|---|
Code Style | Pool.sol:L60 |
Description:
The linked require
check has no error message explicitly defined.
Example:
60require(msg.sender == router);
Recommendation:
We advise one to be set so to increase the legibility of the codebase and aid in validating the require
check's condition.
Alleviation:
An error message was properly introduced for the referenced require
statement.
PLO-07C: Inexplicable Logic Split
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L486-L538 |
Description:
The code of executeSwapToTreasury
is branched into two different logical blocks via an isExactIn
evaluation, however, the blocks are functionally identical.
Example:
472function executeSwapToTreasury(473 bool isExactIn,474 Orders.Order memory _order,475 uint256 sellerTokenAmount,476 uint256 buyerTokenAmount,477 bytes memory callback,478 uint256 pricingModelId479) internal returns (int256, int256) {480 int256 outputSellerTokenAmount = int256(sellerTokenAmount);481 int256 outputBuyerTokenAmount = -1 * int256(buyerTokenAmount);482 address sellerToken = _order.sellerToken;483 uint256 treasuryBalanceInitial = IERC20(sellerToken).balanceOf(address(treasury));484 uint256 treasuryBalanceFinal;485
486 if (isExactIn) {487 if (pricingModelId == 99) {488 require(489 outputBuyerTokenAmount * -1 <= outputSellerTokenAmount,490 "Amount exceeded order size"491 );492 }493
494 ISwapRouter(msg.sender).swapCallback(495 outputBuyerTokenAmount,496 outputSellerTokenAmount,497 callback498 );499 // console.log(500 // "Pool 2nd transfers %s from %s to %s",501 // sellerToken,502 // address(this),503 // treasury504 // );505 // console.log(sellerTokenAmount);506 //IERC20(sellerToken).safeTransfer(treasury, sellerTokenAmount);507 TransferHelper.safeTransfer(sellerToken, treasury, sellerTokenAmount);508
509 treasuryBalanceFinal = IERC20(sellerToken).balanceOf(address(treasury));510 require(511 (treasuryBalanceFinal - treasuryBalanceInitial) == sellerTokenAmount,512 "Swap amount not match"513 );514 return (outputSellerTokenAmount, outputBuyerTokenAmount);515 } else {516 uint256 treasuryBalanceInitial2 = IERC20(_order.sellerToken).balanceOf(517 address(treasury)518 );519 ISwapRouter(msg.sender).swapCallback(520 outputSellerTokenAmount,521 outputBuyerTokenAmount,522 callback523 );524 //console.log(IERC20(_order.sellerToken).balanceOf(address(this)));525 //IERC20(_order.sellerToken).safeTransfer(treasury, sellerTokenAmount);526 TransferHelper.safeTransfer(_order.sellerToken, treasury, sellerTokenAmount);527
528 treasuryBalanceFinal = IERC20(_order.sellerToken).balanceOf(address(treasury));529 //console.log(treasuryBalanceFinal);530 //console.log(treasuryBalanceInitial_2);531 //console.log(_order.sellerToken);532 //console.log(IERC20(_order.sellerToken).balanceOf(address(this)));533 require(534 (treasuryBalanceFinal - treasuryBalanceInitial2) == sellerTokenAmount,535 "Swap amount not match"536 );537 return (outputBuyerTokenAmount, outputSellerTokenAmount);538 }539}
Recommendation:
We advise the logic of executeSwapToTreasury
to be strictly defined by simplifying any statements that are meant to be executed in either case of the isExactIn
conditional.
Alleviation:
The Native team evaluated this exhibit and confirmed its optimization, however, they stated that they will remediate it in a next version of the codebase thus acknowledging it.
PLO-08C: Loop Iterator Optimizations
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L237, L267 |
Description:
The linked for
loops increment / decrement their iterator "safely" due to Solidity's built - in safe arithmetics(post - 0.8.X
).
Example:
237for (uint i = 0; i < _fees.length; i++) {
Recommendation:
We advise the increment / decrement operations to be performed in an unchecked
code block as the last statement within each for
loop to optimize their execution cost.
Alleviation:
Both referenced loop iterators are now optimally incremented in unchecked
code blocks as advised.
PLO-09C: Loose Definition of EIP712 Arguments
Type | Severity | Location |
---|---|---|
Code Style | Pool.sol:L74 |
Description:
The referenced invocation of the EIP712
constructor contains lower-case arguments that are not distinctive of the protocol.
Example:
74) EIP712("native pool", "1") {
Recommendation:
We advise the arguments to be enhanced in what they represent as per the EIP-712 standard.
Alleviation:
The Native team stated that the values present are correct and that these are the correct values they wish to use. We would like to note that as the values are not pool specific (i.e. containing the token symbols of the pool), EIP-712 domains will only differ by the address
which reduces their security in case the project moves to an upgradeable pattern in the future.
PLO-10C: Redundant Input Arguments
Type | Severity | Location |
---|---|---|
Code Style | Pool.sol:L332-L334 |
Description:
The referenced input arguments of the calculateTokenAmount
function remain unutilized and are only assigned to.
Example:
329// private methods330function calculateTokenAmount(331 int256 flexibleAmount,332 bool isExactIn,333 uint256 buyerTokenAmount,334 uint256 sellerTokenAmount,335 Orders.Order memory _order,336 uint256 pricingModelId337) private view returns (uint256, uint256, bool) {
Recommendation:
We advise them to be safely omitted and / or relocated to the returns
declaration of the function, optimizing the legibility of the codebase.
Alleviation:
The referenced arguments have been properly relocated to function-level local variables, optimizing the function's gas cost.
PLO-11C: Repetitive Value Literal
Type | Severity | Location |
---|---|---|
Code Style | Pool.sol:L297, L314, L361, L487 |
Description:
The linked value literal is repeated across the codebase multiple times.
Example:
297require(pricingModelId != 99, "Off-chain pricing unsupported");
Recommendation:
We advise it to be set to a constant
variable instead optimizing the legibility of the codebase.
Alleviation:
The repetitive value literal has been relocated to a contract-level constant
declaration labelled FIXED_PRICE_MODEL_ID
, alleviating this exhibit.
PLO-12C: Suboptimal Struct Declaration Styles
Type | Severity | Location |
---|---|---|
Code Style | Pool.sol:L184-L192, L249 |
Description:
The linked declaration styles of the referenced structs are using index-based argument initialization.
Example:
184SwapParam(185 buyerTokenAmount,186 sellerTokenAmount,187 isExactIn,188 _order,189 recipient,190 callback,191 pricingModelId192)
Recommendation:
We advise the key-value declaration format to be utilized instead in each instance, greatly increasing the legibility of the codebase.
Alleviation:
The key-value declaration format is now in use by both struct
creation statements as advised.
PLO-13C: Variable Mutability Specifiers (Immutable)
Type | Severity | Location |
---|---|---|
Gas Optimization | Pool.sol:L75-L76, L78 |
Description:
The linked variables are assigned to only once during the contract's constructor
.
Example:
65constructor(66 address _treasury,67 address _treasuryOwner,68 address _signer,69 address _pricingModelRegistry,70 uint256[] memory _fees,71 address[] memory _tokenAs,72 address[] memory _tokenBs, 73 uint256[] memory _pricingModelIds74) EIP712("native pool", "1") {75 treasury = _treasury;76 treasuryOwner = _treasuryOwner;77 isSigner[_signer] = true;78 pricingModelRegistry = _pricingModelRegistry;79 isPairAdded = false;80 pairCount = 0;81 executeUpdatePairs(_fees, _tokenAs, _tokenBs, _pricingModelIds);82}
Recommendation:
We advise them to be set as immutable
greatly optimizing their read-access gas cost.
Alleviation:
All referenced variables have been adequately set as immutable
, optimizing their read-access gas cost significantly.