Organization
- @lista-dao
Engagement Type
Cantina Solo
Period
-
Repositories
Researchers
Findings
Informational
4 findings
4 fixed
0 acknowledged
Informational4 findings
ListaV3Factory differs from the original pattern used by Uniswap to prevent code reuse
Severity
- Severity: Informational
Submitted by
Chinmay Farkya
Description
The
UniswapV3Factoryuses anoDelegateCallmodifier 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
ListaV3Factoryis now an implementation contract behind a proxy andnoDelegateCallhad to be removed to make it work.Now other proxies that want to reuse this code can easily point to this deployed
ListaV3Factoryimplementation 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;NoDelegateCallwas 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.
Storage gap in ERC721PermitUpgradeable can be removed
Severity
- Severity: Informational
Submitted by
Chinmay Farkya
Description
The
NonFungiblePositionManagercontract 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
ERC721PermitUpgradeablecontract, which is an abstract contract and will never exist independently. This contract also declares a storage gap.This storage gap in
ERC721PermitUpgradeablemight 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
NonFungiblePositionManagercontract in an upgrade, we can add those to its own storage gap.Right now,
ERC721PermitUpgradeableis only used in theNonFungiblePositionManagercontract 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.solif it is not intended to be used in other contracts, and will not be modified independently of thisNonFungiblePositionManager contract. Make sure to thoroughly review the storage structure ifERC721PermitUpgradeablesources are ever modified.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.
Additional documentation can be added for POOL_INIT_CODE_HASH in the README
Severity
- Severity: Informational
Submitted by
Chinmay Farkya
Description
The
ListaV3Factoryuses 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_HASHdepends 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_HASHevery time Lista plans to deploy these contracts on a new chain, as theinit_codemight change on a different chain.