Lista DAO

Lista V3

Cantina Security Report

Organization

@lista-dao

Engagement Type

Cantina Solo

Period

-

Researchers


Findings

Informational

4 findings

4 fixed

0 acknowledged


Informational4 findings

  1. ListaV3Factory differs from the original pattern used by Uniswap to prevent code reuse

    Severity

    Severity: Informational

    Submitted by

    Chinmay Farkya


    Description

    The UniswapV3Factory uses a noDelegateCall modifier on its createPool function.

    The reasoning given by the Uniswap team for using noDelegateCall in the original Factory contract is to prevent circumventing the license, even GPL license. As pointed out here. If this was not present, anyone could deploy a proxy and use the deployed factory contract as an implementation, thus allowing code reuse.

    Lista has modified the factory to make it an upgradeable contract, which means ListaV3Factory is now an implementation contract behind a proxy and noDelegateCall had to be removed to make it work.

    Now other proxies that want to reuse this code can easily point to this deployed ListaV3Factory implementation contract.

    Lista will use the Factory code under a GPL License, but a third-party proxy can still use it as it wants without meeting the GPL requirements, if they want to.

    This could be incompatible with GPL license compliance : The legal status of delegatecall-into-licensed-bytecode is itself unsettled; NoDelegateCall was a technical deterrent, not a settled legal interpretation.

    Recommendation

    Consider documenting this potential incompatibility with Uniswap's GPL interpretation, or avoid making the factory upgradeable to prevent legal issues in the future.

  2. Storage gap in ERC721PermitUpgradeable can be removed

    Severity

    Severity: Informational

    Submitted by

    Chinmay Farkya


    Description

    The NonFungiblePositionManager contract has a storage gap defined at the end of its storage slots. It is the child contract, so having a storage gap at its tail makes sense.

    It also inherits from ERC721PermitUpgradeable contract, which is an abstract contract and will never exist independently. This contract also declares a storage gap.

    This storage gap in ERC721PermitUpgradeable might be unnecessary if no modifications are expected here that are independent of the actual child contract.

    If we need to add any new state variables to the deployed NonFungiblePositionManager contract in an upgrade, we can add those to its own storage gap.

    Right now, ERC721PermitUpgradeable is only used in the NonFungiblePositionManager contract and we only need to streamline the storage slots of these two contracts, so this change will still be upgrade-safe.

    Recommendation

    Consider removing the storage gap in ERC721PermitUpgradeable.sol if it is not intended to be used in other contracts, and will not be modified independently of this NonFungiblePositionManager contract. Make sure to thoroughly review the storage structure if ERC721PermitUpgradeable sources are ever modified.

  3. Add License Notice

    Severity

    Severity: Informational

    Submitted by

    Chinmay Farkya


    Description

    There needs to be a License Notice file declaring that the codebase has been forked from Uniswap V3 under the GPL License.

    Such a notice already existed in the Moolah repository, but might have been missed when porting the codebase to a new repository.

    Recommendation

    Consider adding a License notice.

  4. Additional documentation can be added for POOL_INIT_CODE_HASH in the README

    Severity

    Severity: Informational

    Submitted by

    Chinmay Farkya


    Description

    The ListaV3Factory uses a pre-computed constant initCodeHash for deploying new Lista pools, just like Uniswap does.

    In the project README, under "Operational Notes" it has been acknowledged that the POOL_INIT_CODE_HASH depends on a lot of factors even if the ListaV3Pool solidity code does not change.

    Recommendation

    Consider adding a note on re-verifying the POOL_INIT_CODE_HASH every time Lista plans to deploy these contracts on a new chain, as the init_code might change on a different chain.