Skip to content

Commit

Permalink
refactor: remove caller constraint fromwithdrawFees (#13)
Browse files Browse the repository at this point in the history 
* refactor: make withdrawFees function publicly callable

* build: update remappings

* refactor: rename withdrawFees to collectFees

docs: improve writing in NatSpec
refactor: get rid of "admin-facing" section
test: rename DEFAULT_FEE to FEE

---------

Co-authored-by: Paul Razvan Berg <[email protected]>
  • Loading branch information
@smol-ninja @PaulRBerg
smol-ninja and PaulRBerg authored Nov 22, 2024
1 parent 3c4272b commit 87b4af2
Show file tree
Hide file tree
Showing 35 changed files with 323 additions and 382 deletions.
Binary file modified bun.lockb
View file
Binary file not shown.
2 changes: 1 addition & 1 deletion remappings.txt
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@openzeppelin/contracts/=node_modules/@openzeppelin/contracts/
@prb/math/=node_modules/@prb/math/
@sablier/lockup/=node_modules/@sablier/lockup/src/core/
@sablier/lockup/=node_modules/@sablier/lockup/src/
@sablier/lockup-precompiles=node_modules/@sablier/lockup/precompiles/
forge-std/=node_modules/forge-std/
solady/=node_modules/solady/
86 changes: 38 additions & 48 deletions src/SablierMerkleFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { ISablierMerkleFactory } from "./interfaces/ISablierMerkleFactory.sol";
import { ISablierMerkleInstant } from "./interfaces/ISablierMerkleInstant.sol";
import { ISablierMerkleLL } from "./interfaces/ISablierMerkleLL.sol";
import { ISablierMerkleLT } from "./interfaces/ISablierMerkleLT.sol";
import { Errors } from "./libraries/Errors.sol";
import { SablierMerkleInstant } from "./SablierMerkleInstant.sol";
import { SablierMerkleLL } from "./SablierMerkleLL.sol";
import { SablierMerkleLT } from "./SablierMerkleLT.sol";
Expand Down Expand Up @@ -64,59 +63,18 @@ contract SablierMerkleFactory is
}

/*//////////////////////////////////////////////////////////////////////////
ADMIN-FACING NON-CONSTANT FUNCTIONS
USER-FACING NON-CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc ISablierMerkleFactory
function resetCustomFee(address campaignCreator) external override onlyAdmin {
delete _customFees[campaignCreator];

// Log the reset.
emit ResetCustomFee({ admin: msg.sender, campaignCreator: campaignCreator });
}

/// @inheritdoc ISablierMerkleFactory
function setCustomFee(address campaignCreator, uint256 newFee) external override onlyAdmin {
MerkleFactory.CustomFee storage customFeeByUser = _customFees[campaignCreator];

// Check: if the user is not in the custom fee list.
if (!customFeeByUser.enabled) {
customFeeByUser.enabled = true;
}

// Effect: update the custom fee for the given campaign creator.
customFeeByUser.fee = newFee;

// Log the update.
emit SetCustomFee({ admin: msg.sender, campaignCreator: campaignCreator, customFee: newFee });
}

/// @inheritdoc ISablierMerkleFactory
function setDefaultFee(uint256 defaultFee_) external override onlyAdmin {
// Effect: update the default fee.
defaultFee = defaultFee_;
function collectFees(ISablierMerkleBase merkleBase) external override {
// Effect: collect the fees from the MerkleBase contract.
uint256 feeAmount = merkleBase.collectFees(admin);

emit SetDefaultFee(msg.sender, defaultFee_);
// Log the fee withdrawal.
emit CollectFees({ admin: admin, merkleBase: merkleBase, feeAmount: feeAmount });
}

/// @inheritdoc ISablierMerkleFactory
function withdrawFees(address payable to, ISablierMerkleBase merkleBase) external override onlyAdmin {
// Check: the withdrawal address is not zero.
if (to == address(0)) {
revert Errors.SablierMerkleFactory_WithdrawToZeroAddress();
}

// Effect: call `withdrawFees` on the MerkleBase contract.
uint256 fees = merkleBase.withdrawFees(to);

// Log the withdrawal.
emit WithdrawFees({ admin: msg.sender, merkleBase: merkleBase, to: to, fees: fees });
}

/*//////////////////////////////////////////////////////////////////////////
USER-FACING NON-CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @inheritdoc ISablierMerkleFactory
function createMerkleInstant(
MerkleBase.ConstructorParams memory baseParams,
Expand Down Expand Up @@ -241,6 +199,38 @@ contract SablierMerkleFactory is
);
}

/// @inheritdoc ISablierMerkleFactory
function resetCustomFee(address campaignCreator) external override onlyAdmin {
delete _customFees[campaignCreator];

// Log the reset.
emit ResetCustomFee({ admin: msg.sender, campaignCreator: campaignCreator });
}

