Omniscia Native Audit

Pool Code Style Findings

Pool Code Style Findings

PLO-01C: Deprecated Conditional Revert Pattern

TypeSeverityLocation
Code StylePool.sol:L127-L129, L135-L137

Description:

The referenced statements implement an if-revert pattern that is ill-advised.

Example:

contracts/Pool.sol
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

TypeSeverityLocation
Gas OptimizationPool.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:

contracts/Pool.sol
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

TypeSeverityLocation
Gas OptimizationPool.sol:L419-L421

Description:

The referenced keccak256 hash of a string literal is computed each time the getMessageHash function is invoked.

Example:

contracts/Pool.sol
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.txOrigin
432 )
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

TypeSeverityLocation
Gas OptimizationPool.sol:L267

Description:

The linked for loop evaluates its limit inefficiently on each iteration.

Example:

contracts/Pool.sol
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

TypeSeverityLocation
Gas OptimizationPool.sol:L206

Description:

The referenced struct variable is declared as memory yet only one of its members is utilized in the function.

Example:

contracts/Pool.sol
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

TypeSeverityLocation
Code StylePool.sol:L60

Description:

The linked require check has no error message explicitly defined.

Example:

contracts/Pool.sol
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

TypeSeverityLocation
Gas OptimizationPool.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:

contracts/Pool.sol
472function executeSwapToTreasury(
473 bool isExactIn,
474 Orders.Order memory _order,
475 uint256 sellerTokenAmount,
476 uint256 buyerTokenAmount,
477 bytes memory callback,
478 uint256 pricingModelId
479) 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 callback
498 );
499 // console.log(
500 // "Pool 2nd transfers %s from %s to %s",
501 // sellerToken,
502 // address(this),
503 // treasury
504 // );
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 callback
523 );
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

TypeSeverityLocation
Gas OptimizationPool.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:

contracts/Pool.sol
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

TypeSeverityLocation
Code StylePool.sol:L74

Description:

The referenced invocation of the EIP712 constructor contains lower-case arguments that are not distinctive of the protocol.

Example:

contracts/Pool.sol
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

TypeSeverityLocation
Code StylePool.sol:L332-L334

Description:

The referenced input arguments of the calculateTokenAmount function remain unutilized and are only assigned to.

Example:

contracts/Pool.sol
329// private methods
330function calculateTokenAmount(
331 int256 flexibleAmount,
332 bool isExactIn,
333 uint256 buyerTokenAmount,
334 uint256 sellerTokenAmount,
335 Orders.Order memory _order,
336 uint256 pricingModelId
337) 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

TypeSeverityLocation
Code StylePool.sol:L297, L314, L361, L487

Description:

The linked value literal is repeated across the codebase multiple times.

Example:

contracts/Pool.sol
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

TypeSeverityLocation
Code StylePool.sol:L184-L192, L249

Description:

The linked declaration styles of the referenced structs are using index-based argument initialization.

Example:

contracts/Pool.sol
184SwapParam(
185 buyerTokenAmount,
186 sellerTokenAmount,
187 isExactIn,
188 _order,
189 recipient,
190 callback,
191 pricingModelId
192)

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)

TypeSeverityLocation
Gas OptimizationPool.sol:L75-L76, L78

Description:

The linked variables are assigned to only once during the contract's constructor.

Example:

contracts/Pool.sol
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 _pricingModelIds
74) 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.