From 49b09eb077a5804a7857f0993344dfc811863144 Mon Sep 17 00:00:00 2001 From: Ben DiFrancesco Date: Sat, 6 Jan 2024 17:06:25 -0500 Subject: [PATCH] Allow the factory owner to transfer ownership of the factory --- src/V3FactoryOwner.sol | 5 ++ .../IUniswapV3FactoryOwnerActions.sol | 22 +++--- test/V3FactoryOwner.t.sol | 69 ++++++++++++++----- test/mocks/MockUniswapV3Factory.sol | 20 +++--- 4 files changed, 79 insertions(+), 37 deletions(-) diff --git a/src/V3FactoryOwner.sol b/src/V3FactoryOwner.sol index 5f8b394..cca3f6d 100644 --- a/src/V3FactoryOwner.sol +++ b/src/V3FactoryOwner.sol @@ -50,6 +50,11 @@ contract V3FactoryOwner { admin = _newAdmin; } + function transferFactoryOwnership(address _newOwner) external { + _revertIfNotAdmin(); + FACTORY.setOwner(_newOwner); + } + function enableFeeAmount(uint24 _fee, int24 _tickSpacing) external { _revertIfNotAdmin(); FACTORY.enableFeeAmount(_fee, _tickSpacing); diff --git a/src/interfaces/IUniswapV3FactoryOwnerActions.sol b/src/interfaces/IUniswapV3FactoryOwnerActions.sol index edce675..c14b172 100644 --- a/src/interfaces/IUniswapV3FactoryOwnerActions.sol +++ b/src/interfaces/IUniswapV3FactoryOwnerActions.sol @@ -2,18 +2,20 @@ pragma solidity ^0.8.23; /// @title The interface for the Uniswap V3 Factory -/// @notice The Uniswap V3 Factory facilitates creation of Uniswap V3 pools and control over the protocol fees +/// @notice The Uniswap V3 Factory facilitates creation of Uniswap V3 pools and control over the +/// protocol fees /// @dev Stripped down and renamed from: /// https://github.com/Uniswap/v3-core/blob/d8b1c635c275d2a9450bd6a78f3fa2484fef73eb/contracts/interfaces/IUniswapV3Factory.sol interface IUniswapV3FactoryOwnerActions { -/// @notice Updates the owner of the factory - /// @dev Must be called by the current owner - /// @param _owner The new owner of the factory - function setOwner(address _owner) external; + /// @notice Updates the owner of the factory + /// @dev Must be called by the current owner + /// @param _owner The new owner of the factory + function setOwner(address _owner) external; - /// @notice Enables a fee amount with the given tickSpacing - /// @dev Fee amounts may never be removed once enabled - /// @param fee The fee amount to enable, denominated in hundredths of a bip (i.e. 1e-6) - /// @param tickSpacing The spacing between ticks to be enforced for all pools created with the given fee amount - function enableFeeAmount(uint24 fee, int24 tickSpacing) external; + /// @notice Enables a fee amount with the given tickSpacing + /// @dev Fee amounts may never be removed once enabled + /// @param fee The fee amount to enable, denominated in hundredths of a bip (i.e. 1e-6) + /// @param tickSpacing The spacing between ticks to be enforced for all pools created with the + /// given fee amount + function enableFeeAmount(uint24 fee, int24 tickSpacing) external; } diff --git a/test/V3FactoryOwner.t.sol b/test/V3FactoryOwner.t.sol index 132b190..ba90a2d 100644 --- a/test/V3FactoryOwner.t.sol +++ b/test/V3FactoryOwner.t.sol @@ -21,7 +21,6 @@ contract V3FactoryOwnerTest is Test { MockUniswapV3Pool pool; MockUniswapV3Factory factory; - event AdminUpdated(address indexed oldAmin, address indexed newAdmin); event FeesClaimed( address indexed pool, @@ -74,7 +73,12 @@ contract Constructor is V3FactoryOwnerTest { address _rewardReceiver ) public { vm.assume(_admin != address(0)); - V3FactoryOwner _factoryOwner = new V3FactoryOwner(_admin, IUniswapV3FactoryOwnerActions(_factory), IERC20(_payoutToken), _payoutAmount, INotifiableRewardReceiver(_rewardReceiver) + V3FactoryOwner _factoryOwner = new V3FactoryOwner( + _admin, + IUniswapV3FactoryOwnerActions(_factory), + IERC20(_payoutToken), + _payoutAmount, + INotifiableRewardReceiver(_rewardReceiver) ); assertEq(_factoryOwner.admin(), _admin); assertEq(address(_factoryOwner.FACTORY()), address(_factory)); @@ -91,7 +95,11 @@ contract Constructor is V3FactoryOwnerTest { ) public { vm.expectRevert(V3FactoryOwner.V3FactoryOwner__InvalidAddress.selector); new V3FactoryOwner( - address(0), IUniswapV3FactoryOwnerActions(_factory), IERC20(_payoutToken), _payoutAmount, INotifiableRewardReceiver(_rewardReceiver) + address(0), + IUniswapV3FactoryOwnerActions(_factory), + IERC20(_payoutToken), + _payoutAmount, + INotifiableRewardReceiver(_rewardReceiver) ); } } @@ -138,25 +146,52 @@ contract SetAdmin is V3FactoryOwnerTest { } } +contract TransferFactoryOwnership is V3FactoryOwnerTest { + function testFuzz_CurriesNewOwnerToSetOwnerMethodOnFactory(address _newOwner) public { + _deployFactoryOwnerWithPayoutAmount(0); + + vm.prank(admin); + factoryOwner.transferFactoryOwnership(_newOwner); + + assertEq(factory.lastParam__setOwner_owner(), _newOwner); + } + + function testFuzz_RevertIf_TheCallerIsNotTheAdmin(address _notAdmin, address _newOwner) public { + _deployFactoryOwnerWithPayoutAmount(0); + vm.assume(_notAdmin != admin); + + vm.expectRevert(V3FactoryOwner.V3FactoryOwner__Unauthorized.selector); + vm.prank(_notAdmin); + factoryOwner.transferFactoryOwnership(_newOwner); + } +} + contract EnableFeeAmount is V3FactoryOwnerTest { - function testFuzz_CurriesParametersToTheEnableFeeAmountMethodOnTheFactory(uint24 _fee, int24 _tickSpacing) public { - _deployFactoryOwnerWithPayoutAmount(0); + function testFuzz_CurriesParametersToTheEnableFeeAmountMethodOnTheFactory( + uint24 _fee, + int24 _tickSpacing + ) public { + _deployFactoryOwnerWithPayoutAmount(0); - vm.prank(admin); - factoryOwner.enableFeeAmount(_fee, _tickSpacing); + vm.prank(admin); + factoryOwner.enableFeeAmount(_fee, _tickSpacing); - assertEq(factory.lastParam__enableFeeAmount_fee(), _fee); - assertEq(factory.lastParam__enableFeeAmount_tickSpacing(), _tickSpacing); - } + assertEq(factory.lastParam__enableFeeAmount_fee(), _fee); + assertEq(factory.lastParam__enableFeeAmount_tickSpacing(), _tickSpacing); + } - function testFuzz_RevertIf_TheCallerIsNotTheAdmin(address _notAdmin, uint24 _fee, int24 _tickSpacing) public { - _deployFactoryOwnerWithPayoutAmount(0); - vm.assume(_notAdmin != admin); + function testFuzz_RevertIf_TheCallerIsNotTheAdmin( + address _notAdmin, + uint24 _fee, + int24 _tickSpacing + ) public { + _deployFactoryOwnerWithPayoutAmount(0); + vm.assume(_notAdmin != admin); - vm.expectRevert(V3FactoryOwner.V3FactoryOwner__Unauthorized.selector); - vm.prank(_notAdmin); - factoryOwner.enableFeeAmount(_fee, _tickSpacing); - } + vm.expectRevert(V3FactoryOwner.V3FactoryOwner__Unauthorized.selector); + vm.prank(_notAdmin); + factoryOwner.enableFeeAmount(_fee, _tickSpacing); + } } contract ClaimFees is V3FactoryOwnerTest { diff --git a/test/mocks/MockUniswapV3Factory.sol b/test/mocks/MockUniswapV3Factory.sol index dbe55b2..180d521 100644 --- a/test/mocks/MockUniswapV3Factory.sol +++ b/test/mocks/MockUniswapV3Factory.sol @@ -4,17 +4,17 @@ pragma solidity 0.8.23; import {IUniswapV3FactoryOwnerActions} from "src/interfaces/IUniswapV3FactoryOwnerActions.sol"; contract MockUniswapV3Factory is IUniswapV3FactoryOwnerActions { - address public lastParam__setOwner_owner; + address public lastParam__setOwner_owner; - uint24 public lastParam__enableFeeAmount_fee; - int24 public lastParam__enableFeeAmount_tickSpacing; + uint24 public lastParam__enableFeeAmount_fee; + int24 public lastParam__enableFeeAmount_tickSpacing; - function setOwner(address _owner) external { - lastParam__setOwner_owner = _owner; - } + function setOwner(address _owner) external { + lastParam__setOwner_owner = _owner; + } - function enableFeeAmount(uint24 fee, int24 tickSpacing) external { - lastParam__enableFeeAmount_fee = fee; - lastParam__enableFeeAmount_tickSpacing = tickSpacing; - } + function enableFeeAmount(uint24 fee, int24 tickSpacing) external { + lastParam__enableFeeAmount_fee = fee; + lastParam__enableFeeAmount_tickSpacing = tickSpacing; + } }