Merge pull request #1 from lista-dao/audit/v3

V3 Audit

Author: razww

NOTICE

@@ -0,0 +1,12 @@
+This code is derived from Uniswap V3 (https://github.com/Uniswap/v3-core,
+https://github.com/Uniswap/v3-periphery), originally licensed under
+Business Source License 1.1, which transitioned to GPL-2.0-or-later
+on 2023-04-01.
+
+Portions of src/core/libraries/ (FullMath, UnsafeMath) are derived
+from work by Remco Bloemen, licensed under MIT.
+
+Modifications by Lista DAO to the original Uniswap V3 code are licensed
+under GPL-2.0-or-later, consistent with the original license terms.
+
+Original files authored by Lista DAO are licensed under MIT.

README.md

@@ -1,33 +1,34 @@
 # Lista V3
 
-A Uniswap V3 fork renamed to Lista V3, with the factory and NFT position manager deployed behind upgradeable proxies.
+A Uniswap V3 fork renamed to Lista V3, with the NFT position manager deployed behind an upgradeable proxy. Factory, pool, and router behavior tracks upstream Uniswap V3.
 
 ## Contracts
 
 ### Core (`src/core/`)
 
-- `ListaV3Factory` — canonical factory. **Upgradeable** via `TransparentUpgradeableProxy`; initializer replaces the constructor and seeds the 500 / 3000 / 10000 fee tiers.
-- `ListaV3Pool` — AMM pool, CREATE2-deployed by the factory, **not upgradeable**.
+- `ListaV3Factory` — canonical factory. Plain (non-upgradeable) deploy; constructor sets the deployer as owner and seeds the 500 / 3000 / 10000 fee tiers. Keeps `NoDelegateCall` as upstream.
+- `ListaV3Pool` — AMM pool, CREATE2-deployed by the factory, not upgradeable.
 - `ListaV3PoolDeployer` — base of the factory; writes transient parameters so the pool constructor can read them back, keeping `POOL_INIT_CODE_HASH` constant for off-chain address derivation.
 
 ### Periphery (`src/periphery/`)
 
-- `NonfungiblePositionManager` — wraps positions as ERC-721 NFTs. **Hybrid upgradeable**: the ERC-721 / permit stack uses `ERC721PermitUpgradeable` (storage-backed, behind the proxy), while `factory` and `WETH9` remain constructor-set `immutable`s in the implementation bytecode. Every impl upgrade must re-pass the original `(factory, WETH9)` to the new impl's constructor.
+- `NonfungiblePositionManager` — wraps positions as ERC-721 NFTs. **Hybrid upgradeable**: the ERC-721 / permit stack uses `ERC721PermitUpgradeable` (storage-backed, behind a `TransparentUpgradeableProxy`), while `factory` and `WETH9` remain constructor-set `immutable`s in the implementation bytecode. Every impl upgrade must re-pass the original `(factory, WETH9)` to the new impl's constructor.
 - `SwapRouter`, `V3Migrator`, `Quoter`, `QuoterV2`, `NonfungibleTokenPositionDescriptor`, `PairFlash` — deployed normally (non-upgradeable).
 
 ## Deployment outline
 
-1. Deploy `ListaV3Factory` impl (no constructor args).
-2. Deploy a shared `ProxyAdmin`.
-3. Deploy `TransparentUpgradeableProxy(factoryImpl, proxyAdmin, abi.encodeWithSelector(ListaV3Factory.initialize.selector, owner))`. Treat the proxy address as *the* factory from here on.
-4. Deploy `NonfungiblePositionManager` impl with `(factoryProxy, WETH9)`.
-5. Deploy `TransparentUpgradeableProxy(npmImpl, proxyAdmin, abi.encodeWithSelector(NonfungiblePositionManager.initialize.selector, tokenDescriptor))`.
-6. Deploy `SwapRouter`, `V3Migrator`, etc. against the factory proxy address.
+1. Deploy `ListaV3Factory` directly. The deployer becomes owner; call `setOwner` afterwards if a different owner is required.
+2. Deploy a `ProxyAdmin` for the NPM proxy (and any future upgradeable contracts).
+3. Deploy `NonfungiblePositionManager` impl with `(factory, WETH9)`.
+4. Deploy `TransparentUpgradeableProxy(npmImpl, proxyAdmin, abi.encodeWithSelector(NonfungiblePositionManager.initialize.selector, tokenDescriptor))`.
+5. Deploy `SwapRouter`, `V3Migrator`, etc. against the factory address.
 
 Operational notes:
 
-- The Factory/NPM implementations should have their initializers consumed post-deploy (e.g. `initialize(0xdead)` / `initialize(0xdead, 0xdead)`) to close the Parity-style impl-takeover window.
-- Pool addresses are derived from the factory proxy via `PoolAddress.computeAddress`. If `ListaV3Pool` bytecode is ever changed, `PoolAddress.POOL_INIT_CODE_HASH` must be recomputed — the value in `src/periphery/libraries/PoolAddress.sol` is only valid for the currently-checked-in pool source and build settings.
+- The NPM implementation should have its initializer consumed post-deploy (e.g. `npmImpl.initialize(0xdead)`) to close the Parity-style impl-takeover window.
+- Every NPM impl upgrade must re-pass the same `(factory, WETH9)` to the new impl's constructor — the values are baked into impl bytecode as immutables. A deploy script that reads `npm.factory()` / `npm.WETH9()` from the existing proxy and forwards them to the new impl's constructor is the safest pattern.
+- Pool addresses are derived from the factory via `PoolAddress.computeAddress`. If `ListaV3Pool` bytecode is ever changed, `PoolAddress.POOL_INIT_CODE_HASH` must be recomputed — the value in `src/periphery/libraries/PoolAddress.sol` is only valid for the currently-checked-in pool source and build settings.
+- Re-verify `POOL_INIT_CODE_HASH` before deploying to a new chain. The value is machine-deterministic given `bytecode_hash = "none"` in `foundry.toml`, but a different toolchain version, optimizer setting, or solc patch can still shift it. Run `testInitCodeHash` in `test/periphery/FullFlowTest.t.sol` against your build environment as a pre-deploy gate; if it fails, update the constant before deploying or off-chain pool address derivation will silently point at the wrong addresses.
 
 ## Build & test
 
@@ -40,6 +41,10 @@ forge test
 
 The end-to-end flow (pool creation, mint, swap, increase / decrease liquidity, collect, transfer, burn) runs in `test/periphery/FullFlowTest.t.sol`, which also exercises the proxy wiring and asserts NPM initializer state / ERC-165 registrations.
 
+## License
+
+Source is GPL-2.0-or-later (see `NOTICE` for derivation and attribution).
+
 ## Dependencies
 
 Git submodules under `lib/`:

audits/260430-Cantina-Lista-V3-Final-Report.pdf

@@ -8,17 +8,19 @@ optimizer_runs = 500
 # deterministic across machines / lib changes (matching Uniswap V3's production config).
 bytecode_hash = "none"
 
-# Factory and NonfungiblePositionManager exceed EIP-170 at the default run count.
-# Compile them with fewer runs (smaller runtime bytecode, slightly more gas per call).
-# Same profile for both so Foundry can resolve Deploy.s.sol (which imports both) as
-# a single compilation unit.
+# NonfungiblePositionManager exceeds EIP-170 at the default run count, so compile
+# it with fewer runs (smaller runtime bytecode, slightly more gas per call).
+# ListaV3Pool is pinned to the same profile so POOL_INIT_CODE_HASH binds to exactly
+# one Pool bytecode regardless of which compilation unit pulls it in — otherwise
+# Foundry produces two Pool artifacts (default + small) and the hardcoded constant
+# would silently match only one of them (per Bailsec audit Issue_06).
 additional_compiler_profiles = [
     { name = "small", optimizer_runs = 50 },
 ]
 
 compilation_restrictions = [
-    { paths = "src/core/ListaV3Factory.sol", optimizer_runs = 50 },
     { paths = "src/periphery/NonfungiblePositionManager.sol", optimizer_runs = 50 },
+    { paths = "src/core/ListaV3Pool.sol", optimizer_runs = 50 },
 ]
 
 # See more config options https://github.com/foundry-rs/foundry/blob/master/crates/config/README.md#all-options

audits/260515-Bailsec-Lista-V3-Differential-Final-Report.pdf

@@ -31,8 +31,7 @@ import {TransparentUpgradeableProxy} from 'lib/openzeppelin-contracts/contracts/
 contract Deploy is Script {
     struct Deployment {
         address proxyAdmin;
-        address factoryImpl;
-        address factoryProxy;
+        address factory;
         address npmImpl;
         address npmProxy;
         address swapRouter;
@@ -50,6 +49,7 @@ contract Deploy is Script {
         address tokenDescriptor = vm.envOr('TOKEN_DESCRIPTOR', address(0));
 
         require(owner != address(0), 'OWNER=0');
+        require(proxyAdminOwner != address(0), 'PROXY_ADMIN_OWNER=0');
         require(weth9 != address(0), 'WETH9=0');
 
         console.log('--- Lista V3 deploy ---');
@@ -63,46 +63,44 @@ contract Deploy is Script {
         vm.startBroadcast();
 
         ProxyAdmin proxyAdmin = new ProxyAdmin();
-        if (proxyAdminOwner != address(this) && proxyAdminOwner != proxyAdmin.owner()) {
+        // ProxyAdmin.owner() is set to msg.sender (the broadcaster) in its constructor.
+        // Transfer only if a different owner was requested.
+        if (proxyAdminOwner != proxyAdmin.owner()) {
             proxyAdmin.transferOwnership(proxyAdminOwner);
         }
 
-        // Factory: impl + proxy
-        ListaV3Factory factoryImpl = new ListaV3Factory();
-        // Consume the impl's initializer so it can't be hijacked on-chain.
-        factoryImpl.initialize(address(0xdead));
-
-        bytes memory factoryInit = abi.encodeWithSelector(ListaV3Factory.initialize.selector, owner);
-        TransparentUpgradeableProxy factoryProxy =
-            new TransparentUpgradeableProxy(address(factoryImpl), address(proxyAdmin), factoryInit);
+        // Factory: plain deploy. owner = msg.sender (the broadcaster). If a separate
+        // owner is required, the broadcaster should call factory.setOwner(owner) after.
+        ListaV3Factory factory = new ListaV3Factory();
+        if (owner != msg.sender) {
+            factory.setOwner(owner);
+        }
 
         // NPM: impl + proxy. factory/WETH9 are constructor immutables on the impl; every
-        // future upgrade MUST re-pass the exact same (factoryProxy, WETH9) to the new impl.
-        NonfungiblePositionManager npmImpl = new NonfungiblePositionManager(address(factoryProxy), weth9);
+        // future upgrade MUST re-pass the exact same (factory, WETH9) to the new impl.
+        NonfungiblePositionManager npmImpl = new NonfungiblePositionManager(address(factory), weth9);
         npmImpl.initialize(address(0xdead));
 
         bytes memory npmInit = abi.encodeWithSelector(NonfungiblePositionManager.initialize.selector, tokenDescriptor);
         TransparentUpgradeableProxy npmProxy =
             new TransparentUpgradeableProxy(address(npmImpl), address(proxyAdmin), npmInit);
 
-        // SwapRouter is not upgradeable; it's a plain deploy against the factory proxy.
-        SwapRouter swapRouter = new SwapRouter(address(factoryProxy), weth9);
+        // SwapRouter is not upgradeable; plain deploy against the factory.
+        SwapRouter swapRouter = new SwapRouter(address(factory), weth9);
 
         vm.stopBroadcast();
 
         out = Deployment({
             proxyAdmin: address(proxyAdmin),
-            factoryImpl: address(factoryImpl),
-            factoryProxy: address(factoryProxy),
+            factory: address(factory),
             npmImpl: address(npmImpl),
             npmProxy: address(npmProxy),
             swapRouter: address(swapRouter)
         });
 
         console.log('--- deployed ---');
         console.log('ProxyAdmin:', out.proxyAdmin);
-        console.log('Factory impl:', out.factoryImpl);
-        console.log('Factory proxy:', out.factoryProxy);
+        console.log('Factory:', out.factory);
         console.log('NPM impl:', out.npmImpl);
         console.log('NPM proxy:', out.npmProxy);
         console.log('SwapRouter:', out.swapRouter);

foundry.toml

@@ -1,18 +1,16 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 pragma solidity =0.7.6;
 
-import 'lib/openzeppelin-contracts-upgradeable/contracts/proxy/Initializable.sol';
-
 import './interfaces/IListaV3Factory.sol';
 
 import './ListaV3PoolDeployer.sol';
+import './NoDelegateCall.sol';
 
 import './ListaV3Pool.sol';
 
 /// @title Canonical Lista V3 factory
 /// @notice Deploys Lista V3 pools and manages ownership and control over pool protocol fees
-/// @dev Deployed behind a TransparentUpgradeableProxy; initialize() replaces the constructor.
-contract ListaV3Factory is IListaV3Factory, ListaV3PoolDeployer, Initializable {
+contract ListaV3Factory is IListaV3Factory, ListaV3PoolDeployer, NoDelegateCall {
     /// @inheritdoc IListaV3Factory
     address public override owner;
 
@@ -21,13 +19,9 @@ contract ListaV3Factory is IListaV3Factory, ListaV3PoolDeployer, Initializable {
     /// @inheritdoc IListaV3Factory
     mapping(address => mapping(address => mapping(uint24 => address))) public override getPool;
 
-    /// @dev Reserved for future layout additions behind the proxy.
-    uint256[50] private __gap;
-
-    function initialize(address owner_) external initializer {
-        require(owner_ != address(0));
-        owner = owner_;
-        emit OwnerChanged(address(0), owner_);
+    constructor() {
+        owner = msg.sender;
+        emit OwnerChanged(address(0), msg.sender);
 
         feeAmountTickSpacing[500] = 10;
         emit FeeAmountEnabled(500, 10);
@@ -42,7 +36,7 @@ contract ListaV3Factory is IListaV3Factory, ListaV3PoolDeployer, Initializable {
         address tokenA,
         address tokenB,
         uint24 fee
-    ) external override returns (address pool) {
+    ) external override noDelegateCall returns (address pool) {
         require(tokenA != tokenB);
         (address token0, address token1) = tokenA < tokenB ? (tokenA, tokenB) : (tokenB, tokenA);
         require(token0 != address(0));

script/Deploy.s.sol

@@ -73,8 +73,6 @@ contract NonfungiblePositionManager is
     /// @dev The address of the token descriptor contract, which handles generating token URIs for position tokens
     address private _tokenDescriptor;
 
-    uint256[45] private __gap;
-
     constructor(address _factory, address _WETH9) PeripheryImmutableState(_factory, _WETH9) {}
 
     function initialize(address _tokenDescriptor_) external initializer {

src/core/ListaV3Factory.sol

@@ -24,6 +24,9 @@ abstract contract ERC721PermitUpgradeable is Initializable, BlockTimestamp, ERC7
     /// @dev The hash of the version string used in the permit signature verification
     bytes32 private _versionHash;
 
+    /// @dev Reserved slots for future state additions to this base. Sized so the base
+    /// (_nameHash + _versionHash + __gap) occupies 50 slots, so additions here consume
+    /// from __gap rather than shifting NonfungiblePositionManager's storage layout.
     uint256[48] private __gap;
 
     function __ERC721Permit_init(

src/periphery/NonfungiblePositionManager.sol

@@ -46,7 +46,7 @@ library NFTSVG {
     function generateSVG(SVGParams memory params) internal pure returns (string memory svg) {
         /*
         address: "0xe8ab59d3bcde16a29912de83a90eb39628cfc163",
-        msg: "Forged in SVG for Lista in 2021 by 0xe8ab59d3bcde16a29912de83a90eb39628cfc163",
+        msg: "Forged in SVG for Uniswap in 2021 by 0xe8ab59d3bcde16a29912de83a90eb39628cfc163",
         sig: "0x2df0e99d9cbfec33a705d83f75666d98b22dea7c1af412c584f7d626d83f02875993df740dc87563b9c73378f8462426da572d7989de88079a382ad96c57b68d1b",
         version: "2"
         */