/// @inheritdoc ISablierMerkleFactory
function setCustomFee(address campaignCreator, uint256 newFee) external override onlyAdmin {
MerkleFactory.CustomFee storage customFeeByUser = _customFees[campaignCreator];

// Check: if the user is not in the custom fee list.
if (!customFeeByUser.enabled) {
customFeeByUser.enabled = true;
}

// Effect: update the custom fee for the given campaign creator.
customFeeByUser.fee = newFee;

// Log the update.
emit SetCustomFee({ admin: msg.sender, campaignCreator: campaignCreator, customFee: newFee });
}

/// @inheritdoc ISablierMerkleFactory
function setDefaultFee(uint256 defaultFee_) external override onlyAdmin {
// Effect: update the default fee.
defaultFee = defaultFee_;

emit SetDefaultFee(msg.sender, defaultFee_);
}

/*//////////////////////////////////////////////////////////////////////////
PRIVATE NON-CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/
Expand Down
10 changes: 5 additions & 5 deletions src/abstracts/SablierMerkleBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,20 +165,20 @@ abstract contract SablierMerkleBase is
}

/// @inheritdoc ISablierMerkleBase
function withdrawFees(address payable to) external override returns (uint256 feeAmount) {
// Check: the msg.sender is the FACTORY.
function collectFees(address factoryAdmin) external override returns (uint256 feeAmount) {
// Check: the caller is the FACTORY.
if (msg.sender != FACTORY) {
revert Errors.SablierMerkleBase_CallerNotFactory(FACTORY, msg.sender);
}

feeAmount = address(this).balance;

// Effect: transfer the fees to the provided address.
(bool success,) = to.call{ value: feeAmount }("");
// Effect: transfer the fees to the factory admin.
(bool success,) = factoryAdmin.call{ value: feeAmount }("");

// Revert if the call failed.
if (!success) {
revert Errors.SablierMerkleBase_FeeWithdrawFailed(to, feeAmount);
revert Errors.SablierMerkleBase_FeeTransferFail(factoryAdmin, feeAmount);
}
}

Expand Down
11 changes: 4 additions & 7 deletions src/interfaces/ISablierMerkleBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,12 @@ interface ISablierMerkleBase is IAdminable {
/// @param amount The amount of tokens to claw back.
function clawback(address to, uint128 amount) external;

/// @notice Withdraws the accrued fees to the provided address.
///
/// @dev This function transfers ETH to the provided address. If the receiver is a contract, it must be able to
/// receive ETH.
/// @notice Collects the accrued fees by transferring them to `FACTORY` admin.
///
/// Requirements:
/// - msg.sender must be the `FACTORY` contract.
///
/// @param to The address to transfer the fees to.
/// @return feeAmount The amount of ETH transferred to the provided address.
function withdrawFees(address payable to) external returns (uint256 feeAmount);
/// @param factoryAdmin The address of the `FACTORY` admin.
/// @return feeAmount The amount of ETH withdrawn.
function collectFees(address factoryAdmin) external returns (uint256 feeAmount);
}
29 changes: 12 additions & 17 deletions src/interfaces/ISablierMerkleFactory.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ interface ISablierMerkleFactory is IAdminable {
EVENTS
//////////////////////////////////////////////////////////////////////////*/

/// @notice Emitted when the accrued fees are collected.
event CollectFees(address indexed admin, ISablierMerkleBase indexed merkleBase, uint256 feeAmount);

