Skip to content

Commit

Permalink
Make the V3FactoryOwner payout amount settable by admin
Browse files Browse the repository at this point in the history 
The setter also enforces that the payout amount is non-zero, and emits an
event. The same applies in the constructor. The constructor also now emits
an AdminSet event for initialization of the original admin.
  • Loading branch information
@apbendi
apbendi committed Feb 15, 2024
1 parent 02d9ffa commit 2c3eab0
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 28 deletions.
31 changes: 26 additions & 5 deletions src/V3FactoryOwner.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,18 @@ contract V3FactoryOwner {
/// @notice Emitted when the existing admin designates a new address as the admin.
event AdminSet(address indexed oldAmin, address indexed newAdmin);

/// @notice Emitted when the admin updates the payout amount.
event PayoutAmountSet(uint256 indexed oldPayoutAmount, uint256 indexed newPayoutAmount);

/// @notice Thrown when an unauthorized account calls a privileged function.
error V3FactoryOwner__Unauthorized();

/// @notice Thrown if the proposed admin is the zero address.
error V3FactoryOwner__InvalidAddress();

/// @notice Thrown if the proposed payout amount is zero.
error V3FactoryOwner__InvalidPayoutAmount();

/// @notice Thrown when the fees collected from a pool are less than the caller expects.
error V3FactoryOwner__InsufficientFeesCollected();

Expand All @@ -63,7 +69,7 @@ contract V3FactoryOwner {
IERC20 public immutable PAYOUT_TOKEN;

/// @notice The raw amount of the payout token which is paid by a user when claiming pool fees.
uint256 public immutable PAYOUT_AMOUNT;
uint256 public payoutAmount;

/// @notice The contract that receives the payout and is notified via method call, when pool fees
/// are claimed.
Expand All @@ -76,7 +82,8 @@ contract V3FactoryOwner {
/// @param _admin The initial admin address for this deployment. Cannot be zero address.
/// @param _factory The v3 factory instance for which this deployment will serve as owner.
/// @param _payoutToken The ERC-20 token in which payouts will be denominated.
/// @param _payoutAmount The raw amount of the payout token required to claim fees from a pool.
/// @param _payoutAmount The initial raw amount of the payout token required to claim fees from
/// a pool.
/// @param _rewardReceiver The contract that will receive the payout when fees are claimed.
constructor(
address _admin,
Expand All @@ -86,11 +93,16 @@ contract V3FactoryOwner {
INotifiableRewardReceiver _rewardReceiver
) {
if (_admin == address(0)) revert V3FactoryOwner__InvalidAddress();
if (_payoutAmount == 0) revert V3FactoryOwner__InvalidPayoutAmount();

admin = _admin;
FACTORY = _factory;
PAYOUT_TOKEN = _payoutToken;
PAYOUT_AMOUNT = _payoutAmount;
payoutAmount = _payoutAmount;
REWARD_RECEIVER = _rewardReceiver;

emit AdminSet(address(0), _admin);
emit PayoutAmountSet(0, _payoutAmount);
}

/// @notice Pass the admin role to a new address. Must be called by the existing admin.
Expand All @@ -102,6 +114,15 @@ contract V3FactoryOwner {
admin = _newAdmin;
}

/// @notice Update the payout amount to a new value. Must be called by admin.
/// @param _newPayoutAmount The value that will be the new payout amount.
function setPayoutAmount(uint256 _newPayoutAmount) external {
_revertIfNotAdmin();
if (_newPayoutAmount == 0) revert V3FactoryOwner__InvalidPayoutAmount();
emit PayoutAmountSet(payoutAmount, _newPayoutAmount);
payoutAmount = _newPayoutAmount;
}

/// @notice Passthrough method that enables a fee amount on the factory. Must be called by the
/// admin.
/// @param _fee The fee param to forward to the factory.
Expand Down Expand Up @@ -163,8 +184,8 @@ contract V3FactoryOwner {
uint128 _amount0Requested,
uint128 _amount1Requested
) external returns (uint128, uint128) {
PAYOUT_TOKEN.safeTransferFrom(msg.sender, address(REWARD_RECEIVER), PAYOUT_AMOUNT);
REWARD_RECEIVER.notifyRewardAmount(PAYOUT_AMOUNT);
PAYOUT_TOKEN.safeTransferFrom(msg.sender, address(REWARD_RECEIVER), payoutAmount);
REWARD_RECEIVER.notifyRewardAmount(payoutAmount);
(uint128 _amount0, uint128 _amount1) =
_pool.collectProtocol(_recipient, _amount0Requested, _amount1Requested);

Expand Down
2 changes: 1 addition & 1 deletion test/UniStaker.integration.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ contract DeployScriptTest is Test, DeployInput {
assertEq(v3FactoryOwner.admin(), UNISWAP_GOVERNOR_TIMELOCK);
assertEq(address(v3FactoryOwner.FACTORY()), address(UNISWAP_V3_FACTORY_ADDRESS));
assertEq(address(v3FactoryOwner.PAYOUT_TOKEN()), PAYOUT_TOKEN_ADDRESS);
assertEq(v3FactoryOwner.PAYOUT_AMOUNT(), PAYOUT_AMOUNT);
assertEq(v3FactoryOwner.payoutAmount(), PAYOUT_AMOUNT);
assertEq(address(v3FactoryOwner.REWARD_RECEIVER()), address(uniStaker));

assertEq(address(uniStaker.REWARD_TOKEN()), PAYOUT_TOKEN_ADDRESS);
Expand Down
144 changes: 122 additions & 22 deletions test/V3FactoryOwner.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@ contract V3FactoryOwnerTest is Test {
MockUniswapV3Pool pool;
MockUniswapV3Factory factory;

event AdminSet(address indexed oldAmin, address indexed newAdmin);
event FeesClaimed(
address indexed pool,
address indexed caller,
address indexed recipient,
uint256 amount0,
uint256 amount1
);

function setUp() public {
vm.label(admin, "Admin");

Expand All @@ -49,6 +40,7 @@ contract V3FactoryOwnerTest is Test {
// In order to fuzz over the payout amount, we require each test to call this method to deploy
// the factory owner before doing anything else.
function _deployFactoryOwnerWithPayoutAmount(uint256 _payoutAmount) public {
vm.assume(_payoutAmount != 0);
factoryOwner = new V3FactoryOwner(admin, factory, payoutToken, _payoutAmount, rewardReceiver);
vm.label(address(factoryOwner), "Factory Owner");
}
Expand All @@ -61,7 +53,7 @@ contract Constructor is V3FactoryOwnerTest {
assertEq(factoryOwner.admin(), admin);
assertEq(address(factoryOwner.FACTORY()), address(factory));
assertEq(address(factoryOwner.PAYOUT_TOKEN()), address(payoutToken));
assertEq(factoryOwner.PAYOUT_AMOUNT(), _payoutAmount);
assertEq(factoryOwner.payoutAmount(), _payoutAmount);
assertEq(address(factoryOwner.REWARD_RECEIVER()), address(rewardReceiver));
}

Expand All @@ -72,7 +64,7 @@ contract Constructor is V3FactoryOwnerTest {
uint256 _payoutAmount,
address _rewardReceiver
) public {
vm.assume(_admin != address(0));
vm.assume(_admin != address(0) && _payoutAmount != 0);
V3FactoryOwner _factoryOwner = new V3FactoryOwner(
_admin,
IUniswapV3FactoryOwnerActions(_factory),
Expand All @@ -83,16 +75,57 @@ contract Constructor is V3FactoryOwnerTest {
assertEq(_factoryOwner.admin(), _admin);
assertEq(address(_factoryOwner.FACTORY()), address(_factory));
assertEq(address(_factoryOwner.PAYOUT_TOKEN()), address(_payoutToken));
assertEq(_factoryOwner.PAYOUT_AMOUNT(), _payoutAmount);
assertEq(_factoryOwner.payoutAmount(), _payoutAmount);
assertEq(address(_factoryOwner.REWARD_RECEIVER()), _rewardReceiver);
}

function testFuzz_EmitsAdminSetEvent(
address _admin,
address _factory,
address _payoutToken,
uint256 _payoutAmount,
address _rewardReceiver
) public {
vm.assume(_admin != address(0) && _payoutAmount != 0);

vm.expectEmit();
emit V3FactoryOwner.AdminSet(address(0), _admin);
new V3FactoryOwner(
_admin,
IUniswapV3FactoryOwnerActions(_factory),
IERC20(_payoutToken),
_payoutAmount,
INotifiableRewardReceiver(_rewardReceiver)
);
}

function testFuzz_EmitsPayoutSetEvent(
address _admin,
address _factory,
address _payoutToken,
uint256 _payoutAmount,
address _rewardReceiver
) public {
vm.assume(_admin != address(0) && _payoutAmount != 0);

vm.expectEmit();
emit V3FactoryOwner.PayoutAmountSet(0, _payoutAmount);
new V3FactoryOwner(
_admin,
IUniswapV3FactoryOwnerActions(_factory),
IERC20(_payoutToken),
_payoutAmount,
INotifiableRewardReceiver(_rewardReceiver)
);
}

function testFuzz_RevertIf_TheAdminIsAddressZero(
address _factory,
address _payoutToken,
uint256 _payoutAmount,
address _rewardReceiver
) public {
vm.assume(_payoutAmount != 0);
vm.expectRevert(V3FactoryOwner.V3FactoryOwner__InvalidAddress.selector);
new V3FactoryOwner(
address(0),
Expand All @@ -102,12 +135,30 @@ contract Constructor is V3FactoryOwnerTest {
INotifiableRewardReceiver(_rewardReceiver)
);
}

function testFuzz_RevertIf_ThePayoutAmountIsZero(
address _admin,
address _factory,
address _payoutToken,
address _rewardReceiver
) public {
vm.assume(_admin != address(0));

vm.expectRevert(V3FactoryOwner.V3FactoryOwner__InvalidPayoutAmount.selector);
new V3FactoryOwner(
_admin,
IUniswapV3FactoryOwnerActions(_factory),
IERC20(_payoutToken),
0,
INotifiableRewardReceiver(_rewardReceiver)
);
}
}

contract SetAdmin is V3FactoryOwnerTest {
function testFuzz_UpdatesTheAdminWhenCalledByTheCurrentAdmin(address _newAdmin) public {
vm.assume(_newAdmin != address(0));
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);

vm.prank(admin);
factoryOwner.setAdmin(_newAdmin);
Expand All @@ -117,18 +168,18 @@ contract SetAdmin is V3FactoryOwnerTest {

function testFuzz_EmitsAnEventWhenUpdatingTheAdmin(address _newAdmin) public {
vm.assume(_newAdmin != address(0));
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);

vm.expectEmit();
vm.prank(admin);
emit AdminSet(admin, _newAdmin);
emit V3FactoryOwner.AdminSet(admin, _newAdmin);
factoryOwner.setAdmin(_newAdmin);
}

function testFuzz_RevertIf_TheCallerIsNotTheCurrentAdmin(address _notAdmin, address _newAdmin)
public
{
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);

vm.assume(_notAdmin != admin);

Expand All @@ -138,20 +189,69 @@ contract SetAdmin is V3FactoryOwnerTest {
}

function test_RevertIf_TheNewAdminIsAddressZero() public {
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);

vm.expectRevert(V3FactoryOwner.V3FactoryOwner__InvalidAddress.selector);
vm.prank(admin);
factoryOwner.setAdmin(address(0));
}
}

contract SetPayoutAmount is V3FactoryOwnerTest {
function testFuzz_UpdatesThePayoutAmountWhenCalledByAdmin(
uint256 _initialPayoutAmount,
uint256 _newPayoutAmount
) public {
vm.assume(_newPayoutAmount != 0);
_deployFactoryOwnerWithPayoutAmount(_initialPayoutAmount);

vm.prank(admin);
factoryOwner.setPayoutAmount(_newPayoutAmount);

assertEq(factoryOwner.payoutAmount(), _newPayoutAmount);
}

function testFuzz_EmitsAnEventWhenUpdatingThePayoutAmount(
uint256 _initialPayoutAmount,
uint256 _newPayoutAmount
) public {
vm.assume(_newPayoutAmount != 0);
_deployFactoryOwnerWithPayoutAmount(_initialPayoutAmount);

vm.expectEmit();
vm.prank(admin);
emit V3FactoryOwner.PayoutAmountSet(_initialPayoutAmount, _newPayoutAmount);
factoryOwner.setPayoutAmount(_newPayoutAmount);
}

function testFuzz_RevertIf_TheCallerIsNotAdmin(
uint256 _initialPayoutAmount,
uint256 _newPayoutAmount,
address _notAdmin
) public {
vm.assume(_notAdmin != admin && _newPayoutAmount != 0);
_deployFactoryOwnerWithPayoutAmount(_initialPayoutAmount);

vm.expectRevert(V3FactoryOwner.V3FactoryOwner__Unauthorized.selector);
vm.prank(_notAdmin);
factoryOwner.setPayoutAmount(_newPayoutAmount);
}

function testFuzz_RevertIf_TheNewPayoutAmountIsZero(uint256 _initialPayoutAmount) public {
_deployFactoryOwnerWithPayoutAmount(_initialPayoutAmount);

vm.expectRevert(V3FactoryOwner.V3FactoryOwner__InvalidPayoutAmount.selector);
vm.prank(admin);
factoryOwner.setPayoutAmount(0);
}
}

contract EnableFeeAmount is V3FactoryOwnerTest {
function testFuzz_ForwardsParametersToTheEnableFeeAmountMethodOnTheFactory(
uint24 _fee,
int24 _tickSpacing
) public {
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);

vm.prank(admin);
factoryOwner.enableFeeAmount(_fee, _tickSpacing);
Expand All @@ -165,7 +265,7 @@ contract EnableFeeAmount is V3FactoryOwnerTest {
uint24 _fee,
int24 _tickSpacing
) public {
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);
vm.assume(_notAdmin != admin);

vm.expectRevert(V3FactoryOwner.V3FactoryOwner__Unauthorized.selector);
Expand All @@ -179,7 +279,7 @@ contract SetFeeProtocol is V3FactoryOwnerTest {
uint8 _feeProtocol0,
uint8 _feeProtocol1
) public {
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);

vm.prank(admin);
factoryOwner.setFeeProtocol(pool, _feeProtocol0, _feeProtocol1);
Expand All @@ -193,7 +293,7 @@ contract SetFeeProtocol is V3FactoryOwnerTest {
uint8 _feeProtocol0,
uint8 _feeProtocol1
) public {
_deployFactoryOwnerWithPayoutAmount(0);
_deployFactoryOwnerWithPayoutAmount(1);
vm.assume(_notAdmin != admin);

vm.expectRevert(V3FactoryOwner.V3FactoryOwner__Unauthorized.selector);
Expand Down Expand Up @@ -283,7 +383,7 @@ contract ClaimFees is V3FactoryOwnerTest {
vm.startPrank(_caller);
payoutToken.approve(address(factoryOwner), _payoutAmount);
vm.expectEmit();
emit FeesClaimed(address(pool), _caller, _recipient, _amount0, _amount1);
emit V3FactoryOwner.FeesClaimed(address(pool), _caller, _recipient, _amount0, _amount1);
factoryOwner.claimFees(pool, _recipient, _amount0, _amount1);
vm.stopPrank();
}
Expand Down

0 comments on commit 2c3eab0

Please sign in to comment.