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:

Copy
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:

Copy
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:

Copy
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

Type	Severity	Location
Gas Optimization		Pool.sol:L267

Description:

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

Example:

Copy
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:

Copy
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:

Copy
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:

Copy
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

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:

Copy
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:

Copy
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:

Copy
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

Type	Severity	Location
Code Style		Pool.sol:L297, L314, L361, L487

Description:

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

Example:

Copy
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:

Copy
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)

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:

Copy
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.