/// @notice Emitted when a {SablierMerkleInstant} campaign is created.
event CreateMerkleInstant(
ISablierMerkleInstant indexed merkleInstant,
Expand Down Expand Up @@ -68,9 +71,6 @@ interface ISablierMerkleFactory is IAdminable {
/// @notice Emitted when the default fee is set by the admin.
event SetDefaultFee(address indexed admin, uint256 defaultFee);

/// @notice Emitted when the fees are claimed by the Sablier admin.
event WithdrawFees(address indexed admin, ISablierMerkleBase indexed merkleBase, address to, uint256 fees);

/*//////////////////////////////////////////////////////////////////////////
CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/
Expand All @@ -96,6 +96,15 @@ interface ISablierMerkleFactory is IAdminable {
NON-CONSTANT FUNCTIONS
//////////////////////////////////////////////////////////////////////////*/

/// @notice Collects the fees accrued in the `merkleBase` contract, and transfers them to the factory admin.
/// @dev Emits a {CollectFees} event.
///
/// Notes:
/// - If the admin is a contract, it must be able to receive ETH.
///
/// @param merkleBase The address of the Merkle contract where the fees are collected from.
function collectFees(ISablierMerkleBase merkleBase) external;

/// @notice Creates a new MerkleInstant campaign for instant distribution of tokens.
///
/// @dev Emits a {CreateMerkleInstant} event.
Expand Down Expand Up @@ -214,18 +223,4 @@ interface ISablierMerkleFactory is IAdminable {
///
/// @param defaultFee The new default fee to be set.
function setDefaultFee(uint256 defaultFee) external;

/// @notice Withdraws the fees accrued in the `merkleBase` contract.
/// @dev Emits a {WithdrawFees} event.
///
/// Notes:
/// - This function transfers ETH to the provided address. If the receiver is a contract, it must be able to receive
/// ETH.
///
/// Requirements:
/// - `msg.sender` must be the admin.
/// - `to` must not be the zero address.
///
/// @param to The address to transfer the fees to.
function withdrawFees(address payable to, ISablierMerkleBase merkleBase) external;
}
8 changes: 1 addition & 7 deletions src/libraries/Errors.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,6 @@ library Errors {

error CallerNotAdmin(address admin, address caller);

/*//////////////////////////////////////////////////////////////////////////
SABLIER-MERKLE-FACTORY
//////////////////////////////////////////////////////////////////////////*/

error SablierMerkleFactory_WithdrawToZeroAddress();

/*//////////////////////////////////////////////////////////////////////////
SABLIER-MERKLE-BASE
//////////////////////////////////////////////////////////////////////////*/
Expand All @@ -34,7 +28,7 @@ library Errors {
error SablierMerkleBase_ClawbackNotAllowed(uint256 blockTimestamp, uint40 expiration, uint40 firstClaimTime);

/// @notice Thrown if the fees withdrawal failed.
error SablierMerkleBase_FeeWithdrawFailed(address to, uint256 amount);
error SablierMerkleBase_FeeTransferFail(address factoryAdmin, uint256 feeAmount);

/// @notice Thrown when trying to claim with an insufficient fee payment.
error SablierMerkleBase_InsufficientFeePayment(uint256 feePaid, uint256 fee);
Expand Down
2 changes: 1 addition & 1 deletion tests/Base.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ abstract contract Base_Test is Assertions, Constants, DeployOptimized, Modifiers
deployProtocolConditionally();

// Set the default fee on the Merkle factory.
merkleFactory.setDefaultFee(defaults.DEFAULT_FEE());
merkleFactory.setDefaultFee(defaults.FEE());

// Create users for testing.
users.campaignOwner = createUser("CampaignOwner");
Expand Down
20 changes: 6 additions & 14 deletions tests/fork/merkle-campaign/MerkleInstant.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ abstract contract MerkleInstant_Fork_Test is Fork_Test {
// Make the campaign owner as the caller.
resetPrank({ msgSender: params.campaignOwner });

uint256 fee = defaults.DEFAULT_FEE();
uint256 fee = defaults.FEE();

vars.expectedMerkleInstant = computeMerkleInstantAddress(
params.campaignOwner, params.campaignOwner, FORK_TOKEN, vars.merkleRoot, params.expiration, fee
Expand Down Expand Up @@ -215,22 +215,14 @@ abstract contract MerkleInstant_Fork_Test is Fork_Test {
}

/*//////////////////////////////////////////////////////////////////////////
WITHDRAW-FEE
COLLECT-FEES
//////////////////////////////////////////////////////////////////////////*/

// Make the factory admin as the caller.
resetPrank({ msgSender: users.admin });

vm.expectEmit({ emitter: address(merkleFactory) });
emit ISablierMerkleFactory.WithdrawFees({
admin: users.admin,
merkleBase: vars.merkleInstant,
to: users.admin,
fees: fee
});
merkleFactory.withdrawFees({ to: payable(users.admin), merkleBase: vars.merkleInstant });
emit ISablierMerkleFactory.CollectFees({ admin: users.admin, merkleBase: vars.merkleInstant, feeAmount: fee });
merkleFactory.collectFees({ merkleBase: vars.merkleInstant });

assertEq(address(vars.merkleInstant).balance, 0, "merkle lockup ether balance");
assertEq(users.admin.balance, fee, "admin ether balance");
assertEq(address(vars.merkleInstant).balance, 0, "merkleInstant ETH balance");
assertEq(users.admin.balance, fee, "admin ETH balance");
}
}
20 changes: 6 additions & 14 deletions tests/fork/merkle-campaign/MerkleLL.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ abstract contract MerkleLL_Fork_Test is Fork_Test {
// Make the campaign owner as the caller.
resetPrank({ msgSender: params.campaignOwner });

uint256 fee = defaults.DEFAULT_FEE();
uint256 fee = defaults.FEE();

vars.expectedLL = computeMerkleLLAddress(
params.campaignOwner, params.campaignOwner, FORK_TOKEN, vars.merkleRoot, params.expiration, fee
Expand Down Expand Up @@ -244,22 +244,14 @@ abstract contract MerkleLL_Fork_Test is Fork_Test {
}

/*//////////////////////////////////////////////////////////////////////////
WITHDRAW-FEE
COLLECT-FEES
//////////////////////////////////////////////////////////////////////////*/

// Make the factory admin as the caller.
resetPrank({ msgSender: users.admin });

vm.expectEmit({ emitter: address(merkleFactory) });
emit ISablierMerkleFactory.WithdrawFees({
admin: users.admin,
merkleBase: vars.merkleLL,
to: users.admin,
fees: fee
});
merkleFactory.withdrawFees({ to: payable(users.admin), merkleBase: vars.merkleLL });
emit ISablierMerkleFactory.CollectFees({ admin: users.admin, merkleBase: vars.merkleLL, feeAmount: fee });
merkleFactory.collectFees({ merkleBase: vars.merkleLL });

assertEq(address(vars.merkleLL).balance, 0, "merkle lockup ether balance");
assertEq(users.admin.balance, fee, "admin ether balance");
assertEq(address(vars.merkleLL).balance, 0, "merkleLL ETH balance");
assertEq(users.admin.balance, fee, "admin ETH balance");
}
}
20 changes: 6 additions & 14 deletions tests/fork/merkle-campaign/MerkleLT.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ abstract contract MerkleLT_Fork_Test is Fork_Test {
// Make the campaign owner as the caller.
resetPrank({ msgSender: params.campaignOwner });

uint256 fee = defaults.DEFAULT_FEE();
uint256 fee = defaults.FEE();

vars.expectedLT = computeMerkleLTAddress(
params.campaignOwner, params.campaignOwner, FORK_TOKEN, vars.merkleRoot, params.expiration, fee
Expand Down Expand Up @@ -246,22 +246,14 @@ abstract contract MerkleLT_Fork_Test is Fork_Test {
}

/*//////////////////////////////////////////////////////////////////////////
WITHDRAW-FEE
COLLECT-FEES
//////////////////////////////////////////////////////////////////////////*/

// Make the factory admin as the caller.
resetPrank({ msgSender: users.admin });

vm.expectEmit({ emitter: address(merkleFactory) });
emit ISablierMerkleFactory.WithdrawFees({
admin: users.admin,
merkleBase: vars.merkleLT,
to: users.admin,
fees: fee
});
merkleFactory.withdrawFees({ to: payable(users.admin), merkleBase: vars.merkleLT });
emit ISablierMerkleFactory.CollectFees({ admin: users.admin, merkleBase: vars.merkleLT, feeAmount: fee });
merkleFactory.collectFees({ merkleBase: vars.merkleLT });

assertEq(address(vars.merkleLT).balance, 0, "merkle lockup ether balance");
assertEq(users.admin.balance, fee, "admin ether balance");
assertEq(address(vars.merkleLT).balance, 0, "merkleLT ETH balance");
assertEq(users.admin.balance, fee, "admin ETH balance");
}
}
8 changes: 4 additions & 4 deletions tests/integration/Integration.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ contract Integration_Test is Base_Test {
//////////////////////////////////////////////////////////////////////////*/

function claim() internal {
merkleBase.claim{ value: defaults.DEFAULT_FEE() }({
merkleBase.claim{ value: defaults.FEE() }({
index: defaults.INDEX1(),
recipient: users.recipient1,
amount: defaults.CLAIM_AMOUNT(),
Expand All @@ -65,7 +65,7 @@ contract Integration_Test is Base_Test {
//////////////////////////////////////////////////////////////////////////*/

function computeMerkleInstantAddress(address campaignOwner, uint40 expiration) internal view returns (address) {
return computeMerkleInstantAddress(campaignOwner, expiration, defaults.DEFAULT_FEE());
return computeMerkleInstantAddress(campaignOwner, expiration, defaults.FEE());
}

function computeMerkleInstantAddress(
Expand Down Expand Up @@ -112,7 +112,7 @@ contract Integration_Test is Base_Test {
//////////////////////////////////////////////////////////////////////////*/

function computeMerkleLLAddress(address campaignOwner, uint40 expiration) internal view returns (address) {
return computeMerkleLLAddress(campaignOwner, expiration, defaults.DEFAULT_FEE());
return computeMerkleLLAddress(campaignOwner, expiration, defaults.FEE());
}

function computeMerkleLLAddress(
Expand Down Expand Up @@ -163,7 +163,7 @@ contract Integration_Test is Base_Test {
//////////////////////////////////////////////////////////////////////////*/

function computeMerkleLTAddress(address campaignOwner, uint40 expiration) internal view returns (address) {
return computeMerkleLTAddress(campaignOwner, expiration, defaults.DEFAULT_FEE());
return computeMerkleLTAddress(campaignOwner, expiration, defaults.FEE());
}

function computeMerkleLTAddress(
Expand Down
Loading

0 comments on commit 87b4af2

Please sign in to comment.